Skip to content

Commit

Permalink
chore: address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
halaprix committed Nov 22, 2023
1 parent 5e0a940 commit ee72ff9
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 50 deletions.
42 changes: 24 additions & 18 deletions contracts/commands/AaveV3BasicBuyCommandV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,19 @@ import { IOperationExecutor, Call } from "../interfaces/IOperationExecutor.sol";
import { BaseDMACommand, ICommand } from "./BaseDMACommand.sol";

struct BasicBuyTriggerData {
address positionAddress;
uint16 triggerType;
uint256 maxCoverage;
address debtToken;
address collateralToken;
bytes32 operationHash;
uint256 executionLTV;
uint256 targetLTV;
uint256 maxBuyPrice;
uint64 deviation;
uint32 maxBaseFeeInGwei;
/* start of common V2 TriggerData parameters */
address positionAddress; // Address of the position - dpm proxy
uint16 triggerType; // Type of trigger
uint256 maxCoverage; // Maximum coverage amount - max amount of additional debt taken to cover execution gas fee
address debtToken; // Address of the debt token
address collateralToken; // Address of the collateral token
bytes32 operationHash; // Hash of the operation execution operation TODO
/* end of common V2 TriggerData parameters */
uint256 executionLTV; // Execution loan-to-value ratio
uint256 targetLTV; // Target loan-to-value ratio
uint256 maxBuyPrice; // maximum buy price
uint64 deviation; // Deviation from target LTV after execution - eg 50 corresponds to 0.5%
uint32 maxBaseFeeInGwei; // Maximum base fee in Gwei
}

