diff --git a/desktop/src/features/channels/readState/readStateManager.test.mjs b/desktop/src/features/channels/readState/readStateManager.test.mjs index 231d00f4d6..6e333be7c9 100644 --- a/desktop/src/features/channels/readState/readStateManager.test.mjs +++ b/desktop/src/features/channels/readState/readStateManager.test.mjs @@ -6427,25 +6427,27 @@ test("identitySwap_aDeferredMidTransaction_bHydrates_bSeesNoAState", async () => mgrB.destroy(); }); -// ── Test 60: MAX-1 boundary — two enqueues from valid one-below-ceiling hydration ── -test("hydrate_maxMinusOneState_twoEnqueues_safeGensNeitherThrows_ngPersistSafe", async () => { - // Thufir's exact repro: blob with pi.gen = MAX_SAFE_INTEGER-1 and ng = MAX_SAFE_INTEGER-1. - // Before the fix, hydration accepted this verbatim (requiredNextGen = MAX_SAFE_INTEGER, - // which isSafeInteger), set nextGen=MAX_SAFE_INTEGER via clamp, and the first enqueue - // minted gen=MAX_SAFE_INTEGER while leaving _nextGen=MAX_SAFE_INTEGER+1 (unsafe). - // The second enqueue then threw synchronously. +// ── Test 60: MAX-2 boundary — two enqueues from valid two-below-ceiling hydration ── +test("hydrate_maxMinusTwoState_twoEnqueues_safeGensNeitherThrows_ngPersistSafe", async () => { + // Paul's exact repro: blob with pi.gen = MAX_SAFE_INTEGER-2 and ng = MAX_SAFE_INTEGER-2. + // Before the headroom fix, the boundary was requiredNextGen >= MAX_SAFE_INTEGER (one notch): + // requiredNextGen = MAX-1 → safe, < MAX → no rebase; clamp sets nextGen = MAX-1. + // Click 1 mints gen=MAX-1, _nextGen=MAX (safe, persisted). + // Click 2 preflight: next=MAX+1 unsafe → THROWS synchronously up markChannelRead. // - // After the fix: requiredNextGen >= MAX_SAFE_INTEGER triggers rebase instead of clamp. + // After the headroom fix: rebase triggers when requiredNextGen >= MAX - GEN_REBASE_HEADROOM + // (i.e. within 2**32 of MAX_SAFE_INTEGER). MAX-1 is well within that window, so the + // rebase fires on the MAX-2 blob just as it did on the MAX-1 blob. // The single pending intent is rebased to gen=1, nextGen=2. Both enqueues get safe // distinct gens; neither throws; the persisted ng after each enqueue is safe. // - // Failure branch: rebase boundary (maxObservedGen = MAX_SAFE_INTEGER-1, requiredNextGen - // = MAX_SAFE_INTEGER triggers rebase rather than clamp). + // Failure branch: rebase boundary (maxObservedGen = MAX_SAFE_INTEGER-2, requiredNextGen + // = MAX-1 >= MAX - GEN_REBASE_HEADROOM → rebase rather than clamp). // Durable surface: v2 blob after each enqueue has safe ng; fresh-manager hydration // succeeds and sees both intents. globalThis.window.localStorage = makeLocalStorage(); const pubkey = "60".repeat(32); - const channelId = `max-minus-one-ch-${"\u006d".repeat(47)}`; + const channelId = `max-minus-two-ch-${"\u006d".repeat(47)}`; const v2Key = `buzz.nip-rs.override-state.v2:${pubkey}`; const ls = globalThis.window.localStorage; const MAX = Number.MAX_SAFE_INTEGER; @@ -6459,12 +6461,12 @@ test("hydrate_maxMinusOneState_twoEnqueues_safeGensNeitherThrows_ngPersistSafe", getConnectionGeneration: () => 0, }; - // Plant blob: one intent at gen=MAX_SAFE_INTEGER-1, ng=MAX_SAFE_INTEGER-1. + // Plant blob: one intent at gen=MAX_SAFE_INTEGER-2, ng=MAX_SAFE_INTEGER-2. ls.setItem( v2Key, JSON.stringify({ - pi: { [channelId]: { gen: MAX - 1, op: "unread" } }, - ng: MAX - 1, + pi: { [channelId]: { gen: MAX - 2, op: "unread" } }, + ng: MAX - 2, }), ); @@ -6472,7 +6474,7 @@ test("hydrate_maxMinusOneState_twoEnqueues_safeGensNeitherThrows_ngPersistSafe", mgr.hydrateFromLocalStorage(); // After hydration: rebase must have fired. - // maxObservedGen = MAX-1 → requiredNextGen = MAX → >= MAX_SAFE_INTEGER → rebase. + // maxObservedGen = MAX-2 → requiredNextGen = MAX-1 → >= MAX - 2**32 → rebase. // One intent rebased to gen=1; nextGen=2. const ngAfterHydrate = pendingOverrideIntentStore.nextGen; assert.ok( @@ -6485,7 +6487,7 @@ test("hydrate_maxMinusOneState_twoEnqueues_safeGensNeitherThrows_ngPersistSafe", ); assert.equal(ngAfterHydrate, 2, "nextGen must be 2 after rebase of 1 intent"); - const ch2 = `max-minus-one-ch2-${"n".repeat(46)}`; + const ch2 = `max-minus-two-ch2-${"n".repeat(46)}`; // First enqueue — must NOT throw, must produce safe gen. mgr.isLoadComplete = false; @@ -6516,7 +6518,7 @@ test("hydrate_maxMinusOneState_twoEnqueues_safeGensNeitherThrows_ngPersistSafe", `persisted ng must be below MAX_SAFE_INTEGER; got ${ng1}`, ); - const ch3 = `max-minus-one-ch3-${"o".repeat(46)}`; + const ch3 = `max-minus-two-ch3-${"o".repeat(46)}`; // Second enqueue — must NOT throw, must produce distinct safe gen. let r2; @@ -6754,3 +6756,61 @@ test("drain_promoteDeferredAllocExhausted_noStrandedLock_retryScheduled_drainSch globalThis.window.clearTimeout = origClearTimeout; } }); + +// ── Test 62: normal blob — no rebase fires on healthy state ────────────────── +test("hydrate_normalBlob_noRebase_verbatimNextGen", () => { + // Control witness for the GEN_REBASE_HEADROOM trigger. A blob with gens + // well below the ceiling (e.g. gen=1000, ng=1001) must NOT trigger a rebase. + // The headroom boundary (requiredNextGen >= MAX_SAFE_INTEGER - 2**32) is + // ~4.3e9 away from MAX_SAFE_INTEGER. At ng=1001 the trigger cannot fire. + // + // Failure branch: clamp/normalize only — requiredNextGen = 1001, nextGen + // must be exactly 1001 after hydration (verbatim, no rebase). + // Durable surface: nextGen equals requiredNextGen exactly; rebase was not + // invoked (intents retain their original gen identities). + globalThis.window.localStorage = makeLocalStorage(); + const pubkey = "62".repeat(32); + const channelId = `normal-blob-ch-${"n".repeat(49)}`; + const v2Key = `buzz.nip-rs.override-state.v2:${pubkey}`; + const ls = globalThis.window.localStorage; + + const fakeRelay = { + fetchEvents: async () => [], + publishEvent: async () => {}, + subscribeFenced: async (_f, _h) => makeFenceHandle({ eose: true }), + subscribeLive: async (_f, _h) => () => {}, + subscribeToReconnects: () => () => {}, + getConnectionGeneration: () => 0, + }; + + // Plant a normal blob: one intent at gen=1000, ng=1001. + ls.setItem( + v2Key, + JSON.stringify({ + pi: { [channelId]: { gen: 1000, op: "unread" } }, + ng: 1001, + }), + ); + + const mgr = new ReadStateManager(pubkey, fakeRelay); + mgr.hydrateFromLocalStorage(); + + // nextGen must equal requiredNextGen = 1001, verbatim — no rebase. + const ngAfterHydrate = pendingOverrideIntentStore.nextGen; + assert.equal( + ngAfterHydrate, + 1001, + `nextGen must be verbatim 1001 for healthy blob; got ${ngAfterHydrate}`, + ); + + // The single intent must retain its original gen=1000. + const intent = pendingOverrideIntentStore.get(channelId); + assert.ok(intent, "intent must be present after hydration"); + assert.equal( + intent.gen, + 1000, + `intent gen must remain 1000 (no rebase); got ${intent.gen}`, + ); + + mgr.destroy(); +}); diff --git a/desktop/src/features/channels/readState/readStateManager.ts b/desktop/src/features/channels/readState/readStateManager.ts index 331a41b8ba..44a472e64c 100644 --- a/desktop/src/features/channels/readState/readStateManager.ts +++ b/desktop/src/features/channels/readState/readStateManager.ts @@ -397,22 +397,24 @@ export class ReadStateManager { async drainPendingIntents(drainGen: number): Promise { try { await drainPendingIntentsImpl(createDrainContext(this), drainGen); - // Reset only on a successful pass; abort paths set drainRetryTimer first. - if (this.drainRetryTimer === null) { - this.drainRetryAttempt = 0; - } - this.drainScheduled = false; - if (this.pendingFreshDrain && !this.destroyed) { - this.pendingFreshDrain = false; - if (!this.drainScheduled && this.isLoadComplete) { - this.drainScheduled = true; - void this.drainPendingIntents(this.loadGeneration); - } - } } finally { - // Always clear drainScheduled even if the impl throws. + // Always clear drainScheduled even if the impl throws, so the manager + // is never permanently latched. this.drainScheduled = false; } + // Success path: only reached when the impl returned without throwing. + // Reset the retry counter (abort paths set drainRetryTimer before returning, + // so timer !== null means the pass aborted and the counter must be preserved). + if (this.drainRetryTimer === null) { + this.drainRetryAttempt = 0; + } + if (this.pendingFreshDrain && !this.destroyed) { + this.pendingFreshDrain = false; + if (!this.drainScheduled && this.isLoadComplete) { + this.drainScheduled = true; + void this.drainPendingIntents(this.loadGeneration); + } + } } /** Schedule a fresh drain pass; sets `pendingFreshDrain` if already in-flight. */ diff --git a/desktop/src/features/channels/readState/readStateStorage.ts b/desktop/src/features/channels/readState/readStateStorage.ts index e393e4eebf..438aca28dd 100644 --- a/desktop/src/features/channels/readState/readStateStorage.ts +++ b/desktop/src/features/channels/readState/readStateStorage.ts @@ -362,6 +362,12 @@ function readOverrideState(pubkey: string): OverrideStateBlob { // // This guarantees no allocation can produce a gen that matches a // retained receipt, since all receipts are renumbered in the same step. + // + // Invariant: after any hydration, at least GEN_REBASE_HEADROOM + // allocations succeed before exhaustion, so allocateGeneration()'s + // throw is unreachable from any valid persisted state and remains + // defense in depth only. + const GEN_REBASE_HEADROOM = 2 ** 32; // ≈4.3e9 — at 100 actions/sec, >1 year without restart { let maxObservedGen = 0; for (const intent of pendingIntents.values()) { @@ -374,17 +380,17 @@ function readOverrideState(pubkey: string): OverrideStateBlob { const requiredNextGen = maxObservedGen + 1; if ( !Number.isSafeInteger(requiredNextGen) || - requiredNextGen >= Number.MAX_SAFE_INTEGER + requiredNextGen >= Number.MAX_SAFE_INTEGER - GEN_REBASE_HEADROOM ) { - // Allocator exhausted — rebase all intent gens and matching receipt - // gens into a compact safe range [1..N]. intents is keyed by - // channelId with at most one entry per channel, so sorting is - // deterministic and produces a stable assignment. + // Allocator exhausted or near-ceiling — rebase all intent gens and + // matching receipt gens into a compact safe range [1..N]. intents + // is keyed by channelId with at most one entry per channel, so + // sorting is deterministic and produces a stable assignment. // - // The ceiling check (`requiredNextGen >= MAX_SAFE_INTEGER`) catches the - // valid one-below-ceiling state (`maxObservedGen = MAX_SAFE_INTEGER - 1`) - // before `allocateGeneration()` can mint `MAX_SAFE_INTEGER` and leave - // `_nextGen` at the unsafe value. + // The headroom trigger fires when requiredNextGen is within + // GEN_REBASE_HEADROOM of MAX_SAFE_INTEGER, ensuring that after any + // hydration at least GEN_REBASE_HEADROOM fresh allocations succeed + // before allocateGeneration() can throw. const sorted = [...pendingIntents.entries()].sort( ([, a], [, b]) => a.gen - b.gen, );