seller recovery: order the resubscribe after NIP-42 (#189 client half), and give the open-pool re-arm an owned schedule (#190) - #198
Conversation
…n-pool re-arm an owned schedule Two seller recovery-loop fixes (#189 client half, #190). They land together because they share the CLOSED-handling arm and the relay fixture the teeth need; splitting them would produce two commits neither of which compiles alone. #189 — the recovery path let p-gated REQs out before NIP-42. nostr-sdk re-sends every REGISTERED subscription as its first act on socket-up (relay/inner.rs:748-752), before that connection has any auth state; auth happens later, in the ingester (:936). mobee-relay evaluates its p-gate against the empty authed pubkey of that unauthenticated session and answers `restricted:` — the permanent prefix — where the truth is the retryable `auth-required:`. nostr-sdk takes it at its word and DELETES the subscription (:1028), so the post-auth resubscribe at :941 cannot see it and never restores it. Carrying registrations across the reconnect therefore killed the kind-1059 money leg on every recovery. The registrations are now dropped BEFORE the new socket comes up, so that first resubscribe has nothing to send and the REQs go out after auth — the order boot always had. A failed reconnect re-registers them, because the SDK's background reconnect is a real recovery path and can only restore what it still knows. Plus the belt the ordering fix cannot cover: the SDK's own background reconnect resubscribes pre-auth too, and that path has no hook. A `restricted:` CLOSED of a subscription whose filters all pin #p to our OWN pubkey, on an authenticated session, is that race and not a gate violation — so it is re-issued once per authenticated session. The taxonomy is untouched: `restricted:` stays permanent-class, a genuine wrong-#p refusal cannot reach the branch (we author these filters), and a second refusal falls through to the paced recovery. Also: a CLOSED for a subscription id we never registered no longer forces a recovery. It cannot be a leg of ours going deaf, and escalating it was costing a full reconnect per cycle — which, before the ordering fix, re-closed the 1059 leg. It is logged with the age of the last backfill and of the last NIP-42 auth, which is what the relay owner needs to tell our own transient `fetch_events` REQ (generated id, same cadence) from a relay-side auth-TTL sweep. #190 — the open-pool re-arm waited on a trigger nothing guarantees. The degrade re-armed only via `open_pool_degraded = false` in the recovery-success arm, so a seat that degrades and then stays healthy has no path back. The re-arm now rides the wrap-backfill tick: owned, unconditional, and not disableable the way the heartbeat is. Acceptance is the relay's EOSE — a response NIP-01 owes us — never our own send having succeeded, and a refusal doubles a capped backoff so a permanently refusing relay costs one REQ per cap interval rather than one per tick. An attempt that draws no verdict at all is treated as a refusal, so there is no timer-less park. This half is defence in depth, not a repair for an observed seat: the reported stuck specimen was withdrawn — every seat seen degraded was flapping on the #189 sawtooth. The gap is structural, and it survives the #189 fix. Teeth (crates/mobee-core/src/seller_node/p_gate_relay_fixture.rs): a relay fixture that answers a p-gated REQ from an unauthenticated session with `restricted:`, as the deployed relay does. The nostr-relay-builder fixture says `auth-required:`, which nostr-sdk keeps and restores by itself, so against it every ordering passes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…y bites The downgrade shipped in the previous commit — a CLOSED for a subscription id we never registered is logged and does nothing else — had no test holding it down, so a refactor could restore the escalation silently. Two teeth now do. The behavioural one runs the real loop WITH the watchdog enabled, because with it disabled "no reconnect happened" is true even when the escalation is restored; and it waits long enough for a reconnect to COMPLETE against the fixture (~6s), because a shorter window reads a recovery in progress as a recovery that never happened. The first draft waited 4s and passed under revert — decorative. It now fails with "a CLOSED for an id we never registered forced a reconnect". The diagnostic itself moved into `unknown_close_diagnostic` so its content is pinned rather than drifting: the relay owner reads that line to separate our own transient `fetch_events` REQ (generated id, same 300s cadence) from a relay-side NIP-42 TTL sweep, and it is useless to them if either age goes missing. Also corrected an overclaim in the #190 re-arm tooth: with the watchdog off, "no reconnect" is the test's PREMISE, not an observation — and saying so is what makes the tooth's real claim legible, which is that the re-arm cannot have come from the recovery path because there is no recovery path in that test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…d drop one that asserted the fix away Both failures came out of `cargo test --workspace`; both targeted runs had been green, which is the point worth recording — a module-filtered green is not the same evidence. The ten-reconnect tooth asserted "zero pre-auth p-gated REQs across all ten cycles". That contradicts what the fix claims. The SDK's background reconnect resubscribes before AUTH and has no hook, and a recovery whose reconnect fails re-registers ON PURPOSE so the SDK can still rescue us — both put a pre-auth REQ on the wire by design, which is the entire reason the retry belt exists. The assertion was asserting the fix away, and it flaked under parallelism because the thing it forbade does happen. Removed. The per-cycle assertions are the real claim and they still bite: under revert, "the kind-1059 money leg was refused on recovery 1 ... authenticated: false". The zero-leak claim now lives only where it is true — the single controlled recovery in the ordering tooth. The same tooth also raced its own boot REQ against the first clear, and counted wrap REQs by cycle index, which a background reconnect's own REQ would misread as this cycle's. It now waits for the boot REQ to land and counts against the previous cycle. The wrong-#p tooth read the SDK's subscription registry once, racing the SDK's processing of the CLOSED against the fixture's record of the REQ — opposite sides of the socket. Bounded wait instead. Plus PR-BODY.md, and the clippy/rustfmt cleanups on the touched files: a let-chain collapse in the re-arm tick, the fixture's unused forced-close variant removed, and `filter_count` put to work asserting that both offer shapes ride ONE subscription. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e prefix not the sentence Cumulative recovery counts grow with uptime and are meaningless across a rebuild, so the contract is now the per-cycle invariant set (deg==rec 1:1, interval exactly 300s, attempts=1/outage ~10s, state TARGETED-ONLY) with pass = deg 0 + state FULL sustained across several consecutive 300s windows. Step 0 is the executable rename guard, with the one correction that matters: the DEGRADE line's trailing parenthetical CHANGED on purpose — #190 requires it to state the real re-arm schedule — so a check matching the old sentence aborts on a correct build. Verified against the binary: the prefix 'seller node RELAY-CLOSED DEGRADE:' is present, 're-armed on the next successful recovery' is gone. Measured literal counts are included, with the note that eprintln! splits a message at each placeholder, so a count describes placeholder placement rather than call sites and must be calibrated against a binary rather than remembered. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Field-validation contract, final two amendments from the fleet's v3 self-test (protocol chain now 5 flaws / 3 parties, each caught by making the check executable):
Also confirmed on the pre-fix build: the four new literals correctly absent (script self-identifies build generation), invariants stable across 25 min. The after-run is armed and waiting on this PR's merge. |
Two amendments from the fleet's self-test of the after-run, folded into the contract so the durable record carries them rather than a comment. state is the weak signal: on the known-bad build a seat sampled FULL, caught inside the few-second armed window between the resubscribe and the next close, so a point sample can read FULL against a 100% degrade rate. PASS now hangs on deg = 0 across >=3 consecutive 300s windows per seat, which cannot be caught mid-cycle; state is corroborating only. And the pass signal is a zero, which a regex that matches nothing produces just as readily. The script now requires a must-be-present line (wrap backfill) to parse non-zero before any zero counts — the denominator, without which a checker cannot tell 'nothing broken' from 'nothing examined'. Step 0 guards the vocabulary in the binary; this guards the parsing. Neither substitutes for the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Gate: BOUNCE — one blocker, otherwise strong; first-in-order so it holds the chain. Blocker: gate 5 ( Fix (minimal, local to the 3 tests): drive Also: strip the root Everything else passed and is verified — #169-arc non-regression (arc tooth still bites on the modified tree), the #189/#190/#unknown-id-CLOSED pairing is real (owned re-arm covers the dropped duty, red-on-reverted), 3 bites re-run rc=101, both corrected tests honest, scope clean, |
…wallet Gate regression, mine. The three loop teeth `tokio::spawn` the runner, and under the `acp` feature the runner holds an AcpDriver whose std mpsc Receiver is !Sync — so `run()`'s future is !Send and the spawn fails to COMPILE. Nine errors on `cargo test -p mobee-core --features acp,gateway,git-delivery,wallet --lib --no-run`; base dev compiles that target rc=0. A LocalSet keeps the loop on this thread, which is also the truer shape: the node runs its loop as one task, not spread across a work-stealing pool. No production code moves — the defect was entirely in how the tests drive it. Why my gates missed it: none of them compiled acp and wallet TOGETHER. The workspace default has no acp; the -p acp run had no wallet. Each feature was covered and the combination was not, which is exactly the shape where a cfg-gated item stays invisible — a gate must compile every SHIPPED combo, and acp+wallet is the seller's real config. That command is now in the gate set. Also drops PR-BODY.md from the repo root: it is a process artifact that does not belong on dev, and removing it dissolves the fold-later bookkeeping — the live PR description is now the only copy of the contract. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Seller recovery: order the resubscribe after NIP-42, and give the open-pool re-arm an owned schedule
Closes the client half of #189 and #190.
Three behavioural changes. The first two are the charter's; the third is a defect found while
instrumenting the first, and it is called out separately below because it rides a two-fix PR.
1. #189 — the recovery path let p-gated REQs out before NIP-42
Mechanism, read out of the SDK source rather than inferred.
RelayInner::post_connectioncallsresubscribe()as its first act on every socket-up (relay/inner.rs:748-752), before thatconnection has any NIP-42 state; auth happens later, in the ingester (
:936), which thenresubscribes a second time (
:941). mobee-relay evaluates its p-gate against the empty authedpubkey of that unauthenticated session and answers
restricted:— the permanent prefix — where thetruth is the retryable
auth-required:.That misclassification is not cosmetic, and the SDK's CLOSED taxonomy is why:
auth-required:MarkAsClosed(:1023-1027)resubscribe()at:941restores it automaticallyrestricted:Remove(:1028):941cannot see it and never restores itRemove(:1029-1034)So carrying subscription registrations across a reconnect killed the kind-1059 money leg on every
recovery. (The relay half is owned elsewhere and is being patched independently — the
misclassification lives in the fork's open-read path, where a pending-auth connection collapses to
an anonymous empty pubkey before the p-gate runs. No relay code is touched here, and this fix does
not depend on that one landing.) The fix is ordering: the registrations are now dropped before the new socket comes
up, so that first resubscribe has nothing to send and the REQs go out after auth — the order boot
has always had. A failed reconnect re-registers them, because the SDK's own background reconnect is
a real recovery path in the field (the run loop distinguishes it in the
RESTOREDline) and canonly restore what it still knows about.
Plus the belt, which is not redundancy.
RelayOptions::reconnect(true)means the SDK alsoreconnects on its own, entirely inside the SDK, with no hook — and that path will always resubscribe
pre-auth. So: a
restricted:CLOSED of a subscription whose filters all pin#pto our ownpubkey, on a session that has authenticated, is re-issued once per authenticated session.
The CLOSED-prefix taxonomy is not softened.
restricted:stays permanent-class. A genuinewrong-
#prefusal cannot reach the branch — we author these filters from our own pubkey, so theonly way the relay can refuse one is by having no authenticated pubkey to compare against. A
subscription carrying any un-pinned filter is excluded, because there the refusal may genuinely be
about the un-pinned half. A second refusal falls through to the paced recovery rather than looping.
subscription_pins_only_our_pubkeycarries the argument in a doc comment.NIP-42 state is tracked from a new
relay.notifications()arm, becauseAuthenticatedneverbecomes a pool notification (
relay/inner.rs:418maps it toNone) — the pool stream the run loopalready watches structurally cannot see it. Both stale readings of that flag are bounded and safe:
stale-false only declines a cheap retry and falls through to the paced recovery; stale-true spends
the one retry the session allows and then does the same.
2. #190 — the open-pool re-arm waited on a trigger nothing guarantees
The degrade re-armed only via
open_pool_degraded = falsein the recovery-success arm(
run.rs:859), so a seat that degrades and then stays healthy has no path back.The re-arm now rides the wrap-backfill tick. Not the heartbeat: the heartbeat is disableable by
config, and a repair must not depend on a tick that may never fire; the backfill tick is
unconditional. Acceptance is the relay's EOSE on the offer subscription — a response NIP-01 owes
us — never the fact that our send succeeded, because a REQ that left the socket proves nothing about
whether the relay took it. A refusal doubles a capped backoff, so a permanently refusing relay costs
one REQ per cap interval rather than one per tick, and an attempt that draws no verdict at all is
treated as a refusal so there is no timer-less park.
run.rs:859stays — a full resubscribe genuinely does restore the grouped REQ. It was only ever thesole path; this adds an owned one alongside it.
Scope, stated honestly. This half is defence in depth, not a repair for an observed seat. The
reported stuck specimen was withdrawn: every seat seen degraded in the field was flapping on the
#189 sawtooth, not stuck. The quiet-seat case follows from
run.rs:859but nobody has observed it.The gap is structural, and it survives the #189 fix — which is why the owned schedule stays.
3. An unknown-id CLOSED no longer forces a recovery
Field seats open every cycle with a CLOSED naming a subscription id the client never registered. It
was escalated to a full recovery, and the recovery then re-closed the 1059 leg: escalating an
unknown-id CLOSED cost a reconnect per cycle on a socket that was never broken. A subscription id
we never registered cannot be a leg of ours going deaf, so it is now logged and nothing else.
Genuine deafness stays covered by the EOSE liveness probe, which is unchanged.
This is only safe because §2 exists, and they must land together. Rocky's field data shows the
unknown-close-triggered recovery was also what re-armed the open-pool half — by accident. Removing
the escalation removes that accidental rescue, and the owned re-arm in §2 is what covers it now.
The no-reconnect tooth is what proves the replacement works without the reconnect.
A candidate for what the unknown id is — a lead, not a claim. Our own periodic wrap backfill
calls
client.fetch_events, which generates its subscription id (pool/mod.rs:815) and runs onexactly the 300s cadence these closes appear on. The relay owner has since enumerated every periodic
mechanism on the relay side and none fits, which places the source outside the relay — client-side
or a fronting proxy's idle timeout — and makes our own transient REQ the leading candidate rather
than merely a plausible one. It is still not asserted here. The new log line carries the age of the
last backfill and the age of the last successful NIP-42 auth, and is the instrument that settles
it: a small backfill age implicates our own REQ, and the auth age bounds anything session-scoped.
The fixture, and a trap worth naming
The teeth needed a relay this repo did not have.
nostr-relay-builder's NIP-42 read gate answers anunauthenticated REQ with
auth-required:(local/inner.rs:961-989) — which nostr-sdk keeps andrestores by itself. Against that fixture every ordering passes and the tooth is decorative. The
next person to test anything in this area will hit the same wall.
crates/mobee-core/src/seller_node/p_gate_relay_fixture.rsspeaks the deployed rule directly — a#pfilter is refusedrestricted:unless the session is authenticated as that very pubkey — whichcovers both cases the teeth must tell apart: the pre-auth race (right
#p, no auth yet) and agenuine violation (someone else's
#p, fully authed). It records every REQ with the session's authstate at arrival, and counts sockets, so "no reconnect was required" is an observable rather than
an inference.
Teeth
Nine new tests, all against the real paths.
recovery_puts_no_p_gated_req_on_the_wire_before_nip42_completesa_genuine_wrong_p_restricted_stays_removed#pis refused, deleted, never retriedwraps_subscription_survives_ten_consecutive_reconnectsopen_pool_rearms_on_an_owned_tick_without_any_reconnectrepeated_open_pool_rejection_backs_off_and_never_hot_loopsopen_pool_rearm_backoff_doubles_and_stays_cappeda_rearm_attempt_with_no_verdict_is_treated_as_a_refusalan_unknown_id_closed_costs_no_reconnect_and_no_resubscribethe_unknown_close_diagnostic_carries_both_ages_and_the_auth_stateRed-on-revert, strong form,
rc=101eachOne of these teeth did not bite on the first try, and the fix is worth recording. The unknown-id
tooth originally slept 4s and then asserted no reconnect had happened. A reconnect against this
fixture takes ~6s, so under revert the recovery was still in flight and the tooth passed — a
decorative tooth. It now waits for the socket count to move with a 20s ceiling, returning early on
the red path. Any "X did not happen" assertion has to outlast the time X takes to happen.
Gates
cargo test --workspace— rc=0, 607 tests (555 in themobee-coresuite). Baseline on theuntouched tip: rc=0 / 598 tests, run on a frozen checkout of
origin/dev. The delta is exactlythe nine teeth above.
ten-reconnect tooth asserted "zero pre-auth p-gated REQs across all ten cycles" — which contradicts
what this fix claims, since the SDK's background reconnect and the deliberate re-register on a
failed recovery both put pre-auth REQs on the wire by design. That assertion was asserting the fix
away; it is gone, the per-cycle claim stands, and the bite was re-verified afterwards. The same
tooth also raced its own boot REQ against the first clear. Recorded because a module-filtered green
is not the same evidence as a full-suite-parallel green — both passed the former.
liveness_probe_answers_only_on_an_authenticated_session,reconnect_reauthenticates_and_delivery_resumes_in_process,wrap_backfill_cursor_clamps_to_the_oldest_unsettled_delivery_and_fails_closed.rustfmt, with the whole-tree skew measured rather than asserted. Under thistoolchain the untouched
run.rsfromorigin/devalready produces 83rustfmt --edition 2024 --checkdiffs, almost all import-ordering from the 2024 style edition. The set-difference ofnormalised diff hunks between the pristine file and mine is empty — this diff contributes zero
formatting deviations.
p_gate_relay_fixture.rsisrc=0outright, being new. CI runs neitherfmtnorclippy(.github/workflows/ci.ymlis build+test), and whole-tree formatting is afleet-level call, not this slice's. Precedent: the crossmint slice's
PR-BODY.md.clippy(--no-default-features --features gateway,git-delivery,wallet --all-targets -D warnings): 57 pre-existing errors across the workspace. Two namerun.rs—:2377and:2447—both in test code this diff does not touch. Zero findings attributable to this change.
Field validation
Pre-registered by the rocky fleet before the fix, which is the right order.
The contract is the invariant set, not cumulative counts. Cumulative totals grow with uptime and
say nothing across a rebuild, so the BEFORE state is characterised by the shape of each cycle,
scoped per-run (the after-script asserts
boots=1):deg = 0across ≥3 consecutive 300s windows (~15 min) per seatattempts=1, outage ~10s per cycleThat 300s regularity is external confirmation of the
run.rs:859mechanism: the heartbeat tickservicing a queued
forced_recoveryis what paces the flap.Why the interval invariant is stated as drift and not as equality.
sinceis second-quantized,so adjacent gaps read 299 and 301 and cancel exactly — a live run shows 1×299 + 22×300 + 1×301 with
cumulative drift 0s over 24 gaps. An "exactly 300s" check calls that a deviation and reports a false
negative on a perfectly periodic flap. The per-gap tolerance absorbs the quantization; the drift
over the run is the clause that carries the claim.
stateis the weak signal;degover a window is the strong one. On the known-bad build a seatsampled
FULL— caught inside the few-second armed window between the resubscribe and the nextclose. A point sample can therefore read
FULLagainst a 100% degrade rate.degcounted over awindow cannot be caught mid-cycle, which is why the pass criterion hangs on it and not on
state.The pass signal is a zero, so the parser needs a positive control.
deg = 0isindistinguishable from a regex that matches nothing, so the after-script requires a
must-be-present line (
wrap backfill, the node's unconditional periodic line) to parse non-zerobefore it will read any zero as meaningful — measured controls 41/35/43 on the current fleet. This
is the denominator: a checker that only reports failures cannot tell "nothing broken" from "nothing
examined". The binary-literal guard in step 0 protects the vocabulary; this protects the parsing.
Both are needed, and neither substitutes for the other.
Step 0 — prove the lines can still appear, before reading their absence
The success signal is the absence of
RELAY-CLOSEDlines, and an absence is only evidence if theline could still have been emitted. The after-script therefore checks the literals in the shipped
binary first and aborts if any is missing. Grep the stable PREFIX, not the full sentence, and
here is why that is not pedantry:
The
DEGRADEline's trailing parenthetical changed by design — #190 requires it to state thereal re-arm schedule instead of the old "re-armed on the next successful recovery". A step 0
matching that old sentence aborts on a correct build. The prefix is the contract; the tail is not.
For the same reason, expected literal counts must be calibrated against a binary, not remembered.
Measured on this branch (
cargo build -p mobee --features wallet,strings | grep -cF):Note these are counts of
stringslines, andeprintln!splits a message at each{}placeholder —so a count is a property of where the placeholders fall, not of how many call sites exist. If the
pre-registered expectations were taken from a differently-built binary they will not match this one,
and the mismatch is not a rename.
New literals this PR adds, all following the existing shape, none replacing anything:
RELAY-CLOSED RETRY:,RELAY-CLOSED RE-ARM,RELAY-CLOSED RE-ARMED:,RELAY-CLOSED UNKNOWN-ID:.