fix: node robustness — streaming overflow DoS, spammer panics, metrics accuracy#161
Open
PrazwalR wants to merge 6 commits into
Open
fix: node robustness — streaming overflow DoS, spammer panics, metrics accuracy#161PrazwalR wants to merge 6 commits into
PrazwalR wants to merge 6 commits into
Conversation
A gossiped Fin proposal part carries an unauthenticated `sequence` wire field that StreamState::insert used directly as `expected_messages = sequence as usize + 1`. A peer sending sequence = u64::MAX overflowed the +1: a panic in overflow-checked/debug builds (one message crashes the node) and a silent wrap in release that leaves the stream permanently incomplete, squatting a per-peer slot until the 60s age sweep. Reject any Fin whose sequence is outside [0, MAX_MESSAGES_PER_STREAM - 1] via a new FinSequenceOutOfRange result that evicts the stream. Adds a regression test (u64::MAX and at-cap) and a boundary test (max in-range sequence still completes). Fixes circlefin#144 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
partition_exponential's guard computed `1 << (num_generators - 1)`, which overflows its own shift once the exponent reaches the platform word size: a panic in debug, and in release a wrapped shift that slips past the guard and builds non-monotonic, overlapping account ranges. Check the shift width first (shift >= usize::BITS) so the case returns the intended graceful error. Adds a regression test. Fixes circlefin#137 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
insert_decided_block's timer spanned insert_certificate, which records its own write time, so the certificate insert was counted twice in the aggregated write_time metric. Scope the block-write timer around the certificate call and attribute the shared commit once. Fixes circlefin#142 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The versioned codec attempted a full protobuf decode before falling back to the versioned path, but every current message carries the 0x01 version prefix and always fails that first attempt. Add a first-byte version pre-check: 0x01 can never begin a valid protobuf message (min field tag is 0x08), so a leading version byte unambiguously selects the versioned path. Fixes circlefin#131 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
record_proposal performed a warn! side effect on equivocation. Return #[must_use] bool (true = recorded, false = equivocation) instead and log at the two call sites with their own context. Updates the unit test to assert on the return value. Fixes circlefin#134 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
catch_up_scan propagated one received_at (the reconnect moment) to every missed block, collapsing their latencies onto a single instant. Timestamp each block with timestamp_now() as it is fetched. Fixes circlefin#128 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a batch of self-contained correctness bugs. One commit per issue; all changes build, test, clippy
-D warnings, and rustfmt clean.Fixes
Fin.sequencewas used asexpected_messages = sequence as usize + 1with no bound. A single gossipedFinwithsequence = u64::MAXpanics overflow-checked builds / wedges the stream in release. Now rejected unless in[0, MAX_MESSAGES_PER_STREAM-1].1 << (num_generators - 1)overflows its own shift; guard shift width first → graceful error.insert_decided_blocktimer spannedinsert_certificate→ cert write time double-counted. Timer rescoped.record_proposalnow returns#[must_use] bool;warn!moved to call sites.0x01-prefixed messages.Tests
Closes #144, #137, #142, #134, #131, #128