From 390d2107c4f92f33a2f652ecc8b8f252d49ba1d8 Mon Sep 17 00:00:00 2001 From: Lena Hierzi Date: Mon, 2 Oct 2023 15:04:13 +0100 Subject: [PATCH] refactor: remove balances variable, remove withdraw and deposit function, as contract shouldn't be able to hold funds --- contracts/OffsetHelper.sol | 59 ++----------- contracts/OffsetHelperStorage.sol | 3 - test/OffsetHelper.test.ts | 140 ++---------------------------- 3 files changed, 16 insertions(+), 186 deletions(-) diff --git a/contracts/OffsetHelper.sol b/contracts/OffsetHelper.sol index 984217d..15068d6 100644 --- a/contracts/OffsetHelper.sol +++ b/contracts/OffsetHelper.sol @@ -325,7 +325,11 @@ contract OffsetHelper is OffsetHelperStorage { uint256 _amountToOffset ) public returns (address[] memory tco2s, uint256[] memory amounts) { // deposit pool token from user to this contract - deposit(_poolToken, _amountToOffset); + IERC20(_poolToken).safeTransferFrom( + msg.sender, + address(this), + _amountToOffset + ); // redeem BCT / NCT for TCO2s (tco2s, amounts) = autoRedeem(_poolToken, _amountToOffset); @@ -351,7 +355,7 @@ contract OffsetHelper is OffsetHelperStorage { returns (address[] memory tco2s, uint256[] memory amounts) { require( - balances[msg.sender][_fromToken] >= _amount, + IERC20(_fromToken).balanceOf(address(this)) >= _amount, "Insufficient NCT/BCT balance" ); @@ -361,16 +365,6 @@ contract OffsetHelper is OffsetHelperStorage { // auto redeem pool token for TCO2; will transfer automatically picked TCO2 to this contract (tco2s, amounts) = PoolTokenImplementation.redeemAuto2(_amount); - // update balances - balances[msg.sender][_fromToken] -= _amount; - uint256 tco2sLen = tco2s.length; - for (uint256 i; i < tco2sLen; ) { - balances[msg.sender][tco2s[i]] += amounts[i]; - unchecked { - i++; - } - } - emit Redeemed(msg.sender, _fromToken, tco2s, amounts); } @@ -394,12 +388,10 @@ contract OffsetHelper is OffsetHelperStorage { continue; } require( - balances[msg.sender][_tco2s[i]] >= _amounts[i], + IERC20(_tco2s[i]).balanceOf(address(this)) >= _amounts[i], "Insufficient TCO2 balance" ); - balances[msg.sender][_tco2s[i]] -= _amounts[i]; - IToucanCarbonOffsets(_tco2s[i]).retire(_amounts[i]); } } @@ -439,9 +431,6 @@ contract OffsetHelper is OffsetHelperStorage { if (amounts[0] < amountIn) { IERC20(_fromToken).approve(dexRouterAddress, 0); } - - // update balances - balances[msg.sender][_poolToken] += _toAmount; } /** @@ -489,9 +478,6 @@ contract OffsetHelper is OffsetHelperStorage { block.timestamp ); amountOut = amounts[len - 1]; - - // update balances - balances[msg.sender][_poolToken] += amountOut; } /** @@ -524,9 +510,6 @@ contract OffsetHelper is OffsetHelperStorage { require(success, "Failed to send surplus back"); } - - // update balances - balances[msg.sender][_poolToken] += _toAmount; } /** @@ -559,34 +542,6 @@ contract OffsetHelper is OffsetHelperStorage { value: fromAmount }(0, path, address(this), block.timestamp); amountOut = amounts[len - 1]; - - // update balances - balances[msg.sender][_poolToken] += amountOut; - } - - /** - * @notice Allow users to withdraw tokens they have deposited. - */ - function withdraw(address _erc20Addr, uint256 _amount) public { - require( - balances[msg.sender][_erc20Addr] >= _amount, - "Insufficient balance" - ); - - IERC20(_erc20Addr).safeTransfer(msg.sender, _amount); - balances[msg.sender][_erc20Addr] -= _amount; - } - - /** - * @notice Allow users to deposit BCT / NCT. - * @dev Needs to be approved - */ - function deposit( - address _erc20Addr, - uint256 _amount - ) public onlyRedeemable(_erc20Addr) { - IERC20(_erc20Addr).safeTransferFrom(msg.sender, address(this), _amount); - balances[msg.sender][_erc20Addr] += _amount; } // ---------------------------------------- diff --git a/contracts/OffsetHelperStorage.sol b/contracts/OffsetHelperStorage.sol index 8b875ff..ffe1d97 100644 --- a/contracts/OffsetHelperStorage.sol +++ b/contracts/OffsetHelperStorage.sol @@ -15,7 +15,4 @@ contract OffsetHelperStorage is Ownable { string[] public tokenSymbolsForPaths; address[][] public paths; address public dexRouterAddress; - - // user => (token => amount) - mapping(address => mapping(address => uint256)) public balances; } diff --git a/test/OffsetHelper.test.ts b/test/OffsetHelper.test.ts index cbfd9de..a0a48e9 100644 --- a/test/OffsetHelper.test.ts +++ b/test/OffsetHelper.test.ts @@ -137,6 +137,8 @@ describe("OffsetHelper", function () { return { offsetHelper, weth, + nct, + bct, testToken, usdc, owner, @@ -587,7 +589,7 @@ describe("OffsetHelper", function () { }); it(`should redeem ${name.toUpperCase()} from deposit`, async function () { - const { offsetHelper, owner, tokens } = await loadFixture( + const { offsetHelper, owner, bct, nct, tokens } = await loadFixture( deployOffsetHelperFixture ); // extracting the the pool token for this loop @@ -614,8 +616,8 @@ describe("OffsetHelper", function () { await poolToken.token().approve(offsetHelper.address, ONE_ETHER) ).wait(); await ( - await offsetHelper.deposit( - networkPoolAddress[poolToken.name], + await (poolToken.name === "BCT" ? bct : nct).transfer( + offsetHelper.address, ONE_ETHER ) ).wait(); @@ -702,7 +704,7 @@ describe("OffsetHelper", function () { for (const name of TOKEN_POOLS) { it(`should retire using an ${name.toUpperCase()} deposit`, async function () { - const { offsetHelper, owner, tokens } = await loadFixture( + const { offsetHelper, owner, bct, nct, tokens } = await loadFixture( deployOffsetHelperFixture ); // extracting the the pool token for this loop @@ -729,8 +731,8 @@ describe("OffsetHelper", function () { await poolToken.token().approve(offsetHelper.address, ONE_ETHER) ).wait(); await ( - await offsetHelper.deposit( - networkPoolAddress[poolToken.name], + await (poolToken.name === "BCT" ? bct : nct).transfer( + offsetHelper.address, ONE_ETHER ) ).wait(); @@ -835,120 +837,6 @@ describe("OffsetHelper", function () { } }); - describe("#deposit() and #withdraw()", function () { - for (const name of TOKEN_POOLS) { - it(`should fail to deposit because we have no ${name.toUpperCase()}`, async function () { - const { offsetHelper, addrs, tokens } = await loadFixture( - deployOffsetHelperFixture - ); - // extracting the the pool token for this loop - const poolToken = tokens[name]; - - await ( - await poolToken - .token() - .connect(addrs[0]) - .approve(offsetHelper.address, ONE_ETHER) - ).wait(); - - await expect( - offsetHelper - .connect(addrs[0]) - .deposit(networkPoolAddress[poolToken.name], ONE_ETHER) - ).to.be.revertedWith("ERC20: transfer amount exceeds balance"); - }); - - it(`should deposit and withdraw 1.0 ${name.toUpperCase()}`, async function () { - const { offsetHelper, owner, tokens } = await loadFixture( - deployOffsetHelperFixture - ); - const poolToken = tokens[name]; - - const preDepositPoolTokenBalance = await poolToken - .token() - .balanceOf(owner.address); - - await ( - await poolToken.token().approve(offsetHelper.address, ONE_ETHER) - ).wait(); - - await ( - await offsetHelper.deposit( - networkPoolAddress[poolToken.name], - ONE_ETHER - ) - ).wait(); - - await ( - await offsetHelper.withdraw( - networkPoolAddress[poolToken.name], - ONE_ETHER - ) - ).wait(); - - const postWithdrawPoolTokenBalance = await poolToken - .token() - .balanceOf(owner.address); - - expect(formatEther(postWithdrawPoolTokenBalance)).to.be.eql( - formatEther(preDepositPoolTokenBalance) - ); - }); - - it(`should fail to withdraw because we haven't deposited enough ${name.toUpperCase()}`, async function () { - const { offsetHelper, tokens } = await loadFixture( - deployOffsetHelperFixture - ); - const poolToken = tokens[name]; - - await ( - await poolToken.token().approve(offsetHelper.address, ONE_ETHER) - ).wait(); - - await ( - await offsetHelper.deposit( - networkPoolAddress[poolToken.name], - ONE_ETHER - ) - ).wait(); - - await expect( - offsetHelper.withdraw( - networkPoolAddress[poolToken.name], - parseEther("2.0") - ) - ).to.be.revertedWith("Insufficient balance"); - }); - - it(`should deposit 1.0 ${name.toUpperCase()}`, async function () { - const { offsetHelper, owner, tokens } = await loadFixture( - deployOffsetHelperFixture - ); - const poolToken = tokens[name]; - - await ( - await poolToken.token().approve(offsetHelper.address, ONE_ETHER) - ).wait(); - - await ( - await offsetHelper.deposit( - networkPoolAddress[poolToken.name], - ONE_ETHER - ) - ).wait(); - - expect( - formatEther( - await offsetHelper.balances( - owner.address, - networkPoolAddress[poolToken.name] - ) - ) - ).to.be.eql("1.0"); - }); - } - }); - describe("#swapExactOut{ETH,Token}() for pool token", function () { for (const name of TOKEN_POOLS) { it(`should swap native Token, e.g., MATIC for 1.0 ${name.toUpperCase()}`, async function () { @@ -1034,7 +922,7 @@ describe("OffsetHelper", function () { }); it(`should swap WETH for 1.0 ${name.toUpperCase()}`, async function () { - const { offsetHelper, weth, owner, tokens } = await loadFixture( + const { offsetHelper, weth, tokens } = await loadFixture( deployOffsetHelperFixture ); const poolToken = tokens[name]; @@ -1064,16 +952,6 @@ describe("OffsetHelper", function () { expect(formatEther(balance)).to.be.eql( formatEther(initialBalance.add(ONE_ETHER)) ); - - // I expect that the user should have his in-contract balance for pool token to be 1.0 - expect( - formatEther( - await offsetHelper.balances( - owner.address, - networkPoolAddress[poolToken.name] - ) - ) - ).to.be.eql("1.0"); }); } });