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>
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