diff --git a/rs/nns/integration_tests/src/update_unassigned_nodes_config.rs b/rs/nns/integration_tests/src/update_unassigned_nodes_config.rs index 497f6a7b4d04..5bdf377481f9 100644 --- a/rs/nns/integration_tests/src/update_unassigned_nodes_config.rs +++ b/rs/nns/integration_tests/src/update_unassigned_nodes_config.rs @@ -28,7 +28,7 @@ fn test_submit_update_ssh_readonly_access_for_all_unassigned_nodes() { .build(); let nns_canisters = NnsCanisters::set_up(&runtime, nns_init_payload).await; - // first we need to make sure that the unassigned nodes config contains a blessed replica version + // first we need to make sure that the unassigned nodes config contains an elected replica version let replica_version = ReplicaVersion::default().to_string(); let payload = DeployGuestosToAllUnassignedNodesPayload { elected_replica_version: replica_version.clone(), diff --git a/rs/nns/integration_tests/src/upgrades_handler.rs b/rs/nns/integration_tests/src/upgrades_handler.rs index 8095c323ed69..3b579b0dc86c 100644 --- a/rs/nns/integration_tests/src/upgrades_handler.rs +++ b/rs/nns/integration_tests/src/upgrades_handler.rs @@ -12,10 +12,10 @@ use ic_nns_test_utils::{ common::NnsInitPayloadsBuilder, governance::{get_pending_proposals, submit_external_update_proposal, wait_for_final_state}, itest_helpers::{NnsCanisters, state_machine_test_on_nns_subnet}, - registry::get_value_or_panic, + registry::get_value, }; -use ic_protobuf::registry::replica_version::v1::BlessedReplicaVersions; -use ic_registry_keys::make_blessed_replica_versions_key; +use ic_protobuf::registry::replica_version::v1::ReplicaVersionRecord; +use ic_registry_keys::make_replica_version_key; use ic_types::ReplicaVersion; use registry_canister::mutations::{ do_deploy_guestos_to_all_unassigned_nodes::DeployGuestosToAllUnassignedNodesPayload, @@ -48,6 +48,15 @@ async fn assert_failed_with_reason(gov: &Canister<'_>, proposal_id: ProposalId, ); } +async fn is_elected_version(registry: &Canister<'_>, replica_version_id: &str) -> bool { + get_value::( + registry, + make_replica_version_key(replica_version_id).as_bytes(), + ) + .await + .is_some() +} + #[test] fn test_submit_and_accept_update_elected_replica_versions_proposal() { state_machine_test_on_nns_subnet(|runtime| async move { @@ -72,7 +81,7 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() { guest_launch_measurements: None, replica_versions_to_unelect: unelect.iter().map(|s| s.to_string()).collect(), }; - let bless_version_payload = |version_id: &str| -> ReviseElectedGuestosVersionsPayload { + let elect_version_payload = |version_id: &str| -> ReviseElectedGuestosVersionsPayload { update_versions_payload(Some(version_id.into()), vec![]) }; let retire_version_payload = |ids: Vec<&str>| -> ReviseElectedGuestosVersionsPayload { @@ -89,11 +98,11 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() { let version_to_elect_and_unelect2 = "version_to_elect_and_unelect2"; let version_to_elect = "version_to_elect"; - // bless three versions + // elect three versions let setup = vec![ - bless_version_payload(version_to_elect_and_unelect1), - bless_version_payload(version_to_elect_and_unelect2), - bless_version_payload(unassigned_nodes_version), + elect_version_payload(version_to_elect_and_unelect1), + elect_version_payload(version_to_elect_and_unelect2), + elect_version_payload(unassigned_nodes_version), ]; for payload in setup { @@ -105,21 +114,18 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() { ); } - assert_eq!( - get_value_or_panic::( - &nns_canisters.registry, - make_blessed_replica_versions_key().as_bytes() - ) - .await, - BlessedReplicaVersions { - blessed_version_ids: vec![ - default_version.to_string(), - version_to_elect_and_unelect1.to_string(), - version_to_elect_and_unelect2.to_string(), - unassigned_nodes_version.to_string(), - ] - } - ); + // Check state of elected versions + for version in [ + default_version, + version_to_elect_and_unelect1, + version_to_elect_and_unelect2, + unassigned_nodes_version, + ] { + assert!(is_elected_version(&nns_canisters.registry, version).await); + } + for version in [version_to_elect] { + assert!(!is_elected_version(&nns_canisters.registry, version).await); + } // update unassigned version let deploy_unassigned_payload = DeployGuestosToAllUnassignedNodesPayload { @@ -165,8 +171,8 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() { Some("All parameters to elect a version have to be either set or unset"), ), ( - bless_version_payload(""), - Some("Blessed an empty version ID"), + elect_version_payload(""), + Some("Elected an empty version ID"), ), ( update_versions_payload( @@ -198,20 +204,13 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() { } } - assert_eq!( - get_value_or_panic::( - &nns_canisters.registry, - make_blessed_replica_versions_key().as_bytes() - ) - .await, - BlessedReplicaVersions { - blessed_version_ids: vec![ - default_version.to_string(), - unassigned_nodes_version.to_string(), - version_to_elect.to_string() - ] - } - ); + // Check state of elected versions + for version in [default_version, unassigned_nodes_version, version_to_elect] { + assert!(is_elected_version(&nns_canisters.registry, version).await); + } + for version in [version_to_elect_and_unelect1, version_to_elect_and_unelect2] { + assert!(!is_elected_version(&nns_canisters.registry, version).await); + } // No proposals should be pending now. let pending_proposals = get_pending_proposals(gov).await; diff --git a/rs/registry/canister/tests/update_subnet_and_bless_replica_version.rs b/rs/registry/canister/tests/update_subnet_and_bless_replica_version.rs index c9cde8ef725f..67577e827110 100644 --- a/rs/registry/canister/tests/update_subnet_and_bless_replica_version.rs +++ b/rs/registry/canister/tests/update_subnet_and_bless_replica_version.rs @@ -6,15 +6,10 @@ use ic_nns_test_utils::{ forward_call_via_universal_canister, set_up_registry_canister, set_up_universal_canister, state_machine_test_on_nns_subnet, }, - registry::{get_value_or_panic, invariant_compliant_mutation_as_atomic_req}, -}; -use ic_protobuf::registry::{ - replica_version::v1::{BlessedReplicaVersions, ReplicaVersionRecord}, - subnet::v1::SubnetRecord, -}; -use ic_registry_keys::{ - make_blessed_replica_versions_key, make_replica_version_key, make_subnet_record_key, + registry::{get_value, get_value_or_panic, invariant_compliant_mutation_as_atomic_req}, }; +use ic_protobuf::registry::{replica_version::v1::ReplicaVersionRecord, subnet::v1::SubnetRecord}; +use ic_registry_keys::{make_replica_version_key, make_subnet_record_key}; use ic_test_utilities_types::ids::subnet_test_id; use assert_matches::assert_matches; @@ -62,14 +57,15 @@ fn test_the_anonymous_user_cannot_elect_a_version() { ) .await; + let replica_version_id = "version_43"; let payload = ReviseElectedGuestosVersionsPayload { - replica_version_to_elect: Some("version_43".into()), + replica_version_to_elect: Some(replica_version_id.into()), release_package_sha256_hex: None, release_package_urls: vec![], guest_launch_measurements: None, replica_versions_to_unelect: vec![], }; - // The anonymous end-user tries to bless a version, bypassing the proposals + // The anonymous end-user tries to elect a version, bypassing the proposals // This should be rejected. let response: Result<(), String> = registry .update_( @@ -80,16 +76,14 @@ fn test_the_anonymous_user_cannot_elect_a_version() { .await; assert_matches!(response, Err(s) if s.contains("is not authorized to call this method: revise_elected_replica_versions")); - // .. And there should therefore be no blessed version + // .. And there should therefore be no elected version assert_eq!( - get_value_or_panic::( + get_value::( ®istry, - make_blessed_replica_versions_key().as_bytes() + make_replica_version_key(replica_version_id).as_bytes() ) .await, - BlessedReplicaVersions { - blessed_version_ids: vec![ReplicaVersion::default().into()] - } + None ); // Go through an upgrade cycle, and verify that it still works the same @@ -104,14 +98,12 @@ fn test_the_anonymous_user_cannot_elect_a_version() { assert_matches!(response, Err(s) if s.contains("is not authorized to call this method: revise_elected_replica_versions")); assert_eq!( - get_value_or_panic::( + get_value::( ®istry, - make_blessed_replica_versions_key().as_bytes() + make_replica_version_key(replica_version_id).as_bytes() ) .await, - BlessedReplicaVersions { - blessed_version_ids: vec![ReplicaVersion::default().into()] - } + None ); Ok(()) @@ -119,7 +111,7 @@ fn test_the_anonymous_user_cannot_elect_a_version() { } #[test] -fn test_a_canister_other_than_the_governance_canister_cannot_bless_a_version() { +fn test_a_canister_other_than_the_governance_canister_cannot_elect_a_version() { state_machine_test_on_nns_subnet(|runtime| async move { // An attacker got a canister that is trying to pass for the governance // canister... @@ -137,14 +129,15 @@ fn test_a_canister_other_than_the_governance_canister_cannot_bless_a_version() { .build(), ) .await; + let replica_version_id = "version_43"; let payload = ReviseElectedGuestosVersionsPayload { - replica_version_to_elect: Some("version_43".into()), + replica_version_to_elect: Some(replica_version_id.into()), release_package_sha256_hex: Some(MOCK_HASH.into()), release_package_urls: vec!["http://release_package.tar.zst".into()], guest_launch_measurements: None, replica_versions_to_unelect: vec![], }; - // The attacker canister tries to bless a version, pretending to be the + // The attacker canister tries to elect a version, pretending to be the // governance canister. This should have no effect. assert!( !forward_call_via_universal_canister( @@ -155,16 +148,14 @@ fn test_a_canister_other_than_the_governance_canister_cannot_bless_a_version() { ) .await ); - // But there should be no blessed version + // But there should be no elected version assert_eq!( - get_value_or_panic::( + get_value::( ®istry, - make_blessed_replica_versions_key().as_bytes() + make_replica_version_key(replica_version_id).as_bytes() ) .await, - BlessedReplicaVersions { - blessed_version_ids: vec![ReplicaVersion::default().into()] - } + None ); Ok(()) }); @@ -188,9 +179,10 @@ fn test_accepted_proposal_mutates_the_registry() { ic_nns_constants::GOVERNANCE_CANISTER_ID ); - // We can bless a new version, the version already in the registry is 42 + // We can elect a new version, the version already in the registry is 42 + let replica_version_id = "version_43"; let payload_v43 = ReviseElectedGuestosVersionsPayload { - replica_version_to_elect: Some("version_43".into()), + replica_version_to_elect: Some(replica_version_id.into()), release_package_sha256_hex: Some(MOCK_HASH.into()), release_package_urls: vec!["http://release_package.tar.zst".into()], guest_launch_measurements: guest_launch_measurements_for_test(), @@ -206,22 +198,18 @@ fn test_accepted_proposal_mutates_the_registry() { .await ); assert_eq!( - get_value_or_panic::( + get_value::( ®istry, - make_blessed_replica_versions_key().as_bytes() + make_replica_version_key(replica_version_id).as_bytes() ) .await, - BlessedReplicaVersions { - blessed_version_ids: vec![ - ReplicaVersion::default().into(), - "version_43".to_string() - ] - } + None ); // Trying to mutate an existing record should have no effect. + let replica_version_id = "version_43"; let payload_v42_mutate = ReviseElectedGuestosVersionsPayload { - replica_version_to_elect: Some("version_43".into()), + replica_version_to_elect: Some(replica_version_id.into()), release_package_sha256_hex: None, release_package_urls: vec![], guest_launch_measurements: guest_launch_measurements_for_test(), @@ -241,7 +229,7 @@ fn test_accepted_proposal_mutates_the_registry() { assert_eq!( get_value_or_panic::( ®istry, - make_replica_version_key("version_43").as_bytes() + make_replica_version_key(replica_version_id).as_bytes() ) .await, ReplicaVersionRecord { @@ -254,8 +242,8 @@ fn test_accepted_proposal_mutates_the_registry() { // Let's now try to upgrade a subnet. // The subnet was added at the beginning of the test - // Set the subnet to a blessed version: it should work - let set_to_blessed_ = DeployGuestosToAllSubnetNodesPayload { + // Set the subnet to an elected version: it should work + let set_to_elected_ = DeployGuestosToAllSubnetNodesPayload { subnet_id: subnet_test_id(999).get(), replica_version_id: ReplicaVersion::default().into(), }; @@ -264,7 +252,7 @@ fn test_accepted_proposal_mutates_the_registry() { &fake_governance_canister, ®istry, "deploy_guestos_to_all_subnet_nodes", - Encode!(&set_to_blessed_).unwrap(), + Encode!(&set_to_elected_).unwrap(), ) .await ); @@ -278,17 +266,17 @@ fn test_accepted_proposal_mutates_the_registry() { ReplicaVersion::default().to_string(), ); - // Try to set the subnet to an unblessed version: it should fail - let try_set_to_unblessed = DeployGuestosToAllSubnetNodesPayload { + // Try to set the subnet to an unelected version: it should fail + let try_set_to_unelected = DeployGuestosToAllSubnetNodesPayload { subnet_id: subnet_test_id(999).get(), - replica_version_id: "unblessed".to_string(), + replica_version_id: "unelected".to_string(), }; assert!( !forward_call_via_universal_canister( &fake_governance_canister, ®istry, "deploy_guestos_to_all_subnet_nodes", - Encode!(&try_set_to_unblessed).unwrap(), + Encode!(&try_set_to_unelected).unwrap(), ) .await ); diff --git a/rs/registry/canister/tests/update_unassigned_nodes_config.rs b/rs/registry/canister/tests/update_unassigned_nodes_config.rs index dff1ff489b3d..65b3268a4f86 100644 --- a/rs/registry/canister/tests/update_unassigned_nodes_config.rs +++ b/rs/registry/canister/tests/update_unassigned_nodes_config.rs @@ -31,7 +31,7 @@ fn test_the_anonymous_user_cannot_update_unassigned_nodes_config() { let payload = UpdateUnassignedNodesConfigPayload { ssh_readonly_access: Some(vec!["some_key".to_string()]), - replica_version: Some("some_unblessed_version".to_string()), + replica_version: Some("some_unelected_version".to_string()), }; // The anonymous end-user tries to update the config, bypassing the proposals @@ -77,7 +77,7 @@ fn test_updating_unassigned_nodes_config_does_not_break_invariants() { let mut payload = UpdateUnassignedNodesConfigPayload { ssh_readonly_access: None, - replica_version: Some("some_unblessed_version".to_string()), + replica_version: Some("some_unelected_version".to_string()), }; assert!( @@ -90,7 +90,7 @@ fn test_updating_unassigned_nodes_config_does_not_break_invariants() { .await ); - // New payload with already-blessed version + // New payload with already-elected version payload = UpdateUnassignedNodesConfigPayload { ssh_readonly_access: None, replica_version: Some(ReplicaVersion::default().into()), diff --git a/rs/state_machine_tests/src/lib.rs b/rs/state_machine_tests/src/lib.rs index 66957fd03b82..8110a633bae3 100644 --- a/rs/state_machine_tests/src/lib.rs +++ b/rs/state_machine_tests/src/lib.rs @@ -81,7 +81,7 @@ use ic_protobuf::{ node::v1::{ConnectionEndpoint, NodeRecord}, node_rewards::v2::NodeRewardsTable, provisional_whitelist::v1::ProvisionalWhitelist as PbProvisionalWhitelist, - replica_version::v1::{BlessedReplicaVersions, ReplicaVersionRecord}, + replica_version::v1::ReplicaVersionRecord, routing_table::v1::{ CanisterMigrations as PbCanisterMigrations, RoutingTable as PbRoutingTable, }, @@ -99,11 +99,10 @@ use ic_registry_client_helpers::{ subnet::{SubnetListRegistry, SubnetRegistry}, }; use ic_registry_keys::{ - NODE_REWARDS_TABLE_KEY, ROOT_SUBNET_ID_KEY, make_blessed_replica_versions_key, - make_canister_migrations_record_key, make_canister_ranges_key, - make_catch_up_package_contents_key, make_chain_key_enabled_subnet_list_key, - make_crypto_node_key, make_crypto_tls_cert_key, make_node_record_key, - make_provisional_whitelist_record_key, make_replica_version_key, + NODE_REWARDS_TABLE_KEY, ROOT_SUBNET_ID_KEY, make_canister_migrations_record_key, + make_canister_ranges_key, make_catch_up_package_contents_key, + make_chain_key_enabled_subnet_list_key, make_crypto_node_key, make_crypto_tls_cert_key, + make_node_record_key, make_provisional_whitelist_record_key, make_replica_version_key, }; use ic_registry_proto_data_provider::{INITIAL_REGISTRY_VERSION, ProtoRegistryDataProvider}; use ic_registry_provisional_whitelist::ProvisionalWhitelist; @@ -316,7 +315,6 @@ pub fn add_global_registry_records( /// Adds initial registry records to the registry managed by the registry data provider: /// - provisional whitelist record; -/// - blessed replica versions record; /// - replica version record. pub fn add_initial_registry_records(registry_data_provider: Arc) { let registry_version = INITIAL_REGISTRY_VERSION; @@ -331,20 +329,8 @@ pub fn add_initial_registry_records(registry_data_provider: Arc