Skip to content

[#545] Make marketplace escrow settlement atomic - #565

Open
greatest0fallt1me wants to merge 1 commit into
Commitlabs-Org:masterfrom
greatest0fallt1me:fix/545-atomic-marketplace-settlement
Open

greatest0fallt1me wants to merge 1 commit into
Commitlabs-Org:masterfrom
greatest0fallt1me:fix/545-atomic-marketplace-settlement

Conversation

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Summary

  • keep marketplace listing and royalty state intact until every payment transfer succeeds
  • preflight the buyer gross balance and validate fee, royalty, and allowlist invariants before external calls
  • commit listing consumption only after seller, fee, and royalty transfers complete
  • add real Soroban-token regression coverage and an operator recovery guide

Acceptance criteria

  • Failed transfers leave listing, royalty, active-listing order, and buyer balance unchanged.
  • Successful settlement consumes a listing exactly once.
  • Buyer, seller, fee recipient, payment token, and royalty configuration are validated before mutation.
  • Failure, duplicate, allowlist, payout-conservation, and event-order regression tests are included.
  • Documentation covers accounting, recovery, monitoring, and limitations.

Design notes

The flow is organized as check, interact, then commit. The reentrancy guard remains active through token calls. A failed invocation exits before the commit block, while a successful invocation removes only the settled listing and its royalty record. The gross buyer debit is checked before the split so fee and royalty rounding cannot underfund the transfer path.

Verification

  • git diff --check passes.
  • cargo test -p commitment-marketplace was attempted but is currently blocked before test compilation by the repository dependency ethnum v1.5.2 using an invalid zero-sized transmute with rustc 1.97.1.
  • The new test suite targets the acceptance invariants and is ready for CI/toolchain validation.

Limitations

The existing contract stores the NFT address but does not perform a cross-contract NFT transfer; adding escrow/approval authority is outside this issue. Offers and auctions retain their separate settlement paths.

Closes #545

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.

[Quality] Make marketplace escrow settlement atomic

1 participant