#182 is done as far as the leak goes: PR #255 unsubscribes on exit and gives the per-trade and single-order watchers deterministic ids. Its third task — the optional central registry — is not done, and this issue carries it.
Status: not broken today, required before #218
Nothing is broken right now. Each create_order / take_order calls derive_trade_key() first, so every per-trade subscription gets a unique pubkey and therefore a unique id; subscribe_single_order has a single caller, in the successful-take path. Two watchers over the same subscription id cannot currently happen.
#324 does not change that either: it forces a pool disconnect/connect, which drives the existing Online path in api::nostr (order book and chat subscriptions). It does not re-arm per-trade watchers.single caller, in the successful-take path. Two watchers over the same subscription id cannot currently happen.
#324 does not change that either: it forces a pool disconnect/connect, which drives the existing Online path in api::nostr (order book and chat subscriptions). It does not re-arm per-trade watchers.
Confirmed trigger: #218
#218 (priority: high, milestone M4) makes it reachable, by its own specification. Step 5 of the apply sequence is:
- Refresh gift-wrap subscriptions for the recovered trade keys.
with the matching acceptance criterion:
and the whole operation is required to be idempotent and retryable — "applying the same snapshot twice must not duplicate trades, sessions, disputes or mappings", and "a partial DB/key update (crash, kill, lost connection mid-apply) must be safely re-appliable".
Two applies of the same snapshot therefore re-arm a watcher for a trade key that already has one: two tasks owning mostro-trade-<pubkey>. The failure is not theoretical — the older watcher's exit path unsubscribes the newer one's live subscription and purges its pending request, which surfaces as NoDaemonResponse on an order the daemon actually accepted. A restore is exactly the moment a user cannot afford that.
So this is an ordering constraint, not a backlog item: it has to land before, or together with, #218 — and it stays small if it does, since all #218 needs is for subscribe_daemon_messages to be idempotent per trade key under a single owner.
(Note for #218: its step 5 and acceptance criteria say "gift-wrap subscriptions", which no longer exists — main renamed that transport in 56747c7 / 67df1ab. The subscription meant is the per-trade daemon-message one.)
Other paths that would reach the same place
The shape to copy
#182 dismissed the registry as "likely overkill" by comparing against v1's lib/features/subscriptions/ — a manager with four subscription types, session listeners and bulk filters. Mostrix is the closer reference (same language, same nostr-sdk, per-trade subscriptions like ours), and there it is three maps plus one function:
subscribed_pubkeys: HashSet<PublicKey> — idempotence by set membership, not by id
pubkey_to_subscription: HashMap<PublicKey, SubscriptionId>
subscription_to_order: HashMap<SubscriptionId, (order_id, trade_index)>
unsubscribe_dm_listener_subscriptions(client) — tears down only the DM-listener subscriptions and leaves the order/dispute scheduler ones intact
What it buys is a single owner of subscription lifecycle. v1 gets the same property from one subscription per type; Mostrix from one DM-router task owning the maps. Neither needs per-task ownership guards, because in neither does more than one task own a subscription.
Our deterministic ids are an advantage over Mostrix here: the id is recomputable from the trade pubkey, so a registry would not need pubkey_to_subscription at all.
If PR #255 merges with its generation guard still in place (SUBSCRIPTION_GENERATIONS / claim_subscription / owns_subscription), that guard is a per-task stand-in for this ownership and should be removed as part of this work — it also claims after subscribing, which leaves the window it is meant to close open.
Where it should live
Not in api/orders.rs — see #120. The relay/subscription layer (rust/src/nostr/) is the right home.
Acceptance criteria
Supersedes the subscription-manager half of the closed #144 (its RequestId half shipped in #172).
#182 is done as far as the leak goes: PR #255 unsubscribes on exit and gives the per-trade and single-order watchers deterministic ids. Its third task — the optional central registry — is not done, and this issue carries it.
Status: not broken today, required before #218
Nothing is broken right now. Each
create_order/take_ordercallsderive_trade_key()first, so every per-trade subscription gets a unique pubkey and therefore a unique id;subscribe_single_orderhas a single caller, in the successful-take path. Two watchers over the same subscription id cannot currently happen.#324 does not change that either: it forces a pool
disconnect/connect, which drives the existingOnlinepath inapi::nostr(order book and chat subscriptions). It does not re-arm per-trade watchers.single caller, in the successful-take path. Two watchers over the same subscription id cannot currently happen.#324 does not change that either: it forces a pool
disconnect/connect, which drives the existingOnlinepath inapi::nostr(order book and chat subscriptions). It does not re-arm per-trade watchers.Confirmed trigger: #218
#218 (
priority: high, milestone M4) makes it reachable, by its own specification. Step 5 of the apply sequence is:with the matching acceptance criterion:
and the whole operation is required to be idempotent and retryable — "applying the same snapshot twice must not duplicate trades, sessions, disputes or mappings", and "a partial DB/key update (crash, kill, lost connection mid-apply) must be safely re-appliable".
Two applies of the same snapshot therefore re-arm a watcher for a trade key that already has one: two tasks owning
mostro-trade-<pubkey>. The failure is not theoretical — the older watcher's exit path unsubscribes the newer one's live subscription and purges its pending request, which surfaces asNoDaemonResponseon an order the daemon actually accepted. A restore is exactly the moment a user cannot afford that.So this is an ordering constraint, not a backlog item: it has to land before, or together with, #218 — and it stays small if it does, since all #218 needs is for
subscribe_daemon_messagesto be idempotent per trade key under a single owner.(Note for #218: its step 5 and acceptance criteria say "gift-wrap subscriptions", which no longer exists —
mainrenamed that transport in56747c7/67df1ab. The subscription meant is the per-trade daemon-message one.)Other paths that would reach the same place
The shape to copy
#182 dismissed the registry as "likely overkill" by comparing against v1's
lib/features/subscriptions/— a manager with four subscription types, session listeners and bulk filters. Mostrix is the closer reference (same language, samenostr-sdk, per-trade subscriptions like ours), and there it is three maps plus one function:subscribed_pubkeys: HashSet<PublicKey>— idempotence by set membership, not by idpubkey_to_subscription: HashMap<PublicKey, SubscriptionId>subscription_to_order: HashMap<SubscriptionId, (order_id, trade_index)>unsubscribe_dm_listener_subscriptions(client)— tears down only the DM-listener subscriptions and leaves the order/dispute scheduler ones intactWhat it buys is a single owner of subscription lifecycle. v1 gets the same property from one subscription per type; Mostrix from one DM-router task owning the maps. Neither needs per-task ownership guards, because in neither does more than one task own a subscription.
Our deterministic ids are an advantage over Mostrix here: the id is recomputable from the trade pubkey, so a registry would not need
pubkey_to_subscriptionat all.If PR #255 merges with its generation guard still in place (
SUBSCRIPTION_GENERATIONS/claim_subscription/owns_subscription), that guard is a per-task stand-in for this ownership and should be removed as part of this work — it also claims after subscribing, which leaves the window it is meant to close open.Where it should live
Not in
api/orders.rs— see #120. The relay/subscription layer (rust/src/nostr/) is the right home.Acceptance criteria
Supersedes the subscription-manager half of the closed #144 (its RequestId half shipped in #172).