-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix: proportional slippage #424
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
packages/lib/modules/pool/actions/add-liquidity/AddLiquidityProvider.tsx
Show resolved
Hide resolved
packages/lib/modules/pool/actions/add-liquidity/AddLiquidityProvider.tsx
Show resolved
Hide resolved
For example, addLiquidityBoosted queries with tokens with 6 decimals would always fail if we used 0% slippage by default. | ||
This 0.01% is big enough to prevent tx simulation errors in all types of proportional adds while keeping the dust amount small. | ||
*/ | ||
export const defaultProportionalSlippagePercentage = '0.01' |
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.
But I thought the SDK rounded down? The only reason the SDK calcs should fail with 0% slippage is if the pool balances move.
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.
The whole reason we are using 0% slippage is to help the user avoid dust.
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.
There are cases where rounding down is not enough due to SDK query not being 100% precise as the comment explains. @brunoguerios has more context but it's an SC limitation.
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.
Yeah, the problem lies in the AddLiquidityBoosted queries not matching the actual transaction 100%. It's an SC limitation.
The error is ~1000 wei, which is negligible on 18 decimal tokens, but not as much on 6 decimals tokens 😕
If you'd like to get even more context, we'd have to get SC team involved in the discussion.
For all other queries that are an exact match, rounding down should be enough.
@gosuto.eth We finally merged the fix.
Recap: pools containing
Erc4626
tokens (no matter if they are boosted or not) require a default 0.01 slippage in proportional mode cause the related SDK queries are not 100% accurate in that case (due to SC limitations). That small slippage + the already existing SDK rounding should avoid the error (some dust is the price to pay in this scenario).Fixes slippage bug reported by Gosuto:
balancer/b-sdk#552
Before:
Using slippage 0% by default causes a tx simulation error due
addLiquidityBoosted queries
not being 100% preciseAlso real slippage was not properly displayed in the quote.
After:
Using 0.01 % slippage by default fixes the issue.
Also real slippage is now properly displayed.
There are other proportional scenarios where the SDK queries are more precise so we could use 0% or 0.0001% instead or 0.01% but I think it is not worthy the complexity.