STX Escrow — Trustless P2P Escrow System

CRITICAL #1: Arbitration Cannot Enforce Resolutions

The arbitration contract is completely disconnected from escrow-core. When finalize-dispute resolves a dispute, there is no mechanism to actually release or refund the escrowed funds. The escrow-core release-escrow requires tx-sender == buyer, and request-refund requires timeout expiry. No function exists for arbitration-triggered resolution.

;; In finalize-dispute — the comment says it all:
;; In production, would trigger escrow release/refund here
(ok { dispute-id: dispute-id, buyer-wins: buyer-wins })

Impact: The entire arbitration system is decorative. Disputes can be filed, voted on, and resolved, but the resolution has zero effect on escrowed funds. Dispute fees (1 STX each) and arbiter stakes are collected for a non-functional system.

Fix: Add an arbiter-resolve function to escrow-core that accepts calls from the arbitration contract principal, or use a trait-based callback pattern.

CRITICAL #2: NFT Escrow Has No NFT Transfer Mechanism

The NFT contract tracks escrow state with nft-deposited and nft-contract/nft-token-id fields, but there is no function to deposit an NFT. The nft-deposited flag can never become true. Since complete-nft-escrow asserts nft-deposited is true, completion is permanently blocked.

;; complete-nft-escrow requires this — but nothing ever sets it:
(asserts! (get nft-deposited escrow) ERR_INVALID_STATE)

;; NFT transfer is commented out:
;; (try! (contract-call? nft-contract transfer token-id
;;        (as-contract tx-sender) buyer))

Impact: STX deposited via create-stx-for-nft-escrow is locked with no path to completion. Only cancel-nft-escrow can recover it.

Fix: Implement NFT deposit using an SIP-009 trait parameter and actual contract-call? transfers.

CRITICAL #3: Anyone Can File Disputes for Any Escrow

The file-dispute function never verifies the escrow exists in escrow-core or that the caller is a party. It hardcodes buyer: filer and seller: CONTRACT_OWNER:

(map-set disputes
  { dispute-id: dispute-id }
  {
    escrow-id: escrow-id,
    buyer: filer,          ;; Simplified - would get from escrow
    seller: CONTRACT_OWNER, ;; Simplified — always the deployer!
    ...
  }
)

Impact: Anyone can file disputes for any escrow-id (real or fabricated) by paying the 1 STX fee. The seller is always set to the contract deployer regardless of the actual escrow parties.

HIGH #4: complete-nft-escrow Has No Access Control

No tx-sender check — any address can trigger escrow completion. If the NFT deposit mechanism were implemented, any third party could force-complete an escrow and send STX to the seller at any time.

(define-public (complete-nft-escrow (escrow-id uint))
  (let (
    (escrow (unwrap! ...))
    ;; No tx-sender authorization check
    ...
  )

Fix: Add (asserts! (or (is-eq tx-sender buyer) (is-eq tx-sender seller)) ERR_NOT_AUTHORIZED).

HIGH #5: deposit-stx Double-Writes with Stale Data

After the first map-set updates stx-deposited: true, the conditional block does a second map-set using the original escrow binding (still showing the pre-update state). The second write overwrites with stale data:

;; First write — correct
(map-set nft-escrows { escrow-id: escrow-id }
  (merge escrow { stx-deposited: true }))

;; Second write — uses stale 'escrow' from let binding
(if (get nft-deposited escrow)
  (map-set nft-escrows { escrow-id: escrow-id }
    (merge escrow { stx-deposited: true, state: STATE_FUNDED }))
  true)

Impact: Currently masked by Finding #2 (nft-deposited is never true), but if fixed, any intermediate state changes would be lost.

HIGH #6: Dispute Response Has No Party Verification

submit-response allows anyone to respond to a dispute and immediately moves it from OPEN to VOTING. No check verifies the responder is the buyer or seller:

(define-public (submit-response (dispute-id uint) ...)
  (let (
    (dispute ...)
    (responder tx-sender) ;; No party check
  )
    ;; Immediately transitions to VOTING
    (map-set disputes ... (merge dispute { state: DISPUTE_VOTING }))

Impact: A malicious actor can push any open dispute to voting before the actual counterparty responds, potentially with a fabricated defense.

MEDIUM #7: user-escrows Map Never Populated

The user-escrows map exists in escrow-core with buyer/seller tracking lists and volume stats, but no function ever writes to it. get-user-stats always returns the empty default.

MEDIUM #8: release-hash Field Is Dead Code

Escrow records include release-hash: (optional (buff 32)), suggesting HTLC-style hash-locked release. But release-escrow never checks this hash — it's always none and never read. Dead code that adds storage cost per escrow.

MEDIUM #9: Zero-Stake Arbiter Registration

register-arbiter has no minimum stake requirement. Since stx-transfer? of 0 STX succeeds in Clarity, anyone can become an arbiter for free, diluting the voting pool with zero-skin-in-the-game participants.

MEDIUM #10: Arbiter Deactivation Locks Stakes Permanently

deactivate-arbiter sets active: false but never returns the arbiter's staked STX. No withdrawal function exists. Deactivated arbiters permanently lose their stake.

MEDIUM #11: finalize-dispute Has No Access Control

Any address can finalize a dispute once the voting period ends. While the outcome is determined by votes, permissionless finalization could be used to front-run timing-sensitive scenarios.

LOW #12: Fee Rounding on Small Amounts

Fee calculation (/ (* amount 50) 10000) truncates to zero for amounts under 200 microSTX. Not exploitable at the 0.1 STX minimum, but the rounding pattern favors users.

LOW #13: No Event Emissions

No print statements in any contract. Off-chain indexers cannot track escrow lifecycle events without polling.

LOW #14: cancel-nft-escrow Missing State Check

The cancel function doesn't verify state != STATE_COMPLETED or state != STATE_CANCELLED. Only checks that both deposits aren't true. A completed escrow could theoretically be re-cancelled.