Skip to content

reclaim_expired_escrow refunds the sender but leaves the delivery record stranded #300

Description

@dev-fani

Problem Statement

reclaim_expired_escrow is permissionless and refunds an expired Locked escrow
to the sender. It makes zero cross-contract calls — the delivery record is
never informed:

record.status = EscrowStatus::Refunded;
save_escrow(&env, delivery_id, &record);
/* TotalLocked decremented, tokens transferred to sender */
env.events().publish((events::escrow_refunded(&env), delivery_id), (record.sender, record.amount));

The corresponding DeliveryRecord retains whatever status it held — Pending,
Active, or InTransit — indefinitely.

Why It Matters

The protocol ends up in a state its own consistency check classifies as invalid.
validate_state_sync maps Cancelled → Refunded as the only synchronized pairing
involving a refund, so a reclaimed escrow leaves combinations like
(Active, Refunded) that get_combined_state reports as desynchronized —
correctly, but with no mechanism to resolve it.

The delivery is also functionally stuck. Its escrow is gone, so confirm_delivery
would fail at mark_holdback_escrow, and cancel_delivery would fail at
refund_escrow because the escrow is no longer Locked (and see issue #295 for
the missing-escrow case). A driver may still be assigned and believe the job is
live.

Because reclaim_expired_escrow is callable by anyone, this state can be induced
by any third party once the 30-day expiry has passed, without the sender's or
driver's involvement.

Proposed Solution

Have the reclaim path transition the delivery to Cancelled, restoring the
synchronized pairing the state machine already defines for a refund.

That requires the escrow contract to hold the delivery contract's address and
cross-call it, which it does not do today — the same wiring issue #294 proposes.
An alternative is a delivery-side reclaim entry point that drives both
contracts in the correct order, keeping the cross-contract direction consistent
with the existing delivery → escrow flow.

Whichever direction is chosen, validate_transition must permit the resulting
delivery transition: InTransit → Cancelled is not currently legal and would need
to be added deliberately, or the reclaim restricted to deliveries in states from
which cancellation is already valid.

Acceptance Criteria

  • Reclaiming an expired escrow leaves delivery and escrow states synchronized
  • get_combined_state reports synchronized after a reclaim
  • The permitted delivery transitions are decided explicitly and reflected in validate_transition
  • Reclaim remains permissionless
  • The expiry precondition and Locked-only guard are unchanged
  • Regression test asserts post-reclaim synchronization from each reachable delivery status

Technical Notes

  • validate_transition currently allows Pending → Cancelled and Active → Cancelled but not InTransit → Cancelled; a reclaim of an in-transit delivery therefore has no legal target state today.
  • EscrowRecord.expires_at is set to created_at + 30 days at creation and is only consulted by this function.
  • The cross-contract direction matters: every existing call runs delivery → escrow, so adding escrow → delivery introduces a new dependency edge — weigh that against a delivery-side entry point.
  • Issue validate_state_sync reports every confirmed delivery as desynchronized because it has no Holdback case #198 covers validate_state_sync's missing Holdback case; this issue concerns a different unsynchronized pairing.

Relevant Files

  • contracts/escrow_contract/lib.rsreclaim_expired_escrow
  • contracts/delivery_contract/lib.rsvalidate_transition, cancel_delivery, validate_state_sync
  • contracts/escrow_contract/test.rstest_reclaim_expired_escrow_refunds_sender

Testing Requirements

  • Integration test: reclaim an expired escrow, assert delivery and escrow are synchronized
  • Unit test: reclaim from each reachable delivery status behaves per the agreed design
  • Regression test: refund amount and TotalLocked decrement unchanged
  • Regression test: reclaim still rejected before expiry and for non-Locked escrows
  • Edge case: reclaim of an InTransit delivery, given the transition gap

Definition of Done

  • Delivery and escrow remain synchronized after reclaim
  • Transition rules updated deliberately
  • Tests above added and passing
  • Formatting, clippy, and full suite clean

Complexity

Medium

Estimated Effort

4–8 hours

Dependencies

Shares the escrow → delivery wiring question with #294; the two should agree on direction.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Stellar WaveIssues in the Stellar wave programbugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions