Skip to content

[CO-03] Fix Checks-Effects-Interactions violation in payment contract release_payment() #281

Description

@Leothosine

Problem

contracts/payment/src/lib.rs:190-243 - release_payment() performs two token transfers BEFORE updating the payment status:

token_client.transfer(..., &payment.restaurant_wallet, &net_amount); // external call
token_client.transfer(..., &treasury, &payment.fee_amount);          // external call
payment.status = PaymentStatus::Released;                             // state update LAST

This violates the Checks-Effects-Interactions pattern. If the token contract triggers re-entry during transfer, a second release_payment() call could execute with status still set to Escrowed, draining the escrow twice.

Proposed Solution

Reorder to Checks-Effects-Interactions:

  1. Validate all preconditions (status == Escrowed, caller auth) - CHECKS
  2. Set payment.status = PaymentStatus::Released and persist to storage - EFFECTS
  3. Perform both token_client.transfer() calls - INTERACTIONS

Add a releasing: bool reentrancy guard to storage that panics if already set on entry.

Acceptance Criteria

  • payment.status is persisted to storage BEFORE the first token_client.transfer() call
  • A mock token contract that calls release_payment() during transfer() is rejected by the reentrancy guard
  • Reentrancy guard is cleared after both transfers complete
  • All existing happy-path tests continue to pass
  • Unit test verifies the order of state mutation vs external calls

Metadata

Metadata

Assignees

Labels

Type

No type

Fields

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