From 830cbc859b23c0bf43373bf47c18aedbb54943a2 Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Thu, 25 Jan 2024 11:20:39 +0100 Subject: [PATCH 1/9] feat(validator): add share_index field to validator --- ferveo-python/examples/exception.py | 2 +- ferveo-python/examples/server_api_precomputed.py | 2 +- ferveo-python/examples/server_api_simple.py | 2 +- ferveo-python/test/test_ferveo.py | 2 +- ferveo-python/test/test_serialization.py | 2 +- ferveo-wasm/examples/node/src/main.test.ts | 2 +- ferveo/benches/benchmarks/validity_checks.rs | 1 + ferveo/examples/bench_primitives_size.rs | 1 + ferveo/src/api.rs | 1 + ferveo/src/bindings_python.rs | 11 ++++++++--- ferveo/src/bindings_wasm.rs | 5 +++++ ferveo/src/dkg.rs | 7 ++++--- ferveo/src/pvss.rs | 2 +- ferveo/src/test_common.rs | 4 ++-- ferveo/src/validator.rs | 4 ++++ 15 files changed, 33 insertions(+), 15 deletions(-) diff --git a/ferveo-python/examples/exception.py b/ferveo-python/examples/exception.py index 4e16aea9..daa067b7 100644 --- a/ferveo-python/examples/exception.py +++ b/ferveo-python/examples/exception.py @@ -16,7 +16,7 @@ def gen_eth_addr(i: int) -> str: validator_keypairs = [Keypair.random() for _ in range(0, shares_num)] validators = [ - Validator(gen_eth_addr(i), keypair.public_key()) + Validator(gen_eth_addr(i), keypair.public_key(), i) for i, keypair in enumerate(validator_keypairs) ] diff --git a/ferveo-python/examples/server_api_precomputed.py b/ferveo-python/examples/server_api_precomputed.py index 7c433e96..72263405 100644 --- a/ferveo-python/examples/server_api_precomputed.py +++ b/ferveo-python/examples/server_api_precomputed.py @@ -21,7 +21,7 @@ def gen_eth_addr(i: int) -> str: validator_keypairs = [Keypair.random() for _ in range(0, shares_num)] validators = [ - Validator(gen_eth_addr(i), keypair.public_key()) + Validator(gen_eth_addr(i), keypair.public_key(), i) for i, keypair in enumerate(validator_keypairs) ] diff --git a/ferveo-python/examples/server_api_simple.py b/ferveo-python/examples/server_api_simple.py index 4f9e8447..5fd2c8e5 100644 --- a/ferveo-python/examples/server_api_simple.py +++ b/ferveo-python/examples/server_api_simple.py @@ -19,7 +19,7 @@ def gen_eth_addr(i: int) -> str: shares_num = 4 validator_keypairs = [Keypair.random() for _ in range(0, shares_num)] validators = [ - Validator(gen_eth_addr(i), keypair.public_key()) + Validator(gen_eth_addr(i), keypair.public_key(), i) for i, keypair in enumerate(validator_keypairs) ] diff --git a/ferveo-python/test/test_ferveo.py b/ferveo-python/test/test_ferveo.py index 6f00b6df..b3496d3e 100644 --- a/ferveo-python/test/test_ferveo.py +++ b/ferveo-python/test/test_ferveo.py @@ -44,7 +44,7 @@ def scenario_for_variant(variant: FerveoVariant, shares_num, threshold, shares_t tau = 1 validator_keypairs = [Keypair.random() for _ in range(0, shares_num)] validators = [ - Validator(gen_eth_addr(i), keypair.public_key()) + Validator(gen_eth_addr(i), keypair.public_key(), i) for i, keypair in enumerate(validator_keypairs) ] validators.sort(key=lambda v: v.address) diff --git a/ferveo-python/test/test_serialization.py b/ferveo-python/test/test_serialization.py index 57032c6a..6c600771 100644 --- a/ferveo-python/test/test_serialization.py +++ b/ferveo-python/test/test_serialization.py @@ -17,7 +17,7 @@ def gen_eth_addr(i: int) -> str: shares_num = 4 validator_keypairs = [Keypair.random() for _ in range(shares_num)] validators = [ - Validator(gen_eth_addr(i), keypair.public_key()) + Validator(gen_eth_addr(i), keypair.public_key(), i) for i, keypair in enumerate(validator_keypairs) ] validators.sort(key=lambda v: v.address) diff --git a/ferveo-wasm/examples/node/src/main.test.ts b/ferveo-wasm/examples/node/src/main.test.ts index 047beeb5..5814a53d 100644 --- a/ferveo-wasm/examples/node/src/main.test.ts +++ b/ferveo-wasm/examples/node/src/main.test.ts @@ -29,7 +29,7 @@ function setupTest(sharesNum :number, threshold: number) { for (let i = 0; i < sharesNum; i++) { const keypair = Keypair.random(); validatorKeypairs.push(keypair); - const validator = new Validator(genEthAddr(i), keypair.publicKey); + const validator = new Validator(genEthAddr(i), keypair.publicKey, i); validators.push(validator); } diff --git a/ferveo/benches/benchmarks/validity_checks.rs b/ferveo/benches/benchmarks/validity_checks.rs index cc7266f7..e6a27b74 100644 --- a/ferveo/benches/benchmarks/validity_checks.rs +++ b/ferveo/benches/benchmarks/validity_checks.rs @@ -32,6 +32,7 @@ fn gen_validators( .map(|i| Validator { address: gen_address(i), public_key: keypairs[i].public_key(), + share_index: i as u32, }) .collect() } diff --git a/ferveo/examples/bench_primitives_size.rs b/ferveo/examples/bench_primitives_size.rs index 79afb8a4..8f993c36 100644 --- a/ferveo/examples/bench_primitives_size.rs +++ b/ferveo/examples/bench_primitives_size.rs @@ -66,6 +66,7 @@ fn gen_validators( .map(|i| Validator { address: gen_address(i), public_key: keypairs[i].public_key(), + share_index: i as u32, }) .collect() } diff --git a/ferveo/src/api.rs b/ferveo/src/api.rs index 3c2295d1..b8c0febf 100644 --- a/ferveo/src/api.rs +++ b/ferveo/src/api.rs @@ -425,6 +425,7 @@ mod test_ferveo_api { .map(|(i, keypair)| Validator { address: gen_address(i), public_key: keypair.public_key(), + share_index: i as u32, }) .collect::>(); diff --git a/ferveo/src/bindings_python.rs b/ferveo/src/bindings_python.rs index f897c8f6..3b2d4ef0 100644 --- a/ferveo/src/bindings_python.rs +++ b/ferveo/src/bindings_python.rs @@ -396,8 +396,9 @@ impl Validator { pub fn new( address: String, public_key: &FerveoPublicKey, + share_index: u32, ) -> PyResult { - let validator = api::Validator::new(address, public_key.0) + let validator = api::Validator::new(address, public_key.0, share_index) .map_err(|err| FerveoPythonError::Other(err.to_string()))?; Ok(Self(validator)) } @@ -756,8 +757,12 @@ mod test_ferveo_python { .iter() .enumerate() .map(|(i, keypair)| { - Validator::new(format!("0x{i:040}"), &keypair.public_key()) - .unwrap() + Validator::new( + format!("0x{i:040}"), + &keypair.public_key(), + i as u32, + ) + .unwrap() }) .collect(); diff --git a/ferveo/src/bindings_wasm.rs b/ferveo/src/bindings_wasm.rs index a1310277..1c0d2a0b 100644 --- a/ferveo/src/bindings_wasm.rs +++ b/ferveo/src/bindings_wasm.rs @@ -421,6 +421,7 @@ impl EthereumAddress { pub struct Validator { address: EthereumAddress, public_key: FerveoPublicKey, + share_index: u32, } #[wasm_bindgen] @@ -429,11 +430,13 @@ impl Validator { pub fn new( address: &EthereumAddress, public_key: &FerveoPublicKey, + share_index: u32, ) -> JsResult { set_panic_hook(); Ok(Self { address: address.clone(), public_key: public_key.clone(), + share_index, }) } @@ -442,6 +445,7 @@ impl Validator { Ok(api::Validator { address: self.address.0.clone(), public_key: self.public_key.0, + share_index: self.share_index, }) } @@ -610,6 +614,7 @@ pub mod test_common { Validator { address: gen_address(i), public_key: keypair.public_key(), + share_index: i as u32, } } } diff --git a/ferveo/src/dkg.rs b/ferveo/src/dkg.rs index e13c4894..ef5d0bf0 100644 --- a/ferveo/src/dkg.rs +++ b/ferveo/src/dkg.rs @@ -375,12 +375,12 @@ mod test_dkg_init { #[test] fn test_dkg_fail_unknown_validator() { let rng = &mut ark_std::test_rng(); - let shares_num = 4; - let known_keypairs = gen_keypairs(shares_num); + let known_keypairs = gen_keypairs(SHARES_NUM); let unknown_keypair = ferveo_common::Keypair::::new(rng); let unknown_validator = Validator:: { - address: gen_address((shares_num + 1) as usize), + address: gen_address((SHARES_NUM + 1) as usize), public_key: unknown_keypair.public_key(), + share_index: SHARES_NUM + 5, // Not in the validator set }; let err = PubliclyVerifiableDkg::::new( &gen_validators(&known_keypairs), @@ -464,6 +464,7 @@ mod test_dealing { let sender = Validator:: { address: gen_address(unknown_validator_i as usize), public_key: ferveo_common::Keypair::::new(rng).public_key(), + share_index: dkg.dkg_params.shares_num + 5, // Not in the validator set }; // check that verification fails assert!(dkg.verify_message(&sender, &pvss).is_err()); diff --git a/ferveo/src/pvss.rs b/ferveo/src/pvss.rs index c8498bb7..193f3c4f 100644 --- a/ferveo/src/pvss.rs +++ b/ferveo/src/pvss.rs @@ -459,7 +459,7 @@ mod test_pvss { use ark_bls12_381::Bls12_381 as EllipticCurve; use ark_ec::AffineRepr; use ark_ff::UniformRand; - use rand::seq::SliceRandom; + use rand::prelude::SliceRandom; use super::*; use crate::{test_common::*, utils::is_sorted, DkgParams}; diff --git a/ferveo/src/test_common.rs b/ferveo/src/test_common.rs index 22d072a2..21739c7e 100644 --- a/ferveo/src/test_common.rs +++ b/ferveo/src/test_common.rs @@ -35,6 +35,7 @@ pub fn gen_validators(keypairs: &[Keypair]) -> Vec> { .map(|(i, keypair)| Validator { address: gen_address(i), public_key: keypair.public_key(), + share_index: i as u32, }) .collect() } @@ -47,8 +48,7 @@ pub fn setup_dkg_for_n_validators( my_validator_index: usize, ) -> TestSetup { let keypairs = gen_keypairs(shares_num); - let mut validators = gen_validators(keypairs.as_slice()); - validators.sort(); + let validators = gen_validators(keypairs.as_slice()); let me = validators[my_validator_index].clone(); let dkg = PubliclyVerifiableDkg::new( &validators, diff --git a/ferveo/src/validator.rs b/ferveo/src/validator.rs index d931ca06..146e62eb 100644 --- a/ferveo/src/validator.rs +++ b/ferveo/src/validator.rs @@ -45,6 +45,8 @@ pub struct Validator { pub address: EthereumAddress, /// The Public key pub public_key: PublicKey, + /// The index of the validator in the given ritual + pub share_index: u32, } impl PartialOrd for Validator { @@ -64,10 +66,12 @@ impl Validator { pub fn new( address: String, public_key: PublicKey, + share_index: u32, ) -> Result { Ok(Self { address: EthereumAddress::from_str(&address)?, public_key, + share_index, }) } } From 6670da7df8e0ff8c2210900cdb44a18dfd220892 Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Thu, 25 Jan 2024 13:00:28 +0100 Subject: [PATCH 2/9] 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(()) +} From 935be2dc056a8d295fff8c2e937fc23f8fa80f7e Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Thu, 25 Jan 2024 14:26:19 +0100 Subject: [PATCH 3/9] refactor(validator): replace dkg validator with validator --- ferveo/examples/bench_primitives_size.rs | 5 +- ferveo/src/api.rs | 7 +- ferveo/src/dkg.rs | 87 +++++++++--------------- ferveo/src/lib.rs | 20 +++--- ferveo/src/pvss.rs | 22 ++---- ferveo/src/test_common.rs | 2 +- 6 files changed, 56 insertions(+), 87 deletions(-) diff --git a/ferveo/examples/bench_primitives_size.rs b/ferveo/examples/bench_primitives_size.rs index 8f993c36..d44d394c 100644 --- a/ferveo/examples/bench_primitives_size.rs +++ b/ferveo/examples/bench_primitives_size.rs @@ -96,14 +96,13 @@ fn setup( for i in 0..shares_num { let mut dkg = setup_dkg(i as usize, shares_num, security_threshold); let message = dkg.share(rng).expect("Test failed"); - let sender = dkg.get_validator(&dkg.me.validator.public_key).unwrap(); + let sender = dkg.get_validator(&dkg.me.public_key).unwrap(); transcripts.push((sender.clone(), message.clone())); } let mut dkg = setup_dkg(0, shares_num, security_threshold); for (sender, pvss) in transcripts.into_iter() { - dkg.apply_message(&sender.validator, &pvss) - .expect("Setup failed"); + dkg.apply_message(&sender, &pvss).expect("Setup failed"); } dkg } diff --git a/ferveo/src/api.rs b/ferveo/src/api.rs index b8c0febf..26da8c57 100644 --- a/ferveo/src/api.rs +++ b/ferveo/src/api.rs @@ -327,7 +327,7 @@ impl AggregatedTranscript { &ciphertext_header.0, aad, &validator_keypair.decryption_key, - dkg.0.me.share_index, + dkg.0.me.share_index as usize, &domain_points, &dkg.0.pvss_params.g_inv(), ) @@ -344,12 +344,13 @@ impl AggregatedTranscript { &ciphertext_header.0, aad, &validator_keypair.decryption_key, - dkg.0.me.share_index, + dkg.0.me.share_index as usize, &dkg.0.pvss_params.g_inv(), )?; + let domain_point = dkg.0.domain.element(dkg.0.me.share_index as usize); Ok(DecryptionShareSimple { share, - domain_point: dkg.0.domain.element(dkg.0.me.share_index), + domain_point, }) } } diff --git a/ferveo/src/dkg.rs b/ferveo/src/dkg.rs index e5615d05..f3c0dc15 100644 --- a/ferveo/src/dkg.rs +++ b/ferveo/src/dkg.rs @@ -1,6 +1,6 @@ -use std::{cmp::Ordering, collections::BTreeMap}; +use std::collections::BTreeMap; -use ark_ec::{pairing::Pairing, AffineRepr, CurveGroup, Group}; +use ark_ec::{pairing::Pairing, AffineRepr, CurveGroup}; use ark_poly::EvaluationDomain; use ferveo_common::PublicKey; use measure_time::print_time; @@ -62,35 +62,27 @@ impl DkgParams { } } -#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)] -pub struct DkgValidator { - pub validator: Validator, - pub share_index: usize, -} - -impl PartialOrd for DkgValidator { - fn partial_cmp(&self, other: &Self) -> Option { - Some(self.cmp(other)) - } -} - -impl Ord for DkgValidator { - fn cmp(&self, other: &Self) -> Ordering { - self.share_index.cmp(&other.share_index) - } -} - -pub type ValidatorsMap = BTreeMap>; +pub type ValidatorsMap = BTreeMap>; pub type PVSSMap = BTreeMap>; #[derive(Debug, Clone)] pub enum DkgState { + // TODO: Do we need to keep track of the block number? Sharing { accumulated_shares: u32, block: u32 }, Dealt, Success { public_key: E::G1Affine }, Invalid, } +impl DkgState { + fn new() -> Self { + DkgState::Sharing { + accumulated_shares: 0, + block: 0, + } + } +} + /// The DKG context that holds all of the local state for participating in the DKG // TODO: Consider removing Clone to avoid accidentally NOT-mutating state. // Currently, we're assuming that the DKG is only mutated by the owner of the instance. @@ -102,7 +94,7 @@ pub struct PubliclyVerifiableDkg { pub validators: ValidatorsMap, pub vss: PVSSMap, pub domain: ark_poly::GeneralEvaluationDomain, - pub me: DkgValidator, + pub me: Validator, pub state: DkgState, } @@ -128,20 +120,14 @@ impl PubliclyVerifiableDkg { let validators: ValidatorsMap = validators .iter() .enumerate() - .map(|(validator_index, validator)| { - ( - validator.address.clone(), - DkgValidator { - validator: validator.clone(), - share_index: validator_index, - }, - ) + .map(|(_validator_index, validator)| { + (validator.address.clone(), validator.clone()) }) .collect(); // Make sure that `me` is a known validator if let Some(my_validator) = validators.get(&me.address) { - if my_validator.validator.public_key != me.public_key { + if my_validator.public_key != me.public_key { return Err(Error::ValidatorPublicKeyMismatch); } } else { @@ -150,32 +136,22 @@ impl PubliclyVerifiableDkg { Ok(Self { dkg_params: *dkg_params, - pvss_params: PubliclyVerifiableParams:: { - g: E::G1::generator(), - h: E::G2::generator(), - }, - vss: BTreeMap::new(), + pvss_params: PubliclyVerifiableParams::::default(), + vss: PVSSMap::::new(), domain, - me: DkgValidator { - validator: me.clone(), - share_index: validators[&me.address].share_index, - }, + me: me.clone(), validators, - state: DkgState::Sharing { - accumulated_shares: 0, - // TODO: Do we need to keep track of the block number? - block: 0, - }, + state: DkgState::new(), }) } pub fn get_validator( &self, public_key: &PublicKey, - ) -> Option<&DkgValidator> { + ) -> Option<&Validator> { self.validators .values() - .find(|validator| &validator.validator.public_key == public_key) + .find(|validator| &validator.public_key == public_key) } /// Create a new PVSS instance within this DKG session, contributing to the final key @@ -414,7 +390,7 @@ mod test_dealing { for i in 0..dkg.dkg_params.shares_num() { let (mut dkg, _) = setup_dkg(i as usize); let message = dkg.share(rng).unwrap(); - let sender = dkg.me.validator.clone(); + let sender = dkg.me.clone(); messages.push((sender, message)); } @@ -497,7 +473,7 @@ mod test_dealing { let pvss = dkg.share(rng).unwrap(); // This validator has already sent a PVSS - let sender = dkg.me.validator.clone(); + let sender = dkg.me.clone(); // First PVSS is accepted assert!(dkg.verify_message(&sender, &pvss).is_ok()); @@ -540,7 +516,7 @@ mod test_dealing { } )); - let sender = dkg.me.validator.clone(); + let sender = dkg.me.clone(); // Sender verifies it's own PVSS transcript assert!(dkg.verify_message(&sender, &pvss).is_ok()); @@ -594,8 +570,7 @@ mod test_dealing { } )); - let sender = dkg.me.validator.clone(); - + let sender = dkg.me.clone(); dkg.state = DkgState::Success { public_key: G1::zero(), }; @@ -623,7 +598,7 @@ mod test_aggregation { fn test_aggregate() { let (mut dkg, _) = setup_dealt_dkg(); let aggregate = dkg.aggregate().unwrap(); - let sender = dkg.me.validator.clone(); + let sender = dkg.me.clone(); assert!(dkg.verify_message(&sender, &aggregate).is_ok()); assert!(dkg.apply_message(&sender, &aggregate).is_ok()); assert!(matches!(dkg.state, DkgState::Success { .. })); @@ -652,7 +627,7 @@ mod test_aggregation { fn test_aggregate_message_state_guards() { let (mut dkg, _) = setup_dealt_dkg(); let aggregate = dkg.aggregate().unwrap(); - let sender = dkg.me.validator.clone(); + let sender = dkg.me.clone(); dkg.state = DkgState::Sharing { accumulated_shares: 0, @@ -675,7 +650,7 @@ mod test_aggregation { let (mut dkg, _) = setup_dealt_dkg(); dkg.dkg_params.shares_num = 10; let aggregate = dkg.aggregate().unwrap(); - let sender = dkg.me.validator.clone(); + let sender = dkg.me.clone(); assert!(dkg.verify_message(&sender, &aggregate).is_err()); } @@ -693,7 +668,7 @@ mod test_aggregation { { *public_key = G1::zero(); } - let sender = dkg.me.validator.clone(); + let sender = dkg.me.clone(); assert!(dkg.verify_message(&sender, &aggregate).is_err()); } } diff --git a/ferveo/src/lib.rs b/ferveo/src/lib.rs index 0db10105..0ca9252e 100644 --- a/ferveo/src/lib.rs +++ b/ferveo/src/lib.rs @@ -177,7 +177,7 @@ mod test_dkg_full { ciphertext_header, aad, &validator_keypair.decryption_key, - validator.share_index, + validator.share_index as usize, &dkg.pvss_params.g_inv(), ) .unwrap() @@ -277,7 +277,7 @@ mod test_dkg_full { &ciphertext.header().unwrap(), AAD, &validator_keypair.decryption_key, - validator.share_index, + validator.share_index as usize, &domain_points, &dkg.pvss_params.g_inv(), ) @@ -431,12 +431,14 @@ mod test_dkg_full { // Current participant receives updates from other participants let updates_for_participant: Vec<_> = share_updates .values() - .map(|updates| *updates.get(validator.share_index).unwrap()) + .map(|updates| { + *updates.get(validator.share_index as usize).unwrap() + }) .collect(); // Each validator uses their decryption key to update their share let decryption_key = validator_keypairs - .get(validator.share_index) + .get(validator.share_index as usize) .unwrap() .decryption_key; @@ -446,7 +448,7 @@ mod test_dkg_full { pvss_aggregated .update_private_key_share_for_recovery( &decryption_key, - validator.share_index, + validator.share_index as usize, updates_for_participant.as_slice(), ) .unwrap() @@ -572,12 +574,14 @@ mod test_dkg_full { // Current participant receives updates from other participants let updates_for_participant: Vec<_> = share_updates .values() - .map(|updates| *updates.get(validator.share_index).unwrap()) + .map(|updates| { + *updates.get(validator.share_index as usize).unwrap() + }) .collect(); // Each validator uses their decryption key to update their share let decryption_key = validator_keypairs - .get(validator.share_index) + .get(validator.share_index as usize) .unwrap() .decryption_key; @@ -587,7 +591,7 @@ mod test_dkg_full { pvss_aggregated .update_private_key_share_for_recovery( &decryption_key, - validator.share_index, + validator.share_index as usize, updates_for_participant.as_slice(), ) .unwrap() diff --git a/ferveo/src/pvss.rs b/ferveo/src/pvss.rs index c2b075b4..af83aa81 100644 --- a/ferveo/src/pvss.rs +++ b/ferveo/src/pvss.rs @@ -150,8 +150,8 @@ impl PubliclyVerifiableSS { // ek_{i}^{eval_i}, i = validator index fast_multiexp( // &evals.evals[i..i] = &evals.evals[i] - &[evals.evals[validator.share_index]], // one share per validator - validator.validator.public_key.encryption_key.into_group(), + &[evals.evals[validator.share_index as usize]], // one share per validator + validator.public_key.encryption_key.into_group(), )[0] }) .collect::>>(); @@ -198,17 +198,12 @@ impl PubliclyVerifiableSS { /// transcript was at fault so that the can issue a new one. This /// function may also be used for that purpose. pub fn verify_full(&self, dkg: &PubliclyVerifiableDkg) -> bool { - let validators = dkg - .validators - .values() - .map(|v| v.validator.clone()) - .collect::>(); - let validators = validators.as_slice(); + let validators = dkg.validators.values().cloned().collect::>(); do_verify_full( &self.coeffs, &self.shares, &dkg.pvss_params, - validators, + &validators, &dkg.domain, ) } @@ -287,17 +282,12 @@ impl PubliclyVerifiableSS { &self, dkg: &PubliclyVerifiableDkg, ) -> Result { - let validators = dkg - .validators - .values() - .map(|v| v.validator.clone()) - .collect::>(); - let validators = validators.as_slice(); + let validators = dkg.validators.values().cloned().collect::>(); do_verify_aggregation( &self.coeffs, &self.shares, &dkg.pvss_params, - validators, + &validators, &dkg.domain, &dkg.vss, ) diff --git a/ferveo/src/test_common.rs b/ferveo/src/test_common.rs index 21739c7e..4faae733 100644 --- a/ferveo/src/test_common.rs +++ b/ferveo/src/test_common.rs @@ -91,7 +91,7 @@ pub fn setup_dealt_dkg_with( shares_num, my_index as usize, ); - let me = dkg.me.validator.clone(); + let me = dkg.me.clone(); let message = dkg.share(rng).unwrap(); (me, message) }) From 280c37e8eda75a982b0c281f8da655f149c035df Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Mon, 29 Jan 2024 14:16:42 +0100 Subject: [PATCH 4/9] chore: remove unused code --- ferveo/src/lib.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/ferveo/src/lib.rs b/ferveo/src/lib.rs index 0ca9252e..0fca990c 100644 --- a/ferveo/src/lib.rs +++ b/ferveo/src/lib.rs @@ -3,9 +3,6 @@ #[cfg(feature = "bindings-wasm")] extern crate alloc; -use ark_ec::pairing::Pairing; -use itertools::zip_eq; - #[cfg(feature = "bindings-python")] pub mod bindings_python; @@ -120,17 +117,6 @@ pub enum Error { pub type Result = std::result::Result; -pub fn make_pvss_map( - transcripts: &[PubliclyVerifiableSS], - validators: &[Validator], -) -> PVSSMap { - let mut pvss_map: PVSSMap = PVSSMap::new(); - zip_eq(transcripts, validators).for_each(|(transcript, validator)| { - pvss_map.insert(validator.address.clone(), transcript.clone()); - }); - pvss_map -} - #[cfg(test)] mod test_dkg_full { use std::collections::HashMap; From 4af8017fa6921c14080dab7790f519cd9394a7d5 Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Mon, 29 Jan 2024 14:56:43 +0100 Subject: [PATCH 5/9] refactor: unify share creating methods --- ferveo-wasm/tests/node.rs | 2 +- ferveo/src/api.rs | 12 ++++++++---- ferveo/src/bindings_python.rs | 4 ++-- ferveo/src/bindings_wasm.rs | 2 +- ferveo/src/dkg.rs | 16 ++++++---------- 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/ferveo-wasm/tests/node.rs b/ferveo-wasm/tests/node.rs index 68e5f641..bae5750b 100644 --- a/ferveo-wasm/tests/node.rs +++ b/ferveo-wasm/tests/node.rs @@ -32,7 +32,7 @@ fn setup_dkg(shares_num: u32, security_threshold: u32) -> TestSetup { // Each validator holds their own DKG instance and generates a transcript every // validator, including themselves let messages = validators.iter().map(|sender| { - let dkg = Dkg::new( + let mut dkg = Dkg::new( TAU, shares_num, security_threshold, diff --git a/ferveo/src/api.rs b/ferveo/src/api.rs index 26da8c57..aab043d6 100644 --- a/ferveo/src/api.rs +++ b/ferveo/src/api.rs @@ -30,7 +30,7 @@ use crate::bindings_python; use crate::bindings_wasm; pub use crate::EthereumAddress; use crate::{ - do_verify_aggregation, Error, PVSSMap, PubliclyVerifiableParams, + do_verify_aggregation, Error, Message, PVSSMap, PubliclyVerifiableParams, PubliclyVerifiableSS, Result, }; @@ -222,10 +222,14 @@ impl Dkg { } pub fn generate_transcript( - &self, + &mut self, rng: &mut R, ) -> Result { - self.0.create_share(rng) + match self.0.share(rng) { + Ok(Message::Deal(transcript)) => Ok(transcript), + Err(e) => Err(e), + _ => Err(Error::InvalidDkgStateToDeal), + } } pub fn aggregate_transcripts( @@ -435,7 +439,7 @@ mod test_ferveo_api { let messages: Vec<_> = validators .iter() .map(|sender| { - let dkg = Dkg::new( + let mut dkg = Dkg::new( tau, shares_num, security_threshold, diff --git a/ferveo/src/bindings_python.rs b/ferveo/src/bindings_python.rs index 69fe1b8d..c35dc291 100644 --- a/ferveo/src/bindings_python.rs +++ b/ferveo/src/bindings_python.rs @@ -491,7 +491,7 @@ impl Dkg { DkgPublicKey(self.0.public_key()) } - pub fn generate_transcript(&self) -> PyResult { + pub fn generate_transcript(&mut self) -> PyResult { let rng = &mut thread_rng(); let transcript = self .0 @@ -776,7 +776,7 @@ mod test_ferveo_python { .iter() .cloned() .map(|sender| { - let dkg = Dkg::new( + let mut dkg = Dkg::new( tau, shares_num, security_threshold, diff --git a/ferveo/src/bindings_wasm.rs b/ferveo/src/bindings_wasm.rs index 1c0d2a0b..5a23909a 100644 --- a/ferveo/src/bindings_wasm.rs +++ b/ferveo/src/bindings_wasm.rs @@ -366,7 +366,7 @@ impl Dkg { } #[wasm_bindgen(js_name = "generateTranscript")] - pub fn generate_transcript(&self) -> JsResult { + pub fn generate_transcript(&mut self) -> JsResult { let rng = &mut thread_rng(); let transcript = self.0.generate_transcript(rng).map_err(map_js_err)?; Ok(Transcript(transcript)) diff --git a/ferveo/src/dkg.rs b/ferveo/src/dkg.rs index f3c0dc15..10f73d7e 100644 --- a/ferveo/src/dkg.rs +++ b/ferveo/src/dkg.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; use ark_ec::{pairing::Pairing, AffineRepr, CurveGroup}; use ark_poly::EvaluationDomain; +use ark_std::UniformRand; use ferveo_common::PublicKey; use measure_time::print_time; use rand::RngCore; @@ -159,24 +160,19 @@ impl PubliclyVerifiableDkg { /// Returns a PVSS dealing message to post on-chain pub fn share(&mut self, rng: &mut R) -> Result> { print_time!("PVSS Sharing"); - let vss = self.create_share(rng)?; match self.state { DkgState::Sharing { .. } | DkgState::Dealt => { + let vss = PubliclyVerifiableSS::::new( + &E::ScalarField::rand(rng), + self, + rng, + )?; Ok(Message::Deal(vss)) } _ => Err(Error::InvalidDkgStateToDeal), } } - // TODO: Make private, use `share` instead. Currently used only in bindings - pub fn create_share( - &self, - rng: &mut R, - ) -> Result> { - use ark_std::UniformRand; - PubliclyVerifiableSS::::new(&E::ScalarField::rand(rng), self, rng) - } - /// Aggregate all received PVSS messages into a single message, prepared to post on-chain pub fn aggregate(&self) -> Result> { match self.state { From d547a81743fc21679cc2e2d199c7e2f97424fe5d Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Mon, 29 Jan 2024 15:09:18 +0100 Subject: [PATCH 6/9] chore: remove unused code --- ferveo/src/pvss.rs | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/ferveo/src/pvss.rs b/ferveo/src/pvss.rs index af83aa81..a5e458d7 100644 --- a/ferveo/src/pvss.rs +++ b/ferveo/src/pvss.rs @@ -417,32 +417,6 @@ pub fn aggregate( } } -pub fn aggregate_for_decryption( - dkg: &PubliclyVerifiableDkg, -) -> Vec> { - // From docs: https://nikkolasg.github.io/ferveo/pvss.html?highlight=aggregate#aggregation - // "Two PVSS instances may be aggregated into a single PVSS instance by adding elementwise each of the corresponding group elements." - let shares = dkg - .vss - .values() - .map(|pvss| pvss.shares.clone()) - .collect::>(); - let first_share = shares - .first() - .expect("Need one or more decryption shares to aggregate") - .to_vec(); - shares - .into_iter() - .skip(1) - // We're assuming that in every PVSS instance, the shares are in the same order - .fold(first_share, |acc, shares| { - acc.into_iter() - .zip_eq(shares) - .map(|(a, b)| (a + b).into()) - .collect() - }) -} - #[cfg(test)] mod test_pvss { use ark_bls12_381::Bls12_381 as EllipticCurve; From e6a7f6e55a34d892e664160f1f8cffe6e88c79da Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Mon, 29 Jan 2024 17:18:53 +0100 Subject: [PATCH 7/9] feature(dkg): prevent panics during transcript aggregation --- ferveo/src/api.rs | 56 +++++++++++++++++++++++++++++++---- ferveo/src/bindings_python.rs | 10 +++++-- ferveo/src/bindings_wasm.rs | 3 +- ferveo/src/dkg.rs | 2 +- ferveo/src/lib.rs | 14 +++++---- ferveo/src/pvss.rs | 30 +++++++++---------- 6 files changed, 85 insertions(+), 30 deletions(-) diff --git a/ferveo/src/api.rs b/ferveo/src/api.rs index aab043d6..c8a9f6c3 100644 --- a/ferveo/src/api.rs +++ b/ferveo/src/api.rs @@ -246,7 +246,7 @@ impl Dkg { for (validator, transcript) in messages { self.0.deal(validator, transcript)?; } - Ok(AggregatedTranscript(crate::pvss::aggregate(&self.0.vss))) + Ok(AggregatedTranscript(crate::pvss::aggregate(&self.0.vss)?)) } pub fn public_params(&self) -> DkgPublicParameters { @@ -268,9 +268,9 @@ fn make_pvss_map(messages: &[ValidatorMessage]) -> PVSSMap { pub struct AggregatedTranscript(PubliclyVerifiableSS); impl AggregatedTranscript { - pub fn new(messages: &[ValidatorMessage]) -> Self { + pub fn new(messages: &[ValidatorMessage]) -> Result { let pvss_map = make_pvss_map(messages); - AggregatedTranscript(crate::pvss::aggregate(&pvss_map)) + Ok(AggregatedTranscript(crate::pvss::aggregate(&pvss_map)?)) } pub fn verify( @@ -625,6 +625,10 @@ mod test_ferveo_api { assert!(result.is_err()); } + // Note that the server and client code are using the same underlying + // implementation for aggregation and aggregate verification. + // Here, we focus on testing user-facing APIs for server and client users. + #[test] fn server_side_local_verification() { let rng = &mut StdRng::seed_from_u64(0); @@ -643,6 +647,41 @@ mod test_ferveo_api { assert!(local_aggregate .verify(dkg.0.dkg_params.shares_num(), &messages) .is_ok()); + + // Test negative cases + + // Notice that the dkg instance is mutable, so we need to get a fresh one + // for every test case + + // Should fail if no transcripts are provided + let mut dkg = + Dkg::new(TAU, SHARES_NUM, SECURITY_THRESHOLD, &validators, &me) + .unwrap(); + let result = dkg.aggregate_transcripts(&[]); + assert!(result.is_err()); + + // Not enough transcripts + let mut dkg = + Dkg::new(TAU, SHARES_NUM, SECURITY_THRESHOLD, &validators, &me) + .unwrap(); + let not_enough_messages = &messages[..SECURITY_THRESHOLD as usize - 1]; + assert!(not_enough_messages.len() < SECURITY_THRESHOLD as usize); + let insufficient_aggregate = + dkg.aggregate_transcripts(not_enough_messages).unwrap(); + let result = insufficient_aggregate.verify(SHARES_NUM, &messages); + assert!(result.is_err()); + + // Unexpected transcripts in the aggregate or transcripts from a different ritual + // Using same DKG parameters, but different DKG instances and validators + let mut dkg = + Dkg::new(TAU, SHARES_NUM, SECURITY_THRESHOLD, &validators, &me) + .unwrap(); + let (bad_messages, _, _) = + make_test_inputs(rng, TAU, SECURITY_THRESHOLD, SHARES_NUM); + let mixed_messages = [&messages[..2], &bad_messages[..1]].concat(); + let bad_aggregate = dkg.aggregate_transcripts(&mixed_messages).unwrap(); + let result = bad_aggregate.verify(SHARES_NUM, &messages); + assert!(result.is_err()); } #[test] @@ -656,7 +695,8 @@ mod test_ferveo_api { let messages = &messages[..SECURITY_THRESHOLD as usize]; // Create an aggregated transcript on the client side - let aggregated_transcript = AggregatedTranscript::new(messages); + let aggregated_transcript = + AggregatedTranscript::new(messages).unwrap(); // We are separating the verification from the aggregation since the client may fetch // the aggregate from a side-channel or decide to persist it and verify it later @@ -668,11 +708,15 @@ mod test_ferveo_api { // Test negative cases + // Should fail if no transcripts are provided + let result = AggregatedTranscript::new(&[]); + assert!(result.is_err()); + // Not enough transcripts let not_enough_messages = &messages[..SECURITY_THRESHOLD as usize - 1]; assert!(not_enough_messages.len() < SECURITY_THRESHOLD as usize); let insufficient_aggregate = - AggregatedTranscript::new(not_enough_messages); + AggregatedTranscript::new(not_enough_messages).unwrap(); let result = insufficient_aggregate.verify(SHARES_NUM, messages); assert!(result.is_err()); @@ -681,7 +725,7 @@ mod test_ferveo_api { let (bad_messages, _, _) = make_test_inputs(rng, TAU, SECURITY_THRESHOLD, SHARES_NUM); let mixed_messages = [&messages[..2], &bad_messages[..1]].concat(); - let bad_aggregate = AggregatedTranscript::new(&mixed_messages); + let bad_aggregate = AggregatedTranscript::new(&mixed_messages).unwrap(); let result = bad_aggregate.verify(SHARES_NUM, messages); assert!(result.is_err()); } diff --git a/ferveo/src/bindings_python.rs b/ferveo/src/bindings_python.rs index c35dc291..fab67561 100644 --- a/ferveo/src/bindings_python.rs +++ b/ferveo/src/bindings_python.rs @@ -113,6 +113,9 @@ impl From for PyErr { "{index}" )) }, + Error::NoTranscriptsToAggregate => { + NoTranscriptsToAggregate::new_err("") + }, }, _ => default(), } @@ -149,6 +152,7 @@ create_exception!(exceptions, InvalidByteLength, PyValueError); create_exception!(exceptions, InvalidVariant, PyValueError); create_exception!(exceptions, InvalidDkgParameters, PyValueError); create_exception!(exceptions, InvalidShareIndex, PyValueError); +create_exception!(exceptions, NoTranscriptsToAggregate, PyValueError); fn from_py_bytes(bytes: &[u8]) -> PyResult { T::from_bytes(bytes) @@ -580,10 +584,12 @@ generate_bytes_serialization!(AggregatedTranscript); #[pymethods] impl AggregatedTranscript { #[new] - pub fn new(messages: Vec) -> Self { + pub fn new(messages: Vec) -> PyResult { let messages: Vec<_> = messages.into_iter().map(|vm| vm.to_inner()).collect(); - Self(api::AggregatedTranscript::new(&messages)) + let inner = api::AggregatedTranscript::new(&messages) + .map_err(FerveoPythonError::FerveoError)?; + Ok(Self(inner)) } pub fn verify( diff --git a/ferveo/src/bindings_wasm.rs b/ferveo/src/bindings_wasm.rs index 5a23909a..1396de13 100644 --- a/ferveo/src/bindings_wasm.rs +++ b/ferveo/src/bindings_wasm.rs @@ -507,7 +507,8 @@ impl AggregatedTranscript { ) -> JsResult { set_panic_hook(); let messages = unwrap_messages_js(messages)?; - let aggregated_transcript = api::AggregatedTranscript::new(&messages); + let aggregated_transcript = + api::AggregatedTranscript::new(&messages).unwrap(); Ok(AggregatedTranscript(aggregated_transcript)) } diff --git a/ferveo/src/dkg.rs b/ferveo/src/dkg.rs index 10f73d7e..8259e5c3 100644 --- a/ferveo/src/dkg.rs +++ b/ferveo/src/dkg.rs @@ -179,7 +179,7 @@ impl PubliclyVerifiableDkg { DkgState::Dealt => { let public_key = self.public_key(); Ok(Message::Aggregate(Aggregation { - vss: aggregate(&self.vss), + vss: aggregate(&self.vss)?, public_key, })) } diff --git a/ferveo/src/lib.rs b/ferveo/src/lib.rs index 0fca990c..f2eb357a 100644 --- a/ferveo/src/lib.rs +++ b/ferveo/src/lib.rs @@ -113,6 +113,10 @@ pub enum Error { /// DKG may not contain duplicated share indices #[error("Duplicated share index: {0}")] DuplicatedShareIndex(u32), + + /// Creating a transcript aggregate requires at least one transcript + #[error("No transcripts to aggregate")] + NoTranscriptsToAggregate, } pub type Result = std::result::Result; @@ -148,7 +152,7 @@ mod test_dkg_full { Vec>, SharedSecret, ) { - let pvss_aggregated = aggregate(&dkg.vss); + let pvss_aggregated = aggregate(&dkg.vss).unwrap(); assert!(pvss_aggregated.verify_aggregation(dkg).is_ok()); let decryption_shares: Vec> = @@ -243,7 +247,7 @@ mod test_dkg_full { ) .unwrap(); - let pvss_aggregated = aggregate(&dkg.vss); + let pvss_aggregated = aggregate(&dkg.vss).unwrap(); pvss_aggregated.verify_aggregation(&dkg).unwrap(); let domain_points = dkg .domain @@ -430,7 +434,7 @@ mod test_dkg_full { // Creates updated private key shares // TODO: Why not using dkg.aggregate()? - let pvss_aggregated = aggregate(&dkg.vss); + let pvss_aggregated = aggregate(&dkg.vss).unwrap(); pvss_aggregated .update_private_key_share_for_recovery( &decryption_key, @@ -461,7 +465,7 @@ mod test_dkg_full { .enumerate() .map(|(share_index, validator_keypair)| { // TODO: Why not using dkg.aggregate()? - let pvss_aggregated = aggregate(&dkg.vss); + let pvss_aggregated = aggregate(&dkg.vss).unwrap(); pvss_aggregated .make_decryption_share_simple( &ciphertext.header().unwrap(), @@ -573,7 +577,7 @@ mod test_dkg_full { // Creates updated private key shares // TODO: Why not using dkg.aggregate()? - let pvss_aggregated = aggregate(&dkg.vss); + let pvss_aggregated = aggregate(&dkg.vss).unwrap(); pvss_aggregated .update_private_key_share_for_recovery( &decryption_key, diff --git a/ferveo/src/pvss.rs b/ferveo/src/pvss.rs index a5e458d7..60910158 100644 --- a/ferveo/src/pvss.rs +++ b/ferveo/src/pvss.rs @@ -381,13 +381,13 @@ impl PubliclyVerifiableSS { /// Aggregate the PVSS instances in `pvss` from DKG session `dkg` /// into a new PVSS instance /// See: https://nikkolasg.github.io/ferveo/pvss.html?highlight=aggregate#aggregation -pub fn aggregate( +pub(crate) fn aggregate( pvss_map: &PVSSMap, -) -> PubliclyVerifiableSS { - let mut pvss_iter = pvss_map.iter(); - let (_, first_pvss) = pvss_iter +) -> Result> { + let mut pvss_iter = pvss_map.values(); + let first_pvss = pvss_iter .next() - .expect("May not aggregate empty PVSS instances"); + .ok_or_else(|| Error::NoTranscriptsToAggregate)?; let mut coeffs = batch_to_projective_g1::(&first_pvss.coeffs); let mut sigma = first_pvss.sigma; @@ -396,25 +396,25 @@ pub fn aggregate( // So now we're iterating over the PVSS instances, and adding their coefficients and shares, and their sigma // sigma is the sum of all the sigma_i, which is the proof of knowledge of the secret polynomial // Aggregating is just adding the corresponding values in pvss instances, so pvss = pvss + pvss_j - for (_, next) in pvss_iter { - sigma = (sigma + next.sigma).into(); + for next_pvss in pvss_iter { + sigma = (sigma + next_pvss.sigma).into(); coeffs .iter_mut() - .zip_eq(next.coeffs.iter()) + .zip_eq(next_pvss.coeffs.iter()) .for_each(|(a, b)| *a += b); shares .iter_mut() - .zip_eq(next.shares.iter()) + .zip_eq(next_pvss.shares.iter()) .for_each(|(a, b)| *a += b); } let shares = E::G2::normalize_batch(&shares); - PubliclyVerifiableSS { + Ok(PubliclyVerifiableSS { coeffs: E::G1::normalize_batch(&coeffs), shares, sigma, phantom: Default::default(), - } + }) } #[cfg(test)] @@ -526,7 +526,7 @@ mod test_pvss { #[test] fn test_aggregate_pvss() { let (dkg, _) = setup_dealt_dkg(); - let aggregate = aggregate(&dkg.vss); + let aggregate = aggregate(&dkg.vss).unwrap(); // Check that a polynomial of the correct degree was created assert_eq!( aggregate.coeffs.len(), @@ -542,15 +542,15 @@ mod test_pvss { assert!(aggregate.verify_aggregation(&dkg).expect("Test failed"),); } - /// Check that if the aggregated pvss transcript has an + /// Check that if the aggregated PVSS transcript has an /// incorrect constant term, the verification fails #[test] fn test_verify_aggregation_fails_if_constant_term_wrong() { let (dkg, _) = setup_dealt_dkg(); - let mut aggregated = aggregate(&dkg.vss); + let mut aggregated = aggregate(&dkg.vss).unwrap(); while aggregated.coeffs[0] == G1::zero() { let (dkg, _) = setup_dkg(0); - aggregated = aggregate(&dkg.vss); + aggregated = aggregate(&dkg.vss).unwrap(); } aggregated.coeffs[0] = G1::zero(); assert_eq!( From 4bb41585cd6f93e58bbd047c27fd3e68ab9e723e Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Mon, 29 Jan 2024 18:09:36 +0100 Subject: [PATCH 8/9] refactor: refactor aggregate method params --- ferveo/src/api.rs | 15 ++++++++++++--- ferveo/src/dkg.rs | 3 ++- ferveo/src/lib.rs | 16 +++++++++++----- ferveo/src/pvss.rs | 14 +++++++++----- 4 files changed, 34 insertions(+), 14 deletions(-) diff --git a/ferveo/src/api.rs b/ferveo/src/api.rs index c8a9f6c3..32ec9f30 100644 --- a/ferveo/src/api.rs +++ b/ferveo/src/api.rs @@ -246,7 +246,12 @@ impl Dkg { for (validator, transcript) in messages { self.0.deal(validator, transcript)?; } - Ok(AggregatedTranscript(crate::pvss::aggregate(&self.0.vss)?)) + let pvss = messages + .iter() + .map(|(_, t)| t) + .cloned() + .collect::>>(); + Ok(AggregatedTranscript(crate::pvss::aggregate(&pvss)?)) } pub fn public_params(&self) -> DkgPublicParameters { @@ -269,8 +274,12 @@ pub struct AggregatedTranscript(PubliclyVerifiableSS); impl AggregatedTranscript { pub fn new(messages: &[ValidatorMessage]) -> Result { - let pvss_map = make_pvss_map(messages); - Ok(AggregatedTranscript(crate::pvss::aggregate(&pvss_map)?)) + let pvss_list = messages + .iter() + .map(|(_, t)| t) + .cloned() + .collect::>>(); + Ok(AggregatedTranscript(crate::pvss::aggregate(&pvss_list)?)) } pub fn verify( diff --git a/ferveo/src/dkg.rs b/ferveo/src/dkg.rs index 8259e5c3..3bba9b47 100644 --- a/ferveo/src/dkg.rs +++ b/ferveo/src/dkg.rs @@ -178,8 +178,9 @@ impl PubliclyVerifiableDkg { match self.state { DkgState::Dealt => { let public_key = self.public_key(); + let pvss_list = self.vss.values().cloned().collect::>(); Ok(Message::Aggregate(Aggregation { - vss: aggregate(&self.vss)?, + vss: aggregate(&pvss_list)?, public_key, })) } diff --git a/ferveo/src/lib.rs b/ferveo/src/lib.rs index f2eb357a..a7e5e7a4 100644 --- a/ferveo/src/lib.rs +++ b/ferveo/src/lib.rs @@ -152,7 +152,8 @@ mod test_dkg_full { Vec>, SharedSecret, ) { - let pvss_aggregated = aggregate(&dkg.vss).unwrap(); + let pvss_list = dkg.vss.values().cloned().collect::>(); + let pvss_aggregated = aggregate(&pvss_list).unwrap(); assert!(pvss_aggregated.verify_aggregation(dkg).is_ok()); let decryption_shares: Vec> = @@ -247,7 +248,8 @@ mod test_dkg_full { ) .unwrap(); - let pvss_aggregated = aggregate(&dkg.vss).unwrap(); + let pvss_list = dkg.vss.values().cloned().collect::>(); + let pvss_aggregated = aggregate(&pvss_list).unwrap(); pvss_aggregated.verify_aggregation(&dkg).unwrap(); let domain_points = dkg .domain @@ -434,7 +436,8 @@ mod test_dkg_full { // Creates updated private key shares // TODO: Why not using dkg.aggregate()? - let pvss_aggregated = aggregate(&dkg.vss).unwrap(); + let pvss_list = dkg.vss.values().cloned().collect::>(); + let pvss_aggregated = aggregate(&pvss_list).unwrap(); pvss_aggregated .update_private_key_share_for_recovery( &decryption_key, @@ -465,7 +468,9 @@ mod test_dkg_full { .enumerate() .map(|(share_index, validator_keypair)| { // TODO: Why not using dkg.aggregate()? - let pvss_aggregated = aggregate(&dkg.vss).unwrap(); + let pvss_list = + dkg.vss.values().cloned().collect::>(); + let pvss_aggregated = aggregate(&pvss_list).unwrap(); pvss_aggregated .make_decryption_share_simple( &ciphertext.header().unwrap(), @@ -577,7 +582,8 @@ mod test_dkg_full { // Creates updated private key shares // TODO: Why not using dkg.aggregate()? - let pvss_aggregated = aggregate(&dkg.vss).unwrap(); + let pvss_list = dkg.vss.values().cloned().collect::>(); + let pvss_aggregated = aggregate(&pvss_list).unwrap(); pvss_aggregated .update_private_key_share_for_recovery( &decryption_key, diff --git a/ferveo/src/pvss.rs b/ferveo/src/pvss.rs index 60910158..07328620 100644 --- a/ferveo/src/pvss.rs +++ b/ferveo/src/pvss.rs @@ -382,9 +382,9 @@ impl PubliclyVerifiableSS { /// into a new PVSS instance /// See: https://nikkolasg.github.io/ferveo/pvss.html?highlight=aggregate#aggregation pub(crate) fn aggregate( - pvss_map: &PVSSMap, + pvss_list: &[PubliclyVerifiableSS], ) -> Result> { - let mut pvss_iter = pvss_map.values(); + let mut pvss_iter = pvss_list.iter(); let first_pvss = pvss_iter .next() .ok_or_else(|| Error::NoTranscriptsToAggregate)?; @@ -494,6 +494,7 @@ mod test_pvss { assert!(!bad_pvss.verify_full(&dkg)); } + // TODO: Move this code to dkg.rs /// Check that the canonical share indices of validators are expected and enforced /// by the DKG methods. #[test] @@ -526,7 +527,8 @@ mod test_pvss { #[test] fn test_aggregate_pvss() { let (dkg, _) = setup_dealt_dkg(); - let aggregate = aggregate(&dkg.vss).unwrap(); + let pvss_list = dkg.vss.values().cloned().collect::>(); + let aggregate = aggregate(&pvss_list).unwrap(); // Check that a polynomial of the correct degree was created assert_eq!( aggregate.coeffs.len(), @@ -547,10 +549,12 @@ mod test_pvss { #[test] fn test_verify_aggregation_fails_if_constant_term_wrong() { let (dkg, _) = setup_dealt_dkg(); - let mut aggregated = aggregate(&dkg.vss).unwrap(); + let pvss_list = dkg.vss.values().cloned().collect::>(); + let mut aggregated = aggregate(&pvss_list).unwrap(); while aggregated.coeffs[0] == G1::zero() { let (dkg, _) = setup_dkg(0); - aggregated = aggregate(&dkg.vss).unwrap(); + let pvss_list = dkg.vss.values().cloned().collect::>(); + aggregated = aggregate(&pvss_list).unwrap(); } aggregated.coeffs[0] = G1::zero(); assert_eq!( From 72b8484010979d5564dff9f89556f0e1564911e1 Mon Sep 17 00:00:00 2001 From: Piotr Roslaniec Date: Tue, 30 Jan 2024 09:20:22 +0100 Subject: [PATCH 9/9] chore: document todos --- ferveo/src/api.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ferveo/src/api.rs b/ferveo/src/api.rs index 32ec9f30..4c6defd2 100644 --- a/ferveo/src/api.rs +++ b/ferveo/src/api.rs @@ -224,6 +224,7 @@ impl Dkg { pub fn generate_transcript( &mut self, rng: &mut R, + // TODO: Replace with Message::Deal? ) -> Result { match self.0.share(rng) { Ok(Message::Deal(transcript)) => Ok(transcript), @@ -235,6 +236,7 @@ impl Dkg { pub fn aggregate_transcripts( &mut self, messages: &[ValidatorMessage], + // TODO: Replace with Message::Aggregate? ) -> Result { // We must use `deal` here instead of to produce AggregatedTranscript instead of simply // creating an AggregatedTranscript from the messages, because `deal` also updates the