Skip to content

feat: Move a few tests away from the blessed versions list #9729

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.

Sign up for GitHub

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

Draft
Bownairo wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

feat: Move a few tests away from the blessed versions list #9729

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f2da381
Update state machine tests
Bownairo Mar 30, 2026
75e27b5
Update integration tests
Bownairo Mar 30, 2026
3034661
Update registry canister tests
Bownairo Mar 30, 2026
d2a7361
Clippy
Bownairo Apr 4, 2026
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
2 changes: 1 addition & 1 deletion rs/nns/integration_tests/src/update_unassigned_nodes_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
77 changes: 38 additions & 39 deletions rs/nns/integration_tests/src/upgrades_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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::<ReplicaVersionRecord>(
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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -105,21 +114,18 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() {
);
}

assert_eq!(
get_value_or_panic::<BlessedReplicaVersions>(
&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 {
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -198,20 +204,13 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() {
}
}

assert_eq!(
get_value_or_panic::<BlessedReplicaVersions>(
&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;
Expand Down
86 changes: 37 additions & 49 deletions rs/registry/canister/tests/update_subnet_and_bless_replica_version.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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_(
Expand All @@ -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::<BlessedReplicaVersions>(
get_value::<ReplicaVersionRecord>(
&registry,
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
Expand All @@ -104,22 +98,20 @@ 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::<BlessedReplicaVersions>(
get_value::<ReplicaVersionRecord>(
&registry,
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(())
});
}

#[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...
Expand All @@ -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(
Expand All @@ -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::<BlessedReplicaVersions>(
get_value::<ReplicaVersionRecord>(
&registry,
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(())
});
Expand All @@ -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(),
Expand All @@ -206,22 +198,18 @@ fn test_accepted_proposal_mutates_the_registry() {
.await
);
assert_eq!(
get_value_or_panic::<BlessedReplicaVersions>(
get_value::<ReplicaVersionRecord>(
&registry,
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(),
Expand All @@ -241,7 +229,7 @@ fn test_accepted_proposal_mutates_the_registry() {
assert_eq!(
get_value_or_panic::<ReplicaVersionRecord>(
&registry,
make_replica_version_key("version_43").as_bytes()
make_replica_version_key(replica_version_id).as_bytes()
)
.await,
ReplicaVersionRecord {
Expand All @@ -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(),
};
Expand All @@ -264,7 +252,7 @@ fn test_accepted_proposal_mutates_the_registry() {
&fake_governance_canister,
&registry,
"deploy_guestos_to_all_subnet_nodes",
Encode!(&set_to_blessed_).unwrap(),
Encode!(&set_to_elected_).unwrap(),
)
.await
);
Expand All @@ -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,
&registry,
"deploy_guestos_to_all_subnet_nodes",
Encode!(&try_set_to_unblessed).unwrap(),
Encode!(&try_set_to_unelected).unwrap(),
)
.await
);
Expand Down
6 changes: 3 additions & 3 deletions rs/registry/canister/tests/update_unassigned_nodes_config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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!(
Expand All @@ -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()),
Expand Down
Loading
Loading