mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
refactor(relay): retire overrides supplied by the upgraded SDK
The upgraded SDK defaults now match our compatibility settings: 1,200 query starts and 6,000 frames per minute, with unlimited established connections unless explicitly configured. Remove the duplicate constants and unlimited-permit workaround while preserving operator connection caps. NIP-77 continuations now use the SDK's separate finite allowance. Retain larger event and subscription-state allowances, authentication and other resource limits. Outbound query pacing and rate-limit recovery still serve peers with older software or stricter policies, so retain them and clarify their historical-limit rationale. Update limit documentation to identify the SDK defaults instead of obsolete temporary overrides. Replace the removed helper's implementation-shaped checks with protocol coverage that holds more than 128 connections open and accepts more than 120 query starts. Existing tests retain burst compatibility and excessive frame rejection checks. All new waits use observable replies and bounded deadlines; no sleeps are introduced. Validation: full locked workspace tests, all-target workspace Clippy with warnings denied, formatting and diff checks pass with the separate startup reconstruction race fix. The effective configured limits remain unchanged; future compatible SDK default changes are guarded by the protocol tests. Assisted-by: GPT-6
This commit is contained in:
@@ -31,17 +31,17 @@ operator configurable; other dependency hardening is pinned in the builder.
|
|||||||
These limits prevent individual connections from overwhelming the relay.
|
These limits prevent individual connections from overwhelming the relay.
|
||||||
The relay advertises the standard `max_subscriptions`, `max_limit`,
|
The relay advertises the standard `max_subscriptions`, `max_limit`,
|
||||||
`default_limit`, `max_message_length`, and `max_subid_length` NIP-11 fields.
|
`default_limit`, `max_message_length`, and `max_subid_length` NIP-11 fields.
|
||||||
rust-nostr 0.45 also enforces fixed ngit-grasp-selected defaults of 1,200 queries,
|
rust-nostr 0.45.3 supplies defaults of 1,200 query starts and 6,000 WebSocket
|
||||||
30 authentication events, and 6,000 WebSocket messages per minute; 20 filters
|
messages per minute. ngit-grasp additionally configures
|
||||||
|
30 authentication events per minute; 20 filters
|
||||||
per REQ; 5 MiB subscription state (raised from rust-nostr's 1 MiB default so
|
per REQ; 5 MiB subscription state (raised from rust-nostr's 1 MiB default so
|
||||||
repository-scale persistent live filters remain admitted); 10 active
|
repository-scale persistent live filters remain admitted); 10 active
|
||||||
negentropy sessions and 50,000 negentropy items per connection; a 5 MiB
|
negentropy sessions and 50,000 negentropy items per connection; a 5 MiB
|
||||||
WebSocket message; and a 10-second handshake deadline. NIP-11 has no standard
|
WebSocket message; and a 10-second handshake deadline. NIP-11 has no standard
|
||||||
fields for most of those controls.
|
fields for most of those controls.
|
||||||
The query allowance is a temporary 10× override of rust-nostr's newly added
|
NIP-77 continuations use a separate 1,200/minute allowance and remain subject
|
||||||
120/minute default because NIP-77 currently charges each SDK-managed `NEG-MSG`
|
to the connection-wide message cap. The older SDK compatibility overrides
|
||||||
continuation separately; it must be reviewed when upstream revises that
|
are no longer needed.
|
||||||
accounting.
|
|
||||||
|
|
||||||
### Per-IP Connection Monitoring
|
### Per-IP Connection Monitoring
|
||||||
|
|
||||||
|
|||||||
@@ -92,14 +92,16 @@ rust-nostr reports its configured ceiling. The learned ceiling never rises in
|
|||||||
the session and resets on reconnect. A one-filter refusal is not recoverable
|
the session and resets on reconnect. A one-filter refusal is not recoverable
|
||||||
by regrouping and uses the ordinary long `filter_incompatible` policy pause.
|
by regrouping and uses the ordinary long `filter_incompatible` policy pause.
|
||||||
|
|
||||||
### Our own embedded relay (nostr-sdk `LocalRelay`, 0.45.0)
|
### Our own embedded relay (nostr-sdk `LocalRelay`, 0.45.3)
|
||||||
|
|
||||||
- `max_reqs` = 500, enforced for REQ only (`src/nostr/builder.rs`).
|
- `max_reqs` = 500, enforced for REQ only (`src/nostr/builder.rs`).
|
||||||
- Negentropy: **no concurrency limit at all** (upstream `TODO`), 60000-byte
|
- Negentropy: ngit-grasp configures at most 10 retained sessions and 50,000
|
||||||
frame limit per NEG message.
|
retained items per connection, with a 60,000-byte protocol frame limit.
|
||||||
|
Continuations have a separate allowance from query starts. LMDB scans run
|
||||||
|
on blocking workers, so a scan does not occupy an async runtime worker.
|
||||||
- 20 filters per REQ by default; no limit on tag values per filter.
|
- 20 filters per REQ by default; no limit on tag values per filter.
|
||||||
- Query result limits (verified 2026-08-06 against the published
|
- Query result limits (verified against the published `nostr-sdk-0.45.3`
|
||||||
`nostr-sdk-0.45.0` crate source, `src/local_relay/local/inner.rs`):
|
crate source, `src/local_relay/local/inner.rs`):
|
||||||
enforced **per filter, not per REQ**. A filter without a `limit` is given
|
enforced **per filter, not per REQ**. A filter without a `limit` is given
|
||||||
`default_filter_limit` (500); the effective limit is then clamped to
|
`default_filter_limit` (500); the effective limit is then clamped to
|
||||||
`min(limit, max_filter_limit, max_query_results)` (defaults: no
|
`min(limit, max_filter_limit, max_query_results)` (defaults: no
|
||||||
@@ -191,18 +193,13 @@ strfry and Ditto have no native limiter at all, deferring to deployment
|
|||||||
infrastructure. The full survey with citations is preserved in this
|
infrastructure. The full survey with citations is preserved in this
|
||||||
file's history (commit `9723ff4`).
|
file's history (commit `9723ff4`).
|
||||||
|
|
||||||
The 120-query allowance is a newly enabled rust-nostr 0.45 LocalRelay default,
|
rust-nostr 0.45.3 supplies the 1,200-query-start allowance that ngit-grasp
|
||||||
not a floor established by that survey. It is unusually restrictive: none of
|
previously selected with a compatibility override. NIP-77 continuations use
|
||||||
the other audited implementations enables an equivalent query-specific,
|
an independent 1,200/minute allowance, while all frames still share the
|
||||||
per-connection default. A finite limit is still useful as one layer of DoS
|
6,000/minute message cap. A finite per-connection limit remains one DoS layer;
|
||||||
protection, but a small per-connection bucket is not sufficient protection by
|
per-IP admission and global resource bounds address clients multiplying
|
||||||
itself because a hostile client can multiply connections; per-IP admission and
|
connections. Outbound pacing remains necessary for peers with older SDKs or
|
||||||
global resource bounds address that threat more directly. rust-nostr also
|
stricter operator-selected limits.
|
||||||
charges SDK-managed NIP-77 `NEG-MSG` continuation frames to this same bucket,
|
|
||||||
so one application-started reconciliation can consume multiple query tokens.
|
|
||||||
ngit-grasp temporarily overrides that default to 1,200 queries/minute while
|
|
||||||
retaining a finite per-connection backstop. Re-evaluate the 10× value after
|
|
||||||
upstream separates or otherwise revises NIP-77 continuation accounting.
|
|
||||||
|
|
||||||
NIP-11 describes hard relay limitations, not rate-limit algorithms. The
|
NIP-11 describes hard relay limitations, not rate-limit algorithms. The
|
||||||
standard fields relevant here are `max_limit` and
|
standard fields relevant here are `max_limit` and
|
||||||
@@ -484,7 +481,7 @@ So concurrency is not a free scaling axis; it is the residual of the ledger:
|
|||||||
first rejects locally queued starts for the existing 65-second cooldown so
|
first rejects locally queued starts for the existing 65-second cooldown so
|
||||||
their incomplete work can be re-derived in priority order, then learns a
|
their incomplete work can be re-derived in priority order, then learns a
|
||||||
shared query-start interval: 600 ms initially (100 starts/minute, below the
|
shared query-start interval: 600 ms initially (100 starts/minute, below the
|
||||||
rust-nostr 120/minute default), doubling on a later rate-limit episode up to
|
older rust-nostr 120/minute limit), doubling on a later rate-limit episode up to
|
||||||
10 seconds. Live, transient REQ, exact-ID fetch and NIP-77 round starts all
|
10 seconds. Live, transient REQ, exact-ID fetch and NIP-77 round starts all
|
||||||
pass through that reactive pacer in addition to background work retaining
|
pass through that reactive pacer in addition to background work retaining
|
||||||
its proactive spacing. Relays that never report a query rate limit still
|
its proactive spacing. Relays that never report a query rate limit still
|
||||||
|
|||||||
@@ -13,9 +13,10 @@ production admission policy.
|
|||||||
| Results per filter | 500 | `NGIT_RELAY_FILTER_LIMIT` | `max_limit`, `default_limit` |
|
| Results per filter | 500 | `NGIT_RELAY_FILTER_LIMIT` | `max_limit`, `default_limit` |
|
||||||
| Serialized event size | 192 KiB | `NGIT_RELAY_MAX_EVENT_SIZE_BYTES` | No equivalent field |
|
| Serialized event size | 192 KiB | `NGIT_RELAY_MAX_EVENT_SIZE_BYTES` | No equivalent field |
|
||||||
| Event writes per minute | 60 | Fixed | No standard field |
|
| Event writes per minute | 60 | Fixed | No standard field |
|
||||||
| Queries per minute | 1,200 | Fixed temporary override | No standard field |
|
| Query starts per minute | 1,200 | Upstream default | No standard field |
|
||||||
|
| NIP-77 continuations per minute | 1,200 | Upstream default | No standard field |
|
||||||
| Authentication events per minute | 30 | Fixed | No standard field |
|
| Authentication events per minute | 30 | Fixed | No standard field |
|
||||||
| WebSocket messages per minute | 6,000 | Fixed | No standard field |
|
| WebSocket messages per minute | 6,000 | Upstream default | No standard field |
|
||||||
| WebSocket message size | 5 MiB | Fixed | `max_message_length` |
|
| WebSocket message size | 5 MiB | Fixed | `max_message_length` |
|
||||||
| Handshake deadline | 10 seconds | Fixed | No standard field |
|
| Handshake deadline | 10 seconds | Fixed | No standard field |
|
||||||
| Subscription-ID length | 250 bytes | Fixed | `max_subid_length` |
|
| Subscription-ID length | 250 bytes | Fixed | `max_subid_length` |
|
||||||
@@ -43,13 +44,11 @@ matches the maximum admitted WebSocket message, and provides about four times
|
|||||||
the observed working-set headroom. NIP-11 has no field for this cumulative
|
the observed working-set headroom. NIP-11 has no field for this cumulative
|
||||||
byte limit, so clients cannot negotiate it.
|
byte limit, so clients cannot negotiate it.
|
||||||
|
|
||||||
The 1,200-query allowance temporarily overrides rust-nostr 0.45's newly
|
The SDK now supplies the 1,200-query-start and 6,000-message allowances that
|
||||||
introduced 120/minute default. A finite per-connection bound remains one useful
|
ngit-grasp previously overrode locally. NIP-77 `NEG-MSG` continuations use a
|
||||||
DoS layer, but the upstream default is unusually restrictive for sync clients
|
separate 1,200/minute allowance and still count against the connection-wide
|
||||||
and currently counts every SDK-managed NIP-77 `NEG-MSG` continuation as another
|
message limit. These finite per-connection quotas do not replace per-IP
|
||||||
query. Review this 10× override after upstream changes that accounting; a small
|
admission or global resource bounds because clients can multiply connections.
|
||||||
per-connection quota is not a substitute for per-IP admission or global
|
|
||||||
resource bounds because clients can multiply connections.
|
|
||||||
|
|
||||||
## Client adaptation
|
## Client adaptation
|
||||||
|
|
||||||
|
|||||||
+5
-47
@@ -36,24 +36,6 @@ use crate::private::PrivateAccess;
|
|||||||
use crate::purgatory::promotion_hooks::NostrPurgatoryPromotionHooks;
|
use crate::purgatory::promotion_hooks::NostrPurgatoryPromotionHooks;
|
||||||
use crate::sync::rejected_index::RejectedEventsIndex;
|
use crate::sync::rejected_index::RejectedEventsIndex;
|
||||||
|
|
||||||
/// Connection-wide frame allowance layered above the operation-specific
|
|
||||||
/// write, query, and authentication quotas.
|
|
||||||
///
|
|
||||||
/// rust-nostr's 300/minute default closed production browser clients during
|
|
||||||
/// ordinary bursty subscription churn. One hundred frames per second preserves a
|
|
||||||
/// bounded catch-all for malformed/non-operation traffic while leaving the
|
|
||||||
/// tighter operation quotas in charge of valid protocol work.
|
|
||||||
const CLIENT_MESSAGES_PER_MINUTE: u32 = 6_000;
|
|
||||||
|
|
||||||
/// Temporary compatibility override for rust-nostr 0.45's newly introduced
|
|
||||||
/// 120-query-per-minute default.
|
|
||||||
///
|
|
||||||
/// A finite per-connection bound remains useful as one DoS layer, but 120 is
|
|
||||||
/// too restrictive for legitimate sync and NIP-77 currently charges every
|
|
||||||
/// SDK-managed `NEG-MSG` continuation against the same allowance. Re-evaluate
|
|
||||||
/// this 10× value after upstream separates or otherwise revises that accounting.
|
|
||||||
const CLIENT_QUERIES_PER_MINUTE: u32 = 1_200;
|
|
||||||
|
|
||||||
/// Cumulative serialized REQ state retained for one client connection.
|
/// Cumulative serialized REQ state retained for one client connection.
|
||||||
///
|
///
|
||||||
/// Repository sync legitimately keeps multiple byte-budgeted live filters
|
/// Repository sync legitimately keeps multiple byte-budgeted live filters
|
||||||
@@ -1145,9 +1127,7 @@ pub async fn create_relay(
|
|||||||
max_reqs: config.relay_max_subscriptions,
|
max_reqs: config.relay_max_subscriptions,
|
||||||
notes_per_minute: 60,
|
notes_per_minute: 60,
|
||||||
})
|
})
|
||||||
.queries_per_minute(CLIENT_QUERIES_PER_MINUTE)
|
|
||||||
.auth_events_per_minute(30)
|
.auth_events_per_minute(30)
|
||||||
.messages_per_minute(CLIENT_MESSAGES_PER_MINUTE)
|
|
||||||
.max_websocket_message_size(5 * 1024 * 1024)
|
.max_websocket_message_size(5 * 1024 * 1024)
|
||||||
.max_event_size(config.relay_max_event_size_bytes)
|
.max_event_size(config.relay_max_event_size_bytes)
|
||||||
.websocket_handshake_timeout(Duration::from_secs(10))
|
.websocket_handshake_timeout(Duration::from_secs(10))
|
||||||
@@ -1160,11 +1140,11 @@ pub async fn create_relay(
|
|||||||
.max_query_results(config.relay_filter_limit)
|
.max_query_results(config.relay_filter_limit)
|
||||||
.default_filter_limit(config.relay_filter_limit);
|
.default_filter_limit(config.relay_filter_limit);
|
||||||
|
|
||||||
// `LocalRelayBuilder` otherwise applies rust-nostr's own finite default.
|
// The SDK defaults to unlimited established connections. Preserve an
|
||||||
// In ngit-grasp, an unset limit deliberately delegates admission control to
|
// explicit operator limit when one is configured.
|
||||||
// the OS and surrounding infrastructure, as documented by every config
|
if let Some(limit) = config.max_connections {
|
||||||
// surface. Explicit operator limits remain exact.
|
builder = builder.max_connections(limit);
|
||||||
builder = builder.max_connections(effective_max_connections(config.max_connections));
|
}
|
||||||
|
|
||||||
let relay = builder.build();
|
let relay = builder.build();
|
||||||
|
|
||||||
@@ -1196,25 +1176,3 @@ pub async fn create_relay(
|
|||||||
},
|
},
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
fn effective_max_connections(configured: Option<usize>) -> usize {
|
|
||||||
configured.unwrap_or(tokio::sync::Semaphore::MAX_PERMITS)
|
|
||||||
}
|
|
||||||
|
|
||||||
#[cfg(test)]
|
|
||||||
mod connection_limit_tests {
|
|
||||||
use super::effective_max_connections;
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn unset_connection_limit_overrides_dependency_default() {
|
|
||||||
assert_eq!(
|
|
||||||
effective_max_connections(None),
|
|
||||||
tokio::sync::Semaphore::MAX_PERMITS
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn configured_connection_limit_remains_exact() {
|
|
||||||
assert_eq!(effective_max_connections(Some(2)), 2);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|||||||
@@ -45,8 +45,8 @@ const SUBSCRIPTION_RESERVED_MARGIN: usize = 2;
|
|||||||
/// allowance is exhausted.
|
/// allowance is exhausted.
|
||||||
///
|
///
|
||||||
/// NIP-11 cannot advertise query-rate windows. One start every 600 ms is 100
|
/// NIP-11 cannot advertise query-rate windows. One start every 600 ms is 100
|
||||||
/// starts/minute, leaving operational slack below rust-nostr 0.45's newly
|
/// starts/minute, leaving operational slack below the 120/minute limit used
|
||||||
/// introduced 120/minute LocalRelay default. This is deliberately reactive:
|
/// by older rust-nostr relays. This is deliberately reactive:
|
||||||
/// proactively pacing non-urgent history needs request-class priority so live
|
/// proactively pacing non-urgent history needs request-class priority so live
|
||||||
/// coverage is not delayed, and belongs in a separate scheduler change.
|
/// coverage is not delayed, and belongs in a separate scheduler change.
|
||||||
const QUERY_PACING_INITIAL_INTERVAL: Duration = Duration::from_millis(600);
|
const QUERY_PACING_INITIAL_INTERVAL: Duration = Duration::from_millis(600);
|
||||||
|
|||||||
@@ -82,3 +82,51 @@ async fn catch_all_limit_still_closes_excessive_frame_burst() {
|
|||||||
);
|
);
|
||||||
relay.stop().await;
|
relay.stop().await;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn sdk_query_allowance_accepts_more_than_120_starts() {
|
||||||
|
let relay = TestRelay::start().await;
|
||||||
|
tokio::time::timeout(Duration::from_secs(15), async {
|
||||||
|
let (mut stream, _) = tokio_tungstenite::connect_async(relay.url()).await.unwrap();
|
||||||
|
for _ in 0..121 {
|
||||||
|
stream
|
||||||
|
.send(Message::Text(
|
||||||
|
r#"["REQ","probe",{"kinds":[1],"limit":0}]"#.into(),
|
||||||
|
))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let response = stream.next().await.unwrap().unwrap().into_text().unwrap();
|
||||||
|
assert_eq!(response, r#"["EOSE","probe"]"#);
|
||||||
|
}
|
||||||
|
stream.close(None).await.unwrap();
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.expect("query burst exceeded deadline");
|
||||||
|
relay.stop().await;
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn sdk_default_allows_more_than_128_established_connections() {
|
||||||
|
let relay = TestRelay::start().await;
|
||||||
|
tokio::time::timeout(Duration::from_secs(30), async {
|
||||||
|
let mut connections = Vec::new();
|
||||||
|
for _ in 0..129 {
|
||||||
|
let (mut stream, _) = tokio_tungstenite::connect_async(relay.url()).await.unwrap();
|
||||||
|
stream
|
||||||
|
.send(Message::Text(
|
||||||
|
r#"["REQ","probe",{"kinds":[1],"limit":0}]"#.into(),
|
||||||
|
))
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let response = stream.next().await.unwrap().unwrap().into_text().unwrap();
|
||||||
|
assert_eq!(response, r#"["EOSE","probe"]"#);
|
||||||
|
connections.push(stream);
|
||||||
|
}
|
||||||
|
for mut stream in connections {
|
||||||
|
stream.close(None).await.unwrap();
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.expect("connection admission exceeded deadline");
|
||||||
|
relay.stop().await;
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user