Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

AaveV3LeverageStrategyExtension audit feedback #142

Merged
merged 29 commits into from
Aug 14, 2023
Merged

AaveV3LeverageStrategyExtension audit feedback #142

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
f3a2d4b
Test showing that settting Emode does not affect getReserveConfigurat…
ckoopmann Jun 15, 2023
77244d3
Start adjusting StrategyExtension test to switcht to wsteth/eth pair
ckoopmann Jun 16, 2023
51c978f
Fixing tests
ckoopmann Jun 19, 2023
c29adfd
Fix test
ckoopmann Jun 21, 2023
bebb8bb
First test case for rebalancing in e-mode
ckoopmann Jun 21, 2023
5526d84
Fix handling of emode ltv / liquidationThreshold
ckoopmann Jun 21, 2023
aa7afc5
Add test case for wrong repay threshold
ckoopmann Jun 22, 2023
dba1428
Fix tests
ckoopmann Jun 22, 2023
718dd42
Remove subtraction of unutilizedLeveragePercentage when delevering
ckoopmann Jun 22, 2023
f2eacaa
Add test for targetLeverageRatio verification
ckoopmann Jun 27, 2023
eeb34fb
Add verification step for targetLeverageRatio >= 1
ckoopmann Jun 27, 2023
9d04e7b
Switch div / mul order in _calculateMinRepayUnits
ckoopmann Jun 27, 2023
0e291b4
Switch div / mul order in calculateChunkRebalanceNotional
ckoopmann Jun 27, 2023
7ab6d28
Switch to latestRoundData on chainlink call
ckoopmann Jun 27, 2023
0e10200
Add check for maximum oracle price age
ckoopmann Jun 27, 2023
7e3f87a
Add tests for outdated price response from oracle
ckoopmann Jun 27, 2023
332caf2
Fix tests
ckoopmann Jun 27, 2023
edbe0b0
Add method to override noRebalanceInProgress modifier
ckoopmann Jul 4, 2023
d9a653f
Fix tests
ckoopmann Jul 4, 2023
bed0e34
Use AaveOracle instead of configured chainlink oracles
ckoopmann Jul 31, 2023
87f4723
Merge branch 'master' into aave-v3-leverage-strategy-extension-audit-…
pblivin0x Aug 1, 2023
5d1cc59
Get AaveOracle address from AddressProvider instead of deploy argument
ckoopmann Aug 2, 2023
140f09b
Merge branch 'aave-v3-leverage-strategy-extension-audit-feedback' of …
ckoopmann Aug 2, 2023
0a69def
Change to get aaveOracle address on the fly from addressProvider
ckoopmann Aug 2, 2023
3b5c204
Clean up and additional code comments
ckoopmann Aug 2, 2023
998fd21
fix(dependencies): Add post audit Aave V3 dependencies (#145)
pblivin0x Aug 11, 2023
8ef6b7b
Updates to test (#146)
snake-poison Aug 14, 2023
125323c
remove .only from test
snake-poison Aug 14, 2023
0bc8b24
test: fix issues with updating block.
snake-poison Aug 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions contracts/adapters/AaveV3LeverageStrategyExtension.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import { Math } from "@openzeppelin/contracts/math/Math.sol";

import {AaveLeverageStrategyExtension} from "./AaveLeverageStrategyExtension.sol";
import {IBaseManager} from "../interfaces/IBaseManager.sol";
import { IChainlinkAggregatorV3 } from "../interfaces/IChainlinkAggregatorV3.sol";
import { IAaveOracle } from "../interfaces/IAaveOracle.sol";
import {IPool} from "../interfaces/IPool.sol";
import {IPoolAddressesProvider} from "../interfaces/IPoolAddressesProvider.sol";
import { DataTypes } from "../interfaces/Datatypes.sol";
Expand All @@ -42,9 +42,11 @@ contract AaveV3LeverageStrategyExtension is AaveLeverageStrategyExtension {
IPoolAddressesProvider public lendingPoolAddressesProvider;
uint256 public maxOraclePriceAge;
bool public overrideNoRebalanceInProgress;
IAaveOracle public aaveOracle;

constructor(
IBaseManager _manager,
IAaveOracle _aaveOracle,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The aaveOracle should come from calling getPriceOracle on the LendingPoolAddressProvider

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, thanks. Will adjust 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adjusted in:
5d1cc59

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact I went one step further and just read the address anew from the addressProvider everytime when I call the oracle.
Thereby we don't get rugged if for some reason the AaveOracle address changes:

0a69def

ContractSettings memory _strategy,
MethodologySettings memory _methodology,
ExecutionSettings memory _execution,
Expand All @@ -67,6 +69,7 @@ contract AaveV3LeverageStrategyExtension is AaveLeverageStrategyExtension {
{
lendingPoolAddressesProvider = _lendingPoolAddressesProvider;
maxOraclePriceAge = _maxOraclePriceAge;
aaveOracle = _aaveOracle;
}

/* ============ Modifiers ============ */
Expand Down Expand Up @@ -195,11 +198,11 @@ contract AaveV3LeverageStrategyExtension is AaveLeverageStrategyExtension {
function _createActionInfo() internal view override returns(ActionInfo memory) {
ActionInfo memory rebalanceInfo;

// Calculate prices from chainlink. Chainlink returns prices with 8 decimal places, but we need 36 - underlyingDecimals decimal places.
// Calculate prices from chainlink. AaveOracle returns prices with 8 decimal places, but we need 36 - underlyingDecimals decimal places.
// This is so that when the underlying amount is multiplied by the received price, the collateral valuation is normalized to 36 decimals.
// To perform this adjustment, we multiply by 10^(36 - 8 - underlyingDecimals)
rebalanceInfo.collateralPrice = _getAssetPrice(strategy.collateralPriceOracle, strategy.collateralDecimalAdjustment);
rebalanceInfo.borrowPrice = _getAssetPrice(strategy.borrowPriceOracle, strategy.borrowDecimalAdjustment);
rebalanceInfo.collateralPrice = _getAssetPrice(strategy.collateralAsset, strategy.collateralDecimalAdjustment);
rebalanceInfo.borrowPrice = _getAssetPrice(strategy.borrowAsset, strategy.borrowDecimalAdjustment);

rebalanceInfo.collateralBalance = strategy.targetCollateralAToken.balanceOf(address(strategy.setToken));
rebalanceInfo.borrowBalance = strategy.targetBorrowDebtToken.balanceOf(address(strategy.setToken));
Expand All @@ -210,12 +213,9 @@ contract AaveV3LeverageStrategyExtension is AaveLeverageStrategyExtension {
return rebalanceInfo;
}

function _getAssetPrice(IChainlinkAggregatorV3 _priceOracle, uint256 _decimalAdjustment) internal view returns (uint256) {
(,int256 rawPrice,, uint256 updatedAt,) = _priceOracle.latestRoundData();
if(maxOraclePriceAge > 0) {
require(updatedAt > block.timestamp.sub(maxOraclePriceAge), "Price data is stale");
}
return rawPrice.toUint256().mul(10 ** _decimalAdjustment);
function _getAssetPrice(address _asset, uint256 _decimalAdjustment) internal view returns (uint256) {
uint256 rawPrice = aaveOracle.getAssetPrice(_asset);
return rawPrice.mul(10 ** _decimalAdjustment);
}

}
2 changes: 1 addition & 1 deletion contracts/interfaces/IAaveOracle.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
pragma solidity ^0.8.10;
pragma solidity 0.6.10;

interface IAaveOracle {
event AssetSourceUpdated(address indexed asset, address indexed source);
Expand Down
24 changes: 4 additions & 20 deletions test/integration/ethereum/aaveV3LeverageStrategyExtension.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ if (process.env.INTEGRATIONTEST) {
maxOraclePriceAge = 600;
leverageStrategyExtension = await deployer.extensions.deployAaveV3LeverageStrategyExtension(
baseManagerV2.address,
aaveOracle.address,
strategy,
methodology,
execution,
Expand All @@ -405,6 +406,7 @@ if (process.env.INTEGRATIONTEST) {

describe("#constructor", async () => {
let subjectManagerAddress: Address;
let subjectAaveOracleAddress: Address;
let subjectContractSettings: AaveContractSettings;
let subjectMethodologySettings: MethodologySettings;
let subjectExecutionSettings: ExecutionSettings;
Expand All @@ -418,6 +420,7 @@ if (process.env.INTEGRATIONTEST) {

beforeEach(async () => {
subjectAaveAddressesProvider = contractAddresses.aaveV3AddressProvider;
subjectAaveOracleAddress = aaveOracle.address;
subjectManagerAddress = baseManagerV2.address;
subjectMaxOraclePriceAge = maxOraclePriceAge;
subjectContractSettings = {
Expand Down Expand Up @@ -466,6 +469,7 @@ if (process.env.INTEGRATIONTEST) {
async function subject(): Promise<AaveV3LeverageStrategyExtension> {
return await deployer.extensions.deployAaveV3LeverageStrategyExtension(
subjectManagerAddress,
subjectAaveOracleAddress,
subjectContractSettings,
subjectMethodologySettings,
subjectExecutionSettings,
Expand Down Expand Up @@ -1748,26 +1752,6 @@ if (process.env.INTEGRATIONTEST) {
await weth.transfer(tradeAdapterMock.address, sendQuantity);
});

describe("when collateralPrice is outdated", async () => {
beforeEach(async () => {
await chainlinkCollateralPriceMock.setPriceAge(maxOraclePriceAge + 1);
});

it("should revert", async () => {
await expect(subject()).to.be.revertedWith("Price data is stale");
});
});

describe("when borrowPrice is outdated", async () => {
beforeEach(async () => {
await chainlinkBorrowPriceMock.setPriceAge(maxOraclePriceAge + 1);
});

it("should revert", async () => {
await expect(subject()).to.be.revertedWith("Price data is stale");
});
});

it("should set the last trade timestamp", async () => {
await subject();

Expand Down
2 changes: 2 additions & 0 deletions utils/deploys/deployExtensions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ export default class DeployExtensions {

public async deployAaveV3LeverageStrategyExtension(
manager: Address,
aaveOracle: Address,
contractSettings: AaveContractSettings,
methdologySettings: MethodologySettings,
executionSettings: ExecutionSettings,
Expand All @@ -380,6 +381,7 @@ export default class DeployExtensions {
): Promise<AaveV3LeverageStrategyExtension> {
return await new AaveV3LeverageStrategyExtension__factory(this._deployerSigner).deploy(
manager,
aaveOracle,
contractSettings,
methdologySettings,
executionSettings,
Expand Down