Skip to content

Fix 1354 claim settlement race - #1340

Open
Peolite1 wants to merge 2 commits into
CalloraOrg:mainfrom
Peolite1:fix-1354-claim-settlement-race
Open

Peolite1 wants to merge 2 commits into
CalloraOrg:mainfrom
Peolite1:fix-1354-claim-settlement-race

Conversation

@Peolite1

@Peolite1 Peolite1 commented Sep 30, 2026 •

Copy link
Copy Markdown

Closes #1354

Summary

This PR fixes a critical race condition between claim_auction and settle_default_liquidation that could be exploited by a winning bidder to reclaim their bid while preventing the credit contract from receiving the settlement funds.

The Issue

Under the previous implementation, if an auction closed and the winning bidder called claim_auction before the factory could call settle_default_liquidation, two things happened:

  1. claim_auction incorrectly refunded the highest_bid back to the winning bidder.
  2. The auction status was set to Claimed.

When the factory subsequently attempted to call settle_default_liquidation, the call would panic with NotClosed because it exclusively expected the status to be Closed. This failure left the credit contract without its rightful settlement payout.

The Fix

To enforce the correct claim semantics safely:

  • src/lib.rs (claim_auction): Removed the token transfer logic that returned the bid to the winner. The highest_bid remains securely held for the credit contract.
  • src/lib.rs (settle_default_liquidation): Updated the status validation to accept both Closed and Claimed states, allowing settlement to succeed even if the winner has already invoked claim_auction.

Testing & Acceptance Criteria Map

A new testing suite has been added to tests/auth_settle.rs mocking a complete auction flow with the bid token.

  • Both orderings covered:
    • claim_then_settle_succeeds: Verifies that if claim_auction is called first, settle_default_liquidation still executes successfully afterward.
    • settle_then_claim_succeeds_with_claim_reverting: Verifies that if settle_default_liquidation runs first, a subsequent call to claim_auction correctly reverts with AlreadySettled.
  • Token balances asserted: Added hard assertions verifying that the credit contract receives the exact highest_bid amount (420 stroops) and the winner's token balance does not increase after claiming.
  • Documents intended semantics: The tests enforce that the winner definitively cannot reclaim their bid, validating the core invariant.
  • Fails on current behavior: The tests were explicitly built to panic under the old logic and now reliably pass with the applied fix.

Security & Failure-Mode Handling

  • Fund Security: Prevents a potential exploit where a winner could win an auction for free by aggressively claiming before settlement.
  • State Invariants: The state transitions now strictly separate the lifecycle of claiming (which shouldn't drain the bid escrow) and settlement (which legitimately pays the credit contract). The already_settled boolean flag ensures double-settlement is still impossible regardless of the ordering.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant