From ee1e20568b4d62c20ec3eb2cb083967d75e9f277 Mon Sep 17 00:00:00 2001 From: The3D <frangellaemilio@gmail.com> Date: Tue, 10 Nov 2020 14:11:01 +0100 Subject: [PATCH 1/2] Added check --- contracts/libraries/helpers/Errors.sol | 2 +- contracts/libraries/logic/ValidationLogic.sol | 7 ++++--- helpers/types.ts | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/contracts/libraries/helpers/Errors.sol b/contracts/libraries/helpers/Errors.sol index 43bf1a5b..2dff6ced 100644 --- a/contracts/libraries/helpers/Errors.sol +++ b/contracts/libraries/helpers/Errors.sol @@ -22,7 +22,7 @@ library Errors { string public constant BORROW_ALLOWANCE_NOT_ENOUGH = '59'; // User borrows on behalf, but allowance are too small //contract specific errors - string public constant VL_AMOUNT_NOT_GREATER_THAN_0 = '1'; // 'Amount must be greater than 0' + string public constant VL_INVALID_AMOUNT = '1'; // 'Amount must be greater than 0' string public constant VL_NO_ACTIVE_RESERVE = '2'; // 'Action requires an active reserve' string public constant VL_RESERVE_FROZEN = '3'; // 'Action cannot be performed because the reserve is frozen' string public constant VL_CURRENT_AVAILABLE_LIQUIDITY_NOT_ENOUGH = '4'; // 'The current liquidity is not enough' diff --git a/contracts/libraries/logic/ValidationLogic.sol b/contracts/libraries/logic/ValidationLogic.sol index 9ee38df3..a08c5420 100644 --- a/contracts/libraries/logic/ValidationLogic.sol +++ b/contracts/libraries/logic/ValidationLogic.sol @@ -36,7 +36,7 @@ library ValidationLogic { function validateDeposit(ReserveLogic.ReserveData storage reserve, uint256 amount) external view { (bool isActive, bool isFrozen, , ) = reserve.configuration.getFlags(); - require(amount > 0, Errors.VL_AMOUNT_NOT_GREATER_THAN_0); + require(amount != 0, Errors.VL_INVALID_AMOUNT); require(isActive, Errors.VL_NO_ACTIVE_RESERVE); require(!isFrozen, Errors.VL_RESERVE_FROZEN); } @@ -62,7 +62,7 @@ library ValidationLogic { uint256 reservesCount, address oracle ) external view { - require(amount > 0, Errors.VL_AMOUNT_NOT_GREATER_THAN_0); + require(amount != 0, Errors.VL_INVALID_AMOUNT); require(amount <= userBalance, Errors.VL_NOT_ENOUGH_AVAILABLE_USER_BALANCE); @@ -139,6 +139,7 @@ library ValidationLogic { require(vars.isActive, Errors.VL_NO_ACTIVE_RESERVE); require(!vars.isFrozen, Errors.VL_RESERVE_FROZEN); + require(amount != 0, Errors.VL_INVALID_AMOUNT); require(vars.borrowingEnabled, Errors.VL_BORROWING_NOT_ENABLED); @@ -232,7 +233,7 @@ library ValidationLogic { require(isActive, Errors.VL_NO_ACTIVE_RESERVE); - require(amountSent > 0, Errors.VL_AMOUNT_NOT_GREATER_THAN_0); + require(amountSent > 0, Errors.VL_INVALID_AMOUNT); require( (stableDebt > 0 && diff --git a/helpers/types.ts b/helpers/types.ts index df6f4afc..75e64f5b 100644 --- a/helpers/types.ts +++ b/helpers/types.ts @@ -83,7 +83,7 @@ export enum ProtocolErrors { CALLER_NOT_POOL_ADMIN = '33', // 'The caller must be the pool admin' //contract specific errors - VL_AMOUNT_NOT_GREATER_THAN_0 = '1', // 'Amount must be greater than 0' + VL_INVALID_AMOUNT = '1', // 'Amount must be greater than 0' VL_NO_ACTIVE_RESERVE = '2', // 'Action requires an active reserve' VL_RESERVE_FROZEN = '3', // 'Action requires an unfrozen reserve' VL_CURRENT_AVAILABLE_LIQUIDITY_NOT_ENOUGH = '4', // 'The current liquidity is not enough' From c81047ca93bcd93e74ef768aa928e83d79ba87fb Mon Sep 17 00:00:00 2001 From: The3D <frangellaemilio@gmail.com> Date: Tue, 10 Nov 2020 14:57:09 +0100 Subject: [PATCH 2/2] Fixes #123 --- .../LendingPoolAddressesProvider.sol | 28 +++++---- .../ILendingPoolAddressesProvider.sol | 6 +- test/lending-pool-addresses-provider.spec.ts | 62 ++++++++++++------- 3 files changed, 61 insertions(+), 35 deletions(-) diff --git a/contracts/configuration/LendingPoolAddressesProvider.sol b/contracts/configuration/LendingPoolAddressesProvider.sol index ee4435bc..43796d36 100644 --- a/contracts/configuration/LendingPoolAddressesProvider.sol +++ b/contracts/configuration/LendingPoolAddressesProvider.sol @@ -28,24 +28,30 @@ contract LendingPoolAddressesProvider is Ownable, ILendingPoolAddressesProvider bytes32 private constant LENDING_RATE_ORACLE = 'LENDING_RATE_ORACLE'; /** - * @dev Sets an address for an id, allowing to cover it or not with a proxy + * @dev Sets an address for an id by updating a proxy implementation * @param id The id - * @param newAddress The address to set, pass address(0) if a proxy is needed * @param implementationAddress The address of the implementation if we want it covered by a proxy * address(0) if we don't want a proxy covering */ - function setAddress( + function setAddressAsProxy( bytes32 id, - address newAddress, address implementationAddress ) external override onlyOwner { - if (implementationAddress != address(0)) { - _updateImpl(id, implementationAddress); - emit AddressSet(id, implementationAddress, true); - } else { - _addresses[id] = newAddress; - emit AddressSet(id, newAddress, false); - } + _updateImpl(id, implementationAddress); + emit AddressSet(id, implementationAddress, true); + } + + /** + * @dev Sets an address for an id replacing the address saved in the addresses map + * @param id The id + * @param newAddress The address to set, pass address(0) if a proxy is needed + */ + function setAddress( + bytes32 id, + address newAddress + ) external override onlyOwner { + _addresses[id] = newAddress; + emit AddressSet(id, newAddress, false); } /** diff --git a/contracts/interfaces/ILendingPoolAddressesProvider.sol b/contracts/interfaces/ILendingPoolAddressesProvider.sol index c0d180e3..c1a7ab78 100644 --- a/contracts/interfaces/ILendingPoolAddressesProvider.sol +++ b/contracts/interfaces/ILendingPoolAddressesProvider.sol @@ -20,7 +20,11 @@ interface ILendingPoolAddressesProvider { function setAddress( bytes32 id, - address newAddress, + address newAddress + ) external; + + function setAddressAsProxy( + bytes32 id, address impl ) external; diff --git a/test/lending-pool-addresses-provider.spec.ts b/test/lending-pool-addresses-provider.spec.ts index ac70c6a0..97327d4f 100644 --- a/test/lending-pool-addresses-provider.spec.ts +++ b/test/lending-pool-addresses-provider.spec.ts @@ -31,28 +31,61 @@ makeSuite('LendingPoolAddressesProvider', (testEnv: TestEnv) => { await expect( addressesProvider.setAddress( utils.keccak256(utils.toUtf8Bytes('RANDOM_ID')), - mockAddress, - ZERO_ADDRESS + mockAddress ) ).to.be.revertedWith(INVALID_OWNER_REVERT_MSG); + + await expect( + addressesProvider.setAddressAsProxy( + utils.keccak256(utils.toUtf8Bytes('RANDOM_ID')), + mockAddress + ) + ).to.be.revertedWith(INVALID_OWNER_REVERT_MSG); + }); - it('Tests adding both a proxied and non-proxied addres with `setAddress()`', async () => { + it('Tests adding a proxied address with `setAddressAsProxy()`', async () => { const {addressesProvider, users} = testEnv; const {INVALID_OWNER_REVERT_MSG} = ProtocolErrors; const currentAddressesProviderOwner = users[1]; - const mockNonProxiedAddress = createRandomAddress(); - const nonProxiedAddressId = utils.keccak256(utils.toUtf8Bytes('RANDOM_NON_PROXIED')); const mockLendingPool = await deployLendingPool(); const proxiedAddressId = utils.keccak256(utils.toUtf8Bytes('RANDOM_PROXIED')); + const proxiedAddressSetReceipt = await waitForTx( + await addressesProvider + .connect(currentAddressesProviderOwner.signer) + .setAddressAsProxy(proxiedAddressId, mockLendingPool.address) + ); + + if (!proxiedAddressSetReceipt.events || proxiedAddressSetReceipt.events?.length < 1) { + throw new Error('INVALID_EVENT_EMMITED'); + } + + expect(proxiedAddressSetReceipt.events[0].event).to.be.equal('ProxyCreated'); + expect(proxiedAddressSetReceipt.events[1].event).to.be.equal('AddressSet'); + expect(proxiedAddressSetReceipt.events[1].args?.id).to.be.equal(proxiedAddressId); + expect(proxiedAddressSetReceipt.events[1].args?.newAddress).to.be.equal( + mockLendingPool.address + ); + expect(proxiedAddressSetReceipt.events[1].args?.hasProxy).to.be.equal(true); + }); + + + it('Tests adding a non proxied address with `setAddress()`', async () => { + const {addressesProvider, users} = testEnv; + const {INVALID_OWNER_REVERT_MSG} = ProtocolErrors; + + const currentAddressesProviderOwner = users[1]; + const mockNonProxiedAddress = createRandomAddress(); + const nonProxiedAddressId = utils.keccak256(utils.toUtf8Bytes('RANDOM_NON_PROXIED')); + const nonProxiedAddressSetReceipt = await waitForTx( await addressesProvider .connect(currentAddressesProviderOwner.signer) - .setAddress(nonProxiedAddressId, mockNonProxiedAddress, ZERO_ADDRESS) + .setAddress(nonProxiedAddressId, mockNonProxiedAddress) ); expect(mockNonProxiedAddress.toLowerCase()).to.be.equal( @@ -70,22 +103,5 @@ makeSuite('LendingPoolAddressesProvider', (testEnv: TestEnv) => { ); expect(nonProxiedAddressSetReceipt.events[0].args?.hasProxy).to.be.equal(false); - const proxiedAddressSetReceipt = await waitForTx( - await addressesProvider - .connect(currentAddressesProviderOwner.signer) - .setAddress(proxiedAddressId, ZERO_ADDRESS, mockLendingPool.address) - ); - - if (!proxiedAddressSetReceipt.events || proxiedAddressSetReceipt.events?.length < 2) { - throw new Error('INVALID_EVENT_EMMITED'); - } - - expect(proxiedAddressSetReceipt.events[0].event).to.be.equal('ProxyCreated'); - expect(proxiedAddressSetReceipt.events[1].event).to.be.equal('AddressSet'); - expect(proxiedAddressSetReceipt.events[1].args?.id).to.be.equal(proxiedAddressId); - expect(proxiedAddressSetReceipt.events[1].args?.newAddress).to.be.equal( - mockLendingPool.address - ); - expect(proxiedAddressSetReceipt.events[1].args?.hasProxy).to.be.equal(true); }); });