From 6670da7df8e0ff8c2210900cdb44a18dfd220892 Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Thu, 25 Jan 2024 13:00:28 +0100 Subject: [PATCH] feat(validator): enforce canonical share indices instead of validator ordering --- ferveo/src/bindings_python.rs | 6 +++++- ferveo/src/dkg.rs | 11 +++++------ ferveo/src/lib.rs | 10 ++++------ ferveo/src/pvss.rs | 24 ++++++++++-------------- ferveo/src/utils.rs | 18 ------------------ ferveo/src/validator.rs | 33 +++++++++++++++++++-------------- 6 files changed, 43 insertions(+), 59 deletions(-) delete mode 100644 ferveo/src/utils.rs diff --git a/ferveo/src/bindings_python.rs b/ferveo/src/bindings_python.rs index 3b2d4ef0..69fe1b8d 100644 --- a/ferveo/src/bindings_python.rs +++ b/ferveo/src/bindings_python.rs @@ -76,7 +76,6 @@ impl From for PyErr { Error::InvalidTranscriptAggregate => { InvalidTranscriptAggregate::new_err("") } - Error::ValidatorsNotSorted => ValidatorsNotSorted::new_err(""), Error::ValidatorPublicKeyMismatch => { ValidatorPublicKeyMismatch::new_err("") } @@ -109,6 +108,11 @@ impl From for PyErr { "num_shares: {num_shares}, security_threshold: {security_threshold}" )) }, + Error::DuplicatedShareIndex(index) => { + InvalidShareIndex::new_err(format!( + "{index}" + )) + }, }, _ => default(), } diff --git a/ferveo/src/dkg.rs b/ferveo/src/dkg.rs index ef5d0bf0..e5615d05 100644 --- a/ferveo/src/dkg.rs +++ b/ferveo/src/dkg.rs @@ -9,8 +9,9 @@ use serde::{de::DeserializeOwned, Deserialize, Serialize}; use serde_with::serde_as; use crate::{ - aggregate, utils::is_sorted, AggregatedPvss, Error, EthereumAddress, - PubliclyVerifiableParams, PubliclyVerifiableSS, Result, Validator, + aggregate, assert_no_share_duplicates, AggregatedPvss, Error, + EthereumAddress, PubliclyVerifiableParams, PubliclyVerifiableSS, Result, + Validator, }; #[derive(Copy, Clone, Debug, Serialize, Deserialize)] @@ -122,10 +123,8 @@ impl PubliclyVerifiableDkg { ) .expect("unable to construct domain"); - // Sort the validators to verify a global ordering - if !is_sorted(validators) { - return Err(Error::ValidatorsNotSorted); - } + assert_no_share_duplicates(validators)?; + let validators: ValidatorsMap = validators .iter() .enumerate() diff --git a/ferveo/src/lib.rs b/ferveo/src/lib.rs index c316c815..0db10105 100644 --- a/ferveo/src/lib.rs +++ b/ferveo/src/lib.rs @@ -19,8 +19,6 @@ pub mod pvss; pub mod refresh; pub mod validator; -mod utils; - #[cfg(test)] mod test_common; @@ -85,10 +83,6 @@ pub enum Error { #[error("Transcript aggregate doesn't match the received PVSS instances")] InvalidTranscriptAggregate, - /// DKG validators must be sorted by their Ethereum address - #[error("DKG validators not sorted")] - ValidatorsNotSorted, - /// The validator public key doesn't match the one in the DKG #[error("Validator public key mismatch")] ValidatorPublicKeyMismatch, @@ -118,6 +112,10 @@ pub enum Error { /// Failed to produce a precomputed variant decryption share #[error("Invalid DKG parameters for precomputed variant: number of shares {0}, threshold {1}")] InvalidDkgParametersForPrecomputedVariant(u32, u32), + + /// DKG may not contain duplicated share indices + #[error("Duplicated share index: {0}")] + DuplicatedShareIndex(u32), } pub type Result = std::result::Result; diff --git a/ferveo/src/pvss.rs b/ferveo/src/pvss.rs index 193f3c4f..c2b075b4 100644 --- a/ferveo/src/pvss.rs +++ b/ferveo/src/pvss.rs @@ -18,8 +18,8 @@ use subproductdomain::fast_multiexp; use zeroize::{self, Zeroize, ZeroizeOnDrop}; use crate::{ - apply_updates_to_private_share, batch_to_projective_g1, - batch_to_projective_g2, utils::is_sorted, Error, PVSSMap, + apply_updates_to_private_share, assert_no_share_duplicates, + batch_to_projective_g1, batch_to_projective_g2, Error, PVSSMap, PubliclyVerifiableDkg, Result, Validator, }; @@ -225,8 +225,7 @@ pub fn do_verify_full( let mut commitment = batch_to_projective_g1::(pvss_coefficients); domain.fft_in_place(&mut commitment); - // At this point, validators must be sorted - assert!(is_sorted(validators)); + assert_no_share_duplicates(validators).expect("Validators must be unique"); // Each validator checks that their share is correct validators @@ -459,10 +458,9 @@ mod test_pvss { use ark_bls12_381::Bls12_381 as EllipticCurve; use ark_ec::AffineRepr; use ark_ff::UniformRand; - use rand::prelude::SliceRandom; use super::*; - use crate::{test_common::*, utils::is_sorted, DkgParams}; + use crate::{test_common::*, DkgParams}; /// Test the happy flow that a pvss with the correct form is created /// and that appropriate validations pass @@ -532,21 +530,19 @@ mod test_pvss { assert!(!bad_pvss.verify_full(&dkg)); } - /// Check that the explicit ordering of validators is expected and enforced + /// Check that the canonical share indices of validators are expected and enforced /// by the DKG methods. #[test] - fn test_ordering_of_validators_is_enforced() { - let rng = &mut ark_std::test_rng(); - + fn test_canonical_share_indices_are_enforced() { let shares_num = 4; let security_threshold = shares_num - 1; let keypairs = gen_keypairs(shares_num); let mut validators = gen_validators(&keypairs); let me = validators[0].clone(); - // Validators are not sorted - validators.shuffle(rng); - assert!(!is_sorted(&validators)); + // Validators (share indices) are not unique + let duplicated_index = 0; + validators.insert(duplicated_index, me.clone()); // And because of that the DKG should fail let result = PubliclyVerifiableDkg::new( @@ -557,7 +553,7 @@ mod test_pvss { assert!(result.is_err()); assert_eq!( result.unwrap_err().to_string(), - Error::ValidatorsNotSorted.to_string() + Error::DuplicatedShareIndex(duplicated_index as u32).to_string() ); } diff --git a/ferveo/src/utils.rs b/ferveo/src/utils.rs deleted file mode 100644 index b8b67b10..00000000 --- a/ferveo/src/utils.rs +++ /dev/null @@ -1,18 +0,0 @@ -pub fn is_sorted(data: I) -> bool -where - I: IntoIterator, - I::Item: Ord, -{ - let mut data = data.into_iter(); - let mut prev = match data.next() { - None => return true, - Some(x) => x, - }; - for x in data { - if prev > x { - return false; - } - prev = x; - } - true -} diff --git a/ferveo/src/validator.rs b/ferveo/src/validator.rs index 146e62eb..a1c73e11 100644 --- a/ferveo/src/validator.rs +++ b/ferveo/src/validator.rs @@ -1,10 +1,12 @@ -use std::{cmp::Ordering, fmt::Display, str::FromStr}; +use std::{collections::HashSet, fmt::Display, str::FromStr}; use ark_ec::pairing::Pairing; use ferveo_common::PublicKey; use serde::{Deserialize, Serialize}; use thiserror::Error; +use crate::Error; + #[derive( Clone, Debug, PartialEq, Eq, Ord, PartialOrd, Serialize, Deserialize, Hash, )] @@ -49,19 +51,6 @@ pub struct Validator { pub share_index: u32, } -impl PartialOrd for Validator { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for Validator { - // Validators are ordered by their address only - fn cmp(&self, other: &Self) -> Ordering { - self.address.cmp(&other.address) - } -} - impl Validator { pub fn new( address: String, @@ -75,3 +64,19 @@ impl Validator { }) } } + +pub fn assert_no_share_duplicates( + validators: &[Validator], +) -> Result<(), Error> { + let mut set = HashSet::new(); + + for validator in validators { + if set.contains(&validator.share_index) { + return Err(Error::DuplicatedShareIndex(validator.share_index)); + } else { + set.insert(validator.share_index); + } + } + + Ok(()) +}