From 7995ed10c5066d798a73a3bf5e28475255987b36 Mon Sep 17 00:00:00 2001 From: Hannah Davis Date: Tue, 27 Aug 2024 17:54:19 +0000 Subject: [PATCH] first pass at review comments, mainly eliminating round enums --- src/flp/szk.rs | 23 ++--- src/vdaf.rs | 1 - src/vdaf/mastic.rs | 215 +++++++++++++++++++++++++-------------------- 3 files changed, 127 insertions(+), 112 deletions(-) diff --git a/src/flp/szk.rs b/src/flp/szk.rs index 842e31026..9caabfacc 100644 --- a/src/flp/szk.rs +++ b/src/flp/szk.rs @@ -26,7 +26,6 @@ use subtle::{Choice, ConstantTimeEq}; // Domain separation tags const DST_PROVE_RANDOMNESS: u16 = 0; const DST_PROOF_SHARE: u16 = 1; -#[allow(dead_code)] const DST_QUERY_RANDOMNESS: u16 = 2; const DST_JOINT_RAND_SEED: u16 = 3; const DST_JOINT_RAND_PART: u16 = 4; @@ -211,7 +210,7 @@ impl ParameterizedDecode<(bool #[derive(Clone, Debug)] pub struct SzkQueryShare { joint_rand_part_opt: Option>, - flp_verifier: Vec, + pub(crate) flp_verifier: Vec, } impl Encode for SzkQueryShare { @@ -257,10 +256,6 @@ impl ParameterizedDecode<(bool } impl SzkQueryShare { - pub(crate) fn verifier_len(&self) -> usize { - self.flp_verifier.len() - } - pub(crate) fn merge_verifiers( mut leader_share: SzkQueryShare, helper_share: SzkQueryShare, @@ -280,7 +275,9 @@ impl SzkQueryShare { } } -/// The state that needs to be stored by an Szk verifier between query() and decide() +/// Szk query state. +/// +/// The state that needs to be stored by an Szk verifier between query() and decide(). pub type SzkQueryState = Option>; /// Verifier type for the SZK proof. @@ -378,10 +375,6 @@ where } } - pub(crate) fn typ(&self) -> &T { - &self.typ - } - fn domain_separation_tag(&self, usage: u16) -> [u8; 8] { let mut dst = [0u8; 8]; dst[0] = MASTIC_VERSION; @@ -475,7 +468,7 @@ where .into_field_vec(self.typ.query_rand_len()) } - pub(crate) fn has_joint_rand(&self) -> bool { + pub(crate) fn requires_joint_rand(&self) -> bool { self.typ.joint_rand_len() > 0 } @@ -563,7 +556,7 @@ where } => Cow::Owned(self.derive_helper_proof_share(proof_share_seed_and_blind)), }; - let (joint_rand, joint_rand_seed, joint_rand_part) = if self.has_joint_rand() { + let (joint_rand, joint_rand_seed, joint_rand_part) = if self.requires_joint_rand() { let ((joint_rand_seed, joint_rand), host_joint_rand_part) = match proof_share { SzkProofShare::Leader { uncompressed_proof_share: _, @@ -718,7 +711,7 @@ mod tests { thread_rng().fill(&mut nonce[..]); let prove_rand_seed = Seed::<16>::generate().unwrap(); let helper_seed = Seed::<16>::generate().unwrap(); - let leader_seed_opt = if szk_typ.has_joint_rand() { + let leader_seed_opt = if szk_typ.requires_joint_rand() { Some(Seed::<16>::generate().unwrap()) } else { None @@ -770,7 +763,7 @@ mod tests { }; //test mutated jr seed - if szk_typ.has_joint_rand() { + if szk_typ.requires_joint_rand() { let joint_rand_seed_opt = Some(Seed::<16>::generate().unwrap()); if let Ok(leader_decision) = szk_typ.decide(verifier, joint_rand_seed_opt.clone()) { assert!(!leader_decision, "Leader accepted wrong jr seed"); diff --git a/src/vdaf.rs b/src/vdaf.rs index b87363c51..8dc8b2fe1 100644 --- a/src/vdaf.rs +++ b/src/vdaf.rs @@ -12,7 +12,6 @@ use crate::flp::szk::SzkError; #[cfg(all(feature = "crypto-dependencies", feature = "experimental"))] use crate::idpf::IdpfError; #[cfg(all(feature = "crypto-dependencies", feature = "experimental"))] -#[cfg(all(feature = "crypto-dependencies", feature = "experimental"))] use crate::vidpf::VidpfError; use crate::{ codec::{CodecError, Decode, Encode, ParameterizedDecode}, diff --git a/src/vdaf/mastic.rs b/src/vdaf/mastic.rs index 39f0c8c85..089372eb2 100644 --- a/src/vdaf/mastic.rs +++ b/src/vdaf/mastic.rs @@ -29,6 +29,7 @@ use subtle::{Choice, ConstantTimeEq}; const DST_PATH_CHECK_BATCH: u16 = 6; const NONCE_SIZE: usize = 16; + /// The main struct implementing the Mastic VDAF. /// Composed of a shared zero knowledge proof system and a verifiable incremental /// distributed point function. @@ -364,15 +365,12 @@ where VidpfKey::gen(VidpfServerId::S0)?, VidpfKey::gen(VidpfServerId::S1)?, ]; - let joint_random_opt = if self.szk.has_joint_rand() { + let joint_random_opt = if self.szk.requires_joint_rand() { Some(Seed::::generate()?) } else { None }; - let szk_random = [ - Seed::::generate()?, - Seed::::generate()?, - ]; + let szk_random = [Seed::generate()?, Seed::generate()?]; let encoded_measurement = self.encode_measurement(weight)?; if encoded_measurement.as_ref().len() != self.vidpf.weight_parameter { @@ -391,49 +389,50 @@ where } } -/// Mastic prepare state +/// Mastic prepare state. /// -/// State held by an aggregator between rounds of Mastic. Includes intermediate +/// State held by an aggregator between rounds of Mastic preparation. Includes intermediate /// state for Szk verification, the output shares currently being validated, and /// parameters of Mastic used for encoding. #[derive(Clone, Debug, Eq, PartialEq)] -pub enum MasticPrepareState { - /// Includes state for performing Szk verification at the root. - FirstRound(SzkQueryState, MasticOutputShare, usize, bool), - /// In all rounds but the first, SZK is not run, so only the output shares and encoding information are stored. - LaterRound(MasticOutputShare, bool), +pub struct MasticPrepareState { + /// Includes output shares for eventual aggregation. + output_shares: MasticOutputShare, + /// if SZK verification is being performed, we also store the relevant state for that operation. + szk_query_state: SzkQueryState, + verifier_len: Option, } -/// Mastic prepare share +/// Mastic prepare share. /// /// Broadcast message from an aggregator between rounds of Mastic. Includes the /// hashed VIDPF proofs for every prefix in the aggregation parameter, and optionally /// the verification message for Szk. #[derive(Clone, Debug)] -pub enum MasticPrepareShare { - /// Includes a batched VIDPF proof and an SZK verification message for the root weight. - FirstRound(Seed, SzkQueryShare), - /// Includes only a batched VIDPF proof as SZK will not be run. - LaterRound(Seed), +pub struct MasticPrepareShare { + /// Batched VIDPF proofs + vidpf_proofs: Seed, + /// If Szk verification of the root weight is needed, an SZK verification message. + szk_query_share_opt: Option>, } impl Encode for MasticPrepareShare { fn encode(&self, bytes: &mut Vec) -> Result<(), CodecError> { - match self { - MasticPrepareShare::FirstRound(seed, query_share) => { - seed.encode(bytes).and_then(|_| query_share.encode(bytes)) - } - MasticPrepareShare::LaterRound(seed) => seed.encode(bytes), + self.vidpf_proofs.encode(bytes)?; + match &self.szk_query_share_opt { + Some(query_share) => query_share.encode(bytes), + None => Ok(()), } } fn encoded_len(&self) -> Option { - match self { - MasticPrepareShare::FirstRound(seed, query_share) => { - Some(seed.encoded_len()? + query_share.encoded_len()?) - } - MasticPrepareShare::LaterRound(seed) => seed.encoded_len(), - } + Some( + self.vidpf_proofs.encoded_len()? + + match &self.szk_query_share_opt { + Some(query_share) => query_share.encoded_len()?, + None => 0, + }, + ) } } @@ -444,21 +443,26 @@ impl ParameterizedDecode, bytes: &mut Cursor<&[u8]>, ) -> Result { - match prep_state { - MasticPrepareState::FirstRound(_, _, verifier_len, requires_joint_rand) => { - Ok(MasticPrepareShare::FirstRound( - Seed::::decode(bytes)?, - SzkQueryShare::::decode_with_param( - &(*requires_joint_rand, *verifier_len), - bytes, - )?, - )) - } - MasticPrepareState::LaterRound(_, _) => { - Ok(MasticPrepareShare::LaterRound(Seed::::decode( + match (&prep_state.szk_query_state, prep_state.verifier_len) { + (Some(_), Some(verifier_len)) => Ok(MasticPrepareShare { + vidpf_proofs: Seed::decode(bytes)?, + szk_query_share_opt: Some(SzkQueryShare::::decode_with_param( + &(true, verifier_len), bytes, - )?)) - } + )?), + }), + (None, Some(verifier_len)) => Ok(MasticPrepareShare { + vidpf_proofs: Seed::decode(bytes)?, + szk_query_share_opt: Some(SzkQueryShare::::decode_with_param( + &(false, verifier_len), + bytes, + )?), + }), + (None, None) => Ok(MasticPrepareShare { + vidpf_proofs: Seed::::decode(bytes)?, + szk_query_share_opt: None, + }), + (Some(_), None) => Err(CodecError::UnexpectedValue), } } } @@ -467,28 +471,20 @@ impl ParameterizedDecode { - ///If Szk is being run, all SzkQueryShares have been summed - /// to produce a verifier to be input to decide() - FirstRound(SzkVerifier), - /// If Szk is not being run, no further computation is necessary as the VIDPF proofs have already - /// been checked. - LaterRound, -} +pub type MasticPrepareMessage = Option>; impl Encode for MasticPrepareMessage { fn encode(&self, bytes: &mut Vec) -> Result<(), CodecError> { match self { - MasticPrepareMessage::FirstRound(verifier) => verifier.encode(bytes), - MasticPrepareMessage::LaterRound => Ok(()), + Some(verifier) => verifier.encode(bytes), + None => Ok(()), } } fn encoded_len(&self) -> Option { match self { - MasticPrepareMessage::FirstRound(verifier) => verifier.encoded_len(), - MasticPrepareMessage::LaterRound => Some(0), + Some(verifier) => verifier.encoded_len(), + None => Some(0), } } } @@ -500,13 +496,16 @@ impl ParameterizedDecode, bytes: &mut Cursor<&[u8]>, ) -> Result { - match prep_state { - MasticPrepareState::FirstRound(_, _, verifier_len, requires_joint_rand) => { - Ok(MasticPrepareMessage::FirstRound( - SzkVerifier::decode_with_param(&(*requires_joint_rand, *verifier_len), bytes)?, - )) - } - MasticPrepareState::LaterRound(_, _) => Ok(MasticPrepareMessage::LaterRound), + match (&prep_state.szk_query_state, prep_state.verifier_len) { + (Some(_), Some(verifier_len)) => Ok(Some( + SzkVerifier::::decode_with_param(&(true, verifier_len), bytes)?, + )), + (None, Some(verifier_len)) => Ok(Some(SzkVerifier::::decode_with_param( + &(false, verifier_len), + bytes, + )?)), + (None, None) => Ok(None), + (Some(_), None) => Err(CodecError::UnexpectedValue), } } } @@ -602,23 +601,29 @@ where }; let (prep_share, prep_state) = if let Some((szk_query_share, szk_query_state)) = szk_verify_opt { - let verifier_len = szk_query_share.verifier_len(); + let verifier_len = szk_query_share.flp_verifier.len(); ( - MasticPrepareShare::FirstRound(xof.into_seed(), szk_query_share), - MasticPrepareState::FirstRound( + MasticPrepareShare { + vidpf_proofs: xof.into_seed(), + szk_query_share_opt: Some(szk_query_share), + }, + MasticPrepareState { + output_shares: MasticOutputShare::::from(output_shares), szk_query_state, - MasticOutputShare::::from(output_shares), - verifier_len, - self.szk.has_joint_rand(), - ), + verifier_len: Some(verifier_len), + }, ) } else { ( - MasticPrepareShare::LaterRound(xof.into_seed()), - MasticPrepareState::LaterRound( - MasticOutputShare::::from(output_shares), - self.szk.has_joint_rand(), - ), + MasticPrepareShare { + vidpf_proofs: xof.into_seed(), + szk_query_share_opt: None, + }, + MasticPrepareState { + output_shares: MasticOutputShare::::from(output_shares), + szk_query_state: None, + verifier_len: None, + }, ) }; Ok((prep_state, prep_share)) @@ -651,13 +656,20 @@ where match (leader_share, helper_share) { ( - MasticPrepareShare::FirstRound(leader_vidpf_proof, leader_query_share), - MasticPrepareShare::FirstRound(helper_vidpf_proof, helper_query_share), + MasticPrepareShare { + vidpf_proofs: leader_vidpf_proof, + szk_query_share_opt: Some(leader_query_share), + }, + MasticPrepareShare { + vidpf_proofs: helper_vidpf_proof, + szk_query_share_opt: Some(helper_query_share), + }, ) => { if leader_vidpf_proof == helper_vidpf_proof { - Ok(MasticPrepareMessage::FirstRound( - SzkQueryShare::merge_verifiers(leader_query_share, helper_query_share), - )) + Ok(Some(SzkQueryShare::merge_verifiers( + leader_query_share, + helper_query_share, + ))) } else { Err(VdafError::Uncategorized( "Vidpf proof verification failed".to_string(), @@ -665,11 +677,17 @@ where } } ( - MasticPrepareShare::LaterRound(leader_vidpf_proof), - MasticPrepareShare::LaterRound(helper_vidpf_proof), + MasticPrepareShare { + vidpf_proofs: leader_vidpf_proof, + szk_query_share_opt: None, + }, + MasticPrepareShare { + vidpf_proofs: helper_vidpf_proof, + szk_query_share_opt: None, + }, ) => { if leader_vidpf_proof == helper_vidpf_proof { - Ok(MasticPrepareMessage::LaterRound) + Ok(None) } else { Err(VdafError::Uncategorized( "Vidpf proof verification failed".to_string(), @@ -700,25 +718,30 @@ where input: MasticPrepareMessage, ) -> Result, VdafError> { match (state, input) { - (MasticPrepareState::LaterRound(output_share, _), MasticPrepareMessage::LaterRound) => { - Ok(PrepareTransition::Finish(output_share)) - } ( - MasticPrepareState::FirstRound(query_state, output_share, _, _), - MasticPrepareMessage::FirstRound(verifier), + MasticPrepareState { + output_shares, + szk_query_state: _, + verifier_len: _, + }, + None, + ) => Ok(PrepareTransition::Finish(output_shares)), + ( + MasticPrepareState { + output_shares, + szk_query_state, + verifier_len: _, + }, + Some(verifier), ) => { - if self.szk.decide(verifier, query_state)? { - Ok(PrepareTransition::Finish(output_share)) + if self.szk.decide(verifier, szk_query_state)? { + Ok(PrepareTransition::Finish(output_shares)) } else { Err(VdafError::Uncategorized( "Szk proof failed verification".to_string(), )) } } - _ => Err(VdafError::Uncategorized( - "Prepare state and message disagree on whether Szk verification should occur" - .to_string(), - )), } } @@ -771,8 +794,8 @@ where [i * self.vidpf.weight_parameter..(i + 1) * self.vidpf.weight_parameter]; result.push( self.szk - .typ() - .decode_result(&self.szk.typ().truncate(encoded_result.to_vec())?[..], 1)?, + .typ + .decode_result(&self.szk.typ.truncate(encoded_result.to_vec())?[..], 1)?, ); } Ok(result)