-
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
chore: BalCoW v1.3.0 release #135
Conversation
* chore: smallest of fixes * feat: dont publicly expose the current commitment * feat: use _viewlock_ check for commitment * docs: improve readme * feat: use SafeERC20 to support tokens without transfer return value * test: assert the expected pool bytecode is deployed * feat: use SafeERC20 to support underlying tokens without transfer return value
* fix: renaming TokenAmountInAboveMaxIn for AboveMaxRatio * fix: swapExactAmountOut natspec
* ci: ensure gas snapshots are updated * chore: break snapshots to see if it is caught * fix: fetch some git history so diff has something to compare against * fix: run diff after running tests * fix: rename gas snapshot test so its run in CI * fix: run diff after running _integration_ tests * chore: update forge snapshots to their current values
* chore: checkout btt bfactory tests from feat/new-unit-tests * chore: update BFactory tests for custom errors * chore: get BCoWFactory tests to run for the time being * test: mock _newBPool and test it separately * test: unit test methods defined in BCoWFactory * fix: feedback from review * chore: setBLabs -> BLabs * fix: feedback from review
* chore: adding BNum test files from #122 * fix: inheriting BConst * fix: reusing constants * feat: improving btt test with fuzzing * chore: cleanup tree and tests * feat: minor improvements to tree + fuzz * fix: minor tweaks to tree and comments * fix: clean up bound * fix: test:scaffold script * fix: applying tree updated names * fix: bdiv tests * fix: improving tree branching and expectations * fix: rm unnecessary conditional * fix: revert dumb bun break * fix: example parenthesis
* test: adding BTT tests for BMath (#123) * fix: improving tree and filling empty blocks * fix: solhint json * fix: branch phrasing * fix: parenthesis * fix: rm revert reason branches * fix: improving calc spot price branches * fix: improving calcOutGivenIn branches and tests * feat: improving calc In and Out branches and tests * feat: improving calcPoolOut tree and tests * feat: adding calcSingleIn calcs * feat: adding calcSingleOut calcs * feat: adding caclPoolIn calcs expectations * feat: improving calcPoolIn tests * feat: populating missing tests * refactor: mv setup to file start * feat: cleanup internal branch assumptions * fix: rm fn mutability warnings * feat: adding some missing branches * fix: revert expectation comment --------- Co-authored-by: teddy <[email protected]>
* feat: adding address zero checks in setters * fix: updating gas snapshots * fix: comment in test
* ci: setup bulloak in CI * fix: use explicit search instead of depending on bulloaks globbing
* feat: adding join and exit snaps * feat: caching array.length into memory * feat: changing /2 for >>1 when possible * feat: making join/exit integration test 3-token * feat: making factory immutable in BPool * fix: comments * fix: failing bytecode match tests * fix: optimizing _afterFinalize loop * fix: wbtc units in integration test
* fix: failing tests on setController (address zero) * fix: update gas snapshot
* fix: typos in IBCoWPool * fix: replace erc20 for token * fix: increase/decrease approval arg names * chore: update gas snapshots * fix: consistent argument naming * fix: rm global ffi and fix yarn:test * fix: consistent _newBPool return * fix: rm obvious return arg names * fix: consistent returns in BPool * fix: using trailing underscore for orderHash * fix: adding strict linter rule and fixing issues * feat: adding deny warnings to foundry toml * fix: update gas snapshot * fix: failing tests * fix: rm deny warnings * feat: raising optimizer to 500 runs * fix: failing test
* test: migrate some easy methods to BTT * test: btt tests for bind * test: btt tests for unbind * refactor: move BPoolBase onto its own file to please bulloak * fix: fix bind/unbind tree * refactor: manually mock every bind side effect for unbind * chore: renames in bpool tree * fix: various renames in .tree and .t.sol from code review * refactor: every non-trivial method has its own tree now * refactor: better mocking * refactor: flatten trees * fix: small fixes from code review * test: fuzz start total weight and existing tokens * test: test the entire behaviour of reentrancy locks * fix: consistent bind and unbind trees (#140) * fix: consistent bind and unbind trees * fix: reentrancy lock expectations * fix: unfuzzing initial weight in bind --------- Co-authored-by: Weißer Hase <[email protected]>
* feat: adding finalized and notFinalized modifiers to BPool * fix: rm global ffi and fix yarn:test * fix: setSwapFee is notFinalized * fix: rm remanent changes
* ci: ensure smock files are up to date * chore: update existing smocked files
* feat: make a pools factory public * chore: use autogenerated mockbpool * fix: nits * fix: gas snapshot --------- Co-authored-by: Weißer Hase <[email protected]> Co-authored-by: Weißer Hase <[email protected]>
* fix: max order duration check * fix: rm SpotPriceAfterBelowMaxPrice error for SpotPriceAboveMaxPrice * fix: test names * fix: gas snapshot
* fix: isBPool argument name * fix: updating smock contracts to latest changes * fix: gas snapshot * fix: fmt
* chore: mock internal bpool methods * chore: move common secondToken up to base contract * test: btt joinPool tests * refactor: move bound token balance out of the base contract * fix: feedback from review * fix: update tree * chore: remove preexisting unit tests replaced by ones in this pr * fix: consistent tree indent * fix: join btt test file (#145) * fix: improving join BTT test file * fix: minor final touches * fix: using _mintPoolShare instead of deal --------- Co-authored-by: Weißer Hase <[email protected]>
* test: btt tests for exitPool * chore: delete preexisting tests * refactor: consistently use tokens array for tree tests * test: assume behaviour of _pullPoolShare correct * test: exitFee behaviour * test: small fixes from feedback * fix: only mock transfers where required
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.
approved!
* refactor: explicitly set weights in every scenario, remove it from base contract * test: btt tests for joinswapExternAmountIn * chore: remove preexisting unit tests replaced by ones in this pr * test: query tokenIn balance
* refactor: explicitly set weights in every scenario, remove it from base contract * test: btt tests for joinswapExternAmountIn * chore: remove preexisting unit tests replaced by ones in this pr * test: query tokenIn balance * test: btt tests for exitswapPoolAmountIn * chore: remove preexisting unit tests replaced by ones in this pr * fix: feedback from review
* test: btt tests for bpool.swapExactAmountIn * chore: delete preexisting unit tests * test: small renames from feedback * test: be explicit about untestable code * test: adding skipped test for unreachable condition * test: code wasnt so unreachable after all * refactor: get rid of _setRecord * test: btt tests for bcowpool.verify * chore: delete preexisting unit tests * chore: testcase renaming from review * chore: get rid of _setTokens altogether * test: fuzz all possible valid order.sellAmount values * chore: rename correctOrder -> validOrder * fix: rename base file so it is skipped by coverage * test: ensure verify asks for ERC20 balances * chore: make bun happy
* test: btt tests for bpool.swapExactAmountIn * chore: delete preexisting unit tests * test: small renames from feedback * test: be explicit about untestable code * test: adding skipped test for unreachable condition * test: code wasnt so unreachable after all * refactor: get rid of _setRecord * test: btt tests for bcowpool.verify * chore: delete preexisting unit tests * chore: testcase renaming from review * chore: get rid of _setTokens altogether * test: fuzz all possible valid order.sellAmount values * chore: rename correctOrder -> validOrder * test: btt tests for bpool.finalize * test: btt tests for bcowpool.finalize * chore: remove preexisting unit tests replaced by ones in this pr * fix: feedback from review * refactor: make caller==controller default scenario * fix: reorganize .tree * fix: feedback from review * fix: feedback from review, calling internal method directly
* test: btt tests for joinswapPoolAmountOut * fix: typos in tree * fix: missing stuff from dev mergeback
* test: btt tests for bcowpool.commit * chore: remove preexisting unit tests replaced by ones in this pr * test: reorganize commit tree * fix: make bulloak happy
* feat: adding BTT tests for BPool getters * feat: adding tests for spotPrice getters * feat: adding btt tests for bPool setters (#166) * fix: rm legacy bPool tests * fix: manually calculating spot price * fix: test issues on merge conflicts * fix: using funny spot prices --------- Co-authored-by: teddy <[email protected]>
* feat: non-weighted tradeable order test * fix: adding GetTradeableOrder library * feat: adding weights to the math * feat: variable weights * feat: adding test for helper * feat: adding support for weights * fix: fmt * fix: sending correct price vector * fix: making helper return valid signature * feat: deprecated weights in helper * feat: updated buyAmount as per aaf44a3 * refactor: mv GetTradeableOrder to libraries dir * fix: update comment on BCoWHelper * fix: rm unused variable warning * feat: adding BTT test for BCoWHelper * fix: linter warnings * fix: rm remanent line change * fix: rm unused lines * fix: missing natspec * fix: uncommenting assertion in test * fix: missing natspec * fix: safety checks comments * feat: vendoring interface from cow-amm * fix: lint run * chore: merge dev * fix: gas snapshot * refactor: reordered helper flow and cleanup * feat: vendoring GetTradeableOrder from cow-amm * feat: improving commitment expectation * fix: typos in comments * fix: overwriting sellAmount in order to avoid rounding issues (#154) * fix: typos in comments * fix: overwriting sellAmount in order to avoid rounding issues * feat: improving the test case to cover both cases * chore: addressing contract changes from PR comments * chore: addressing comments in tests * fix: adding helper mock * fix: addressing comments from PR * refactor: deprecate fuzzed integration valid order test in favour of unit test * feat: simplifying helper integration test * feat: improving unit test for valid order * feat: adding call tokens expectation * fix: branching different behaviours given skewness * fix: adding comments on the skewness sign
* feat: adding btt tests for bToken * feat: testing address zero checks on increase/decrease approvals * feat: adding more checks to bToken tests
* test: btt tests for bpool.swapExactAmountIn * chore: delete preexisting unit tests * test: small renames from feedback * test: be explicit about untestable code * test: adding skipped test for unreachable condition * test: code wasnt so unreachable after all * refactor: get rid of _setRecord * test: btt tests for bcowpool.verify * chore: delete preexisting unit tests * chore: testcase renaming from review * chore: get rid of _setTokens altogether * test: fuzz all possible valid order.sellAmount values * chore: rename correctOrder -> validOrder * test: btt tests for bcowpool.isValidSignature * chore: remove preexisting unit tests replaced by ones in this pr * fix: more descriptive tree
* test: btt tests for bpool.swapExactAmountIn * chore: delete preexisting unit tests * test: small renames from feedback * test: be explicit about untestable code * test: adding skipped test for unreachable condition * test: code wasnt so unreachable after all * refactor: get rid of _setRecord * test: btt tests for bcowpool.verify * chore: delete preexisting unit tests * chore: testcase renaming from review * chore: get rid of _setTokens altogether * test: fuzz all possible valid order.sellAmount values * chore: rename correctOrder -> validOrder * test: btt tests for bpool.finalize * test: btt tests for bcowpool.finalize * chore: remove preexisting unit tests replaced by ones in this pr * fix: feedback from review * refactor: make caller==controller default scenario * test: btt tests for bcowpool constructor * chore: remove preexisting unit tests replaced by ones in this pr * fix: feedback from review * fix: make bulloak happy * fix: mergeback mistake --------- Co-authored-by: Weißer Hase <[email protected]>
* 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: btt tests for exitswapExternAmountOut * chore: remove preexisting unit tests replaced by ones in this pr * fix: dont call setup twice * chore: finally remove bpool.t.sol
* chore: preparing package for release * fix: broken links
* feat: adding deployment script with faucet erc20s * fix: missing files * feat: update appData to a valid one * fix: natspec in IFaucet * fix: final tweaks * refactor: deployment script * chore: testnet deployment (updated registry) * fix: rm solhint 1-contract-per-file rule in script dir * fix: update bDao address * fix: gas snapshots * fix: refresh snapshots * fix: extra line * chore: testnet redeployment * chore: add grafiti to factory * chore: deployment addresses * chore: rm grafiti from factory * refactor: reorganize deployment and scripting architecture * fix: linter warnings * fix: deprecate mainnet and testnet id constants * fix: contract comments * feat: adding explicit revert on unknown chains * feat: adding BCoWHelper to deployment script and registry
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.
most comments are questions, a few can be easily fixed, can take care of those tomorrow
pool.bind(address(dai), DAI_LP_AMOUNT, 8e18); // 80% weight | ||
pool.bind(address(weth), WETH_LP_AMOUNT, 2e18); // 20% weight | ||
pool.bind(address(wbtc), WBTC_LP_AMOUNT, 10e18); // +100% weight (unused in swaps) |
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.
now it's 40% 10% 10%
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.
nee, is indistinct, because we're never swapping WBTC, from DAI-WETH perspective pool is 80-20
test/unit/BCoWPool/BCoWPoolBase.sol
Outdated
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.
we didn't remove this 😭
address public cowSolutionSettler = makeAddr('cowSolutionSettler'); | ||
bytes32 public domainSeparator = bytes32(bytes2(0xf00b)); | ||
address public vaultRelayer = makeAddr('vaultRelayer'); | ||
address public tokenIn; |
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.
we could get rid of these to be consistent with BPoolBase
test/unit/BPool/BPool_Bind.t.sol
Outdated
function setUp() public virtual override { | ||
super.setUp(); | ||
|
||
vm.mockCall(tokens[0], abi.encodePacked(IERC20.transferFrom.selector), abi.encode()); |
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.
opinion on giving tokens[0]
its own variable?
│ └── it should revert // division by zero | ||
├── when token balance out is zero | ||
│ └── it should return zero | ||
├── when swap fee and exit fee are zero |
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.
in retrospect, do you consider all of these testcases would have been easier/cleaner with an array of valid scenarios, like I just did for bPow
?
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.
wdym? we require both to be zero 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.
I commented on the wrong line, sorry. I meant all of the non-reverting cases that should return correct value
* chore: remove unused utils directory * test: missing case for joinswapPoolAmountOut * test: cover different bmul branches explicitly * test: cover {push,pull}PoolShares * test: token ratio > spot price before on swapExactAmountOut * test: cover bpowApprox with a few valid scenarios
* chore: rename BPoolBase.sol BPoolBase.t.sol * chore: remove BCoWPoolBase.sol * chore: remove token{In,Out} from base setup * chore: give token to bind its own variable
* chore: adding deployment addresses * fix: addressing comments in PR * fix: ouch
PR addresses changes proposed by Wonderland's Internal Review [W] and Certora's Security Assessment [C].
8a60ea2
#115EMPTY_COMMITMENT
(unused) chore: small improvements from8a60ea2
#115MAX_ORDER_DURATION
inminutes
chore: small improvements from8a60ea2
#1158a60ea2
#1158a60ea2
#115add address argument toTokenNotBound
error feat: adding address arg to TokenNotBound error #136MAX_ORDER_DURATION
check fix: more informational findings from Certora #138extras
bLabs
naming forbDao
fix: replace bLabs for bDao #147