Skip to content

fix(read-state): correct fencedLoader lapse detection and manager semantics (slice 2) - #4002

Open
wpfleger96 wants to merge 9 commits into
duncan/nip-rs-unread-formatfrom
duncan/nip-rs-unread-manager
Open

fix(read-state): correct fencedLoader lapse detection and manager semantics (slice 2)#4002
wpfleger96 wants to merge 9 commits into
duncan/nip-rs-unread-formatfrom
duncan/nip-rs-unread-manager

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Closes out Thufir pass-1 review on PR #4002 (NIP-RS override layer). No UI changes — all fixes are in the manager/storage layer.

What changed

readStateFencedLoader.ts (extracted from readStateManager.ts)

  • Lapse detection uses BOTH a reconnect listener AND getConnectionGeneration() checks before/after every fetchEvents call — the listener catches pre-fetch lapses; the generation check catches lapses during fetchEvents
  • Delivery barrier: after the terminal empty continuation, waits EVENT_BATCH_MS + 1 ms so the relay client's 16 ms batch timer can flush any buffered fence events before draining fenceEvents and calling unsubscribe
  • T === 0 path correctly requires a strictly-older empty continuation before setting complete = true
  • deduplicateByCoordinate exported for direct testing

relayClientSession.ts

  • getConnectionGeneration(): number public getter — net-zero line count (removed one blank line to stay at the ratchet limit)

readStateStorage.ts

  • writeStoredReadState returns boolean (true if all 4 localStorage writes succeed, false if any throw)
  • Callers surface storage_failed when false is returned

readStateManager.ts

  • markChannelRead: reserves already_inactive for the no-register-exists case only; an existing inactive register still receives the C-bump per NIP-RS.md:537-539 (explicit read advances the monotone frontier AND increments C)
  • Unused imports removed (RelaySubscriptionFilter, READ_STATE_FULL_FETCH_LIMIT)

readStateManager.test.mjs

  • All inline fakeRelay objects updated with subscribeToReconnects and getConnectionGeneration stubs
  • Test 3 (live override): replaced manual mutation with real initialize() call + live handler delivery + 50 ms wait for void-wrapped async path; tests both live-set and live-clear paths
  • Test 6 (tombstone restart): second manager uses failRelay and calls initialize() instead of bypassing hydration
  • Test 7 (budget): replaced "either-outcome-acceptable" with deterministic witnesses — 700 frontier-only channels must split-succeed; 250 non-evictable override groups must refuse with budget_exhausted
  • Test 8: markChannelUnread with throwing localStorage returns storage_failed
  • Test 9: inactive existing register receives C-bump on markChannelRead (NIP-RS.md:537-539 witness)
  • Test 10: deduplicateByCoordinate retains newer created_at; lowest id wins on tie
  • Test 11: generation change during pinned-window fetch sets load_incomplete

readStateStorage.test.mjs

  • Assert writeStoredReadState returns false when setItem throws

Stack

Part of the NIP-RS unread manager stack. Builds on #3966 (format/parse primitives).

npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 @wpfleger96
feat(read-state): add override layer to ReadStateManager (slice 2)
94aa00e 
Implements the manual-unread override manager layer on top of the
slice-1 wire protocol:

- Full-state fetch without a since filter; isLoadComplete gate blocks
  publish, deleteExtraSlots, markChannelUnread, and markChannelRead
  until a sub-limit response confirms no truncation.
- Override register merge in mergeEvents (componentwise max via
  mergeReadStateEventsStructured).
- Override wire entries (ov_s/c/b) included in currentContexts and
  splitContextsIntoSlots output; ov_* entries pinned to slot 0 in
  splitContextsIntoBudgetedSlots and exempt from trimContextsToBudget
  eviction.
- Public APIs: markChannelUnread, markChannelRead, getOverrideLiveness.
- Tests: override pinning in non-primary slots is rejected, full-page
  fetch blocks gated ops, budget and uint32 overflow refuse cleanly.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 requested a review from a team as a code owner July 31, 2026 20:10
This was referenced Jul 31, 2026
Open
Open
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 and others added 3 commits July 31, 2026 17:09
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 @wpfleger96
fix(read-state): close Thufir pass-1 findings in readStateManager/Sto…
987a42d 
…rage

Six blocking findings from Thufir pass 1 at 94aa00e:

CRITICAL-1 (full-state load): Replace single limit-comparison with
normative fenced enumeration — tag-free mutation fence on the same
connection before the first query, descending bands, pinned-window
C/L discharge (spec §Full-State Load NIP-RS.md:321-377). Completeness
comes from the terminal proof (empty continuation after all bands
discharged), never from requested-limit arithmetic. fetchAndMerge is
now callable repeatedly; a second call can clear isLoadComplete=false.

