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-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/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..d44d394c 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() } @@ -95,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 3c2295d1..4c6defd2 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,15 +222,21 @@ impl Dkg { } pub fn generate_transcript( - &self, + &mut self, rng: &mut R, + // TODO: Replace with Message::Deal? ) -> 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( &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 @@ -242,7 +248,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 { @@ -264,9 +275,13 @@ fn make_pvss_map(messages: &[ValidatorMessage]) -> PVSSMap { pub struct AggregatedTranscript(PubliclyVerifiableSS); impl AggregatedTranscript { - pub fn new(messages: &[ValidatorMessage]) -> Self { - let pvss_map = make_pvss_map(messages); - AggregatedTranscript(crate::pvss::aggregate(&pvss_map)) + pub fn new(messages: &[ValidatorMessage]) -> Result { + let pvss_list = messages + .iter() + .map(|(_, t)| t) + .cloned() + .collect::>>(); + Ok(AggregatedTranscript(crate::pvss::aggregate(&pvss_list)?)) } pub fn verify( @@ -327,7 +342,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 +359,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, }) } } @@ -425,6 +441,7 @@ mod test_ferveo_api { .map(|(i, keypair)| Validator { address: gen_address(i), public_key: keypair.public_key(), + share_index: i as u32, }) .collect::>(); @@ -433,7 +450,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, @@ -619,6 +636,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); @@ -637,6 +658,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] @@ -650,7 +706,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 @@ -662,11 +719,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()); @@ -675,7 +736,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 f897c8f6..fab67561 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,14 @@ impl From for PyErr { "num_shares: {num_shares}, security_threshold: {security_threshold}" )) }, + Error::DuplicatedShareIndex(index) => { + InvalidShareIndex::new_err(format!( + "{index}" + )) + }, + Error::NoTranscriptsToAggregate => { + NoTranscriptsToAggregate::new_err("") + }, }, _ => default(), } @@ -145,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) @@ -396,8 +404,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)) } @@ -486,7 +495,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 @@ -575,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( @@ -756,8 +767,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(); @@ -767,7 +782,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 a1310277..1396de13 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)) @@ -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, }) } @@ -503,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)) } @@ -610,6 +615,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..3bba9b47 100644 --- a/ferveo/src/dkg.rs +++ b/ferveo/src/dkg.rs @@ -1,7 +1,8 @@ -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 ark_std::UniformRand; use ferveo_common::PublicKey; use measure_time::print_time; use rand::RngCore; @@ -9,8 +10,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)] @@ -61,35 +63,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. @@ -101,7 +95,7 @@ pub struct PubliclyVerifiableDkg { pub validators: ValidatorsMap, pub vss: PVSSMap, pub domain: ark_poly::GeneralEvaluationDomain, - pub me: DkgValidator, + pub me: Validator, pub state: DkgState, } @@ -122,27 +116,19 @@ 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() - .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 { @@ -151,32 +137,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 @@ -184,31 +160,27 @@ 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 { 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, })) } @@ -375,12 +347,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), @@ -415,7 +387,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)); } @@ -464,6 +436,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()); @@ -497,7 +470,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 +513,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 +567,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 +595,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 +624,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 +647,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 +665,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 c316c815..a7e5e7a4 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; @@ -19,8 +16,6 @@ pub mod pvss; pub mod refresh; pub mod validator; -mod utils; - #[cfg(test)] mod test_common; @@ -85,10 +80,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,21 +109,18 @@ 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), -} -pub type Result = std::result::Result; + /// DKG may not contain duplicated share indices + #[error("Duplicated share index: {0}")] + DuplicatedShareIndex(u32), -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 + /// Creating a transcript aggregate requires at least one transcript + #[error("No transcripts to aggregate")] + NoTranscriptsToAggregate, } +pub type Result = std::result::Result; + #[cfg(test)] mod test_dkg_full { use std::collections::HashMap; @@ -164,7 +152,8 @@ mod test_dkg_full { Vec>, SharedSecret, ) { - let pvss_aggregated = aggregate(&dkg.vss); + 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> = @@ -179,7 +168,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() @@ -259,7 +248,8 @@ mod test_dkg_full { ) .unwrap(); - let pvss_aggregated = aggregate(&dkg.vss); + 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 @@ -279,7 +269,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(), ) @@ -433,22 +423,25 @@ 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; // Creates updated private key shares // TODO: Why not using dkg.aggregate()? - let pvss_aggregated = aggregate(&dkg.vss); + 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, - validator.share_index, + validator.share_index as usize, updates_for_participant.as_slice(), ) .unwrap() @@ -475,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); + let pvss_list = + dkg.vss.values().cloned().collect::>(); + let pvss_aggregated = aggregate(&pvss_list).unwrap(); pvss_aggregated .make_decryption_share_simple( &ciphertext.header().unwrap(), @@ -574,22 +569,25 @@ 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; // Creates updated private key shares // TODO: Why not using dkg.aggregate()? - let pvss_aggregated = aggregate(&dkg.vss); + 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, - 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 c8498bb7..07328620 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, }; @@ -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, ) } @@ -225,8 +220,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 @@ -288,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, ) @@ -392,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( - pvss_map: &PVSSMap, -) -> PubliclyVerifiableSS { - let mut pvss_iter = pvss_map.iter(); - let (_, first_pvss) = pvss_iter +pub(crate) fn aggregate( + pvss_list: &[PubliclyVerifiableSS], +) -> Result> { + let mut pvss_iter = pvss_list.iter(); + 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; @@ -407,51 +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(), - } -} - -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)] @@ -459,10 +422,9 @@ mod test_pvss { use ark_bls12_381::Bls12_381 as EllipticCurve; use ark_ec::AffineRepr; use ark_ff::UniformRand; - use rand::seq::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 +494,20 @@ mod test_pvss { assert!(!bad_pvss.verify_full(&dkg)); } - /// Check that the explicit ordering of validators is expected and enforced + // TODO: Move this code to dkg.rs + /// 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 +518,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() ); } @@ -566,7 +527,8 @@ mod test_pvss { #[test] fn test_aggregate_pvss() { let (dkg, _) = setup_dealt_dkg(); - let aggregate = aggregate(&dkg.vss); + 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(), @@ -582,15 +544,17 @@ 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 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); + let pvss_list = dkg.vss.values().cloned().collect::>(); + aggregated = aggregate(&pvss_list).unwrap(); } aggregated.coeffs[0] = G1::zero(); assert_eq!( diff --git a/ferveo/src/test_common.rs b/ferveo/src/test_common.rs index 22d072a2..4faae733 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, @@ -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) }) 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 d931ca06..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, )] @@ -45,29 +47,36 @@ pub struct Validator { pub address: EthereumAddress, /// The Public key pub public_key: PublicKey, -} - -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) - } + /// The index of the validator in the given ritual + pub share_index: u32, } 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, }) } } + +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(()) +}