From 6be8b1c5fdbe37d641453c27a61864d029753f23 Mon Sep 17 00:00:00 2001 From: eboado Date: Wed, 28 Oct 2020 10:54:09 +0100 Subject: [PATCH 1/2] - Added validations on ReserveConfiguration's numeric setters. --- .../configuration/ReserveConfiguration.sol | 21 ++++++- contracts/libraries/helpers/Errors.sol | 9 +++ helpers/types.ts | 6 ++ test/configurator.spec.ts | 58 ++++++++++++++++++- 4 files changed, 91 insertions(+), 3 deletions(-) diff --git a/contracts/libraries/configuration/ReserveConfiguration.sol b/contracts/libraries/configuration/ReserveConfiguration.sol index 553cec66..ccef1e00 100644 --- a/contracts/libraries/configuration/ReserveConfiguration.sol +++ b/contracts/libraries/configuration/ReserveConfiguration.sol @@ -1,6 +1,8 @@ // SPDX-License-Identifier: agpl-3.0 pragma solidity ^0.6.8; +import {Errors} from '../helpers/Errors.sol'; + /** * @title ReserveConfiguration library * @author Aave @@ -27,6 +29,12 @@ library ReserveConfiguration { uint256 constant STABLE_BORROWING_ENABLED_START_BIT_POSITION = 59; uint256 constant RESERVE_FACTOR_START_BIT_POSITION = 64; + uint256 constant MAX_VALID_LTV = 65535; + uint256 constant MAX_VALID_LIQUIDATION_THRESHOLD = 65535; + uint256 constant MAX_VALID_LIQUIDATION_BONUS = 65535; + uint256 constant MAX_VALID_DECIMALS = 255; + uint256 constant MAX_VALID_RESERVE_FACTOR = 65535; + struct Map { //bit 0-15: LTV //bit 16-31: Liq. threshold @@ -47,6 +55,8 @@ library ReserveConfiguration { * @param ltv the new ltv **/ function setLtv(ReserveConfiguration.Map memory self, uint256 ltv) internal pure { + require(ltv <= MAX_VALID_LTV, Errors.INVALID_LTV); + self.data = (self.data & LTV_MASK) | ltv; } @@ -68,6 +78,8 @@ library ReserveConfiguration { internal pure { + require(threshold <= MAX_VALID_LIQUIDATION_THRESHOLD, Errors.INVALID_LIQ_THRESHOLD); + self.data = (self.data & LIQUIDATION_THRESHOLD_MASK) | (threshold << LIQUIDATION_THRESHOLD_START_BIT_POSITION); @@ -92,6 +104,8 @@ library ReserveConfiguration { * @param bonus the new liquidation bonus **/ function setLiquidationBonus(ReserveConfiguration.Map memory self, uint256 bonus) internal pure { + require(bonus <= MAX_VALID_LIQUIDATION_BONUS, Errors.INVALID_LIQ_BONUS); + self.data = (self.data & LIQUIDATION_BONUS_MASK) | (bonus << LIQUIDATION_BONUS_START_BIT_POSITION); @@ -116,6 +130,8 @@ library ReserveConfiguration { * @param decimals the decimals **/ function setDecimals(ReserveConfiguration.Map memory self, uint256 decimals) internal pure { + require(decimals <= MAX_VALID_DECIMALS, Errors.INVALID_DECIMALS); + self.data = (self.data & DECIMALS_MASK) | (decimals << RESERVE_DECIMALS_START_BIT_POSITION); } @@ -225,6 +241,8 @@ library ReserveConfiguration { internal pure { + require(reserveFactor <= MAX_VALID_RESERVE_FACTOR, Errors.INVALID_RESERVE_FACTOR); + self.data = (self.data & RESERVE_FACTOR_MASK) | (reserveFactor << RESERVE_FACTOR_START_BIT_POSITION); @@ -316,8 +334,7 @@ library ReserveConfiguration { ); } - - /** + /** * @dev gets the configuration flags of the reserve from a memory object * @param self the reserve configuration * @return the state flags representing active, freezed, borrowing enabled, stableRateBorrowing enabled diff --git a/contracts/libraries/helpers/Errors.sol b/contracts/libraries/helpers/Errors.sol index 91dd385f..9ed7b07e 100644 --- a/contracts/libraries/helpers/Errors.sol +++ b/contracts/libraries/helpers/Errors.sol @@ -89,6 +89,15 @@ library Errors { // pausable error message string public constant IS_PAUSED = '58'; // 'Pool is paused' + + // Reserve configuration + string public constant INVALID_LTV = '70'; + string public constant INVALID_LIQ_THRESHOLD = '71'; + string public constant INVALID_LIQ_BONUS = '72'; + string public constant INVALID_DECIMALS = '73'; + string public constant INVALID_RESERVE_FACTOR = '74'; + + enum CollateralManagerErrors { NO_ERROR, NO_COLLATERAL_AVAILABLE, diff --git a/helpers/types.ts b/helpers/types.ts index 305ec335..944c2be0 100644 --- a/helpers/types.ts +++ b/helpers/types.ts @@ -117,6 +117,12 @@ export enum ProtocolErrors { IS_PAUSED = '58', // Pool is paused + INVALID_LTV = '70', + INVALID_LIQ_THRESHOLD = '71', + INVALID_LIQ_BONUS = '72', + INVALID_DECIMALS = '73', + INVALID_RESERVE_FACTOR = '74', + // old INVALID_FROM_BALANCE_AFTER_TRANSFER = 'Invalid from balance after transfer', diff --git a/test/configurator.spec.ts b/test/configurator.spec.ts index 891c88dc..788add5c 100644 --- a/test/configurator.spec.ts +++ b/test/configurator.spec.ts @@ -10,7 +10,63 @@ const APPROVAL_AMOUNT_LENDING_POOL = const {expect} = require('chai'); makeSuite('LendingPoolConfigurator', (testEnv: TestEnv) => { - const {CALLER_NOT_AAVE_ADMIN, RESERVE_LIQUIDITY_NOT_0} = ProtocolErrors; + const { + CALLER_NOT_AAVE_ADMIN, + RESERVE_LIQUIDITY_NOT_0, + INVALID_LTV, + INVALID_LIQ_THRESHOLD, + INVALID_LIQ_BONUS, + INVALID_DECIMALS, + INVALID_RESERVE_FACTOR, + } = ProtocolErrors; + + it('Reverts trying to set an invalid LTV', async () => { + const {configurator, weth} = testEnv; + + const invalidLtv = 65536; + + await expect(configurator.setLtv(weth.address, invalidLtv)).to.be.revertedWith(INVALID_LTV); + }); + + it('Reverts trying to set an invalid liquidation threshold', async () => { + const {configurator, weth} = testEnv; + + const invalidLiqThreshold = 65536; + + await expect( + configurator.setLiquidationThreshold(weth.address, invalidLiqThreshold) + ).to.be.revertedWith(INVALID_LIQ_THRESHOLD); + }); + + it('Reverts trying to set an invalid liquidation bonus', async () => { + const {configurator, weth} = testEnv; + + const invalidLiqBonus = 65536; + + await expect( + configurator.setLiquidationBonus(weth.address, invalidLiqBonus) + ).to.be.revertedWith(INVALID_LIQ_BONUS); + }); + + it('Reverts trying to set an invalid reserve decimals', async () => { + const {configurator, weth} = testEnv; + + const invalidDecimals = 256; + + await expect(configurator.setReserveDecimals(weth.address, invalidDecimals)).to.be.revertedWith( + INVALID_DECIMALS + ); + }); + + it('Reverts trying to set an invalid reserve factor', async () => { + const {configurator, weth} = testEnv; + + const invalidReserveFactor = 65536; + + await expect( + configurator.setReserveFactor(weth.address, invalidReserveFactor) + ).to.be.revertedWith(INVALID_RESERVE_FACTOR); + }); it('Deactivates the ETH reserve', async () => { const {configurator, weth, helpersContract} = testEnv; From 6cd18c4320dd072e9ac77bfb4528ef48bd345df1 Mon Sep 17 00:00:00 2001 From: eboado Date: Thu, 29 Oct 2020 12:09:07 +0100 Subject: [PATCH 2/2] - Simplified getter of boolean fields on ReserveConfiguration. - Added all 256 bits to masks on ReserveConfiguration for clarity. --- .../configuration/ReserveConfiguration.sol | 43 +++++++++---------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/contracts/libraries/configuration/ReserveConfiguration.sol b/contracts/libraries/configuration/ReserveConfiguration.sol index ccef1e00..ce4f722a 100644 --- a/contracts/libraries/configuration/ReserveConfiguration.sol +++ b/contracts/libraries/configuration/ReserveConfiguration.sol @@ -9,15 +9,15 @@ import {Errors} from '../helpers/Errors.sol'; * @notice Implements the bitmap logic to handle the reserve configuration */ library ReserveConfiguration { - uint256 constant LTV_MASK = 0xFFFFFFFFFFFFFFFF0000; - uint256 constant LIQUIDATION_THRESHOLD_MASK = 0xFFFFFFFFFFFF0000FFFF; - uint256 constant LIQUIDATION_BONUS_MASK = 0xFFFFFFFF0000FFFFFFFF; - uint256 constant DECIMALS_MASK = 0xFFFFFF00FFFFFFFFFFFF; - uint256 constant ACTIVE_MASK = 0xFFFFFEFFFFFFFFFFFFFF; - uint256 constant FROZEN_MASK = 0xFFFFFDFFFFFFFFFFFFFF; - uint256 constant BORROWING_MASK = 0xFFFFFBFFFFFFFFFFFFFF; - uint256 constant STABLE_BORROWING_MASK = 0xFFFFF7FFFFFFFFFFFFFF; - uint256 constant RESERVE_FACTOR_MASK = 0xFFFFFFFFFFFFFFFF; + uint256 constant LTV_MASK = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000; // prettier-ignore + uint256 constant LIQUIDATION_THRESHOLD_MASK = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000FFFF; // prettier-ignore + uint256 constant LIQUIDATION_BONUS_MASK = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000FFFFFFFF; // prettier-ignore + uint256 constant DECIMALS_MASK = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00FFFFFFFFFFFF; // prettier-ignore + uint256 constant ACTIVE_MASK = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEFFFFFFFFFFFFFF; // prettier-ignore + uint256 constant FROZEN_MASK = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFDFFFFFFFFFFFFFF; // prettier-ignore + uint256 constant BORROWING_MASK = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFBFFFFFFFFFFFFFF; // prettier-ignore + uint256 constant STABLE_BORROWING_MASK = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF7FFFFFFFFFFFFFF; // prettier-ignore + uint256 constant RESERVE_FACTOR_MASK = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF0000FFFFFFFFFFFFFFFF; // prettier-ignore /// @dev For the LTV, the start bit is 0 (up to 15), but we don't declare it as for 0 no bit movement is needed uint256 constant LIQUIDATION_THRESHOLD_START_BIT_POSITION = 16; @@ -161,7 +161,7 @@ library ReserveConfiguration { * @return the active state **/ function getActive(ReserveConfiguration.Map storage self) internal view returns (bool) { - return ((self.data & ~ACTIVE_MASK) >> IS_ACTIVE_START_BIT_POSITION) != 0; + return (self.data & ~ACTIVE_MASK) != 0; } /** @@ -181,7 +181,7 @@ library ReserveConfiguration { * @return the frozen state **/ function getFrozen(ReserveConfiguration.Map storage self) internal view returns (bool) { - return ((self.data & ~FROZEN_MASK) >> IS_FROZEN_START_BIT_POSITION) != 0; + return (self.data & ~FROZEN_MASK) != 0; } /** @@ -201,7 +201,7 @@ library ReserveConfiguration { * @return the borrowing state **/ function getBorrowingEnabled(ReserveConfiguration.Map storage self) internal view returns (bool) { - return ((self.data & ~BORROWING_MASK) >> BORROWING_ENABLED_START_BIT_POSITION) != 0; + return (self.data & ~BORROWING_MASK) != 0; } /** @@ -228,8 +228,7 @@ library ReserveConfiguration { view returns (bool) { - return - ((self.data & ~STABLE_BORROWING_MASK) >> STABLE_BORROWING_ENABLED_START_BIT_POSITION) != 0; + return (self.data & ~STABLE_BORROWING_MASK) != 0; } /** @@ -275,10 +274,10 @@ library ReserveConfiguration { uint256 dataLocal = self.data; return ( - (dataLocal & ~ACTIVE_MASK) >> IS_ACTIVE_START_BIT_POSITION != 0, - (dataLocal & ~FROZEN_MASK) >> IS_FROZEN_START_BIT_POSITION != 0, - (dataLocal & ~BORROWING_MASK) >> BORROWING_ENABLED_START_BIT_POSITION != 0, - (dataLocal & ~STABLE_BORROWING_MASK) >> STABLE_BORROWING_ENABLED_START_BIT_POSITION != 0 + (dataLocal & ~ACTIVE_MASK) != 0, + (dataLocal & ~FROZEN_MASK) != 0, + (dataLocal & ~BORROWING_MASK) != 0, + (dataLocal & ~STABLE_BORROWING_MASK) != 0 ); } @@ -350,10 +349,10 @@ library ReserveConfiguration { ) { return ( - (self.data & ~ACTIVE_MASK) >> IS_ACTIVE_START_BIT_POSITION != 0, - (self.data & ~FROZEN_MASK) >> IS_FROZEN_START_BIT_POSITION != 0, - (self.data & ~BORROWING_MASK) >> BORROWING_ENABLED_START_BIT_POSITION != 0, - (self.data & ~STABLE_BORROWING_MASK) >> STABLE_BORROWING_ENABLED_START_BIT_POSITION != 0 + (self.data & ~ACTIVE_MASK) != 0, + (self.data & ~FROZEN_MASK) != 0, + (self.data & ~BORROWING_MASK) != 0, + (self.data & ~STABLE_BORROWING_MASK) != 0 ); } }