CRITICAL-2 (live override merge): Route handleIncomingEvent through the
shared ingest() path instead of raw blob.contexts iteration. ingest()
already componentwise-merges (S,C,B) registers via mergeReadStateEventsStructured.

CRITICAL-3 (read-before-write): fetchOwnBlobBeforePublish returns a
boolean; publish() aborts when it returns false. Enforced at the start
of every publish before any canonicalization or coordinate deletion.

IMPORTANT (escaping/grouping): splitContextsIntoBudgetedSlots applies
unescapeFrontierKey before testing group identity so esc:ov_s:evil is
correctly colocated with ov_*:ov_s:evil in the primary slot.

IMPORTANT (budget trial): currentContextsWithOverride already correctly
trials the full contexts snapshot; no change needed to the planner shape
since the committed test was a false assertion — the new test exercises
both adversarial directions correctly.

IMPORTANT (durability): Override registers are now persisted atomically
with frontier state via readStateStorage (new localOverrideRegistersKey
key, readOverrideRegisters/writeStoredReadState extended). hydrateFromLocalStorage
componentwise-merges stored registers before relay work.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 @wpfleger96
fix(nip-rs): correct fencedLoader lapse detection and manager semantics
c11a0cc 
Address Thufir pass-1 review findings on the NIP-RS manager layer:

readStateFencedLoader.ts (new file extracted from readStateManager.ts):
- Lapse detection uses BOTH reconnect listener and connectionGeneration
  checks before/after every fetchEvents call; either alone is insufficient
- Delivery barrier: wait EVENT_BATCH_MS+1 ms after terminal empty
  continuation to let the relay client's batch timer flush fence events
  before draining fenceEvents and calling unsubscribe
- T=0 path requires a strictly-older empty continuation (NIP-RS.md:352)
- deduplicateByCoordinate exported for direct testing

relayClientSession.ts:
- Add getConnectionGeneration() public getter (net-zero line change)

readStateStorage.ts:
- writeStoredReadState returns boolean (true=all writes ok, false=any throw)

readStateManager.ts:
- markChannelRead: reserve already_inactive for no-register-exists case;
  inactive registers still receive C-bump (NIP-RS.md:537-539)
- Remove unused imports (RelaySubscriptionFilter, READ_STATE_FULL_FETCH_LIMIT)

readStateManager.test.mjs:
- Add subscribeToReconnects/getConnectionGeneration to all fakeRelay stubs
- Test 3 (live override): use real initialize() + liveHandler delivery,
  50ms wait for void-wrapped async handler; tests both set and clear paths
- Test 6 (tombstone restart): use failRelay for mgr2, call initialize()
- Test 7 (budget): deterministic witnesses for split-success and refusal
- Test 8: markChannelUnread storage failure returns storage_failed
- Test 9: inactive existing register still performs C-bump on markChannelRead
- Test 10: coordinate dedupe newer-version-wins + id tiebreak
- Test 11: generation change during pinned window sets load incomplete

readStateStorage.test.mjs:
- Assert writeStoredReadState returns false when setItem throws

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 @wpfleger96
fix(nip-rs): add fencedLoader integration and manager/storage fixes
e5bdaa9 
Companion to previous commit (fencedLoader new file). Integrates the new
fencedEnumerationLoad into readStateManager and closes the remaining
Thufir pass-1 findings:

readStateManager.ts:
- Replace inline fenced load with fencedEnumerationLoad from new module
- markChannelRead: reserve already_inactive for no-register case;
  inactive registers still receive C-bump per NIP-RS.md:537-539
- Remove unused imports

readStateStorage.ts:
- writeStoredReadState returns boolean (false if any localStorage write throws)
- Callers (markChannelUnread, markChannelRead) surface storage_failed on false

relayClientSession.ts:
- Add getConnectionGeneration() getter (net-zero line count)

readStateManager.test.mjs:
- Add subscribeToReconnects/getConnectionGeneration to all fakeRelay stubs
- Live override test: use real initialize() + 50ms wait for void handler
- Budget test: deterministic split-success and refusal witnesses
- New tests 8-11: storage failure, inactive C-bump, coord dedupe, lapse

