fix: apply max_slip_fee cap in buy-side inversion - #1413
Conversation
|
Crate versions that have been updated:
Runtime version has not been increased. |
| *r_fresh.asset.delta_hub_reserve | ||
| ); | ||
| } | ||
|
|
There was a problem hiding this comment.
i would add a test for the case when both cap is fired, so the trade size is relative big compared to both asset in and asset out.
| // Step 2: Invert buy-side slip to find D_gross from D_net | ||
| let d_gross = if let Some(slip) = slip { | ||
| invert_buy_side_slip(d_net, slip.asset_out_hub_reserve, slip.asset_out_delta)? | ||
| let d_gross_uncapped = invert_buy_side_slip(d_net, slip.asset_out_hub_reserve, slip.asset_out_delta)?; |
There was a problem hiding this comment.
why we suddenly add bunch of code here ? same for the next change?
i havent thought it but i. believe it should be just possible to pass the max fee into the function and handle it there ?
There was a problem hiding this comment.
I intentionally kept the cap logic in calculate_buy_state_changes to keep invert_* functions as pure math without business constraints. Passing max_slip_fee into them would work but couples the inversion logic to a policy decision.
Happy to go either way though - from my side it was just a matter of preference
|
Quick benchmark at commit b5df01d has been executed successfully. |
|
in favor of #1456 |
3 participants
Description
Fixes a bug where
max_slip_feecap was not applied during buy-side trade inversion in theOmnipool math, breaking the invariant
delta_hub_reserve_in − total_protocol_fee = D_net.Changes:
calculate_buy_state_changes— afterinvert_buy_side_slip, check if cap fired and overrided_grosswith the capped linear formula; same check afterinvert_sell_side_feesfor thesell-side leg of buy inversion
buy_slip_fee_cap_invariant_holdsandsell_slip_fee_cap_invariant_holdsRelated Issue
#1412
Motivation and Context
The forward path (
calculate_slip_fee_amount) correctly caps the slip fee; the inverse path didnot - so the two paths produced inconsistent results when the slip rate exceeded
max_slip_fee.Sell trades are unaffected (no inversion involved).
How Has This Been Tested?
Two regression tests, one per inversion step, each verified to fail before the fix and pass after.
Checklist: