-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: adding CoW helper MVP #121
Changes from 3 commits
699d9d7
271e00a
48888e3
28565f7
1a44f5e
8bf22dd
3683943
9422865
be1701d
24553ab
441d117
5630797
627d878
cb2fb52
5207c18
6f76f85
de99df4
bd64dca
ac17f69
6a14084
96f0302
e8c4d33
22480c5
3e5bfea
68e8368
5afd282
9390e64
b1ba43a
9ec1c34
05d6271
9923fc0
8d72ff8
699996e
1bec1fd
1ffda8b
063d3f6
471b0e6
e0de518
d0bba5c
d8c49e6
abe6f00
b2056be
ffb1bc0
7719c39
7db4e8e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -120,6 +120,10 @@ contract BCoWHelperTest is Test { | |
} | ||
|
||
function test_OrderWhenThePoolIsSupported(bytes32 domainSeparator) external { | ||
// it should call tokens | ||
helper.mock_call_tokens(address(pool), tokens); | ||
helper.expectCall_tokens(address(pool)); | ||
|
||
// it should query the domain separator from the pool | ||
pool.expectCall_SOLUTION_SETTLER_DOMAIN_SEPARATOR(); | ||
pool.mock_call_SOLUTION_SETTLER_DOMAIN_SEPARATOR(domainSeparator); | ||
|
@@ -157,8 +161,9 @@ contract BCoWHelperTest is Test { | |
|
||
function test_OrderGivenAPriceVector(uint256 priceSkewness, uint256 balanceToken0, uint256 balanceToken1) external { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't really follow what are the afaict we will skew the price by a particular factor from current spot price in order to allow for naturally ocurring slippage, is that correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. making it |
||
// skew the price by max 50% (more could result in reverts bc of max swap ratio) | ||
priceSkewness = bound(priceSkewness, 5000, 15_000); | ||
vm.assume(priceSkewness != 10_000); // avoids no-skewness revert | ||
uint256 base = 1e18; | ||
priceSkewness = bound(priceSkewness, 0.5e18, 1.5e18); | ||
vm.assume(priceSkewness != base); // avoids no-skewness revert | ||
|
||
balanceToken0 = bound(balanceToken0, 1e18, 1e36); | ||
0xteddybear marked this conversation as resolved.
Show resolved
Hide resolved
|
||
balanceToken1 = bound(balanceToken1, 1e18, 1e36); | ||
|
@@ -167,12 +172,12 @@ contract BCoWHelperTest is Test { | |
|
||
uint256[] memory prices = new uint256[](2); | ||
prices[0] = balanceToken1; | ||
prices[1] = balanceToken0 * priceSkewness / 10_000; | ||
prices[1] = balanceToken0 * priceSkewness / base; | ||
|
||
// it should return a valid pool order | ||
(GPv2Order.Data memory ammOrder,,,) = helper.order(address(pool), prices); | ||
|
||
assertEq(address(ammOrder.buyToken), priceSkewness > 10_000 ? tokens[0] : tokens[1]); | ||
assertEq(address(ammOrder.buyToken), priceSkewness > 1e18 ? tokens[0] : tokens[1]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use the |
||
|
||
// this call should not revert | ||
pool.verify(ammOrder); | ||
|
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.
I believe a unit test is enough for this
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.
this one was chosen to maintain the shape in case the feature gets added later, the unit test of the helper reverting is there, and the integration perspective we keep it here, in the future, another helper will not revert on weighted pools, and this test can be reimplemented not to revert
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.
... if this is meant to stay then wdyt about removing the commented code below?