reclaim expired sales

[jburnt]

allow users to reclaim expired sales since only managers were allowed to reject them

[crucible-burnt]

Security Review — ENG-1448 Fix ✅

Excellent fix for the marketplace PendingSale expiry vulnerability. Well-structured refactor with comprehensive test coverage.

What it fixes: execute_approve_sale never checked if the pending sale had expired. An attacker could create a pending sale, wait for it to expire (expecting a refund), then have the seller approve it anyway — effectively stealing the NFT for free since the buyer already considered their funds returned.

Code review:

1. Expiry check on approve — execute_approve_sale now takes env: Env and rejects expired sales with PendingSaleExpired. ✅
2. New ReclaimExpiredSale message — Lets the buyer reclaim funds after expiry. Only the buyer can call it, and only after expiration. Clean authorization model. ✅
3. Refactored remove_pending_sale — DRY extraction from execute_reject_sale, reused by reclaim. Adds reason attribute to the rejection event for auditability. ✅
4. Test coverage (6 new tests):
   • test_approve_expired_pending_sale_fails — approve after expiry blocked ✅
   • test_reclaim_expired_sale — buyer gets refund + NFT stays with seller ✅
   • test_reclaim_expired_sale_not_yet_expired — premature reclaim blocked ✅
   • test_reclaim_expired_sale_unauthorized — random user cannot reclaim ✅
   • test_reject_sale_emits_reason — event audit trail ✅
   • test_manager_can_reject_expired_sale — manager retains reject power post-expiry ✅

One observation: The manager can still RejectSale after expiry (tested and intentional), which is good — gives the manager a fallback path to clean up expired sales without requiring the buyer to act.

Ref: Closes ENG-1448. Critical severity — was exploitable on any marketplace with approval queues.

LGTM — approve.

[Peartes]

Just double approving based on the ai review