fix(otc): settlement atomicity — CAS guards, escrow cleanup, honest completion

[Scottcjn]

⚠️ Draft — money-path / CHAOTIC-tier. Requesting review before merge. Closes the settlement-atomicity race cluster from the otc-bridge tri-brain review (4 review loops: Codex 5.5 + Grok + GPT-OSS 120B, each finding folded in).

The unifying fix

Every money-path mutation now claims its state transition atomically (compare-and-swap) before any irreversible escrow/payout call, and only acts if it won the claim.

match_order

• Release the taker's escrow on the lost-CAS (409) and exception paths — a concurrent-match loser's RTC is no longer orphaned in a job no order references.
• Clear the escrow marker after a successful commit, so the except handler can't refund a live matched order.

cancel_order

• CAS open→cancelled and commit before refunding — a commit failure can no longer roll back to open with escrow already gone (re-matchable with no escrow). Reports escrow_refunded.

auto-expire (list_orders + match)

• Guard each expiry with AND status='open' — a stale expiry scan can't clobber/refund a just-matched row.
• Commit the expired transition before refunding (two-phase), same ordering as cancel.

confirm_order

• Atomic matched→settling claim before any external call — serializes double-confirm by design, not by accident of payout idempotency.
• 3-way honest outcome: completed only on real payout success (+ trade row + preimage); settlement_recovery if escrow was released but payout failed (idempotent re-drive); revert to matched if escrow was never touched (retryable).
• changes() guard on the completed UPDATE → no orphan trade row.
• Exception recovery moves a committed settling claim to a recoverable state — never wedged. escrow_released is marked before the accept call, so an accept timeout (funds may have moved, no response) is treated as ambiguous → settlement_recovery, never a retryable matched that could double-release.

safe_refund_escrow()

Raise-proof post-commit refund helper (logs + alerts, never 500s the caller after a durable commit). Used by the expiry and cancel paths.

Tests — tests/test_otc_bridge_settlement_atomicity.py (8)

payout-failure→settlement_recovery (no trade, ok:false, 200); escrow-failure→matched (retryable); double-confirm rejected; mid-settlement exception recovery both before and after escrow release; cancel-of-matched doesn't refund. 53 otc tests pass.

⚠️ Known limitations (documented, separate follow-ups — not silently left)

1. Crash between a durable commit and the following external call (settling→accept; cancel-commit→refund) can still strand state/funds. In-request recovery covers the exception case; closing the crash case needs a settling_at timestamp + a reconciler/sweeper. Follow-up PR.
2. Payout ok = queued, not on-chain-confirmed — pre-existing optimistic completion. On-chain confirmation polling is a separate architectural change.
3. Confirm response shape change is intentional: the old contract returned completed/ok:true even when the payout failed (it lied). New: ok reflects payout success, status ∈ {completed,settlement_recovery,matched}, trade_id/htlc_secret only on success, still HTTP 200 (the frontend branches on data.ok, not status code).

🤖 Generated with Claude Code

[github-actions]

⚠️ BCOS v2 Scan Results

Metric	Value
Trust Score	52/100
Certificate ID	BCOS-d3291212
Tier	L1 (not met)

What does this mean?

The BCOS (Beacon Certified Open Source) engine scans for:

• SPDX license header compliance
• Known CVE vulnerabilities (OSV database)
• Static analysis findings (Semgrep)
• SBOM completeness
• Dependency freshness
• Test infrastructure evidence
• Review attestation tier

Full report | What is BCOS?

---

BCOS v2 Engine - Free & Open Source (MIT) - Elyan Labs

[jaxint]

LGTM! Thanks for contributing to RustChain. Approved.

[Scottcjn]

🔴 Tri-brain review — CAS guard helps, but settlement state machine still unsafe (do not merge yet)

The compare-and-swap on settling -> completed is the right idea, but Codex + Grok converge that the surrounding flow has money-path gaps. 4 BLOCKING:

1. Preimage released before payout confirms (:1362): any payout_result["ok"] is treated as completed, and the test even accepts {"phase": "pending"}. A queued transfer isn't settled — finalizing the order and returning the HTLC preimage before payout confirmation lets the counterparty claim while the payout may never land. Wait for confirmation.
2. Refund stranding on crash (:930, :1530): commits expired/cancelled before safe_refund_escrow(). A crash in between strands funds — the order is no longer eligible for expiry/cancel retry and no alert fires. Needs a durable refund-pending state / outbox + startup reconciliation.
3. Double-pay window (:1414): a failed settling->completed CAS after the irreversible payout just rolls back + 409s. If a recovery path flipped the row back to matched, the paid order stays retryable → pay twice. Needs an explicit recovery state + alert, not a silent revert.
4. settling with no resume metadata (:1260): commit before external calls with no timeout recovery → a crash wedges the order in settling forever.

Plus: terminal state is inconsistent for the same "accept failed" symptom depending on whether the HTTP call raised vs returned an error body (Grok :1199-1249); confirm now always-200 with new settlement_recovery/matched states but no caller coordination; and the PR commits 3 binary SQLite DBs under tests/.tmp_signed_transfer/ (remove + gitignore — unauditable artifacts).

Direction: make terminal/refund commits happen only after the external action succeeds (durable outbox + reconciliation), don't finalize/release preimage until payout is confirmed, and add an explicit recovery+alert state on post-payout CAS failure. Happy to pair on the rework.

[galanime]

Reviewed otc-bridge/otc_bridge.py and tests/test_otc_bridge_settlement_atomicity.py with a focus on the new settlement recovery paths.

Two concrete observations:

1. The PR currently adds three binary SQLite files under tests/.tmp_signed_transfer/. Those look like generated test artifacts rather than source fixtures. Keeping them in the branch makes code review and future rebases harder, and it risks non-deterministic churn if SQLite/page layout changes. I would drop them from the PR and generate any needed DB state inside the tests or via checked-in text fixtures.

2. safe_refund_escrow() is the path operators will rely on when a post-commit refund fails, but the helper currently swallows the original exception with log.error(...) only. In a money-path recovery branch, I would strongly prefer log.exception(...) (or equivalent structured traceback capture) and passing the exception class/message into send_bridge_alert(...), otherwise the reconciliation breadcrumb is thinner than the state machine around it.

I received RTC compensation for this review.

[Scottcjn]

Marked ready — tri-brain converged (6 review loops)

Reviewed adversarially by Codex 5.5 + Grok (+ GPT-OSS where it finished) across 6 recurrent-depth loops. The open-finding count contracted every loop, which is the convergence signal:

Loop	New in-scope findings
1	5 distinct in-request races
2	~6 commit-ordering bugs (the fixes' own edges)
3	2 boundary refinements (accept-timeout point-of-no-return; expiry commit-order)
4	1 raise-safety nit + the architectural residual
5	2 (both consequences of loop-4's incomplete safe_refund_escrow rollout)
6	0 new bugs — 1 latent-fragility hardening + intentional/documented items

Last-loop hardening (loop 5–6)

• match_order lost-race + exception cleanup now use the raise-proof safe_refund_escrow (all 5 refund sites consistent).
• Precise escrow-release classification at /accept: ConnectionError (never reached node) → retryable matched; read-timeout/other (sent, unknown) → settlement_recovery. +2 tests.
• Enforced "released ⇒ never matched" directly on escrow_released (not coincidentally via payout_status).
• Failed settlement → HTTP 502 (200 only on completion), so monitoring doesn't read a failed financial op as success.

Reviewer attention — intentional behavior changes (not bugs)

1. Confirm response contract: ok now reflects payout success; status ∈ {completed, settlement_recovery, matched}; trade_id/htlc_secret only on success; 502 on failure. The old contract reported completed/ok:true even when payout failed.
2. Trades recorded only on genuine completion (volume/history no longer counts failed settlements).
3. New statuses settling/settlement_recovery (no CHECK constraint on orders.status; surfaced by GET /api/orders/<id>, hidden from the open book).

Known limitation → follow-up (NOT in this PR)

A process crash between the settling-commit and the final commit can still wedge an order in settling. In-request recovery covers the exception case; the crash case needs a settling_at timestamp + a reconciler/sweeper. I'd take that as a separate PR.

55 otc tests pass. Your call on merge — and on whether the 502-on-failure contract is what you want for downstream consumers.