fix(transport)(sphere-sdk#473): defer DM cursor advance until dispatch resolves - #476
Closed
vrogojin wants to merge 1121 commits into
Closed
fix(transport)(sphere-sdk#473): defer DM cursor advance until dispatch resolves#476vrogojin wants to merge 1121 commits into
vrogojin wants to merge 1121 commits into
Conversation
…uler
Convert both monotonicity-violation arms from "throw + defer
recovery to awaitNextFlush + microtask baseline refresh" to
"recover in place + continue":
- (i) tokenMissing — when the token-set check finds token IDs
present in `previousData` but missing from this flush's `pkg`,
extract their TXF entries from `previousData` and ingest them
back into `pkg`. By construction `previousData` contains every
missing entry, so the recovery is total. Also union the
in-memory `opState` arrays with `previousData`'s opState via
`unionOpStateWithSentWins` so the per-entry-key OrbitDB write
does NOT tombstone live OUTBOX / SENT / etc. entries that
exist in the baseline but are absent from this flush's `data`.
The SENT-wins dedup drops any OUTBOX entry whose id matches a
SENT entry's id (the transition is terminal and the OUTBOX
residue must not be resurrected by the union).
- (ii) unknownBundleCids — the existing #255 / PR #262 inline
fetch+merge stays; only the fallback changes. If a foreign
bundle's CAR is unfetchable (network down, malformed CAR,
IPFS unavailable), we now log at warn-level and proceed with
whatever subset we merged. Subsequent cross-device syncs from
any peer will detect the same residual and re-attempt — the
aggregator-pointer-as-source-of-truth design principle covers
correctness while the auto-merge speeds metadata convergence.
Surface changes:
- Emit a new `storage:monotonicity-recovered` event whenever
anything was recovered or any residual remains. The payload
carries `recoveredTokenIds`, `mergedUnknownBundleCids`,
`residualTokenMissingIds`, `residualUnknownBundleCids`, and
`recoveredOutboxIdsDroppedAsSent` (capped at 100 each for log
volume). Distinct event type from `storage:error` — operators
see auto-merges as routine convergence work, not alarms.
- For truly-unrecoverable residuals we still emit `storage:error`
with the legacy POINTER_MONOTONICITY_VIOLATION code so existing
dashboards keyed on that literal still fire, but no longer
throw — the publish proceeds with the best-effort superset and
the next flush re-evaluates.
Helpers:
- `unionOpStateWithSentWins(currentOp, previousOp)` —
forward-compatible union helper that reads only `entry.id` /
`entry.tokenId` and applies the SENT-wins-over-OUTBOX dedup
rule. Richer per-entry fields ride through opaquely.
Issue #264 design principle: convergence MUST be guaranteed by
the aggregator pointer versions alone; profile metadata
convergence is eventually-consistent via auto-merge. The
at-least-once Nostr gate is no longer held closed by a transient
monotonicity violation — incoming TOKEN_TRANSFER events that
trigger a flush will see the auto-merge land and the gate ack.
…gate
- tests/unit/profile/pointer-monotonicity.test.ts:
* "race-stale flush" — assert auto-merge residual path: pinCalls
> 0, BOTH `storage:monotonicity-recovered` and legacy
`storage:error` events fire, no throw escapes.
* "token-loss" — assert clean recovery path: no throw, pinCalls
> 0, single `storage:monotonicity-recovered` event with
`recoveredTokenCount=1` and zero residual, NO `storage:error`
(residual count is zero, gate stays open).
- tests/unit/profile/monotonicity-inline-merge.test.ts:
* Two #255 fallback-throw tests rewritten to assert the auto-merge
residual contract: BOTH the recovered + legacy error events
fire with the right shape, AND the unfetchable residual stays
OUT of `lastLoadedFromBundleCids` (so the next flush re-runs the
bundle-set check and re-attempts the inline fetch — this is the
eventual-convergence story the legacy
`refreshBaselineForMonotonicity` microtask used to sabotage).
- tests/integration/pointer/category-C.test.ts (C9 + C10):
* `attemptsUsed=2` on fast-path-miss scenarios — cherry-picked
from #263 (commit 5a47983) since this branch ports the same
fast-path optimization.
- tests/integration/pointer/category-M.test.ts (M5):
* Device B starts at localVersion=1 so its fast-path attempt 0
lands at v=2 directly without firing fetchAndJoin (which would
break the "happy-path conservation" contract).
* Fake aggregator models REQUEST_ID_EXISTS for duplicate writes
at the same requestId — matches the production aggregator
contract and the SPEC's idempotency requirement.
- tests/unit/profile/lifecycle-manager-pointer-win-broadcast.test.ts:
* `buildStubPointer` now implements `winBroadcastsEnabled() {
return true; }` so the pre-#264 broadcast-emission contract
still runs end-to-end. The default-OFF policy is a
ProfilePointerLayer concern enforced by config normalization;
these tests pin the publisher-side wiring when broadcasts are
ON.
Code change (defensive):
- profile/profile-token-storage/lifecycle-manager.ts: the gate now
treats a missing `pointer.winBroadcastsEnabled` method as
flag=false (fail-closed). Keeps legacy stub-pointer test
harnesses (no broadcast method at all) compatible with the new
gate without per-test boilerplate.
Removed in flush-scheduler:
- The legacy `queueMicrotask(refreshBaselineForMonotonicity)` was
deleted from the residual emit path. Pre-#264 it rebuilt
`lastLoadedFromBundleCids` from OrbitDB's `listActiveBundles`,
which under the new auto-merge contract would mark the
unfetchable residual CID as "in baseline" — silently skipping
the next flush's inline-fetch retry. The auto-merge IS the
recovery mechanism; refreshing the baseline sabotages it.
Full vitest suite (482 files / 8158 tests) passes; lint + typecheck
clean.
…default Adds tests/unit/profile/monotonicity-auto-merge.test.ts covering two surfaces introduced by #264 that the test rewrites in the prior commit didn't pin in isolation: 1. unionOpStateWithSentWins helper (8 cases): - empty inputs - OUTBOX union by id, current-wins on collision - SENT-wins drops a single OUTBOX entry - SENT-wins drops multiple OUTBOX entries - SENT union by id, current-wins on collision - TOMBSTONES union by tokenId, JSON dedup for entries without tokenId - audit + finalizationQueue union by id, current-wins - empty/missing string ids excluded from SENT-wins match (avoid false positives on `id: ''` records) 2. enablePointerWinBroadcasts default-OFF policy (4 cases): - default (no config) → winBroadcastsEnabled() false, no storage:pointer-published event emit - explicit false → same suppression - explicit true → storage:pointer-published fires with the full signed payload + broadcast tag - truthy non-boolean values ('true' string, 1 number) coerced to false by the strict `=== true` policy — fail-closed defense against accidentally-truthy values from env-var unmarshalling or JSON parsing Per #264's "drop the 2 sibling-adoption cases from reconcile-and-publish.test.ts" instruction: that file was added by #263 alongside the siblingHighestV plumbing this branch INTENTIONALLY omits per option (b) of the implementation note. It does not exist on this branch, so the drop is satisfied by construction. Code change (test-affordance): - profile/profile-token-storage/flush-scheduler.ts: `OpStateArrays` and `unionOpStateWithSentWins` are now exported so the helper can be tested in isolation without driving a full ProfileTokenStorageProvider flush. Full vitest suite: 483 files / 8170 tests / 13 skipped pass. Lint + typecheck clean.
…ervability Addresses the six findings from the /steelman adversarial review on this branch: CRITICAL fixes: 1. Amplification-minimal opState union (flush-scheduler.ts) — the naive union re-emitted every previous entry through the per-entry writer. Because `writeProfileKey` encrypts with a fresh random IV (`profile/encryption.ts:122`), every re-emit lands a new OrbitDB OpLog row even when the plaintext is identical — O(previous-size) OpLog amplification per recovery cycle under sustained cross- device churn. The union now ONLY adds previous entries whose id is missing from current (the entries that would otherwise be tombstoned by the diff). Entries shared by both sides are written once, from current. SENT-wins dedup still applies post-union. 2. Drop `transfer:operator-alert` from residual emits (flush-scheduler.ts) — pre-#264 a monotonicity violation was rare + hard-throw and warranted paging. Post-#264 the residual fires on routine network transients (gateway 503, peer offline). Pagers keyed on `data.alert === 'transfer:operator-alert'` would burn out on-call within hours of a flaky IPFS gateway. Operators who DO want paging on residuals can opt in via the `autoMergeResidual: true` discriminator paired with the unchanged `code: POINTER_MONOTONICITY_VIOLATION`. WARNING fixes: 3. Symmetric `winBroadcastsEnabled` guard in Sphere subscriber (core/Sphere.ts) — mirrors the `typeof === 'function' && pointer.winBroadcastsEnabled()` pattern from lifecycle-manager.ts. Fail-closed for any future test stub or duck-typed pointer that lacks the accessor. 4. Preserve probeHistory on fast-path success (ProfilePointerLayer.ts) — the #263 fast-path returns `probeHistory: []`; unconditionally assigning would clobber any fingerprint populated by a prior discovery / recoverLatest / probe call, destroying the UI same- wallet-clustering signal via `getProbeFingerprint()`. Now only assigns when `result.probeHistory.length > 0`. 5. Test fixture for `truncated=true` boundary (tests/unit/profile/pointer-monotonicity.test.ts) — recovers 150 tokens to verify the 100-cap slice + `truncated: true` flag correctly track per the contract documented in storage-provider.ts. Without this, a regression flipping the cap or the flag would silently degrade operator visibility. 6. Bridge `storage:monotonicity-recovered` through Sphere (types/index.ts, core/Sphere.ts) — adds the Sphere event type + payload + forwarder in the existing token-storage event bridge. Operators subscribing via `sphere.on('storage:monotonicity- recovered', ...)` now see auto-merge convergence work without dropping to provider-direct subscriptions. The forward is a pure informational pass-through with `providerId` added for fan-out attribution. Tests: - 2 new union-contract assertions in monotonicity-auto-merge.test.ts pinning the amplification-minimal contract (shared ids appear once, previous-only ids fill in). - 1 new large-scale-recovery test in pointer-monotonicity.test.ts pinning the truncated=true cap boundary. - All 3 existing alert-field assertions updated to discriminate on `autoMergeResidual: true` and verify `data.alert === undefined`. Full vitest suite: 483 files / 8173 tests / 13 skipped pass. Lint + typecheck clean.
…metric guards Addresses the 6 findings from the second /steelman pass on the prior remediation commit (c47720b): CRITICAL fixes: 1. Revert "amplification-minimal" union — it doesn't actually reduce writes. On re-analysis the union has always been size `|current ∪ previous|`, and the writer's per-entry diff writes every live entry regardless. Amplification reduction is a WRITER-layer concern (plaintext-aware diff or deterministic-IV encryption — broader refactors out of scope for #264). Restore the simpler naive Map-based union: dedupe by id via `Map.set` (current wins on collision), no-id entries collect in a separate bucket. This also fixes two regressions the variant introduced: - intra-current duplicate ids no longer collapse - empty-string ids (`id: ''`) routed to no-id bucket, losing Map dedup Update docstring to acknowledge the writer-layer follow-up. 2. Drop the 2 decorative "amplification-minimal" tests — they produced `length === 2` under BOTH the naive and "fixed" union, so they didn't pin anything. Replace with 2 new regression tests pinning the contracts the variant actually broke: - intra-current duplicate ids collapse to one entry per id - empty-string id is dedup-able via Map 3. Apply symmetric probeHistory guard to `#discoverLatestVersionInner` — the prior fix only landed on `publish()`. Since `recoverLatest()` runs as `#recoverLatestInner → #discoverLatestVersionInner`, an empty `probeVersions` (discovery resolved on first probe) would still clobber the preserved fingerprint. Guard symmetrically. WARNING fixes: 4. Symmetric typeof guard for `getSignerForWinBroadcast` in BOTH `lifecycle-manager.ts:publishAggregatorPointerBestEffort` AND `core/Sphere.ts:maybeInstallPointerWinSubscription`. A pointer stub returning `winBroadcastsEnabled() === true` but lacking `getSignerForWinBroadcast` would TypeError; the symmetric `typeof === 'function'` check fails closed earlier. 5. Export `MONOTONICITY_RECOVERY_PAYLOAD_CAP = 100` so tests pin the boundary atomically with the implementation. Adds three boundary tests: - recoveredCount === CAP → truncated=false (locks `>` vs `>=`) - recoveredCount === CAP + 1 → truncated=true (smallest count that exceeds the cap) - recoveredCount well above CAP (150) → truncated=true with full count surfaced in the count field 6. New `tests/unit/profile/steelman-remediation-264.test.ts` covering: - probeHistory preservation: a stubbed empty-probeHistory publish does not change `getProbeFingerprint()` output. - Sphere bridge: the `storage:monotonicity-recovered` event shape pinned at the provider boundary so schema drift between flush-scheduler emit and Sphere forward is compile-detectable. - symmetric subscriber guard: stub without `winBroadcastsEnabled` short-circuits cleanly; stub with `winBroadcastsEnabled() === true` but missing `getSignerForWinBroadcast` also short-circuits. Updated existing tests: - SENT-wins null/undefined-id test rewritten to actually use `null`, `undefined`, numeric, and object ids (previous fixture used empty strings, which the reverted union now accepts as real ids). Full vitest suite: 484 files / 8181 tests / 13 skipped pass. Lint + typecheck clean.
…ests + strict-boolean guards
Addresses round-3 /steelman findings — primarily that round-2's new
remediation tests didn't actually exercise production code:
CRITICAL fixes (non-tautological tests):
1. Rewrite `steelman-remediation-264.test.ts` to drive PRODUCTION
code paths instead of re-implementing predicates:
- **probeHistory preservation** now uses `vi.mock` on
`discover-algorithm.ts:findLatestValidVersion` to control
`result.probeVersions`, then invokes the REAL
`ProfilePointerLayer.discoverLatestVersion()` public method.
The production conditional at `ProfilePointerLayer.ts:541`
decides whether to overwrite `#lastProbeVersions`. We observe
via the real public `getProbeFingerprint()` getter.
Verified non-tautological by temporarily inverting the
production guard (`if (true)`) and confirming the test FAILS.
- **Sphere event bridge** now constructs a minimal Sphere via
`Object.create(Sphere.prototype)` + the real
`subscribeToProviderEvents` private method. A fake provider
emits `storage:monotonicity-recovered`, the bridge fires the
real `emitEvent` which dispatches through `this.eventHandlers`
(the same Map `sphere.on()` registers into). Asserts
providerId attached + field defaults applied.
- **Symmetric subscriber guard** now invokes the REAL
`maybeInstallPointerWinSubscription` private method via cast.
Stubs `_storage.getPointerLayer()` to return partial pointer
shapes (missing methods or wrong boolean type) and asserts
no subscription installation via a spy on
`_transport.subscribeToBroadcast`. The production guard at
`core/Sphere.ts:5477-5491` is on the call stack.
WARNING fixes:
2. Tighten `winBroadcastsEnabled()` guards to `=== true` in BOTH
`lifecycle-manager.ts:1275-1290` AND `core/Sphere.ts:5478-5490`.
Mirrors the production `ProfilePointerLayer` constructor's
`=== true` normalization, so test stubs returning truthy
non-boolean (`1`, `'yes'`, `{}`) fail closed in both guard
sites — same policy as production config.
NOTE fixes (documentation):
3. Document the `winBroadcastsEnabled` / `getSignerForWinBroadcast`
API pairing contract on `ProfilePointerLayer.winBroadcastsEnabled()`:
implementations MUST expose both methods; partial implementations
are treated as flag=false (fail-closed).
4. Document init-time-only semantics of the
`enablePointerWinBroadcasts` flag: a runtime false→true flip
without a fresh publish leaves the subscriber dormant until the
next emitted `storage:pointer-published` event. Acceptable
trade-off given the flag's default-OFF design.
Full vitest suite: 484 files / 8184 tests / 13 skipped pass.
Lint + typecheck clean.
Test sanity check: temporarily inverting the production probeHistory
guard caused exactly the probeHistory preservation test to FAIL,
confirming the test is now genuinely sensitive to the production
code path.
…ten cold-start test
Round-4 /steelman flagged 3 small WARNINGs and several NOTEs. Round
verdict was already non-tautological for all rewritten tests. This
commit applies the trivial improvements:
DOC additions on `ProfilePointerLayer.winBroadcastsEnabled()`:
1. **Accessor contract**: `winBroadcastsEnabled()` MUST be a pure
accessor and MUST NOT throw. Documents the silent-failure
paths if a stub does throw — publisher returns transient with
ok=true (broadcasts silently disabled), subscriber logs
"subscription install failed" and indefinitely re-arms. A real
`ProfilePointerLayer` reads the frozen config snapshot and
cannot throw; alternative implementations MUST honor the same
contract.
2. **Receive-only-wallet caveat**: a wallet with the flag enabled
but no publishing path will never emit
`storage:pointer-published`, so its
`maybeInstallPointerWinSubscription` is never triggered via
own-publish. For pure receive endpoints the wiring layer
must call `maybeInstallPointerWinSubscription()` explicitly
(e.g. on init). Documents the gap so consumers aren't
surprised.
TEST tighten in `tests/unit/profile/steelman-remediation-264.test.ts`:
3. The cold-start fingerprint test now also asserts the
empty→non-empty transition: after an empty-probe discovery
preserves the empty fingerprint, a follow-on real probe MUST
produce a non-empty fingerprint. Catches a hypothetical buggy
variant `if (length === 0) assign` that the prior assertion
missed.
INFRASTRUCTURE:
4. Add `.tmp/` to `.gitignore` so the test-soak workspace
(mnemonics + per-iteration logs) doesn't accidentally land in
a commit. Was hand-unstaged twice across the remediation
rounds; gitignore prevents future slips.
Items intentionally deferred to a follow-up issue (round-4 NOTEs):
- End-to-end coverage of auto-merge through
`sphere.payments.send()` → PaymentsModule → provider.
- Convert `MONOTONICITY_RECOVERY_PAYLOAD_CAP` dynamic import in
tests to a static import.
- Comment on `JSON.stringify` non-canonical dedup invariant in
`unionByTokenIdOrJson`.
- The Nostr at-least-once gate behavior change (residuals no
longer block `since` advancement) — by design per #264 spec.
Belongs in PR description / operator runbook, not in code.
Full vitest suite: 484 files / 8184 tests / 13 skipped pass.
Lint + typecheck clean.
…accuracy
Round-5 /steelman caught a real WARNING: the round-4 doc claimed the
publisher returns `{ ok: true, transient: true }` when
`winBroadcastsEnabled()` throws, but the actual flow escaped the
inner short-circuit to the outer catch which returns
`{ ok: false, transient: true, code: 'UNCLASSIFIED' }`. The doc was
factually wrong AND the misclassification could make a perfectly-
successful publish appear to fail.
This commit BOTH tightens the implementation AND corrects the doc:
CODE — defensive try/catch around `winBroadcastsEnabled()` in both
guard sites (lifecycle-manager + Sphere):
- Treats accessor throw as flag=false (fail-closed policy).
- Publish still reports success; subscriber early-returns cleanly.
- Single log line per throw — host-log level in lifecycle-manager,
debug-level in Sphere — so operators can correlate accessor
contract violations without polluting `storage:error`.
DOC — corrects ProfilePointerLayer.winBroadcastsEnabled() docstring
to reflect the defensive treatment as the actual contract:
"Both the publisher and subscriber guards wrap the accessor call in
a defensive try/catch that treats a throw as flag=false". Honest
about why the defensive treatment exists: a real ProfilePointerLayer
reads its frozen config snapshot and cannot throw; this defensive
treatment exists so a misbehaving stub or alternative implementation
cannot silently corrupt publish-success classification.
DOC — receive-only-wallet caveat clarified: explicitly notes that
`core/Sphere.ts` does NOT currently arrange an init-time call;
receive-only-wallet support for pointer-win broadcasts requires a
follow-up wiring change. Acceptable today because the flag is
default-OFF and broadcasts are an optimization layer.
TEST — adds one new test asserting the defensive try/catch contract:
a pointer stub that throws from `winBroadcastsEnabled()` MUST cause
the subscriber path to early-return without subscription
installation and without throw-escape. The test would fail under
the round-4 code (throw would escape to the outer catch and surface
as a noisy log), pinning the new defensive treatment.
TEST — drops the redundant `expect(fp2).not.toBe(fp0)` assertion in
the cold-start test. `fp2.length > 0` already covers it since
`fp0 === ''`. R4 steelman noted both lines were equivalent.
Full vitest suite: 484 files / 8185 tests / 13 skipped pass.
Lint + typecheck clean.
…gical
Round-6 /steelman flagged that the round-5 defensive-throw test was
decorative: the outer broad catch already returns void + finally
resets _pointerWinInstallInFlight, so the assertions
(`resolves.toBeUndefined()`, `subscribeCalls === 0`, `inFlight === false`)
all pass even if the inner defensive try/catch were removed.
This commit tightens the test to actually pin the R5 inner-catch
behavior by spying on the logger:
- INNER (R5) path emits `logger.debug('Sphere', '... accessor
contract violation, treating as flag=false: ...')`. Test
asserts this debug line was emitted.
- OUTER (R4) broad-catch path emits `logger.warn('Sphere', '...
subscription install failed (will retry on next event)...')`.
Test asserts this warn was NOT emitted.
Sanity-checked by temporarily replacing the inner try/catch with a
direct expression (so throws escape to the outer catch): the test
FAILED on `expect(accessorViolationLog).toBeDefined()` — confirming
the test now genuinely distinguishes the two code paths.
This was the last round-6 NOTE-level item. Verdict: SHIP IT after
this commit lands.
Full vitest suite: 484 files / 8185 tests / 13 skipped pass.
Lint + typecheck clean.
Adds `OrbitDbConfig.httpOnlyIpfs?: boolean` and defaults it to `true` in
`createNodeProfileProviders` and `createBrowserProfileProviders`.
When enabled, the OrbitDB adapter:
- skips Helia's `FsBlockstore` (uses default MemoryBlockstore),
- skips the Helia libp2p `directory` (no peer-id/keychain on disk),
- forces libp2p into isolated mode (no DHT, bootstrap, peerDiscovery,
autoNAT, dcutr, delegatedRouting, ipnsFetch, ipnsPublish — only
identify/identifyPush/keychain/ping + the gossipsub stub required
by OrbitDB v3 remain),
- disables Helia's default block brokers (`bitswap` would hang
without peers; `trustlessGateway` was walking public gateways like
`trustless-gateway.link` and `4everland.io` and timing out on
every CLI invocation).
The blockstore-get monkey-patch already swallowed `NotFoundError`;
extend it to also swallow `InvalidConfigurationError` so OrbitDB sees
a clean missing-block signal instead of erroring out when `blockBrokers: []`.
OrbitDB's level DB (OpLog heads) is still persisted under `directory`
in lightweight mode — only Helia / libp2p artefacts go memory-only.
Cross-device durability is served by the operator-side Kubo gateway
via HTTP plus the snapshot prefetch in `profile/ipfs-client.ts`, NOT
by libp2p/Bitswap.
Root cause: `createNodeProfileProviders` did not pass `bootstrapPeers`
to the adapter, so the adapter took the full `libp2pDefaults()` path —
80+ TCP connections to port 4001, ~3 min wall-clock per CLI invocation
(blocked §D of the issue #265 / PR #264 soak on 2026-05-25). The
"isolated mode" branch already existed but only triggered on explicit
`bootstrapPeers: []`, which the wallet factories never passed.
Tests:
- 3 new positive tests verifying CRUD, no FsBlockstore on disk, and
httpOnlyIpfs winning over a non-empty bootstrapPeers list.
- 1 adversarial reopen test pinning the fast-fail contract (<5s,
typically <100ms) so we don't accidentally regress to the public
trustless-gateway walk.
Verification: typecheck clean, tsup build clean, lint 0 errors,
7581 unit tests pass, 449 integration tests pass (21 in the adapter
suite including the 4 new cases).
…r Kubo Round 2 of the issue #266 lightweight-client work, addressing user direction: "store all its related records in memory only" and "support talking to our remote IPFS Kubo node via http only". Round 1 (previous commit) stripped libp2p networking but kept Helia's on-disk FsBlockstore for cross-process recovery. That worked for the performance fix (CLI startup 1.6s vs 14+ min previously, 0 TCP connections on port 4001) but still wrote ~MB of CAR blocks to disk per session. Round 2 goes fully memory-only and uses operator-controlled Kubo gateways as the durable source of truth: - New module `profile/http-block-broker.ts` exports `createHttpBlockBroker({ gateways })` — a Helia `BlockBroker` whose `retrieve()` issues `POST /api/v0/block/get?arg=<cid>` against each operator Kubo, races them via `Promise.any`, honours per-request timeout (10s default) and the caller's AbortSignal. Helia's `NetworkedStorage` rehashes the response, so a misbehaving gateway can DoS but cannot forge data. - When `httpOnlyIpfs: true` AND `ipfsGateways: [urls]`, the adapter installs ONE HTTP broker (no bitswap, no public trustless gateways). A local memory-blockstore miss falls through to operator Kubo via HTTP, the bytes are re-cached in memory by Helia's NetworkedStorage, and OrbitDB sees the block as it would with a disk-backed adapter. - When `httpOnlyIpfs: true` but no `ipfsGateways` (e.g. CI tests with `bootstrapPeers: []` and no network), `blockBrokers: []` is installed so a local miss returns FAST (the monkey-patched `blockstore.get` swallows the resulting `InvalidConfigurationError` and returns `undefined`). - `OrbitDbConfig` gains `ipfsGateways?: ReadonlyArray<string>` so the raw adapter doesn't have to reach back into `ProfileConfig`. `ProfileStorageProvider.connect()` passes through `config.ipfsGateways` from `ProfileConfig` so the existing `createNodeProfileProviders` wiring (which already populates `ipfsGateways` from the network config) reaches the broker automatically. - `directory` is no longer passed into Helia when `httpOnlyIpfs: true` (skips both `FsBlockstore` and the libp2p datastore — peer id and keychain on disk). OrbitDB's level DB (OpLog heads) still uses `directory` for persistence; only the IPFS layer goes memory-only. Tests: - New end-to-end recovery test stands up a tiny in-process HTTP server that mimics `POST /api/v0/block/get`. Writes data with adapter A, snoops Helia's blockstore.put to populate the fake Kubo, closes A, opens adapter B in the same directory but with a fresh Helia memory blockstore. B HTTP-fetches the missing blocks from the fake Kubo and decodes the OpLog cleanly. - Fast-fail reopen test (no gateways): a missing block returns `null` quickly, no 30s walk over public trustless gateways. - Updated `httpOnlyIpfs skips FsBlockstore` to assert the `<dir>/blocks/` directory does NOT exist (was: DOES exist after round 1). Verification: - 22 adapter tests pass (was 21 in round 1; +1 for HTTP recovery). - 75 integration tests across profile + oplog-bundle + adapter suites pass. - typecheck clean, tsup build clean. - Live CLI smoke (testnet, fresh profile-mode wallet): `sphere init --profile` → 1.6s (was 14+ min hang) `sphere balance` (fresh process) → 21s, 0 TCP on port 4001, VmRSS 1672 KB, 1 thread, fails fast on a missing block because the freshly-initted wallet's blocks hadn't been pushed to operator Kubo yet (orthogonal issue: shutdown-gate flush completion on short-lived CLI invocations — does NOT cause the libp2p / DHT cost that issue #266 was about).
…ved tradeoff) Round 3 of the issue #266 work: per user direction "preserving local helia storage (but no libp2p bootstraping)", restore the on-disk FsBlockstore in lightweight mode. Round 2's "memory only" approach worked end-to-end in unit tests (via the new HTTP block broker against operator Kubo) but exposed a fresh-wallet edge case in the live CLI: a `sphere init` exits before the async flush-scheduler has pushed blocks to operator Kubo, so the next CLI invocation cannot HTTP-fetch them back. Keeping FsBlockstore on the same dataDir resolves this without requiring a separate fix to the flush-on-init timing window. Disk blocks are MB-scale — negligible vs the libp2p networking costs we strip (the actual issue #266 problem). The HTTP block broker (added in round 2) stays. It serves cross-DEVICE recovery — a fresh device pulls the snapshot CAR + OpLog blocks from operator Kubo over HTTP, no libp2p needed. Helia's NetworkedStorage only consults brokers on a local blockstore miss, so FsBlockstore hits stay fast. Tests updated: - "httpOnlyIpfs: true KEEPS FsBlockstore" now asserts the `<dir>/blocks/` directory DOES exist (was: does NOT exist in round 2). - "reopen on same directory" now asserts the prior write IS recovered via FsBlockstore (was: returns null + fast-fail). - The HTTP-broker recovery test now explicitly WIPES `<dir>/blocks/` between phases to simulate a cross-device move (otherwise FsBlockstore would satisfy the read locally before the broker fires). Live CLI smoke (testnet, fresh profile-mode wallet): `sphere init --profile --autoGenerate` → 2.3s (was 14+ min hang) `sphere balance` (fresh process) → 2.5s SUCCESS "No tokens found." (correct) 0 TCP on port 4001 (was 21s with ORBITDB_READ_FAILED) The original 80+ libp2p TCP connections / 3 min CPU spin / 14 min hang are all gone, AND full single-device functionality is preserved.
…-266 feat(profile)(#266): HTTP-only IPFS mode for wallet/CLI clients
`FlushScheduler.flushToIpfs` previously awaited
`engine.consolidate()` inline whenever bundle count exceeded
`CONSOLIDATION_THRESHOLD` (3). The engine carries a 30 s TOCTOU
sleep (`consolidation.ts:199`) so cross-device concurrent
consolidations can detect each other's `consolidation.pending`
state before both pin redundant CARs.
The cost of that inline 30 s sleep is paid by every flush that
trips the threshold — which means every CLI session that
received >3 tokens via the faucet, or every wallet that
accumulated >3 active bundles. Each flush now blocks 30 s,
which:
- times out shutdown's `awaitNextFlush(30 s)` → the at-least-
once Nostr replay loop documented in #268;
- serializes incoming TOKEN_TRANSFER receives at 30 s each
via `handleIncomingTransfer → awaitAllProvidersDurable →
awaitNextFlush`;
- manifests as the §C.2 soak hang in
`manual-test-full-recovery.sh`: `sphere payments sync`
spinning for 14+ minutes at 200 %+ CPU because each of 6
replayed faucet events triggers a 30 s blocked flush.
Detach consolidation from the inline path: the durability-
critical work (pin + bundle ref + publish) returns immediately
once it completes, and consolidation continues in the background
via a fire-and-forget IIFE. An in-memory
`consolidationInFlight` flag dedupes concurrent flushes on the
same instance so a burst of N flushes spawns exactly one
background task. The engine's `isConsolidationInProgress` check
still handles cross-device concurrency, and its 30 s TOCTOU
sleep `.unref()`s its timer so a process exit during the sleep
simply cancels it.
Reproducer (`/tmp/sphere-issue-268/repro-c2.sh`):
- Before: §C.2 invoice pay spins for 14 min at 200 %+ CPU
- After: §C.2 invoice pay completes in 19.8 s, payment
submitted, no `awaitNextFlush failed` messages
All 7581 unit tests pass.
…y check
The pointer-monotonicity TOKEN-SET check at
`flush-scheduler.ts:489` previously did a raw set-difference of
storage keys (`previousTokens.keys()` vs `tokens.keys()`). The
diff over-reported violations whenever PaymentsModule retired a
token via its normal lifecycle:
(a) **Tombstone** — `removeToken()` appends a
`{tokenId, stateHash, timestamp}` row to `_tombstones` and
deletes the `_<tokenId>` wallet entry. The active key
legitimately disappears in the next flush.
(b) **Archive (state transition)** — `addToken()` archives the
prior state under `archived-<tokenId>` and deletes the
wallet-level `id_v1` entry whose `token.id` no longer
matches the new state. The active `_<tokenId>` key is
re-populated by the new state on the next
`createStorageData()` call, but during the invoice-pay /
receive-finalize race the active key can disappear
transiently while the archive entry remains the canonical
record.
Both retirements throw `POINTER_MONOTONICITY_VIOLATION` →
`forceFlushSerialized` rejects → `awaitNextFlush` throws →
`handleIncomingTransfer` marks the event non-durable →
`AT-LEAST-ONCE` Nostr replay → next reconnect re-runs the same
event → cascade. With ~7 events queued, the daemon + CLI both
spin at 200 %+ CPU; the soak (issue #268) hangs `invoice pay`
for 3+ minutes after `receive --finalize`.
Recognize legitimate removals by checking the missing storage
key against:
- the underlying token id (prefix-stripped) of any current
`_<id>` OR `archived-<id>` entry — covers (b);
- the tombstone row tokenId set — covers (a).
The change is local to the check: no public API surface, no
behavior change for genuine violations (cross-device race, partial
save() bug). Verified with the §C.2 reproducer:
Before: invoice pay timed out after 180 s (status: hang)
After: invoice pay completes in 26 s, status: submitted
All 1910 profile unit tests + 515 PaymentsModule tests pass.
fix(profile)(#268): unblock §C.2 — detach consolidation + fix monotonicity over-reporting
…match) as permanent `finalizeStrandedReceivedToken` previously treated SDK `VerificationError` with `verificationResult.message === 'Recipient address mismatch'` as transient — the catch logged at error level and returned, leaving the token in 'pending'. `drainPendingFinalizations` then burned its full timeoutMs window every sync because `hasUnconfirmedOrInflight()` kept seeing the unresolved token. With the at-least-once Nostr replay (PR #240) every replayed `transfer:incoming` re-triggered the same drain, surfacing as the §D.1 hang in `manual-test-full-recovery.sh` discovered while validating #268. By the time this catch fires, `finalizeTransferToken` has already run `tryRecoverSigningServiceForRecipient` (PR #251/#255) and exhausted every tracked HD address — none of our signers derive the predicate the sender targeted. Retrying with the same inputs cannot succeed. Classification (additive — `InvalidJsonStructureError` path from PR #233 is untouched): - VerificationError with verificationResult.status=1 AND verificationResult.message containing "address mismatch" or equal to "Recipient address mismatch" → permanent NOT-OUR-STATE. - Other VerificationError causes (e.g., "Predicate verification failed", "Recipient data verification failed") stay transient — those can be genuine bad-proof / signature blips that retry resolves. Handling mirrors the structural path: 1. Token marked 'invalid' so recoverStrandedReceivedTokens skips it. 2. Proof-polling job removed. 3. `transfer:operator-alert` emitted with code='not-our-state' (canonical DispositionReason for "structurally valid, just not spendable by us") and a message explaining the cause + manual workaround (activate missing HD index, re-import pointer). Funds are NOT lost: the aggregator-confirmed transfer is unchanged. The mark is wallet-local. If the sender targeted an HD index this wallet hasn't activated yet, value sits in the aggregator state addressed to a predicate we DO derive (just not iterated). A reversibility hook (auto-retry on `address:activated`) is deferred to a follow-up; for now operators see the alert and can re-scan after activating the missing index. Tests: two new units in `PaymentsModule.proof-polling-persistence.test.ts` exercise the positive (Recipient address mismatch → invalid + not-our-state alert) and negative (Predicate verification failed → transient) classifier paths. All 436 existing PaymentsModule tests remain green. Refs: discovered while validating PR for #268 against §D.1 of `manual-test-full-recovery.sh`.
…ismatch fix(payments)(#269): classify VerificationError(Recipient address mismatch) as permanent
…ity-issue-264 fix(profile)(#264): auto-merge monotonicity violations; gate broadcasts behind flag (default OFF)
…least-once gate; bound replay budget §C.2 of `manual-test-full-recovery.sh` hangs on `integration/all-fixes` HEAD `6102d59` with all 3 node processes pegged at 90-175% CPU for 6+ minutes (forensics: `.tmp/soak-postmerge-271/`). 134 `[AT-LEAST-ONCE] not durable` warnings appear for only 14 unique TOKEN_TRANSFER event IDs — ~10 replays per event. Root cause: the per-flush remote-durability HEAD-check (`LifecycleManager.verifyFlushDurability` → `verifyPinLeg`, default 30 s) ran inline in `flush-scheduler.flushToIpfs()`. Under contended testnet, the operator IPFS gateway didn't serve the just-pinned CID back within 30 s → `awaitNextFlush` rejected with `FLUSH_DURABILITY_TIMEOUT` → `awaitAllProvidersDurable` returned false → the Nostr `since` cursor refused to advance → every reconnect re-fired the same 14 events → each replay scheduled another flush → busy-spin. This is the fourth surface of the same coupling defect: - #266 (HTTP-only IPFS) — startup costs in the budget - #268 (consolidation TOCTOU + monotonicity over-reporting) — extra work in the budget - #269 (`Recipient address mismatch` retried as transient) — infinite spin in the budget - #272 (this) — gateway propagation jitter outside the budget Parallel architectural + code-quality reviews converged: the per-flush HEAD-check is conflating LOCAL crash-safety (which is satisfied by CAR pin POST 200 + OrbitDB ref written + publish call ok) with REMOTE gateway propagation observability (a property of operator infrastructure, not of the receiver's correctness). Awaiting the remote-observability leg inline made every gateway hiccup amplify into a replay storm. Fixes: 1) **Patch 1 (root cause)** — `flush-scheduler.ts`: launch `verifyFlushDurability` as a fire-and-forget background task instead of awaiting it inline. On failure, emit a new typed `storage:durability-deferred` event for operator triage. The synchronous flush completes as soon as local-durability conditions are met; the at-least-once gate advances; the Nostr cursor moves forward. 2) **Patch 2 (defense-in-depth)** — `NostrTransportProvider.ts`: add a per-event failure cooldown ledger. After a durability miss, subsequent dispatches of the same event ID are skipped during an exponential backoff (30 s → 60 s → 120 s cap). After the `MAX_REPLAY_ATTEMPTS` budget (3) exhausts, the cursor advances anyway with an operator alert — matching the acceptance criterion "[AT-LEAST-ONCE] not durable count per token bounded by a small constant (≤3) rather than unbounded replay." LRU-capped at 256 entries to bound memory. 3) **Patch 3 (perf)** — `lifecycle-manager.ts`: short-circuit `verifyFlushDurability` when both CIDs match the last-verified watermarks. Prevents repeated ~80 HEAD probes against unchanged CIDs across replays. 4) **Patch 4 (timer leak)** — `profile-token-storage-provider.ts`: clear the `setTimeout` in `awaitNextFlush`'s `Promise.race` via `finally` (code-reviewer's confirmed bug #3 — setTimeout was never cleared when `chained` settled first, accumulating timer handles under replay storms). Acceptance criteria from issue #272: - §C.2 clears within reasonable window without CPU pegging > 50% sustained — Patch 1 removes the gating coupling, Patch 2 caps replays even if the gate somehow re-engages. - `[AT-LEAST-ONCE] not durable` count per token bounded by ≤ 3 — Patch 2 enforces `DURABILITY_MAX_REPLAY_ATTEMPTS = 3` directly. - §C.3 / §C.4 / §D / §E continue to pass — local-durability invariant (CAR pin POST + OrbitDB ref + publish call ok) is unchanged; only the remote-observability leg moves out of the sync path. Tests: - `tests/unit/transport/NostrTransportProvider.durabilityCooldown272.test.ts` — 8 tests covering `recordDurabilityMiss` exponential backoff, `isInDurabilityCooldown` lifecycle, LRU eviction, and the `MAX_REPLAY_ATTEMPTS` budget exhaustion behavior. - `tests/unit/profile/lifecycle-manager-verify-shortcircuit-272.test.ts` — 4 tests covering the CID-change short-circuit (network call count via mocked `fetch` HEAD). - Full suite: 8177 passed | 13 skipped (unchanged from baseline). Long-term follow-ups (not in this PR): - Replace the boolean `tokenTransferDurable` ack with a persistent per-event retry ledger so process restart doesn't lose the cooldown budget (architecture review's structural recommendation). - Plug the `MultiAddressTransportMux.dispatchWalletEvent` gate bypass (code-reviewer's confirmed bug #1 — separate from #272, needs its own issue). Closes #272.
…y-loop fix(profile,transport)(#272): decouple per-flush HEAD-verify from at-least-once gate
…mps, namespaces, spans Address issue #274 — SDK API call latency profiling. Step one is a toggleable verbose logger that can show every step of SDK operations with a prefix + timestamp. This is the diagnostic infrastructure; the actual perf fix (persistent processedEventIds, daemon-RPC, etc.) is separate work tracked in #274. ## Logger extension (core/logger.ts) Extends — does not replace — the existing 150-LoC singleton. All 1114 existing `logger.debug(tag, msg, ...args)` call sites continue to work unchanged. New surface: - Five levels: trace | debug | info | warn | error - `getLogger('payments:send')` returns a namespaced logger with `.child()`, lazy `*Lazy()` builders, and a `.time()` Span primitive that emits ONE record per call carrying durationMs + marks. - `setDebug('payments:*,transport:nostr=trace')` runtime API + env bootstrap (`SPHERE_DEBUG` / `SPHERE_LOG` / `localStorage.SPHERE_DEBUG`). Spec grammar supports namespace globs, per-namespace level qualifiers, and `-pattern` negation. - Pluggable `LogSink` array (default = console). `addSink`, `clearSinks`, `createRingBufferSink(capacity)` for crash-diagnostic capture. - ISO-8601 timestamp + level + namespace prefix when timestamps are enabled. Auto-enabled by `setDebug(spec)`; legacy `configure({debug: true})` leaves the bare `[Tag] msg` console shape untouched. - Secret redaction: 60+ field-name denylist (covers `privateKey`, `mnemonic`, `nsec`, `encryptionKey`, `masterKey`, `chainCode`, `wif`, `xpriv`, `xprv`, `peerId`, `accessToken`, `ciphertext`, `iv`, `salt`, `nonce`, plus camelCase/snake_case/kebab variants) plus a regex-based catch-all. Deep recursive redaction with cycle detection and an 8-level depth cap (fail-closed at the cap). - Log-injection defense: control characters in `message` and namespace strings are escaped to `\xNN` notation before formatting. Spec parsing rejects oversized inputs (>8 KB or >256 entries) and patterns with control characters. ## Hot-path instrumentation Per perf-engineer's report on `/tmp/soak-272c/`, instrumented these sites with timing spans (zero behaviour change — all spans are zero-cost when the namespace is disabled): - `PaymentsModule.send` — span per UXF/legacy dispatch route - `PaymentsModule.receive` — span with fetch/load/finalize marks - `PaymentsModule.handleIncomingTransfer` — span ended in finally so all 15+ internal early-return paths are covered - `PaymentsModule.awaitAllProvidersDurable` — the §C.2 cost dominator; per-provider duration marks - `NostrTransportProvider.fetchPendingEvents` — events fetched + dispatch - `ProfileTokenStorageProvider.awaitNextFlush` — entry log with timeout - `AccountingModule.payInvoice` — entry log for cross-correlation - `L1PaymentsModule.send` — span with success/fail outcome - `Sphere.init` — lifecycle span (created flag) ## Tests - `tests/unit/core/logger.test.ts` — all 20 legacy contract tests pass unchanged (validates back-compat) - `tests/unit/core/logger.extended.test.ts` — 66 new tests covering spec parsing, namespace resolution, env bootstrap, redaction (deep + arrays + denylist coverage), spans, ring buffer, multi-sink, log-injection, DoS-bound spec rejection - 3456 module tests pass (PaymentsModule + AccountingModule + NostrTransport + profile + core all green) - tsup build clean, tsc --noEmit clean ## Operator usage ```bash SPHERE_DEBUG='payments:*,transport:nostr=info' npm run cli -- balance # Browser: localStorage.setItem('SPHERE_DEBUG', 'payments:send=trace') # Runtime: setDebug('payments:*'); disableDebug(); ``` Output line shape with timestamps on: ``` [2026-05-26T14:23:01.482Z] [DEBUG] [payments:send] span.end send {...} ``` Branched off integration/all-fixes (not main) per the issue's directive — the §C.2/§C.4 forensics being instrumented live only on that branch.
feat(sphere-sdk)(#274): toggleable verbose debug logging with namespaces, spans, redaction
…V5 bundle dedup Issue #275 root cause: `processedEventIds: Set<string>` in NostrTransportProvider lived in-memory only, so every fresh CLI process re-walked the relay backlog. The §C soak forensics showed 71.5% of wall-clock (381s of 533s) spent on duplicate dispatches — 169 dispatches across 15 unique event IDs. P1 — persist processedEventIds + failedEventCooldowns per-wallet - Two-tier dedup: in-flight set (in-memory, concurrent-arrival dedup) + processed set (persisted, cross-process dedup). - markEventProcessed moves the persistent add from line 1564 (pre-dispatch) to the success branches AFTER cursor advance. Preserves at-least-once for failed TOKEN_TRANSFER events that weren't durable. - FIFO cap at LIMITS.PROCESSED_EVENT_IDS_CAP (10k) with single- victim eviction per insert. - Debounced write at LIMITS.PROCESSED_EVENT_IDS_FLUSH_MS (200ms), serialized via persistDedupInFlight to avoid races. - failedEventCooldowns now persists too, so the DURABILITY_MAX_REPLAY_ATTEMPTS=3 budget accumulates across process restarts rather than resetting per-process. - Hydrated in subscribeToEvents AND fetchPendingEvents before EOSE burst so the first CLI command of a new process gets the benefit. Disconnect flushes any pending write. - setIdentity clears both sets, cancels armed timer, marks dedupHydrated=false so next connect repopulates from new pubkey's storage namespace. P2 — hoist V6/V5 bundle dedup in handleIncomingTransfer - Pre-check processedCombinedTransferIds.has(transferId) BEFORE calling processCombinedTransferBundle. Skip the bundle parse + save calls AND the trailing awaitAllProvidersDurable() (which was costing 2-3s per duplicate dispatch). - Symmetric hoist for V5 INSTANT_SPLIT via processedSplitGroupIds. - Returns `true` so transport advances lastEventTs past the duplicate. - Inner dedup in processCombinedTransferBundle retained as defense-in-depth for direct callers. Tests: +9 unit tests covering hydration on connect, hydration of cooldowns with stale-entry drop, debounced coalescing, no-id short-circuit, and setIdentity timer cancellation. Existing 51 transport tests still pass. Full suite: 8279 tests pass. Expected §C soak speedup per OPTIMIZATION-FINDINGS.md: 533s → ~150s (3.5×) on steady-state runs. Refs #274.
…est gaps Follow-up to bb9dedf. First soak run revealed the dominant code path in production uses MultiAddressTransportMux, which had its own in-memory `processedEventIds` set (line 170) — unpersisted. Every fresh CLI invocation re-walked the relay backlog via the Mux's `handleEvent` and paid the legacy SDK-format path's 4-8s addToken probe per event. The outer-provider fix in bb9dedf was untouched by the Mux path. Mux additions (mirror NostrTransportProvider): - `STORAGE_KEYS_GLOBAL.MUX_PROCESSED_EVENT_IDS` (global key, no pubkey suffix — Mux is per-Sphere-instance, storage is per-wallet) - `hydrateProcessedDedup()` called from `updateSubscriptions()` before EOSE replay arrives - `schedulePersistDedup()` / `persistDedupNow()` / `doPersistDedup()` with same 200ms debounce + in-flight serialization pattern - `disconnect()` flushes pending write before tearing down - `clearProcessedEvents()` also cancels timer + resets hydrated flag - Unlike NostrTransportProvider, the Mux dispatch model unconditionally advances `lastEventTs` per-address, so no two-tier in-flight set needed; every add is a commitment to advance. - FIFO cap unified at `LIMITS.PROCESSED_EVENT_IDS_CAP` (was a local static 10_000; same value, now sourced from one place). NostrTransportProvider fix: - `onTokenTransfer` drain (Issue #247 buffered transfers): the pre-Mux buffer drain now calls `markEventProcessed(transfer.id)` after the handler succeeds. Without this, the events that arrive in the pre-Mux window were never persisted in the dedup set, so every fresh CLI invocation re-walked them (the empty `transferHandlers` set short-circuits `handleTokenTransfer` with `false`, which skips the success branch in `handleEvent`). Test gaps closed (Review 3 GAP 1/2/3/4/5): - +6 new tests in `PaymentsModule.dedup-hoist.test.ts` covering the V6 + V5 hoist short-circuit (both directions: hits, misses) - Stale-drop test now asserts on map state (was a no-op `expect(true).toBe(true)`) - Disconnect-flush test added - FIFO boundary test added (verifies `> cap` not `>= cap`) - Cooldown gate test rewritten to assert on map state rather than on ambiguous ts-write proxy 8287 unit tests pass. Refs #275.
…unused Steelman review of 5fa50cc flagged the comment on `MultiAddressTransportMux.clearProcessedEvents()` as misleading: it claimed callers from "address change or periodic cleanup," but a codebase-wide grep returns zero external callers. The Mux's dedup set is intentionally shared across all addresses (they share one relay event stream), so address add/remove must NOT clear it. Document the actual intent: kept as a public surface for future forced-reset scenarios; full-wipe goes through Sphere.clear() → storage.clear() which implicitly removes the persisted MUX_PROCESSED_EVENT_IDS key. No functional change. Refs #275.
…-flight dedup
`sphere wallet use <name>` wedged for 58 min at 330% CPU with 900+ open
file descriptors all pointing at TWO OrbitDB block files (the OpLog
HEAD entry block + the most-recent snapshot block). Pattern: a tight
read loop hitting the same CIDs hundreds of times in rapid succession.
Each `helia.blockstore.get(cid)` walked `BlockStorage → NetworkedStorage
→ IdentityBlockstore → FsBlockstore` and opened the file. Even though
the steady-state FsBlockstore.get correctly auto-closes the FD on
stream EOF, the close is event-loop scheduled — under a synchronous
storm of `get(cid)` the close handlers run AFTER the next batch of
opens, FDs accumulate well past safe limits, and the process spins.
Extract the inline Helia-v6 → OrbitDB-v3 drain shim from `orbitdb-adapter`
into a standalone `profile/helia-blockstore-shim.ts` and layer two
defenses on top:
1. 64-entry LRU keyed by canonical `cid.toString()`. 100 % of cached-CID
`get` calls return synchronously with the bytes — zero `fs.open`
syscalls, zero new FDs. Per-entry 1 MiB cap bounds the worst-case
cache footprint at 64 MiB (typical OrbitDB blocks are sub-KiB).
2. In-flight Promise dedup: a second `get(cid)` arriving before the
first resolves shares the same Promise. Caps the transient FD
footprint at ONE pending read per CID under any concurrent storm.
Cache invalidation:
- `put(cid, ...)` evicts (defensive; CIDs are content-addressed).
- `delete(cid, ...)` evicts (so a future `helia.gc()` cannot serve
stale bytes for a removed block).
- Negative results (NotFoundError / InvalidConfigurationError) are
NOT cached — a subsequent HTTP-broker fetch + put recovers the
block and the next get must observe it.
Tests:
- 18 unit tests covering drain semantics, miss-error swallowing,
LRU recency / eviction, in-flight dedup, put/delete invalidation,
and uninstall (`tests/unit/profile/helia-blockstore-shim.test.ts`).
- 1 integration test exercising the shim over a real `FsBlockstore`,
asserting that 1000 sequential reads of the same CID open ZERO
new FDs and 100 concurrent reads issue at most ONE underlying
open (`tests/integration/helia-blockstore-shim-fd.test.ts`,
Linux-only via /proc/self/fd).
All 7723 existing unit tests pass; typecheck clean.
Refs #234 (drain shim origin), #266 (InvalidConfigurationError
swallowing), #278 (this fix).
…uMax=0 Self-review pass surfaced two robustness gaps in the LRU shim: 1. `cidKey()` fallback to `String(cid)` would collapse any plain JS object to `[object Object]` — a single cache key shared across every distinct non-CID input. Tighten to reject the default `Object.prototype.toString` sentinel so colliding inputs bypass the cache instead of serving wrong bytes for each other. Real CIDs produce base32 / base58btc strings that never start with `[object `. 2. `lruMax: 0` worked by set-then-evict on every touch — wasteful churn. Add an early-return so cache is genuinely disabled when configured to zero. (No production caller passes 0; this only matters for tests / hypothetical "disable cache" overrides.) Plus two unit tests covering the new paths (21 total in the shim suite, all green; integration test suite unchanged at 2 / 2 green).
…leak fix(profile)(#278): bound helia.blockstore.get with LRU + in-flight dedup
… files Follow-up to PR #432 (which fixed the same flake in `nametag-normalization.test.ts`). Surfaced on PR #326's CI: the `wallet-clear.test.ts > destroy() shuts down tokenStorageProviders > should not throw if tokenStorage shutdown fails` test failed with `Wallet already exists. Use Sphere.load() or Sphere.clear() first.` at `Sphere.create()` — exact same flake class as commit 9bf3e90 and PR #432 fixed. Root cause (mirror of 9bf3e90 + #432): seven additional integration test files use shared `path.join(__dirname, '.test-X')` directories with cleanTestDir() in beforeEach/afterEach. Under parallel-worker load, the FS race between cleanTestDir() and the next test's `Sphere.init` → `Sphere.exists` lets a partially-saved wallet.json from the FileStorageProvider's proper-lockfile path slip through — exists() returns true when it shouldn't and Sphere.create throws. Fix mirrors 9bf3e90/#432: each beforeEach now generates a fresh per-test TEST_DIR under `os.tmpdir()` with `Date.now()` + random suffix. tmpfs gives no fsync / no cross-process lock contention, and the unique path guarantees zero FS interaction between tests in the same file. afterEach still runs cleanTestDir() to keep tmpdir from growing. Files fixed: - tests/integration/wallet-clear.test.ts - tests/integration/provider-disable-sync.test.ts - tests/integration/operator-escape-hatch-bootstrap.test.ts - tests/integration/history-sync.test.ts (custom DEVICE_A/B_DIR) - tests/integration/tracked-addresses.test.ts - tests/integration/market-module.test.ts (custom cleanupTestDir) - tests/integration/nametag-overwrite-guard.test.ts Verified: - Full integration suite: 486/486 pass, 9 skipped (no regression). - Typecheck clean.
…egration-flakes test(integration): per-test TEST_DIR for 7 remaining flake-prone test files
… surfaces nametag Two related changes that close the loop for self-hosted nametag minting: 1) aggregator-proxy serves the trust base. The compose file bind- mounts ./data/genesis:/etc/aggregator-config:ro into agg-proxy and the nginx config exposes https://<aggregator-domain>/.well-known/trust-base.json as a public alias. Wallets pointed at our aggregator can now download the matching trust base over HTTPS — required for the SDK's verification path to come up without skipVerification=true. The /health location also picks up the pass-through fix from PR #323 so the rich backend response (role, database, sharding) is visible externally — necessary to land both changes against integration/all-fixes together. 2) render-discovery.sh surfaces the nametag. The discovery doc at https://<faucet-domain>/.well-known/faucet.json was showing nametag: null even after a successful mint. The watcher now tails for the nametag_verified log line (emitted by js-faucet AFTER mint+resolve completes) and rewrites identity.json with the verified nametag. The registering_nametag line is NOT used as a signal because it fires BEFORE the mint commits and would surface unverified state. Verified live end-to-end on this host: - faucet logs: aggregator_override_active, nametag_verified - mongo: commitments=1, aggregator_records=1 - relay: NAMETAG_BINDING (kind 30078) event for xaleava landed - discovery: identity.nametag="xaleava" The companion js-faucet PR #3 (env override wiring) and sphere-sdk PR #324 (relay + run-faucet env passthrough) are both still required for the end-to-end flow.
…m-proxy feat(infra)(#321): proxy serves trust-base + faucet surfaces verified nametag
…to clear stale CLI locks
FILE_LOCK_STALE_MS (~920s) is calibrated for the worst-case browser
publishOnce hold time, but CLI processes spawn-publish-exit in seconds.
When a CLI crashes (SIGKILL, OOM, soak teardown) before releasing its
proper-lockfile marker, the next CLI invocation has to wait ~15 minutes
for proper-lockfile's mtime-based stale detection — but the 30s mutex
acquire timeout trips first, surfacing PUBLISH_BUSY even though no
process actually holds the lock.
Supplement mtime-staleness with a PID-liveness probe:
1. After acquiring the lock, write a sibling `<lockPath>.owner.json`
containing {pid, hostname, acquiredAt}.
2. Before each acquire attempt, if a lock dir exists, read the owner
metadata. If hostname matches local AND `process.kill(pid, 0)`
reports ESRCH, steal the lock (rmdir + unlink) and retry. The
subsequent proper-lockfile mkdir is atomic, so concurrent stealers
can't double-steal.
3. On release, remove the owner metadata before releasing the lock dir
to prevent a contender from mkdir-ing then reading our stale owner
entry.
Defensive: false-positive "dead" detection would cause data corruption
from concurrent writers. The probe treats as alive on any of:
- hostname mismatch (cross-host PID probing is meaningless)
- metadata file missing/unreadable/malformed (we don't know who holds it)
- PID == process.pid (impossible via the in-process Mutex layer, but
defended anyway — never false-positive self-steal)
- process.kill returns EPERM (process exists but owned by another user)
- any unexpected fs/probe error
Only ESRCH is treated as proven dead.
FILE_LOCK_STALE_MS is preserved as the safety-net fallback for the
cross-host case where PID probing is meaningless.
Tests: 7 new unit tests covering live PID held, dead PID stolen,
cross-host metadata, self-PID, missing metadata, malformed metadata,
clean acquire/release writes-and-removes owner metadata. All existing
mutex tests still pass (10/10).
Closes #336.
…id-probe fix(profile/pointer): PID-liveness probe to clear stale CLI file locks (#336)
…D-links `uxf/ipld.ts:elementToIpldBlock` now encodes `element.children` and `element.header.predecessor` as dag-cbor **Tag 42 CID-links** (CIDv1, dag-cbor codec, sha2-256). The hash canonical form and the IPLD canonical form remain a single bit-identical form, so `sha256(elementBytes) === cid.multihash.digest` continues to hold for every element block — the only change is that each child / predecessor bstr becomes a CID tag inside the canonical CBOR. Tag 42 framing restores the "client builds CAR, Kubo pins recursively, receiver exports recursively" mental model that PR #213 Option C had broken: - Publisher: a single POST to `/api/v0/dag/import?pin-roots=true`. Kubo's recursive walker follows every Tag 42 link and pins the whole DAG under one root pin. The #434 per-block `/pin/add` loop (`pinDirectBlocksToGateway`) is no longer needed and was never on main. - Receiver: a single POST to `/api/v0/dag/export?arg=<root>`. Kubo's recursive walker streams the whole DAG. The UXF-aware walker (`isUxfElement` / `walkUxfElement` / `contentHashBytesToCid`) and the receiver-side root-shape peek (the #434 fork) are gone; the generic `collectCidLinks` BFS is sufficient for every dag-cbor block in the bundle (UXF, envelope, manifest, lean snapshot). Net code change: -144 lines. No client-side awareness of UXF link encoding, no special-case fast-path peek. Wire format intentionally changes — testnet posture, no backward compatibility. Wallets re-mint or migrate by re-receiving tokens (the issue explicitly authorizes "lose all previous tokens"). PR #213's canonical-hashing invariant is preserved because Tag 42 CIDs are just a different CBOR framing of the same digest. Mechanical changes: * uxf/cid-utils.ts (new): shared `contentHashToCid` / `cidToContentHash` / `createSha256Digest` helpers. Exists to break a circular import between hash.ts (builds canonical form with CIDs) and ipld.ts (encodes that form). * uxf/hash.ts: `prepareChildrenForHashing` returns `Record<string, CID | CID[] | null>`. `computeElementHash` predecessor is `contentHashToCid(...)` instead of `hexToBytes(...)`. * uxf/ipld.ts: mirrors the hash canonical form. `buildCanonicalHeader` emits a CID for predecessor. `decodeIpldElement` predecessor + `decodeIpldChildren` expect a CID only (Option C `Uint8Array` is rejected at the parse boundary). `decodeChildBytes` deleted — no callers. * profile/ipfs-client.ts: `isUxfElement` / `walkUxfElement` / `walkUxfChildValue` / `contentHashBytesToCid` deleted. The `fetchCarFromIpfsLegacy` dag-cbor decode branch calls `collectCidLinks` unconditionally. Dead constants `MULTIHASH_SHA256` and `SHA256_DIGEST_BYTES` removed. Docstrings on `pinCarBlocksToIpfs` updated to reference #435. * Tests: - tests/unit/uxf/hash.test.ts — `prepareChildrenForHashing` now verifies CID instances + dag-cbor codec + sha2-256 multihash + 32-byte digest preservation. - tests/unit/uxf/ipld.test.ts — `children encoded as ...` flipped to Tag 42 CID-links. The legacy "Option C accepts Tag 42 too" backward-compat test is replaced by a test that verifies the receiver REJECTS Option-C-shaped Uint8Array children with `SERIALIZATION_ERROR`. - tests/unit/profile/fetchCarFromIpfs.test.ts, tests/integration/transfer/uxf-cid-blockwalk-223.test.ts, tests/unit/payments/transfer/bundle-acquirer.non-profile-error-223.test.ts — comments / test names updated to describe the generic Tag 42 walker (the test bodies pass unchanged because the end-to-end multi-block walk still works — only the rationale changed). * tests/fixtures/uxf-t2d-reference-snapshot — fixture regenerated via the documented `UXF_T2D_REFERENCE_SNAPSHOT_REGEN=1` seam. Bundle grew from 2317 → 2373 bytes (Tag 42 framing per child reference). `_marker` bumped v5 → v6, `EXPECTED_MARKER` in the regression test bumped in lockstep, and a new v6 history entry added to the README documenting the format change. Supersedes the #434 fix branch (`fix/issue-434-profile-recovery-zero-tokens`), which restored the per-block `/pin/add` loop as a workaround for exactly the issue this PR fixes at the encoding layer.
Five high-priority findings from the pre-merge code review (PR #436): 1. **Cross-realm `instanceof CID` → `CID.asCID()`** (uxf/ipld.ts:388, :522, :776, :902, :906). `instanceof` silently fails in bundled environments where `@ipld/dag-cbor` and `uxf/ipld.ts` resolve to different module realms (Webpack code-splitting, worker_threads, vm sandboxes). `CID.asCID()` is the cross-realm-safe predicate the multiformats library documents — and the same pattern `collectCidLinks` in `profile/ipfs-client.ts` already uses. 2. **`cidToContentHash` validates digest length === 32** (uxf/cid-utils.ts). Restores the fail-fast guard that the deleted `decodeChildBytes` helper had. A CID with `multihash.code === 0x12` but a 28-byte digest now throws SERIALIZATION_ERROR with a specific 'expected 32 bytes, got N' message instead of producing a malformed ContentHash that later trips the generic 'Invalid content hash' brand validator. 3. **`cidToContentHash` validates `cid.code === DAG_CBOR_CODE`** (uxf/cid-utils.ts). Without this, an adversary could place a raw-codec (0x55) CID with a valid sha2-256 digest in a manifest or as a child reference; the Audit #333 H2 binding check is not codec-aware, so the manifest tokenId could bind to a raw-bytes block instead of an element block. UXF CIDs are always dag-cbor; enforcing the codec at the parse boundary closes the gap. 4. **`buildCanonicalForm` unknown-typeId guard** (uxf/ipld.ts:706). Mirrors the guard in `hash.ts:computeElementHash`. Pre-fix, a future `UxfElementType` added without updating `ELEMENT_TYPE_IDS` would cause `computeElementHash` to throw `UNKNOWN_ELEMENT_TYPE` while `elementToIpldBlock` silently encoded `{type: undefined}` — breaking the bit-identical canonical-form invariant that #435 is designed to preserve. 5. **Explicit wire-shape test for `header[3]` predecessor** (tests/ unit/uxf/ipld.test.ts). The self-consistency `sha256(bytes) === cid.multihash.digest` test would still pass under a regression to `hexToBytes(predecessor)` because `computeElementHash` and `elementToIpldBlock` share `buildCanonicalHeader`. The new test inspects the dag-cbor-decoded wire bytes directly and asserts header[3] is a CID instance with dag-cbor + sha2-256 codecs and 32-byte digest. **Bonus**: predecessor decoder now emits a dedicated 'legacy PR-#213 Option C encoding' error when handed a `Uint8Array`, instead of the generic 'got object' — operator triage can distinguish a cutover- window legacy-format issue from genuinely corrupt bytes. Verification: - npm run typecheck — clean - npx vitest run tests/unit/ — 8387 passed | 5 skipped (468 files) - npx vitest run tests/regression/uxf-t2d-reference-snapshot.test.ts — fixture still byte-identical (CID.asCID returns the same CID instance, no encoding change) - npx eslint uxf/ipld.ts uxf/cid-utils.ts — 0 errors, 0 warnings
Add the SWAP analog of the existing transfer / accounting / recovery trios: - manual-test-swap-roundtrip.sh — soak script asserting the propose → accept → deposit → completed flow on real testnet. Scenario A is the happy-path 50 UCT for 5 ETH atomic swap with integer-only smallest-unit net-delta assertions on all four legs (alice -50 UCT +5 ETH, bob +50 UCT -5 ETH) plus a poison-pill scan across every step log. Asymmetric faucet (alice 100 UCT only, bob 100 ETH only) is deliberate — there's no fallback liquidity that could mask a UCT/ETH cross-talk bug. Scenario B exercises `sphere swap reject --reason` (acceptor declines, both sides observe cancelled, no balance change). Scenario C exercises pre-announce `sphere swap cancel` (proposer rescinds; deposits_returned is false because the local-only branch was taken — the JSON output's cleanest signal that no escrow round-trip happened). Soak is parametrized by `SCENARIO=A|AB|ABC` (default AB) and `ESCROW=` (default `@escrow-testnet`); shares the same `KEEP=1` / `SWAP_TEST_DIR=` / `SUFFIX=` env contract as the other soaks. - docs/DEMO-PLAYBOOK-SWAP-ROUNDTRIP.md — presenter-friendly companion walking the same flow live in front of an audience (~20 min for A+B, ~14 min for A alone). Sections mirror the soak with talk tracks at each step, an at-a-glance table, an exit-code contract reference for `swap wait`, a 9-row failure-mode table, and a command quick-reference + presenter cheat sheet. Both artifacts depend on sphere-cli's swap-reject/swap-cancel/swap-wait commands shipped under sphere-sdk#437 — the playbook §0 documents the dependency check.
Pre-merge code-review findings on the soak + playbook:
- Soak Scenarios B and C's assert_grep patterns target the human
renderer's `key : value` form (unquoted), but the calls passed
`--json`, which produces double-quoted JSON keys instead. The
patterns never matched and both scenarios would always assert FAIL.
Drop `--json` from the swap-reject + swap-cancel calls — the soak
only needs presence checks, and the human renderer is what the
patterns are written against.
- Reversed stderr redirect on 7 sync calls (`2>&1 > file` instead of
`> file 2>&1`): the former duplicates stderr to the terminal then
redirects stdout to the file, so sync errors silently disappear.
Fix all seven (the two log-capturing ones at §4 and §6 plus the
five `>/dev/null` sites in Scenarios B and C).
- Add §2.5 escrow liveness pre-flight (`sphere swap ping $ESCROW`)
so an unreachable escrow surfaces as a clear "escrow not online"
message rather than the misleading "couldn't extract swap_id"
cascade at §3. Mirrors the pattern in
manual-test-accounting-roundtrip.sh.
- Add a load-bearing comment in §7 documenting the pipefail +
subshell + tee exit-code contract — easy to break under future
edits if the reader doesn't know why it works.
- Playbook §4 and §9 polling loops used `${SWAP:0:16}` against
`sphere swap list` output, but the list table only renders the
first 8 hex chars. The pattern would never match and presenters
following the playbook would loop the full 60s before the
proposal "appears". Same fix the soak already had.
Soak `bash -n` passes; 127/127 unit tests still green.
…trip-soak feat(uxf): swap-roundtrip soak + demo playbook (#437)
…ers register (#443) * fix(transport)(sphere-sdk#442): gate mux relay sub until module handlers register `ensureTransportMux()` opened the WebSocket and (via `connect()` / `addAddress()`) immediately called `updateSubscriptions()` BEFORE non-critical modules registered their DM handlers in `Promise.allSettled`. Any DM the relay replayed between subscription open and the late `swap.load()` / `accounting.load()` calls landed in CommunicationsModule's inbox (its own `onMessage` registered early via the address adapter's #223 pending-queue) but never reached the late fan-out subscribers — so a `swap_proposal:` DM showed up in `sphere dm history` while `sphere swap list` returned "No swaps found". Mirror the #423 fix for the mux path: - MultiAddressTransportMux: new `suppressSubscriptions()` / `armSubscriptions()` / `isSubscriptionsArmed()` API. When suppressed, `updateSubscriptions()` short-circuits BEFORE the stale-ID unsubscribe (regression-guarded by the new "no event blackout" test) so the multi-address-switch path keeps delivering events through the gate window. Default-armed for backward compat with direct-construction consumers (tests, custom hosts). - AddressTransportAdapter: delegate `suppressSubscriptions` / `armSubscriptions` / `isSubscriptionsArmed` to the mux for API parity with NostrTransportProvider (#423). - Sphere.ensureTransportMux: call `mux.suppressSubscriptions()` BEFORE `mux.connect()` so the initial addAddress auto-update no-ops. - Sphere.initializeModules: call `mux.armSubscriptions()` at the end, alongside the existing #423 outer-transport arm. Without this the wallet would never open a relay sub at all — total event blackout worse than the original race. - Sphere.initializeAddressModules: suppress before addAddress and arm after `Promise.allSettled`, covering the same race for non-primary addresses. Test surface: `tests/unit/transport/MultiAddressTransportMux.subscribeGate442.test.ts` pins eight invariants — default-armed backward compat, suppressed no-relay-traffic, arm opens with all tracked pubkeys, always-rebuild on re-arm for multi-address switch, suppress is not a tear-down, suppressed updateSubscriptions stays before the unsubscribe (no event blackout guard), pre-connect arming defers to connect time, and adapter delegates to mux. All 9013 unit tests still pass. * docs(transport)(sphere-sdk#442): flag suppress-window self-heal in scheduleResubscribe and onReconnected Steelman review on PR #443 caught that scheduleResubscribe (after a relay-initiated CLOSED frame) and the onReconnected callback both call updateSubscriptions unconditionally. If either fires inside the bootstrap suppression window, the rebuild short-circuits at the gate and no relay sub gets re-established from that callback. The explicit armSubscriptions at the end of Sphere.initializeModules always rebuilds, so any dropped rebuild self-heals — but the behavior is non-obvious and worth a comment for the next reader. No runtime change.
…etSwapStatus (#446) * fix(swap): lazy-load terminal swaps from storage in getSwapStatus `loadFromStorage` deliberately parks terminal swap entries in `_storedTerminalEntries` (the index) instead of `this.swaps` (the working set) to bound memory across the `terminalPurgeTtlMs` window — a 7-day default that can accumulate hundreds of records on a heavy wallet. `getSwapStatus` only checked `this.swaps` though, so any post-CLI-restart `sphere swap status $id` against a `completed` swap threw `SWAP_NOT_FOUND` even though the record was sitting on disk one async hop away. That bug surfaced on the manual swap-roundtrip soak's section-9 verification (`sphere swap status` after the swap reached `completed`): every soak run failed the terminal-state check despite the swap actually completing cleanly. The matching #442 fix unblocked sections 1-8 of that soak; this completes the round trip. Fix: - `loadTerminalSwapFromStorage(swapId)` — new private helper that reads the per-swap storage key only when `swapId` is in `terminalSwapIds`. Returns null on cache miss, malformed JSON, or missing `swap` field — every failure path collapses to the same `SWAP_NOT_FOUND` surface the existing caller already expects, so a corrupted record cannot poison bootstrap or coerce the lookup into reading arbitrary keys. - `getSwapStatus` — falls through to the lazy load on `this.swaps.get` miss. The loaded record is NOT inserted into `this.swaps`: the memory bound that `loadFromStorage` establishes has to survive arbitrary status queries against the terminal working set. Subsequent queries re-read from storage; that is the trade-off, and it matches the original design intent. Tests (`SwapModule.status.test.ts`): four new invariants pin the contract — lazy-load happens on cache miss and returns the persisted ref without inserting into `this.swaps` (UT-009), terminalSwapIds is the gate so arbitrary `swap:` keys are not readable through `getSwapStatus` (UT-010), corrupted JSON degrades to SWAP_NOT_FOUND without throwing (UT-011), and the in-memory entry wins when both exist so the storage path does not fire (UT-012). All 9017 unit tests still pass. * fix(swap)(sphere-sdk#445): refuse queryEscrow for lazy-loaded terminal swaps Steelman review on PR #446 caught a real footgun: getSwapStatus's fire-and-forget escrow status DM is dispatched even for terminal swaps that were just lazy-loaded from storage, but the downstream status_result DM handler resolves the swap via this.swaps.get — it has no path for lazy-loading from storage — so the escrow response is silently dropped on arrival. Net result: the DM fires (network noise, escrow load), no caller-visible effect. Refuse it explicitly: track whether the swap came from the lazy-load path, and short-circuit shouldQuery for those. Emit a debug log when the caller explicitly passed queryEscrow=true so misuse is at least diagnosable. Also tighten the loadTerminalSwapFromStorage JSDoc — the gate semantic 'gate true, no record' is a normal, expected case for ids that loadFromStorage decided to purge, not the 'truly unknown' the prior wording implied. Behavior unchanged; documentation matches reality. New test UT-SWAP-STATUS-013 pins the queryEscrow-refusal contract.
… runs (#449) The old `grep -c | grep -v ':0$' | wc -l` pipeline tripped `set -euo pipefail` on every successful soak run. When all log files are clean (the success case), every line out of `grep -c` is `file:0`; the inner `grep -v ':0$'` filters them all out and exits 1 (no matches); pipefail propagates; the command substitution inherits exit 1 — and `set -e` then aborts the script BEFORE the "ASSERT OK (poison-pill-clean)" branch ever runs. Net effect: every green run ended at the Section 10 banner with EXITCODE=1 and no assertion line printed, masquerading as a real poison-pill hit when nothing actually went wrong. Replace the count-parsing pipeline with `grep -l` (list filenames with matches) wrapped in `|| true` so emptiness is the no-op case the assertion expects. Local check: empty input correctly prints ASSERT OK, seeded input correctly prints ASSERT FAIL with the filename. Surfaced during the swap-roundtrip soak verification of sphere-sdk PR #443 + #446.
…452) * fix(profile)(sphere-sdk#450): bound pre-shutdown publish-retry spin The §D.1 soak hang in #450 was a 6+ hour, ~150% CPU spin in `LifecycleManager.awaitPendingPublishCleared`. The retry loop had no detection for "we keep failing identically against the same pending CID," so it burned every available iteration of the shutdown deadline trying a publish that was deterministically failing under contended- testnet conditions. Two changes: 1. Diagnostic — `profile/aggregator-pointer/discover-algorithm.ts`: distinguish "deadline already past at start" from "deadline expired during discovery" in the `RETRY_EXHAUSTED` message. The original "after 0ms" wording hid the load-bearing fact that `reconcile-algorithm.ts`'s shared 5-min wall-clock budget can be exhausted by the initial discovery, leaving conflict rediscovery with a negative budget. Operators chasing the loop now see whether the deadline was real or already-spent at entry. 2. Loop break — `profile/profile-token-storage/lifecycle-manager.ts`: `awaitPendingPublishCleared` now tracks consecutive `(cid, code)` failures across iterations and bails after `STUCK_PENDING_PUBLISH_THRESHOLD = 3` identical fires, emitting a new `storage:pending-publish-stuck` event with `{ cid, consecutiveFailures, lastError, elapsedMs, reason }`. The `pendingPublishCid` marker is preserved across the bail so the next cold start retries via the existing recovery path; the companion `shutdown:verification-timeout` event still fires so dashboards routing on the existing leg signal continue to work. The threshold of 3 keeps the loop runtime bounded at roughly `3 × per-attempt cost` instead of the full verification deadline — under contended-testnet load that is ~15 minutes worst case (vs. 6+ hours observed) and on a healthy testnet a single transient blip still gets two free retries before triggering the stuck signal. Tests: - `tests/unit/profile/pointer/discover-algorithm.test.ts` — two new cases assert each branch of the discovery deadline message. - `tests/unit/profile/lifecycle-manager-pending-publish-stuck-450.test.ts` — new file. Asserts the stuck event fires with the right payload after exactly 3 stable failures, the bail short-circuits well before the verification deadline, the marker is preserved, and a rotating failure code keeps the counter reset (no false-positive fire for genuinely changing transients). All 9022 unit tests pass; `npm run typecheck` clean; no new lint findings on touched files. * fix(profile)(sphere-sdk#450): prefer typed code over message in stuck-detection catch arm Steelman follow-up: the catch arm in awaitPendingPublishCleared previously used err.message as part of the stuck-detection failureKey. publishAggregatorPointerBestEffort converts every internal throw to a structured result before returning, so the catch arm is currently dead code — but if a future regression let a typed AggregatorPointerError escape with a time-varying message (e.g. the new "Discovery exceeded wall-clock deadline after Nms (budget=Bms)" wording from the sibling fix), the failureKey would change every loop and the counter would never reach the threshold. Prefer err.code when available so the key stays stable.
…phases (#453) The pre-#444 at-least-once Nostr cursor gate awaited the FULL flush per TOKEN_TRANSFER receive — including the aggregator pointer publish + IPFS HEAD-verify. When the cross-device leg blipped (publish transient, gateway propagation lag), handleIncomingTransfer returned false, the Nostr transport refused to advance lastEventTs, and the per-event cooldown ledger armed a 30s+ exponential backoff. Short-lived CLI processes exited before the cooldown could elapse; the relay aged out the TOKEN_TRANSFER event before the next CLI run; the receiver's wallet showed no balance despite local OrbitDB+Helia state already being durable. This splits flushToIpfs into local-commit and remote-publish phases. Per-receive: bundle CAR is pinned to local Helia + OrbitDB bundle ref is written synchronously. Cross-device publish is deferred and batched via the dirty-flush debouncer — multiple TOKEN_TRANSFER receives in the debounce window coalesce into ONE aggregator pointer update. Changes: - flushToIpfs(options?: { skipPublish?: boolean }): when skipPublish, skip publishSnapshotIfWired + verification leg; call notifyProfileDirty to schedule a deferred snapshot publish. - forceFlushSerializedLocal(): new public FlushScheduler method. - awaitNextLocalFlush(timeoutMs?): new public method on Profile provider + new optional method on TokenStorageProvider interface. - PaymentsModule.awaitAllProvidersDurable: prefers awaitNextLocalFlush (falls back to awaitNextFlush for providers without a local variant). - ProfileTokenStorageProvider.shutdown: drainDeferredDirtyPublishOnShutdown fires the deferred publish ONCE if a signal is pending (armed timer or dirtyFlushPending latch), no-op on idle wallets — covers graceful CLI exit before the debounce window. Local-loss failures (OrbitDB write throws, CAR pin fails) still pin the Nostr cursor for at-least-once replay, preserving the safety invariant of issue #105.
…r nametag resolution failure (#458) The @escrow-testnet nametag binding is not currently published on the testnet relay, so swap commands using --escrow @escrow-testnet fail with "Could not resolve recipient" until the operator republishes the binding event. Rather than mutate the canonical default (the nametag remains the documented reference), this change: - Adds a "Troubleshooting: escrow nametag resolution" subsection to the swap demo playbook that surfaces the production testnet escrow's raw DIRECT address as the fallback override, with the canonical fix (operator re-publishes the binding) called out for escrow ops. - Cross-links the §11 failure-table entry for nametag resolution failures to the new troubleshooting subsection. - Extends the §0 prerequisites and §0 Workspace ESCROW shell-var comment to mention the fallback override without changing the default value. - Adds a parallel "Troubleshooting: escrow address" subsection to QUICKSTART-CLI.md §11 (swap), with a DIRECT-form swap-propose example. - Documents the same override in the manual-test-swap-roundtrip.sh script header, and emits a targeted hint from the §2.5 escrow ping pre-flight when ESCROW=@escrow-testnet fails, pointing the operator at the DIRECT fallback re-run command. No code paths touched. The DIRECT address used as fallback is the production testnet escrow service's actual address — only the nametag binding is missing.
…ess (#462) Captures the root-cause analysis for sphere-sdk#455 after sphere-cli PR #45 shipped the operational workaround (local mint instead of HTTP faucet). Verdicts on the four hypotheses in the issue body: - H1 faucet HTTP API race — REFUTED. Faucet blocks on sendTokenTransfer.join() before returning HTTP 200 (FaucetService.java:265). Same code path for bulk and single-coin requests. - H2 payload encoding — REFUTED. Faucet emits the Sphere-wallet {sourceToken, transferTx} shape per-request regardless of bulk/single. - H3 outer empty-handler buffer race — CONFIRMED as source of the '[AT-LEAST-ONCE] not durable' warn line, but NOT the cause of token loss. Sphere.fetchPendingEvents calls outer NostrTransportProvider's fetch even when MUX is suppressed; outer's handler set is always empty in MUX mode so events buffer + the cooldown ledger is armed. - H4 relay retention — REFUTED. fetchPendingEvents uses 3-day lookback and the issue's own log line shows the event arrived at the receiver. H3-Extension (new finding) — the dominant root cause of the missing-token symptom: MuxAdapter.dispatchTokenTransfer (MultiAddressTransportMux.ts:2286) calls async handlers WITHOUT await. Handler Promises are fire-and-forget. payments.receive()'s 'await fetchPendingEvents()' resolves while PaymentsModule.handleIncomingTransfer is still running; the subsequent load() reads storage before addToken has written. Bulk fan-out (7 parallel events) statistically masks the race; single-coin is a binary coin flip. Doc includes a sketched fix (await + propagate durability through mux dispatch) plus suppression of outer's fetchPendingEvents under MUX as a secondary cleanup. Investigation-only; no SDK code changed.
… during module load (#460) PRs #442/#443 introduced a `suppressSubscriptions`/`armSubscriptions` gate on `MultiAddressTransportMux` so the relay subscription stays closed during `Sphere.initializeModules` until every non-critical module has finished registering its DM/transfer/payment-request handlers. The existing 8 unit tests in `MultiAddressTransportMux.subscribeGate442.test.ts` pin the gate semantics at the mux level but never observe gate state from INSIDE module load — a future PR that accidentally calls `armSubscriptions` inline during `ensureTransportMux`, or moves the post-`allSettled` arm above the await, passes every existing test. This pin patches `CommunicationsModule.prototype.load` to record `isSubscriptionsArmed()` at three points (entry, after a microtask hop, exit) via the adapter's delegate. All three must be `false`; after `Sphere.init` resolves, the gate must be `true`. A second variant flips the spied `load()` into a thrower to confirm the `Promise.allSettled` swallow path still arms the gate exactly once. The `isSubscriptionsArmed()` accessor already existed on both `MultiAddressTransportMux` and `AddressTransportAdapter` (added with #442) — no production code changes required.
… hazard from PR #453 review (#463) Issue #454 ranks 13 review findings from the post-merge code review of PR #453 (which addressed #444). This change lands the 4 HIGH findings plus finding #9 (correctness gap). The remaining MEDIUM/LOW/CLEANUP findings stay as a follow-up tracker on the issue. Finding #1 — Drain race in drainDeferredDirtyPublishOnShutdown. The shutdown drain called `onProfileDirtyFlush` directly, leaving `dirtyFlushPromise` null during the await. A concurrent `notifyProfileDirty()` arriving in that window armed a fresh `dirtyFlushTimer` which the post-drain `cancelDirtyFlushTimer()` then silently cancelled — the exact bug-class PR #453 claimed to fix. Fix routes the drain through `publishSnapshotIfWired()` which tracks `dirtyFlushPromise` so concurrent signals latch into `dirtyFlushPending`, and loops (≤4 iterations) so a latched signal that re-arms the timer in `publishSnapshotIfWired`'s finally is captured and re-fired rather than dropped. Finding #2 — SIGKILL recovery marker on skipPublish path. The Issue #444 `skipPublish` branch deliberately does NOT stamp `pendingPublishCid` (that field stores SNAPSHOT CIDs; local-only flushes only pin a BUNDLE CID). A SIGKILL during the debounce window therefore left no persistent retry marker — `retryPendingPublishIfAny` was a no-op on next boot. Fix adds a sibling persistent boolean marker `pendingDeferredPublishMarker` (storage key `PROFILE_PENDING_DEFERRED_PUBLISH`), stamped in the skipPublish branch and restored on initialize() to trigger a deferred best-effort `publishSnapshotIfWired()` recovery. Cleared on a successful publish (debounce-fire, shutdown drain, or any full save-side flush). Finding #3 — Doc/code contradiction in awaitNextLocalFlush. The JSDoc claimed `pendingPublishCid` is set for next-tick retry; the implementation explicitly doesn't. Updated the JSDoc to accurately describe the new deferred-publish flow (notifyProfileDirty for the in-process debouncer + pendingDeferredPublishMarker for SIGKILL recovery) plus the finding #4 HEAD-verify suppression. Finding #4 — Background HEAD-verify still runs on skipPublish. `startBackgroundDurabilityVerify` fired even when `skipPublish=true`, reintroducing exactly the per-receive propagation-coupling Issue #444 set out to break. Fix adds `!options?.skipPublish` to the `shouldVerify` predicate so the verify leg runs only on full (publish-included) flushes. The deferred-publish dispatch path runs its own verify round-trip via the publisher's `verifyFlushDurability`. Finding #9 — Structural cast in PaymentsModule.awaitAllProvidersDurable. The `(provider as { awaitNextLocalFlush?: ... }).awaitNextLocalFlush` cast let any provider exposing the method NAME silently win — even a no-op stub would have silently advanced the Nostr cursor. Fix uses the typed optional declarations on the TokenStorageProvider interface: `provider.awaitNextLocalFlush ?? provider.awaitNextFlush`. Misshaped providers now fail at compile time rather than runtime. Tests: - New file tests/unit/profile/profile-token-storage-454-followups.test.ts covers drain race (#1), SIGKILL marker + reboot recovery (#2), HEAD-verify gating on skipPublish (#4), and the structural-cast fix path (#9). - All 9042 unit tests pass (560 test files). Not addressed in this PR (tracked as follow-ups on #454): - MEDIUM findings #5-#8, #10 - LOW findings #11-#14 - CLEANUP findings #15-#16
…handler completion (#465) `AddressTransportAdapter.dispatchTokenTransfer` (and siblings) iterated over registered handlers synchronously, called `handler(transfer)` and discarded the returned Promise. `PaymentsModule.handleIncomingTransfer` is async, so the dispatch returned (and `MultiAddressTransportMux. handleTokenTransfer`'s caller chain — ultimately `fetchPendingEvents`) resolved BEFORE the handler finished writing the token to storage. The next caller step (`PaymentsModule.load()` inside `payments.receive()`) then raced against the in-flight `addToken` write and roughly half the time observed empty storage. This was the real root cause behind the single-coin faucet flakiness investigated in #455. Bulk faucet (#391) masked the bug: 7 events serialize through OrbitDB's write lock and the per-event microtasks usually all finish before the follow-up `load()` reads. The race is masked there, not fixed. Fix (Option A — minimal-surface async dispatch contract): - Every `dispatch*` method on `AddressTransportAdapter` is now `async` and `await Promise.allSettled(...)` over each handler invocation. `Promise.allSettled` preserves per-handler fault isolation — a rejection in one handler does not block the others (do NOT short-circuit to `Promise.all`). - Upstream callers in `MultiAddressTransportMux` (`handleTokenTransfer`, `handlePaymentRequest`, `handlePaymentRequestResponse`, gift-wrap routing) now `await` the dispatch. The chain `fetchPendingEvents → handleEvent → dispatchWalletEvent → handleTokenTransfer → dispatchTokenTransfer → handler` is fully awaited end-to-end. Sweep — sibling dispatch sites in the same file all converted to async for contract uniformity: - `dispatchMessage` (line 2276 → 2306) - `dispatchTokenTransfer` (the #464 site) - `dispatchPaymentRequest` - `dispatchPaymentRequestResponse` - `dispatchReadReceipt` - `dispatchTypingIndicator` - `dispatchComposingIndicator` - `dispatchInstantSplitBundle` Late-drain hardening: `onTokenTransfer(handler)` (and siblings) used to drain `pendingTransfers` by calling `handler(transfer)` synchronously, same shape as the dispatch bug. Drains are now tracked as Promises in `inFlightDrains` and exposed via `flushPendingDrains()`. The mux's `fetchPendingEvents` calls `flushPendingDrains()` on every adapter before returning, so a late-registered handler's buffered drain is fully observed by the caller's next state read. Why Option A over Option B (explicit drain): the bulk path's parallelism comes from `fetchPendingEvents`'s upstream `Promise.all` over multiple events, not from per-dispatch parallelism. Option A keeps the contract uniform with no caller-side bookkeeping. If a future soak shows meaningful bulk-path regression, switching to Option B is a non-breaking refinement. Tests: - New: `tests/unit/transport/MultiAddressTransportMux.dispatch-await.test.ts` (8 tests) — proves dispatch does NOT resolve until the async handler resolves (gated via controllable Promise). Covers `dispatchTokenTransfer`, `dispatchMessage`, `dispatchPaymentRequest`, `dispatchPaymentRequestResponse`, late-drain flush, and async- rejection isolation. - Updated: `tests/unit/transport/MultiAddressTransportMux.pending-queue-223.test.ts` converted to async — all 7 tests await dispatch and `flushPendingDrains()` where they previously relied on sync drain. - Updated: `tests/e2e/uxf-bundle-cross-instance-import-223.test.ts` `injectTokenTransfer` is now async, call site awaits. Test results: tests/unit (8418 passed, 5 skipped), tests/unit/transport (196 passed), tests/unit/modules/PaymentsModule (479 passed), tests/unit/core (650 passed). Build (tsup) and lint clean (zero new errors or warnings). Closes #464.
…ey missing (#459) Three sites in `modules/swap/SwapModule.ts` used `peer.transportPubkey ?? peer.chainPubkey` to address swap DMs. When a resolved binding was partially propagated (transportPubkey not yet published / observed), the fallback sealed the NIP-17 DM to the chain pubkey — which the receiver's wallet does NOT subscribe to. The proposer saw `status: proposed`; the acceptor saw nothing. No error, no event, no warning. Demo session 2026-06-09 reproduced this live: first `sphere swap propose --to bob-demo06` silently dropped; a retry 5 min later landed normally after binding propagation completed. Convert all three sites to fail-fast via a new `requireTransportPubkey` helper that throws a typed `SWAP_PEER_NO_TRANSPORT` SphereError with actionable remediation text. Sites: 1. `proposeSwap` counterparty resolve (line ~1133) — reject the call. 2. `proposeSwap` escrow peer resolve (line ~1190) — reject the call. 3. `getSwapStatus` status-DM send (line ~2260) — fire-and-forget, so the throw is caught by the existing `.catch` and logged as a warning. Dramatically better than a silent black-hole — operators see the actionable warning in their logs. The thrown error's message text identifies the peer (nametag preferred, DIRECT address as fallback), explains the binding propagation cause, and recommends retry after the recipient's binding finishes publishing. Closes #457.
…tSwaps and write-side error codes (#461) PR #446 (closing #445) added lazy-load for terminal swaps in getSwapStatus only. Three other public surfaces still treated terminal swaps as unknown — they iterated only `this.swaps`, which loadFromStorage deliberately keeps free of terminal entries to bound the in-memory working set across the terminalPurgeTtlMs window. This commit completes the fix: 1. resolveSwapId(prefix) now scans `_storedTerminalEntries` in addition to `this.swaps`. A `sphere swap status <prefix>` against a terminal swap no longer dies at the prefix-resolver before getSwapStatus' lazy-load can run. 2. getSwaps(filter) gains an `includeTerminal: boolean` option (default false — preserves existing CLI list behavior). When true, terminal index entries are materialized as stub SwapRefs (id/role/progress/createdAt only — full record is one getSwapStatus call away). 3. Write-side methods (acceptSwap, cancelSwap, deposit, rejectSwap, verifyPayout) now route through a new private `requireSwap()` helper. For terminal swaps that lazy-load from storage, it throws the new `SWAP_ALREADY_TERMINAL` error code instead of the misleading `SWAP_NOT_FOUND`. Live swaps still in `this.swaps` flow through unchanged — the legacy fine-grained `SWAP_ALREADY_COMPLETED` / `SWAP_ALREADY_CANCELLED` / `SWAP_WRONG_STATE` codes are preserved for the in-memory case. `resolveSwapId` also gains the new `SWAP_AMBIGUOUS_PREFIX` code (previously the same `SWAP_NOT_FOUND` was thrown for both no-match and ambiguous-match — callers had no way to give the user a "use more characters" hint). 27 new tests in tests/unit/modules/SwapModule.terminal-blindness.test.ts pin the contract across all three surfaces. Backward compatibility: - `includeTerminal` is additive and defaults to false — no behavioral change for existing getSwaps callers. - Write-side methods now throw `SWAP_ALREADY_TERMINAL` instead of `SWAP_NOT_FOUND` for the terminal-swap case. Callers that catch `SWAP_NOT_FOUND` to detect "cannot mutate" should also catch `SWAP_ALREADY_TERMINAL`.
…@escrow-test-02 (#468) * feat(swap)(sphere-sdk#456): hardcode default escrow address to @escrow-testnet-v1 The previous default @escrow-testnet was never published by the production escrow daemon (root cause analysis in #456 comment thread). Rather than try to recover an unrecoverable nametag binding (operator has no path to publish to that name without rotating the wallet identity), rotate the canonical default to a fresh versioned nametag. Changes - constants.ts: new exported DEFAULT_ESCROW_ADDRESS = '@escrow-testnet-v1'. Versioned suffix so future operator rotations can move to -v2/-v3 without breaking older SDK builds. - core/Sphere.ts: resolveSwapConfig now defaults SwapModuleConfig. defaultEscrowAddress to DEFAULT_ESCROW_ADDRESS when caller did not set one. Wallet initialised with swap: true (no explicit escrow override) can now propose / accept swaps against the canonical escrow nametag without any per-call wiring. Operator deployment work follows in escrow-service: republish the wallet's nametag binding for 'escrow-testnet-v1' against the production escrow's existing transport key. * feat(swap)(sphere-sdk#456): export DEFAULT_ESCROW_ADDRESS for consumers * feat(swap)(sphere-sdk#456): rotate DEFAULT_ESCROW_ADDRESS to @escrow-test-01 @escrow-testnet-v1 attempt landed on the production tenant's secondary HD address via custom multi-address routing in escrow-service, but the routing had subtle relay-subscription gaps (NIP-17 chat sub for the secondary transport pubkey did not stay armed reliably after switch-back). Going with a simple identity rotation instead: the escrow tenant's wallet data is wiped and reinitialised with SPHERE_NAMETAG=escrow-test-01 so the nametag is the tenant's sole primary identity. No cross-address routing needed; sphere.resolve('@escrow-test-01') hits a single transport pubkey that the escrow's only CommunicationsModule subscribes to. * feat(swap)(sphere-sdk#456): rotate DEFAULT_ESCROW_ADDRESS to @escrow-test-02 (escrow-test-01 squatted) * docs(swap): update DEFAULT_ESCROW_ADDRESS JSDoc to reflect final value (-02)
…h resolves
The Mux's chat-side `since` cursor (`lastDmEventTs`, per
`STORAGE_KEYS_GLOBAL.LAST_DM_EVENT_TS_*`) was advanced to wall-clock-now
the moment `NIP17.unwrap` succeeded — BEFORE the async handler chain
(`AddressTransportAdapter.dispatchMessage` and its siblings, which in
turn drive `SwapModule.handleIncomingDM`, `AccountingModule`, etc.)
had a chance to run to completion. If the host CLI process exited
between the unwrap and dispatch-completion (the soak's 3-second-poll
loop is exactly this shape), the cursor was strictly past the event's
effective visibility window while the swap / message remained
unpersisted. On the next CLI boot the relay's `since`-filtered
subscription excluded the event from its reply, and the event was
permanently invisible to the receiver without an out-of-band republish.
Fix:
1. `MultiAddressTransportMux.routeGiftWrap` (`transport/
MultiAddressTransportMux.ts:1170-1346`) restructured to:
- Extract `NIP17.unwrap` into its own try/catch that `continue`s to
the next address on decryption failure (preserving the old
"wrong-recipient" path).
- Wrap the entire per-address dispatch logic in a try/finally with a
local `dispatched` flag. Each successful per-event terminal branch
(self-wrap dispatch, read receipt, composing indicator, kind-14
legacy paths, chat message) sets `dispatched = true` immediately
before `return`. Returns from no-handler paths
(`!isChatMessage(pm)` after the unwrap succeeds with no matching
terminal branch) leave `dispatched` false on purpose — the
`processedEventIds` dedup (issue #275) absorbs re-delivery on the
next look-back-window boot.
- The `finally` block calls `updateLastDmEventTimestamp` only when
`dispatched` is true.
2. Chat subscription look-back buffer doubled:
- `MultiAddressTransportMux.ts:998`: `globalDmSince -
NIP17_TIMESTAMP_RANDOMIZATION` → `... - 2 *
NIP17_TIMESTAMP_RANDOMIZATION`.
- `NostrTransportProvider.ts:3152`: same change in the OG provider's
chat subscription path.
The single buffer only catches the publish-to-cursor-advance window;
the new finally-block keeps that gap tight, but the 2× buffer is
belt-and-braces against residual clock skew and relay propagation
lag. The persistent `processedEventIds` dedup (#275) absorbs the
extra backlog with no double-handling cost.
Behavior parity:
- A dispatch throw under the OLD code was swallowed by the outer
`try { ... } catch { continue; }` and the loop attempted to unwrap
with the next address (nonsensical — the event was only encrypted to
the one address whose keyManager succeeded). The NEW code lets the
throw propagate out of `routeGiftWrap`; `handleEvent` already wraps
the call in a try/catch (line 1143) that logs the failure with the
event id. Net behavior: identical user-visible outcome, slightly
more diagnostic logs on dispatch failure.
Test plan:
- [x] All four existing Mux test suites pass (39/39): `dispatch-await`,
`pending-queue-223`, `sharedClient`, `subscribeGate442`.
- [x] `npx tsc --noEmit` clean against the edited files.
- [ ] End-to-end validation: `manual-test-swap-roundtrip.sh` Scenario A
10 consecutive runs against testnet with zero `proposal-ingest-
timeout` retries (the soak loop currently used as a load-bearing
cross-process DM smoke test).
- [ ] Spot-check the trader-roundtrip soak (sphere-sdk#474) end-to-end
once the trader-agent image is rebuilt against current main; the
controller → tenant ACP DM hops exercise the same cross-process race
pattern.
A deterministic unit test for `routeGiftWrap`'s cursor-advance order
requires a stubbed `NIP17.unwrap` + a real Mux instantiation + a gated
async handler. The plumbing cost (Mux constructor depends on nostr-js
client, IndexedDB-style storage, keyManager fixtures) is heavy; the
existing #455 / #464 test infrastructure plus the 10-consecutive-soak
discipline is the canonical signal. A focused unit test is a
follow-up.
Root cause + ranking of the five candidate mechanisms (M1-M5) lives in
docs/uxf/ISSUE-473-INVESTIGATION.md on branch
feat/474-trader-roundtrip-soak.
References:
- Closes sphere-sdk#473
- Related: sphere-sdk#455, #464, #465 (in-process variant of the same
family; closed by PR #465's mux-dispatch await fix)
- Related: sphere-sdk#274/#275 (persistent dedup set the fix depends on)
- Sibling: sphere-sdk#474 (trader-roundtrip soak; hard-depended on this
fix for its controller→tenant DM hops; PR #475)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root cause for #473 (intermittent cross-process Nostr DM misses): the Mux's chat-side
sincecursor was advanced at NIP-17 unwrap time, BEFORE the async handler chain ran to completion. CLI exits during the soak's 3-second poll loop killed the handler mid-flight; the cursor advance persisted; the relay'ssince-filtered subscription then permanently excluded the event from later boots.This PR ships the smallest defensive fix (~80 LOC).
Changes
MultiAddressTransportMux.routeGiftWrap— restructured into a per-addressunwrap→ try/finally with adispatchedflag. The cursor advance only fires inside the finally, after a terminal dispatch branch setsdispatched = true. No-op paths (own-message skip, unknown kind after successful unwrap) intentionally leave the flag false; theprocessedEventIdsdedup (perf: cross-process Nostr dedup persistence (#274 follow-up — 71% of §C runtime is duplicate dispatch) #275) absorbs re-delivery on the next look-back window.Chat subscription look-back buffer doubled (Mux:998 + NostrTransportProvider:3152) —
globalDmSince - NIP17_TIMESTAMP_RANDOMIZATION→… - 2 × NIP17_TIMESTAMP_RANDOMIZATION. Belt-and-braces against residual clock skew + relay propagation lag, now that the primary cursor-advance gap is closed.Behavior parity
Under the OLD code, a dispatch throw was silently swallowed by
try { ... } catch { continue; }(which then tried unwrap with the NEXT address — nonsensical). Under the NEW code, the throw propagates out ofrouteGiftWrap;handleEventalready wraps the call in a try/catch (MultiAddressTransportMux.ts:1143) that logs the failure. Net behavior: identical user-visible outcome, slightly more diagnostic logging.Test plan
dispatch-await,pending-queue-223,sharedClient,subscribeGate442).npx tsc --noEmitclean against the edited files.manual-test-swap-roundtrip.shScenario A — 10 consecutive runs against testnet with zeroproposal-ingest-timeoutretries. Today's miss rate is ~5–15%; the acceptance bar is 0/10.A deterministic unit test for
routeGiftWrap's cursor-advance order requires stubbedNIP17.unwrap+ a real Mux instantiation + a gated async handler. The plumbing cost is heavy; the canonical signal is the soak running 10× without retries. A focused unit test is a small follow-up.Root-cause + ranking
Full investigation lives at
docs/uxf/ISSUE-473-INVESTIGATION.mdon the sibling branchfeat/474-trader-roundtrip-soak(PR #475). One-paragraph TL;DR:Mechanism rankings:
created_at) — BLOCKING. Fixed here.now) — UNLIKELY. Subscriptions usesince = lastDmTs - buffer, notsince = now.Related
processedEventIdsdedup that makes look-back-window re-delivery safe)