fix: fault-isolate fee batch sweeper, gate manual refunds, fix cleanup incentive, implement real partial liquidation

[dev-susa]

Summary

Fixes 4 assigned bugs, each a separate commit:

• Closes fee_batch_sweeper: closed issue #446's per-item fault isolation was never implemented — one bad pair still reverts the whole batch #539 — fee_batch_sweeper::claim_all_fees never implemented the per-item fault isolation issue fee_batch_sweeper: one failing market/token pair reverts the entire batch, blocking fee collection for all other markets in that call #446 was supposedly fixed for: one bad (market, token) pair still reverted the whole batch. Switched to try_claim_fees and added a failed: Vec<(Address, Address)> field to BatchClaimResult so callers can see which pairs failed instead of having the whole call revert.
• Closes order_cleanup: cancel_expired_order pays the cleanup incentive to its own contract address, not the caller #536 — order_cleanup::cancel_expired_order forwarded its own contract address instead of the real caller to order_handler::cleanup_expired_order, so the 10% cleanup incentive was always paid to order_cleanup itself (which has no withdrawal path), never to the keeper who did the work.
• Closes order_cleanup: record_manual_refund has no admin/role check — any caller can emit spoofed refund audit events #537 — order_cleanup::record_manual_refund only checked that the caller-supplied admin address signed the call — satisfiable by any caller passing their own address — letting anyone publish spoofed "admin refund" audit events. Added an initialize(admin, role_store) entrypoint and a CONTROLLER role check before publishing the event.
• Closes liquidation_handler: execute_partial_liquidation never actually reduces the position — pure stub #533 — liquidation_handler::execute_partial_liquidation was a pure stub: it validated liquidatability, computed what a partial close would look like, and emitted an event — but never touched order_handler storage or moved a token. Added a real order_handler::partial_liquidate_position entrypoint (mirroring liquidate_position/execute_adl's delegation pattern) that liquidation_handler now calls. This also required adding a retain_collateral option to decrease_position: a plain proportional collateral release (the existing behavior, used by voluntary decreases and ADL) leaves a position's collateral/size ratio unchanged, so it can never cure an already-unhealthy position — only keeping collateral fixed while size shrinks can restore health. Every existing call site keeps the old behavior (retain_collateral: false); only the new partial-liquidation path opts in.

Incidental fixes

While implementing/testing the above, hit several pre-existing compile/test breaks on main unrelated to these 4 issues, needed to get the workspace and test suite building/passing at all:

