diff --git a/rs/consensus/src/consensus/metrics.rs b/rs/consensus/src/consensus/metrics.rs index 05380a9aa6b..b13c60d4ae7 100644 --- a/rs/consensus/src/consensus/metrics.rs +++ b/rs/consensus/src/consensus/metrics.rs @@ -189,7 +189,6 @@ pub struct EcdsaStats { pub quadruples_in_creation: CounterPerEcdsaKeyId, pub ongoing_xnet_reshares: CounterPerEcdsaKeyId, pub xnet_reshare_agreements: CounterPerEcdsaKeyId, - pub available_quadruples_with_key_transcript: usize, } impl From<&EcdsaPayload> for EcdsaStats { @@ -238,11 +237,6 @@ impl From<&EcdsaPayload> for EcdsaStats { .filter(|(_, status)| matches!(status, CompletedReshareRequest::Unreported(_))), &keys, ), - available_quadruples_with_key_transcript: payload - .available_quadruples - .values() - .filter(|quadruple| quadruple.key_unmasked_ref.is_some()) - .count(), } } } @@ -283,7 +277,6 @@ pub struct FinalizerMetrics { pub ecdsa_quadruples_in_creation: IntGaugeVec, pub ecdsa_ongoing_xnet_reshares: IntGaugeVec, pub ecdsa_xnet_reshare_agreements: IntCounterVec, - pub ecdsa_available_quadruples_with_key_transcript: IntGauge, // canister http payload metrics pub canister_http_success_delivered: IntCounter, pub canister_http_timeouts_delivered: IntCounter, @@ -346,10 +339,6 @@ impl FinalizerMetrics { "The number of available ECDSA quadruples", &[ECDSA_KEY_ID_LABEL], ), - ecdsa_available_quadruples_with_key_transcript: metrics_registry.int_gauge( - "consensus_ecdsa_available_quadruples_with_key_transcript", - "The number of available ECDSA quadruples with key transcript", - ), ecdsa_quadruples_in_creation: metrics_registry.int_gauge_vec( "consensus_ecdsa_quadruples_in_creation", "The number of ECDSA quadruples in creation", @@ -440,8 +429,6 @@ impl FinalizerMetrics { &self.ecdsa_xnet_reshare_agreements, &ecdsa.xnet_reshare_agreements, ); - self.ecdsa_available_quadruples_with_key_transcript - .set(ecdsa.available_quadruples_with_key_transcript as i64); } } } diff --git a/rs/consensus/src/ecdsa/payload_builder/quadruples.rs b/rs/consensus/src/ecdsa/payload_builder/quadruples.rs index 0e580a66928..32f36532689 100644 --- a/rs/consensus/src/ecdsa/payload_builder/quadruples.rs +++ b/rs/consensus/src/ecdsa/payload_builder/quadruples.rs @@ -226,7 +226,7 @@ pub(super) fn update_quadruples_in_creation( lambda_masked, kappa_times_lambda, key_times_lambda, - Some(key_transcript.unmasked_transcript()), + key_transcript.unmasked_transcript(), ), ); } @@ -254,10 +254,7 @@ pub(super) fn purge_old_key_quadruples( ecdsa_payload.available_quadruples.retain(|id, quadruple| { matched_quadruples.contains(id) - || match quadruple.key_unmasked_ref.as_ref() { - Some(transcript) => transcript.as_ref().transcript_id == current_key_transcript_id, - None => true, - } + || quadruple.key_unmasked_ref.as_ref().transcript_id == current_key_transcript_id }); } @@ -425,7 +422,9 @@ pub(super) mod test_utils { let sig_inputs = create_sig_inputs(caller); let quadruple_id = ecdsa_payload.uid_generator.next_quadruple_id(key_id); let mut quadruple_ref = sig_inputs.sig_inputs_ref.presig_quadruple_ref.clone(); - quadruple_ref.key_unmasked_ref = key_transcript; + if let Some(transcript) = key_transcript { + quadruple_ref.key_unmasked_ref = transcript; + } ecdsa_payload .available_quadruples .insert(quadruple_id.clone(), quadruple_ref); @@ -1035,30 +1034,6 @@ pub(super) mod tests { assert_eq!(payload.available_quadruples.len(), 3); } - #[test] - fn test_unmatched_quadruples_without_key_are_not_purged() { - let mut rng = reproducible_rng(); - let (mut payload, _, _) = set_up(&mut rng, subnet_test_id(1), Height::from(100)); - let key_id = payload.key_transcript.key_id.clone(); - - // Create three quadruples without key transcript - for i in 0..3 { - create_available_quadruple_with_key_transcript(&mut payload, i, key_id.clone(), None); - } - - // None of them are matched to a context - let contexts = BTreeMap::from_iter([fake_sign_with_ecdsa_context_with_quadruple( - 1, - key_id.clone(), - None, - )]); - - // None of them should be purged - assert_eq!(payload.available_quadruples.len(), 3); - purge_old_key_quadruples(&mut payload, &contexts); - assert_eq!(payload.available_quadruples.len(), 3); - } - #[test] fn test_unmatched_quadruples_of_different_key_are_purged() { let mut rng = reproducible_rng(); diff --git a/rs/consensus/src/ecdsa/payload_verifier.rs b/rs/consensus/src/ecdsa/payload_verifier.rs index 339fbe50830..f86590bf0c3 100644 --- a/rs/consensus/src/ecdsa/payload_verifier.rs +++ b/rs/consensus/src/ecdsa/payload_verifier.rs @@ -1349,7 +1349,7 @@ mod test { lambda_masked_ref: masked_transcript_1, kappa_times_lambda_ref: masked_transcript_1, key_times_lambda_ref: masked_transcript_1, - key_unmasked_ref: Some(transcript_ref_0), + key_unmasked_ref: transcript_ref_0, }, ); } diff --git a/rs/consensus/src/ecdsa/test_utils.rs b/rs/consensus/src/ecdsa/test_utils.rs index ece50058349..f75c3164afb 100644 --- a/rs/consensus/src/ecdsa/test_utils.rs +++ b/rs/consensus/src/ecdsa/test_utils.rs @@ -265,7 +265,7 @@ impl From<&ThresholdEcdsaSigInputs> for TestSigInputs { .unwrap(), key_times_lambda_ref: MaskedTranscript::try_from((height, quad.key_times_lambda())) .unwrap(), - key_unmasked_ref: Some(UnmaskedTranscript::try_from((height, key)).unwrap()), + key_unmasked_ref: UnmaskedTranscript::try_from((height, key)).unwrap(), }, key_transcript_ref: UnmaskedTranscript::try_from((height, key)).unwrap(), }; @@ -1144,7 +1144,7 @@ pub(crate) fn create_sig_inputs_with_args( lambda_masked_ref, kappa_unmasked_times_lambda_masked_ref, key_unmasked_times_lambda_masked_ref, - Some(key_unmasked_ref), + key_unmasked_ref, ); let sig_inputs_ref = ThresholdEcdsaSigInputsRef::new( ExtendedDerivationPath { diff --git a/rs/consensus/src/ecdsa/utils.rs b/rs/consensus/src/ecdsa/utils.rs index 6dd6f62414b..2ee1a851714 100644 --- a/rs/consensus/src/ecdsa/utils.rs +++ b/rs/consensus/src/ecdsa/utils.rs @@ -245,7 +245,7 @@ pub(super) fn build_signature_inputs( let quadruple = block_reader .available_quadruple(&request_id.quadruple_id)? .clone(); - let key_transcript_ref = quadruple.key_unmasked_ref?; + let key_transcript_ref = quadruple.key_unmasked_ref; let inputs = ThresholdEcdsaSigInputsRef::new( extended_derivation_path, context.message_hash, @@ -419,11 +419,7 @@ pub(crate) fn get_quadruple_ids_to_deliver( quadruple_ids.insert(ecdsa.key_transcript.key_id.clone(), BTreeSet::new()); for (quadruple_id, quadruple) in &ecdsa.available_quadruples { - let Some(quadruple_key_transcript) = quadruple.key_unmasked_ref.as_ref() else { - continue; - }; - - if current_key_transcript_id != quadruple_key_transcript.as_ref().transcript_id { + if current_key_transcript_id != quadruple.key_unmasked_ref.as_ref().transcript_id { continue; } @@ -770,7 +766,7 @@ mod tests { quadruple_id.1 = Some(key_id.clone()); quadruple_ids.push(quadruple_id.clone()); let mut quadruple_ref = sig_inputs.sig_inputs_ref.presig_quadruple_ref.clone(); - quadruple_ref.key_unmasked_ref = Some(key_transcript); + quadruple_ref.key_unmasked_ref = key_transcript; ecdsa_payload .available_quadruples .insert(quadruple_id, quadruple_ref.clone()); diff --git a/rs/consensus/utils/src/lib.rs b/rs/consensus/utils/src/lib.rs index b7c76ba6ac9..725179b8e8b 100644 --- a/rs/consensus/utils/src/lib.rs +++ b/rs/consensus/utils/src/lib.rs @@ -817,13 +817,15 @@ mod tests { kappa_times_lambda.transcript_id = fake_transcript_id(id + 2); let mut key_times_lambda = lambda_masked.clone(); key_times_lambda.transcript_id = fake_transcript_id(id + 3); + let mut key_unmasked = kappa_unmasked.clone(); + key_unmasked.transcript_id = fake_transcript_id(id + 4); let h = Height::from(0); PreSignatureQuadrupleRef { kappa_unmasked_ref: UnmaskedTranscript::try_from((h, &kappa_unmasked)).unwrap(), lambda_masked_ref: MaskedTranscript::try_from((h, &lambda_masked)).unwrap(), kappa_times_lambda_ref: MaskedTranscript::try_from((h, &kappa_times_lambda)).unwrap(), key_times_lambda_ref: MaskedTranscript::try_from((h, &key_times_lambda)).unwrap(), - key_unmasked_ref: None, + key_unmasked_ref: UnmaskedTranscript::try_from((h, &key_unmasked)).unwrap(), } } @@ -864,7 +866,7 @@ mod tests { ] .into_iter() .cycle(); - for i in (0..40).step_by(4) { + for i in (0..50).step_by(5) { let quadruple = fake_quadruple(i as u64); let rv = rvs.next().unwrap(); for r in quadruple.get_refs() { diff --git a/rs/types/types/src/consensus/ecdsa_refs.rs b/rs/types/types/src/consensus/ecdsa_refs.rs index 157eeb1a79f..6794f32685b 100644 --- a/rs/types/types/src/consensus/ecdsa_refs.rs +++ b/rs/types/types/src/consensus/ecdsa_refs.rs @@ -1311,15 +1311,14 @@ impl IDkgTranscriptParamsRef { /// Counterpart of PreSignatureQuadruple that holds transcript references, /// instead of the transcripts. -#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Clone, Debug, Hash, PartialEq, Eq, Serialize, Deserialize)] #[cfg_attr(test, derive(ExhaustiveSet))] pub struct PreSignatureQuadrupleRef { pub kappa_unmasked_ref: UnmaskedTranscript, pub lambda_masked_ref: MaskedTranscript, pub kappa_times_lambda_ref: MaskedTranscript, pub key_times_lambda_ref: MaskedTranscript, - // TODO(CON-1193): remove `Option` - pub key_unmasked_ref: Option, + pub key_unmasked_ref: UnmaskedTranscript, } #[derive(Clone, Debug)] @@ -1337,8 +1336,7 @@ impl PreSignatureQuadrupleRef { lambda_masked_ref: MaskedTranscript, kappa_times_lambda_ref: MaskedTranscript, key_times_lambda_ref: MaskedTranscript, - // TODO(CON-1193): remove `Option` - key_unmasked_ref: Option, + key_unmasked_ref: UnmaskedTranscript, ) -> Self { Self { kappa_unmasked_ref, @@ -1392,21 +1390,7 @@ impl PreSignatureQuadrupleRef { self.lambda_masked_ref.as_mut().update(height); self.kappa_times_lambda_ref.as_mut().update(height); self.key_times_lambda_ref.as_mut().update(height); - // TODO(CON-1193): update key_unmasked_ref - } -} - -// TODO(CON-1193): remove once `key_unmasked_ref` is no longer optional -impl Hash for PreSignatureQuadrupleRef { - fn hash(&self, state: &mut H) { - self.kappa_unmasked_ref.hash(state); - self.lambda_masked_ref.hash(state); - self.kappa_times_lambda_ref.hash(state); - self.key_times_lambda_ref.hash(state); - - if let Some(key_ref) = &self.key_unmasked_ref { - key_ref.hash(state); - } + self.key_unmasked_ref.as_mut().update(height); } } @@ -1417,9 +1401,7 @@ impl From<&PreSignatureQuadrupleRef> for pb::PreSignatureQuadrupleRef { lambda_masked_ref: Some((&quadruple.lambda_masked_ref).into()), kappa_times_lambda_ref: Some((&quadruple.kappa_times_lambda_ref).into()), key_times_lambda_ref: Some((&quadruple.key_times_lambda_ref).into()), - key_unmasked_ref: quadruple - .key_unmasked_ref - .map(|transcript| (&transcript).into()), + key_unmasked_ref: Some((&quadruple.key_unmasked_ref).into()), } } } @@ -1447,11 +1429,10 @@ impl TryFrom<&pb::PreSignatureQuadrupleRef> for PreSignatureQuadrupleRef { "PreSignatureQuadrupleRef::quadruple::key_times_lamdba_ref", )?; - let key_unmasked_ref = quadruple - .key_unmasked_ref - .as_ref() - .map(UnmaskedTranscript::try_from) - .transpose()?; + let key_unmasked_ref: UnmaskedTranscript = try_from_option_field( + quadruple.key_unmasked_ref.as_ref(), + "PreSignatureQuadrupleRef::quadruple::key_unmasked_ref", + )?; Ok(Self::new( kappa_unmasked_ref,