-
Notifications
You must be signed in to change notification settings - Fork 15
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: Relative Slippage Limits (PRO-1207) #4547
Conversation
Looks fine to me - some tests would be nice... |
519acb3
to
a0e8ae5
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4547 +/- ##
=====================================
- Coverage 73% 73% -0%
=====================================
Files 401 401
Lines 66210 66303 +93
Branches 66210 66303 +93
=====================================
+ Hits 48090 48117 +27
- Misses 15802 15861 +59
- Partials 2318 2325 +7 ☔ View full report in Codecov by Sentry. |
a0e8ae5
to
c144792
Compare
/// Sets the maximum percentage increase (in number of ticks) of the price of the brought | ||
/// asset during a swap. |
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.
nit: I think this merits a better explanation. If it's measured in ticks, it's not really a percentage. It's also not necessarily an increase. Also, I think the price could actually move by more than the maximum expressed by this number (because of the whole max/min thing).
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.
Ticks have a direct relationship to a percentage (1 to 1 mapping). It is a percentage measured in ticks.
Not sure what you mean by it is not always an increase?
I'll add clarification regarding the fuzzy nature of the maximum.
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 thought ticks were multiplicative? They can be mapped to a percentage, but they aren't a representation of a percentage (not like pips/bips for example).
Re. increase - I meant the price can also decrease but I see you mention it's in terms of the 'bought asset'.
@@ -962,6 +964,21 @@ pub mod pallet { | |||
_ => Err(Error::<T>::UnsupportedCall)?, | |||
} | |||
} | |||
|
|||
/// Sets the allowed percentage increase (in number of ticks) of the price of the brought |
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.
should be "bought"
…ero-liquidity * origin/main: fix: disable try-state checks (#4576) chore: debug solana in CI 🐛 (#4580) refactor: pass tx_ref as an extrinsic parameter (#4579) fix: remove bounded balance check (#4575) Solana: update image to latest tag (#4574) feat: add boost lp account to bouncer and fund it on setup_swaps (#4552) feat: Expose tx_hash on BroadcastSuccess event (#4561) feat: Relative Slippage Limits (PRO-1207) (#4547) chore: disable localnet solana in CI ⏱️ (#4569) feat: store prewitnessed deposits with id (#4496) Feat: Scheduled Swaps Subscription (#4525) # Conflicts: # state-chain/pallets/cf-pools/src/tests.rs
…utxo * origin/main: feat: expose command for broker fee withdrawal (#4581) Chore/fix arbitrum deployment (#4570) Added Range order pool price to the pool_price_v2 rpc call (#4548) fix: just check that the balance after is greater than before (#4587) chore: add origin to ccm failed (#4586) fix: runtime upgrade state check uses AllPalletsWithoutSystem (#4583) chore: add zellic audit to repo (#4585) fix: disable try-state checks (#4576) chore: debug solana in CI 🐛 (#4580) refactor: pass tx_ref as an extrinsic parameter (#4579) fix: remove bounded balance check (#4575) Solana: update image to latest tag (#4574) feat: add boost lp account to bouncer and fund it on setup_swaps (#4552) feat: Expose tx_hash on BroadcastSuccess event (#4561) feat: Relative Slippage Limits (PRO-1207) (#4547) chore: disable localnet solana in CI ⏱️ (#4569) feat: store prewitnessed deposits with id (#4496) Feat: Scheduled Swaps Subscription (#4525)
Note: I noticed the benchmarks haven't been run for a while in the pools pallet, and so the schedule_limit_order_update didn't have upto date weights. So that is fixed here too.