Skip to content

add test for resolving dispute refund when buyer is not set#192

Merged
JSE19 merged 1 commit into
JSE-ORG:mainfrom
Elizabeth-dev270:resolve-dispute-Refund-To-Buyer
Jun 1, 2026
Merged

add test for resolving dispute refund when buyer is not set#192
JSE19 merged 1 commit into
JSE-ORG:mainfrom
Elizabeth-dev270:resolve-dispute-Refund-To-Buyer

Conversation

@Elizabeth-dev270
Copy link
Copy Markdown
Contributor

closes #151

PR: Fix [BUG-006] resolve_dispute RefundToBuyer panic (regression test)

Summary

  • Issue: [BUG-006] resolve_dispute(RefundToBuyer) may unwrap a None buyer and trap the contract (panic), leaving escrows permanently Disputed.
  • This change adds a regression test that reproduces the unsafe path and asserts the contract returns EscrowHasNoBuyer instead of trapping.

What changed

  • Added test: contracts/escrow/src/test_resolve_dispute_no_buyer.rs
    • Constructs an EscrowData with buyer: None and state: Disputed and an active DisputeData entry.
    • Calls resolve_dispute(..., ResolutionType::Refund) as the admin and asserts EscrowHasNoBuyer is returned (no trap).

Why

  • The repository previously allowed a panic when resolving a dispute that refunded the buyer while escrow.buyer was None (open-buyer escrows or accidental None). That panics the WASM host and permanently freezes funds. The test prevents regressions and confirms code paths handling None behave as expected (return a typed error rather than trap).

Files touched

  • Added: contracts/escrow/src/test_resolve_dispute_no_buyer.rs

Testing

  • Run the contract test suite (escrow crate):
cd contracts/escrow
cargo test
  • Or run the single test:
cd contracts/escrow
cargo test test_resolve_dispute_refund_no_buyer_returns_error

Recommended follow-ups (suggested fixes)

  • Option A (safer): Make buyer: Address non-optional in EscrowData and require a buyer at create_escrow time. This eliminates the None path but is a breaking storage/API change and requires migration.
  • Option B (narrower): Keep buyer: Option<Address> but harden all flows that assume a buyer exists:
    • Return EscrowHasNoBuyer (or another clear error) anywhere a buyer is required (already done in several places). Ensure no .unwrap() or unchecked clones remain for buyer.
    • Add unit/integration tests covering open-buyer and funded escrows.

Notes

  • This PR only adds a regression test; it does not modify runtime behavior. The test documents the bug and prevents regressions while a code fix is applied.

Next steps

  • If you want, I can implement Option B (guard resolve_dispute/payout paths to return a clear error without panicking) and add corresponding tests, then open a PR with the code fix and this test included.

@drips-wave
Copy link
Copy Markdown

drips-wave Bot commented Jun 1, 2026

@Elizabeth-dev270 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@JSE19 JSE19 merged commit d8facdf into JSE-ORG:main Jun 1, 2026
1 of 4 checks passed
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.

[BUG]

2 participants