feat(ecdsa): CON-1217 Payload builder/verifier for improved tECDSA la…

…tency

rs/consensus/src/ecdsa/payload_builder.rs

@@ -24,6 +24,7 @@ use ic_logger::{error, info, warn, ReplicaLogger};
 use ic_registry_client_helpers::subnet::SubnetRegistry;
 use ic_registry_subnet_features::EcdsaConfig;
 use ic_replicated_state::{metadata_state::subnet_call_context_manager::*, ReplicatedState};
+use ic_types::consensus::ecdsa::ECDSA_IMPROVED_LATENCY;
 use ic_types::{
     batch::ValidationContext,
     consensus::{
@@ -288,24 +289,49 @@ fn create_summary_payload_helper(
     };
 
     let mut ecdsa_summary = if is_new_key_transcript {
-        ecdsa::EcdsaPayload {
-            signature_agreements: ecdsa_payload.signature_agreements.clone(),
-            ongoing_signatures: ecdsa_payload.ongoing_signatures.clone(),
-            available_quadruples: BTreeMap::new(),
-            quadruples_in_creation: BTreeMap::new(),
-            uid_generator: ecdsa_payload.uid_generator.clone(),
-            // This will clear the current ongoing reshares, and
-            // the execution requests will be restarted with the
-            // new key and different transcript IDs.
-            ongoing_xnet_reshares: BTreeMap::new(),
-            xnet_reshare_agreements: ecdsa_payload.xnet_reshare_agreements.clone(),
-            idkg_transcripts: BTreeMap::new(),
-            key_transcript,
+        if ECDSA_IMPROVED_LATENCY {
+            ecdsa::EcdsaPayload {
+                signature_agreements: ecdsa_payload.signature_agreements.clone(),
+                ongoing_signatures: BTreeMap::new(),
+                // We keep available quadruples for now, even if the key transcript
+                // changed, as we don't know if they are part of ongoing signature
+                // requests. Instead we will purge them once the certified state
+                // height catches up with the height of this summary block.
+                available_quadruples: ecdsa_payload.available_quadruples.clone(),
+                quadruples_in_creation: BTreeMap::new(),
+                uid_generator: ecdsa_payload.uid_generator.clone(),
+                // This will clear the current ongoing reshares, and
+                // the execution requests will be restarted with the
+                // new key and different transcript IDs.
+                ongoing_xnet_reshares: BTreeMap::new(),
+                xnet_reshare_agreements: ecdsa_payload.xnet_reshare_agreements.clone(),
+                idkg_transcripts: BTreeMap::new(),
+                key_transcript,
+            }
+        } else {
+            ecdsa::EcdsaPayload {
+                signature_agreements: ecdsa_payload.signature_agreements.clone(),
+                ongoing_signatures: ecdsa_payload.ongoing_signatures.clone(),
+                available_quadruples: BTreeMap::new(),
+                quadruples_in_creation: BTreeMap::new(),
+                uid_generator: ecdsa_payload.uid_generator.clone(),
+                // This will clear the current ongoing reshares, and
+                // the execution requests will be restarted with the
+                // new key and different transcript IDs.
+                ongoing_xnet_reshares: BTreeMap::new(),
+                xnet_reshare_agreements: ecdsa_payload.xnet_reshare_agreements.clone(),
+                idkg_transcripts: BTreeMap::new(),
+                key_transcript,
+            }
         }
     } else {
         ecdsa::EcdsaPayload {
             signature_agreements: ecdsa_payload.signature_agreements.clone(),
-            ongoing_signatures: ecdsa_payload.ongoing_signatures.clone(),
+            ongoing_signatures: if ECDSA_IMPROVED_LATENCY {
+                BTreeMap::new()
+            } else {
+                ecdsa_payload.ongoing_signatures.clone()
+            },
             available_quadruples: ecdsa_payload.available_quadruples.clone(),
             quadruples_in_creation: ecdsa_payload.quadruples_in_creation.clone(),
             uid_generator: ecdsa_payload.uid_generator.clone(),
@@ -614,31 +640,51 @@ pub(crate) fn create_data_payload_helper_2(
     let request_expiry_time = ecdsa_config
         .signature_request_timeout_ns
         .and_then(|timeout| context_time.checked_sub(Duration::from_nanos(timeout)));
-    signatures::update_signature_agreements(all_signing_requests, signature_builder, ecdsa_payload);
-    let new_signing_requests = get_signing_requests(
-        height,
-        request_expiry_time,
-        ecdsa_payload,
-        all_signing_requests,
-        enabled_signing_keys,
-        ecdsa_payload_metrics,
-    );
-    signatures::update_ongoing_signatures(
-        new_signing_requests,
-        ecdsa_config.quadruples_to_create_in_advance,
-        ecdsa_payload,
-        log,
-    )?;
+
+    if ECDSA_IMPROVED_LATENCY {
+        signatures::update_signature_agreements_improved_latency(
+            all_signing_requests,
+            signature_builder,
+            request_expiry_time,
+            ecdsa_payload,
+            enabled_signing_keys,
+            ecdsa_payload_metrics,
+        );
+    } else {
+        signatures::update_signature_agreements(
+            all_signing_requests,
+            signature_builder,
+            ecdsa_payload,
+        );
+        let new_signing_requests = get_signing_requests(
+            height,
+            request_expiry_time,
+            ecdsa_payload,
+            all_signing_requests,
+            enabled_signing_keys,
+            ecdsa_payload_metrics,
+        );
+        signatures::update_ongoing_signatures(
+            new_signing_requests,
+            ecdsa_config.quadruples_to_create_in_advance,
+            ecdsa_payload,
+            log,
+        )?;
+    }
 
     if matches!(certified_height, CertifiedHeight::ReachedSummaryHeight) {
         quadruples::purge_old_key_quadruples(ecdsa_payload, all_signing_requests);
     }
 
-    let matched_quadruples = all_signing_requests
-        .values()
-        .filter_map(|context| context.matched_quadruple.as_ref())
-        .filter(|(qid, _)| ecdsa_payload.available_quadruples.contains_key(qid))
-        .count();
+    let matched_quadruples = if ECDSA_IMPROVED_LATENCY {
+        all_signing_requests
+            .values()
+            .filter_map(|context| context.matched_quadruple.as_ref())
+            .filter(|(qid, _)| ecdsa_payload.available_quadruples.contains_key(qid))
+            .count()
+    } else {
+        0
+    };
     quadruples::make_new_quadruples_if_needed(ecdsa_config, ecdsa_payload, matched_quadruples);
 
     let mut new_transcripts =

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

@@ -239,8 +239,7 @@ pub(super) fn purge_old_key_quadruples(
 pub(super) fn make_new_quadruples_if_needed(
     ecdsa_config: &EcdsaConfig,
     ecdsa_payload: &mut ecdsa::EcdsaPayload,
-    //TODO(CON-1149): Pass value to helper
-    _matched_quadruples: usize,
+    matched_quadruples: usize,
 ) {
     if let Some(key_transcript) = &ecdsa_payload.key_transcript.current {
         let node_ids: Vec<_> = key_transcript.receivers().iter().copied().collect();
@@ -249,7 +248,7 @@ pub(super) fn make_new_quadruples_if_needed(
             key_transcript.registry_version(),
             ecdsa_config,
             ecdsa_payload,
-            0,
+            matched_quadruples,
         )
     }
 }

rs/consensus/src/ecdsa/payload_builder/signatures.rs

@@ -1,14 +1,18 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, BTreeSet};
 
-use ic_ic00_types::{Payload, SignWithECDSAReply};
+use ic_error_types::RejectCode;
+use ic_ic00_types::{EcdsaKeyId, Payload, SignWithECDSAReply};
 use ic_logger::{debug, ReplicaLogger};
 use ic_replicated_state::metadata_state::subnet_call_context_manager::SignWithEcdsaContext;
 use ic_types::{
-    consensus::ecdsa, crypto::canister_threshold_sig::ExtendedDerivationPath, messages::CallbackId,
+    consensus::ecdsa,
+    crypto::canister_threshold_sig::ExtendedDerivationPath,
+    messages::{CallbackId, RejectContext},
+    Time,
 };
 use phantom_newtype::Id;
 
-use crate::ecdsa::signer::EcdsaSignatureBuilder;
+use crate::{consensus::metrics::EcdsaPayloadMetrics, ecdsa::signer::EcdsaSignatureBuilder};
 
 use super::EcdsaPayloadError;
 
@@ -113,6 +117,132 @@ pub(crate) fn update_ongoing_signatures(
     Ok(())
 }
 
+/// Update signature agreements in the data payload by:
+/// - dropping agreements that don't have a [SignWithEcdsaContext] anymore (because
+///   the response has been delivered)
+/// - setting remaining agreements to "Reported" (the signing response was delivered
+///   in the previous round, the context will be removed when the previous block is
+///   finalized)
+/// - rejecting signature contexts that are expired or request an invalid key.
+/// - adding new agreements as "Unreported" by combining shares in the ECDSA pool.
+pub(crate) fn update_signature_agreements_improved_latency(
+    all_requests: &BTreeMap<CallbackId, SignWithEcdsaContext>,
+    signature_builder: &dyn EcdsaSignatureBuilder,
+    request_expiry_time: Option<Time>,
+    payload: &mut ecdsa::EcdsaPayload,
+    valid_keys: &BTreeSet<EcdsaKeyId>,
+    ecdsa_payload_metrics: Option<&EcdsaPayloadMetrics>,
+) {
+    let all_random_ids = all_requests
+        .iter()
+        .map(|(_, context)| context.pseudo_random_id)
+        .collect::<BTreeSet<_>>();
+
+    // We first clean up the existing signature_agreements by keeping those
+    // that can still be found in the signing_requests for dedup purpose.
+    // We only need the "Reported" status because they would have already
+    // been reported when the previous block become finalized.
+    payload.signature_agreements = payload
+        .signature_agreements
+        .keys()
+        .filter(|random_id| all_random_ids.contains(*random_id))
+        .map(|random_id| (*random_id, ecdsa::CompletedSignature::ReportedToExecution))
+        .collect();
+
+    // Then we collect new signatures into the signature_agreements
+    for (callback_id, context) in all_requests {
+        if payload
+            .signature_agreements
+            .contains_key(&context.pseudo_random_id)
+        {
+            continue;
+        }
+        if !valid_keys.contains(&context.key_id) {
+            // Reject new requests with unknown key Ids.
+            // Note that no quadruples are consumed at this stage.
+            let response = ic_types::messages::Response {
+                originator: context.request.sender,
+                respondent: ic_types::CanisterId::ic_00(),
+                originator_reply_callback: *callback_id,
+                refund: context.request.payment,
+                response_payload: ic_types::messages::Payload::Reject(RejectContext::new(
+                    RejectCode::CanisterReject,
+                    format!(
+                        "Invalid or disabled key_id in signature request: {:?}",
+                        context.key_id
+                    ),
+                )),
+            };
+            payload.signature_agreements.insert(
+                context.pseudo_random_id,
+                ecdsa::CompletedSignature::Unreported(response),
+            );
+
+            if let Some(metrics) = ecdsa_payload_metrics {
+                metrics.payload_errors_inc("invalid_keyid_requests");
+            }
+            continue;
+        }
+
+        // We can only remove expired requests once they were matched with a
+        // quadruple. Otherwise the context may be matched with a quadruple
+        // at the next certified state height, which then wouldn't be removed.
+        let Some((quadruple_id, _)) = context.matched_quadruple.as_ref() else {
+            continue;
+        };
+
+        if request_expiry_time.is_some_and(|expiry| context.batch_time < expiry) {
+            let response = ic_types::messages::Response {
+                originator: context.request.sender,
+                respondent: ic_types::CanisterId::ic_00(),
+                originator_reply_callback: *callback_id,
+                refund: context.request.payment,
+                response_payload: ic_types::messages::Payload::Reject(RejectContext::new(
+                    RejectCode::CanisterError,
+                    "Signature request expired",
+                )),
+            };
+            payload.signature_agreements.insert(
+                context.pseudo_random_id,
+                ecdsa::CompletedSignature::Unreported(response),
+            );
+            payload.available_quadruples.remove(quadruple_id);
+
+            if let Some(metrics) = ecdsa_payload_metrics {
+                metrics.payload_errors_inc("expired_requests");
+            }
+
+            continue;
+        }
+
+        let Some(signature) = signature_builder.get_completed_signature_from_context(context)
+        else {
+            continue;
+        };
+
+        let response = ic_types::messages::Response {
+            originator: context.request.sender,
+            respondent: ic_types::CanisterId::ic_00(),
+            originator_reply_callback: *callback_id,
+            // Execution is responsible for burning the appropriate cycles
+            // before pushing the new context, so any remaining cycles can
+            // be refunded to the canister.
+            refund: context.request.payment,
+            response_payload: ic_types::messages::Payload::Data(
+                SignWithECDSAReply {
+                    signature: signature.signature.clone(),
+                }
+                .encode(),
+            ),
+        };
+        payload.signature_agreements.insert(
+            context.pseudo_random_id,
+            ecdsa::CompletedSignature::Unreported(response),
+        );
+        payload.available_quadruples.remove(quadruple_id);
+    }
+}
+
 /// Helper to build threshold signature inputs from the context and
 /// the pre-signature quadruple
 pub(crate) fn build_signature_inputs(