claim() leaves past-due packages in Created state: locked funds and aggregates stay stale until manual refund #423

Problem

When a claim arrives after a package's expires_at, both claim and claim_with_proof return an error but leave the package in PackageStatus::Created:

// app/onchain/contracts/aid_escrow/src/lib.rs (claim)
if package.expires_at > 0 && now > package.expires_at {
    return Err(Error::PackageExpired);   // ← returns early, no state transition
}

No decrement_locked, no add_to_status_totals, and no status write happen on this path. The funds stay counted in KEY_TOTAL_LOCKED and in the Created (KEY_TOTAL_COMMITTED) aggregate until an admin manually calls refund(id).

Consequences, distinctly:

• Solvency pool leak. create_package rejects new packages when contract_balance < current_locked + amount. Because expired-but-unclaimed packages never decrement KEY_TOTAL_LOCKED, their escrowed funds are invisible as available pool, and the pool shrinks over time as packages expire. This is a liveness bug: the admin must remember to sweep every expired package by hand to reclaim capacity.
• Aggregates lie. get_aggregates(token) returns total_committed that overstates genuinely claimable commitments and total_expired_cancelled that undercounts expiry. Dashboards and indexers built on get_aggregates report funds as committed that are actually unclaimable.
• The documentation contradicts itself. BOUNDARY_VALIDATION_BEHAVIOR.md section "Late Claim Behavior" says the package "remains Created", but the same file's "Test Coverage" and "Conclusion" describe package_status_auto_expires_on_first_late_claim_attempt and "late claim attempts automatically expire the package status on-chain". The shipped tests do not match the shipped code, so a contributor cannot tell which behavior is intended.

Root cause

The expiry check was added as a guard that returns early, but the corresponding state transition (mark Expired, release locked funds, move aggregates Created → Expired) was only implemented in the admin refund path, not on the claim path.

Why this is architecturally hard

1. The transition must be atomic with the check. Marking Expired on a late claim is not just a status write: it must also decrement_locked and move KEY_TOTAL_COMMITTED → KEY_TOTAL_EXPIRED_CANCELLED for the token, or the invariant Σ locked + Σ claimed + Σ surplus == total_funded (see the closed issue Invariant test: Σ locked_tokens + Σ claimed_tokens + Σ surplus == total_funded #223) drifts. A naive "set status = Expired" makes the problem worse.
2. Aggregates vs. the storage model. add_to_status_totals uses PackageStatus::Refunded as its own bucket mapping; the design must decide whether Expired is written by the claim path (and what refund then sees) or only by refund, and how refund's existing was_committed logic stays correct.
3. The check is duplicated. claim and claim_with_proof each inline the now > expires_at guard, and refund/cancel_package/extend_expiry each have their own slightly different reading of expiry. The fix must decide whether to introduce a shared assert_not_expired/expire_if_past_due helper rather than a fifth copy.
4. Backend parity. BOUNDARY_VALIDATION_BEHAVIOR.md also documents a backend ClaimsService.cleanupExpiredClaims() cron that archives and revokes/refunds. If the contract starts auto-expiring on claim, the backend's "Created → archived" flow and its onchain revoke/refund step must be reconciled so a package is not both auto-expired and then refund-attempted.

Proposed design

Prefer an idempotent transition helper invoked on every claim/claim_with_proof when expires_at > 0 && now > expires_at: set status = Expired, decrement_locked, move the amount from Created to the expired/cancelled bucket, then return Error::PackageExpired. refund must remain able to convert an Expired package to Refunded and transfer the funds. Keep view_package_status cheap (still a single read).

Acceptance criteria

Contract

• A late claim/claim_with_proof on a Created package transitions it to Expired and returns Error::PackageExpired.
• After auto-expiry, get_total_locked(token) no longer includes that package's amount and get_aggregates(token) shows the amount in total_expired_cancelled, not total_committed.
• refund(id) on an auto-expired package still transfers funds to the admin and ends in Refunded without double-decrementing locked totals.

Tests

• Tests in app/onchain/contracts/aid_escrow/tests/ assert the locked/aggregate deltas after a late claim, and that a second late claim does not double-move the amount.
• A test pins the refund after auto-expiry path so no invariant drift is introduced.

Documentation

• BOUNDARY_VALIDATION_BEHAVIOR.md is rewritten so "Late Claim Behavior", "Test Coverage", and "Conclusion" describe one consistent behavior, and the contradictory auto-expiry statements are removed.

Out of scope

Adding a recipient-index or pagination to package reads, and any change to expires_at == 0 (never-expire) semantics, are separate issues.

Getting started

Files: app/onchain/contracts/aid_escrow/src/lib.rs (claim, claim_with_proof, refund, add_to_status_totals, decrement_locked), app/onchain/contracts/aid_escrow/BOUNDARY_VALIDATION_BEHAVIOR.md, app/onchain/contracts/aid_escrow/tests/boundary_validation_tests.rs.

cd app/onchain
make test        # cargo test -- --nocapture

Good first files to read: src/lib.rs refund (the existing Created→Expired→Refunded logic) and tests/aggregates.rs to see how totals are asserted today.