ValidatorSet::sort_validators fails to deduplicate addresses and Validator violates Ord contract #477

Problem

At commit 6e764023ee6515fe70573e123ed2db912a7207b4, ValidatorSet::sort_validators in crates/types/src/validator_set.rs attempts in-place sorting and deduplication. However, it sorts primarily by descending voting power before calling vals.dedup(). Because dedup() only eliminates consecutive elements equal under PartialEq, duplicate validator address entries with different voting powers remain non-adjacent and are never removed. Even when voting powers match, any difference in public keys causes PartialEq to evaluate to false, retaining both entries.

This leaves multiple entries with the same address in ValidatorSet. total_voting_power() iterates over every entry and sums their voting power, whereas vote accumulation in consensus engines keys voters by unique Address and ValidatorSet::get_by_address() only yields the first match. This phantom voting power inflates the denominator of the 2/3 + 1 supermajority calculation with uncastable voting weight. If duplicate power exceeds 1/3 of the aggregate, the consensus round cannot achieve supermajority even if all valid validators precommit, producing an unrecoverable chain halt.

Additionally, Validator derives PartialEq across address, public_key, and voting_power, but manually implements Ord solely by comparing self.address.cmp(&other.address). This violates the standard library Ord contract where a.cmp(&b) == Equal must hold if and only if a == b. When two entries share an address but carry different voting powers, cmp returns Equal while PartialEq returns false, causing undefined ordering behavior in sorted slices and standard collections.

Furthermore, aggregate voting power overflow is checked only during total_voting_power(), deferring integer overflow panics to runtime execution inside consensus state transitions instead of validating at construction in ValidatorSet::new.

Reproduction

Run the following test case against crates/types/src/validator_set.rs:

#[test]
fn sort_validators_deduplicates_by_address() {
    let mut rng = StdRng::seed_from_u64(0x42);
    let sk = PrivateKey::generate(&mut rng);

    let v1 = Validator::new(sk.public_key(), 10);
    let v2 = Validator::new(sk.public_key(), 20);

    let vs = ValidatorSet::new(vec![v1.clone(), v2.clone()]);
    assert_eq!(vs.len(), 1);
    assert_eq!(vs.total_voting_power(), 20);
}

On current main, this assertion fails:

assertion `left == right` failed
  left: 2
 right: 1

The validator set retains both entries, and total_voting_power() evaluates to 30 instead of 20.

Expected Behavior

ValidatorSet::new must ensure validator address identities are unique in the resulting set, keeping the highest voting power entry for any address and reflecting only true castable power in total_voting_power(). Validator must implement total ordering consistent with equality, and aggregate voting power overflow must fail fast during construction.

Proposed Fix

Derive PartialOrd and Ord directly on Validator. In ValidatorSet::sort_validators, sort by descending voting power with Validator::cmp tie-breaking, then retain unique addresses in linear time using a HashSet<Address>. Validate non-overflow of total voting power at construction time in ValidatorSet::new.

I have the fix and regression test suite ready on a local branch. Could a maintainer please assign this issue to me so I can submit a PR? Thanks!