Merge branch 'eichhorl/2' into 'master'

fix(CON-894): do not reset ECDSA signature metric

We do this by adding a new metric that is persisted in the replicated state. 

See merge request dfinity-lab/public/ic!10024

rs/execution_environment/src/execution_environment.rs

@@ -41,9 +41,12 @@ use ic_logger::{error, info, warn, ReplicaLogger};
 use ic_metrics::{MetricsRegistry, Timer};
 use ic_registry_provisional_whitelist::ProvisionalWhitelist;
 use ic_registry_subnet_type::SubnetType;
-use ic_replicated_state::canister_state::system_state::PausedExecutionId;
 use ic_replicated_state::canister_state::NextExecution;
 use ic_replicated_state::ExecutionTask;
+use ic_replicated_state::{
+    canister_state::system_state::PausedExecutionId,
+    metadata_state::subnet_call_context_manager::SubnetCallContext,
+};
 use ic_replicated_state::{
     metadata_state::subnet_call_context_manager::{
         EcdsaDealingsContext, SetupInitialDkgContext, SignWithEcdsaContext,
@@ -386,6 +389,13 @@ impl ExecutionEnvironment {
                             },
                         );
 
+                        if matches!(
+                            (&context, &response.response_payload),
+                            (&SubnetCallContext::SignWithEcsda(_), &Payload::Data(_))
+                        ) {
+                            state.metadata.subnet_metrics.ecdsa_signature_agreements += 1;
+                        }
+
                         state.push_subnet_output_response(
                             Response {
                                 originator: request.sender,

rs/execution_environment/src/scheduler.rs

@@ -1954,6 +1954,10 @@ fn observe_replicated_state_metrics(
 
     metrics.observe_consumed_cycles(consumed_cycles_total);
 
+    metrics
+        .ecdsa_signature_agreements
+        .set(state.metadata.subnet_metrics.ecdsa_signature_agreements as i64);
+
     let observe_reading = |status: CanisterStatusType, num: i64| {
         metrics
             .registered_canisters

rs/execution_environment/src/scheduler/scheduler_metrics.rs

@@ -88,6 +88,7 @@ pub(super) struct SchedulerMetrics {
     pub(super) canister_paused_install_code: Histogram,
     pub(super) canister_aborted_install_code: Histogram,
     pub(super) inducted_messages: IntCounterVec,
+    pub(super) ecdsa_signature_agreements: IntGauge,
 }
 
 const LABEL_MESSAGE_KIND: &str = "kind";
@@ -200,6 +201,10 @@ impl SchedulerMetrics {
                 "replicated_state_consumed_cycles_since_replica_started",
                 "Number of cycles consumed since replica started",
             ),
+            ecdsa_signature_agreements: metrics_registry.int_gauge(
+                "replicated_state_ecdsa_signature_agreements_total",
+                "Total number of ECDSA signature agreements created",
+            ),
             input_queue_messages: metrics_registry.int_gauge_vec(
                 "execution_input_queue_messages",
                 "Count of messages currently enqueued in input queues, by message kind.",

rs/execution_environment/src/scheduler/tests.rs

@@ -10,6 +10,7 @@ use ic00::{
 };
 use ic_btc_types::NetworkInRequest;
 use ic_config::subnet_config::{CyclesAccountManagerConfig, SchedulerConfig, SubnetConfigs};
+use ic_error_types::RejectCode;
 use ic_ic00_types::{
     self as ic00, BitcoinGetBalanceArgs, CanisterIdRecord, CanisterStatusType, EcdsaCurve,
     EmptyBlob, Method, Payload as _,
@@ -30,7 +31,7 @@ use ic_test_utilities::{
     },
 };
 use ic_test_utilities_metrics::{fetch_gauge, fetch_int_gauge, fetch_int_gauge_vec, metric_vec};
-use ic_types::messages::{CallbackId, Payload, MAX_RESPONSE_COUNT_BYTES};
+use ic_types::messages::{CallbackId, Payload, RejectContext, Response, MAX_RESPONSE_COUNT_BYTES};
 use ic_types::methods::SystemMethod;
 use ic_types::{time::UNIX_EPOCH, ComputeAllocation, Cycles, NumBytes};
 use proptest::prelude::*;
@@ -2491,6 +2492,148 @@ fn scheduler_maintains_canister_order() {
     }
 }
 
+#[test]
+fn ecdsa_signature_agreements_metric_is_updated() {
+    let ecdsa_key = EcdsaKeyId {
+        curve: EcdsaCurve::Secp256k1,
+        name: String::from("secp256k1"),
+    };
+    let mut test = SchedulerTestBuilder::new()
+        .with_ecdsa_key(ecdsa_key.clone())
+        .build();
+
+    let canister_id = test.create_canister();
+
+    let payload = Encode!(&SignWithECDSAArgs {
+        message_hash: [0; 32],
+        derivation_path: Vec::new(),
+        key_id: ecdsa_key
+    })
+    .unwrap();
+
+    // inject two signing request
+    test.inject_call_to_ic00(
+        Method::SignWithECDSA,
+        payload.clone(),
+        test.ecdsa_signature_fee(),
+        canister_id,
+        InputQueueType::RemoteSubnet,
+    );
+    test.inject_call_to_ic00(
+        Method::SignWithECDSA,
+        payload,
+        test.ecdsa_signature_fee(),
+        canister_id,
+        InputQueueType::RemoteSubnet,
+    );
+    test.execute_round(ExecutionRoundType::OrdinaryRound);
+
+    // Check that the SubnetCallContextManager contains both requests.
+    let sign_with_ecdsa_contexts = &test
+        .state()
+        .metadata
+        .subnet_call_context_manager
+        .sign_with_ecdsa_contexts;
+    assert_eq!(sign_with_ecdsa_contexts.len(), 2);
+
+    // reject the first one
+    let (callback_id, context) = sign_with_ecdsa_contexts.iter().next().unwrap();
+    let response = Response {
+        originator: context.request.sender,
+        respondent: ic_types::CanisterId::ic_00(),
+        originator_reply_callback: *callback_id,
+        refund: context.request.payment,
+        response_payload: Payload::Reject(RejectContext {
+            code: RejectCode::SysFatal,
+            message: "".into(),
+        }),
+    };
+
+    test.state_mut().consensus_queue.push(response);
+    test.execute_round(ExecutionRoundType::OrdinaryRound);
+
+    observe_replicated_state_metrics(
+        test.scheduler().own_subnet_id,
+        test.state(),
+        1.into(),
+        &test.scheduler().metrics,
+        &no_op_logger(),
+    );
+
+    let ecdsa_signature_agreements_before = test
+        .state()
+        .metadata
+        .subnet_metrics
+        .ecdsa_signature_agreements;
+    let metric_before = fetch_int_gauge(
+        test.metrics_registry(),
+        "replicated_state_ecdsa_signature_agreements_total",
+    )
+    .unwrap();
+
+    // metric and state variable should not have been updated
+    assert_eq!(ecdsa_signature_agreements_before, metric_before);
+    assert_eq!(0, metric_before);
+
+    let sign_with_ecdsa_contexts = &test
+        .state()
+        .metadata
+        .subnet_call_context_manager
+        .sign_with_ecdsa_contexts;
+    assert_eq!(sign_with_ecdsa_contexts.len(), 1);
+
+    // send a reply to the second request
+    let (callback_id, context) = sign_with_ecdsa_contexts.iter().next().unwrap();
+    let response = Response {
+        originator: context.request.sender,
+        respondent: ic_types::CanisterId::ic_00(),
+        originator_reply_callback: *callback_id,
+        refund: context.request.payment,
+        response_payload: Payload::Data(
+            ic00::SignWithECDSAReply {
+                signature: vec![1, 2, 3],
+            }
+            .encode(),
+        ),
+    };
+
+    test.state_mut().consensus_queue.push(response);
+    test.execute_round(ExecutionRoundType::OrdinaryRound);
+
+    observe_replicated_state_metrics(
+        test.scheduler().own_subnet_id,
+        test.state(),
+        2.into(),
+        &test.scheduler().metrics,
+        &no_op_logger(),
+    );
+
+    let ecdsa_signature_agreements_after = test
+        .state()
+        .metadata
+        .subnet_metrics
+        .ecdsa_signature_agreements;
+    let metric_after = fetch_int_gauge(
+        test.metrics_registry(),
+        "replicated_state_ecdsa_signature_agreements_total",
+    )
+    .unwrap();
+
+    assert_eq!(
+        ecdsa_signature_agreements_before + 1,
+        ecdsa_signature_agreements_after
+    );
+    assert_eq!(ecdsa_signature_agreements_after, metric_after);
+
+    // Check that the request was removed.
+    let sign_with_ecdsa_contexts = &test
+        .state()
+        .metadata
+        .subnet_call_context_manager
+        .sign_with_ecdsa_contexts;
+    assert!(sign_with_ecdsa_contexts.is_empty());
+}
+
 #[test]
 fn consumed_cycles_ecdsa_outcalls_are_added_to_consumed_cycles_total() {
     let ecdsa_key = EcdsaKeyId {

rs/protobuf/def/state/metadata/v1/metadata.proto

@@ -168,6 +168,7 @@ message SubnetMetrics {
   types.v1.NominalCycles consumed_cycles_by_deleted_canisters = 1;
   types.v1.NominalCycles consumed_cycles_http_outcalls = 2;
   types.v1.NominalCycles consumed_cycles_ecdsa_outcalls = 3;
+  optional uint64 ecdsa_signature_agreements = 4;
 }
 
 message BitcoinGetSuccessorsFollowUpResponses {

rs/protobuf/gen/state/state.metadata.v1.rs

@@ -256,6 +256,8 @@ pub struct SubnetMetrics {
     #[prost(message, optional, tag = "3")]
     pub consumed_cycles_ecdsa_outcalls:
         ::core::option::Option<super::super::super::types::v1::NominalCycles>,
+    #[prost(uint64, optional, tag = "4")]
+    pub ecdsa_signature_agreements: ::core::option::Option<u64>,
 }
 #[allow(clippy::derive_partial_eq_without_eq)]
 #[derive(Clone, PartialEq, ::prost::Message)]

rs/replicated_state/src/metadata_state.rs

@@ -418,6 +418,7 @@ pub struct SubnetMetrics {
     pub consumed_cycles_by_deleted_canisters: NominalCycles,
     pub consumed_cycles_http_outcalls: NominalCycles,
     pub consumed_cycles_ecdsa_outcalls: NominalCycles,
+    pub ecdsa_signature_agreements: u64,
 }
 
 impl From<&SubnetMetrics> for pb_metadata::SubnetMetrics {
@@ -428,6 +429,7 @@ impl From<&SubnetMetrics> for pb_metadata::SubnetMetrics {
             ),
             consumed_cycles_http_outcalls: Some((&item.consumed_cycles_http_outcalls).into()),
             consumed_cycles_ecdsa_outcalls: Some((&item.consumed_cycles_ecdsa_outcalls).into()),
+            ecdsa_signature_agreements: Some(item.ecdsa_signature_agreements),
         }
     }
 }
@@ -450,6 +452,7 @@ impl TryFrom<pb_metadata::SubnetMetrics> for SubnetMetrics {
                 "SubnetMetrics::consumed_cycles_ecdsa_outcalls",
             )
             .unwrap_or_else(|_| NominalCycles::from(0_u128)),
+            ecdsa_signature_agreements: item.ecdsa_signature_agreements.unwrap_or_default(),
         })
     }
 }