feat: adding bcowpool verify btt tests #155
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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]; | ||
0xteddybear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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); | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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({ | ||
0xteddybear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 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); | ||
| } | ||
|
There was a problem hiding this comment. since we checked for order There was a problem hiding this comment. seems reasonable, I don't think it's a different branch since it covers the exact same code. As far as I understood, we can use a single testcase for different values if they don't cover different lines/branches of code. CCing @drgorillamd in case he considers we should use a separate testcase There was a problem hiding this comment. This makes sense to me, you have a doubt about it? I'm missing something? |
||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
0xteddybear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| │ └── it should revert | ||
| ├── when order is valid for more than MAX_ORDER_DURATION | ||
0xteddybear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| │ └── 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 | ||
0xteddybear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| │ └── 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 | ||
0xteddybear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| │ └── it should revert | ||
| ├── when computed token amount out is less than order min sell amount | ||
0xteddybear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| │ └── it should revert | ||
| └── when preconditions are met | ||
| └── it should return | ||
|
There was a problem hiding this comment. i feel like we're forgetting about querying the balances, are we not? |
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
watch out this contract and BPoolBase don't have the
.t.solending, is this intended?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the idea being files where testcases are defined end in
.t.sol, while all others don't, I tried to be consistent withtest/utils/Utils.solnot ending in.t.solThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmmm i understand that the coverage for example skips all
.t.soltests alltogether, so perhaps we should name them all as.t.sol