• adl_handler: missing is_market_paused_key import.
• liquidation_handler: same missing import, plus Error::MarketPaused and Error::InvalidMarket both assigned discriminant 6 (duplicate discriminant on a #[contracterror] enum).
• fee_batch_sweeper's own pre-existing tests used .collect() into a soroban_sdk::Vec, which doesn't implement FromIterator — the crate's tests never compiled.
• tests/order_cleanup.rs: ContractEvents has no .contains() in this soroban-sdk version (replaced with filter_by_contract + exact-equality); two existing tests minted collateral to the user's wallet instead of transferring it into order_vault, so create_order always reverted with ZeroCollateral.

These are called out explicitly in the relevant commit messages rather than bundled silently.

Test plan

• cargo check --workspace — clean
• cargo test -p fee-batch-sweeper -p order-cleanup -p order-handler -p liquidation-handler --lib — all green except 3 pre-existing, unrelated order_handler heartbeat-timing test failures, confirmed present on a clean upstream/main checkout before any of these changes (custom_heartbeat_timeout_is_respected, execute_order_records_keeper_heartbeat, keeper_goes_stale_after_timeout_and_role_is_revocable)
• cargo test -p gmx-integration-tests --test order_cleanup — 7/7 pass, including new tests for order_cleanup: cancel_expired_order pays the cleanup incentive to its own contract address, not the caller #536/order_cleanup: record_manual_refund has no admin/role check — any caller can emit spoofed refund audit events #537
• New unit test in fee_batch_sweeper reproducing fee_batch_sweeper: closed issue #446's per-item fault isolation was never implemented — one bad pair still reverts the whole batch #539: one panicking pair alongside one healthy pair in the same batch — healthy pair's fees are still claimed, failing pair is reported in failed
• New unit tests in liquidation_handler reproducing liquidation_handler: execute_partial_liquidation never actually reduces the position — pure stub #533: partial close actually reduces size_in_usd in order_handler storage, retains collateral, pays the configured keeper fee, a 100% factor behaves like a full close, and a healthy position still reverts
• Note: could not run a full cargo test --workspace locally — this sandbox's disk is independently at ~100% capacity (unrelated to this change) and a full workspace build/link exhausts it; all specifically affected packages were verified individually as above

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

https://claude.ai/code/session_017FJeNNQgXHQf7omFK8fzax

[drips-wave]

@dev-susa 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! 🚀

Learn more about application limits

[dev-susa]

Ran a self-review pass (`/code-review high`) on this PR before it gets external review, and it caught a real fund-drain bug in the #533 fix that I've now addressed in a follow-up commit:

Found: liquidate_position (pre-existing) and the new partial_liquidate_position both withdraw the configured keeper execution fee from the pool before calling decrease_position, which independently re-reads the position's collateral fresh from storage — unaware the fee was already paid out. On a full close this double-pays the fee out of the pool (fee + full original collateral, instead of fee + remaining collateral). On a genuinely partial close it's worse: the stored collateral_amount is never reduced by the fee at all, so the position's on-chain collateral stays permanently overstated and the same fee can be harvested again on a later partial liquidation of the same position.

Fixed: the fee-reduced collateral is now written back to the position's storage entry before decrease_position runs, so it always operates on the corrected value. Added PartialLiquidatePositionResult-adjacent regression tests: liquidate_position_with_keeper_fee_does_not_double_pay_account (forces liquidatability via min_collateral_factor instead of a price crash so the effect is directly observable against ~0 realised PnL) and an extended assertion on execute_partial_liquidation_pays_configured_keeper_fee confirming stored collateral drops by exactly the fee paid.

Also tightened the PartialLiquidationExecuted.liquidation_fee field's doc comment — it now carries the real, paid keeper_execution_fee rather than the synthetic value the pre-fix stub computed but never transferred to anyone, which is worth flagging explicitly since the field name is unchanged.

All directly affected unit and integration test suites pass; cargo check --workspace is clean.

[dev-susa]

Rebased onto upstream/main (which had advanced with an unrelated basis-points-divisor-centralization refactor touching liquidation_handler, order_cleanup, order_handler, and decrease_position_utils) to clear a merge conflict that had appeared on this PR.

Conflict resolution: in liquidation_handler::execute_partial_liquidation, kept this PR's #533 fix (delegating the real close to order_handler instead of the old local-computation stub) and adopted upstream's new gmx_math::BPS_DIVISOR constant in place of the stub's hardcoded 10_000.

Also fixed while re-verifying post-rebase (pre-existing, unrelated to #533/#536/#537/#539 — confirmed identical on a clean upstream/main worktree checkout): two compile breaks in order_cleanup's own unit test module — Address::generate called outside the scope of its local testutils::Address import, and a contract Error compared directly against a try_* client result that actually decodes to soroban_sdk::Error.

Re-verified after rebase: cargo check --workspace clean; liquidation-handler 16/16, order-cleanup unit 3/3 + integration 7/7, liquidation integration 6/6, adl-handler, fee-batch-sweeper, and gmx-decrease-position-utils all pass. order-handler is 64/67 — the same 3 pre-existing heartbeat-timing failures noted earlier in this PR, reconfirmed present on clean upstream/main.

PR now shows mergeable: MERGEABLE.

[IbrahimIjai]

veeery cool