fix(order-handler): charge liquidation keeper fee to the trader, not the pool - #705
Merged
IbrahimIjai merged 2 commits intoAug 31, 2026
Conversation
…the pool (Closes SO4-Markets#540) liquidate_position pays the liquidation keeper's execution fee by withdrawing real tokens from the market pool via withdraw_from_pool, and (per the earlier SO4-Markets#629 fix) debits data_store's pool_amount ledger to match. But the in-memory `position` read at the top of the function was never written back to storage with the fee deducted, so the subsequent decrease_position call re-reads the full, un-reduced collateral_amount from storage and computes the trader's payout as if the fee had never been taken. Net effect: the fee's real token outflow is correctly tracked in pool_amount, but that ledger debit isn't backed by any reduction in what the trader is owed — the pool_amount debit is effectively an uncompensated loss to LPs on every fee-bearing liquidation, while the trader receives exactly what they'd get if the fee were zero. Fix: write `position.collateral_amount -= fee_to_transfer` back to storage before calling decrease_position, so the fee is deducted from the trader's own collateral (via decrease_position's own collateral_sum/output_amount computation) rather than absorbed by the pool. Verified with a before/after comparison (fee vs. no fee): the combined ledger (pool_amount + collateral_sum) and the pool's real token balance are now both unaffected by whether a fee is configured — the fee only ever changes who gets paid, never what leaves the pool or how the pool's accounting reads.
|
@dev-debbie-umoh Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
order_handler::liquidate_positionpays the liquidation keeper's execution fee by withdrawing real tokens directly from the market pool, and (per the earlier #629 fix) correctly debitsdata_store'spool_amountledger to match that real withdrawal. But the in-memorypositionread at the top of the function was never written back to storage with the fee deducted.decrease_position, called immediately after, re-readsposition.collateral_amountfresh from storage — full and un-reduced — and computes the trader's payout as if the fee had never been taken.Net effect: the fee's real token outflow is correctly tracked in
pool_amount(so there's no ledger-vs-real-balance drift in the narrow sense #629 fixed), but that debit isn't backed by any corresponding reduction in what the trader is owed. Every fee-bearing liquidation quietly transfers value from LPs (pool_amountabsorbs the fee) to nowhere in particular — the trader still receives their full, un-reduced payout, and the pool is out the fee amount with no offsetting benefit.Fix
Write
position.collateral_amount -= fee_to_transferback to storage before callingdecrease_position, so the fee is deducted from the trader's own collateral —decrease_position's owncollateral_sum/output_amountcomputation then correctly reflects it, and the trader (not the pool) bears the cost.Closes #540
Test plan
cargo test -p order-handler --lib— 64 passed, 0 failed (3 unrelated pre-existing failures on unmodifiedmaintoo:custom_heartbeat_timeout_is_respected,execute_order_records_keeper_heartbeat,keeper_goes_stale_after_timeout_and_role_is_revocable— oracle price-lookup issue unrelated to this change, verified independently)liquidate_position_keeper_fee_ledger_matches_real_balance: compares an otherwise-identical liquidation with vs. without a keeper fee configured. Verified this test fails without the fix (combined ledger differs from the real balance by exactly the fee) and passes with it (both the combined ledgerpool_amount + collateral_sumand the pool's real token balance are unaffected by whether a fee is configured — only who gets paid changes).