Skip to content

Missing Idempotency Guard in Settlement Finalization #23

Description

@elizabetheonoja-art

Problem Statement / Feature Objective
The settlement finalization function can be called multiple times for the same batch proposal, either accidentally (due to user error) or maliciously (to double-claim tokens). Even though internal state may have already been marked as finalized, the function may process the same batch again if the finalized flag is not checked atomically at the very beginning of the function. The current implementation may check settlement status after some state mutations have already begun, allowing partial re-execution. The objective is to implement a strict idempotency guard: check the settlement status BEFORE any state mutation and reject with SettledAlready if the batch has already been finalized.

Technical Invariants & Bounds
The finalized flag is stored as DataKey::BatchFinalized(batch_id) -> bool. The check must occur on the very first line of finalize_settlement() (or as the first operation after auth check). The flag must be set to true immediately after the check passes, BEFORE any external calls or state mutations. If the function panics after setting the flag but before completing, the flag remains true (the settlement is still considered finalized). The batch_id is a BytesN<32> (SHA-256 hash of the settlement proposal). The storage cost for the flag is 1 byte (bool) plus key overhead. The operation cost increase: ~2,500 instructions for the read-check-write sequence. The invariant: for any batch_id, finalize_settlement() must be called at most once successfully.

Codebase Navigation Guide
Settlement finalization: contracts/settlement/src/lib.rs — finalize_settlement() at line 142. Storage keys: contracts/settlement/src/storage.rs — DataKey::BatchFinalized may already exist but may not be checked early enough. Proposal creation: contracts/settlement/src/lib.rs — propose_settlement() at line 85. Current idempotency: may exist in contracts/settlement/src/state.rs — is_settled() helper. Tests: contracts/settlement/src/test.rs.

Implementation Blueprint
Step 1: In contracts/settlement/src/storage.rs, ensure DataKey::BatchFinalized(BytesN<32>) variant exists. If not, add it. Step 2: In finalize_settlement(), add as the first line (after require_auth()): let batch_id = proposal.batch_id; if storage::instance().has(&DataKey::BatchFinalized(batch_id.clone())) { panic_with_error!(env, SettlementError::AlreadySettled); }. Step 3: Immediately after the check, before any other logic, set storage::instance().set(&DataKey::BatchFinalized(batch_id.clone()), &true). Step 4: Add a test: call finalize_settlement() twice with the same batch_id; first call succeeds, second call fails with AlreadySettled. Step 5: Add a test that verifies partial execution does not leave state inconsistent: create a scenario where finalize_settlement() would panic mid-execution (e.g., oracle failure) and verify the batch is still marked as finalized (the flag approach is optimistic). Step 6: Run cargo test --package settlement.

Metadata

Metadata

Labels

Complexity: HardcoreIssues requiring deep systems-level engineering rigorGrantFox OSSIssue tracked in GrantFox OSSLayer: Core-EngineCore contract engine layerMaybe RewardedIssue may be eligible for a GrantFox rewardOfficial CampaignCampaign: Official CampaignType: Core-ArchitectureCore architecture design issues

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions