Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(registry): Remove obsolete ECDSA API #3827

Merged
merged 2 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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