feat(ecdsa): CON-1196 Purge unmatched quadruples referencing old key …

…transcripts once certified height reaches the latest summary height

rs/consensus/src/ecdsa/payload_builder.rs

@@ -493,6 +493,11 @@ pub(crate) fn create_data_payload(
     Ok(new_payload)
 }
 
+pub(crate) enum CertifiedHeight {
+    ReachedSummaryHeight,
+    BelowSummaryHeight,
+}
+
 pub(crate) fn create_data_payload_helper(
     subnet_id: SubnetId,
     context: &ValidationContext,
@@ -551,13 +556,20 @@ pub(crate) fn create_data_payload_helper(
         .subnet_call_context_manager
         .ecdsa_dealings_contexts;
 
+    let certified_height = if context.certified_height >= summary_block.height() {
+        CertifiedHeight::ReachedSummaryHeight
+    } else {
+        CertifiedHeight::BelowSummaryHeight
+    };
+
     create_data_payload_helper_2(
         &mut ecdsa_payload,
         height,
         context.time,
         &ecdsa_config,
         &enabled_signing_keys,
         next_interval_registry_version,
+        certified_height,
         &receivers,
         all_signing_requests,
         ecdsa_dealings_contexts,
@@ -577,6 +589,7 @@ pub(crate) fn create_data_payload_helper_2(
     ecdsa_config: &EcdsaConfig,
     enabled_signing_keys: &BTreeSet<EcdsaKeyId>,
     next_interval_registry_version: RegistryVersion,
+    certified_height: CertifiedHeight,
     receivers: &[NodeId],
     all_signing_requests: &BTreeMap<CallbackId, SignWithEcdsaContext>,
     ecdsa_dealings_contexts: &BTreeMap<CallbackId, EcdsaDealingsContext>,
@@ -614,6 +627,11 @@ pub(crate) fn create_data_payload_helper_2(
         ecdsa_payload,
         log,
     )?;
+
+    if matches!(certified_height, CertifiedHeight::ReachedSummaryHeight) {
+        quadruples::purge_old_key_quadruples(ecdsa_payload, all_signing_requests);
+    }
+
     quadruples::make_new_quadruples_if_needed(ecdsa_config, ecdsa_payload);
 
     let mut new_transcripts =
@@ -1848,6 +1866,7 @@ mod tests {
             &EcdsaConfig::default(),
             &valid_keys,
             RegistryVersion::from(9),
+            CertifiedHeight::ReachedSummaryHeight,
             &[node_test_id(0)],
             &sign_with_ecdsa_contexts,
             &BTreeMap::default(),
@@ -1871,6 +1890,7 @@ mod tests {
             &EcdsaConfig::default(),
             &valid_keys,
             RegistryVersion::from(9),
+            CertifiedHeight::ReachedSummaryHeight,
             &[node_test_id(0)],
             &sign_with_ecdsa_contexts,
             &BTreeMap::default(),
@@ -2876,6 +2896,8 @@ mod tests {
             assert_eq!(metrics.critical_error_ecdsa_key_transcript_missing.get(), 1);
 
             // Now, quadruples and xnet reshares should be purged
+            // TODO(CON-1149): Extend once available quadruples are no longer immediately purged
+            // on key reshares.
             assert!(payload_4.available_quadruples.is_empty());
             assert!(payload_4.quadruples_in_creation.is_empty());
             assert!(payload_4.ongoing_xnet_reshares.is_empty());
@@ -2944,6 +2966,7 @@ mod tests {
                 &ecdsa_config,
                 &valid_keys,
                 registry_version,
+                CertifiedHeight::ReachedSummaryHeight,
                 &node_ids,
                 &BTreeMap::default(),
                 &BTreeMap::default(),
@@ -3109,6 +3132,7 @@ mod tests {
                 &ecdsa_config,
                 &valid_keys,
                 registry_version,
+                CertifiedHeight::ReachedSummaryHeight,
                 &node_ids,
                 &BTreeMap::default(),
                 &BTreeMap::default(),
@@ -3137,6 +3161,7 @@ mod tests {
                 &ecdsa_config,
                 &valid_keys,
                 registry_version,
+                CertifiedHeight::ReachedSummaryHeight,
                 &node_ids,
                 &BTreeMap::default(),
                 &BTreeMap::default(),
@@ -3163,6 +3188,7 @@ mod tests {
                 &ecdsa_config,
                 &valid_keys,
                 registry_version,
+                CertifiedHeight::ReachedSummaryHeight,
                 &node_ids,
                 &BTreeMap::default(),
                 &BTreeMap::default(),

rs/consensus/src/ecdsa/payload_builder/quadruples.rs

@@ -3,13 +3,15 @@ use super::EcdsaPayloadError;
 use crate::ecdsa::pre_signer::EcdsaTranscriptBuilder;
 use ic_logger::{debug, error, ReplicaLogger};
 use ic_registry_subnet_features::EcdsaConfig;
+use ic_replicated_state::metadata_state::subnet_call_context_manager::SignWithEcdsaContext;
 use ic_types::{
     consensus::ecdsa::{self, TranscriptAttributes},
     crypto::{canister_threshold_sig::idkg::IDkgTranscript, AlgorithmId},
+    messages::CallbackId,
     Height, NodeId, RegistryVersion,
 };
 
-use std::collections::BTreeSet;
+use std::collections::{BTreeMap, BTreeSet};
 
 /// Update the quadruples in the payload by:
 /// - making new configs when pre-conditions are met;
@@ -206,6 +208,32 @@ pub(super) fn update_quadruples_in_creation(
     Ok(new_transcripts)
 }
 
+/// Purge all available but unmatched quadruples that are referencing a different key transcript
+/// than the one currently used.
+pub(super) fn purge_old_key_quadruples(
+    ecdsa_payload: &mut ecdsa::EcdsaPayload,
+    all_signing_requests: &BTreeMap<CallbackId, SignWithEcdsaContext>,
+) {
+    let Some(unmasked_transcript) = ecdsa_payload.key_transcript.current.as_ref() else {
+        return;
+    };
+    let current_key_transcript_id = unmasked_transcript.transcript_id();
+
+    let matched_quadruples = all_signing_requests
+        .values()
+        .flat_map(|context| context.matched_quadruple.clone())
+        .map(|(quadruple_id, _)| quadruple_id)
+        .collect::<BTreeSet<_>>();
+
+    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,
+            }
+    });
+}
+
 /// Creating new quadruples if necessary by updating quadruples_in_creation,
 /// considering currently available quadruples, quadruples in creation, and
 /// ecdsa configs.
@@ -275,7 +303,7 @@ pub(super) mod test_utils {
     use std::collections::BTreeMap;
 
     use ic_types::{
-        consensus::ecdsa::{self, EcdsaPayload, QuadrupleId},
+        consensus::ecdsa::{self, EcdsaPayload, QuadrupleId, UnmaskedTranscript},
         NodeId, RegistryVersion,
     };
 
@@ -295,12 +323,27 @@ pub(super) mod test_utils {
     }
 
     pub fn create_available_quadruple(ecdsa_payload: &mut EcdsaPayload, caller: u8) -> QuadrupleId {
+        create_available_quadruple_with_key_transcript(ecdsa_payload, caller, None)
+    }
+
+    pub fn create_available_quadruple_with_key_transcript(
+        ecdsa_payload: &mut EcdsaPayload,
+        caller: u8,
+        key_transcript: Option<UnmaskedTranscript>,
+    ) -> QuadrupleId {
         let sig_inputs = create_sig_inputs(caller);
         let quadruple_id = ecdsa_payload.uid_generator.next_quadruple_id();
-        let quadruple_ref = &sig_inputs.sig_inputs_ref.presig_quadruple_ref;
+        let mut quadruple_ref = sig_inputs.sig_inputs_ref.presig_quadruple_ref.clone();
+        quadruple_ref.key_unmasked_ref = key_transcript;
         ecdsa_payload
             .available_quadruples
-            .insert(quadruple_id.clone(), quadruple_ref.clone());
+            .insert(quadruple_id.clone(), quadruple_ref);
+
+        for (t_ref, transcript) in sig_inputs.idkg_transcripts {
+            ecdsa_payload
+                .idkg_transcripts
+                .insert(t_ref.transcript_id, transcript);
+        }
 
         quadruple_id
     }
@@ -312,15 +355,18 @@ pub(super) mod tests {
     use super::*;
 
     use crate::ecdsa::test_utils::{
-        set_up_ecdsa_payload, EcdsaPayloadTestHelper, TestEcdsaBlockReader,
-        TestEcdsaTranscriptBuilder,
+        fake_sign_with_ecdsa_context_with_quadruple, set_up_ecdsa_payload, EcdsaPayloadTestHelper,
+        TestEcdsaBlockReader, TestEcdsaTranscriptBuilder,
+    };
+    use ic_crypto_test_utils_canister_threshold_sigs::{
+        generate_key_transcript, CanisterThresholdSigTestEnvironment, IDkgParticipants,
     };
-    use ic_crypto_test_utils_canister_threshold_sigs::CanisterThresholdSigTestEnvironment;
     use ic_crypto_test_utils_reproducible_rng::{reproducible_rng, ReproducibleRng};
     use ic_logger::replica_logger::no_op_logger;
     use ic_test_utilities::types::ids::subnet_test_id;
     use ic_types::{
-        consensus::ecdsa::EcdsaPayload, crypto::canister_threshold_sig::idkg::IDkgTranscriptId,
+        consensus::ecdsa::{EcdsaPayload, UnmaskedTranscript},
+        crypto::canister_threshold_sig::idkg::IDkgTranscriptId,
         SubnetId,
     };
 
@@ -581,4 +627,146 @@ pub(super) mod tests {
         assert_eq!(payload.peek_next_transcript_id().id(), 5);
         assert!(config_ids(&payload).is_empty());
     }
+
+    fn get_current_unmasked_key_transcript(payload: &EcdsaPayload) -> UnmaskedTranscript {
+        let transcript = payload.key_transcript.current.clone();
+        transcript.unwrap().unmasked_transcript()
+    }
+
+    #[test]
+    fn test_matched_quadruples_are_not_purged() {
+        let mut rng = reproducible_rng();
+        let (mut payload, env, _) = set_up(&mut rng, subnet_test_id(1), Height::from(100));
+        let key_id = payload.key_transcript.key_id.clone();
+        let key_transcript = get_current_unmasked_key_transcript(&payload);
+
+        let (dealers, receivers) = env.choose_dealers_and_receivers(
+            &IDkgParticipants::AllNodesAsDealersAndReceivers,
+            &mut rng,
+        );
+        let transcript = generate_key_transcript(
+            &env,
+            &dealers,
+            &receivers,
+            AlgorithmId::ThresholdEcdsaSecp256k1,
+            &mut rng,
+        );
+        let key_transcript2 =
+            UnmaskedTranscript::try_from((Height::from(200), &transcript)).unwrap();
+
+        // Create three quadruples, with the current, a different, no key transcript.
+        let quadruple_ids = vec![
+            create_available_quadruple_with_key_transcript(&mut payload, 1, Some(key_transcript)),
+            create_available_quadruple_with_key_transcript(&mut payload, 2, Some(key_transcript2)),
+            create_available_quadruple_with_key_transcript(&mut payload, 3, None),
+        ];
+
+        // All three quadruples are matched with a context
+        let contexts = BTreeMap::from_iter(quadruple_ids.into_iter().map(|id| {
+            fake_sign_with_ecdsa_context_with_quadruple(id.id() as u8, key_id.clone(), Some(id))
+        }));
+
+        // 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_current_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();
+        let key_transcript = get_current_unmasked_key_transcript(&payload);
+
+        // Create three quadruples of the current key transcript
+        for i in 0..3 {
+            create_available_quadruple_with_key_transcript(&mut payload, i, Some(key_transcript));
+        }
+
+        // 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_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, 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();
+        let (mut payload, env, _) = set_up(&mut rng, subnet_test_id(1), Height::from(100));
+        let key_id = payload.key_transcript.key_id.clone();
+
+        let (dealers, receivers) = env.choose_dealers_and_receivers(
+            &IDkgParticipants::AllNodesAsDealersAndReceivers,
+            &mut rng,
+        );
+        let transcript = generate_key_transcript(
+            &env,
+            &dealers,
+            &receivers,
+            AlgorithmId::ThresholdEcdsaSecp256k1,
+            &mut rng,
+        );
+        let other_key_transcript =
+            UnmaskedTranscript::try_from((Height::from(200), &transcript)).unwrap();
+
+        // Create two quadruples of the other key transcript
+        let quadruple_ids = (0..2)
+            .map(|i| {
+                create_available_quadruple_with_key_transcript(
+                    &mut payload,
+                    i,
+                    Some(other_key_transcript),
+                )
+            })
+            .collect::<Vec<_>>();
+
+        // The first one is matched to a context
+        let contexts = BTreeMap::from_iter([fake_sign_with_ecdsa_context_with_quadruple(
+            1,
+            key_id.clone(),
+            Some(quadruple_ids[0].clone()),
+        )]);
+
+        // The second one should be purged
+        assert_eq!(payload.available_quadruples.len(), 2);
+        purge_old_key_quadruples(&mut payload, &contexts);
+        assert_eq!(payload.available_quadruples.len(), 1);
+
+        assert_eq!(
+            payload.available_quadruples.into_keys().next().unwrap(),
+            quadruple_ids[0]
+        );
+    }
 }