From dda3f7f6b3d118aab45e46e3826743e98dd99524 Mon Sep 17 00:00:00 2001 From: chappie-daemon Date: Sat, 24 Oct 2026 21:38:34 -0400 Subject: [PATCH 1/5] test(relay): order the CLOSE-during-mint test by program order (#963) --- .changeset/close-mint-sync.md | 9 +++ crates/nostream-relay/tests/handlers.rs | 103 +++++++++++++++++++----- 2 files changed, 90 insertions(+), 22 deletions(-) create mode 100644 .changeset/close-mint-sync.md diff --git a/.changeset/close-mint-sync.md b/.changeset/close-mint-sync.md new file mode 100644 index 00000000..1b1c12d1 --- /dev/null +++ b/.changeset/close-mint-sync.md @@ -0,0 +1,9 @@ +--- +--- + +fix(relay): order the CLOSE-during-mint test's release by program order (#963) + +`a_close_during_the_invite_mint_leaves_the_event_but_not_the_read` no longer orders +its gate release against the `CLOSE` with a 50 ms sleep. It owns its socket state and +drives `subscribe::handle` and the synchronous `unsubscribe::handle` directly, so the +abort is a program-order predecessor of the release. No production code changes. diff --git a/crates/nostream-relay/tests/handlers.rs b/crates/nostream-relay/tests/handlers.rs index ffaec0c6..584397dc 100644 --- a/crates/nostream-relay/tests/handlers.rs +++ b/crates/nostream-relay/tests/handlers.rs @@ -1,7 +1,10 @@ //! The five handlers over a real socket. Carries the behaviour of //! `test/unit/handlers/event-message-handler.spec.ts` (200 cases), //! `subscribe-message-handler.spec.ts` (58), `count-message-handler.spec.ts` (10), -//! `auth-message-handler.spec.ts` (12) and `unsubscribe-message-handler.spec.ts` (1). +//! `auth-message-handler.spec.ts` (12) and `unsubscribe-message-handler.spec.ts` (1). One +//! exception, `a_close_during_the_invite_mint_leaves_the_event_but_not_the_read`, +//! drives the two handlers directly so its `CLOSE` is ordered against a held mint by +//! program order, not a timer. //! //! Every assertion is a frame as received, byte for byte, which is what makes these a port //! rather than an approximation. The EVENT pipeline's reason strings are pinned one test per @@ -12,16 +15,22 @@ mod common; +use std::sync::Arc; use std::time::Duration; -use common::{CLIENT_SECRET, NOW_SECS, RELAY_SECRET, Server, relay_keys, signed_event}; +use common::{CLIENT_SECRET, NOW_MS, NOW_SECS, RELAY_SECRET, Server, relay_keys, signed_event}; use nostream_config::settings::{ ClientLimits, ClientSubscriptionLimits, ConnectionLimits, CreatedAtLimits, EventIdLimits, EventLimits, EventRateLimit, KindLimits, Limits, MessageLimits, MessageRateLimit, Nip05Settings, Payments, PubkeyLimits, RateLimit, }; use nostream_core::Event; +use nostream_core::clock::TestClock; +use nostream_core::filter::Filter; +use nostream_core::messages::{SubscribeMessage, UnsubscribeMessage}; +use nostream_relay::connection::Connection; use nostream_relay::relay_key::RelayKeySource; +use nostream_relay::sink::FrameSink; use nostream_relay::testing::{Fixture, nip42_settings, nip43_settings, nip45_settings, settings}; use nostream_storage::repositories::User; @@ -1199,6 +1208,9 @@ async fn an_invite_request_mints_one_kind_28935_event_before_eose() { #[tokio::test] async fn a_close_during_the_invite_mint_leaves_the_event_but_not_the_read() { + // The `CLOSE` is a direct handler call: `unsubscribe::handle` is synchronous, so in this + // task the abort is a program-order predecessor of the gate release. A `CLOSE` over a + // socket is covered by `a_close_aborts_an_in_flight_read_without_a_frame`. let mut settings = settings(); settings.nip43 = Some(nip43_settings()); @@ -1208,42 +1220,89 @@ async fn a_close_during_the_invite_mint_leaves_the_event_but_not_the_read() { .seed_event(signed_event(&CLIENT_SECRET, 1, vec![], "stored", NOW_SECS)) .await; - let server = start(fixture).await; - let (mut client, challenge) = server.connect().await; + let (sink, mut frames) = FrameSink::channel(1024 * 1024); + let connection = Arc::new(Connection::new( + "127.0.0.1".to_owned(), + nostream_core::nip42::Nip42Session::with_challenge("challenge", 0), + )); + assert!(connection.authenticate( + nostream_core::crypto::public_key(&CLIENT_SECRET).expect("client public key"), + nostream_core::EventId::from_byte_array([3; 32]), + NOW_SECS, + )); + let context = Arc::new(fixture.context( + Arc::new(TestClock::new(NOW_MS)), + relay_keys(), + Arc::clone(&connection), + sink, + )); + + let reading = tokio::spawn({ + let context = Arc::clone(&context); + + async move { + nostream_relay::handlers::subscribe::handle( + SubscribeMessage { + subscription_id: "invite".to_owned(), + filters: vec![Filter { + kinds: Some(vec![nostream_core::Kind(28_935), nostream_core::Kind(1)]), + ..Default::default() + }], + }, + &context, + ) + .await; + } + }); - let auth = auth_event(&challenge, NOW_SECS); - client - .send(&format!( - r#"["AUTH",{}]"#, - serde_json::to_string(&auth).expect("auth json") - )) - .await; - assert!(client.expect_frame().await.ends_with(r#"true,""]"#)); + // The read is registered before the mint's only `await`, so it exists by the time the + // mint is held. + for _ in 0..100 { + if connection.read_in_flight("invite") { + break; + } - client.send(r#"["REQ","invite",{"kinds":[28935,1]}]"#).await; + tokio::task::yield_now().await; + } // The mint's one `await` is held, which is the window a `CLOSE` lands in. - for _ in 0..200 { - if server.fixture.invites.creates_started() == 1 { + for _ in 0..100 { + if fixture.invites.creates_started() == 1 { break; } - tokio::time::sleep(Duration::from_millis(5)).await; + tokio::task::yield_now().await; } assert_eq!( - server.fixture.invites.creates_started(), + fixture.invites.creates_started(), 1, "the invite mint is in flight" ); + assert!(connection.read_in_flight("invite"), "the read is in flight"); + + nostream_relay::handlers::unsubscribe::handle( + UnsubscribeMessage { + subscription_id: "invite".to_owned(), + }, + &context, + ); + + // The abort precedes the release below, so a reintroduced timer or a reordered release + // fails with this sentence rather than as a flake. + assert!( + !connection.read_in_flight("invite"), + "the abort precedes the release of the mint" + ); - client.send(r#"["CLOSE","invite"]"#).await; - tokio::time::sleep(Duration::from_millis(50)).await; gate.add_permits(1); + reading + .await + .expect("the handler finishes once the mint is released"); // The capture emits the invite event for a subscription the client has closed // (`subscribe-message-handler.ts:111`), and that stays. - let first = client.expect_frame().await; + let first = frames.try_recv().expect("the invite event frame"); assert!(first.starts_with(r#"["EVENT","invite","#), "{first}"); let event: serde_json::Value = serde_json::from_str( @@ -1258,10 +1317,10 @@ async fn a_close_during_the_invite_mint_leaves_the_event_but_not_the_read() { // Nothing else: the stored read was cancelled with the subscription, so no stored // event and no `EOSE`. assert!( - client.next_message().await.is_none(), + frames.try_recv().is_err(), "a closed subscription streams nothing" ); - assert_eq!(server.fixture.invites.minted().len(), 1); + assert_eq!(fixture.invites.minted().len(), 1); } #[tokio::test] From 6bac1ac7bfd30bfa30a56e65c7c7fb6c93a92ec9 Mon Sep 17 00:00:00 2001 From: chappie-daemon Date: Sat, 24 Oct 2026 21:48:18 -0400 Subject: [PATCH 2/5] docs(relay): say what the abort guard in the CLOSE test actually catches (#963) --- crates/nostream-relay/tests/handlers.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/nostream-relay/tests/handlers.rs b/crates/nostream-relay/tests/handlers.rs index 584397dc..ca69207e 100644 --- a/crates/nostream-relay/tests/handlers.rs +++ b/crates/nostream-relay/tests/handlers.rs @@ -1288,8 +1288,9 @@ async fn a_close_during_the_invite_mint_leaves_the_event_but_not_the_read() { &context, ); - // The abort precedes the release below, so a reintroduced timer or a reordered release - // fails with this sentence rather than as a flake. + // Nothing between the abort above and the release below suspends, so a read still + // registered here means the `unsubscribe::handle` call was dropped or moved behind this + // assertion; whether the cancellation reaches the read is what the frames below decide. assert!( !connection.read_in_flight("invite"), "the abort precedes the release of the mint" From 4f259ce1ef3a22cb248d7c785159a1a441893877 Mon Sep 17 00:00:00 2001 From: chappie-daemon Date: Sat, 24 Oct 2026 22:07:06 -0400 Subject: [PATCH 3/5] test(relay): spawn subscribe::begin in the CLOSE-during-mint test (#963) --- crates/nostream-relay/tests/handlers.rs | 46 ++++++++++--------------- 1 file changed, 19 insertions(+), 27 deletions(-) diff --git a/crates/nostream-relay/tests/handlers.rs b/crates/nostream-relay/tests/handlers.rs index ca69207e..caa40a90 100644 --- a/crates/nostream-relay/tests/handlers.rs +++ b/crates/nostream-relay/tests/handlers.rs @@ -1237,33 +1237,26 @@ async fn a_close_during_the_invite_mint_leaves_the_event_but_not_the_read() { sink, )); - let reading = tokio::spawn({ - let context = Arc::clone(&context); - - async move { - nostream_relay::handlers::subscribe::handle( - SubscribeMessage { - subscription_id: "invite".to_owned(), - filters: vec![Filter { - kinds: Some(vec![nostream_core::Kind(28_935), nostream_core::Kind(1)]), - ..Default::default() - }], - }, - &context, - ) - .await; - } - }); - - // The read is registered before the mint's only `await`, so it exists by the time the - // mint is held. - for _ in 0..100 { - if connection.read_in_flight("invite") { - break; - } + // The seam runs the handler's prefix on this task, so the subscription and its read are + // registered by the time `begin` returns; only the mint and the read are spawned. + let reading = tokio::spawn( + nostream_relay::handlers::subscribe::begin( + SubscribeMessage { + subscription_id: "invite".to_owned(), + filters: vec![Filter { + kinds: Some(vec![nostream_core::Kind(28_935), nostream_core::Kind(1)]), + ..Default::default() + }], + }, + &context, + ) + .expect("the subscription is accepted"), + ); - tokio::task::yield_now().await; - } + assert!( + connection.read_in_flight("invite"), + "the read is registered before the mint starts" + ); // The mint's one `await` is held, which is the window a `CLOSE` lands in. for _ in 0..100 { @@ -1279,7 +1272,6 @@ async fn a_close_during_the_invite_mint_leaves_the_event_but_not_the_read() { 1, "the invite mint is in flight" ); - assert!(connection.read_in_flight("invite"), "the read is in flight"); nostream_relay::handlers::unsubscribe::handle( UnsubscribeMessage { From 3c1c824a47106936da5c7076bb369b07e797d1dc Mon Sep 17 00:00:00 2001 From: chappie-daemon Date: Sat, 24 Oct 2026 22:18:01 -0400 Subject: [PATCH 4/5] docs(relay): name subscribe::begin in the changeset (#963) --- .changeset/close-mint-sync.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/close-mint-sync.md b/.changeset/close-mint-sync.md index 1b1c12d1..a349d688 100644 --- a/.changeset/close-mint-sync.md +++ b/.changeset/close-mint-sync.md @@ -5,5 +5,5 @@ fix(relay): order the CLOSE-during-mint test's release by program order (#963) `a_close_during_the_invite_mint_leaves_the_event_but_not_the_read` no longer orders its gate release against the `CLOSE` with a 50 ms sleep. It owns its socket state and -drives `subscribe::handle` and the synchronous `unsubscribe::handle` directly, so the +drives `subscribe::begin` and the synchronous `unsubscribe::handle` directly, so the abort is a program-order predecessor of the release. No production code changes. From fa1f09bd46b572e39cca1f2f938e6f4c70bf344e Mon Sep 17 00:00:00 2001 From: chappie-daemon Date: Sun, 25 Oct 2026 02:27:20 +0000 Subject: [PATCH 5/5] docs(relay): say what the CLOSE test owns, not that it owns a socket (#963) The changeset claimed the test owns its socket state. It has not had one since the rewrite: the test owns a Connection and a FrameSink, and subscribe::begin's prefix registers the subscription and its read on the test's own task. That clause is what tells a reader why the release no longer needs a timer. Wording supplied by the operator: two rounds closed only half of the previous finding, and a third lens pass over one sentence costs more than it decides. --- .changeset/close-mint-sync.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.changeset/close-mint-sync.md b/.changeset/close-mint-sync.md index a349d688..171f39e8 100644 --- a/.changeset/close-mint-sync.md +++ b/.changeset/close-mint-sync.md @@ -4,6 +4,8 @@ fix(relay): order the CLOSE-during-mint test's release by program order (#963) `a_close_during_the_invite_mint_leaves_the_event_but_not_the_read` no longer orders -its gate release against the `CLOSE` with a 50 ms sleep. It owns its socket state and -drives `subscribe::begin` and the synchronous `unsubscribe::handle` directly, so the -abort is a program-order predecessor of the release. No production code changes. +its gate release against the `CLOSE` with a 50 ms sleep. It owns a `Connection` and a +`FrameSink` rather than a socket, and drives `subscribe::begin` — whose prefix registers +the subscription and its read on the test's own task — plus the synchronous +`unsubscribe::handle`, so the abort is a program-order predecessor of the release. No +production code changes.