feat: single-owner registry for per-trade subscriptions

[Forte11Cuba]

Close #325

Every per-trade watcher's exit path runs a destructive pair — unsubscribe on its deterministic id plus purge_pending_request — unconditionally. Today that is safe only by accident: every caller of subscribe_daemon_messages derives a fresh trade key first, so two watchers can never share an id. The restore apply (#218) breaks that invariant by design: it must be idempotent, and re-applying a snapshot re-arms watchers for trade keys that already have one. The older watcher's exit would then kill the newer one's live subscription and purge its pending record — surfacing as NoDaemonResponse on an order the daemon actually accepted, during a restore.

Change

New rust/src/nostr/subscriptions.rs — a single-owner registry for the per-trade subscription lifecycle, following the repo's own ACTIVE_CHATS precedent (messages.rs) rather than importing Mostrix's shape:

• Idempotence by membership: try_claim admits exactly one owner per trade key. A second claim bounces — the live subscription and its pending record stay untouched.
• A bounce is a lease refresh: the bounce marks the key re-armed; the owner's idle-timeout exit consumes the mark and resets its timer instead of dismantling. Without this, a re-arm landing at minute 29 of the 30-minute idle window would be "covered" for seconds.
• Teardown is atomic w.r.t. claims: the destructive pair runs under the registry lock, so a concurrent claim either bounces before it (owner keeps running) or lands after it (key is free, subscribes from scratch) — never in between. Note: ACTIVE_CHATS releases before unsubscribing and leaves exactly that window open; this module documents why not to copy that ordering.
• Shutdown vs idle: Shutdown/closed-channel exits tear down unconditionally even with a re-arm mark pending — the watcher's receiver is dead, honoring the mark would spin on a closed channel. Post-reconnect coverage belongs to the re-arm paths (Restore: idempotent transactional reconstruction of trades, sessions and disputes #218/Relay liveness watchdog: detect and recover dead subscriptions #291).

daemon_message_subscription_id moves into the module (id + ownership together, in line with #120). subscribe_single_order is out of scope: single caller, not on the #218 path.

Summary by CodeRabbit

• Bug Fixes

  • Improved reliability of per-trade daemon-message subscriptions by preventing duplicate active watchers.
  • Subscriptions now recover after idle periods without disrupting pending requests.
  • Cleanup is handled consistently when watchers shut down or communication channels close.
  • Subscription identifiers are generated consistently, reducing conflicts and improving message routing.

• Tests

  • Added coverage for subscription recovery, cleanup behavior, pending-request preservation, and identifier uniqueness.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 61b5aed4-2d27-4632-881e-c9093e68d6b9

📥 Commits

Reviewing files that changed from the base of the PR and between 1d87cf6 and 454c823.

📒 Files selected for processing (3)

• rust/src/api/orders.rs
• rust/src/nostr/mod.rs
• rust/src/nostr/subscriptions.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

Walkthrough

The PR adds a centralized registry for per-trade daemon-message subscriptions. Watchers now use single-owner claims, stable subscription IDs, lease-aware rearming, and targeted teardown. Tests cover ownership, pending requests, subscription IDs, and shutdown behavior.

Changes

Per-trade subscription lifecycle

Layer / File(s)	Summary
Subscription registry and identity rust/src/nostr/subscriptions.rs, rust/src/nostr/mod.rs	The new registry tracks one owner per trade key. It provides stable subscription IDs and claim or release operations.
Lease-aware teardown and validation rust/src/nostr/subscriptions.rs	Idle watchers can rearm without removing pending requests. Shutdown and closed channels always unsubscribe, purge pending state, and remove ownership. Tests cover these lifecycle paths.
Daemon watcher integration rust/src/api/orders.rs	Daemon watchers claim ownership before setup, use the centralized subscription-ID helper, and distinguish idle expiry from shutdown. A regression test verifies that rearming preserves pending requests.

Priority: ⬇️ Low

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 454c8

Per-trade daemon subscriptions now remain single-owner across repeated setup while preserving pending requests during rearming and cleaning up on shutdown or closed channels. No concrete current-head merge-blocking risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant DaemonWatcher
  participant SubscriptionRegistry
  participant RelayClient
  participant PendingRequests
  DaemonWatcher->>SubscriptionRegistry: try_claim(trade_key)
  SubscriptionRegistry-->>DaemonWatcher: owner or rearm signal
  DaemonWatcher->>RelayClient: subscribe with stable subscription ID
  DaemonWatcher->>SubscriptionRegistry: teardown_or_rearm(trade_key)
  alt Lease rearmed
    SubscriptionRegistry-->>DaemonWatcher: continue watcher
  else Teardown
    SubscriptionRegistry->>RelayClient: unsubscribe subscription ID
    SubscriptionRegistry->>PendingRequests: purge trade request
  end
  DaemonWatcher->>SubscriptionRegistry: teardown(trade_key) on shutdown
  SubscriptionRegistry->>RelayClient: unconditional unsubscribe
  SubscriptionRegistry->>PendingRequests: purge trade request

Loading

Suggested reviewers: grunch

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding a single-owner registry for per-trade subscriptions.
Linked Issues check	✅ Passed	The implementation addresses issue #325. It adds single-owner claims, prevents duplicate watchers from tearing down live subscriptions or purging pending requests, supports lease-aware rearming, perfo…
Out of Scope Changes check	✅ Passed	The changes are within scope. The registry module, public module export, subscription lifecycle changes, deterministic subscription ID handling, and tests directly support the linked issue objectives.
Docstring Coverage	✅ Passed	Docstring coverage is 88.89% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 3 files.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A rabbit guards one listening thread
Stable IDs where hops are led
Idle ears renew the tune
Closed paths clean the burrow soon
Pending notes remain in flight
Until teardown makes things right}

_{Comment @coderabbitai help to get the list of available commands.}

[grunch]

This branch conflicts with main since #408 (d49f3c1) merged; GitHub reports it as not mergeable. Both PRs edit rust/src/api/orders.rs.

What to do on rebase:

• Rebase onto main and resolve rust/src/api/orders.rs. feat: Linux accessibility contract and Web persistence for Mortsom #408 changed three regions there: the daemon-message status sync (after status_for_action, it now spawns confirm_payout_completion when a trade enters SettledHoldInvoice), the stale sweep (sweep_action gained a local_status argument and a SyncSuccess arm, and run_stale_sweep_once also examines SettledHoldInvoice trades), and two new helpers next to fetch_public_order_status (newest_book_status, apply_payout_completed). Your hunks around subscribe_daemon_messages and orders_subscription_id are adjacent to those, so expect textual conflicts rather than semantic ones.
• One semantic point to keep in mind: feat: Linux accessibility contract and Web persistence for Mortsom #408 relies on the per-trade public-status subscription your registry now owns. The seller learns of the completed payout only from the public kind-38383 success event (the daemon sends PurchaseCompleted to the buyer alone), and confirm_payout_completion exists precisely because that subscription sometimes misses the event. If the registry changes when a watcher is torn down, make sure the watcher for a trade that just reached SettledHoldInvoice stays alive until the trade is terminal, or the fallback in feat: Linux accessibility contract and Web persistence for Mortsom #408 will be doing all the work.
• Re-run cd rust && cargo clippy -- -D warnings && cargo clippy --target wasm32-unknown-unknown -- -D warnings && cargo test after resolving; the web target is checked by CI as well.