Skip to content
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

V3 BatchSwaps #263

Merged
merged 19 commits into from
Mar 25, 2024
Merged

V3 BatchSwaps #263

merged 19 commits into from
Mar 25, 2024

Conversation

johngrantuk
Copy link
Member

@johngrantuk johngrantuk commented Feb 22, 2024

Biggest change is how we create call for V3 batchSwaps. Each independent path in swapExactIn/Out needs to have its minAmountOut/maxAmountIn set correctly using the user defined slippage (compared to V2 where we just updated a single limit index). Discussed with Daniel/Juani and we belive the Router swap queries (not curently implemented) will be able to return the updated path amounts. These can then be used with user definied slippage to update the relevant paths during call build.

@johngrantuk johngrantuk linked an issue Feb 22, 2024 that may be closed by this pull request
Copy link
Member

@brunoguerios brunoguerios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swap code is getting quite complex 👀
Makes sense given that we have to support single and batch swaps from 2 vault versions 😅
Overall I think looks good 👍
Will do a final review once Router is updated and we're able to add proper integration tests.

src/entities/swap/swapV3/index.ts Outdated Show resolved Hide resolved
src/entities/swap/types.ts Outdated Show resolved Hide resolved
@johngrantuk johngrantuk marked this pull request as ready for review March 21, 2024 11:02
Copy link
Member

@brunoguerios brunoguerios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall implementation looks really nice. 👏
I added some small comments for us to discuss.

Note: if you get the branch up to date with main, it has extended timeouts on the failing tests, so they don't break CI anymore.

src/entities/swap/types.ts Outdated Show resolved Hide resolved
src/entities/swap/types.ts Outdated Show resolved Hide resolved
test/anvil/anvil-global-setup.ts Show resolved Hide resolved
src/entities/swap/swaps/v3/index.ts Show resolved Hide resolved
Copy link
Member

@brunoguerios brunoguerios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍

@johngrantuk johngrantuk merged commit 39681ce into main Mar 25, 2024
4 checks passed
@johngrantuk johngrantuk deleted the 226-v3-batchswap branch March 25, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

V3: BatchSwap
2 participants