Skip to content

Refuse large commits at relays of unknown limit; settle exposure once (#2332) - #3396

Open
erskingardner wants to merge 5 commits into
masterfrom
2332-commit-floor
Open

erskingardner wants to merge 5 commits into
masterfrom
2332-commit-floor

Conversation

@erskingardner

@erskingardner erskingardner commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

Problem

A group change over 64 KiB sent to a relay whose event limit MDK doesn't know can close the relay's socket mid-frame. The send then reads as "possibly seen", and the commit stays pending for good: the group can't make another change. A relay that answered rate-limited: or auth-required: had received the commit, yet a later refusal there could still roll it back, which breaks the rule that a commit members may hold never rolls back (#1577).

Fix

A commit over 64 KiB is refused, before anything is sent, at a relay whose limit is unknown. The session asks before marking any target attempted, and the commit entry point checks again. A relay the device can't connect to holds the commit for a retry instead of refusing it, so an offline device never gets a false "too large". Welcomes and application messages are unchanged. Exposure is now settled in one transition. A target counts as unexposed only after a proven pre-send failure, or a definitive refusal of the attempt in hand. Retryable relay answers and interrupted attempts count as exposure, and so do rows stored by the previous release.

Verification

  • Local: cargo fmt --all --check; just tracing-audit; just naming-gate; cargo clippy --workspace --all-targets --all-features --locked -- -D warnings; scoped default clippy for cgka-traits, marmot-session, transport-nostr-adapter, transport-nostr, marmot-app
  • Local: full suites for cgka-traits (188), transport-nostr-adapter (248), transport-nostr (160), marmot-session with test-policy-overrides (239); storage-sqlite fanout/outbound, cgka-engine hydration_quarantine, targeted marmot-app tests (90)
  • Local: end-to-end smoke (the two simulator tests, the two wn-cli tests) and just simulator-smoke (509 + 4 tests), all at 8448d53
  • Review: Codex (gpt-6.1-sol) and Cursor (grok-4.7-high-fast) recommend merge at 8448d53 (final round, after merging master with Publish the account read state on the worker (#3273) #3315)

Part of #2332 (the size floor, decisions D8/D9, and the single exposure transition). The read-back, Stuck state, rollback rule and host surface follow in later PRs.

Notes for review

Press hardest on two things:

  • The single exposure transition (OutboundFanout::settle_exposure and fold_stored_exposure in crates/traits/src/transport_adapter.rs). The bit is never cleared. It stays clear only for a proven pre-send failure, or a definitive refusal, of the attempt this in-memory value started (HeldAttempts, never serialized). A result recorded on an Attempting loaded from disk sets it, and so does a stored status that records exposure, before anything replaces that status.
  • R5, the next PR's rollback rule, trusts this bit. A pending commit whose targets all have a clear bit and ended in a definitive refusal, a pre-send failure, or no contact rolls back without any read about the event. Any path that leaves a bit clear wrongly would roll back a commit a member may hold.

Interim effect, intended: until the read-back lands, a commit answered rate-limited: or auth-required: and then refused everywhere stays pending. It gets no read-back and no Stuck state yet.

Rows stored before this change: a RetryableUnavailable with no pre-send proof is treated as exposed. The previous release could store a category-less one after an auth-required: answer, so its category can't separate the cases. The cost falls only on fanouts pending across the upgrade: if every relay then refuses definitively, they stay pending instead of rolling back.

🤖 Generated with Claude Code

erskingardner and others added 4 commits November 13, 2026 14:03
…#2332)

A commit over 64 KiB sent to a relay whose event limit is unknown can
close the socket mid-frame. The target then reads as possibly exposed
and the commit stays pending for good. Commits now meet a 64 KiB floor
at such relays: the session asks the adapter before marking a target
attempted, and the commit entry point applies the floor again. A relay
the device cannot connect to holds the commit as a pre-send retry
instead of refusing it.

Exposure is settled in one transition out of Attempting. The bit stays
clear only for a proven pre-send failure or a definitive refusal of
the current attempt, which the fanout tracks in memory; any result on
an attempt loaded from disk sets it. A relay that answered
rate-limited: or auth-required: received the event, so a later refusal
there reports possible exposure.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
A row stored before pre-send proof existed records a relay's
rate-limited: answer as RetryableUnavailable with a clear bit, and a
row stored before the bit existed may be PossiblyExposed with a clear
bit. Re-marking either target, or recording a pre-mark refusal on it,
dropped that evidence, so a later definitive refusal could end the
target Failed and unexposed. Both the single transition and a new
attempt now set the bit first.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
A stored RetryableUnavailable row holding a rate-limited: answer with no
pre-send proof and a clear bit, as the previous release wrote it, stays
pending after both a size refusal before marking and a definitive
refusal of the next retry.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@coderabbitai

coderabbitai Bot commented Nov 13, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

Next included review available in 43 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 81 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Team
  • Run ID: 433522c1-a54d-4f55-be0c-7eb54c98bd28
Commits

Reviewing files that changed from the base of the PR and between 2e6a15c and 8448d53.

Files selected for processing: 22

  • CHANGELOG.md
  • crates/marmot-app/src/client/sync/queued_outbound_schedule_tests.rs
  • crates/marmot-app/src/error_kind_table.rs
  • crates/marmot-app/src/moderation_reports/tests.rs
  • crates/marmot-app/src/relay_event_limits.rs
  • crates/marmot-app/src/runtime/tests.rs
  • crates/marmot-app/src/tests.rs
  • crates/marmot-app/src/unlinked_publish_gate.rs
  • crates/marmot-session/src/runtime.rs
  • crates/marmot-session/tests/runtime.rs
  • crates/marmot-session/tests/runtime/commit_floor.rs
  • crates/marmot-session/tests/runtime/frozen_fanout.rs
  • crates/marmot-session/tests/support/nostr_stack.rs
  • crates/traits/src/transport_adapter.rs
  • crates/transport-nostr-adapter/src/lib.rs
  • crates/transport-nostr-adapter/src/sdk_client.rs
  • crates/transport-nostr/src/known_limits.rs
  • crates/transport-nostr/src/lib.rs
  • crates/transport-nostr/src/publish_accounting_tests.rs
  • crates/transport-nostr/src/safety.rs
  • crates/transport-nostr/src/test_support.rs
  • crates/transport-nostr/src/tests.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@erskingardner
erskingardner added this pull request to the merge queue Nov 13, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Nov 13, 2026
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.

1 participant