Skip to content

gossip-sqlite: cap relays per public key (MAX_RELAYS_PER_PK) - #1512

Open
ZeroPointSynergy wants to merge 3 commits into
nostrdevkit:masterfrom
ZeroPointSynergy:fix/issue-1433-sqlite-gossip-cap
Open

ZeroPointSynergy wants to merge 3 commits into
nostrdevkit:masterfrom
ZeroPointSynergy:fix/issue-1433-sqlite-gossip-cap

Conversation

@ZeroPointSynergy

Copy link
Copy Markdown

Fixes #1433 (SQLite backend part).

Summary

The in-memory gossip backend already caps distinct relays per public key at MAX_RELAYS_PER_PK (+1, mirroring ). The SQLite backend had no such cap in , allowing unbounded growth of rows via a connected relay.

This change mirrors the in-memory cap in the SQLite backend: skip inserting a new relay once the per-key count exceeds MAX_RELAYS_PER_PK, and propagate DB errors instead of masking them.

Verification

  • cargo test -p nostr-gossip-sqlite: 9/9 pass
  • adversarial review applied (off-by-one + error masking fixed)

@codecov

codecov Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.64706% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
gossip/nostr-gossip-sqlite/src/store.rs 97.64% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

Cover the MAX_RELAYS_PER_PK cap added to `update_relay_per_user`, which
codecov reported as uncovered (patch coverage 81.25% < 81.76% target),
failing the codecov/patch check.

The test asserts, against the SQLite backend:
- a public key stores at most MAX_RELAYS_PER_PK + 1 distinct relays,
  mirroring the in-memory backend;
- further distinct relays are skipped instead of growing the table;
- already-known relays keep being updated once the key is capped, so the
  cap does not suppress `received_events`/flags updates;
- the cap is scoped per public key, so a capped key cannot starve others.

Verified by mutation testing: disabling the cap (11 vs 8 rows), changing
`>` to `>=` (7 vs 8), dropping the `exists` check (received_events 1 vs 2)
and unscoping the COUNT query (second key 0 vs 8) each make the test fail.
@ZeroPointSynergy

Copy link
Copy Markdown
Author

Added a test to cover the relay cap, since the codecov patch check was failing (81.25% vs 81.76% target — the new SELECT COUNT(*) and the early return Ok(()) were uncovered).

test_relay_cap_per_public_key asserts, against the SQLite backend:

  • a public key stores at most MAX_RELAYS_PER_PK + 1 distinct relays (mirroring the in-memory backend, which accepts while relays_len <= MAX_RELAYS_PER_PK);
  • further distinct relays are skipped rather than growing relays_per_user;
  • already-known relays keep being updated once the key is capped (a second event from the same relay increments received_events), so the cap does not suppress updates;
  • the cap is scoped per public key, so a capped key cannot starve another key's relay list.

I mutation-tested the test rather than trusting a green run — each of these broken implementations makes it fail:

Mutation Result
cap condition removed 11 rows vs 8 expected
> changed to >= 7 rows vs 8 expected
if !exists wrapper dropped received_events 1 vs 2 expected
COUNT query not scoped to the key second key 0 rows vs 8 expected

cargo test -p nostr-gossip-sqlite: 10/10 pass. cargo fmt --check and cargo clippy --all-targets -- -D warnings are clean on the pinned 1.85.0 toolchain. I also let cargo fmt fix a formatting drift in the exists query introduced by the previous commit.

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.

gossip store has no resource bounds: unbounded relay-hint accumulation (memory + SQLite) via connected relay

1 participant