kaiax/vrank: drop duplicate VRankCandidate before verifying it - #998
Open
hyunsooda wants to merge 1 commit into
Open
kaiax/vrank: drop duplicate VRankCandidate before verifying it#998hyunsooda wants to merge 1 commit into
hyunsooda wants to merge 1 commit into
Conversation
ian0371
previously approved these changes
Jul 30, 2026
Only the first VRankCandidate per (block, round, sender) is kept, but the duplicate check sat after the BLS verification, so every replay paid for a pairing (~0.75ms) before being discarded. A ~200 byte packet therefore bought that much CPU on each receiving validator, repeatable for as long as the peer stayed connected: a valid duplicate returns nil, so node/cn never drops it. Move the check up to right after the ECDSA recovery that produces the sender. The stored message already passed both signature checks and a later one cannot replace it, so re-verifying it decides nothing. Constraint: the sender is only known after ECDSA recovery, so the check cannot move any earlier than that Rejected: singleflight around the first verification | the race exists only for the first message of a view and is bounded by concurrentPerPeer=3 Rejected: cache of verified wire-message hashes | the (view, sender) check already subsumes byte-identical replays and saves the larger cost Confidence: high Scope-risk: narrow Directive: this check must stay ahead of GetBlsPubkey/VerifySignature - the ordering is the fix, and collector state alone cannot detect a regression Not-tested: a duplicate arriving concurrently with the first message on separate peer workers, which may still verify more than once
hyunsooda
force-pushed
the
fix/skip-verify-duplicate-vrank-candidate
branch
from
August 6, 2026 02:20
a08b59f to
e49a4b9
Compare
da1suk8
approved these changes
Aug 6, 2026
yoomee1313
approved these changes
Aug 6, 2026
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
4 participants
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.
Proposed changes
The duplicate check for
VRankCandidatesat after the BLS verification, so every replay paid for a pairing before being discarded — and a valid duplicate returns nil, so the peer is never dropped and can keep sending. It now runs right after the ECDSA recovery that produces the sender.Types of changes
Checklist
I have read the CLA Document and I hereby sign the CLAin first time contribute after having read CLA$ make test)Related issues
Further comments