fix(read-state): enforce byte-budget eviction in currentContexts()#1305
Merged
Conversation
The relay rejects read-state events whose NIP-44 ciphertext exceeds the
relay's content limit. The previous guard used an entry-count threshold
(8k/10k) which is the wrong metric: at 8k msg: entries the JSON blob is
already ~544KB of keys alone, well past the relay's limit.
Replace the entry-count eviction with a serialized byte-budget loop.
currentContexts() now calls trimContextsToBudget() which serializes the
full {v:1, client_id, contexts} blob, checks its byte length, and evicts
oldest msg: entries first (lowest timestamp), then oldest thread: entries,
until the JSON fits within READ_STATE_MAX_PLAINTEXT_BYTES (32KB). Channel
keys are never evicted. The 32KB plaintext budget produces ~45KB ciphertext
after NIP-44's ~1.4x overhead, well under the relay's 256KB content cap
and NIP-44's 65,535-byte plaintext hard limit.
A console.warn fires when trim occurs so the fix is visible in staging.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…h on overflow Address two IMPORTANT findings from Thufir's review of #1305: 1. O(n²) re-encode loop — the eviction loop called encoder.encode() on every iteration, O(n) per step × n steps = O(n²). With 7,500 entries this caused a confirmed 4.7s UI freeze on first publish after upgrade. Replace with an O(n) pass: compute total bytes once, subtract each entry's per-entry delta (key.length + 4 + timestamp digits), collect entries to evict, delete them in a batch, then do one final encode as the authoritative check (handles JSON comma-accounting edge cases the delta estimate ignores). 2. Silent failure when channel-only blob exceeds budget — after exhausting all msg:/thread: entries the function returned without checking whether the remaining blob still exceeded the budget. The caller would then proceed to publish a blob the relay would reject. Fix: trimContextsToBudget() now returns { evicted, fitsAfterTrim }. currentContexts() returns null when fitsAfterTrim is false; publish() and initialize() both guard against null and skip the publish. Also address two MINOR findings: - JSDoc now documents that trimContextsToBudget mutates contexts in place - Empty-map test replaced with a test that actually exercises the fitsAfterTrim=false path (channel-only blob smaller than budget) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Auto-format via biome format --write; remove two unused fullSize variables in readStateManager.test.mjs (inlined into budget computation). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
wpfleger96
added a commit
that referenced
this pull request
Jun 29, 2026
…ixes Builds on the byte-budget eviction base (#1305) with two proactive improvements and full correctness hardening from three review passes. `evictDominatedEntries()`: drops `thread:` and `msg:` entries from `currentContexts()` whose timestamp is already covered by the parent channel frontier. Proactive O(n) pass before the byte-budget trim fires. Only runs when `parentResolver` is set; skips silently when null so we never evict what we can't verify is dominated. 6 unit tests: dominated eviction, newer-than-channel survival, equal-ts eviction, unresolvable parent, missing parent frontier, channel-key immunity. When channel keys alone exceed 32 KB (degenerate case: thousands of channels), `splitContextsIntoBudgetedSlots()` partitions them round-robin across up to 8 independent kind:30078 slots. Thread/msg entries go into the primary slot and are trimmed to budget. Extra slot IDs are persisted in localStorage. `mergeEvents` now unions all own-slot blobs (max-merge) instead of taking the single highest-created_at blob. `fetchOwnBlobBeforePublish` fetches all own slot d-tags in one query. `publishOneSlot` extracted to avoid duplication. 5 pure-function tests: single-slot fit, growth to 2 slots, max-slots exceeded → null, thread/msg in primary slot, thread/msg trimmed when primary slot full. Both single-slot and split paths: compute union of all slot contexts, compare to `lastPublishedContexts`, return early if identical. No relay writes when state is unchanged. `deleteExtraSlots()` publishes kind:5 delete events (NIP-33 a-tag format: `30078:<pubkey>:read-state:<slotId>`) for each extra slot blob and clears `extraSlotIds`. Prevents stale blobs from re-inflating `lastPublishedContexts` on subsequent debounce cycles. - `lastPublishedContexts = {}` reset moved inside the `extraSlotIds.length > 0` guard — resetting unconditionally cleared relay-fetched state on every debounce cycle, reintroducing the retry storm in steady-state single-slot mode. - `publishSplitSlots_noopSuppression_skipsWhenUnchanged` integration test: stubs `publishOneSlot` via dot notation, verifies the no-op path fires on second publish with unchanged state. 1150/1150 tests pass, biome clean. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
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
1 participant
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.
Fixes the
[ReadStateManager] publish failedrelay rejection that breaks cross-device read state sync.Problem
currentContexts()guarded blob size with an entry-count threshold (8k/10k). Entry count is the wrong metric — the relay enforces a byte-size limit on event content, not an entry count. At 8kmsg:entries the JSON blob is already ~544KB of keys alone, well past the relay's limit. The relay responds withOK false events too long, whichrelayClientSession.tssurfaces as the[ReadStateManager] publish failedwarning. BecauselastPublishedContextsis never updated on failure, the manager retries on every debounce cycle (~5s).Fix
Replace the entry-count eviction block in
currentContexts()with a serialized byte-budget loop via a new exported pure functiontrimContextsToBudget():{v:1, client_id, contexts}blob and check its UTF-8 byte length.msg:entries andthread:entries separately, sort each oldest-first (lowest timestamp).msg:first, thenthread:, one entry at a time, re-checking the serialized size after each deletion.channel:) are never evicted.console.warnfires when trim occurs so the fix is visible in staging.READ_STATE_MAX_PLAINTEXT_BYTES = 32_768(32KB) is added toreadStateFormat.ts. This produces ~45KB NIP-44 ciphertext (~1.4× overhead), well under the relay's 256KB content cap and NIP-44's 65,535-byte plaintext hard limit.Files changed
desktop/src/features/channels/readState/readStateFormat.ts— addREAD_STATE_MAX_PLAINTEXT_BYTESdesktop/src/features/channels/readState/readStateManager.ts— addtrimContextsToBudget(), replace eviction block incurrentContexts()desktop/src/features/channels/readState/readStateManager.test.mjs— 5 new tests covering the trim pathStack
Stack: this PR → #1309