Skip to content

Commit

Permalink
chore(registry): Remove obsolete ECDSA API (#3827)
Browse files Browse the repository at this point in the history
This PR removes the legacy ECDSA fields from the Registry canister's
subnet operations since those were obsolete.
  • Loading branch information
aterga authored Feb 6, 2025
1 parent cc2a047 commit 5e85add
Show file tree
Hide file tree
Showing 26 changed files with 10 additions and 157 deletions.
1 change: 0 additions & 1 deletion rs/boundary_node/ic_boundary/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ pub fn test_subnet_record() -> SubnetRecord {
max_number_of_canisters: 0,
ssh_readonly_access: vec![],
ssh_backup_access: vec![],
ecdsa_config: None,
chain_key_config: None,
}
}
Expand Down
2 changes: 1 addition & 1 deletion rs/crypto/src/keygen/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,7 @@ mod rotate_idkg_dealing_encryption_keys {
#[test]
fn should_not_rotate_key_when_no_ecdsa_config_exists() {
let setup = Setup::builder()
.with_ecdsa_subnet_config(EcdsaSubnetConfig::new_without_ecdsa_config(
.with_ecdsa_subnet_config(EcdsaSubnetConfig::new_without_chain_key_config(
subnet_id(),
Some(node_id()),
))
Expand Down
5 changes: 2 additions & 3 deletions rs/crypto/temp_crypto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1095,7 +1095,6 @@ impl EcdsaSubnetConfig {
max_number_of_canisters: 0,
ssh_readonly_access: vec![],
ssh_backup_access: vec![],
ecdsa_config: None,
chain_key_config: Some(ChainKeyConfig {
key_configs: vec![KeyConfig {
key_id: Some(ic_protobuf::types::v1::MasterPublicKeyId {
Expand All @@ -1117,9 +1116,9 @@ impl EcdsaSubnetConfig {
}
}

pub fn new_without_ecdsa_config(subnet_id: SubnetId, node_id: Option<NodeId>) -> Self {
pub fn new_without_chain_key_config(subnet_id: SubnetId, node_id: Option<NodeId>) -> Self {
let mut subnet_config = Self::new(subnet_id, node_id, None);
subnet_config.subnet_record.ecdsa_config = None;
subnet_config.subnet_record.chain_key_config = None;
subnet_config
}

Expand Down
2 changes: 1 addition & 1 deletion rs/crypto/tests/integration_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1212,7 +1212,7 @@ fn should_return_all_keys_registered_from_check_keys_with_registry_if_no_ecdsa_c
)
.with_time_source(Arc::clone(&time) as Arc<_>)
.with_node_id(node_id())
.with_ecdsa_subnet_config(EcdsaSubnetConfig::new_without_ecdsa_config(
.with_ecdsa_subnet_config(EcdsaSubnetConfig::new_without_chain_key_config(
subnet_id(),
Some(node_id()),
))
Expand Down
5 changes: 0 additions & 5 deletions rs/nns/integration_tests/src/subnet_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ fn test_submit_and_accept_update_subnet_proposal() {
max_number_of_canisters: 100,
ssh_readonly_access: vec![],
ssh_backup_access: vec![],
ecdsa_config: None,
chain_key_config: None,
};

Expand Down Expand Up @@ -87,9 +86,6 @@ fn test_submit_and_accept_update_subnet_proposal() {
is_halted: Some(true),
halt_at_cup_height: Some(true),
features: None,
ecdsa_config: None,
ecdsa_key_signing_enable: None,
ecdsa_key_signing_disable: None,
max_number_of_canisters: Some(200),
ssh_readonly_access: Some(vec!["pub_key_0".to_string()]),
ssh_backup_access: Some(vec!["pub_key_1".to_string()]),
Expand Down Expand Up @@ -171,7 +167,6 @@ fn test_submit_and_accept_update_subnet_proposal() {
max_number_of_canisters: 200,
ssh_readonly_access: vec!["pub_key_0".to_string()],
ssh_backup_access: vec!["pub_key_1".to_string()],
ecdsa_config: None,
chain_key_config: None,
}
);
Expand Down
1 change: 0 additions & 1 deletion rs/prep/src/subnet_configuration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,6 @@ impl SubnetConfig {
max_number_of_canisters: self.max_number_of_canisters,
ssh_readonly_access: self.ssh_readonly_access,
ssh_backup_access: self.ssh_backup_access,
ecdsa_config: None,
chain_key_config: self.chain_key_config,
};

Expand Down
10 changes: 2 additions & 8 deletions rs/protobuf/def/registry/subnet/v1/subnet.proto
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,6 @@ message SubnetRecord {
// to make sure the NNS can be backed up.
repeated string ssh_backup_access = 26;

// ECDSA Config. This field cannot be set back to `None` once it has been set
// to `Some`. To remove a key, the list of `key_ids` can be set to not include a particular key.
// If a removed key is not held by another subnet, it will be lost.
//
// Deprecated; please use chain_key_config instead.
EcdsaConfig ecdsa_config = 27;

// If `true`, the subnet will be halted after reaching the next cup height: it will no longer
// create or execute blocks.
//
Expand All @@ -81,14 +74,15 @@ message SubnetRecord {
// key. If the removed key is not held by another subnet, it will be lost.
optional ChainKeyConfig chain_key_config = 29;

reserved 1, 2, 4, 6, 13, 20, 21, 22;
reserved 1, 2, 4, 6, 13, 20, 21, 22, 27;
reserved "ic_version_id";
reserved "initial_dkg_transcript";
reserved "ingress_bytes_per_block_soft_cap";
reserved "gossip_config";
reserved "max_instructions_per_message";
reserved "max_instructions_per_round";
reserved "max_instructions_per_install_code";
reserved "ecdsa_config";
}

message EcdsaInitialization {
Expand Down
7 changes: 0 additions & 7 deletions rs/protobuf/src/gen/registry/registry.subnet.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,6 @@ pub struct SubnetRecord {
/// to make sure the NNS can be backed up.
#[prost(string, repeated, tag = "26")]
pub ssh_backup_access: ::prost::alloc::vec::Vec<::prost::alloc::string::String>,
/// ECDSA Config. This field cannot be set back to `None` once it has been set
/// to `Some`. To remove a key, the list of `key_ids` can be set to not include a particular key.
/// If a removed key is not held by another subnet, it will be lost.
///
/// Deprecated; please use chain_key_config instead.
#[prost(message, optional, tag = "27")]
pub ecdsa_config: ::core::option::Option<EcdsaConfig>,
/// If `true`, the subnet will be halted after reaching the next cup height: it will no longer
/// create or execute blocks.
///
Expand Down
7 changes: 0 additions & 7 deletions rs/protobuf/src/gen/state/registry.subnet.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,6 @@ pub struct SubnetRecord {
/// to make sure the NNS can be backed up.
#[prost(string, repeated, tag = "26")]
pub ssh_backup_access: ::prost::alloc::vec::Vec<::prost::alloc::string::String>,
/// ECDSA Config. This field cannot be set back to `None` once it has been set
/// to `Some`. To remove a key, the list of `key_ids` can be set to not include a particular key.
/// If a removed key is not held by another subnet, it will be lost.
///
/// Deprecated; please use chain_key_config instead.
#[prost(message, optional, tag = "27")]
pub ecdsa_config: ::core::option::Option<EcdsaConfig>,
/// If `true`, the subnet will be halted after reaching the next cup height: it will no longer
/// create or execute blocks.
///
Expand Down
7 changes: 0 additions & 7 deletions rs/protobuf/src/gen/types/registry.subnet.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,6 @@ pub struct SubnetRecord {
/// to make sure the NNS can be backed up.
#[prost(string, repeated, tag = "26")]
pub ssh_backup_access: ::prost::alloc::vec::Vec<::prost::alloc::string::String>,
/// ECDSA Config. This field cannot be set back to `None` once it has been set
/// to `Some`. To remove a key, the list of `key_ids` can be set to not include a particular key.
/// If a removed key is not held by another subnet, it will be lost.
///
/// Deprecated; please use chain_key_config instead.
#[prost(message, optional, tag = "27")]
pub ecdsa_config: ::core::option::Option<EcdsaConfig>,
/// If `true`, the subnet will be halted after reaching the next cup height: it will no longer
/// create or execute blocks.
///
Expand Down
1 change: 0 additions & 1 deletion rs/registry/admin/src/create_subnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,6 @@ impl ProposeToCreateSubnetCmd {
chain_key_config,

// Deprecated fields.
ecdsa_config: None,
ingress_bytes_per_block_soft_cap: Default::default(),
gossip_max_artifact_streams_per_peer: Default::default(),
gossip_max_chunk_wait_ms: Default::default(),
Expand Down
4 changes: 0 additions & 4 deletions rs/registry/admin/src/recover_subnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,6 @@ impl ProposeToUpdateRecoveryCupCmd {
replacement_nodes,
registry_store_uri,
chain_key_config,

// Deprecated fields
ecdsa_config: None,
}
}
}
Expand Down Expand Up @@ -242,7 +239,6 @@ mod tests {
.unwrap_or_else(|err| panic!("Invalid state hash: {}", err)),
replacement_nodes: None,
registry_store_uri: None,
ecdsa_config: None,
chain_key_config: None,
}
}
Expand Down
6 changes: 0 additions & 6 deletions rs/registry/admin/src/update_subnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,6 @@ impl ProposeToUpdateSubnetCmd {
chain_key_signing_disable,

// Deprecated fields
ecdsa_config: None,
ecdsa_key_signing_enable: None,
ecdsa_key_signing_disable: None,
max_artifact_streams_per_peer: None,
max_chunk_wait_ms: None,
max_duplicity: None,
Expand Down Expand Up @@ -381,9 +378,6 @@ mod tests {
is_halted: None,
halt_at_cup_height: None,
features: None,
ecdsa_config: None,
ecdsa_key_signing_enable: None,
ecdsa_key_signing_disable: None,
max_number_of_canisters: None,
ssh_readonly_access: None,
ssh_backup_access: None,
Expand Down
5 changes: 0 additions & 5 deletions rs/registry/canister/canister/registry.did
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ type CreateSubnetPayload = record {
gossip_pfn_evaluation_period_ms : nat32;
max_ingress_messages_per_block : nat64;
max_number_of_canisters : nat64;
ecdsa_config : opt EcdsaInitialConfig;
chain_key_config : opt InitialChainKeyConfig;
gossip_max_artifact_streams_per_peer : nat32;
replica_version_id : text;
Expand Down Expand Up @@ -255,7 +254,6 @@ type RecoverSubnetPayload = record {
replacement_nodes : opt vec principal;
subnet_id : principal;
registry_store_uri : opt record { text; text; nat64 };
ecdsa_config : opt EcdsaInitialConfig;
chain_key_config : opt InitialChainKeyConfig;
state_hash : blob;
time_ns : nat64;
Expand Down Expand Up @@ -423,9 +421,6 @@ type UpdateSubnetPayload = record {
chain_key_config : opt ChainKeyConfig;
chain_key_signing_enable : opt vec MasterPublicKeyId;
chain_key_signing_disable : opt vec MasterPublicKeyId;
ecdsa_config : opt EcdsaConfig;
ecdsa_key_signing_enable : opt vec EcdsaKeyId;
ecdsa_key_signing_disable : opt vec EcdsaKeyId;
};

type ChainKeyConfig = record {
Expand Down
4 changes: 2 additions & 2 deletions rs/registry/canister/src/invariants/crypto/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1020,11 +1020,11 @@ mod ecdsa_signing_subnet_lists {
max_queue_size: Default::default(),
})
.collect();
let ecdsa_config = ChainKeyConfig {
let chain_key_config = ChainKeyConfig {
key_configs,
..Default::default()
};
Some(ChainKeyConfigPb::from(ecdsa_config))
Some(ChainKeyConfigPb::from(chain_key_config))
} else {
None
};
Expand Down
9 changes: 0 additions & 9 deletions rs/registry/canister/src/mutations/do_create_subnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,6 @@ impl Registry {
/// Ensures that a valid `subnet_id` is specified for `KeyConfigRequest`s.
/// Ensures that master public keys (a) exist and (b) are present on the requested subnet.
fn validate_create_subnet_payload(&self, payload: &CreateSubnetPayload) {
assert_eq!(
payload.ecdsa_config, None,
"Field ecdsa_config is deprecated. Please use chain_key_config instead.",
);

// Verify that all Nodes exist
payload.node_ids.iter().for_each(|node_id| {
match self.get(
Expand Down Expand Up @@ -285,9 +280,6 @@ pub struct CreateSubnetPayload {
pub ssh_readonly_access: Vec<String>,
pub ssh_backup_access: Vec<String>,

// Obsolete. Please use `chain_key_config` instead.
pub ecdsa_config: Option<EcdsaInitialConfig>,

pub chain_key_config: Option<InitialChainKeyConfig>,

// TODO(NNS1-2444): The fields below are deprecated and they are not read anywhere.
Expand Down Expand Up @@ -524,7 +516,6 @@ impl From<CreateSubnetPayload> for SubnetRecord {
.expect("Invalid InitialChainKeyConfig")
})
.map(ChainKeyConfigPb::from),
ecdsa_config: None, // obsolete (chain_key_config is used instead now)
}
}
}
Expand Down
10 changes: 0 additions & 10 deletions rs/registry/canister/src/mutations/do_recover_subnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
use crate::chain_key::{InitialChainKeyConfigInternal, KeyConfigRequestInternal};
use crate::{
common::LOG_PREFIX,
mutations::do_create_subnet::EcdsaInitialConfig,
registry::{Registry, Version},
};
use candid::{CandidType, Deserialize, Encode};
Expand Down Expand Up @@ -225,11 +224,6 @@ impl Registry {
/// This is similar to validation in do_create_subnet except for constraints to avoid requesting
/// keys from the subnet.
fn validate_recover_subnet_payload(&self, payload: &RecoverSubnetPayload) {
assert_eq!(
payload.ecdsa_config, None,
"Field ecdsa_config is deprecated. Please use chain_key_config instead.",
);

let Some(initial_chain_key_config) = &payload.chain_key_config else {
return; // Nothing to do.
};
Expand Down Expand Up @@ -271,9 +265,6 @@ pub struct RecoverSubnetPayload {
/// downloaded
pub registry_store_uri: Option<(String, String, u64)>,

/// Obsolete. Please use `chain_key_config` instead.
pub ecdsa_config: Option<EcdsaInitialConfig>,

/// Chain key configuration must be specified if keys will be recovered to this subnet.
/// Any keys that this subnet could sign for will immediately be available to sign with.
/// Any new keys will not.
Expand Down Expand Up @@ -512,7 +503,6 @@ mod test {
state_hash: vec![],
replacement_nodes: None,
registry_store_uri: None,
ecdsa_config: None,
chain_key_config: None,
}
}
Expand Down
Loading

0 comments on commit 5e85add

Please sign in to comment.