From 85dd939f19376045a99dc3ede6dd18287240f338 Mon Sep 17 00:00:00 2001 From: teddy Date: Tue, 23 Jul 2024 15:20:48 -0300 Subject: [PATCH] feat: btt tests for push/pull underlying (#173) * test: btt tests for push/pull underlying * chore: remove preexisting unit tests replaced by ones in this pr * fix: safeERC20 returns different errors with vs without return data * fix: finalize test that was broken on main * fix: feedback from review --- test/unit/BPool.t.sol | 58 ----------------- test/unit/BPool/BPool.t.sol | 124 +++++++++++++++++++++++++++++++++++- test/unit/BPool/BPool.tree | 24 +++++++ 3 files changed, 147 insertions(+), 59 deletions(-) diff --git a/test/unit/BPool.t.sol b/test/unit/BPool.t.sol index 5e1f0abc..a3d752d8 100644 --- a/test/unit/BPool.t.sol +++ b/test/unit/BPool.t.sol @@ -495,61 +495,3 @@ contract BPool_Unit_ExitswapExternAmountOut is BasePoolTest { bPool.exitswapExternAmountOut(tokenOut, _fuzz.tokenAmountOut, type(uint256).max); } } - -contract BPool_Unit__PullUnderlying is BasePoolTest { - function test_Call_TransferFrom(address _erc20, address _from, uint256 _amount) public { - assumeNotForgeAddress(_erc20); - - vm.mockCall( - _erc20, abi.encodeWithSelector(IERC20.transferFrom.selector, _from, address(bPool), _amount), abi.encode(true) - ); - - vm.expectCall(address(_erc20), abi.encodeWithSelector(IERC20.transferFrom.selector, _from, address(bPool), _amount)); - bPool.call__pullUnderlying(_erc20, _from, _amount); - } - - function test_Revert_ERC20False(address _erc20, address _from, uint256 _amount) public { - assumeNotForgeAddress(_erc20); - vm.mockCall( - _erc20, abi.encodeWithSelector(IERC20.transferFrom.selector, _from, address(bPool), _amount), abi.encode(false) - ); - - vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, _erc20)); - bPool.call__pullUnderlying(_erc20, _from, _amount); - } - - function test_Success_NoReturnValueERC20(address _erc20, address _from, uint256 _amount) public { - assumeNotForgeAddress(_erc20); - vm.mockCall( - _erc20, abi.encodeWithSelector(IERC20.transferFrom.selector, _from, address(bPool), _amount), abi.encode() - ); - - bPool.call__pullUnderlying(_erc20, _from, _amount); - } -} - -contract BPool_Unit__PushUnderlying is BasePoolTest { - function test_Call_Transfer(address _erc20, address _to, uint256 _amount) public { - assumeNotForgeAddress(_erc20); - - vm.mockCall(_erc20, abi.encodeWithSelector(IERC20.transfer.selector, _to, _amount), abi.encode(true)); - - vm.expectCall(address(_erc20), abi.encodeWithSelector(IERC20.transfer.selector, _to, _amount)); - bPool.call__pushUnderlying(_erc20, _to, _amount); - } - - function test_Revert_ERC20False(address _erc20, address _to, uint256 _amount) public { - assumeNotForgeAddress(_erc20); - vm.mockCall(_erc20, abi.encodeWithSelector(IERC20.transfer.selector, _to, _amount), abi.encode(false)); - - vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, _erc20)); - bPool.call__pushUnderlying(_erc20, _to, _amount); - } - - function test_Success_NoReturnValueERC20(address _erc20, address _to, uint256 _amount) public { - assumeNotForgeAddress(_erc20); - vm.mockCall(_erc20, abi.encodeWithSelector(IERC20.transfer.selector, _to, _amount), abi.encode()); - - bPool.call__pushUnderlying(_erc20, _to, _amount); - } -} diff --git a/test/unit/BPool/BPool.t.sol b/test/unit/BPool/BPool.t.sol index 7b1f2ec3..07954497 100644 --- a/test/unit/BPool/BPool.t.sol +++ b/test/unit/BPool/BPool.t.sol @@ -4,6 +4,8 @@ pragma solidity 0.8.25; import {BPoolBase} from './BPoolBase.sol'; import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; +import {SafeERC20} from '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol'; +import {Address} from '@openzeppelin/contracts/utils/Address.sol'; import {BMath} from 'contracts/BMath.sol'; import {IBPool} from 'interfaces/IBPool.sol'; @@ -26,6 +28,10 @@ contract BPool is BPoolBase, BMath { // sPf = (10 / 20) * (1 / (1-0.1)) = 0.555...e18 (round-up) uint256 public spotPriceWithoutFee = 0.5e18; uint256 public spotPrice = 0.555555555555555556e18; + address public transferRecipient = makeAddr('transfer recipient'); + address public transferFromSpender = makeAddr('transfer from spender'); + address public transferredToken = makeAddr('underlying token'); + uint256 public transferredAmount = 10e18; function setUp() public virtual override { super.setUp(); @@ -370,7 +376,7 @@ contract BPool is BPoolBase, BMath { } function test_FinalizeRevertWhen_CallerIsNotController(address _caller) external { - vm.assume(_caller != address(this)); + vm.assume(_caller != controller); vm.startPrank(_caller); // it should revert vm.expectRevert(IBPool.BPool_CallerIsNotController.selector); @@ -419,4 +425,120 @@ contract BPool is BPoolBase, BMath { // it finalizes the pool assertEq(bPool.call__finalized(), true); } + + function test__pushUnderlyingRevertWhen_UnderlyingTokenReturnsFalse() external { + vm.mockCall( + transferredToken, + abi.encodeWithSelector(IERC20.transfer.selector, transferRecipient, transferredAmount), + abi.encode(false) + ); + // it should revert + vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, transferredToken)); + bPool.call__pushUnderlying(transferredToken, transferRecipient, transferredAmount); + } + + function test__pushUnderlyingWhenUnderlyingTokenDoesntReturnAValue() external { + vm.mockCall( + transferredToken, + abi.encodeWithSelector(IERC20.transfer.selector, transferRecipient, transferredAmount), + abi.encode() + ); + // it assumes transfer success + bPool.call__pushUnderlying(transferredToken, transferRecipient, transferredAmount); + } + + function test__pushUnderlyingRevertWhen_UnderlyingTokenRevertsWithoutData() external { + // it should revert with FailedInnerCall + vm.mockCallRevert( + transferredToken, + abi.encodeWithSelector(IERC20.transfer.selector, transferRecipient, transferredAmount), + abi.encode() + ); + vm.expectRevert(Address.FailedInnerCall.selector); + bPool.call__pushUnderlying(transferredToken, transferRecipient, transferredAmount); + } + + function test__pushUnderlyingRevertWhen_UnderlyingTokenRevertsWithData(bytes memory errorData) external { + vm.assume(keccak256(errorData) != keccak256(bytes(''))); + vm.mockCallRevert( + transferredToken, + abi.encodeWithSelector(IERC20.transfer.selector, transferRecipient, transferredAmount), + errorData + ); + // it should revert with same error data + vm.expectRevert(errorData); + bPool.call__pushUnderlying(transferredToken, transferRecipient, transferredAmount); + } + + function test__pushUnderlyingWhenUnderlyingTokenReturnsTrue() external { + vm.mockCall( + transferredToken, + abi.encodeWithSelector(IERC20.transfer.selector, transferRecipient, transferredAmount), + abi.encode(true) + ); + // it calls underlying transfer + vm.expectCall( + transferredToken, abi.encodeWithSelector(IERC20.transfer.selector, transferRecipient, transferredAmount) + ); + bPool.call__pushUnderlying(transferredToken, transferRecipient, transferredAmount); + } + + function test__pullUnderlyingRevertWhen_UnderlyingTokenReturnsFalse() external { + vm.mockCall( + transferredToken, + abi.encodeWithSelector(IERC20.transferFrom.selector, transferFromSpender, address(bPool), transferredAmount), + abi.encode(false) + ); + // it should revert + vm.expectRevert(abi.encodeWithSelector(SafeERC20.SafeERC20FailedOperation.selector, transferredToken)); + bPool.call__pullUnderlying(transferredToken, transferFromSpender, transferredAmount); + } + + function test__pullUnderlyingWhenUnderlyingTokenDoesntReturnAValue() external { + // it assumes transferFrom success + vm.mockCall( + transferredToken, + abi.encodeWithSelector(IERC20.transferFrom.selector, transferFromSpender, address(bPool), transferredAmount), + abi.encode() + ); + // it assumes transferFrom success + bPool.call__pullUnderlying(transferredToken, transferFromSpender, transferredAmount); + } + + function test__pullUnderlyingRevertWhen_UnderlyingTokenRevertsWithoutData() external { + vm.mockCallRevert( + transferredToken, + abi.encodeWithSelector(IERC20.transferFrom.selector, transferFromSpender, address(bPool), transferredAmount), + abi.encode() + ); + // it should revert with FailedInnerCall + vm.expectRevert(Address.FailedInnerCall.selector); + bPool.call__pullUnderlying(transferredToken, transferFromSpender, transferredAmount); + } + + function test__pullUnderlyingRevertWhen_UnderlyingTokenRevertsWithData(bytes memory errorData) external { + vm.assume(keccak256(errorData) != keccak256(bytes(''))); + vm.mockCallRevert( + transferredToken, + abi.encodeWithSelector(IERC20.transferFrom.selector, transferFromSpender, address(bPool), transferredAmount), + errorData + ); + // it should revert with same error data + vm.expectRevert(errorData); + bPool.call__pullUnderlying(transferredToken, transferFromSpender, transferredAmount); + } + + function test__pullUnderlyingWhenUnderlyingTokenReturnsTrue() external { + vm.mockCall( + transferredToken, + abi.encodeWithSelector(IERC20.transferFrom.selector, transferFromSpender, address(bPool), transferredAmount), + abi.encode(true) + ); + // it calls underlying transferFrom + vm.expectCall( + transferredToken, + abi.encodeWithSelector(IERC20.transferFrom.selector, transferFromSpender, address(bPool), transferredAmount) + ); + bPool.call__pullUnderlying(transferredToken, transferFromSpender, transferredAmount); + } } diff --git a/test/unit/BPool/BPool.tree b/test/unit/BPool/BPool.tree index 7a034e3a..85659d4c 100644 --- a/test/unit/BPool/BPool.tree +++ b/test/unit/BPool/BPool.tree @@ -141,3 +141,27 @@ BPool::finalize ├── it mints initial pool shares ├── it sends initial pool shares to controller └── it calls _afterFinalize hook + +BPool::_pushUnderlying +├── when underlying token returns false +│ └── it should revert +├── when underlying token doesnt return a value +│ └── it assumes transfer success +├── when underlying token reverts without data +│ └── it should revert // FailedInnerCall +├── when underlying token reverts with data +│ └── it should revert +└── when underlying token returns true + └── it calls underlying transfer + +BPool::_pullUnderlying +├── when underlying token returns false +│ └── it should revert +├── when underlying token doesnt return a value +│ └── it assumes transferFrom success +├── when underlying token reverts without data +│ └── it should revert // FailedInnerCall +├── when underlying token reverts with data +│ └── it should revert +└── when underlying token returns true + └── it calls underlying transferFrom