-
Notifications
You must be signed in to change notification settings - Fork 11
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
Refactor swap examples #510
Conversation
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 like this new flow!
Splitting query/transaction examples + reusing them is nice!
And the fact that you're not relying on manipulating storage slots for adding token balance to test accounts feels better as well.
setupExampleFork will get a bit more complex once you swap + add liquidity to prepare for other examples, but I'd say it's quite straightforward to follow.
One thing that came to my mind is that I'm used to leverage examples as debugging tools, where I simply adapt chains/pools/tokens to a different scenario to see if everything is working as expected. This new flow is a bit less flexible in that sense, but I'd say it makes sense to create separate debug files and use those instead of trying to reuse examples for that purpose.
…r-swap-examples
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 think this works great! 👏
I'd say we should follow this pattern with other actions (add/remove/etc..)
And it would be nice to sync so we make sure we're covering everything we need to provide good DX.
…r-swap-examples
Closes #339
My suggestion for re-shaping examples. Feel free to critique, modify, ect.
Summary
pnpm example ./examples/swaps/swapV2.ts
uses SOR path with old school v2 approval of vaultpnpm example ./examples/swaps/swapV3.ts
uses custom path with permit2 approvalConsole Logs