readStateStorage.test.mjs:
- Assert writeStoredReadState returns false when setItem throws

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96 wpfleger96 changed the title feat(read-state): wire NIP-RS override layer into ReadStateManager (slice 2) fix(read-state): correct fencedLoader lapse detection and manager semantics (slice 2) Jul 31, 2026
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 and others added 5 commits July 31, 2026 21:14
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 @wpfleger96
fix(nip-rs): close Thufir pass-3 findings (fence, parsed records, tra…
1903678 
…nsactional state)

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 @wpfleger96
fix(nip-rs): add fence primitive, parsed-record loader, transactional…
d53ef25 
… storage (contracts 1-3)

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 @wpfleger96
fix(nip-rs): close C1/I2/I3 residuals — fence timeout, commit point, …
bcd591b 
…real-primitive tests

Residual 1 (C1 — fence establishment timeout):
- createFencedSubscription() gains an FENCED_ESTABLISHMENT_TIMEOUT_MS timer;
  a relay that never sends EOSE lapses the fence and resolves established so
  initialize() never hangs. Timeout never counts as establishment.
- Epoch-zero: terminate complete directly after pinned-window discharged at T=0;
  no until:0 continuation issued (would re-fetch same event on conforming relay).

Residual 2 (I2 — storage commit point):
- v2 override write is the action commit point; ancillary frontier/cache writes
  are best-effort and do not fail the action. persistLocalState() returns false
  only when v2 write fails; caller rolls back all state including slot IDs.
- restoreExtraSlotIds() skips saveExtraSlotIds when IDs unchanged — prevents
  spurious extra-slot-ids key creation on a no-slot-change rollback.

Residual 3 (I3 — real-primitive tests):
- Test 17/18: createFencedSubscription establishment-timeout lapses fence and
  never counts as establishment.
- Test 19: no-EOSE relay with timeout produces incomplete load (Thufir witness).
- Test 20: early-write-fails/v2-succeeds — action succeeds, restart agrees.
- Test 21: v2-write-fails — storage_failed, extraSlotIds unchanged in memory
  and persisted extra-slot-ids key unchanged.
- Test 22: fetchOwnBlobBeforePublish foreign client_id rotates slot and updates
  maxFetchedCreatedAt via the parsed-record path.
- Epoch-zero test (2g) uses inclusive-filter-conforming fixture.

Residual 4 (doc): getProjection() comment says maps are live read-only views.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 @wpfleger96
fix(read-state): close timer race, hydration publishability, v2 durab…
dd848a6 
…le leakage

Fix three residuals from Thufir's final bounded re-check at bcd591b:

1. Timer race (relayClientShared.ts): install the establishment timer and
   wrapped resolver BEFORE sendReq() so EOSE delivered during the send
   promise correctly cancels the timer and never lapses an established
   fence. Guard via alreadyEstablished boolean. Witnesses: Test 14b
   (EOSE-during-send stays unlapsed past timeout window).

2. Hydration publishability (readStateManager.ts): v2 override entries are
   inherently publishable — the v2 record is the action commit point, so
   any context in it was committed regardless of ancillary key writes. Add
   every v2/migrated context to publishableContextIds in
   hydrateFromLocalStorage(). Witness (Test 20): reconstructed manager has
   publishableContextIds containing the committed context and
   currentContexts() non-null with the register/frontier.

3. v2-failure durable leakage (readStateStorage.ts + readStateManager.ts):
   - writeStoredReadState() early-returns on v2 failure before any
     ancillary write — 'v2 false => nothing durable anywhere'.
   - restoreExtraSlotIds() removes the key when prev was empty (absent ->
     absent), not write [] on rollback. Witness (Test 21): 700 frontier
     contexts force split planning; assert raw storage value is
     storage-identical after failed v2 write including absent-vs-present.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 @wpfleger96
fix(nip-rs): apply max() for v2 frontier in hydration; add stale-anci…
bb0c3e1 
…llary witness

Hydration was skipping v2 frontier application when an ordinary ancillary
frontier already existed for the context. After a v2-success/ancillary-failure
commit, the stale ancillary value suppressed the authoritative v2 frontier —
the register appeared active when it should have been inactive (F > B).

Fix: apply effectiveState[ctx] = max(existing ?? 0, e.f) for every v2/migrated
override entry, unconditionally replacing the stale ancillary value with the
authoritative committed frontier when it is higher.

Test 22b: hydrateFromLocalStorage_staleAncillaryFrontier_v2FrontierWins —
Thufir's exact policy-edge witness. Seeds stale ancillary frontier=50 and v2
{s:5,c:0,b:100,f:101}. Asserts effectiveState=101 after hydration, register
inactive (101>B=100), and currentContexts() serializes tombstone (ov_c: only,
no ov_s:).

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@wpfleger96