Skip to content

Commit

Permalink
Merge branch 'maksym/ic-1687-with-ecdsa-key-exc' into 'master'
Browse files Browse the repository at this point in the history 
refactor: [EXC-1633] use generic iDKG keys in ExecutionTest helper

This MR switches `ExecutionTestBuilder` to use generic iDKG keys instead of specific ECDSA ones. 

See merge request dfinity-lab/public/ic!20073
  • Loading branch information
@maksymar
maksymar committed Jul 1, 2024
2 parents 32be302 + 87d98af commit eb47b63
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 95 deletions.
88 changes: 33 additions & 55 deletions rs/execution_environment/src/execution_environment/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2314,34 +2314,23 @@ fn test_sign_with_threshold_key_fee_charged() {
}
);

match method {
Method::SignWithECDSA => {
let contexts = test
.state()
.metadata
.subnet_call_context_manager
.sign_with_ecdsa_contexts();
let (_, context) = contexts.iter().next().unwrap();
assert_eq!(context.request.payment.get(), payment - fee);

assert_eq!(
test.state()
.metadata
.subnet_metrics
.consumed_cycles_ecdsa_outcalls,
NominalCycles::from(fee)
);
}
Method::SignWithSchnorr => {
let contexts = test
.state()
.metadata
.subnet_call_context_manager
.sign_with_schnorr_contexts();
let (_, context) = contexts.iter().next().unwrap();
assert_eq!(context.request.payment.get(), payment - fee);
}
let subnet_call_context_manager = &test.state().metadata.subnet_call_context_manager;
let contexts = match method {
Method::SignWithECDSA => subnet_call_context_manager.sign_with_ecdsa_contexts(),
Method::SignWithSchnorr => subnet_call_context_manager.sign_with_schnorr_contexts(),
_ => panic!("Unexpected method"),
};
let (_, context) = contexts.iter().next().unwrap();
assert_eq!(context.request.payment.get(), payment - fee);

if let Method::SignWithECDSA = method {
assert_eq!(
test.state()
.metadata
.subnet_metrics
.consumed_cycles_ecdsa_outcalls,
NominalCycles::from(fee)
);
}

assert_eq!(
Expand Down Expand Up @@ -2615,36 +2604,25 @@ fn test_sign_with_threshold_key_fee_ignored_for_nns() {
state: IngressState::Processing,
}
);
match method {
Method::SignWithECDSA => {
let contexts = test
.state()
.metadata
.subnet_call_context_manager
.sign_with_ecdsa_contexts();
let (_, context) = contexts.iter().next().unwrap();
assert_eq!(context.request.payment, Cycles::zero());

assert_eq!(
test.state()
.metadata
.subnet_metrics
.consumed_cycles_ecdsa_outcalls,
NominalCycles::from(0)
);
}
Method::SignWithSchnorr => {
let contexts = test
.state()
.metadata
.subnet_call_context_manager
.sign_with_schnorr_contexts();
let (_, context) = contexts.iter().next().unwrap();
assert_eq!(context.request.payment, Cycles::zero());
}

let subnet_call_context_manager = &test.state().metadata.subnet_call_context_manager;
let contexts = match method {
Method::SignWithECDSA => subnet_call_context_manager.sign_with_ecdsa_contexts(),
Method::SignWithSchnorr => subnet_call_context_manager.sign_with_schnorr_contexts(),
_ => panic!("Unexpected method"),
}
};
let (_, context) = contexts.iter().next().unwrap();
assert_eq!(context.request.payment, Cycles::zero());

if let Method::SignWithECDSA = method {
assert_eq!(
test.state()
.metadata
.subnet_metrics
.consumed_cycles_ecdsa_outcalls,
NominalCycles::from(0)
);
}
assert_eq!(
test.state()
.metadata
Expand Down
35 changes: 18 additions & 17 deletions rs/execution_environment/src/scheduler/test_utilities.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ use ic_interfaces::execution_environment::{
};
use ic_logger::{replica_logger::no_op_logger, ReplicaLogger};
use ic_management_canister_types::{
CanisterInstallMode, CanisterStatusType, EcdsaKeyId, InstallCodeArgs, MasterPublicKeyId,
Method, Payload, IC_00,
CanisterInstallMode, CanisterStatusType, InstallCodeArgs, MasterPublicKeyId, Method, Payload,
IC_00,
};
use ic_metrics::MetricsRegistry;
use ic_registry_routing_table::{CanisterIdRange, RoutingTable};
Expand Down Expand Up @@ -648,7 +648,7 @@ pub(crate) struct SchedulerTestBuilder {
rate_limiting_of_heap_delta: bool,
deterministic_time_slicing: bool,
log: ReplicaLogger,
ecdsa_keys: Vec<EcdsaKeyId>,
idkg_keys: Vec<MasterPublicKeyId>,
metrics_registry: MetricsRegistry,
round_summary: Option<ExecutionRoundSummary>,
canister_snapshot_flag: bool,
Expand All @@ -673,7 +673,7 @@ impl Default for SchedulerTestBuilder {
rate_limiting_of_heap_delta: false,
deterministic_time_slicing: true,
log: no_op_logger(),
ecdsa_keys: vec![],
idkg_keys: vec![],
metrics_registry: MetricsRegistry::new(),
round_summary: None,
canister_snapshot_flag: false,
Expand Down Expand Up @@ -723,15 +723,15 @@ impl SchedulerTestBuilder {
}
}

pub fn with_ecdsa_key(self, ecdsa_key: EcdsaKeyId) -> Self {
pub fn with_idkg_key(self, idkg_key: MasterPublicKeyId) -> Self {
Self {
ecdsa_keys: vec![ecdsa_key],
idkg_keys: vec![idkg_key],
..self
}
}

pub fn with_ecdsa_keys(self, ecdsa_keys: Vec<EcdsaKeyId>) -> Self {
Self { ecdsa_keys, ..self }
pub fn with_idkg_keys(self, idkg_keys: Vec<MasterPublicKeyId>) -> Self {
Self { idkg_keys, ..self }
}

pub fn with_batch_time(self, batch_time: Time) -> Self {
Expand Down Expand Up @@ -775,34 +775,35 @@ impl SchedulerTestBuilder {
state.metadata.batch_time = self.batch_time;

let config = SubnetConfig::new(self.subnet_type).cycles_account_manager_config;
for ecdsa_key in &self.ecdsa_keys {
state.metadata.network_topology.idkg_signing_subnets.insert(
MasterPublicKeyId::Ecdsa(ecdsa_key.clone()),
vec![self.own_subnet_id],
);
for idkg_key in &self.idkg_keys {
state
.metadata
.network_topology
.idkg_signing_subnets
.insert(idkg_key.clone(), vec![self.own_subnet_id]);
state
.metadata
.network_topology
.subnets
.get_mut(&self.own_subnet_id)
.unwrap()
.idkg_keys_held
.insert(MasterPublicKeyId::Ecdsa(ecdsa_key.clone()));
.insert(idkg_key.clone());

registry_settings.chain_key_settings.insert(
MasterPublicKeyId::Ecdsa(ecdsa_key.clone()),
idkg_key.clone(),
ChainKeySettings {
max_queue_size: 20,
pre_signatures_to_create_in_advance: 5,
},
);
}
let idkg_subnet_public_keys: BTreeMap<_, _> = self
.ecdsa_keys
.idkg_keys
.into_iter()
.map(|key_id| {
(
MasterPublicKeyId::Ecdsa(key_id),
key_id,
MasterPublicKey {
algorithm_id: AlgorithmId::Secp256k1,
public_key: b"abababab".to_vec(),
Expand Down
41 changes: 20 additions & 21 deletions rs/execution_environment/src/scheduler/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use ic_interfaces::execution_environment::SubnetAvailableMemory;
use ic_logger::replica_logger::no_op_logger;
use ic_management_canister_types::{
self as ic00, BoundedHttpHeaders, CanisterHttpResponsePayload, CanisterIdRecord,
CanisterStatusType, DerivationPath, EcdsaCurve, EcdsaKeyId, EmptyBlob, Method, Payload as _,
CanisterStatusType, DerivationPath, EcdsaKeyId, EmptyBlob, Method, Payload as _,
TakeCanisterSnapshotArgs, UninstallCodeArgs,
};
use ic_registry_routing_table::CanisterIdRange;
Expand Down Expand Up @@ -3480,20 +3480,17 @@ 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 key_id = make_ecdsa_key_id(0);
let mut test = SchedulerTestBuilder::new()
.with_ecdsa_key(ecdsa_key.clone())
.with_idkg_key(MasterPublicKeyId::Ecdsa(key_id.clone()))
.build();

let canister_id = test.create_canister();

let payload = Encode!(&SignWithECDSAArgs {
message_hash: [0; 32],
derivation_path: DerivationPath::new(Vec::new()),
key_id: ecdsa_key
key_id,
})
.unwrap();

Expand Down Expand Up @@ -3613,13 +3610,9 @@ fn ecdsa_signature_agreements_metric_is_updated() {

#[test]
fn consumed_cycles_ecdsa_outcalls_are_added_to_consumed_cycles_total() {
let ecdsa_key = EcdsaKeyId {
curve: EcdsaCurve::Secp256k1,
name: String::from("secp256k1"),
};

let key_id = make_ecdsa_key_id(0);
let mut test = SchedulerTestBuilder::new()
.with_ecdsa_key(ecdsa_key.clone())
.with_idkg_key(MasterPublicKeyId::Ecdsa(key_id.clone()))
.build();

let fee = test.ecdsa_signature_fee();
Expand Down Expand Up @@ -3648,7 +3641,7 @@ fn consumed_cycles_ecdsa_outcalls_are_added_to_consumed_cycles_total() {
Encode!(&SignWithECDSAArgs {
message_hash: [0; 32],
derivation_path: DerivationPath::new(Vec::new()),
key_id: ecdsa_key
key_id,
})
.unwrap(),
payment,
Expand Down Expand Up @@ -5333,7 +5326,7 @@ fn test_is_next_method_added_to_task_queue() {
assert_eq!(heartbeat_and_timer_canister_ids, BTreeSet::from([canister]));
}

pub(crate) fn make_key_id(id: u64) -> EcdsaKeyId {
pub(crate) fn make_ecdsa_key_id(id: u64) -> EcdsaKeyId {
EcdsaKeyId::from_str(&format!("Secp256k1:key_{:?}", id)).unwrap()
}

Expand All @@ -5358,9 +5351,9 @@ fn inject_ecdsa_signing_request(test: &mut SchedulerTest, key_id: &EcdsaKeyId) {

#[test]
fn test_sign_with_ecdsa_contexts_are_not_updated_without_quadruples() {
let key_id = make_key_id(0);
let key_id = make_ecdsa_key_id(0);
let mut test = SchedulerTestBuilder::new()
.with_ecdsa_key(key_id.clone())
.with_idkg_key(MasterPublicKeyId::Ecdsa(key_id.clone()))
.build();

inject_ecdsa_signing_request(&mut test, &key_id);
Expand All @@ -5380,9 +5373,9 @@ fn test_sign_with_ecdsa_contexts_are_not_updated_without_quadruples() {

#[test]
fn test_sign_with_ecdsa_contexts_are_updated_with_quadruples() {
let key_id = make_key_id(0);
let key_id = make_ecdsa_key_id(0);
let mut test = SchedulerTestBuilder::new()
.with_ecdsa_key(key_id.clone())
.with_idkg_key(MasterPublicKeyId::Ecdsa(key_id.clone()))
.build();
let pre_sig_id = PreSigId(0);
let pre_sig_ids = BTreeSet::from_iter([pre_sig_id]);
Expand Down Expand Up @@ -5431,9 +5424,15 @@ fn test_sign_with_ecdsa_contexts_are_updated_with_quadruples() {

#[test]
fn test_sign_with_ecdsa_contexts_are_matched_under_multiple_keys() {
let key_ids: Vec<_> = (0..3).map(make_key_id).collect();
let key_ids: Vec<_> = (0..3).map(make_ecdsa_key_id).collect();
let mut test = SchedulerTestBuilder::new()
.with_ecdsa_keys(key_ids.clone())
.with_idkg_keys(
key_ids
.iter()
.cloned()
.map(MasterPublicKeyId::Ecdsa)
.collect(),
)
.build();

// Deliver 2 quadruples for the first key, 1 for the second, 0 for the third
Expand Down
3 changes: 1 addition & 2 deletions rs/test_utilities/execution_environment/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1720,8 +1720,7 @@ impl ExecutionTestBuilder {
}

pub fn with_idkg_key(mut self, key_id: MasterPublicKeyId) -> Self {
self.idkg_keys_with_signing_enabled
.insert(key_id.clone(), true);
self.idkg_keys_with_signing_enabled.insert(key_id, true);
self
}

Expand Down

0 comments on commit eb47b63

Please sign in to comment.