feat(validator): enforce canonical share indices instead of validator…

… ordering

ferveo/src/bindings_python.rs

@@ -76,7 +76,6 @@ impl From<FerveoPythonError> for PyErr {
                 Error::InvalidTranscriptAggregate => {
                     InvalidTranscriptAggregate::new_err("")
                 }
-                Error::ValidatorsNotSorted => ValidatorsNotSorted::new_err(""),
                 Error::ValidatorPublicKeyMismatch => {
                     ValidatorPublicKeyMismatch::new_err("")
                 }
@@ -109,6 +108,11 @@ impl From<FerveoPythonError> for PyErr {
                         "num_shares: {num_shares}, security_threshold: {security_threshold}"
                     ))
                 },
+                Error::DuplicatedShareIndex(index) => {
+                    InvalidShareIndex::new_err(format!(
+                        "{index}"
+                    ))
+                },
             },
             _ => default(),
         }

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<E: Pairing> PubliclyVerifiableDkg<E> {
         )
         .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<E> = validators
             .iter()
             .enumerate()

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<T> = std::result::Result<T, Error>;

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<E: Pairing>(
     let mut commitment = batch_to_projective_g1::<E>(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()
         );
     }
 

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<E: Pairing> {
     pub share_index: u32,
 }
 
-impl<E: Pairing> PartialOrd for Validator<E> {
-    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl<E: Pairing> Ord for Validator<E> {
-    // Validators are ordered by their address only
-    fn cmp(&self, other: &Self) -> Ordering {
-        self.address.cmp(&other.address)
-    }
-}
-
 impl<E: Pairing> Validator<E> {
     pub fn new(
         address: String,
@@ -75,3 +64,19 @@ impl<E: Pairing> Validator<E> {
         })
     }
 }
+
+pub fn assert_no_share_duplicates<E: Pairing>(
+    validators: &[Validator<E>],
+) -> 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(())
+}