Skip to content

sdk: add subscription EOSE query and atomic reissue - #1495

Draft
agent-p1p wants to merge 2 commits into
nostrdevkit:masterfrom
erskingardner:sdk/reissue-subscription
Draft

agent-p1p wants to merge 2 commits into
nostrdevkit:masterfrom
erskingardner:sdk/reissue-subscription

Conversation

@agent-p1p

@agent-p1p agent-p1p commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Relay and client notifications are broadcast channels, so a receiver that falls behind loses messages, EOSE included. with_gaps() (#1489) reports that a gap happened, but not which subscriptions lost their EOSE. A client that keeps long-lived subscriptions open, such as MDK, then can't tell whether a subscription's stored events all arrived, and can't safely ask the relay to send them again, so its users can end up with silently missing messages. Re-sending as two send_msg calls can split on the bounded outbound queue: a lone CLOSE leaves the relay without the subscription, and a lone REQ repeats an open ID, which relays such as strfry reject with CLOSED duplicate:, after which the SDK drops the subscription for good.

Fix

Relay::subscription and Relay::subscriptions now return a SubscriptionData that reports, among other things, whether the subscription's current REQ has been answered with EOSE, read from the state the relay's read loop already keeps. It can also reissue a registered long-lived subscription: CLOSE and a REQ with the stored filters are queued as one unit (both or neither), and the REQ counts as pending, so the restoration on reconnect doesn't send a second one. The sender now restarts EOSE and limit tracking whenever it dispatches a REQ, not only on a new connection, so both describe the query the relay is actually answering. This is a narrow, subscription-specific method, not a return of the general batch_msgs removed in #1240.

Description

MDK's repair after a notification gap, for each live subscription on each relay:

  1. If the relay already answered the current REQ with EOSE, nothing needs doing.
  2. Otherwise, reissue the subscription so the relay sends its stored events again.

New API:

  • Breaking: Relay::subscription(&self, id: &SubscriptionId) -> Option<SubscriptionData> and Relay::subscriptions(&self) -> HashMap<SubscriptionId, SubscriptionData> now return the registered SubscriptionData instead of its filters. Its fields are private, and it has getters only, with no constructors: filters() -> &[Filter], received_eose() -> bool and is_closed() -> bool. Repair step 1 is relay.subscription(id).await.map(|s| s.received_eose()). Client::subscription/subscriptions still return per-relay filters.
  • Relay::reissue_subscription(&self, id: &SubscriptionId) -> ReissueSubscription<'_, '_>: an IntoFuture builder with output Result<(), Error>, like Relay::unsubscribe. It reserves two slots with Sender::try_reserve_many(2) and queues CLOSE then REQ under the subscriptions lock, like queue_msg. It errors if the subscription isn't registered (NotFound), is auto-closing (Invalid), if the relay can't read (Unsupported) or isn't operational, or if the queue can't fit both frames (State).

Behaviour fix (separate first commit): subscription_req_started reset received_eose and the pre-EOSE event count only when connection > subscribed_connection. A REQ sent again on the same connection therefore kept received_eose = true, so the EOSE query and the verify_subscriptions limit check both described the previous REQ. The reset now happens every time the sender dispatches a tracked REQ, under the subscriptions write lock that EOSE handling also takes. New-session behaviour is unchanged. This also covers a REQ re-sent with send_msg for a registered subscription, which the relay answers with a new stored-events query too.

Notes to the reviewers

  • Why not restore batch_msgs. sdk: redesign send msg APIs #1240 removed Relay::batch_msgs and Client::batch_msgs in the API redesign (sdk: redesign APIs #920). The only atomic sequence needed here is "reissue this registered subscription". A narrow method keeps the queue reservation internal, always uses the stored filters (so callers can't desync the registry), and reuses the pending-REQ bookkeeping from sdk: avoid duplicate REQ for subscriptions queued while disconnected #1492 and de3031a. No Client counterpart for now; MDK can use client.relay(url).
  • While disconnected, the reissue is queued, not rejected. This matches send_msg and subscribe, and avoids a status check that could race with a reconnect. The queued REQ counts as pending, so the restoration in post_connection skips the subscription. The next session sends CLOSE (for an ID that session doesn't know) followed by exactly one REQ. A test checks this with a relay that answers duplicate REQs with CLOSED duplicate:. The reset happens on dispatch, not on enqueue, so SubscriptionData::received_eose describes the previous REQ until then.
  • Known race (also in a code comment). On a same-connection reissue, frames the relay sent for the previous REQ before it handled the CLOSE may arrive after the reset. A late EOSE sets the flag early. That is acceptable because the filters are the same, so that EOSE covers the same stored events. Likewise, late events from the previous REQ count toward the new REQ's limit. This only matters with verify_subscriptions and a single filter with a limit, and the two queries can't be told apart on the wire.
  • Review change: the first version added a dedicated Relay::subscription_received_eose. Following review, the state is exposed through SubscriptionData instead. Because the registry sits behind an async lock, the getters return an owned snapshot. Clone is implemented by hand because the pre-EOSE event count is an AtomicUsize.
  • Sender::try_reserve_many is available since tokio 1.37, the workspace minimum.
  • Tests use the recording connection from test_utils. They cover:
    • SubscriptionData: no entry before subscribing, then the stored filters with received_eose() == false, then true after EOSE; is_closed() after an auth-required: CLOSED;
    • adjacent, ordered CLOSE + REQ on a live connection, with the EOSE flag and the limit counter restarting;
    • nothing queued with a full queue or with one free slot;
    • reissue while disconnected, followed by exactly one REQ after reconnect;
    • the error cases;
    • a raw same-connection REQ resend for the fix commit.
  • just precommit passes.

Checklist

  • I followed the contribution guidelines
  • I updated the relevant CHANGELOG.md (if applicable)
  • I understand and can explain all code in this PR

🤖 Generated with Claude Code

@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.12536% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
nostr-sdk/src/pool/mod.rs 25.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@agent-p1p

Copy link
Copy Markdown
Contributor Author

This PR depends on #1496 to go green. The failing "Check stable" job isn't caused by this PR: master fails the same job. Rust 1.99 deprecates AtomicUsize::fetch_update, which PendingReqCounter::decrement uses, and the job builds with -D warnings. #1496 fixes that. Once it's merged, I'll rebase this branch onto master so CI runs against the fix.

Comment thread nostr-sdk/src/relay/mod.rs Outdated
agent-p1p and others added 2 commits October 19, 2026 09:45
The relay answers every REQ with a new stored-events query, but the
sender reset a subscription's EOSE flag and pre-EOSE event count only
when it dispatched the REQ on a new session. A REQ sent again on the
same connection (e.g., CLOSE followed by REQ) kept the state of the
previous REQ, so the EOSE flag and the `verify_subscriptions` limit
check described the old query: the limit check skipped the replayed
events, or counted them on top of the earlier ones.

Reset both whenever the sender dispatches a tracked REQ, under the
subscriptions write lock that EOSE handling also takes. The new-session
behavior is unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Notification receivers that fall behind lose messages, EOSE included,
and `NotificationStream::with_gaps` reports that a gap happened but not
which subscriptions lost their EOSE. To repair a long-lived subscription
after a gap, a client needs to know whether the relay already answered
its current REQ with EOSE and, if not, reissue it as CLOSE followed by
REQ. The two frames must be queued together: a lone CLOSE leaves the
relay without the subscription, and a lone REQ repeats an open ID, which
some relays reject as a duplicate, after which the subscription is
dropped. Two `send_msg` calls can split, since the outbound queue is
bounded and fed with `try_send`.

Return `SubscriptionData` from `Relay::subscriptions` and
`Relay::subscription` instead of the filters. Its fields are private,
and its getters expose the filters, whether the current REQ received
EOSE (the flag the relay read loop already records), and whether the
relay closed the subscription.

Add `Relay::reissue_subscription`, which reserves two slots in the
outbound queue and queues CLOSE and a REQ with the stored filters, or
nothing. The REQ is tracked as pending, so the restoration on reconnect
doesn't send another one, also when the reissue is queued while
disconnected.

This is narrower than the `batch_msgs` methods removed in
nostrdevkit#1240: the only atomic sequence
needed is the reissue of a registered subscription.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@agent-p1p
agent-p1p force-pushed the sdk/reissue-subscription branch from a24efae to f20d3f6 Compare October 19, 2026 10:03

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants