discover: admit only CNPeers members into the CN storage - #1013
Open
hyunsooda wants to merge 2 commits into
Open
discover: admit only CNPeers members into the CN storage#1013hyunsooda wants to merge 2 commits into
hyunsooda wants to merge 2 commits into
Conversation
The node type in a discovery packet is a sender-supplied field and the CN storage has no cardinality bound, so any reachable key pair can occupy the list the dial scheduler samples for CN candidates. The CN storage now takes an entry only when its address is in CNPeers or its NodeId is a configured bootnode, bounding the list by an authenticated set rather than a capacity limit. Discovery stays open otherwise: the filter is nil before the fork and whenever CNPeers is unavailable, and peer admission is unchanged. Constraint: KIP-311 R2 forbids a BN from filtering discovery by CNPeers, so only nodes running the CN service push the list down Constraint: KIP-311 L206 keeps CNPeers out of EN admission, so only discovery entry selection changed Rejected: Bound the flat storages by capacity | eviction turns dilution into a deterministic lockout, since revalidation replaces one entry per 10s Rejected: Enforce CNPeers on EN admission | would reject the nonCNPeers CN that R7 requires an EN to serve Directive: Keep the check keyed on the destination storage, not the claimed node type, or the bootnode fan-out path bypasses it Confidence: high Scope-risk: narrow Not-tested: a self-declared CN outside CNPeers on a live network Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ian0371
previously approved these changes
Aug 7, 2026
2dvorak
reviewed
Aug 11, 2026
Contributor
There was a problem hiding this comment.
We could have better coverage with the following test cases:
BaseServer→ntab forward—nilntab in both existing tests means the three new added lines are dead to CI.- Table-level shrink —
dropsOutsidersonly coversnil → {member}, i.e. startup. No allowlist-to-allowlist case exists.
And one minor thing is: TestServerENBypassesCNPeerFilter says SetCNPeers is no-op for PN/EN, but it became stale comment.
The push into the discovery table ran only when srv.ntab was set, which no test did, so nothing pinned it ahead of the CN-only early return. Exercise it on an EN, the only path where moving it below that return is observable. The table test covered a first allowlist arriving but not one replacing another, so nothing required an update to drop a member that left rather than accumulate. The EN test also still described SetCNPeers as a no-op there, which stopped being true once the discovery filter applied to every node type. Confidence: high Scope-risk: narrow Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@2dvorak All added. Thank you |
2dvorak
approved these changes
Aug 11, 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
3 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 node type in a discovery packet is supplied by the sender and the CN storage is unbounded, so any reachable key pair can fill the list the dial scheduler samples for CN candidates. The CN storage now accepts an entry only when its address is in
CNPeersor its NodeId is a configured bootnode, bounding the list by an authenticated set instead of a capacity limit. Discovery stays open elsewhere: the filter is off before the hard fork and wheneverCNPeersis unavailable, bootnodes are exempt so they keep seeding CN lookups, and peer admission is unchanged.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