From d3f0378f6603eb9ff822d98aba6f08c3d1bbbbdb Mon Sep 17 00:00:00 2001 From: Luke Parker Date: Thu, 1 Aug 2024 05:28:06 -0400 Subject: [PATCH] Deduplicate and better document in processor key_gen --- processor/src/key_gen.rs | 262 ++++++++++++++++++++++----------------- processor/src/main.rs | 2 +- 2 files changed, 151 insertions(+), 113 deletions(-) diff --git a/processor/src/key_gen.rs b/processor/src/key_gen.rs index e34177e2..6c09c192 100644 --- a/processor/src/key_gen.rs +++ b/processor/src/key_gen.rs @@ -1,4 +1,7 @@ -use std::collections::{HashSet, HashMap}; +use std::{ + io, + collections::{HashSet, HashMap}, +}; use zeroize::Zeroizing; @@ -187,6 +190,8 @@ impl KeysDb { ensure at least `t` people participated and the DKG result can be reconstructed. We do lose fault tolerance, yet only by losing those faulty. Accordingly, this is accepted. + + Returns the coerced keys and faulty participants. */ fn coerce_keys( key_bytes: &[impl AsRef<[u8]>], @@ -257,6 +262,7 @@ impl KeyGen { pub fn in_set(&self, session: &Session) -> bool { // We determine if we're in set using if we have the parameters for a session's key generation + // We only have these if we were told to generate a key for this session ParamsDb::get(&self.db, session).is_some() } @@ -284,19 +290,20 @@ impl KeyGen { ) -> Vec { const SUBSTRATE_KEY_CONTEXT: &[u8] = b"substrate"; const NETWORK_KEY_CONTEXT: &[u8] = b"network"; - let context = |session: Session, key| { + fn context(session: Session, key_context: &[u8]) -> [u8; 32] { // TODO2: Also embed the chain ID/genesis block let mut transcript = RecommendedTranscript::new(b"Serai eVRF Key Gen"); transcript.append_message(b"network", N::ID); transcript.append_message(b"session", session.0.to_le_bytes()); - transcript.append_message(b"key", key); - <[u8; 32]>::try_from(&(&transcript.challenge(b"context"))[.. 32]).unwrap() - }; + transcript.append_message(b"key", key_context); + (&(&transcript.challenge(b"context"))[.. 32]).try_into().unwrap() + } match msg { CoordinatorMessage::GenerateKey { session, threshold, evrf_public_keys } => { info!("Generating new key. Session: {session:?}"); + // Unzip the vector of eVRF keys let substrate_evrf_public_keys = evrf_public_keys.iter().map(|(key, _)| *key).collect::>(); let network_evrf_public_keys = @@ -304,40 +311,46 @@ impl KeyGen { let mut participation = Vec::with_capacity(2048); let mut faulty = HashSet::new(); - { - let (coerced_keys, faulty_is) = coerce_keys::(&substrate_evrf_public_keys); + + // Participate for both Substrate and the network + fn participate( + context: [u8; 32], + threshold: u16, + evrf_public_keys: &[impl AsRef<[u8]>], + evrf_private_key: &Zeroizing<::F>, + faulty: &mut HashSet, + output: &mut impl io::Write, + ) { + let (coerced_keys, faulty_is) = coerce_keys::(evrf_public_keys); for faulty_i in faulty_is { faulty.insert(faulty_i); } - EvrfDkg::::participate( + let participation = EvrfDkg::::participate( &mut OsRng, generators(), - context(session, SUBSTRATE_KEY_CONTEXT), + context, threshold, &coerced_keys, - &self.substrate_evrf_private_key, - ) - .unwrap() - .write(&mut participation) - .unwrap(); - } - { - let (coerced_keys, faulty_is) = coerce_keys::(&network_evrf_public_keys); - for faulty_i in faulty_is { - faulty.insert(faulty_i); - } - EvrfDkg::::participate( - &mut OsRng, - generators(), - context(session, NETWORK_KEY_CONTEXT), - threshold, - &coerced_keys, - &self.network_evrf_private_key, - ) - .unwrap() - .write(&mut participation) - .unwrap(); + evrf_private_key, + ); + participation.unwrap().write(output).unwrap(); } + participate::( + context::(session, SUBSTRATE_KEY_CONTEXT), + threshold, + &substrate_evrf_public_keys, + &self.substrate_evrf_private_key, + &mut faulty, + &mut participation, + ); + participate::( + context::(session, NETWORK_KEY_CONTEXT), + threshold, + &network_evrf_public_keys, + &self.network_evrf_private_key, + &mut faulty, + &mut participation, + ); // Save the params ParamsDb::set( @@ -350,11 +363,11 @@ impl KeyGen { let mut faulty = faulty.into_iter().collect::>(); faulty.sort(); - let mut res = Vec::with_capacity(1 + faulty.len()); - res.push(ProcessorMessage::Participation { session, participation }); + let mut res = Vec::with_capacity(faulty.len() + 1); for faulty in faulty { res.push(ProcessorMessage::Blame { session, participant: faulty }); } + res.push(ProcessorMessage::Participation { session, participation }); res } @@ -444,100 +457,125 @@ impl KeyGen { } } - let mut res = Vec::with_capacity(1); - let substrate_dkg = match EvrfDkg::::verify( - &mut OsRng, - generators(), - context(session, SUBSTRATE_KEY_CONTEXT), - threshold, - // Ignores the list of participants who couldn't have their keys coerced due to prior - // handling those - &coerce_keys::(&substrate_evrf_public_keys).0, - &substrate_participations - .iter() - .map(|(key, participation)| { - ( - *key, - Participation::read(&mut participation.as_slice(), n) - .expect("prior read participation was invalid"), + // If we now have the threshold participating, verify their `Participation`s + fn verify_dkg( + txn: &mut impl DbTxn, + session: Session, + true_if_substrate_false_if_network: bool, + threshold: u16, + evrf_public_keys: &[impl AsRef<[u8]>], + substrate_participations: &mut HashMap>, + network_participations: &mut HashMap>, + ) -> Result, Vec> { + // Parse the `Participation`s + let participations = (if true_if_substrate_false_if_network { + &*substrate_participations + } else { + &*network_participations + }) + .iter() + .map(|(key, participation)| { + ( + *key, + Participation::read( + &mut participation.as_slice(), + evrf_public_keys.len().try_into().unwrap(), ) - }) - .collect(), - ) - .unwrap() - { - VerifyResult::Valid(dkg) => dkg, - VerifyResult::Invalid(faulty) => { - for participant in faulty { - // Remove from both maps for simplicity's sake - // There's no point in having one DKG complete yet not the other - assert!(substrate_participations.remove(&participant).is_some()); - assert!(network_participations.remove(&participant).is_some()); - res.push(ProcessorMessage::Blame { session, participant }); + .expect("prior read participation was invalid"), + ) + }) + .collect(); + + // Actually call verify on the DKG + match EvrfDkg::::verify( + &mut OsRng, + generators(), + context::( + session, + if true_if_substrate_false_if_network { + SUBSTRATE_KEY_CONTEXT + } else { + NETWORK_KEY_CONTEXT + }, + ), + threshold, + // Ignores the list of participants who were faulty, as they were prior blamed + &coerce_keys::(evrf_public_keys).0, + &participations, + ) + .unwrap() + { + // If the DKG was valid, return it + VerifyResult::Valid(dkg) => Ok(dkg), + // This DKG had faulty participants, so create blame messages for them + VerifyResult::Invalid(faulty) => { + let mut blames = vec![]; + for participant in faulty { + // Remove from both maps for simplicity's sake + // There's no point in having one DKG complete yet not the other + assert!(substrate_participations.remove(&participant).is_some()); + assert!(network_participations.remove(&participant).is_some()); + blames.push(ProcessorMessage::Blame { session, participant }); + } + // Since we removed `Participation`s, write the updated versions to the database + ParticipationDb::set( + txn, + &session, + &(substrate_participations.clone(), network_participations.clone()), + ); + Err(blames)? + } + VerifyResult::NotEnoughParticipants => { + // This is the first DKG, and we checked we were at the threshold OR + // This is the second DKG, as the first had no invalid participants, so we're still + // at the threshold + panic!("not enough participants despite checking we were at the threshold") } - ParticipationDb::set( - txn, - &session, - &(substrate_participations.clone(), network_participations.clone()), - ); - return res; } - VerifyResult::NotEnoughParticipants => { - panic!("not enough participants despite checking we were at the threshold") - } - }; - let network_dkg = match EvrfDkg::::verify( - &mut OsRng, - generators(), - context(session, NETWORK_KEY_CONTEXT), + } + + let substrate_dkg = match verify_dkg::( + txn, + session, + true, threshold, - // Ignores the list of participants who couldn't have their keys coerced due to prior - // handling those - &coerce_keys::(&network_evrf_public_keys).0, - &network_participations - .iter() - .map(|(key, participation)| { - ( - *key, - Participation::read(&mut participation.as_slice(), n) - .expect("prior read participation was invalid"), - ) - }) - .collect(), - ) - .unwrap() - { - VerifyResult::Valid(dkg) => dkg, - VerifyResult::Invalid(faulty) => { - for participant in faulty { - assert!(substrate_participations.remove(&participant).is_some()); - assert!(network_participations.remove(&participant).is_some()); - res.push(ProcessorMessage::Blame { session, participant }); - } - ParticipationDb::set( - txn, - &session, - &(substrate_participations.clone(), network_participations.clone()), - ); - return res; - } - VerifyResult::NotEnoughParticipants => { - // We may have lost the required amount of participants when doing the Substrate DKG - return res; - } + &substrate_evrf_public_keys, + &mut substrate_participations, + &mut network_participations, + ) { + Ok(dkg) => dkg, + // If we had any blames, immediately return them as necessary for the safety of + // `verify_dkg` (it assumes we don't call it again upon prior errors) + Err(blames) => return blames, }; - let substrate_keys = substrate_dkg.keys(&self.substrate_evrf_private_key); - let mut network_keys = network_dkg.keys(&self.network_evrf_private_key); + let network_dkg = match verify_dkg::( + txn, + session, + false, + threshold, + &network_evrf_public_keys, + &mut substrate_participations, + &mut network_participations, + ) { + Ok(dkg) => dkg, + Err(blames) => return blames, + }; + + // Get our keys from each DKG // TODO: Some of these keys may be decrypted by us, yet not actually meant for us, if // another validator set our eVRF public key as their eVRF public key. We either need to // ensure the coordinator tracks amount of shares we're supposed to have by the eVRF public // keys OR explicitly reduce to the keys we're supposed to have based on our `i` index. + let substrate_keys = substrate_dkg.keys(&self.substrate_evrf_private_key); + let mut network_keys = network_dkg.keys(&self.network_evrf_private_key); + // Tweak the keys for the network for network_keys in &mut network_keys { N::tweak_keys(network_keys); } GeneratedKeysDb::save_keys::(txn, &session, &substrate_keys, &network_keys); + // Since no one we verified was invalid, and we had the threshold, yield the new keys vec![ProcessorMessage::GeneratedKeyPair { session, substrate_key: substrate_keys[0].group_key().to_bytes(), diff --git a/processor/src/main.rs b/processor/src/main.rs index acfdfaf4..2d05ad4d 100644 --- a/processor/src/main.rs +++ b/processor/src/main.rs @@ -131,7 +131,7 @@ struct TributaryMutable { `Burn`s. Substrate also decides when to move to a new multisig, hence why this entire object is - Substate-mutable. + Substrate-mutable. Since MultisigManager should always be verifiable, and the Tributary is temporal, MultisigManager being entirely SubstrateMutable shows proper data pipe-lining.