cluster: dedup definition peers by peer ID - #4635
Open
pinebit wants to merge 1 commit into
Open
Conversation
Reject cluster definitions containing operators whose ENRs encode the same public key. Peers previously deduplicated operators by ENR string only, so distinct ENRs sharing a key (and thus a peer ID) passed verification and collapsed the peer index map built during DKG setup, causing an index out-of-range panic in newFrostP2P. category: bug ticket: none Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4635 +/- ##
==========================================
- Coverage 58.16% 57.84% -0.32%
==========================================
Files 247 247
Lines 34056 34056
==========================================
- Hits 19807 19699 -108
- Misses 11779 11910 +131
+ Partials 2470 2447 -23 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
1 participant
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.
Deduplicate operators in
Definition.Peers()by derived peer ID instead of by raw ENR string, and rename the error todefinition contains duplicate peer ids.Two distinct ENR strings can encode the same public key (e.g. different sequence numbers or extra fields), producing the same libp2p peer ID. Such definitions passed the previous ENR-string dedup and collapsed the
peerMapbuilt indkg.Run(NodeIdxreturns the first matching operator index), which madenewFrostP2Ppanic with an index out-of-range when a survivingPeerIdxexceeded the map length. When the duplicated operator was last, the ceremony instead proceeded with an inconsistent peer map.Peer-ID dedup subsumes the ENR-string check, since identical ENR strings always yield identical peer IDs. All callers of
Peers()(dkg ceremony, node run, edit protocols) now reject such definitions with a clean error.category: bug
ticket: none