Repository navigation
fix(relay): bound reconnect replay with resumed since (#105) - #106
ContextVM-org wants to merge 3 commits into
Conversation
Restored subscriptions re-request only the newest sane created_at minus a 30-minute guard (RESUBSCRIBE_SINCE_GUARD_SECONDS) instead of the whole session, so a relay reconnect can no longer re-dispatch stored requests whose ids have aged out of the dedup cache and re-execute completed tool calls. Future-dated events cannot move the resume point; filters without since are restored unchanged. The transports' seenEventIds cache grows to 40,000 entries (DEFAULT_DEDUP_CACHE_SIZE, ~20,000 encrypted requests) to cover the replay window.
|
Location: src/relay/applesauce-relay-pool.ts:539 In the locked I reproduced an automatic reconnect without rebuilding the pool: an event one hour old was replayed despite a current watermark. After forcing dedup-cache eviction, that This leaves the original reconnect replay defect unresolved once the larger cache fills. The added tests miss this path because they explicitly invoke A filter evaluation deferred until each request subscription/retry would be better imo, and add a regression test using an actual connection drop and automatic recovery would cover the important part as well. Location: src/relay/applesauce-relay-pool.ts:528 The condition accepts timestamps up to 1,800 seconds ahead of the receiver’s clock. It also updates the watermark before the transport validates or decrypts the event. I reproduced this with a validly signed kind-1059 event containing an undecryptable payload and This request-loss probe passes on the base commit and fails on this PR. The existing future-event test only covers timestamps far beyond the allowed interval. clamping the watermark to local current time, or exclude future timestamps would work, and adding coverage for a rejected envelope just inside the current future-time allowance should be good to go. |
…stamps Review findings on the reconnect-resume change: - applesauce-relay@6.2.1 evaluates a filter function once at req() construction and connection retries reuse that result, so the resume point only applied on pool rebuilds. The filter input is now a deferred stream, recomputed on every REQ (re)send including automatic recovery after a connection drop. - The watermark accepted created_at up to 1800s ahead of the local clock and advanced before the transport validated the envelope: a rejected near-future envelope could push the restore point past unseen requests from slower clocks. Future timestamps are now excluded outright. Regression tests cover automatic recovery after a real connection drop (no rebuild) and a near-future envelope followed by a slower-clock request.
Review follow-ups: the reconnect loss window wording now names the real condition (created_at trailing the newest event seen, e.g. a lagging relay rather than only a slow sender clock), the resumeWatermark comment matches the implemented rule, and the unused test-helper type entry is gone.
2 participants
Restored subscriptions re-request only the newest sane created_at minus a 30-minute guard (RESUBSCRIBE_SINCE_GUARD_SECONDS) instead of the whole session, so a relay reconnect can no longer re-dispatch stored requests whose ids have aged out of the dedup cache and re-execute completed tool calls. Future-dated events cannot move the resume point; filters without since are restored unchanged.
The transports' seenEventIds cache grows to 40,000 entries (DEFAULT_DEDUP_CACHE_SIZE, ~20,000 encrypted requests) to cover the replay window.