order_handler: liquidation keeper fee leaves the pool with no data_store ledger debit, silently inflating pool_amount vs real balance

[abayomicornelius]

Problem

order_handler::liquidate_position (contracts/order_handler/src/lib.rs, ~lines 1575-1601) pays the liquidation keeper's execution fee by withdrawing real tokens directly from the market pool, but never debits any data_store accounting key for that withdrawal:

let mut fee_to_transfer: i128 = 0;
if keeper_execution_fee > 0 {
    let keeper_fee_i128 = keeper_execution_fee as i128;
    fee_to_transfer = if keeper_fee_i128 <= position.collateral_amount {
        keeper_fee_i128
    } else {
        position.collateral_amount
    };

    if fee_to_transfer > 0 {
        MarketTokenClient::new(&env, &market_props.market_token).withdraw_from_pool(
            &handler, &collateral_token, &keeper, &fee_to_transfer,
        );
    }
}

withdraw_from_pool (contracts/market_token/src/lib.rs) is a pure SEP-41 token transfer out of the market contract's real balance — it has no concept of pool_amount/collateral_sum and touches no data_store key. Immediately afterward, decrease_position is called (libs/decrease_position_utils/src/lib.rs, decrease_position) and re-reads the position fresh from persistent storage, whose collateral_amount was never reduced by fee_to_transfer (the local position variable read earlier in liquidate_position was never written back). So decrease_position computes collateral_delta from the full, un-reduced collateral_amount (line ~256-265), debits collateral_sum by that full collateral_delta (line ~312-318), and pays output_amount — built from that same full collateral_delta (line ~267) — in full to the trader via its own separate withdraw_from_pool call (line ~350-355).

Why it matters

Every liquidation with a nonzero liquidation_execution_fee configured causes the market pool's real SEP-41 token balance to shrink by fee_to_transfer + output_amount, while the tracked ledger (pool_amount + collateral_sum in data_store) is only debited for output_amount's worth of collateral (via collateral_sum) — fee_to_transfer leaves with no corresponding ledger entry anywhere. Two consequences, both silent:

1. The trader isn't charged for the fee at all. output_amount is computed from the full, unreduced collateral_amount, so the position holder receives exactly what they would have received if the liquidation fee were zero — the pool (not the trader) absorbs the entire cost.
2. The pool's tracked accounting permanently overstates its real backing. market_utils::get_pool_value (libs/market_utils/src/lib.rs, ~line 501) computes GM token NAV directly from the pool_amount ledger keys read via get_u128_batch, not from the market contract's live token balance. Since pool_amount is never decremented for fee_to_transfer, every liquidation fee paid this way inflates recorded pool value relative to what the contract actually holds — the exact class of drift issue fee_handler: FEE_KEEPER can claim more fees than are actually available #254's balance-before-transfer guard exists to catch in fee_handler, except here it corrupts the primary ledger key itself rather than a single claimable balance, and nothing catches it.

Over the life of a market, the cumulative sum of every liquidation's fee_to_transfer becomes an unrecoverable, silently-growing gap between pool_amount-derived NAV and real backing.

Scope

In scope

• contracts/order_handler/src/lib.rs::liquidate_position — the keeper-fee withdrawal and its interaction with the subsequent decrease_position call.

Out of scope

• decrease_position's own pnl/fee/collateral bookkeeping in isolation, which is internally consistent — the bug is specifically the interaction between the two withdrawals.
• The prior fix for order_handler: liquidate_position pays keeper fee from order_vault, which never holds position collateral #387/order_handler: liquidation keeper fee is paid from order_vault, which no longer holds the position's collateral #411 (paying the fee via market_token instead of order_vault), which correctly solved the "wrong contract balance" problem this issue does not dispute.

Suggested fix

Before calling decrease_position, persist the fee deduction against the position so decrease_position's own collateral_delta/output_amount computation reflects it — e.g. write position.collateral_amount -= fee_to_transfer back to storage prior to the call — and debit collateral_sum (or pool_amount, whichever bucket conceptually backs the keeper-fee withdrawal) by fee_to_transfer at the same point withdraw_from_pool is called, mirroring how every other pool-affecting transfer in this codebase pairs its token movement with a data_store ledger update.

Verification

cargo test -p order-handler

Add a test that liquidates a position with a nonzero liquidation_execution_fee, then reads pool_amount/collateral_sum from data_store afterward and asserts they equal the market contract's real live token balance for that token (not merely that the keeper received the fee, which the existing tests in liquidation_handler already check).

[safal207]

Hi, I'd like to work on this issue.

Relevant experience: I recently implemented exact cancellation/end-time settlement for Soroban payment streams in Streampay-Org/StreamPay-Contracts#161 (merged), including boundary timestamps, one-time settlement, rollback, and value-conservation tests. I also implemented stateful financial-invariant coverage in Commitlabs-Org/Commitlabs-Contracts#556 (merged).

Proposed approach:

• Start with a failing regression that records the real SEP-41 market-token balance, pool_amount, collateral_sum, trader payout, and keeper fee before and after liquidation, then asserts exact conservation.
• Make the keeper fee reduce the position collateral before decrease_position reloads it, while debiting exactly the canonical ledger bucket that backs the withdrawal and avoiding a double debit.
• Cover fee = 0, fee < collateral, fee = collateral, and capped fee > collateral, plus retry/terminal-state behavior so liquidation cannot spend the fee twice.
• Verify atomic rollback: if either the storage update or transfer fails, the position, accounting keys, real balance, and events remain unchanged.

I can open the first draft PR within 24 hours of assignment and will keep the change limited to the liquidation keeper-fee interaction.

[Johnsource-hub]

@Johnsource-hub has applied to work on this issue as part of the Stellar Wave Program's 8th wave.

Hello, I’m a full stack developer with hands-on experience building with variety of tools including Nextjs, nodejs, nestjs, React, Typescript and more

I focus on writing clear, maintainable code and making sure features work as expected end to end, not just in isolation. I communicate progress regularly and keep things transparent as I work. I’m confident I can complete this task within an ETA of 4 HOURS

ℹ️ Repo Maintainers: To accept this application, review their application or assign @Johnsource-hub to this issue.

[dev-debbie-umoh]

@dev-debbie-umoh has applied to work on this issue as part of the Stellar Wave Program's 8th wave.

Let's squash this.

ℹ️ Repo Maintainers: To accept this application, review their application or assign @dev-debbie-umoh to this issue.

[drips-wave]

Congratulations, @dev-debbie-umoh! 🎉 Your application was accepted by the repo's maintainers, and the issue is due on August 31, 2026.

🧑‍💻 @dev-debbie-umoh: Please resolve the issue such that the repo's maintainers have enough time to review your contribution before the due date. You'll earn Points for completing the issue on-time, which will make you eligible for a share of the Stellar Wave Program's reward pool.

Warning

When opening a PR, please link it to this issue to ensure it gets tracked accurately. Points are awarded when this issue is marked as completed by the maintainer.

🤠 Repo maintainers: Please keep an eye on the contributor's progress and review their work before the due date. You can manage this issue, including adjusting its complexity and points, here.

🌊 Happy Wave 🌊