From 02b964127681b70c025bb1e0ceac9a2f60e3b613 Mon Sep 17 00:00:00 2001 From: Lasse Herskind <16536249+LHerskind@users.noreply.github.com> Date: Thu, 14 Oct 2021 13:58:41 +0100 Subject: [PATCH] fix: Reject deposits directly to implementation --- .../libraries/helpers/StaticATokenErrors.sol | 1 + .../protocol/tokenization/StaticATokenLM.sol | 13 ++++- ...ic-atoken-liquidity-mining-rewards.spec.ts | 31 ++++++++++-- .../static-atoken-liquidity-mining.spec.ts | 45 ++++++++++++++++- ...ic-atoken-liquidity-mining-rewards.spec.ts | 24 +++++++++- .../static-atoken-liquidity-mining.spec.ts | 48 +++++++++++++++++-- 6 files changed, 149 insertions(+), 13 deletions(-) diff --git a/contracts/protocol/libraries/helpers/StaticATokenErrors.sol b/contracts/protocol/libraries/helpers/StaticATokenErrors.sol index 2798207b..6220b5a5 100644 --- a/contracts/protocol/libraries/helpers/StaticATokenErrors.sol +++ b/contracts/protocol/libraries/helpers/StaticATokenErrors.sol @@ -9,4 +9,5 @@ library StaticATokenErrors { string public constant INVALID_RECIPIENT = '5'; string public constant INVALID_CLAIMER = '6'; string public constant ONLY_ONE_AMOUNT_FORMAT_ALLOWED = '7'; + string public constant ONLY_PROXY_MAY_CALL = '8'; } diff --git a/contracts/protocol/tokenization/StaticATokenLM.sol b/contracts/protocol/tokenization/StaticATokenLM.sol index 6fb61a8d..64deb527 100644 --- a/contracts/protocol/tokenization/StaticATokenLM.sol +++ b/contracts/protocol/tokenization/StaticATokenLM.sol @@ -70,6 +70,17 @@ contract StaticATokenLM is // user => unclaimedRewards (in RAYs) mapping(address => uint256) private _unclaimedRewards; + bool public isImplementation; + + modifier onlyProxy() { + require(isImplementation == false, StaticATokenErrors.ONLY_PROXY_MAY_CALL); + _; + } + + constructor() public { + isImplementation = true; + } + ///@inheritdoc VersionedInitializable function getRevision() internal pure virtual override returns (uint256) { return STATIC_ATOKEN_LM_REVISION; @@ -294,7 +305,7 @@ contract StaticATokenLM is uint256 amount, uint16 referralCode, bool fromUnderlying - ) internal returns (uint256) { + ) internal onlyProxy returns (uint256) { require(recipient != address(0), StaticATokenErrors.INVALID_RECIPIENT); _updateRewards(); diff --git a/test-suites/test-aave/mainnet/static-atoken-lm-no-incentives-controller/static-atoken-liquidity-mining-rewards.spec.ts b/test-suites/test-aave/mainnet/static-atoken-lm-no-incentives-controller/static-atoken-liquidity-mining-rewards.spec.ts index cee20bb7..8ba58a39 100644 --- a/test-suites/test-aave/mainnet/static-atoken-lm-no-incentives-controller/static-atoken-liquidity-mining-rewards.spec.ts +++ b/test-suites/test-aave/mainnet/static-atoken-lm-no-incentives-controller/static-atoken-liquidity-mining-rewards.spec.ts @@ -10,6 +10,8 @@ import { WETH9, AToken, StaticATokenLM, + InitializableAdminUpgradeabilityProxyFactory, + StaticAToken, } from '../../../../types'; import { impersonateAccountsHardhat, @@ -27,6 +29,7 @@ import { AbiCoder, formatEther, verifyTypedData } from 'ethers/lib/utils'; import { _TypedDataEncoder } from 'ethers/lib/utils'; import { expect, use } from 'chai'; +import { zeroAddress } from 'hardhat/node_modules/ethereumjs-util'; //use(solidity); @@ -80,6 +83,7 @@ describe('StaticATokenLM: aToken wrapper with static balances and NO liquidity m let enj: ERC20; let aenj: AToken; + let staticATokenImplementation: StaticATokenLM; let staticAToken: StaticATokenLM; let snap: string; @@ -99,8 +103,25 @@ describe('StaticATokenLM: aToken wrapper with static balances and NO liquidity m aenj = ATokenFactory.connect(AENJ, userSigner); stkAave = ERC20Factory.connect(STKAAVE, userSigner); - staticAToken = await new StaticATokenLMFactory(userSigner).deploy(); - await staticAToken.initialize(LENDING_POOL, AENJ, 'Wrapped aENJ', 'waaenj'); + staticATokenImplementation = await new StaticATokenLMFactory(userSigner).deploy(); + await staticATokenImplementation.initialize(LENDING_POOL, AENJ, 'Wrapped aENJ', 'waaenj'); + + const proxy = await new InitializableAdminUpgradeabilityProxyFactory(userSigner).deploy(); + const encodedInitializedParams = staticATokenImplementation.interface.encodeFunctionData( + 'initialize', + [LENDING_POOL, AENJ, 'Wrapped aENJ', 'waaenj'] + ); + + await proxy['initialize(address,address,bytes)']( + staticATokenImplementation.address, + zeroAddress(), + encodedInitializedParams + ); + + staticAToken = StaticATokenLMFactory.connect(proxy.address, userSigner); + + expect(await staticATokenImplementation.isImplementation()).to.be.eq(true); + expect(await staticAToken.isImplementation()).to.be.eq(false); expect(await staticAToken.decimals()).to.be.eq(18); expect(await staticAToken.name()).to.be.eq('Wrapped aENJ'); @@ -264,7 +285,7 @@ describe('StaticATokenLM: aToken wrapper with static balances and NO liquidity m const bGas = BigNumber.from(bReceipt['gasUsed']); expect(aGas).to.be.gt(250000); - expect(bGas).to.be.lt(200000); + expect(bGas).to.be.lt(210000); await DRE.network.provider.send('evm_setAutomine', [true]); await DRE.network.provider.send('evm_mine', []); @@ -296,8 +317,8 @@ describe('StaticATokenLM: aToken wrapper with static balances and NO liquidity m const aGas = BigNumber.from(aReceipt['gasUsed']); const bGas = BigNumber.from(bReceipt['gasUsed']); - expect(aGas).to.be.lt(25000); - expect(bGas).to.be.lt(25000); + expect(aGas).to.be.lt(35000); + expect(bGas).to.be.lt(35000); await DRE.network.provider.send('evm_setAutomine', [true]); }); diff --git a/test-suites/test-aave/mainnet/static-atoken-lm-no-incentives-controller/static-atoken-liquidity-mining.spec.ts b/test-suites/test-aave/mainnet/static-atoken-lm-no-incentives-controller/static-atoken-liquidity-mining.spec.ts index d40b0dbc..16dd03df 100644 --- a/test-suites/test-aave/mainnet/static-atoken-lm-no-incentives-controller/static-atoken-liquidity-mining.spec.ts +++ b/test-suites/test-aave/mainnet/static-atoken-lm-no-incentives-controller/static-atoken-liquidity-mining.spec.ts @@ -14,6 +14,7 @@ import { AToken, StaticAToken, StaticATokenLM, + InitializableAdminUpgradeabilityProxyFactory, } from '../../../../types'; import { IAaveIncentivesControllerFactory } from '../../../../types/IAaveIncentivesControllerFactory'; import { @@ -68,6 +69,7 @@ const LM_ERRORS = { INVALID_RECIPIENT: '5', INVALID_CLAIMER: '6', ONLY_ONE_AMOUNT_FORMAT_ALLOWED: '7', + ONLY_PROXY_MAY_CALL: '8', }; type tBalancesInvolved = { @@ -147,6 +149,7 @@ describe('StaticATokenLM: aToken wrapper with static balances and NO liquidity m let enjWhale: providers.JsonRpcSigner; + let staticATokenImplementation: StaticATokenLM; let staticAToken: StaticATokenLM; let snap: string; @@ -167,8 +170,25 @@ describe('StaticATokenLM: aToken wrapper with static balances and NO liquidity m aenj = ATokenFactory.connect(AENJ, userSigner); stkAave = ERC20Factory.connect(STKAAVE, userSigner); - staticAToken = await new StaticATokenLMFactory(userSigner).deploy(); - await staticAToken.initialize(LENDING_POOL, AENJ, 'Wrapped aENJ', 'waaenj'); + staticATokenImplementation = await new StaticATokenLMFactory(userSigner).deploy(); + await staticATokenImplementation.initialize(LENDING_POOL, AENJ, 'Wrapped aENJ', 'waaenj'); + + const proxy = await new InitializableAdminUpgradeabilityProxyFactory(userSigner).deploy(); + const encodedInitializedParams = staticATokenImplementation.interface.encodeFunctionData( + 'initialize', + [LENDING_POOL, AENJ, 'Wrapped aENJ', 'waaenj'] + ); + + await proxy['initialize(address,address,bytes)']( + staticATokenImplementation.address, + zeroAddress(), + encodedInitializedParams + ); + + staticAToken = StaticATokenLMFactory.connect(proxy.address, userSigner); + + expect(await staticATokenImplementation.isImplementation()).to.be.eq(true); + expect(await staticAToken.isImplementation()).to.be.eq(false); expect(await staticAToken.getIncentivesController()).to.be.eq(zeroAddress()); expect(await staticAToken.ASSET()).to.be.eq(await staticAToken.UNDERLYING_ASSET_ADDRESS()); @@ -199,6 +219,27 @@ describe('StaticATokenLM: aToken wrapper with static balances and NO liquidity m await evmRevert(snap); }); + it('Deposit ENJ directly to implementation (expect revert)', async () => { + const amountToDeposit = utils.parseEther('5'); + const amountToWithdraw = MAX_UINT_AMOUNT; + + // Just preparation + await waitForTx( + await enj.approve(staticATokenImplementation.address, amountToDeposit, defaultTxParams) + ); + + // Depositing + await expect( + staticATokenImplementation.deposit( + userSigner._address, + amountToDeposit, + 0, + true, + defaultTxParams + ) + ).to.be.revertedWith(LM_ERRORS.ONLY_PROXY_MAY_CALL); + }); + it('Deposit ENJ on waaenj, then withdraw of the whole balance in underlying', async () => { const amountToDeposit = utils.parseEther('5'); const amountToWithdraw = MAX_UINT_AMOUNT; diff --git a/test-suites/test-aave/mainnet/static-atoken-lm/static-atoken-liquidity-mining-rewards.spec.ts b/test-suites/test-aave/mainnet/static-atoken-lm/static-atoken-liquidity-mining-rewards.spec.ts index ea30edab..238fd6e3 100644 --- a/test-suites/test-aave/mainnet/static-atoken-lm/static-atoken-liquidity-mining-rewards.spec.ts +++ b/test-suites/test-aave/mainnet/static-atoken-lm/static-atoken-liquidity-mining-rewards.spec.ts @@ -10,6 +10,7 @@ import { WETH9, AToken, StaticATokenLM, + InitializableAdminUpgradeabilityProxyFactory, } from '../../../../types'; import { impersonateAccountsHardhat, @@ -27,6 +28,7 @@ import { AbiCoder, formatEther, verifyTypedData } from 'ethers/lib/utils'; import { _TypedDataEncoder } from 'ethers/lib/utils'; import { expect, use } from 'chai'; +import { zeroAddress } from 'ethereumjs-util'; //use(solidity); @@ -81,6 +83,7 @@ describe('StaticATokenLM: aToken wrapper with static balances and liquidity mini let aweth: AToken; let stkAave: ERC20; + let staticATokenImplementation: StaticATokenLM; let staticAToken: StaticATokenLM; let snap: string; @@ -97,14 +100,31 @@ describe('StaticATokenLM: aToken wrapper with static balances and liquidity mini aweth = ATokenFactory.connect(AWETH, userSigner); stkAave = ERC20Factory.connect(STKAAVE, userSigner); - staticAToken = await new StaticATokenLMFactory(userSigner).deploy(); - await staticAToken.initialize( + staticATokenImplementation = await new StaticATokenLMFactory(userSigner).deploy(); + await staticATokenImplementation.initialize( LENDING_POOL, AWETH, 'Static Aave Interest Bearing WETH', 'stataWETH' ); + const proxy = await new InitializableAdminUpgradeabilityProxyFactory(userSigner).deploy(); + const encodedInitializedParams = staticATokenImplementation.interface.encodeFunctionData( + 'initialize', + [LENDING_POOL, AWETH, 'Static Aave Interest Bearing WETH', 'stataWETH'] + ); + + await proxy['initialize(address,address,bytes)']( + staticATokenImplementation.address, + zeroAddress(), + encodedInitializedParams + ); + + staticAToken = StaticATokenLMFactory.connect(proxy.address, userSigner); + + expect(await staticATokenImplementation.isImplementation()).to.be.eq(true); + expect(await staticAToken.isImplementation()).to.be.eq(false); + expect(await staticAToken.decimals()).to.be.eq(18); expect(await staticAToken.name()).to.be.eq('Static Aave Interest Bearing WETH'); expect(await staticAToken.symbol()).to.be.eq('stataWETH'); diff --git a/test-suites/test-aave/mainnet/static-atoken-lm/static-atoken-liquidity-mining.spec.ts b/test-suites/test-aave/mainnet/static-atoken-lm/static-atoken-liquidity-mining.spec.ts index d5b14dd1..47c6e4ef 100644 --- a/test-suites/test-aave/mainnet/static-atoken-lm/static-atoken-liquidity-mining.spec.ts +++ b/test-suites/test-aave/mainnet/static-atoken-lm/static-atoken-liquidity-mining.spec.ts @@ -14,6 +14,7 @@ import { AToken, StaticAToken, StaticATokenLM, + InitializableAdminUpgradeabilityProxyFactory, } from '../../../../types'; import { IAaveIncentivesControllerFactory } from '../../../../types/IAaveIncentivesControllerFactory'; import { @@ -39,6 +40,7 @@ import { } from '../../../../helpers/contracts-helpers'; import { IAaveIncentivesController } from '../../../../types/IAaveIncentivesController'; import { deploySelfdestructTransferMock } from '../../../../helpers/contracts-deployments'; +import { zeroAddress } from 'ethereumjs-util'; const { expect, use } = require('chai'); @@ -66,6 +68,7 @@ const LM_ERRORS = { INVALID_RECIPIENT: '5', INVALID_CLAIMER: '6', ONLY_ONE_AMOUNT_FORMAT_ALLOWED: '7', + ONLY_PROXY_MAY_CALL: '8', }; type tBalancesInvolved = { @@ -143,6 +146,7 @@ describe('StaticATokenLM: aToken wrapper with static balances and liquidity mini let aweth: AToken; let stkAave: ERC20; + let staticATokenImplementation: StaticATokenLM; let staticAToken: StaticATokenLM; let snap: string; @@ -162,15 +166,31 @@ describe('StaticATokenLM: aToken wrapper with static balances and liquidity mini aweth = ATokenFactory.connect(AWETH, userSigner); stkAave = ERC20Factory.connect(STKAAVE, userSigner); - staticAToken = await new StaticATokenLMFactory(userSigner).deploy(); - await staticAToken.initialize( + staticATokenImplementation = await new StaticATokenLMFactory(userSigner).deploy(); + await staticATokenImplementation.initialize( LENDING_POOL, AWETH, 'Static Aave Interest Bearing WETH', - 'stataAAVE' + 'stataWETH' ); + const proxy = await new InitializableAdminUpgradeabilityProxyFactory(userSigner).deploy(); + const encodedInitializedParams = staticATokenImplementation.interface.encodeFunctionData( + 'initialize', + [LENDING_POOL, AWETH, 'Static Aave Interest Bearing WETH', 'stataWETH'] + ); + + await proxy['initialize(address,address,bytes)']( + staticATokenImplementation.address, + zeroAddress(), + encodedInitializedParams + ); + + staticAToken = StaticATokenLMFactory.connect(proxy.address, userSigner); + expect(await staticAToken.getIncentivesController()).to.be.eq(INCENTIVES_CONTROLLER); + expect(await staticATokenImplementation.isImplementation()).to.be.eq(true); + expect(await staticAToken.isImplementation()).to.be.eq(false); ctxtParams = { staticAToken: staticAToken, @@ -194,6 +214,28 @@ describe('StaticATokenLM: aToken wrapper with static balances and liquidity mini await evmRevert(snap); }); + it('Deposit WETH directly to implementation (expect revert)', async () => { + const amountToDeposit = utils.parseEther('5'); + const amountToWithdraw = MAX_UINT_AMOUNT; + + // Just preparation + await waitForTx(await weth.deposit({ value: amountToDeposit })); + await waitForTx( + await weth.approve(staticATokenImplementation.address, amountToDeposit, defaultTxParams) + ); + + // Depositing + await expect( + staticATokenImplementation.deposit( + userSigner._address, + amountToDeposit, + 0, + true, + defaultTxParams + ) + ).to.be.revertedWith(LM_ERRORS.ONLY_PROXY_MAY_CALL); + }); + it('Deposit WETH on stataWETH, then withdraw of the whole balance in underlying', async () => { const amountToDeposit = utils.parseEther('5'); const amountToWithdraw = MAX_UINT_AMOUNT;