/**
Expand Down Expand Up @@ -80,7 +82,6 @@ contract AaveV3BasicBuyCommandV2 is BaseDMACommand {
revert EmptyAddress("price oracle");
}
priceOracle = IPriceOracleGetter(priceOracleAddress);
self = address(this);
}

/**
Expand Down Expand Up @@ -118,12 +119,12 @@ contract AaveV3BasicBuyCommandV2 is BaseDMACommand {
triggerData,
(BasicBuyTriggerData)
);
validateTriggerType(basicBuyTriggerData.triggerType, AAVE_V3_BASIC_BUY_TRIGGER_TYPE);
_validateTriggerType(basicBuyTriggerData.triggerType, AAVE_V3_BASIC_BUY_TRIGGER_TYPE);
AaveV3BasicBuyCommandV2(self).validateoperationHash(
executionData,
basicBuyTriggerData.operationHash
);
validateSelector(operationExecutor.executeOp.selector, executionData);
_validateSelector(operationExecutor.executeOp.selector, executionData);
IAccountImplementation(basicBuyTriggerData.positionAddress).execute(
address(operationExecutor),
executionData
Expand All @@ -134,7 +135,7 @@ contract AaveV3BasicBuyCommandV2 is BaseDMACommand {
* @inheritdoc ICommand
*/
function isTriggerDataValid(
bool omitted,
bool,
bytes memory triggerData
) external view override returns (bool) {
BasicBuyTriggerData memory basicBuyTriggerData = abi.decode(
Expand All @@ -154,11 +155,14 @@ contract AaveV3BasicBuyCommandV2 is BaseDMACommand {
// assure that the execution LTV is lower than the lower target, so it wont execute again
bool executionLtvBelowLowerTarget = basicBuyTriggerData.executionLTV < lowerBoundTarget;
// assure that the trigger type is the correct one
bool triggerTypeCorrect = basicBuyTriggerData.triggerType == AAVE_V3_BASIC_BUY_TRIGGER_TYPE;
bool triggerTypeCorrect = _isTriggerTypeValid(
basicBuyTriggerData.triggerType,
AAVE_V3_BASIC_BUY_TRIGGER_TYPE
);
// assure that the upper bound of target LTV is lower than the max LTV, it would revert on execution
bool upperTargetBelowMaxLtv = upperBoundTarget < maxLtv;
// assure that the deviation is valid ( above minimal allowed deviation)
bool deviationValid = deviationIsValid(basicBuyTriggerData.deviation);
bool deviationValid = _isDeviationValid(basicBuyTriggerData.deviation);
return
executionLtvBelowLowerTarget &&
triggerTypeCorrect &&
Expand All @@ -181,6 +185,8 @@ contract AaveV3BasicBuyCommandV2 is BaseDMACommand {
(uint256 totalCollateralBase, uint256 totalDebtBase, , , uint256 maxLtv, ) = lendingPool
.getUserAccountData(basicBuyTriggerData.positionAddress);

/* if there is no debt or colalteral we skip the checks - it will happen eg if the user closed
the position but havent removed the trigger */
if (totalCollateralBase == 0 || totalDebtBase == 0) {
return false;
}
Expand All @@ -203,7 +209,7 @@ contract AaveV3BasicBuyCommandV2 is BaseDMACommand {
bool priceBelowMax = currentPrice <= basicBuyTriggerData.maxBuyPrice;

// blocks base fee has to be below the limit set by the user (maxBaseFeeInGwei)
bool baseFeeValid = baseFeeIsValid(basicBuyTriggerData.maxBaseFeeInGwei);
bool baseFeeValid = _isBaseFeeValid(basicBuyTriggerData.maxBaseFeeInGwei);

// upper bound of target LTV after execution has to be below the max LTV
// it is checked again as maxLtv might have changed since the trigger was created
Expand Down
56 changes: 33 additions & 23 deletions contracts/commands/AaveV3BasicSellCommandV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,23 @@ import { IAccountImplementation } from "../interfaces/IAccountImplementation.sol
import { IOperationExecutor, Call } from "../interfaces/IOperationExecutor.sol";
import { BaseDMACommand, ICommand } from "./BaseDMACommand.sol";

/**
* @dev Struct representing the data for a basic sell trigger.
*/
struct BasicSellTriggerData {
address positionAddress;
uint16 triggerType;
uint256 maxCoverage;
address debtToken;
address collateralToken;
bytes32 operationHash;
uint256 execLTV;
uint256 targetLTV;
uint256 minSellPrice;
uint64 deviation;
uint32 maxBaseFeeInGwei;
/* start of common V2 TriggerData parameters */
address positionAddress; // Address of the position - dpm proxy
uint16 triggerType; // Type of trigger
uint256 maxCoverage; // Maximum coverage amount - max amount of additional debt taken to cover execution gas fee
address debtToken; // Address of the debt token
address collateralToken; // Address of the collateral token
bytes32 operationHash; // Hash of the operation execution operation TODO
/* end of common V2 TriggerData parameters */
uint256 executionLtv; // Execution loan-to-value ratio
uint256 targetLTV; // Target loan-to-value ratio
uint256 minSellPrice; // Minimum sell price
uint64 deviation; // Deviation from target LTV after execution - eg 50 corresponds to 0.5%
uint32 maxBaseFeeInGwei; // Maximum base fee in Gwei
}

contract AaveV3BasicSellCommandV2 is BaseDMACommand {
Expand Down Expand Up @@ -75,7 +80,6 @@ contract AaveV3BasicSellCommandV2 is BaseDMACommand {
revert EmptyAddress("price oracle");
}
priceOracle = IPriceOracleGetter(priceOracleAddress);
self = address(this);
}

/**
Expand All @@ -90,6 +94,10 @@ contract AaveV3BasicSellCommandV2 is BaseDMACommand {
(uint256 totalCollateralBase, uint256 totalDebtBase, , , , ) = lendingPool
.getUserAccountData(basicSellTriggerData.positionAddress);

/* Calculate the loan-to-value (LTV) ratio for Aave V3
LTV is the ratio of the total debt to the total collateral, expressed as a percentage
The result is multiplied by 10000 to preserve precision
eg 0.67 (67%) LTV is stored as 6700 */
uint256 ltv = (totalDebtBase * 10000) / totalCollateralBase;

(uint256 lowerBoundTarget, uint256 upperBoundTarget) = basicSellTriggerData
Expand All @@ -113,12 +121,12 @@ contract AaveV3BasicSellCommandV2 is BaseDMACommand {
triggerData,
(BasicSellTriggerData)
);
validateTriggerType(basicSellTriggerData.triggerType, AAVE_V3_BASIC_SELL_TRIGGER_TYPE);
_validateTriggerType(basicSellTriggerData.triggerType, AAVE_V3_BASIC_SELL_TRIGGER_TYPE);
AaveV3BasicSellCommandV2(self).validateoperationHash(
executionData,
basicSellTriggerData.operationHash
);
validateSelector(operationExecutor.executeOp.selector, executionData);
_validateSelector(operationExecutor.executeOp.selector, executionData);
IAccountImplementation(basicSellTriggerData.positionAddress).execute(
address(operationExecutor),
executionData
Expand All @@ -129,7 +137,7 @@ contract AaveV3BasicSellCommandV2 is BaseDMACommand {
* @inheritdoc ICommand
*/
function isTriggerDataValid(
bool omitted,
bool,
bytes memory triggerData
) external view override returns (bool) {
BasicSellTriggerData memory basicSellTriggerData = abi.decode(
Expand All @@ -144,14 +152,16 @@ contract AaveV3BasicSellCommandV2 is BaseDMACommand {
);

// assure that the execution LTV is higher or equal than the upper bound target, so it wont execute again
bool executionLtvAboveUpperTarget = basicSellTriggerData.execLTV >= upperBoundTarget;
bool executionLtvAboveUpperTarget = basicSellTriggerData.executionLtv >= upperBoundTarget;
// assure that the trigger type is the correct one
bool triggerTypeCorrect = basicSellTriggerData.triggerType ==
AAVE_V3_BASIC_SELL_TRIGGER_TYPE;
bool triggerTypeCorrect = _isTriggerTypeValid(
basicSellTriggerData.triggerType,
AAVE_V3_BASIC_SELL_TRIGGER_TYPE
);
// assure the execution LTV is lower than max LTV
bool executionLtvBelowMaxLtv = basicSellTriggerData.execLTV < maxLtv;
bool executionLtvBelowMaxLtv = basicSellTriggerData.executionLtv < maxLtv;
// assure that the deviation is valid ( above minimal allowe deviation)
bool deviationValid = deviationIsValid(basicSellTriggerData.deviation);
bool deviationValid = _isDeviationValid(basicSellTriggerData.deviation);
return
executionLtvAboveUpperTarget &&
triggerTypeCorrect &&
Expand Down Expand Up @@ -186,16 +196,16 @@ contract AaveV3BasicSellCommandV2 is BaseDMACommand {
uint256 currentPrice = priceOracle.getAssetPrice(basicSellTriggerData.collateralToken);

// LTV has to be below or equal the execution LTV set by the user to execute
bool ltvAboveExecution = ltv >= basicSellTriggerData.execLTV;
bool ltvAboveExecution = ltv >= basicSellTriggerData.executionLtv;

// oracle price has to be above the minSellPirce set by the user
bool priceAboveMin = currentPrice >= basicSellTriggerData.minSellPrice;

// blocks base fee has to be below the limit set by the user (maxBaseFeeInGwei)
bool baseFeeValid = baseFeeIsValid(basicSellTriggerData.maxBaseFeeInGwei);
bool baseFeeValid = _isBaseFeeValid(basicSellTriggerData.maxBaseFeeInGwei);

// is execution LTV lower than max LTV - we revert early if that's not true
bool executionLtvBelowMaxLtv = basicSellTriggerData.execLTV < maxLtv;
bool executionLtvBelowMaxLtv = basicSellTriggerData.executionLtv < maxLtv;

return ltvAboveExecution && priceAboveMin && baseFeeValid && executionLtvBelowMaxLtv;
}
Expand Down
45 changes: 36 additions & 9 deletions contracts/commands/BaseDMACommand.sol
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,15 @@ abstract contract BaseDMACommand is ReentrancyGuard, ICommand {
if (weth == address(0)) {
revert EmptyAddress("weth");
}
self = address(this);
}

/**
* @dev Checks if the provided deviation is valid.
* @param deviation The deviation value to check.
* @return A boolean indicating whether the deviation is valid or not.
*/
function deviationIsValid(uint256 deviation) internal pure returns (bool) {
function _isDeviationValid(uint256 deviation) internal pure returns (bool) {
return deviation >= MIN_ALLOWED_DEVIATION;
}

Expand All @@ -98,33 +99,59 @@ abstract contract BaseDMACommand is ReentrancyGuard, ICommand {
* @param maxAcceptableBaseFeeInGwei The maximum acceptable base fee in Gwei.
* @return A boolean indicating whether the base fee is valid or not.
*/
function baseFeeIsValid(uint256 maxAcceptableBaseFeeInGwei) internal view returns (bool) {
return block.basefee <= maxAcceptableBaseFeeInGwei * (10 ** 9);
function _isBaseFeeValid(uint256 maxAcceptableBaseFeeInGwei) internal view returns (bool) {
return block.basefee <= maxAcceptableBaseFeeInGwei * 1 gwei;
}

/**
* @dev Validates the trigger type.
* @dev Validates the trigger type. Reverts if that's not the case
* @param triggerType The actual trigger type.
* @param expectedTriggerType The expected trigger type.
*/
function validateTriggerType(uint16 triggerType, uint16 expectedTriggerType) internal pure {
if (triggerType != expectedTriggerType) {
function _validateTriggerType(uint16 triggerType, uint16 expectedTriggerType) internal pure {
if (!_isTriggerTypeValid(triggerType, expectedTriggerType)) {
revert InvalidTriggerType(triggerType);
}
}

/**
* @dev Validates the selector.
* @dev Checks if the given trigger type is valid.
* @param triggerType The trigger type to check.
* @param expectedTriggerType The expected trigger type.
* @return A boolean indicating whether the trigger type is valid or not.
*/
function _isTriggerTypeValid(
uint16 triggerType,
uint16 expectedTriggerType
) internal pure returns (bool) {
return triggerType != expectedTriggerType;
}

/**
* @dev Validates the selector. Reverts in case it is not.
* @param expectedSelector The expected selector.
* @param executionData The execution data containing the selector.
*/
function validateSelector(bytes4 expectedSelector, bytes memory executionData) public pure {
function _validateSelector(bytes4 expectedSelector, bytes memory executionData) internal pure {
bytes4 selector = abi.decode(executionData, (bytes4));
if (selector != expectedSelector) {
if (_isSelectorValid(expectedSelector, selector)) {
revert InvalidSelector(selector);
}
}

/**
* @dev Checks if the given selector is valid by comparing it with the expected selector.
* @param expectedSelector The expected selector.
* @param selector The selector to be checked.
* @return A boolean indicating whether the selector is valid or not.
*/
function _isSelectorValid(
bytes4 expectedSelector,
bytes4 selector
) internal pure returns (bool) {
return selector != expectedSelector;
}

/**
* @dev Validates the operation hash.
* @param _data The operation executor execution data containing the operation hash.
Expand Down

0 comments on commit ee72ff9

Please sign in to comment.