From c93d26ea7a261235210c774a23b6cc3151a4e736 Mon Sep 17 00:00:00 2001 From: Yudistira Putra <85178972+Yudis-bit@users.noreply.github.com> Date: Sun, 4 Oct 2026 12:27:33 +0700 Subject: [PATCH] fix(types): deduplicate validator addresses and align Validator Ord with 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. --- crates/types/src/validator_set.rs | 436 ++++++++++++++++-------------- 1 file changed, 234 insertions(+), 202 deletions(-) diff --git a/crates/types/src/validator_set.rs b/crates/types/src/validator_set.rs index 1fdf92d5..f8cd9fe1 100644 --- a/crates/types/src/validator_set.rs +++ b/crates/types/src/validator_set.rs @@ -1,202 +1,234 @@ -// Copyright 2025 Circle Internet Group, Inc. All rights reserved. -// -// SPDX-License-Identifier: Apache-2.0 -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// adapted from https://github.com/informalsystems/malachite/tree/v0.4.0/code/crates/test -use core::slice; -use std::sync::Arc; - -use malachitebft_core_types::VotingPower; -use serde::{Deserialize, Serialize}; - -use crate::signing::PublicKey; -use crate::{Address, ArcContext}; - -/// A validator is a public key and voting power -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] -pub struct Validator { - pub address: Address, - pub public_key: PublicKey, - pub voting_power: VotingPower, -} - -impl Validator { - pub fn new(public_key: PublicKey, voting_power: VotingPower) -> Self { - Self { - address: Address::from_public_key(&public_key), - public_key, - voting_power, - } - } -} - -impl PartialOrd for Validator { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for Validator { - fn cmp(&self, other: &Self) -> std::cmp::Ordering { - self.address.cmp(&other.address) - } -} - -impl malachitebft_core_types::Validator for Validator { - fn address(&self) -> &Address { - &self.address - } - - fn public_key(&self) -> &PublicKey { - &self.public_key - } - - fn voting_power(&self) -> VotingPower { - self.voting_power - } -} - -/// A validator set contains a list of validators sorted by address. -#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] -pub struct ValidatorSet { - pub validators: Arc>, -} - -impl ValidatorSet { - pub fn new(validators: impl IntoIterator) -> Self { - let mut validators: Vec<_> = validators.into_iter().collect(); - ValidatorSet::sort_validators(&mut validators); - - assert!(!validators.is_empty()); - - Self { - validators: Arc::new(validators), - } - } - - /// Get the number of validators in the set - pub fn len(&self) -> usize { - self.validators.len() - } - - /// Check if the set is empty - pub fn is_empty(&self) -> bool { - self.validators.is_empty() - } - - /// Iterate over the validators in the set - pub fn iter(&self) -> slice::Iter<'_, Validator> { - self.validators.iter() - } - - /// The total voting power of the validator set - pub fn total_voting_power(&self) -> VotingPower { - self.validators - .iter() - .try_fold(0u64, |acc, v| acc.checked_add(v.voting_power)) - .expect("total voting power overflow") - } - - /// Get a validator by its index - pub fn get_by_index(&self, index: usize) -> Option<&Validator> { - self.validators.get(index) - } - - /// Get a validator by its address - pub fn get_by_address(&self, address: &Address) -> Option<&Validator> { - self.validators.iter().find(|v| &v.address == address) - } - - pub fn get_by_public_key(&self, public_key: &PublicKey) -> Option<&Validator> { - self.validators.iter().find(|v| &v.public_key == public_key) - } - - /// In place sort and deduplication of a list of validators - fn sort_validators(vals: &mut Vec) { - // Sort the validators according to the current Tendermint requirements - // (v. 0.34 -> first by validator power, descending, then by address, ascending) - use core::cmp::Reverse; - vals.sort_unstable_by(|v1, v2| { - let a = (Reverse(v1.voting_power), &v1.address); - let b = (Reverse(v2.voting_power), &v2.address); - a.cmp(&b) - }); - - vals.dedup(); - } - - pub fn get_keys(&self) -> Vec { - self.validators.iter().map(|v| v.public_key).collect() - } -} - -impl malachitebft_core_types::ValidatorSet for ValidatorSet { - fn count(&self) -> usize { - self.validators.len() - } - - fn total_voting_power(&self) -> VotingPower { - self.total_voting_power() - } - - fn get_by_address(&self, address: &Address) -> Option<&Validator> { - self.get_by_address(address) - } - - fn get_by_index(&self, index: usize) -> Option<&Validator> { - self.validators.get(index) - } -} - -#[cfg(test)] -mod tests { - use rand::rngs::StdRng; - use rand::SeedableRng; - - use super::*; - - use crate::signing::PrivateKey; - - #[test] - fn new_validator_set_vp() { - let mut rng = StdRng::seed_from_u64(0x42); - - let sk1 = PrivateKey::generate(&mut rng); - let sk2 = PrivateKey::generate(&mut rng); - let sk3 = PrivateKey::generate(&mut rng); - - let v1 = Validator::new(sk1.public_key(), 1); - let v2 = Validator::new(sk2.public_key(), 2); - let v3 = Validator::new(sk3.public_key(), 3); - - let vs = ValidatorSet::new(vec![v1, v2, v3]); - assert_eq!(vs.total_voting_power(), 6); - } - - #[test] - #[should_panic(expected = "total voting power overflow")] - fn total_voting_power_overflow_panics() { - let mut rng = StdRng::seed_from_u64(0x42); - - let sk1 = PrivateKey::generate(&mut rng); - let sk2 = PrivateKey::generate(&mut rng); - - let v1 = Validator::new(sk1.public_key(), u64::MAX); - let v2 = Validator::new(sk2.public_key(), 1); - - let vs = ValidatorSet::new(vec![v1, v2]); - let _ = vs.total_voting_power(); - } -} +// Copyright 2025 Circle Internet Group, Inc. All rights reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// adapted from https://github.com/informalsystems/malachite/tree/v0.4.0/code/crates/test +use core::slice; +use std::sync::Arc; + +use malachitebft_core_types::VotingPower; +use serde::{Deserialize, Serialize}; + +use crate::signing::PublicKey; +use crate::{Address, ArcContext}; + +/// A validator is a public key and voting power +#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +pub struct Validator { + pub address: Address, + pub public_key: PublicKey, + pub voting_power: VotingPower, +} + +impl Validator { + pub fn new(public_key: PublicKey, voting_power: VotingPower) -> Self { + Self { + address: Address::from_public_key(&public_key), + public_key, + voting_power, + } + } +} + +impl malachitebft_core_types::Validator for Validator { + fn address(&self) -> &Address { + &self.address + } + + fn public_key(&self) -> &PublicKey { + &self.public_key + } + + fn voting_power(&self) -> VotingPower { + self.voting_power + } +} + +/// A validator set contains a list of validators sorted by address. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct ValidatorSet { + pub validators: Arc>, +} + +impl ValidatorSet { + pub fn new(validators: impl IntoIterator) -> Self { + let mut validators: Vec<_> = validators.into_iter().collect(); + ValidatorSet::sort_validators(&mut validators); + + assert!(!validators.is_empty()); + + // Verify that total voting power does not overflow u64 + validators + .iter() + .try_fold(0u64, |acc, v| acc.checked_add(v.voting_power)) + .expect("total voting power overflow"); + + Self { + validators: Arc::new(validators), + } + } + + /// Get the number of validators in the set + pub fn len(&self) -> usize { + self.validators.len() + } + + /// Check if the set is empty + pub fn is_empty(&self) -> bool { + self.validators.is_empty() + } + + /// Iterate over the validators in the set + pub fn iter(&self) -> slice::Iter<'_, Validator> { + self.validators.iter() + } + + /// The total voting power of the validator set + pub fn total_voting_power(&self) -> VotingPower { + self.validators + .iter() + .try_fold(0u64, |acc, v| acc.checked_add(v.voting_power)) + .expect("total voting power overflow") + } + + /// Get a validator by its index + pub fn get_by_index(&self, index: usize) -> Option<&Validator> { + self.validators.get(index) + } + + /// Get a validator by its address + pub fn get_by_address(&self, address: &Address) -> Option<&Validator> { + self.validators.iter().find(|v| &v.address == address) + } + + pub fn get_by_public_key(&self, public_key: &PublicKey) -> Option<&Validator> { + self.validators.iter().find(|v| &v.public_key == public_key) + } + + /// In place sort and deduplication of a list of validators + fn sort_validators(vals: &mut Vec) { + // Sort the validators according to the current Tendermint requirements + // (v. 0.34 -> first by validator power, descending, then by address, ascending) + use core::cmp::Reverse; + vals.sort_unstable_by(|v1, v2| { + let a = (Reverse(v1.voting_power), &v1.address); + let b = (Reverse(v2.voting_power), &v2.address); + a.cmp(&b) + }); + + let mut seen = std::collections::HashSet::new(); + vals.retain(|v| seen.insert(v.address)); + } + + pub fn get_keys(&self) -> Vec { + self.validators.iter().map(|v| v.public_key).collect() + } +} + +impl malachitebft_core_types::ValidatorSet for ValidatorSet { + fn count(&self) -> usize { + self.validators.len() + } + + fn total_voting_power(&self) -> VotingPower { + self.total_voting_power() + } + + fn get_by_address(&self, address: &Address) -> Option<&Validator> { + self.get_by_address(address) + } + + fn get_by_index(&self, index: usize) -> Option<&Validator> { + self.validators.get(index) + } +} + +#[cfg(test)] +mod tests { + use rand::rngs::StdRng; + use rand::SeedableRng; + + use super::*; + + use crate::signing::PrivateKey; + + #[test] + fn new_validator_set_vp() { + let mut rng = StdRng::seed_from_u64(0x42); + + let sk1 = PrivateKey::generate(&mut rng); + let sk2 = PrivateKey::generate(&mut rng); + let sk3 = PrivateKey::generate(&mut rng); + + let v1 = Validator::new(sk1.public_key(), 1); + let v2 = Validator::new(sk2.public_key(), 2); + let v3 = Validator::new(sk3.public_key(), 3); + + let vs = ValidatorSet::new(vec![v1, v2, v3]); + assert_eq!(vs.total_voting_power(), 6); + } + + #[test] + #[should_panic(expected = "total voting power overflow")] + fn total_voting_power_overflow_panics_on_construction() { + let mut rng = StdRng::seed_from_u64(0x42); + + let sk1 = PrivateKey::generate(&mut rng); + let sk2 = PrivateKey::generate(&mut rng); + + let v1 = Validator::new(sk1.public_key(), u64::MAX); + let v2 = Validator::new(sk2.public_key(), 1); + + // Panics immediately upon construction rather than deferring to consensus runtime + let _ = ValidatorSet::new(vec![v1, v2]); + } + + #[test] + fn validator_ord_consistent_with_partialeq() { + 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 v3 = Validator::new(sk.public_key(), 10); + + // Consistent ordering and equality: + assert_ne!(v1.cmp(&v2), std::cmp::Ordering::Equal); + assert_ne!(v1, v2); + + assert_eq!(v1.cmp(&v3), std::cmp::Ordering::Equal); + assert_eq!(v1, v3); + } + + #[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); + + assert_eq!(v1.address, v2.address); + + let vs = ValidatorSet::new(vec![v1.clone(), v2.clone()]); + + // Retains highest voting power entry and deduplicates by address: + assert_eq!(vs.len(), 1); + assert_eq!(vs.total_voting_power(), 20); + + let fetched = vs.get_by_address(&v1.address).unwrap(); + assert_eq!(fetched.voting_power, 20); + } +}