Conversation
…ith PartialEq Validator derived PartialEq across address, public key, and voting power, but manually implemented Ord solely by address. This violated the std::cmp::Ord total order invariant when addresses matched but voting power differed. sort_validators sorted primarily by descending voting power before calling vals.dedup(), leaving non-adjacent duplicate address entries intact. This inflated total_voting_power with uncastable phantom power and compromised the 2/3 + 1 supermajority calculation. Deduplication now filters duplicate addresses via HashSet retention, and aggregate voting power overflow is validated on construction.
Yudis-bit
requested review from
ZhiyuCircle,
ancazamfir,
romac and
sergio-mena
as code owners
October 4, 2026 05:24
Contributor
|
Contributor
|
Hi @Yudis-bit, Thank you for your interest in contributing to Arc Node. This PR has been automatically closed because you are not assigned to issue #477. We require contributors to be explicitly assigned to an issue before submitting a PR. To contribute properly:
Please see our CONTRIBUTING.md for more details. |
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.
Summary
Align
Validatorordering with equality to uphold the standard libraryOrdcontract, and enforce validator address uniqueness duringValidatorSet::sort_validatorsto prevent phantom voting power from inflating the consensus quorum threshold.Details
ValidatorderivedPartialEqcomparing address, public key, and voting power, but manually implementedOrdcomparing solely address. When two entries shared an address but had different voting power,cmpreturnedEqualwhile==evaluated tofalse, violating total order and corrupting sorting routines and standard collection lookups. DerivingPartialOrdandOrdensures strict consistency across all fields.In
ValidatorSet::sort_validators, entries were sorted by descending voting power before callingvals.dedup(). Becausededup()only eliminates consecutive equal elements, non-adjacent entries for identical addresses remained in the set. Sinceget_by_addressreturns only the first match whiletotal_voting_powersums every entry, duplicate entries contributed uncastable voting power to the 2/3 + 1 supermajority denominator. Retaining entries through aHashSet<Address>filter preserves the highest voting power entry for each address in linear time and eliminates duplicates. Additionally, aggregate power overflow is validated during construction inValidatorSet::newto fail fast.Testing
Verified with
cargo test -p arc-consensus-types --target x86_64-pc-windows-gnu. All 181 unit tests and 31 integration tests passed, including regressions verifying that duplicates are deduplicated to the highest-power entry,Ordaligns withPartialEq, and total power overflow panics at construction.Ran
cargo clippy -p arc-consensus-types -- -D warningsandrustfmt --check crates/types/src/validator_set.rswith clean exits.Closes: #477