Skip to content

Commit

Permalink
feat(ecdsa): CON-1193 Make key_unmasked_ref in `PreSignatureQuadrup…
Browse files Browse the repository at this point in the history
…leRef` required
  • Loading branch information
eichhorl committed Feb 20, 2024
1 parent 554a778 commit b467393
Show file tree
Hide file tree
Showing 7 changed files with 24 additions and 83 deletions.
13 changes: 0 additions & 13 deletions rs/consensus/src/consensus/metrics.rs
Expand Up @@ -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 {
Expand Down Expand Up @@ -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(),
}
}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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);
}
}
}
Expand Down
35 changes: 5 additions & 30 deletions rs/consensus/src/ecdsa/payload_builder/quadruples.rs
Expand Up @@ -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(),
),
);
}
Expand Down Expand Up @@ -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
});
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion rs/consensus/src/ecdsa/payload_verifier.rs
Expand Up @@ -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,
},
);
}
Expand Down
4 changes: 2 additions & 2 deletions rs/consensus/src/ecdsa/test_utils.rs
Expand Up @@ -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(),
};
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 3 additions & 7 deletions rs/consensus/src/ecdsa/utils.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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());
Expand Down
6 changes: 4 additions & 2 deletions rs/consensus/utils/src/lib.rs
Expand Up @@ -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(),
}
}

Expand Down Expand Up @@ -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() {
Expand Down
37 changes: 9 additions & 28 deletions rs/types/types/src/consensus/ecdsa_refs.rs
Expand Up @@ -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<UnmaskedTranscript>,
pub key_unmasked_ref: UnmaskedTranscript,
}

#[derive(Clone, Debug)]
Expand All @@ -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<UnmaskedTranscript>,
key_unmasked_ref: UnmaskedTranscript,
) -> Self {
Self {
kappa_unmasked_ref,
Expand Down Expand Up @@ -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<H: Hasher>(&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);
}
}

Expand All @@ -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()),
}
}
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit b467393

Please sign in to comment.