From 5321bbf69fa7dfa9f9b51c4fd984319d21153811 Mon Sep 17 00:00:00 2001 From: teddy Date: Tue, 9 Jul 2024 13:45:52 -0300 Subject: [PATCH 01/16] test: btt tests for bpool.swapExactAmountIn --- test/unit/BPool/BPool_SwapExactAmountIn.t.sol | 125 ++++++++++++++++++ test/unit/BPool/BPool_SwapExactAmountIn.tree | 26 ++++ 2 files changed, 151 insertions(+) create mode 100644 test/unit/BPool/BPool_SwapExactAmountIn.t.sol create mode 100644 test/unit/BPool/BPool_SwapExactAmountIn.tree diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol new file mode 100644 index 00000000..a6dcb54f --- /dev/null +++ b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol @@ -0,0 +1,125 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.25; + +import {BPoolBase} from './BPoolBase.sol'; +import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol'; + +import {BNum} from 'contracts/BNum.sol'; +import {IBPool} from 'interfaces/IBPool.sol'; + +contract BPoolSwapExactAmountIn is BPoolBase, BNum { + // Valid scenario + address public tokenIn = token; + uint256 public tokenAmountIn = 3e18; + + uint256 public tokenInBalance = 10e18; + uint256 public tokenOutBalance = 40e18; + // pool is expected to keep 2X the value of tokenIn than tokenOut + uint256 public tokenInWeight = 2e18; + uint256 public tokenOutWeight = 1e18; + + address public tokenOut = secondToken; + // (tokenInBalance / tokenInWeight) / (tokenOutBalance/ tokenOutWeight) + uint256 public spotPriceBeforeSwapWithoutFee = 0.125e18; + uint256 public spotPriceBeforeSwap = bmul(spotPriceBeforeSwapWithoutFee, bdiv(BONE, bsub(BONE, MIN_FEE))); + // from bmath: 40*(1-(10/(10+3*(1-10^-6)))^2) + uint256 public expectedAmountOut = 16.3313500227545254e18; + // (tokenInBalance / tokenInWeight) / (tokenOutBalance/ tokenOutWeight) + // (13 / 2) / (40-expectedAmountOut/ 1) + uint256 public spotPriceAfterSwapWithoutFee = 0.274624873250014625e18; + uint256 public spotPriceAfterSwap = bmul(spotPriceAfterSwapWithoutFee, bdiv(BONE, bsub(BONE, MIN_FEE))); + + function setUp() public virtual override { + super.setUp(); + bPool.set__finalized(true); + address[] memory _tokens = new address[](2); + _tokens[0] = tokenIn; + _tokens[1] = tokenOut; + bPool.set__tokens(_tokens); + _setRecord(tokenIn, IBPool.Record({bound: true, index: 0, denorm: tokenInWeight})); + _setRecord(tokenOut, IBPool.Record({bound: true, index: 1, denorm: tokenOutWeight})); + + vm.mockCall(tokenIn, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(tokenInBalance))); + vm.mockCall(tokenOut, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(tokenOutBalance))); + } + + function test_RevertWhen_ReentrancyLockIsSet() external { + bPool.call__setLock(_MUTEX_TAKEN); + // it should revert + vm.expectRevert(IBPool.BPool_Reentrancy.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceAfterSwap); + } + + function test_RevertWhen_PoolIsNotFinalized() external { + bPool.set__finalized(false); + // it should revert + vm.expectRevert(IBPool.BPool_PoolNotFinalized.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceAfterSwap); + } + + function test_RevertWhen_TokenInIsNotBound() external { + // it should revert + vm.expectRevert(IBPool.BPool_TokenNotBound.selector); + bPool.swapExactAmountIn(makeAddr('unknown token'), tokenAmountIn, tokenOut, expectedAmountOut, spotPriceAfterSwap); + } + + function test_RevertWhen_TokenOutIsNotBound() external { + // it should revert + vm.expectRevert(IBPool.BPool_TokenNotBound.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn, makeAddr('unknown token'), expectedAmountOut, spotPriceAfterSwap); + } + + function test_RevertWhen_TokenInExceedsMaxAllowedRatio(uint256 tokenAmountIn_) external { + tokenAmountIn_ = bound(tokenAmountIn_, bmul(tokenInBalance, MAX_IN_RATIO) + 1, type(uint256).max); + // it should revert + vm.expectRevert(IBPool.BPool_TokenAmountInAboveMaxRatio.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn_, tokenOut, 0, 0); + } + + function test_RevertWhen_SpotPriceBeforeSwapExceedsMaxPrice() external { + // it should revert + vm.expectRevert(IBPool.BPool_SpotPriceAboveMaxPrice.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceBeforeSwap - 1); + } + + function test_RevertWhen_SpotPriceAfterSwapExceedsMaxPrice() external { + // it should revert + vm.expectRevert(IBPool.BPool_SpotPriceAboveMaxPrice.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceAfterSwap - 1); + } + + function test_RevertWhen_ReturnedTokenOutIsLessThanMinAmountOut() external { + // it should revert + vm.expectRevert(IBPool.BPool_TokenAmountOutBelowMinOut.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut + 1, spotPriceAfterSwap); + } + + function test_WhenPreconditionsAreMet() external { + // it sets reentrancy lock + bPool.expectCall__setLock(_MUTEX_TAKEN); + // it calls _pullUnderlying for tokenIn + bPool.mock_call__pullUnderlying(tokenIn, address(this), tokenAmountIn); + bPool.expectCall__pullUnderlying(tokenIn, address(this), tokenAmountIn); + // it calls _pushUnderlying for tokenOut + bPool.mock_call__pushUnderlying(tokenOut, address(this), expectedAmountOut); + bPool.expectCall__pushUnderlying(tokenOut, address(this), expectedAmountOut); + // it emits a LOG_CALL event + bytes memory _data = abi.encodeCall( + IBPool.swapExactAmountIn, (tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceAfterSwap) + ); + vm.expectEmit(); + emit IBPool.LOG_CALL(IBPool.swapExactAmountIn.selector, address(this), _data); + // it emits a LOG_SWAP event + vm.expectEmit(); + emit IBPool.LOG_SWAP(address(this), tokenIn, tokenOut, tokenAmountIn, expectedAmountOut); + + // it returns the tokenOut amount swapped + // it returns the spot price after the swap + (uint256 out, uint256 priceAfter) = + bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceAfterSwap); + assertEq(out, expectedAmountOut); + assertEq(priceAfter, spotPriceAfterSwap); + // it clears the reeentrancy lock + assertEq(bPool.call__getLock(), _MUTEX_FREE); + } +} diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.tree b/test/unit/BPool/BPool_SwapExactAmountIn.tree new file mode 100644 index 00000000..bce5d93c --- /dev/null +++ b/test/unit/BPool/BPool_SwapExactAmountIn.tree @@ -0,0 +1,26 @@ +BPool::SwapExactAmountIn +├── when reentrancy lock is set +│ └── it should revert +├── when pool is not finalized +│ └── it should revert +├── when token in is not bound +│ └── it should revert +├── when token out is not bound +│ └── it should revert +├── when token in exceeds max allowed ratio +│ └── it should revert +├── when spot price before swap exceeds maxPrice +│ └── it should revert +├── when spot price after swap exceeds maxPrice +│ └── it should revert +├── when returned tokenOut is less than minAmountOut +│ └── it should revert +└── when preconditions are met + ├── it emits a LOG_CALL event + ├── it sets the reentrancy lock + ├── it emits a LOG_SWAP event + ├── it calls _pullUnderlying for tokenIn + ├── it calls _pushUnderlying for tokenOut + ├── it returns the tokenOut amount swapped + ├── it returns the spot price after the swap + └── it clears the reeentrancy lock From 432f63a616cee6d73bdd036f921f557bd4fbcd06 Mon Sep 17 00:00:00 2001 From: teddy Date: Tue, 9 Jul 2024 17:22:14 -0300 Subject: [PATCH 02/16] chore: delete preexisting unit tests --- test/unit/BPool.t.sol | 224 ------------------------------------------ 1 file changed, 224 deletions(-) diff --git a/test/unit/BPool.t.sol b/test/unit/BPool.t.sol index 4611daf6..96130212 100644 --- a/test/unit/BPool.t.sol +++ b/test/unit/BPool.t.sol @@ -1007,230 +1007,6 @@ contract BPool_Unit_ExitPool is BasePoolTest { } } -contract BPool_Unit_SwapExactAmountIn is SwapExactAmountInUtils { - function test_Revert_NotBoundTokenIn( - SwapExactAmountIn_FuzzScenario memory _fuzz, - address _tokenIn - ) public happyPath(_fuzz) { - assumeNotForgeAddress(_tokenIn); - vm.assume(_tokenIn != tokenIn); - vm.assume(_tokenIn != tokenOut); - - vm.expectRevert(IBPool.BPool_TokenNotBound.selector); - bPool.swapExactAmountIn(_tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - } - - function test_Revert_NotBoundTokenOut( - SwapExactAmountIn_FuzzScenario memory _fuzz, - address _tokenOut - ) public happyPath(_fuzz) { - assumeNotForgeAddress(_tokenOut); - vm.assume(_tokenOut != tokenIn); - vm.assume(_tokenOut != tokenOut); - - vm.expectRevert(IBPool.BPool_TokenNotBound.selector); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, _tokenOut, 0, type(uint256).max); - } - - function test_Revert_NotFinalized(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - _setFinalize(false); - - vm.expectRevert(IBPool.BPool_PoolNotFinalized.selector); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - } - - function test_Revert_TokenAmountInAboveMaxIn(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - uint256 _tokenAmountIn = bmul(_fuzz.tokenInBalance, MAX_IN_RATIO) + 1; - - vm.expectRevert(IBPool.BPool_TokenAmountInAboveMaxRatio.selector); - bPool.swapExactAmountIn(tokenIn, _tokenAmountIn, tokenOut, 0, type(uint256).max); - } - - function test_Revert_SpotPriceAboveMaxPrice( - SwapExactAmountIn_FuzzScenario memory _fuzz, - uint256 _maxPrice - ) public happyPath(_fuzz) { - uint256 _spotPriceBefore = calcSpotPrice( - _fuzz.tokenInBalance, _fuzz.tokenInDenorm, _fuzz.tokenOutBalance, _fuzz.tokenOutDenorm, _fuzz.swapFee - ); - vm.assume(_spotPriceBefore > 0); - _maxPrice = bound(_maxPrice, 0, _spotPriceBefore - 1); - - vm.expectRevert(IBPool.BPool_SpotPriceAboveMaxPrice.selector); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, _maxPrice); - } - - function test_Revert_TokenAmountOutBelowMinOut( - SwapExactAmountIn_FuzzScenario memory _fuzz, - uint256 _minAmountOut - ) public happyPath(_fuzz) { - uint256 _tokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance, - _fuzz.tokenOutDenorm, - _fuzz.tokenAmountIn, - _fuzz.swapFee - ); - _minAmountOut = bound(_minAmountOut, _tokenAmountOut + 1, type(uint256).max); - - vm.expectRevert(IBPool.BPool_TokenAmountOutBelowMinOut.selector); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, _minAmountOut, type(uint256).max); - } - - function test_Revert_Reentrancy(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - _expectRevertByReentrancy(); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - } - - function test_Revert_SpotPriceAfterBelowSpotPriceBefore() public { - vm.skip(true); - // TODO: this revert might be unreachable. Find a way to test it or remove the revert in the code. - } - - function test_Revert_SpotPriceAfterAboveMaxPrice(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - uint256 _tokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance, - _fuzz.tokenOutDenorm, - _fuzz.tokenAmountIn, - _fuzz.swapFee - ); - uint256 _spotPriceBefore = calcSpotPrice( - _fuzz.tokenInBalance, _fuzz.tokenInDenorm, _fuzz.tokenOutBalance, _fuzz.tokenOutDenorm, _fuzz.swapFee - ); - uint256 _spotPriceAfter = calcSpotPrice( - _fuzz.tokenInBalance + _fuzz.tokenAmountIn, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance - _tokenAmountOut, - _fuzz.tokenOutDenorm, - _fuzz.swapFee - ); - vm.assume(_spotPriceAfter > _spotPriceBefore); - - vm.expectRevert(IBPool.BPool_SpotPriceAboveMaxPrice.selector); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, _spotPriceBefore); - } - - function test_Revert_SpotPriceBeforeAboveTokenRatio(SwapExactAmountIn_FuzzScenario memory _fuzz) public { - // Replicating _assumeHappyPath, but removing irrelevant assumptions and conditioning the revert - _fuzz.tokenInDenorm = bound(_fuzz.tokenInDenorm, MIN_WEIGHT, MAX_WEIGHT); - _fuzz.tokenOutDenorm = bound(_fuzz.tokenOutDenorm, MIN_WEIGHT, MAX_WEIGHT); - _fuzz.swapFee = bound(_fuzz.swapFee, MIN_FEE, MAX_FEE); - _fuzz.tokenInBalance = bound(_fuzz.tokenInBalance, MIN_BALANCE, type(uint256).max / _fuzz.tokenInDenorm); - _fuzz.tokenOutBalance = bound(_fuzz.tokenOutBalance, MIN_BALANCE, type(uint256).max / _fuzz.tokenOutDenorm); - vm.assume(_fuzz.tokenAmountIn < type(uint256).max - _fuzz.tokenInBalance); - vm.assume(_fuzz.tokenInBalance + _fuzz.tokenAmountIn < type(uint256).max / _fuzz.tokenInDenorm); - _assumeCalcSpotPrice( - _fuzz.tokenInBalance, _fuzz.tokenInDenorm, _fuzz.tokenOutBalance, _fuzz.tokenOutDenorm, _fuzz.swapFee - ); - vm.assume(_fuzz.tokenAmountIn <= bmul(_fuzz.tokenInBalance, MAX_IN_RATIO)); - uint256 _spotPriceBefore = calcSpotPrice( - _fuzz.tokenInBalance, _fuzz.tokenInDenorm, _fuzz.tokenOutBalance, _fuzz.tokenOutDenorm, _fuzz.swapFee - ); - _assumeCalcOutGivenIn(_fuzz.tokenInBalance, _fuzz.tokenAmountIn, _fuzz.swapFee); - uint256 _tokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance, - _fuzz.tokenOutDenorm, - _fuzz.tokenAmountIn, - _fuzz.swapFee - ); - vm.assume(_tokenAmountOut > BONE); - _assumeCalcSpotPrice( - _fuzz.tokenInBalance + _fuzz.tokenAmountIn, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance - _tokenAmountOut, - _fuzz.tokenOutDenorm, - _fuzz.swapFee - ); - vm.assume(_spotPriceBefore > bdiv(_fuzz.tokenAmountIn, _tokenAmountOut)); - - _setValues(_fuzz); - - vm.expectRevert(IBPool.BPool_SpotPriceBeforeAboveTokenRatio.selector); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - } - - function test_Emit_LogSwap(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - uint256 _tokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance, - _fuzz.tokenOutDenorm, - _fuzz.tokenAmountIn, - _fuzz.swapFee - ); - - vm.expectEmit(); - emit IBPool.LOG_SWAP(address(this), tokenIn, tokenOut, _fuzz.tokenAmountIn, _tokenAmountOut); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - } - - function test_Set_ReentrancyLock(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - _expectSetReentrancyLock(); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - } - - function test_Pull_TokenAmountIn(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - vm.expectCall( - address(tokenIn), - abi.encodeWithSelector(IERC20.transferFrom.selector, address(this), address(bPool), _fuzz.tokenAmountIn) - ); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - } - - function test_Push_TokenAmountOut(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - uint256 _tokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance, - _fuzz.tokenOutDenorm, - _fuzz.tokenAmountIn, - _fuzz.swapFee - ); - - vm.expectCall(address(tokenOut), abi.encodeWithSelector(IERC20.transfer.selector, address(this), _tokenAmountOut)); - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - } - - function test_Returns_AmountAndPrice(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - uint256 _expectedTokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance, - _fuzz.tokenOutDenorm, - _fuzz.tokenAmountIn, - _fuzz.swapFee - ); - uint256 _expectedSpotPriceAfter = calcSpotPrice( - _fuzz.tokenInBalance + _fuzz.tokenAmountIn, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance - _expectedTokenAmountOut, - _fuzz.tokenOutDenorm, - _fuzz.swapFee - ); - - (uint256 _tokenAmountOut, uint256 _spotPriceAfter) = - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - - assertEq(_tokenAmountOut, _expectedTokenAmountOut); - assertEq(_spotPriceAfter, _expectedSpotPriceAfter); - } - - function test_Emit_LogCall(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - vm.expectEmit(); - bytes memory _data = abi.encodeWithSelector( - BPool.swapExactAmountIn.selector, tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max - ); - emit IBPool.LOG_CALL(BPool.swapExactAmountIn.selector, address(this), _data); - - bPool.swapExactAmountIn(tokenIn, _fuzz.tokenAmountIn, tokenOut, 0, type(uint256).max); - } -} - contract BPool_Unit_SwapExactAmountOut is BasePoolTest { address tokenIn; address tokenOut; From e66ed9f7de39fbd63f5c47eb0aefa649dc491c06 Mon Sep 17 00:00:00 2001 From: teddy Date: Wed, 10 Jul 2024 16:14:59 -0300 Subject: [PATCH 03/16] test: small renames from feedback --- test/unit/BPool/BPool_SwapExactAmountIn.t.sol | 14 +++++++------- test/unit/BPool/BPool_SwapExactAmountIn.tree | 6 +++--- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol index a6dcb54f..7004f971 100644 --- a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol +++ b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol @@ -69,7 +69,7 @@ contract BPoolSwapExactAmountIn is BPoolBase, BNum { bPool.swapExactAmountIn(tokenIn, tokenAmountIn, makeAddr('unknown token'), expectedAmountOut, spotPriceAfterSwap); } - function test_RevertWhen_TokenInExceedsMaxAllowedRatio(uint256 tokenAmountIn_) external { + function test_RevertWhen_TokenAmountInExceedsMaxAllowedRatio(uint256 tokenAmountIn_) external { tokenAmountIn_ = bound(tokenAmountIn_, bmul(tokenInBalance, MAX_IN_RATIO) + 1, type(uint256).max); // it should revert vm.expectRevert(IBPool.BPool_TokenAmountInAboveMaxRatio.selector); @@ -82,16 +82,16 @@ contract BPoolSwapExactAmountIn is BPoolBase, BNum { bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceBeforeSwap - 1); } - function test_RevertWhen_SpotPriceAfterSwapExceedsMaxPrice() external { + function test_RevertWhen_CalculatedTokenAmountOutIsLessThanMinAmountOut() external { // it should revert - vm.expectRevert(IBPool.BPool_SpotPriceAboveMaxPrice.selector); - bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceAfterSwap - 1); + vm.expectRevert(IBPool.BPool_TokenAmountOutBelowMinOut.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut + 1, spotPriceAfterSwap); } - function test_RevertWhen_ReturnedTokenOutIsLessThanMinAmountOut() external { + function test_RevertWhen_SpotPriceAfterSwapExceedsMaxPrice() external { // it should revert - vm.expectRevert(IBPool.BPool_TokenAmountOutBelowMinOut.selector); - bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut + 1, spotPriceAfterSwap); + vm.expectRevert(IBPool.BPool_SpotPriceAboveMaxPrice.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceAfterSwap - 1); } function test_WhenPreconditionsAreMet() external { diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.tree b/test/unit/BPool/BPool_SwapExactAmountIn.tree index bce5d93c..b583ee76 100644 --- a/test/unit/BPool/BPool_SwapExactAmountIn.tree +++ b/test/unit/BPool/BPool_SwapExactAmountIn.tree @@ -7,13 +7,13 @@ BPool::SwapExactAmountIn │ └── it should revert ├── when token out is not bound │ └── it should revert -├── when token in exceeds max allowed ratio +├── when token amount in exceeds max allowed ratio │ └── it should revert ├── when spot price before swap exceeds maxPrice │ └── it should revert -├── when spot price after swap exceeds maxPrice +├── when calculated token amount out is less than minAmountOut │ └── it should revert -├── when returned tokenOut is less than minAmountOut +├── when spot price after swap exceeds maxPrice │ └── it should revert └── when preconditions are met ├── it emits a LOG_CALL event From 1acff62a584a4ef88bec75615909773c48919475 Mon Sep 17 00:00:00 2001 From: teddy Date: Wed, 10 Jul 2024 18:36:01 -0300 Subject: [PATCH 04/16] test: be explicit about untestable code --- test/unit/BPool/BPool_SwapExactAmountIn.t.sol | 5 +++++ test/unit/BPool/BPool_SwapExactAmountIn.tree | 2 ++ 2 files changed, 7 insertions(+) diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol index 7004f971..b1d1c6b9 100644 --- a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol +++ b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol @@ -94,6 +94,11 @@ contract BPoolSwapExactAmountIn is BPoolBase, BNum { bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut, spotPriceAfterSwap - 1); } + function test_RevertWhen_SpotPriceBeforeSwapExceedsTokenRatioAfterSwap() external { + // it should revert + vm.skip(true); + } + function test_WhenPreconditionsAreMet() external { // it sets reentrancy lock bPool.expectCall__setLock(_MUTEX_TAKEN); diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.tree b/test/unit/BPool/BPool_SwapExactAmountIn.tree index b583ee76..6045922f 100644 --- a/test/unit/BPool/BPool_SwapExactAmountIn.tree +++ b/test/unit/BPool/BPool_SwapExactAmountIn.tree @@ -15,6 +15,8 @@ BPool::SwapExactAmountIn │ └── it should revert ├── when spot price after swap exceeds maxPrice │ └── it should revert +├── when spot price before swap exceeds token ratio after swap +│ └── it should revert └── when preconditions are met ├── it emits a LOG_CALL event ├── it sets the reentrancy lock From d111e8b5f26471ff1e07a17e678f8a4f012c0559 Mon Sep 17 00:00:00 2001 From: teddy Date: Thu, 11 Jul 2024 10:57:32 -0300 Subject: [PATCH 05/16] test: adding skipped test for unreachable condition --- test/unit/BPool/BPool_SwapExactAmountIn.t.sol | 10 ++++++++++ test/unit/BPool/BPool_SwapExactAmountIn.tree | 2 ++ 2 files changed, 12 insertions(+) diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol index 3f2887e3..c7478ef2 100644 --- a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol +++ b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol @@ -87,6 +87,16 @@ contract BPoolSwapExactAmountIn is BPoolBase, BNum { bPool.swapExactAmountIn(tokenIn, tokenAmountIn, tokenOut, expectedAmountOut + 1, spotPriceAfterSwap); } + function test_RevertWhen_SpotPriceAfterSwapExceedsSpotPriceBeforeSwap() external { + // it should revert + // skipping since the code for this is unreachable without manually + // overriding `calcSpotPrice` in a mock: + // P_{sb} = \frac{\frac{b_i}{w_i}}{\frac{b_o}{w_o}} + // P_{sa} = \frac{\frac{b_i + a_i}{w_i}}{\frac{b_o - a_o}{w_o}} + // ...and both a_i (amount in) and a_o (amount out) are uints + vm.skip(true); + } + function test_RevertWhen_SpotPriceAfterSwapExceedsMaxPrice() external { // it should revert vm.expectRevert(IBPool.BPool_SpotPriceAboveMaxPrice.selector); diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.tree b/test/unit/BPool/BPool_SwapExactAmountIn.tree index 6045922f..8d78fd2e 100644 --- a/test/unit/BPool/BPool_SwapExactAmountIn.tree +++ b/test/unit/BPool/BPool_SwapExactAmountIn.tree @@ -13,6 +13,8 @@ BPool::SwapExactAmountIn │ └── it should revert ├── when calculated token amount out is less than minAmountOut │ └── it should revert +├── when spot price after swap exceeds spot price before swap +│ └── it should revert ├── when spot price after swap exceeds maxPrice │ └── it should revert ├── when spot price before swap exceeds token ratio after swap From 9f074dc97ffdefe5563605f4afc07afcc26f1daa Mon Sep 17 00:00:00 2001 From: teddy Date: Thu, 11 Jul 2024 15:54:59 -0300 Subject: [PATCH 06/16] test: code wasnt so unreachable after all --- test/unit/BPool/BPool_SwapExactAmountIn.t.sol | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol index c7478ef2..709b3461 100644 --- a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol +++ b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol @@ -104,8 +104,20 @@ contract BPoolSwapExactAmountIn is BPoolBase, BNum { } function test_RevertWhen_SpotPriceBeforeSwapExceedsTokenRatioAfterSwap() external { + uint256 tokenAmountIn_ = 30e18; + uint256 balanceTokenIn_ = 36_830_000_000_000_000_000_000_000_000_000; + uint256 weightTokenIn_ = 1e18; + uint256 balanceTokenOut_ = 18_100_000_000_000_000_000_000_000_000_000; + uint256 weightTokenOut_ = 1e18; + uint256 swapFee_ = 0.019e18; + vm.mockCall(tokenIn, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(balanceTokenIn_))); + vm.mockCall(tokenOut, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(balanceTokenOut_))); + bPool.set__records(tokenIn, IBPool.Record({bound: true, index: 0, denorm: weightTokenIn_})); + bPool.set__records(tokenOut, IBPool.Record({bound: true, index: 1, denorm: weightTokenOut_})); + bPool.set__swapFee(swapFee_); // it should revert - vm.skip(true); + vm.expectRevert(IBPool.BPool_SpotPriceBeforeAboveTokenRatio.selector); + bPool.swapExactAmountIn(tokenIn, tokenAmountIn_, tokenOut, 0, type(uint256).max); } function test_WhenPreconditionsAreMet() external { From 3f85e43baa93083fef5a492de3f69fc60d63a97c Mon Sep 17 00:00:00 2001 From: teddy Date: Thu, 11 Jul 2024 18:36:31 -0300 Subject: [PATCH 07/16] refactor: get rid of _setRecord --- test/unit/BPool.t.sol | 32 ++++++++----------- test/unit/BPool/BPool.t.sol | 4 +-- test/unit/BPool/BPoolBase.sol | 6 +--- test/unit/BPool/BPool_Bind.t.sol | 2 +- test/unit/BPool/BPool_ExitPool.t.sol | 4 +-- test/unit/BPool/BPool_JoinPool.t.sol | 4 +-- test/unit/BPool/BPool_SwapExactAmountIn.t.sol | 4 +-- test/unit/BPool/BPool_Unbind.t.sol | 6 ++-- 8 files changed, 27 insertions(+), 35 deletions(-) diff --git a/test/unit/BPool.t.sol b/test/unit/BPool.t.sol index 16868e32..ddd5182b 100644 --- a/test/unit/BPool.t.sol +++ b/test/unit/BPool.t.sol @@ -34,7 +34,7 @@ abstract contract BasePoolTest is Test, BConst, Utils, BMath { function _setRandomTokens(uint256 _length) internal returns (address[] memory _tokensToAdd) { _tokensToAdd = _getDeterministicTokenArray(_length); for (uint256 i = 0; i < _length; i++) { - _setRecord(_tokensToAdd[i], IBPool.Record({bound: true, index: i, denorm: 0})); + bPool.set__records(_tokensToAdd[i], IBPool.Record({bound: true, index: i, denorm: 0})); } _setTokens(_tokensToAdd); } @@ -57,10 +57,6 @@ abstract contract BasePoolTest is Test, BConst, Utils, BMath { bPool.set__tokens(_tokens); } - function _setRecord(address _token, IBPool.Record memory _record) internal { - bPool.set__records(_token, _record); - } - function _setSwapFee(uint256 _swapFee) internal { bPool.set__swapFee(_swapFee); } @@ -274,7 +270,7 @@ abstract contract SwapExactAmountInUtils is BasePoolTest { _mockTransfer(tokenOut); // Set balances - _setRecord( + bPool.set__records( tokenIn, IBPool.Record({ bound: true, @@ -284,7 +280,7 @@ abstract contract SwapExactAmountInUtils is BasePoolTest { ); _mockPoolBalance(tokenIn, _fuzz.tokenInBalance); - _setRecord( + bPool.set__records( tokenOut, IBPool.Record({ bound: true, @@ -434,7 +430,7 @@ contract BPool_Unit_GetNormalizedWeight is BasePoolTest { _weight = bound(_weight, MIN_WEIGHT, MAX_WEIGHT); _totalWeight = bound(_totalWeight, MIN_WEIGHT, MAX_TOTAL_WEIGHT); vm.assume(_weight < _totalWeight); - _setRecord(_token, IBPool.Record({bound: true, index: 0, denorm: _weight})); + bPool.set__records(_token, IBPool.Record({bound: true, index: 0, denorm: _weight})); _setTotalWeight(_totalWeight); assertEq(bPool.getNormalizedWeight(_token), bdiv(_weight, _totalWeight)); @@ -694,9 +690,9 @@ contract BPool_Unit_GetSpotPrice is BasePoolTest { } function _setValues(GetSpotPrice_FuzzScenario memory _fuzz) internal { - _setRecord(_fuzz.tokenIn, IBPool.Record({bound: true, index: 0, denorm: _fuzz.tokenInDenorm})); + bPool.set__records(_fuzz.tokenIn, IBPool.Record({bound: true, index: 0, denorm: _fuzz.tokenInDenorm})); _mockPoolBalance(_fuzz.tokenIn, _fuzz.tokenInBalance); - _setRecord(_fuzz.tokenOut, IBPool.Record({bound: true, index: 0, denorm: _fuzz.tokenOutDenorm})); + bPool.set__records(_fuzz.tokenOut, IBPool.Record({bound: true, index: 0, denorm: _fuzz.tokenOutDenorm})); _mockPoolBalance(_fuzz.tokenOut, _fuzz.tokenOutBalance); _setSwapFee(_fuzz.swapFee); } @@ -763,9 +759,9 @@ contract BPool_Unit_GetSpotPriceSansFee is BasePoolTest { } function _setValues(GetSpotPriceSansFee_FuzzScenario memory _fuzz) internal { - _setRecord(_fuzz.tokenIn, IBPool.Record({bound: true, index: 0, denorm: _fuzz.tokenInDenorm})); + bPool.set__records(_fuzz.tokenIn, IBPool.Record({bound: true, index: 0, denorm: _fuzz.tokenInDenorm})); _mockPoolBalance(_fuzz.tokenIn, _fuzz.tokenInBalance); - _setRecord(_fuzz.tokenOut, IBPool.Record({bound: true, index: 0, denorm: _fuzz.tokenOutDenorm})); + bPool.set__records(_fuzz.tokenOut, IBPool.Record({bound: true, index: 0, denorm: _fuzz.tokenOutDenorm})); _mockPoolBalance(_fuzz.tokenOut, _fuzz.tokenOutBalance); _setSwapFee(0); } @@ -840,7 +836,7 @@ contract BPool_Unit_SwapExactAmountOut is BasePoolTest { _mockTransfer(tokenOut); // Set balances - _setRecord( + bPool.set__records( tokenIn, IBPool.Record({ bound: true, @@ -850,7 +846,7 @@ contract BPool_Unit_SwapExactAmountOut is BasePoolTest { ); _mockPoolBalance(tokenIn, _fuzz.tokenInBalance); - _setRecord( + bPool.set__records( tokenOut, IBPool.Record({ bound: true, @@ -1187,7 +1183,7 @@ contract BPool_Unit_JoinswapExternAmountIn is BasePoolTest { _mockTransferFrom(tokenIn); // Set balances - _setRecord( + bPool.set__records( tokenIn, IBPool.Record({ bound: true, @@ -1367,7 +1363,7 @@ contract BPool_Unit_JoinswapPoolAmountOut is BasePoolTest { _mockTransferFrom(tokenIn); // Set balances - _setRecord( + bPool.set__records( tokenIn, IBPool.Record({ bound: true, @@ -1599,7 +1595,7 @@ contract BPool_Unit_ExitswapPoolAmountIn is BasePoolTest { _mockTransfer(tokenOut); // Set balances - _setRecord( + bPool.set__records( tokenOut, IBPool.Record({ bound: true, @@ -1847,7 +1843,7 @@ contract BPool_Unit_ExitswapExternAmountOut is BasePoolTest { _mockTransfer(tokenOut); // Set balances - _setRecord( + bPool.set__records( tokenOut, IBPool.Record({ bound: true, diff --git a/test/unit/BPool/BPool.t.sol b/test/unit/BPool/BPool.t.sol index b3bebdee..264fef02 100644 --- a/test/unit/BPool/BPool.t.sol +++ b/test/unit/BPool/BPool.t.sol @@ -33,13 +33,13 @@ contract BPool is BPoolBase { } function test_IsBoundWhenTokenIsBound(address _token) external { - _setRecord(_token, IBPool.Record({bound: true, index: 0, denorm: 0})); + bPool.set__records(_token, IBPool.Record({bound: true, index: 0, denorm: 0})); // it returns true assertTrue(bPool.isBound(_token)); } function test_IsBoundWhenTokenIsNOTBound(address _token) external { - _setRecord(_token, IBPool.Record({bound: false, index: 0, denorm: 0})); + bPool.set__records(_token, IBPool.Record({bound: false, index: 0, denorm: 0})); // it returns false assertFalse(bPool.isBound(_token)); } diff --git a/test/unit/BPool/BPoolBase.sol b/test/unit/BPool/BPoolBase.sol index 0516fcec..a205e743 100644 --- a/test/unit/BPool/BPoolBase.sol +++ b/test/unit/BPool/BPoolBase.sol @@ -24,7 +24,7 @@ contract BPoolBase is Test, BConst, Utils { function _setRandomTokens(uint256 _length) internal returns (address[] memory _tokensToAdd) { _tokensToAdd = _getDeterministicTokenArray(_length); for (uint256 i = 0; i < _length; i++) { - _setRecord(_tokensToAdd[i], IBPool.Record({bound: true, index: i, denorm: 0})); + bPool.set__records(_tokensToAdd[i], IBPool.Record({bound: true, index: i, denorm: 0})); } _setTokens(_tokensToAdd); } @@ -32,8 +32,4 @@ contract BPoolBase is Test, BConst, Utils { function _setTokens(address[] memory _tokens) internal { bPool.set__tokens(_tokens); } - - function _setRecord(address _token, IBPool.Record memory _record) internal { - bPool.set__records(_token, _record); - } } diff --git a/test/unit/BPool/BPool_Bind.t.sol b/test/unit/BPool/BPool_Bind.t.sol index 0b146ac7..38e2862c 100644 --- a/test/unit/BPool/BPool_Bind.t.sol +++ b/test/unit/BPool/BPool_Bind.t.sol @@ -36,7 +36,7 @@ contract BPoolBind is BPoolBase { } function test_RevertWhen_TokenIsAlreadyBound() external whenCallerIsController { - _setRecord(tokens[0], IBPool.Record({bound: true, index: 0, denorm: tokenWeight})); + bPool.set__records(tokens[0], IBPool.Record({bound: true, index: 0, denorm: tokenWeight})); // it should revert vm.expectRevert(IBPool.BPool_TokenAlreadyBound.selector); bPool.bind(tokens[0], tokenBindBalance, tokenWeight); diff --git a/test/unit/BPool/BPool_ExitPool.t.sol b/test/unit/BPool/BPool_ExitPool.t.sol index fac066ce..858cbef1 100644 --- a/test/unit/BPool/BPool_ExitPool.t.sol +++ b/test/unit/BPool/BPool_ExitPool.t.sol @@ -31,8 +31,8 @@ contract BPoolExitPool is BPoolBase, BNum { bPool.call__mintPoolShare(INIT_POOL_SUPPLY); bPool.set__tokens(_tokensToMemory()); // token weights are not used for all-token exits - _setRecord(tokens[0], IBPool.Record({bound: true, index: 0, denorm: 0})); - _setRecord(tokens[1], IBPool.Record({bound: true, index: 1, denorm: 0})); + bPool.set__records(tokens[0], IBPool.Record({bound: true, index: 0, denorm: 0})); + bPool.set__records(tokens[1], IBPool.Record({bound: true, index: 1, denorm: 0})); // underlying balances are used instead vm.mockCall(tokens[0], abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(token0Balance))); vm.mockCall(tokens[1], abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(token1Balance))); diff --git a/test/unit/BPool/BPool_JoinPool.t.sol b/test/unit/BPool/BPool_JoinPool.t.sol index 1f501275..eeb01efe 100644 --- a/test/unit/BPool/BPool_JoinPool.t.sol +++ b/test/unit/BPool/BPool_JoinPool.t.sol @@ -27,8 +27,8 @@ contract BPoolJoinPool is BPoolBase { bPool.call__mintPoolShare(INIT_POOL_SUPPLY); bPool.set__tokens(_tokensToMemory()); // token weights are not used for all-token joins - _setRecord(tokens[0], IBPool.Record({bound: true, index: 0, denorm: 0})); - _setRecord(tokens[1], IBPool.Record({bound: true, index: 1, denorm: 0})); + bPool.set__records(tokens[0], IBPool.Record({bound: true, index: 0, denorm: 0})); + bPool.set__records(tokens[1], IBPool.Record({bound: true, index: 1, denorm: 0})); // underlying balances are used instead vm.mockCall(tokens[0], abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(token0Balance))); vm.mockCall(tokens[1], abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(token1Balance))); diff --git a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol index 709b3461..9537bf7f 100644 --- a/test/unit/BPool/BPool_SwapExactAmountIn.t.sol +++ b/test/unit/BPool/BPool_SwapExactAmountIn.t.sol @@ -35,8 +35,8 @@ contract BPoolSwapExactAmountIn is BPoolBase, BNum { tokenOut = tokens[1]; bPool.set__finalized(true); bPool.set__tokens(tokens); - _setRecord(tokenIn, IBPool.Record({bound: true, index: 0, denorm: tokenInWeight})); - _setRecord(tokenOut, IBPool.Record({bound: true, index: 1, denorm: tokenOutWeight})); + bPool.set__records(tokenIn, IBPool.Record({bound: true, index: 0, denorm: tokenInWeight})); + bPool.set__records(tokenOut, IBPool.Record({bound: true, index: 1, denorm: tokenOutWeight})); vm.mockCall(tokenIn, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(tokenInBalance))); vm.mockCall(tokenOut, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(tokenOutBalance))); diff --git a/test/unit/BPool/BPool_Unbind.t.sol b/test/unit/BPool/BPool_Unbind.t.sol index 19eb9ced..6f0e782c 100644 --- a/test/unit/BPool/BPool_Unbind.t.sol +++ b/test/unit/BPool/BPool_Unbind.t.sol @@ -42,7 +42,7 @@ contract BPoolUnbind is BPoolBase { } function test_RevertWhen_PoolIsFinalized() external whenCallerIsController { - _setRecord(tokens[0], IBPool.Record({bound: true, index: 0, denorm: 0})); + bPool.set__records(tokens[0], IBPool.Record({bound: true, index: 0, denorm: 0})); bPool.set__finalized(true); // it should revert vm.expectRevert(IBPool.BPool_PoolIsFinalized.selector); @@ -50,7 +50,7 @@ contract BPoolUnbind is BPoolBase { } modifier whenTokenCanBeUnbound() { - _setRecord(tokens[0], IBPool.Record({bound: true, index: 0, denorm: tokenWeight})); + bPool.set__records(tokens[0], IBPool.Record({bound: true, index: 0, denorm: tokenWeight})); bPool.set__totalWeight(totalWeight); address[] memory tokens = new address[](1); tokens[0] = tokens[0]; @@ -81,7 +81,7 @@ contract BPoolUnbind is BPoolBase { } function test_WhenTokenIsNOTLastOnTheTokensArray() external whenCallerIsController whenTokenCanBeUnbound { - _setRecord(tokens[1], IBPool.Record({bound: true, index: 0, denorm: tokenWeight})); + bPool.set__records(tokens[1], IBPool.Record({bound: true, index: 0, denorm: tokenWeight})); bPool.set__tokens(_tokensToMemory()); bPool.unbind(tokens[0]); // it removes the token record From 63b60588403b7c4c70a8aa7316d2273d70ecd9f8 Mon Sep 17 00:00:00 2001 From: teddy Date: Thu, 11 Jul 2024 19:57:30 -0300 Subject: [PATCH 08/16] test: btt tests for bcowpool.verify --- test/unit/BCoWPool/BCoWPoolBase.sol | 28 +++++ test/unit/BCoWPool/BCoWPool_Verify.t.sol | 127 +++++++++++++++++++++++ test/unit/BCoWPool/BCoWPool_Verify.tree | 23 ++++ 3 files changed, 178 insertions(+) create mode 100644 test/unit/BCoWPool/BCoWPoolBase.sol create mode 100644 test/unit/BCoWPool/BCoWPool_Verify.t.sol create mode 100644 test/unit/BCoWPool/BCoWPool_Verify.tree diff --git a/test/unit/BCoWPool/BCoWPoolBase.sol b/test/unit/BCoWPool/BCoWPoolBase.sol new file mode 100644 index 00000000..3418e59e --- /dev/null +++ b/test/unit/BCoWPool/BCoWPoolBase.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.25; + +import {BPoolBase} from '../BPool/BPoolBase.sol'; +import {BCoWConst} from 'contracts/BCoWConst.sol'; +import {BNum} from 'contracts/BNum.sol'; + +import {ISettlement} from 'interfaces/ISettlement.sol'; +import {MockBCoWPool} from 'test/manual-smock/MockBCoWPool.sol'; + +contract BCoWPoolBase is BPoolBase, BCoWConst, BNum { + bytes32 public appData = bytes32('appData'); + address public cowSolutionSettler = makeAddr('cowSolutionSettler'); + bytes32 public domainSeparator = bytes32(bytes2(0xf00b)); + address public vaultRelayer = makeAddr('vaultRelayer'); + address public tokenIn; + address public tokenOut; + MockBCoWPool bCoWPool; + + function setUp() public virtual override { + super.setUp(); + tokenIn = tokens[0]; + tokenOut = tokens[1]; + vm.mockCall(cowSolutionSettler, abi.encodePacked(ISettlement.domainSeparator.selector), abi.encode(domainSeparator)); + vm.mockCall(cowSolutionSettler, abi.encodePacked(ISettlement.vaultRelayer.selector), abi.encode(vaultRelayer)); + bCoWPool = new MockBCoWPool(cowSolutionSettler, appData); + } +} diff --git a/test/unit/BCoWPool/BCoWPool_Verify.t.sol b/test/unit/BCoWPool/BCoWPool_Verify.t.sol new file mode 100644 index 00000000..ea756477 --- /dev/null +++ b/test/unit/BCoWPool/BCoWPool_Verify.t.sol @@ -0,0 +1,127 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.25; + +import {IERC20} from '@cowprotocol/interfaces/IERC20.sol'; +import {GPv2Order} from '@cowprotocol/libraries/GPv2Order.sol'; + +import {BCoWPoolBase} from './BCoWPoolBase.sol'; +import {IBCoWPool} from 'interfaces/IBCoWPool.sol'; +import {IBPool} from 'interfaces/IBPool.sol'; + +contract BCoWPoolVerify is BCoWPoolBase { + // Valid scenario: + uint256 public tokenAmountIn = 1e18; + uint256 public tokenInBalance = 100e18; + uint256 public tokenOutBalance = 80e18; + // pool is expected to keep 4X the value of tokenIn than tokenOut + uint256 public tokenInWeight = 4e18; + uint256 public tokenOutWeight = 1e18; + // from bmath: (with fee zero) 80*(1-(100/(100+1))^(4)) + uint256 public expectedAmountOut = 3.12157244137469736e18; + GPv2Order.Data correctOrder; + + function setUp() public virtual override { + super.setUp(); + bCoWPool.set__tokens(tokens); + bCoWPool.set__records(tokenIn, IBPool.Record({bound: true, index: 0, denorm: tokenInWeight})); + bCoWPool.set__records(tokenOut, IBPool.Record({bound: true, index: 1, denorm: tokenOutWeight})); + vm.mockCall(tokenIn, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(tokenInBalance))); + vm.mockCall(tokenOut, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(tokenOutBalance))); + + correctOrder = GPv2Order.Data({ + sellToken: IERC20(tokenOut), + buyToken: IERC20(tokenIn), + receiver: GPv2Order.RECEIVER_SAME_AS_OWNER, + sellAmount: expectedAmountOut, + buyAmount: tokenAmountIn, + validTo: uint32(block.timestamp + 1 minutes), + appData: appData, + feeAmount: 0, + kind: GPv2Order.KIND_SELL, + partiallyFillable: false, + sellTokenBalance: GPv2Order.BALANCE_ERC20, + buyTokenBalance: GPv2Order.BALANCE_ERC20 + }); + } + + function test_RevertWhen_BuyTokenIsNotBound() external { + correctOrder.buyToken = IERC20(makeAddr('unknown token')); + // it should revert + vm.expectRevert(IBPool.BPool_TokenNotBound.selector); + bCoWPool.verify(correctOrder); + } + + function test_RevertWhen_SellTokenIsNotBound() external { + correctOrder.sellToken = IERC20(makeAddr('unknown token')); + // it should revert + vm.expectRevert(IBPool.BPool_TokenNotBound.selector); + bCoWPool.verify(correctOrder); + } + + function test_RevertWhen_OrderReceiverIsNotRECEIVER_SAME_AS_OWNERFlag() external { + correctOrder.receiver = makeAddr('somebodyElse'); + // it should revert + vm.expectRevert(IBCoWPool.BCoWPool_ReceiverIsNotBCoWPool.selector); + bCoWPool.verify(correctOrder); + } + + function test_RevertWhen_OrderIsValidForMoreThanMAX_ORDER_DURATION(uint256 _timeOffset) external { + _timeOffset = bound(_timeOffset, MAX_ORDER_DURATION + 1, type(uint32).max - block.timestamp); + correctOrder.validTo = uint32(block.timestamp + _timeOffset); + // it should revert + vm.expectRevert(IBCoWPool.BCoWPool_OrderValidityTooLong.selector); + bCoWPool.verify(correctOrder); + } + + function test_RevertWhen_FeeAmountIsNotZero(uint256 _fee) external { + _fee = bound(_fee, 1, type(uint256).max); + correctOrder.feeAmount = _fee; + // it should revert + vm.expectRevert(IBCoWPool.BCoWPool_FeeMustBeZero.selector); + bCoWPool.verify(correctOrder); + } + + function test_RevertWhen_OrderKindIsNotKIND_SELL(bytes32 _orderKind) external { + vm.assume(_orderKind != GPv2Order.KIND_SELL); + correctOrder.kind = _orderKind; + // it should revert + vm.expectRevert(IBCoWPool.BCoWPool_InvalidOperation.selector); + bCoWPool.verify(correctOrder); + } + + function test_RevertWhen_TokenBalanceMarkerForBuyTokenIsNotDirectERC20Balances(bytes32 _balanceKind) external { + vm.assume(_balanceKind != GPv2Order.BALANCE_ERC20); + correctOrder.buyTokenBalance = _balanceKind; + // it should revert + vm.expectRevert(IBCoWPool.BCoWPool_InvalidBalanceMarker.selector); + bCoWPool.verify(correctOrder); + } + + function test_RevertWhen_TokenBalanceMarkerForSellTokenIsNotDirectERC20Balances(bytes32 _balanceKind) external { + vm.assume(_balanceKind != GPv2Order.BALANCE_ERC20); + correctOrder.sellTokenBalance = _balanceKind; + // it should revert + vm.expectRevert(IBCoWPool.BCoWPool_InvalidBalanceMarker.selector); + bCoWPool.verify(correctOrder); + } + + function test_RevertWhen_OrderBuyAmountExceedsMAX_IN_RATIO(uint256 _buyAmount) external { + _buyAmount = bound(_buyAmount, bmul(tokenInBalance, MAX_IN_RATIO) + 1, type(uint256).max); + correctOrder.buyAmount = _buyAmount; + // it should revert + vm.expectRevert(IBPool.BPool_TokenAmountInAboveMaxRatio.selector); + bCoWPool.verify(correctOrder); + } + + function test_RevertWhen_ComputedTokenAmountOutIsLessThanOrderMinSellAmount() external { + correctOrder.sellAmount += 1; + // it should revert + vm.expectRevert(IBPool.BPool_TokenAmountOutBelowMinOut.selector); + bCoWPool.verify(correctOrder); + } + + function test_WhenPreconditionsAreMet() external { + // it should return + bCoWPool.verify(correctOrder); + } +} diff --git a/test/unit/BCoWPool/BCoWPool_Verify.tree b/test/unit/BCoWPool/BCoWPool_Verify.tree new file mode 100644 index 00000000..e4037e67 --- /dev/null +++ b/test/unit/BCoWPool/BCoWPool_Verify.tree @@ -0,0 +1,23 @@ +BCoWPool::Verify +├── when buyToken is not bound +│ └── it should revert +├── when sellToken is not bound +│ └── it should revert +├── when order receiver is not RECEIVER_SAME_AS_OWNER flag +│ └── it should revert +├── when order is valid for more than MAX_ORDER_DURATION +│ └── it should revert +├── when fee amount is not zero +│ └── it should revert +├── when order kind is not KIND_SELL +│ └── it should revert +├── when TokenBalance marker for buy token is not direct ERC20 balances +│ └── it should revert +├── when TokenBalance marker for sell token is not direct ERC20 balances +│ └── it should revert +├── when order buy amount exceeds MAX_IN_RATIO +│ └── it should revert +├── when computed token amount out is less than order min sell amount +│ └── it should revert +└── when preconditions are met + └── it should return From 1d3a30f9c0c07f2ed908b93de684f306c701ba36 Mon Sep 17 00:00:00 2001 From: teddy Date: Thu, 11 Jul 2024 19:58:43 -0300 Subject: [PATCH 09/16] chore: delete preexisting unit tests --- test/unit/BCoWPool.t.sol | 143 +-------------------------------------- test/unit/BPool.t.sol | 105 ---------------------------- 2 files changed, 1 insertion(+), 247 deletions(-) diff --git a/test/unit/BCoWPool.t.sol b/test/unit/BCoWPool.t.sol index 7c7e566c..0a73ea41 100644 --- a/test/unit/BCoWPool.t.sol +++ b/test/unit/BCoWPool.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.25; -import {BasePoolTest, SwapExactAmountInUtils} from './BPool.t.sol'; +import {BasePoolTest} from './BPool.t.sol'; import {IERC20} from '@cowprotocol/interfaces/IERC20.sol'; import {GPv2Order} from '@cowprotocol/libraries/GPv2Order.sol'; import {IERC1271} from '@openzeppelin/contracts/interfaces/IERC1271.sol'; @@ -142,147 +142,6 @@ contract BCoWPool_Unit_Commit is BaseCoWPoolTest { } } -contract BCoWPool_Unit_Verify is BaseCoWPoolTest, SwapExactAmountInUtils { - function setUp() public virtual override(BaseCoWPoolTest, BasePoolTest) { - BaseCoWPoolTest.setUp(); - } - - function _assumeHappyPath(SwapExactAmountIn_FuzzScenario memory _fuzz) internal pure override { - // safe bound assumptions - _fuzz.tokenInDenorm = bound(_fuzz.tokenInDenorm, MIN_WEIGHT, MAX_WEIGHT); - _fuzz.tokenOutDenorm = bound(_fuzz.tokenOutDenorm, MIN_WEIGHT, MAX_WEIGHT); - // LP fee when swapping via CoW will always be zero - _fuzz.swapFee = 0; - - // min - max - calcSpotPrice (spotPriceBefore) - _fuzz.tokenInBalance = bound(_fuzz.tokenInBalance, MIN_BALANCE, type(uint256).max / _fuzz.tokenInDenorm); - _fuzz.tokenOutBalance = bound(_fuzz.tokenOutBalance, MIN_BALANCE, type(uint256).max / _fuzz.tokenOutDenorm); - - // MAX_IN_RATIO - vm.assume(_fuzz.tokenAmountIn <= bmul(_fuzz.tokenInBalance, MAX_IN_RATIO)); - - _assumeCalcOutGivenIn(_fuzz.tokenInBalance, _fuzz.tokenAmountIn, _fuzz.swapFee); - uint256 _tokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance, - _fuzz.tokenOutDenorm, - _fuzz.tokenAmountIn, - _fuzz.swapFee - ); - vm.assume(_tokenAmountOut > BONE); - } - - modifier assumeNotBoundToken(address _token) { - for (uint256 i = 0; i < TOKENS_AMOUNT; i++) { - vm.assume(tokens[i] != _token); - } - _; - } - - function test_Revert_NonBoundBuyToken(address _otherToken) public assumeNotBoundToken(_otherToken) { - GPv2Order.Data memory order = correctOrder; - order.buyToken = IERC20(_otherToken); - vm.expectRevert(IBPool.BPool_TokenNotBound.selector); - bCoWPool.verify(order); - } - - function test_Revert_NonBoundSellToken(address _otherToken) public assumeNotBoundToken(_otherToken) { - GPv2Order.Data memory order = correctOrder; - order.sellToken = IERC20(_otherToken); - vm.expectRevert(IBPool.BPool_TokenNotBound.selector); - bCoWPool.verify(order); - } - - function test_Revert_ReceiverIsNotBCoWPool(address _receiver) public { - vm.assume(_receiver != GPv2Order.RECEIVER_SAME_AS_OWNER); - GPv2Order.Data memory order = correctOrder; - order.receiver = _receiver; - vm.expectRevert(IBCoWPool.BCoWPool_ReceiverIsNotBCoWPool.selector); - bCoWPool.verify(order); - } - - function test_Revert_LargeDurationOrder(uint256 _timeOffset) public { - _timeOffset = bound(_timeOffset, MAX_ORDER_DURATION + 1, type(uint32).max - block.timestamp); - GPv2Order.Data memory order = correctOrder; - order.validTo = uint32(block.timestamp + _timeOffset); - vm.expectRevert(IBCoWPool.BCoWPool_OrderValidityTooLong.selector); - bCoWPool.verify(order); - } - - function test_Revert_NonZeroFee(uint256 _fee) public { - _fee = bound(_fee, 1, type(uint256).max); - GPv2Order.Data memory order = correctOrder; - order.feeAmount = _fee; - vm.expectRevert(IBCoWPool.BCoWPool_FeeMustBeZero.selector); - bCoWPool.verify(order); - } - - function test_Revert_InvalidOrderKind(bytes32 _orderKind) public { - vm.assume(_orderKind != GPv2Order.KIND_SELL); - GPv2Order.Data memory order = correctOrder; - order.kind = _orderKind; - vm.expectRevert(IBCoWPool.BCoWPool_InvalidOperation.selector); - bCoWPool.verify(order); - } - - function test_Revert_InvalidBuyBalanceKind(bytes32 _balanceKind) public { - vm.assume(_balanceKind != GPv2Order.BALANCE_ERC20); - GPv2Order.Data memory order = correctOrder; - order.buyTokenBalance = _balanceKind; - vm.expectRevert(IBCoWPool.BCoWPool_InvalidBalanceMarker.selector); - bCoWPool.verify(order); - } - - function test_Revert_InvalidSellBalanceKind(bytes32 _balanceKind) public { - vm.assume(_balanceKind != GPv2Order.BALANCE_ERC20); - GPv2Order.Data memory order = correctOrder; - order.sellTokenBalance = _balanceKind; - vm.expectRevert(IBCoWPool.BCoWPool_InvalidBalanceMarker.selector); - bCoWPool.verify(order); - } - - function test_Revert_TokenAmountInAboveMaxIn( - SwapExactAmountIn_FuzzScenario memory _fuzz, - uint256 _offset - ) public happyPath(_fuzz) { - _offset = bound(_offset, 1, type(uint256).max - _fuzz.tokenInBalance); - uint256 _tokenAmountIn = bmul(_fuzz.tokenInBalance, MAX_IN_RATIO) + _offset; - GPv2Order.Data memory order = correctOrder; - order.buyAmount = _tokenAmountIn; - - vm.expectRevert(IBPool.BPool_TokenAmountInAboveMaxRatio.selector); - bCoWPool.verify(order); - } - - function test_Revert_InsufficientReturn( - SwapExactAmountIn_FuzzScenario memory _fuzz, - uint256 _offset - ) public happyPath(_fuzz) { - uint256 _tokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, _fuzz.tokenInDenorm, _fuzz.tokenOutBalance, _fuzz.tokenOutDenorm, _fuzz.tokenAmountIn, 0 - ); - _offset = bound(_offset, 1, _tokenAmountOut); - GPv2Order.Data memory order = correctOrder; - order.buyAmount = _fuzz.tokenAmountIn; - order.sellAmount = _tokenAmountOut + _offset; - - vm.expectRevert(IBPool.BPool_TokenAmountOutBelowMinOut.selector); - bCoWPool.verify(order); - } - - function test_Success_HappyPath(SwapExactAmountIn_FuzzScenario memory _fuzz) public happyPath(_fuzz) { - uint256 _tokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, _fuzz.tokenInDenorm, _fuzz.tokenOutBalance, _fuzz.tokenOutDenorm, _fuzz.tokenAmountIn, 0 - ); - GPv2Order.Data memory order = correctOrder; - order.buyAmount = _fuzz.tokenAmountIn; - order.sellAmount = _tokenAmountOut; - - bCoWPool.verify(order); - } -} - contract BCoWPool_Unit_IsValidSignature is BaseCoWPoolTest { function setUp() public virtual override { super.setUp(); diff --git a/test/unit/BPool.t.sol b/test/unit/BPool.t.sol index ddd5182b..6627eb9e 100644 --- a/test/unit/BPool.t.sol +++ b/test/unit/BPool.t.sol @@ -248,111 +248,6 @@ abstract contract BasePoolTest is Test, BConst, Utils, BMath { } } -abstract contract SwapExactAmountInUtils is BasePoolTest { - address tokenIn; - address tokenOut; - - struct SwapExactAmountIn_FuzzScenario { - uint256 tokenAmountIn; - uint256 tokenInBalance; - uint256 tokenInDenorm; - uint256 tokenOutBalance; - uint256 tokenOutDenorm; - uint256 swapFee; - } - - function _setValues(SwapExactAmountIn_FuzzScenario memory _fuzz) internal { - tokenIn = tokens[0]; - tokenOut = tokens[1]; - - // Create mocks for tokenIn and tokenOut (only use the first 2 tokens) - _mockTransferFrom(tokenIn); - _mockTransfer(tokenOut); - - // Set balances - bPool.set__records( - tokenIn, - IBPool.Record({ - bound: true, - index: 0, // NOTE: irrelevant for this method - denorm: _fuzz.tokenInDenorm - }) - ); - _mockPoolBalance(tokenIn, _fuzz.tokenInBalance); - - bPool.set__records( - tokenOut, - IBPool.Record({ - bound: true, - index: 0, // NOTE: irrelevant for this method - denorm: _fuzz.tokenOutDenorm - }) - ); - _mockPoolBalance(tokenOut, _fuzz.tokenOutBalance); - - // Set swapFee - _setSwapFee(_fuzz.swapFee); - // Set finalize - _setFinalize(true); - } - - function _assumeHappyPath(SwapExactAmountIn_FuzzScenario memory _fuzz) internal view virtual { - // safe bound assumptions - _fuzz.tokenInDenorm = bound(_fuzz.tokenInDenorm, MIN_WEIGHT, MAX_WEIGHT); - _fuzz.tokenOutDenorm = bound(_fuzz.tokenOutDenorm, MIN_WEIGHT, MAX_WEIGHT); - _fuzz.swapFee = bound(_fuzz.swapFee, MIN_FEE, MAX_FEE); - - // min - max - calcSpotPrice (spotPriceBefore) - _fuzz.tokenInBalance = bound(_fuzz.tokenInBalance, MIN_BALANCE, type(uint256).max / _fuzz.tokenInDenorm); - _fuzz.tokenOutBalance = bound(_fuzz.tokenOutBalance, MIN_BALANCE, type(uint256).max / _fuzz.tokenOutDenorm); - - // max - calcSpotPrice (spotPriceAfter) - vm.assume(_fuzz.tokenAmountIn < type(uint256).max - _fuzz.tokenInBalance); - vm.assume(_fuzz.tokenInBalance + _fuzz.tokenAmountIn < type(uint256).max / _fuzz.tokenInDenorm); - - // internal calculation for calcSpotPrice (spotPriceBefore) - _assumeCalcSpotPrice( - _fuzz.tokenInBalance, _fuzz.tokenInDenorm, _fuzz.tokenOutBalance, _fuzz.tokenOutDenorm, _fuzz.swapFee - ); - - // MAX_IN_RATIO - vm.assume(_fuzz.tokenAmountIn <= bmul(_fuzz.tokenInBalance, MAX_IN_RATIO)); - - // L338 BPool.sol - uint256 _spotPriceBefore = calcSpotPrice( - _fuzz.tokenInBalance, _fuzz.tokenInDenorm, _fuzz.tokenOutBalance, _fuzz.tokenOutDenorm, _fuzz.swapFee - ); - - _assumeCalcOutGivenIn(_fuzz.tokenInBalance, _fuzz.tokenAmountIn, _fuzz.swapFee); - uint256 _tokenAmountOut = calcOutGivenIn( - _fuzz.tokenInBalance, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance, - _fuzz.tokenOutDenorm, - _fuzz.tokenAmountIn, - _fuzz.swapFee - ); - vm.assume(_tokenAmountOut > BONE); - - // internal calculation for calcSpotPrice (spotPriceAfter) - _assumeCalcSpotPrice( - _fuzz.tokenInBalance + _fuzz.tokenAmountIn, - _fuzz.tokenInDenorm, - _fuzz.tokenOutBalance - _tokenAmountOut, - _fuzz.tokenOutDenorm, - _fuzz.swapFee - ); - - vm.assume(bmul(_spotPriceBefore, _tokenAmountOut) <= _fuzz.tokenAmountIn); - } - - modifier happyPath(SwapExactAmountIn_FuzzScenario memory _fuzz) { - _assumeHappyPath(_fuzz); - _setValues(_fuzz); - _; - } -} - contract BPool_Unit_GetCurrentTokens is BasePoolTest { function test_Returns_CurrentTokens(uint256 _length) public { vm.assume(_length > 0); From c9d196be1cde350fdf02150bcab6797c894a297d Mon Sep 17 00:00:00 2001 From: teddy Date: Mon, 15 Jul 2024 15:58:06 -0300 Subject: [PATCH 10/16] chore: testcase renaming from review --- test/unit/BCoWPool/BCoWPool_Verify.t.sol | 12 ++++++------ test/unit/BCoWPool/BCoWPool_Verify.tree | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/test/unit/BCoWPool/BCoWPool_Verify.t.sol b/test/unit/BCoWPool/BCoWPool_Verify.t.sol index ea756477..bc0ab998 100644 --- a/test/unit/BCoWPool/BCoWPool_Verify.t.sol +++ b/test/unit/BCoWPool/BCoWPool_Verify.t.sol @@ -58,14 +58,14 @@ contract BCoWPoolVerify is BCoWPoolBase { bCoWPool.verify(correctOrder); } - function test_RevertWhen_OrderReceiverIsNotRECEIVER_SAME_AS_OWNERFlag() external { + function test_RevertWhen_OrderReceiverFlagIsNotSameAsOwner() external { correctOrder.receiver = makeAddr('somebodyElse'); // it should revert vm.expectRevert(IBCoWPool.BCoWPool_ReceiverIsNotBCoWPool.selector); bCoWPool.verify(correctOrder); } - function test_RevertWhen_OrderIsValidForMoreThanMAX_ORDER_DURATION(uint256 _timeOffset) external { + function test_RevertWhen_OrderValidityIsTooLong(uint256 _timeOffset) external { _timeOffset = bound(_timeOffset, MAX_ORDER_DURATION + 1, type(uint32).max - block.timestamp); correctOrder.validTo = uint32(block.timestamp + _timeOffset); // it should revert @@ -89,7 +89,7 @@ contract BCoWPoolVerify is BCoWPoolBase { bCoWPool.verify(correctOrder); } - function test_RevertWhen_TokenBalanceMarkerForBuyTokenIsNotDirectERC20Balances(bytes32 _balanceKind) external { + function test_RevertWhen_BuyTokenBalanceFlagIsNotERC20Balances(bytes32 _balanceKind) external { vm.assume(_balanceKind != GPv2Order.BALANCE_ERC20); correctOrder.buyTokenBalance = _balanceKind; // it should revert @@ -97,7 +97,7 @@ contract BCoWPoolVerify is BCoWPoolBase { bCoWPool.verify(correctOrder); } - function test_RevertWhen_TokenBalanceMarkerForSellTokenIsNotDirectERC20Balances(bytes32 _balanceKind) external { + function test_RevertWhen_SellTokenBalanceFlagIsNotERC20Balances(bytes32 _balanceKind) external { vm.assume(_balanceKind != GPv2Order.BALANCE_ERC20); correctOrder.sellTokenBalance = _balanceKind; // it should revert @@ -105,7 +105,7 @@ contract BCoWPoolVerify is BCoWPoolBase { bCoWPool.verify(correctOrder); } - function test_RevertWhen_OrderBuyAmountExceedsMAX_IN_RATIO(uint256 _buyAmount) external { + function test_RevertWhen_OrderBuyAmountExceedsMaxRatio(uint256 _buyAmount) external { _buyAmount = bound(_buyAmount, bmul(tokenInBalance, MAX_IN_RATIO) + 1, type(uint256).max); correctOrder.buyAmount = _buyAmount; // it should revert @@ -113,7 +113,7 @@ contract BCoWPoolVerify is BCoWPoolBase { bCoWPool.verify(correctOrder); } - function test_RevertWhen_ComputedTokenAmountOutIsLessThanOrderMinSellAmount() external { + function test_RevertWhen_CalculatedTokenAmountOutIsLessThanOrderSellAmount() external { correctOrder.sellAmount += 1; // it should revert vm.expectRevert(IBPool.BPool_TokenAmountOutBelowMinOut.selector); diff --git a/test/unit/BCoWPool/BCoWPool_Verify.tree b/test/unit/BCoWPool/BCoWPool_Verify.tree index e4037e67..5b0f142d 100644 --- a/test/unit/BCoWPool/BCoWPool_Verify.tree +++ b/test/unit/BCoWPool/BCoWPool_Verify.tree @@ -3,21 +3,21 @@ BCoWPool::Verify │ └── it should revert ├── when sellToken is not bound │ └── it should revert -├── when order receiver is not RECEIVER_SAME_AS_OWNER flag +├── when order receiver flag is not same as owner │ └── it should revert -├── when order is valid for more than MAX_ORDER_DURATION +├── when order validity is too long │ └── it should revert ├── when fee amount is not zero │ └── it should revert ├── when order kind is not KIND_SELL │ └── it should revert -├── when TokenBalance marker for buy token is not direct ERC20 balances +├── when buy token balance flag is not ERC20 balances │ └── it should revert -├── when TokenBalance marker for sell token is not direct ERC20 balances +├── when sell token balance flag is not ERC20 balances │ └── it should revert -├── when order buy amount exceeds MAX_IN_RATIO +├── when order buy amount exceeds max ratio │ └── it should revert -├── when computed token amount out is less than order min sell amount +├── when calculated token amount out is less than order sell amount │ └── it should revert └── when preconditions are met └── it should return From 69f49e70f968a095104114b91db149d9936b95e4 Mon Sep 17 00:00:00 2001 From: teddy Date: Mon, 15 Jul 2024 16:03:19 -0300 Subject: [PATCH 11/16] chore: get rid of _setTokens altogether --- test/unit/BPool.t.sol | 6 +----- test/unit/BPool/BPoolBase.sol | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/test/unit/BPool.t.sol b/test/unit/BPool.t.sol index 6627eb9e..27414ed3 100644 --- a/test/unit/BPool.t.sol +++ b/test/unit/BPool.t.sol @@ -36,7 +36,7 @@ abstract contract BasePoolTest is Test, BConst, Utils, BMath { for (uint256 i = 0; i < _length; i++) { bPool.set__records(_tokensToAdd[i], IBPool.Record({bound: true, index: i, denorm: 0})); } - _setTokens(_tokensToAdd); + bPool.set__tokens(_tokensToAdd); } function _mockTransfer(address _token) internal { @@ -53,10 +53,6 @@ abstract contract BasePoolTest is Test, BConst, Utils, BMath { vm.mockCall(_token, abi.encodeWithSelector(IERC20.balanceOf.selector, address(bPool)), abi.encode(_balance)); } - function _setTokens(address[] memory _tokens) internal { - bPool.set__tokens(_tokens); - } - function _setSwapFee(uint256 _swapFee) internal { bPool.set__swapFee(_swapFee); } diff --git a/test/unit/BPool/BPoolBase.sol b/test/unit/BPool/BPoolBase.sol index a205e743..4e9ccd21 100644 --- a/test/unit/BPool/BPoolBase.sol +++ b/test/unit/BPool/BPoolBase.sol @@ -26,10 +26,6 @@ contract BPoolBase is Test, BConst, Utils { for (uint256 i = 0; i < _length; i++) { bPool.set__records(_tokensToAdd[i], IBPool.Record({bound: true, index: i, denorm: 0})); } - _setTokens(_tokensToAdd); - } - - function _setTokens(address[] memory _tokens) internal { - bPool.set__tokens(_tokens); + bPool.set__tokens(_tokensToAdd); } } From 23c5a1b7d65de1b26af52a7c21b4cc33ed3dcaeb Mon Sep 17 00:00:00 2001 From: teddy Date: Mon, 15 Jul 2024 16:14:24 -0300 Subject: [PATCH 12/16] test: fuzz all possible valid order.sellAmount values --- test/unit/BCoWPool/BCoWPool_Verify.t.sol | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/unit/BCoWPool/BCoWPool_Verify.t.sol b/test/unit/BCoWPool/BCoWPool_Verify.t.sol index bc0ab998..dcdf37e1 100644 --- a/test/unit/BCoWPool/BCoWPool_Verify.t.sol +++ b/test/unit/BCoWPool/BCoWPool_Verify.t.sol @@ -120,7 +120,9 @@ contract BCoWPoolVerify is BCoWPoolBase { bCoWPool.verify(correctOrder); } - function test_WhenPreconditionsAreMet() external { + function test_WhenPreconditionsAreMet(uint256 _sellAmount) external { + _sellAmount = bound(_sellAmount, 0, correctOrder.sellAmount); + correctOrder.sellAmount = _sellAmount; // it should return bCoWPool.verify(correctOrder); } From a134e2cf812589421bf3d9cca30956684c7fa86d Mon Sep 17 00:00:00 2001 From: teddy Date: Mon, 15 Jul 2024 16:15:48 -0300 Subject: [PATCH 13/16] chore: rename correctOrder -> validOrder --- test/unit/BCoWPool/BCoWPool_Verify.t.sol | 50 ++++++++++++------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/test/unit/BCoWPool/BCoWPool_Verify.t.sol b/test/unit/BCoWPool/BCoWPool_Verify.t.sol index dcdf37e1..983d0d91 100644 --- a/test/unit/BCoWPool/BCoWPool_Verify.t.sol +++ b/test/unit/BCoWPool/BCoWPool_Verify.t.sol @@ -18,7 +18,7 @@ contract BCoWPoolVerify is BCoWPoolBase { uint256 public tokenOutWeight = 1e18; // from bmath: (with fee zero) 80*(1-(100/(100+1))^(4)) uint256 public expectedAmountOut = 3.12157244137469736e18; - GPv2Order.Data correctOrder; + GPv2Order.Data validOrder; function setUp() public virtual override { super.setUp(); @@ -28,7 +28,7 @@ contract BCoWPoolVerify is BCoWPoolBase { vm.mockCall(tokenIn, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(tokenInBalance))); vm.mockCall(tokenOut, abi.encodePacked(IERC20.balanceOf.selector), abi.encode(uint256(tokenOutBalance))); - correctOrder = GPv2Order.Data({ + validOrder = GPv2Order.Data({ sellToken: IERC20(tokenOut), buyToken: IERC20(tokenIn), receiver: GPv2Order.RECEIVER_SAME_AS_OWNER, @@ -45,85 +45,85 @@ contract BCoWPoolVerify is BCoWPoolBase { } function test_RevertWhen_BuyTokenIsNotBound() external { - correctOrder.buyToken = IERC20(makeAddr('unknown token')); + validOrder.buyToken = IERC20(makeAddr('unknown token')); // it should revert vm.expectRevert(IBPool.BPool_TokenNotBound.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_RevertWhen_SellTokenIsNotBound() external { - correctOrder.sellToken = IERC20(makeAddr('unknown token')); + validOrder.sellToken = IERC20(makeAddr('unknown token')); // it should revert vm.expectRevert(IBPool.BPool_TokenNotBound.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_RevertWhen_OrderReceiverFlagIsNotSameAsOwner() external { - correctOrder.receiver = makeAddr('somebodyElse'); + validOrder.receiver = makeAddr('somebodyElse'); // it should revert vm.expectRevert(IBCoWPool.BCoWPool_ReceiverIsNotBCoWPool.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_RevertWhen_OrderValidityIsTooLong(uint256 _timeOffset) external { _timeOffset = bound(_timeOffset, MAX_ORDER_DURATION + 1, type(uint32).max - block.timestamp); - correctOrder.validTo = uint32(block.timestamp + _timeOffset); + validOrder.validTo = uint32(block.timestamp + _timeOffset); // it should revert vm.expectRevert(IBCoWPool.BCoWPool_OrderValidityTooLong.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_RevertWhen_FeeAmountIsNotZero(uint256 _fee) external { _fee = bound(_fee, 1, type(uint256).max); - correctOrder.feeAmount = _fee; + validOrder.feeAmount = _fee; // it should revert vm.expectRevert(IBCoWPool.BCoWPool_FeeMustBeZero.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_RevertWhen_OrderKindIsNotKIND_SELL(bytes32 _orderKind) external { vm.assume(_orderKind != GPv2Order.KIND_SELL); - correctOrder.kind = _orderKind; + validOrder.kind = _orderKind; // it should revert vm.expectRevert(IBCoWPool.BCoWPool_InvalidOperation.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_RevertWhen_BuyTokenBalanceFlagIsNotERC20Balances(bytes32 _balanceKind) external { vm.assume(_balanceKind != GPv2Order.BALANCE_ERC20); - correctOrder.buyTokenBalance = _balanceKind; + validOrder.buyTokenBalance = _balanceKind; // it should revert vm.expectRevert(IBCoWPool.BCoWPool_InvalidBalanceMarker.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_RevertWhen_SellTokenBalanceFlagIsNotERC20Balances(bytes32 _balanceKind) external { vm.assume(_balanceKind != GPv2Order.BALANCE_ERC20); - correctOrder.sellTokenBalance = _balanceKind; + validOrder.sellTokenBalance = _balanceKind; // it should revert vm.expectRevert(IBCoWPool.BCoWPool_InvalidBalanceMarker.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_RevertWhen_OrderBuyAmountExceedsMaxRatio(uint256 _buyAmount) external { _buyAmount = bound(_buyAmount, bmul(tokenInBalance, MAX_IN_RATIO) + 1, type(uint256).max); - correctOrder.buyAmount = _buyAmount; + validOrder.buyAmount = _buyAmount; // it should revert vm.expectRevert(IBPool.BPool_TokenAmountInAboveMaxRatio.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_RevertWhen_CalculatedTokenAmountOutIsLessThanOrderSellAmount() external { - correctOrder.sellAmount += 1; + validOrder.sellAmount += 1; // it should revert vm.expectRevert(IBPool.BPool_TokenAmountOutBelowMinOut.selector); - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } function test_WhenPreconditionsAreMet(uint256 _sellAmount) external { - _sellAmount = bound(_sellAmount, 0, correctOrder.sellAmount); - correctOrder.sellAmount = _sellAmount; + _sellAmount = bound(_sellAmount, 0, validOrder.sellAmount); + validOrder.sellAmount = _sellAmount; // it should return - bCoWPool.verify(correctOrder); + bCoWPool.verify(validOrder); } } From 068e06f832a52a39b86a1e3e81a3f4bb806c8d66 Mon Sep 17 00:00:00 2001 From: teddy Date: Wed, 17 Jul 2024 19:30:15 -0300 Subject: [PATCH 14/16] fix: rename base file so it is skipped by coverage --- test/unit/BCoWPool/{BCoWPoolBase.sol => BCoWPoolBase.t.sol} | 0 test/unit/BCoWPool/BCoWPool_Verify.t.sol | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename test/unit/BCoWPool/{BCoWPoolBase.sol => BCoWPoolBase.t.sol} (100%) diff --git a/test/unit/BCoWPool/BCoWPoolBase.sol b/test/unit/BCoWPool/BCoWPoolBase.t.sol similarity index 100% rename from test/unit/BCoWPool/BCoWPoolBase.sol rename to test/unit/BCoWPool/BCoWPoolBase.t.sol diff --git a/test/unit/BCoWPool/BCoWPool_Verify.t.sol b/test/unit/BCoWPool/BCoWPool_Verify.t.sol index 983d0d91..efd0a20f 100644 --- a/test/unit/BCoWPool/BCoWPool_Verify.t.sol +++ b/test/unit/BCoWPool/BCoWPool_Verify.t.sol @@ -4,7 +4,7 @@ pragma solidity 0.8.25; import {IERC20} from '@cowprotocol/interfaces/IERC20.sol'; import {GPv2Order} from '@cowprotocol/libraries/GPv2Order.sol'; -import {BCoWPoolBase} from './BCoWPoolBase.sol'; +import {BCoWPoolBase} from './BCoWPoolBase.t.sol'; import {IBCoWPool} from 'interfaces/IBCoWPool.sol'; import {IBPool} from 'interfaces/IBPool.sol'; From 56dcaa8ee03836ae0ccec79269bb2a9e9f2f05d4 Mon Sep 17 00:00:00 2001 From: teddy Date: Wed, 17 Jul 2024 19:40:00 -0300 Subject: [PATCH 15/16] test: ensure verify asks for ERC20 balances --- test/unit/BCoWPool/BCoWPool_Verify.t.sol | 5 ++++- test/unit/BCoWPool/BCoWPool_Verify.tree | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/test/unit/BCoWPool/BCoWPool_Verify.t.sol b/test/unit/BCoWPool/BCoWPool_Verify.t.sol index efd0a20f..d4af3924 100644 --- a/test/unit/BCoWPool/BCoWPool_Verify.t.sol +++ b/test/unit/BCoWPool/BCoWPool_Verify.t.sol @@ -123,7 +123,10 @@ contract BCoWPoolVerify is BCoWPoolBase { function test_WhenPreconditionsAreMet(uint256 _sellAmount) external { _sellAmount = bound(_sellAmount, 0, validOrder.sellAmount); validOrder.sellAmount = _sellAmount; - // it should return + // it should ask the balance of the buy token + vm.expectCall(tokenIn, abi.encodeCall(IERC20.balanceOf, (address(bCoWPool)))); + // it should ask the balance of the sell token + vm.expectCall(tokenOut, abi.encodeCall(IERC20.balanceOf, (address(bCoWPool)))); bCoWPool.verify(validOrder); } } diff --git a/test/unit/BCoWPool/BCoWPool_Verify.tree b/test/unit/BCoWPool/BCoWPool_Verify.tree index 5b0f142d..771a2ae0 100644 --- a/test/unit/BCoWPool/BCoWPool_Verify.tree +++ b/test/unit/BCoWPool/BCoWPool_Verify.tree @@ -20,4 +20,5 @@ BCoWPool::Verify ├── when calculated token amount out is less than order sell amount │ └── it should revert └── when preconditions are met - └── it should return + ├── it should ask the balance of the buy token + └── it should ask the balance of the sell token From 898d135b0a8f4bad312ee98d837831004c20b553 Mon Sep 17 00:00:00 2001 From: teddy Date: Mon, 22 Jul 2024 13:44:28 -0300 Subject: [PATCH 16/16] chore: make bun happy --- test/unit/BCoWPool/BCoWPool_Verify.t.sol | 4 ++-- test/unit/BCoWPool/BCoWPool_Verify.tree | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/BCoWPool/BCoWPool_Verify.t.sol b/test/unit/BCoWPool/BCoWPool_Verify.t.sol index d4af3924..3af73f50 100644 --- a/test/unit/BCoWPool/BCoWPool_Verify.t.sol +++ b/test/unit/BCoWPool/BCoWPool_Verify.t.sol @@ -123,9 +123,9 @@ contract BCoWPoolVerify is BCoWPoolBase { function test_WhenPreconditionsAreMet(uint256 _sellAmount) external { _sellAmount = bound(_sellAmount, 0, validOrder.sellAmount); validOrder.sellAmount = _sellAmount; - // it should ask the balance of the buy token + // it should query the balance of the buy token vm.expectCall(tokenIn, abi.encodeCall(IERC20.balanceOf, (address(bCoWPool)))); - // it should ask the balance of the sell token + // it should query the balance of the sell token vm.expectCall(tokenOut, abi.encodeCall(IERC20.balanceOf, (address(bCoWPool)))); bCoWPool.verify(validOrder); } diff --git a/test/unit/BCoWPool/BCoWPool_Verify.tree b/test/unit/BCoWPool/BCoWPool_Verify.tree index 771a2ae0..56187a2a 100644 --- a/test/unit/BCoWPool/BCoWPool_Verify.tree +++ b/test/unit/BCoWPool/BCoWPool_Verify.tree @@ -20,5 +20,5 @@ BCoWPool::Verify ├── when calculated token amount out is less than order sell amount │ └── it should revert └── when preconditions are met - ├── it should ask the balance of the buy token - └── it should ask the balance of the sell token + ├── it should query the balance of the buy token + └── it should query the balance of the sell token