diff --git a/Cargo.lock b/Cargo.lock index 9162869b2275..c946fb3fa4b4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6194,6 +6194,7 @@ dependencies = [ "ic-registry-local-store", "ic-registry-replicator", "ic-test-utilities-tmpdir", + "ic-test-utilities-types", "ic-types", "rand 0.8.6", "reqwest", @@ -6403,6 +6404,7 @@ dependencies = [ "ic-registry-proto-data-provider", "ic-registry-routing-table", "ic-registry-subnet-type", + "ic-test-utilities-types", "ic-types", "indoc", "ipnet", @@ -9119,6 +9121,7 @@ dependencies = [ "ic-registry-keys", "ic-registry-subnet-type", "ic-registry-transport", + "ic-test-utilities-types", "ic-types", "pocket-ic", "prost", @@ -9728,6 +9731,7 @@ dependencies = [ "ic-metrics", "ic-types", "ic-types-cycles", + "ic-types-test-utils", "prometheus", "slog", ] @@ -12010,6 +12014,7 @@ dependencies = [ "ic-state-machine-tests", "ic-test-utilities", "ic-test-utilities-metrics", + "ic-test-utilities-types", "ic-types", "ic-types-cycles", "ic-types-test-utils", @@ -12335,6 +12340,7 @@ dependencies = [ "ic-registry-subnet-type", "ic-registry-transport", "ic-state-manager", + "ic-test-utilities-types", "ic-types", "json5", "maplit", @@ -12845,6 +12851,7 @@ dependencies = [ "ic-registry-routing-table", "ic-registry-transport", "ic-test-utilities-logger", + "ic-test-utilities-types", "ic-types", "ic-types-test-utils", "pocket-ic", @@ -14793,6 +14800,7 @@ dependencies = [ "ic-registry-resource-limits", "ic-registry-subnet-features", "ic-registry-subnet-type", + "ic-test-utilities-types", "ic-types", "ic-types-cycles", "tempfile", @@ -15075,7 +15083,6 @@ dependencies = [ "rand 0.8.6", "rand_chacha 0.3.1", "rstest", - "rusty-fork", "serde", "serde_bytes", "serde_cbor", @@ -19437,6 +19444,7 @@ dependencies = [ "ic-registry-subnet-type", "ic-registry-transport", "ic-sns-wasm", + "ic-test-utilities-types", "ic-transport-types", "ic-types", "ic-universal-canister", diff --git a/packages/pocket-ic/BUILD.bazel b/packages/pocket-ic/BUILD.bazel index 383384d6254d..7babd099627a 100644 --- a/packages/pocket-ic/BUILD.bazel +++ b/packages/pocket-ic/BUILD.bazel @@ -147,6 +147,7 @@ rust_test( "//rs/registry/helpers", "//rs/registry/proto_data_provider", "//rs/registry/transport", + "//rs/test_utilities/types", "//rs/types/base_types", "//rs/types/types", "//rs/universal_canister/lib", diff --git a/packages/pocket-ic/Cargo.toml b/packages/pocket-ic/Cargo.toml index ef455179503f..4ad1deacaea9 100644 --- a/packages/pocket-ic/Cargo.toml +++ b/packages/pocket-ic/Cargo.toml @@ -83,6 +83,7 @@ ic-registry-client = { path = "../../rs/registry/client" } ic-registry-client-helpers = { path = "../../rs/registry/helpers" } ic-registry-proto-data-provider = { path = "../../rs/registry/proto_data_provider" } ic-registry-transport = { path = "../../rs/registry/transport" } +ic-test-utilities-types = { path = "../../rs/test_utilities/types" } ic-types = { path = "../../rs/types/types" } ic-universal-canister = { path = "../../rs/universal_canister/lib" } maplit = { workspace = true } diff --git a/packages/pocket-ic/tests/unix.rs b/packages/pocket-ic/tests/unix.rs index b7989f9e544c..fe14b68f2fdc 100644 --- a/packages/pocket-ic/tests/unix.rs +++ b/packages/pocket-ic/tests/unix.rs @@ -15,7 +15,7 @@ use ic_registry_client_helpers::node_operator::NodeOperatorRegistry; use ic_registry_client_helpers::subnet::SubnetRegistry; use ic_registry_proto_data_provider::ProtoRegistryDataProvider; use ic_registry_transport::pb::v1::RegistryGetLatestVersionResponse; -use ic_types::ReplicaVersion; +use ic_test_utilities_types::ids::test_replica_version; use ic_universal_canister::{UNIVERSAL_CANISTER_WASM, wasm}; use maplit::btreemap; use pocket_ic::common::rest::{ @@ -753,7 +753,7 @@ fn create_subnet_in_registry_canister() { for node_id in [node_id_1, node_id_2] { let create_subnet_payload = CreateSubnetPayload { node_ids: vec![NodeId::new(PrincipalId(node_id))], - replica_version_id: ReplicaVersion::default().to_string(), + replica_version_id: test_replica_version().to_string(), ..Default::default() }; update_candid_as::<_, (Result,)>( diff --git a/rs/artifact_pool/benches/load_blocks.rs b/rs/artifact_pool/benches/load_blocks.rs index a43e9eb94e6a..bb5ca3fb3f25 100644 --- a/rs/artifact_pool/benches/load_blocks.rs +++ b/rs/artifact_pool/benches/load_blocks.rs @@ -13,7 +13,7 @@ use ic_interfaces::time_source::SysTimeSource; use ic_logger::replica_logger::no_op_logger; use ic_test_utilities_consensus::{fake::*, make_genesis}; use ic_test_utilities_types::{ - ids::{node_test_id, subnet_test_id}, + ids::{node_test_id, subnet_test_id, test_replica_version}, messages::SignedIngressBuilder, }; use ic_types::consensus::dkg::{DkgDataPayload, DkgSummary}; @@ -36,6 +36,7 @@ where let mut consensus_pool = ConsensusPoolImpl::new( node_test_id(0), subnet_test_id(0), + &test_replica_version(), make_genesis(DkgSummary::fake()).into(), pool_config, ic_metrics::MetricsRegistry::new(), @@ -158,6 +159,7 @@ fn add_validated_block(criterion: &mut Criterion) { let mut pool = ConsensusPoolImpl::new( node_test_id(0), subnet_test_id(0), + &test_replica_version(), make_genesis(DkgSummary::fake()).into(), pool_config.clone(), ic_metrics::MetricsRegistry::new(), diff --git a/rs/artifact_pool/src/backup.rs b/rs/artifact_pool/src/backup.rs index 2e8328828494..675f47212a9c 100644 --- a/rs/artifact_pool/src/backup.rs +++ b/rs/artifact_pool/src/backup.rs @@ -632,7 +632,7 @@ impl BackupArtifact { mod tests { use super::*; use ic_test_utilities_consensus::fake::*; - use ic_test_utilities_types::ids::node_test_id; + use ic_test_utilities_types::ids::{node_test_id, test_replica_version}; use ic_types::{ RegistryVersion, batch::*, @@ -643,7 +643,10 @@ mod tests { #[test] fn test_random_tape_conversion() { - let artifact = RandomTape::fake(RandomTapeContent::new(Height::from(22))); + let artifact = RandomTape::fake(RandomTapeContent::new( + Height::from(22), + test_replica_version(), + )); let mut buf = Vec::new(); pb::RandomTape::from(artifact.clone()) .encode(&mut buf) @@ -659,6 +662,7 @@ mod tests { let artifact = Finalization::fake(FinalizationContent::new( Height::from(22), CryptoHashOf::from(CryptoHash(vec![1, 2, 3])), + test_replica_version(), )); let mut buf = Vec::new(); pb::Finalization::from(artifact.clone()) @@ -675,6 +679,7 @@ mod tests { let artifact = Notarization::fake(NotarizationContent::new( Height::from(22), CryptoHashOf::from(CryptoHash(vec![1, 2, 3])), + test_replica_version(), )); let mut buf = Vec::new(); pb::Notarization::from(artifact.clone()) @@ -702,6 +707,7 @@ mod tests { certified_height: Height::from(42), time: UNIX_EPOCH, }, + test_replica_version(), ), node_test_id(333), ); diff --git a/rs/artifact_pool/src/canister_http_pool.rs b/rs/artifact_pool/src/canister_http_pool.rs index bb1eec4fb4e2..9afec11ef9a5 100644 --- a/rs/artifact_pool/src/canister_http_pool.rs +++ b/rs/artifact_pool/src/canister_http_pool.rs @@ -269,7 +269,7 @@ mod tests { use ic_test_utilities_consensus::fake::FakeSigner; use ic_test_utilities_types::ids::node_test_id; use ic_types::{ - CanisterId, ReplicaVersion, + CanisterId, artifact::IdentifiableArtifact, canister_http::{ CanisterHttpPaymentReceipt, CanisterHttpResponseContent, CanisterHttpResponseMetadata, @@ -305,7 +305,7 @@ mod tests { content_hash: CryptoHashOf::from(CryptoHash(vec![1, 2, 3])), content_size: 42, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: ic_test_utilities_types::ids::test_replica_version(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }, @@ -330,7 +330,7 @@ mod tests { content_hash: ic_types::crypto::crypto_hash(response), content_size: 42, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: ic_test_utilities_types::ids::test_replica_version(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }, diff --git a/rs/artifact_pool/src/consensus_pool.rs b/rs/artifact_pool/src/consensus_pool.rs index f7301ae0b37a..a29ef7292180 100644 --- a/rs/artifact_pool/src/consensus_pool.rs +++ b/rs/artifact_pool/src/consensus_pool.rs @@ -22,9 +22,9 @@ use ic_interfaces::{ use ic_logger::{ReplicaLogger, warn}; use ic_metrics::buckets::linear_buckets; use ic_protobuf::types::v1 as pb; -use ic_types::NodeId; use ic_types::crypto::CryptoHashOf; use ic_types::{Height, SubnetId, Time, artifact::ConsensusMessageId, consensus::*}; +use ic_types::{NodeId, ReplicaVersion}; use prometheus::{Histogram, IntCounter, IntGauge, histogram_opts, labels, opts}; use std::time::Instant; use std::{marker::PhantomData, sync::Arc, time::Duration}; @@ -419,6 +419,7 @@ impl ConsensusPoolImpl { pub fn new( node_id: NodeId, subnet_id: SubnetId, + replica_version: &ReplicaVersion, cup_proto: pb::CatchUpPackage, config: ArtifactPoolConfig, registry: ic_metrics::MetricsRegistry, @@ -443,7 +444,7 @@ impl ConsensusPoolImpl { config .spool_path .join(subnet_id.to_string()) - .join(ic_types::ReplicaVersion::default().to_string()), + .join(replica_version.to_string()), Duration::from_secs(config.retention_time_secs), Duration::from_secs(config.purging_interval_secs), registry, @@ -1056,9 +1057,9 @@ mod tests { use ic_test_utilities_consensus::{fake::*, make_genesis}; use ic_test_utilities_registry::{SubnetRecordBuilder, setup_registry}; use ic_test_utilities_time::FastForwardTimeSource; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; + use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, test_replica_version}; use ic_types::{ - RegistryVersion, ReplicaVersion, + RegistryVersion, artifact::IdentifiableArtifact, batch::ValidationContext, consensus::{BlockProposal, RandomBeacon, dkg::DkgSummary}, @@ -1080,6 +1081,7 @@ mod tests { ConsensusPoolImpl::new( node_id, subnet_id, + &test_replica_version(), catch_up_package.into(), config, registry, @@ -1102,6 +1104,7 @@ mod tests { certified_height: Height::from(42), time: UNIX_EPOCH, }, + test_replica_version(), ) } @@ -1123,6 +1126,7 @@ mod tests { let mut random_beacon = RandomBeacon::fake(RandomBeaconContent::new( Height::from(0), CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), )); let msg_0 = random_beacon.clone().into_message(); let msg_id_0 = random_beacon.get_id(); @@ -1182,18 +1186,21 @@ mod tests { let random_beacon_1 = RandomBeacon::fake(RandomBeaconContent::new( Height::from(1), CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), )) .into_message(); let random_beacon_2 = RandomBeacon::fake(RandomBeaconContent::new( Height::from(2), CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), )) .into_message(); let random_beacon_3 = RandomBeacon::fake(RandomBeaconContent::new( Height::from(3), CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), )) .into_message(); @@ -1264,6 +1271,7 @@ mod tests { let random_beacon = RandomBeacon::fake(RandomBeaconContent::new( Height::from(1), CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), )); let random_beacon_share_1 = @@ -1336,6 +1344,7 @@ mod tests { let random_beacon = RandomBeacon::fake(RandomBeaconContent::new( Height::from(1), CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), )); let id = random_beacon.get_id(); @@ -1376,7 +1385,7 @@ mod tests { let fake_finalization = |height: Height| { Finalization::fake(FinalizationContent { - version: ReplicaVersion::default(), + version: test_replica_version(), height, block: crypto_hash(&fake_block(height, Rank(0))), }) @@ -1389,7 +1398,7 @@ mod tests { let fake_notarization = |height: Height| { Notarization::fake(NotarizationContent { - version: ReplicaVersion::default(), + version: test_replica_version(), height, block: crypto_hash(&fake_block(height, Rank(0))), }) @@ -1402,7 +1411,7 @@ mod tests { let fake_beacon = |height: Height| { RandomBeacon::fake(RandomBeaconContent { - version: ReplicaVersion::default(), + version: test_replica_version(), height, parent: CryptoHashOf::from(CryptoHash(vec![])), }) @@ -1419,7 +1428,7 @@ mod tests { let fake_tape = |height: Height| { RandomTape::fake(RandomTapeContent { - version: ReplicaVersion::default(), + version: test_replica_version(), height, }) .into_message() @@ -1461,7 +1470,7 @@ mod tests { HashedRandomBeacon::new( crypto_hash, RandomBeacon::fake(RandomBeaconContent { - version: ReplicaVersion::default(), + version: test_replica_version(), height: Height::from(height_offset), parent: CryptoHashOf::from(CryptoHash(vec![])), }), @@ -1538,6 +1547,7 @@ mod tests { msg: Notarization::fake(NotarizationContent::new( block.height(), ic_types::crypto::crypto_hash(block), + test_replica_version(), )) .into_message(), timestamp: time_source.get_relative_time(), @@ -1550,6 +1560,7 @@ mod tests { msg: Finalization::fake(FinalizationContent::new( block.height(), ic_types::crypto::crypto_hash(block), + test_replica_version(), )) .into_message(), timestamp: time_source.get_relative_time(), @@ -1640,7 +1651,7 @@ mod tests { let root_path = backup_dir .path() .join(subnet_id.to_string()) - .join(ic_types::ReplicaVersion::default().to_string()); + .join(test_replica_version().to_string()); let mut pool = new_from_cup_without_bytes( node_test_id(0), subnet_id, @@ -1672,15 +1683,21 @@ mod tests { let random_beacon = RandomBeacon::fake(RandomBeaconContent::new( Height::from(1), CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), + )); + let random_tape = RandomTape::fake(RandomTapeContent::new( + Height::from(2), + test_replica_version(), )); - let random_tape = RandomTape::fake(RandomTapeContent::new(Height::from(2))); let notarization = Notarization::fake(NotarizationContent::new( Height::from(2), CryptoHashOf::from(CryptoHash(vec![1, 2, 3])), + test_replica_version(), )); let finalization = Finalization::fake(FinalizationContent::new( Height::from(3), CryptoHashOf::from(CryptoHash(vec![1, 2, 3])), + test_replica_version(), )); // height 3, non-final @@ -1697,6 +1714,7 @@ mod tests { certified_height: Height::from(41), time: UNIX_EPOCH, }, + test_replica_version(), ); let proposal3 = BlockProposal::fake(block, node_test_id(333)); @@ -1714,11 +1732,13 @@ mod tests { certified_height: Height::from(42), time: UNIX_EPOCH, }, + test_replica_version(), ); let proposal3_final = BlockProposal::fake(block.clone(), node_test_id(333)); let notarization3 = Notarization::fake(NotarizationContent::new( Height::from(3), ic_types::crypto::crypto_hash(&block), + test_replica_version(), )); let block = Block::new( @@ -1734,10 +1754,12 @@ mod tests { certified_height: Height::from(42), time: UNIX_EPOCH, }, + test_replica_version(), ); let finalization_at_4 = Finalization::fake(FinalizationContent::new( Height::from(4), ic_types::crypto::crypto_hash(&block), + test_replica_version(), )); let proposal = BlockProposal::fake(block, node_test_id(333)); @@ -1755,6 +1777,7 @@ mod tests { certified_height: Height::from(41), time: UNIX_EPOCH, }, + test_replica_version(), ); let proposal_non_final = BlockProposal::fake(block, node_test_id(333)); @@ -1765,6 +1788,7 @@ mod tests { RandomBeacon::fake(RandomBeaconContent::new( Height::from(4), CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), )), ); @@ -2042,12 +2066,20 @@ mod tests { let random_beacon = RandomBeacon::fake(RandomBeaconContent::new( Height::from(1), CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), + )); + let random_tape = RandomTape::fake(RandomTapeContent::new( + Height::from(2), + test_replica_version(), + )); + let random_tape3 = RandomTape::fake(RandomTapeContent::new( + Height::from(3), + test_replica_version(), )); - let random_tape = RandomTape::fake(RandomTapeContent::new(Height::from(2))); - let random_tape3 = RandomTape::fake(RandomTapeContent::new(Height::from(3))); let notarization = Notarization::fake(NotarizationContent::new( Height::from(3), CryptoHashOf::from(CryptoHash(vec![1, 2, 3])), + test_replica_version(), )); let block = Block::new( CryptoHashOf::from(CryptoHash(Vec::new())), @@ -2062,10 +2094,12 @@ mod tests { certified_height: Height::from(42), time: UNIX_EPOCH, }, + test_replica_version(), ); let finalization = Finalization::fake(FinalizationContent::new( Height::from(4), ic_types::crypto::crypto_hash(&block), + test_replica_version(), )); let proposal = BlockProposal::fake(block, node_test_id(333)); @@ -2208,6 +2242,7 @@ mod tests { let mut pool = TestConsensusPool::new( node_test_id(0), subnet_id, + test_replica_version(), pool_config, time_source, registry, @@ -2277,7 +2312,7 @@ mod tests { HashedRandomBeacon::new( crypto_hash, RandomBeacon::fake(RandomBeaconContent { - version: ReplicaVersion::default(), + version: test_replica_version(), height, parent: CryptoHashOf::from(CryptoHash(vec![])), }), @@ -2286,13 +2321,14 @@ mod tests { None, )); let notarization = Notarization::fake(NotarizationContent { - version: ReplicaVersion::default(), + version: test_replica_version(), height, block: crypto_hash(&fake_block(height, Rank(0))), }); let random_beacon = RandomBeacon::fake(RandomBeaconContent::new( height, CryptoHashOf::from(CryptoHash(Vec::new())), + test_replica_version(), )); let cup_id = cup.get_id(); let notarization_id = notarization.get_id(); diff --git a/rs/artifact_pool/src/consensus_pool_cache.rs b/rs/artifact_pool/src/consensus_pool_cache.rs index fbcc91782af6..8c3e2a234c51 100644 --- a/rs/artifact_pool/src/consensus_pool_cache.rs +++ b/rs/artifact_pool/src/consensus_pool_cache.rs @@ -512,7 +512,7 @@ mod test { use ic_test_utilities_consensus::fake::*; use ic_test_utilities_registry::{SubnetRecordBuilder, setup_registry}; use ic_test_utilities_time::FastForwardTimeSource; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; + use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, test_replica_version}; use ic_types::consensus::*; use ic_types::crypto::crypto_hash; use ic_types::time::UNIX_EPOCH; @@ -539,6 +539,7 @@ mod test { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let time_source = FastForwardTimeSource::new(); let subnet_id = subnet_test_id(1); + let replica_version = test_replica_version(); let committee = vec![node_test_id(0)]; let dkg_interval_length = 3; let subnet_records = vec![( @@ -553,6 +554,7 @@ mod test { let mut pool = TestConsensusPool::new( node_test_id(0), subnet_id, + replica_version.clone(), pool_config, time_source, registry, @@ -579,6 +581,7 @@ mod test { let finalization = Finalization::fake(FinalizationContent::new( block.height(), block.content.get_hash().clone(), + replica_version, )); // 2. Cache can be updated by finalization @@ -649,6 +652,7 @@ mod test { let mut pool = TestConsensusPool::new( node_test_id(0), subnet_test_id(1), + test_replica_version(), pool_config, FastForwardTimeSource::new(), setup_registry(subnet_test_id(1), subnet_records), @@ -723,6 +727,7 @@ mod test { let mut pool = TestConsensusPool::new( node_test_id(0), subnet_test_id(1), + test_replica_version(), pool_config, FastForwardTimeSource::new(), setup_registry(subnet_test_id(1), subnet_records), diff --git a/rs/artifact_pool/src/dkg_pool.rs b/rs/artifact_pool/src/dkg_pool.rs index 74f3a1416bb0..ed39f4a1ddbd 100644 --- a/rs/artifact_pool/src/dkg_pool.rs +++ b/rs/artifact_pool/src/dkg_pool.rs @@ -202,7 +202,7 @@ mod test { use ic_interfaces::dkg::DkgPool; use ic_logger::replica_logger::no_op_logger; use ic_test_utilities_consensus::fake::FakeSigner; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; + use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, test_replica_version}; use ic_types::{ NodeId, crypto::threshold_sig::ni_dkg::{NiDkgId, NiDkgTag, NiDkgTargetSubnet}, @@ -218,7 +218,7 @@ mod test { target_subnet: NiDkgTargetSubnet::Local, }; dkg::Message { - content: dkg::DealingContent::new(dummy_dealing(0), dkg_id), + content: dkg::DealingContent::new(dummy_dealing(0), dkg_id, test_replica_version()), signature: BasicSignature::fake(node_id), } } diff --git a/rs/artifact_pool/src/lib.rs b/rs/artifact_pool/src/lib.rs index 5388ebb0e794..cec57a3ab2fe 100644 --- a/rs/artifact_pool/src/lib.rs +++ b/rs/artifact_pool/src/lib.rs @@ -43,10 +43,13 @@ pub fn set_replica_version>(filepath: P, replica_version: &Replic /// Check that the replica version of the pool matches that of this process. If /// it does not, delete the contents of the old pool directory and create a new /// one. -pub fn ensure_persistent_pool_replica_version_compatibility(pool_path: PathBuf) { +pub fn ensure_persistent_pool_replica_version_compatibility( + pool_path: PathBuf, + replica_version: &ReplicaVersion, +) { let mut replica_version_file_path = pool_path.clone(); replica_version_file_path.push("replica_version"); - if get_replica_version(&replica_version_file_path) != Some(ReplicaVersion::default()) { + if get_replica_version(&replica_version_file_path).as_ref() != Some(replica_version) { if pool_path.exists() { for entry in fs::read_dir(&pool_path).expect("Couldn't read the directory") { let path = entry.expect("Couldn't read the metadata").path(); @@ -58,7 +61,7 @@ pub fn ensure_persistent_pool_replica_version_compatibility(pool_path: PathBuf) } } std::fs::create_dir_all(&pool_path).expect("Couldn't create a directory"); - set_replica_version(replica_version_file_path, &ReplicaVersion::default()); + set_replica_version(replica_version_file_path, replica_version); } } @@ -99,36 +102,46 @@ impl IntoInner for UnvalidatedArtifact { #[cfg(test)] mod tests { use super::*; - use std::convert::TryFrom; + use std::str::FromStr; #[test] fn test_ensure_persistent_pool_replica_version_compatibility() { + let replica_version = ReplicaVersion::from_str("12344556").unwrap(); ic_test_utilities::artifact_pool_config::with_test_pool_config(|config| { - ensure_persistent_pool_replica_version_compatibility(config.persistent_pool_db_path()); + ensure_persistent_pool_replica_version_compatibility( + config.persistent_pool_db_path(), + &replica_version, + ); let mut replica_version_file_path = config.persistent_pool_db_path(); replica_version_file_path.push("replica_version"); // Ensure that a file was added indicating which replica version the // directory was made with. assert_eq!( - ReplicaVersion::default(), + replica_version, get_replica_version(&replica_version_file_path).unwrap() ); let mut random_file_path = config.persistent_pool_db_path(); random_file_path.push("random_file"); std::fs::write(&random_file_path, "stuff").unwrap(); - ensure_persistent_pool_replica_version_compatibility(config.persistent_pool_db_path()); + ensure_persistent_pool_replica_version_compatibility( + config.persistent_pool_db_path(), + &replica_version, + ); // Ensure that the directory was not deleted by checking for the file. assert_eq!(std::fs::read_to_string(&random_file_path).unwrap(), "stuff"); set_replica_version( &replica_version_file_path, - &ReplicaVersion::try_from("somerandomversion").unwrap(), + &ReplicaVersion::from_str("somerandomversion").unwrap(), ); - ensure_persistent_pool_replica_version_compatibility(config.persistent_pool_db_path()); + ensure_persistent_pool_replica_version_compatibility( + config.persistent_pool_db_path(), + &replica_version, + ); // Now that the folder has a different replica version it should // have been deleted and created with the new replica version. @@ -141,7 +154,7 @@ mod tests { } assert_eq!( - ReplicaVersion::default(), + replica_version, get_replica_version(replica_version_file_path).unwrap() ); }) diff --git a/rs/artifact_pool/src/test_utils.rs b/rs/artifact_pool/src/test_utils.rs index b3eb1badcd46..f954e0a314e7 100644 --- a/rs/artifact_pool/src/test_utils.rs +++ b/rs/artifact_pool/src/test_utils.rs @@ -13,7 +13,7 @@ use ic_interfaces::consensus_pool::{ }; use ic_logger::ReplicaLogger; use ic_test_utilities_consensus::{fake::*, make_genesis}; -use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; +use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, test_replica_version}; use ic_types::{ Height, artifact::ConsensusMessageId, @@ -545,7 +545,7 @@ fn finalization_ops() -> PoolSectionOps { let height = Height::from(i); let block_proposal = fake_block_proposal(height); let block = block_proposal.content.get_hash().clone(); - let content = FinalizationContent::new(height, block); + let content = FinalizationContent::new(height, block, test_replica_version()); let signature = MultiSignature::fake(); let msg = ConsensusMessage::Finalization(Finalization { content, signature }); ops.insert(ValidatedConsensusArtifact { @@ -562,7 +562,7 @@ fn notarization_ops() -> PoolSectionOps { let height = Height::from(i); let block_proposal = fake_block_proposal(height); let block = block_proposal.content.get_hash().clone(); - let content = NotarizationContent::new(height, block); + let content = NotarizationContent::new(height, block, test_replica_version()); let signature = MultiSignature::fake(); let msg = ConsensusMessage::Notarization(Notarization { content, signature }); ops.insert(ValidatedConsensusArtifact { @@ -580,7 +580,7 @@ fn random_beacon_share_ops() -> PoolSectionOps { for j in 0..3 { let random_beacon = fake_random_beacon(Height::from(i)); let parent = ic_types::crypto::crypto_hash(&random_beacon); - let content = RandomBeaconContent::new(height, parent); + let content = RandomBeaconContent::new(height, parent, random_beacon.content.version); let signature = ThresholdSigShareOf::new(ThresholdSigShare(vec![])); let signer = node_test_id(j); let signature = ThresholdSignatureShare { signature, signer }; @@ -601,7 +601,7 @@ pub(crate) fn notarization_share_ops() -> PoolSectionOps PoolSectionOps PoolSectionOps PoolSectionOps { let mut ops = PoolSectionOps::new(); for i in 3..19 { - let random_tape = RandomTape::fake(RandomTapeContent::new(Height::from(i))); + let random_tape = RandomTape::fake(RandomTapeContent::new( + Height::from(i), + test_replica_version(), + )); let msg = ConsensusMessage::RandomTape(random_tape); ops.insert(ValidatedConsensusArtifact { msg, @@ -650,7 +653,7 @@ fn random_tape_share_ops() -> PoolSectionOps { for i in 5..20 { let height = Height::from(i); for j in 0..3 { - let content = RandomTapeContent::new(height); + let content = RandomTapeContent::new(height, test_replica_version()); let signature = ThresholdSigShareOf::new(ThresholdSigShare(vec![])); let signer = node_test_id(j); let signature = ThresholdSignatureShare { signature, signer }; diff --git a/rs/backup/BUILD.bazel b/rs/backup/BUILD.bazel index a7ce3edfb6c3..8f4fc0198b5f 100644 --- a/rs/backup/BUILD.bazel +++ b/rs/backup/BUILD.bazel @@ -84,6 +84,7 @@ rust_test( "//rs/registry/helpers", "//rs/registry/local_store", "//rs/test_utilities/tmpdir", + "//rs/test_utilities/types", "//rs/types/types", "@crate_index//:anyhow", "@crate_index//:chrono", diff --git a/rs/backup/Cargo.toml b/rs/backup/Cargo.toml index 8b3f4de3a662..9117cad91be3 100644 --- a/rs/backup/Cargo.toml +++ b/rs/backup/Cargo.toml @@ -33,6 +33,7 @@ url = { workspace = true } ic-registry-client = { path = "../registry/client" } ic-registry-local-store = { path = "../registry/local_store" } ic-test-utilities-tmpdir = { path = "../test_utilities/tmpdir" } +ic-test-utilities-types = { path = "../test_utilities/types" } rstest = { workspace = true } [[bin]] diff --git a/rs/backup/src/backup_helper.rs b/rs/backup/src/backup_helper.rs index e171595740db..1bf91968c51f 100644 --- a/rs/backup/src/backup_helper.rs +++ b/rs/backup/src/backup_helper.rs @@ -1127,6 +1127,7 @@ mod tests { use ic_registry_client::client::RegistryClientImpl; use ic_registry_local_store::LocalStoreImpl; use ic_test_utilities_tmpdir::tmpdir; + use ic_test_utilities_types::ids::test_replica_version; use ic_types::PrincipalId; use rstest::rstest; @@ -1455,7 +1456,7 @@ mod tests { subnet_id: PrincipalId::from_str(FAKE_SUBNET_ID) .map(SubnetId::from) .unwrap(), - initial_replica_version: ReplicaVersion::try_from("fake_replica_version").unwrap(), + initial_replica_version: test_replica_version(), root_dir: temp_dir.join("backup"), excluded_dirs: vec![], ssh_private_key: "fake_ssh_private_key".into(), diff --git a/rs/boundary_node/ic_boundary/BUILD.bazel b/rs/boundary_node/ic_boundary/BUILD.bazel index 4dd595ad6703..92790635f64c 100644 --- a/rs/boundary_node/ic_boundary/BUILD.bazel +++ b/rs/boundary_node/ic_boundary/BUILD.bazel @@ -136,6 +136,7 @@ rust_test( "//rs/registry/proto_data_provider", "//rs/registry/routing_table", "//rs/registry/subnet_type", + "//rs/test_utilities/types", "//rs/types/base_types", "//rs/types/types", "@crate_index//:anyhow", diff --git a/rs/boundary_node/ic_boundary/Cargo.toml b/rs/boundary_node/ic_boundary/Cargo.toml index df806a895184..8c01cc8ffd55 100644 --- a/rs/boundary_node/ic_boundary/Cargo.toml +++ b/rs/boundary_node/ic_boundary/Cargo.toml @@ -87,6 +87,7 @@ url = { workspace = true } x509-parser = { workspace = true } [dev-dependencies] +ic-test-utilities-types = { path = "../../test_utilities/types" } indoc = { workspace = true } tempfile = { workspace = true } tokio-tungstenite = "0.29" diff --git a/rs/boundary_node/ic_boundary/src/test_utils.rs b/rs/boundary_node/ic_boundary/src/test_utils.rs index 0ab977a30a00..b4a5b2450f03 100644 --- a/rs/boundary_node/ic_boundary/src/test_utils.rs +++ b/rs/boundary_node/ic_boundary/src/test_utils.rs @@ -28,9 +28,9 @@ use ic_registry_keys::{ use ic_registry_proto_data_provider::ProtoRegistryDataProvider; use ic_registry_routing_table::{CanisterIdRange, RoutingTable as RoutingTableIC}; use ic_registry_subnet_type::SubnetType; +use ic_test_utilities_types::ids::test_replica_version; use ic_types::{ - CanisterId, RegistryVersion, SubnetId, crypto::threshold_sig::ThresholdSigPublicKey, - replica_version::ReplicaVersion, time::Time, + CanisterId, RegistryVersion, SubnetId, crypto::threshold_sig::ThresholdSigPublicKey, time::Time, }; use crate::routes::ProxyRouter; @@ -111,7 +111,7 @@ pub fn test_subnet_record() -> SubnetRecord { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: INITIAL_NOTARY_DELAY.as_millis() as u64, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 59, dkg_dealings_per_block: 1, start_as_nns: false, diff --git a/rs/consensus/benches/validate_payload.rs b/rs/consensus/benches/validate_payload.rs index da97149a3cc9..643983ab112d 100644 --- a/rs/consensus/benches/validate_payload.rs +++ b/rs/consensus/benches/validate_payload.rs @@ -49,7 +49,7 @@ use ic_test_utilities_registry::{SubnetRecordBuilder, setup_registry}; use ic_test_utilities_state::ReplicatedStateBuilder; use ic_test_utilities_time::FastForwardTimeSource; use ic_test_utilities_types::{ - ids::{canister_test_id, node_test_id, subnet_test_id}, + ids::{canister_test_id, node_test_id, subnet_test_id, test_replica_version}, messages::SignedIngressBuilder, }; use ic_types::{ @@ -117,6 +117,7 @@ where let mut consensus_pool = ConsensusPoolImpl::new( node_test_id(0), subnet_test_id(0), + &test_replica_version(), make_genesis(summary).into(), pool_config.clone(), ic_metrics::MetricsRegistry::new(), diff --git a/rs/consensus/cup_utils/src/lib.rs b/rs/consensus/cup_utils/src/lib.rs index 8bf354154c3d..e28239e2f79c 100644 --- a/rs/consensus/cup_utils/src/lib.rs +++ b/rs/consensus/cup_utils/src/lib.rs @@ -245,6 +245,7 @@ fn bootstrap_idkg_summary( #[cfg(test)] mod tests { use super::*; + use std::str::FromStr; use ic_crypto_test_utils_ni_dkg::dummy_initial_dkg_transcript; use ic_interfaces_registry::{RegistryClient, RegistryVersionedRecord}; @@ -327,7 +328,7 @@ mod tests { ); assert_eq!( result.content.version(), - &ReplicaVersion::try_from("TestID").unwrap() + &ReplicaVersion::from_str("TestID").unwrap() ); assert_eq!(result.signature.signer.dealer_subnet, subnet_test_id(0)); } diff --git a/rs/consensus/dkg/src/lib.rs b/rs/consensus/dkg/src/lib.rs index be56990a6079..b77f1f920293 100644 --- a/rs/consensus/dkg/src/lib.rs +++ b/rs/consensus/dkg/src/lib.rs @@ -21,6 +21,7 @@ use ic_types::{ Signed, threshold_sig::ni_dkg::{NiDkgId, NiDkgTargetSubnet, config::NiDkgConfig}, }, + replica_config::ReplicaConfig, }; use rayon::prelude::*; use std::{ @@ -64,6 +65,7 @@ const REMOTE_DKG_REPEATED_FAILURE_ERROR: &str = "Attempts to run this DKG repeat pub struct DkgImpl { node_id: NodeId, subnet_id: SubnetId, + replica_version: ReplicaVersion, registry_client: Arc, state_reader: Arc>, crypto: Arc, @@ -76,8 +78,7 @@ pub struct DkgImpl { impl DkgImpl { /// Build a new DKG component pub fn new( - node_id: NodeId, - subnet_id: SubnetId, + replica_config: ReplicaConfig, registry_client: Arc, state_reader: Arc>, crypto: Arc, @@ -86,9 +87,15 @@ impl DkgImpl { metrics_registry: ic_metrics::MetricsRegistry, logger: ReplicaLogger, ) -> Self { + let ReplicaConfig { + node_id, + subnet_id, + replica_version, + } = replica_config; Self { node_id, subnet_id, + replica_version, registry_client, state_reader, crypto, @@ -125,7 +132,11 @@ impl DkgImpl { let content = match ic_interfaces::crypto::NiDkgAlgorithm::create_dealing(&*self.crypto, config) { - Ok(dealing) => DealingContent::new(dealing, config.dkg_id().clone()), + Ok(dealing) => DealingContent::new( + dealing, + config.dkg_id().clone(), + self.replica_version.clone(), + ), Err(err) => { match config.dkg_id().target_subnet { NiDkgTargetSubnet::Local => error!( @@ -189,7 +200,7 @@ impl DkgImpl { return Mutations::new(); }; - if message.content.version != ReplicaVersion::default() { + if message.content.version != self.replica_version { return Mutations::from(ChangeAction::RemoveFromUnvalidated((*message).clone())); } @@ -455,10 +466,12 @@ mod tests { errors::create_transcript_error::DkgCreateTranscriptError, }, }, + replica_config::ReplicaConfig, time::UNIX_EPOCH, }; use payload_validator::validate_payload; - use std::{collections::BTreeSet, convert::TryFrom}; + use std::collections::BTreeSet; + use std::str::FromStr; use test_utils::{extract_dealings_from_highest_block, extract_remote_dkgs_from_highest_block}; use utils::{tags_iter, vetkd_key_ids_for_subnet}; @@ -504,6 +517,7 @@ mod tests { dkg_pool, registry, state_manager, + replica_config, .. } = DependenciesBuilder::single_subnet( pool_config, @@ -530,8 +544,10 @@ mod tests { let dkg_key_manager = new_dkg_key_manager(crypto.clone(), logger.clone(), &PoolReader::new(&pool)); let dkg = DkgImpl::new( - replica_1, - subnet_id, + ReplicaConfig { + node_id: replica_1, + ..replica_config.clone() + }, registry.clone(), state_manager.clone(), crypto.clone(), @@ -600,8 +616,10 @@ mod tests { let dkg_key_manager_2 = new_dkg_key_manager(crypto.clone(), logger.clone(), &PoolReader::new(&pool)); let dkg_2 = DkgImpl::new( - replica_2, - subnet_id, + ReplicaConfig { + node_id: replica_2, + ..replica_config + }, registry, state_manager, crypto, @@ -688,8 +706,10 @@ mod tests { let dkg_key_manager = new_dkg_key_manager(crypto.clone(), logger.clone(), &PoolReader::new(&pool)); let dkg = DkgImpl::new( - node_test_id(3), - replica_config.subnet_id, + ReplicaConfig { + node_id: node_test_id(3), + ..replica_config.clone() + }, registry.clone(), state_manager.clone(), crypto.clone(), @@ -704,8 +724,10 @@ mod tests { let dkg_key_manager = new_dkg_key_manager(crypto.clone(), logger.clone(), &PoolReader::new(&pool)); let dkg = DkgImpl::new( - node_test_id(1), - replica_config.subnet_id, + ReplicaConfig { + node_id: node_test_id(1), + ..replica_config + }, registry, state_manager, crypto, @@ -795,8 +817,10 @@ mod tests { let dkg_key_manager = new_dkg_key_manager(crypto.clone(), logger.clone(), &PoolReader::new(&pool)); let dkg = DkgImpl::new( - node_test_id(1), - replica_config.subnet_id, + ReplicaConfig { + node_id: node_test_id(1), + ..replica_config.clone() + }, registry.clone(), state_manager.clone(), crypto, @@ -1079,8 +1103,10 @@ mod tests { &PoolReader::new(&consensus_pool_1), ); let dkg_1 = DkgImpl::new( - node_id_1, - replica_config_1.subnet_id, + ReplicaConfig { + node_id: node_id_1, + ..replica_config_1.clone() + }, registry_1, state_manager_1, crypto.clone(), @@ -1096,8 +1122,10 @@ mod tests { &PoolReader::new(&consensus_pool_2), ); let dkg_2 = DkgImpl::new( - node_id_2, - replica_config_2.subnet_id, + ReplicaConfig { + node_id: node_id_2, + ..replica_config_2.clone() + }, registry_2, state_manager_2, crypto.clone(), @@ -1328,7 +1356,7 @@ mod tests { // that it gets rejected. let mut invalid_dealing_message = valid_dealing_message.clone(); invalid_dealing_message.content.version = - ReplicaVersion::try_from("invalid_version").unwrap(); + ReplicaVersion::from_str("invalid_version").unwrap(); node_2.dkg_pool.insert(UnvalidatedArtifact { message: invalid_dealing_message.clone(), @@ -1527,6 +1555,8 @@ mod tests { let state_manager_2 = dependencies_2.state_manager.clone(); let subnet_id_1 = dependencies_1.replica_config.subnet_id; let subnet_id_2 = dependencies_2.replica_config.subnet_id; + let replica_version_1 = dependencies_1.replica_config.replica_version; + let replica_version_2 = dependencies_2.replica_config.replica_version; let mut pool_1 = dependencies_1.pool; let mut pool_2 = dependencies_2.pool; @@ -1570,8 +1600,11 @@ mod tests { &PoolReader::new(&pool_1), ); let dkg_1 = DkgImpl::new( - node_test_id(1), - subnet_id_1, + ReplicaConfig { + node_id: node_test_id(1), + subnet_id: subnet_id_1, + replica_version: replica_version_1, + }, registry_1, state_manager_1, crypto_1, @@ -1582,8 +1615,11 @@ mod tests { ); let dkg_2 = DkgImpl::new( - node_test_id(2), - subnet_id_2, + ReplicaConfig { + node_id: node_test_id(2), + subnet_id: subnet_id_2, + replica_version: replica_version_2, + }, registry_2, state_manager_2, crypto_2.clone(), @@ -2091,8 +2127,10 @@ mod tests { &PoolReader::new(&deps.pool), ); let receiver_dkg = DkgImpl::new( - node_test_id(2), - deps.replica_config.subnet_id, + ReplicaConfig { + node_id: node_test_id(2), + ..deps.replica_config.clone() + }, deps.registry.clone(), deps.state_manager.clone(), deps.crypto.clone(), diff --git a/rs/consensus/dkg/src/payload_validator.rs b/rs/consensus/dkg/src/payload_validator.rs index 20475ae15d1e..730d383b3415 100644 --- a/rs/consensus/dkg/src/payload_validator.rs +++ b/rs/consensus/dkg/src/payload_validator.rs @@ -272,6 +272,7 @@ mod tests { use ic_test_utilities_state::get_initial_state; use ic_test_utilities_types::ids::{ NODE_1, NODE_2, NODE_3, SUBNET_1, SUBNET_2, node_test_id, subnet_test_id, + test_replica_version, }; use ic_types::{ Height, NodeId, RegistryVersion, @@ -283,6 +284,7 @@ mod tests { }, crypto::threshold_sig::ni_dkg::{NiDkgId, NiDkgTag, NiDkgTargetSubnet}, messages::CallbackId, + replica_config::ReplicaConfig, time::UNIX_EPOCH, }; use std::{ @@ -688,6 +690,7 @@ mod tests { target_subnet: NiDkgTargetSubnet::Local, dkg_tag, }, + test_replica_version(), ); Message::fake(content, dealer_id) @@ -791,8 +794,11 @@ mod tests { ); let key_manager = Arc::new(Mutex::new(key_manager)); let dkg_impl = DkgImpl::new( - node_id, - subnet_id, + ReplicaConfig { + node_id, + subnet_id, + replica_version: test_replica_version(), + }, registry.clone(), state_manager.clone(), crypto.clone(), diff --git a/rs/consensus/dkg/src/test_utils.rs b/rs/consensus/dkg/src/test_utils.rs index 7632acc8228c..bebd3003617b 100644 --- a/rs/consensus/dkg/src/test_utils.rs +++ b/rs/consensus/dkg/src/test_utils.rs @@ -9,7 +9,7 @@ use ic_test_artifact_pool::consensus_pool::TestConsensusPool; use ic_test_utilities::state_manager::RefMockStateManager; use ic_test_utilities_consensus::fake::FakeContentSigner; use ic_test_utilities_types::{ - ids::{node_test_id, subnet_test_id}, + ids::{node_test_id, subnet_test_id, test_replica_version}, messages::RequestBuilder, }; use ic_types::{ @@ -153,7 +153,11 @@ pub(super) fn extract_dkg_configs_from_highest_block( /// Create a dealing from the node `node_idx` pub(super) fn create_dealing(node_idx: u64, dkg_id: NiDkgId) -> Message { - let content = DealingContent::new(dummy_dealing(node_idx as u8), dkg_id); + let content = DealingContent::new( + dummy_dealing(node_idx as u8), + dkg_id, + test_replica_version(), + ); Message::fake(content, node_test_id(node_idx)) } diff --git a/rs/consensus/dkg/src/utils.rs b/rs/consensus/dkg/src/utils.rs index 5c222f89cc26..3b203e05dd21 100644 --- a/rs/consensus/dkg/src/utils.rs +++ b/rs/consensus/dkg/src/utils.rs @@ -304,6 +304,7 @@ mod tests { parent.height.increment(), Rank(0), parent.context.clone(), + parent.version.clone(), ); BlockProposal::fake( block, diff --git a/rs/consensus/idkg/src/payload_builder.rs b/rs/consensus/idkg/src/payload_builder.rs index 9cb13ec2e9d9..14536853eded 100644 --- a/rs/consensus/idkg/src/payload_builder.rs +++ b/rs/consensus/idkg/src/payload_builder.rs @@ -727,7 +727,9 @@ mod tests { idkg::*, }; use ic_test_utilities_registry::{SubnetRecordBuilder, add_subnet_record}; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, user_test_id}; + use ic_test_utilities_types::ids::{ + node_test_id, subnet_test_id, test_replica_version, user_test_id, + }; use ic_types::{ Height, Randomness, RegistryVersion, batch::BatchPayload, @@ -1482,6 +1484,7 @@ mod tests { certified_height: Height::from(42), time: UNIX_EPOCH, }, + test_replica_version(), ); assert_proposal_conversion(b); diff --git a/rs/consensus/idkg/src/utils.rs b/rs/consensus/idkg/src/utils.rs index 82f0a6ed8e1a..2e56ca38544d 100644 --- a/rs/consensus/idkg/src/utils.rs +++ b/rs/consensus/idkg/src/utils.rs @@ -532,7 +532,7 @@ mod tests { use ic_registry_subnet_features::KeyConfig; use ic_test_utilities_consensus::{fake::Fake, idkg::*}; use ic_test_utilities_registry::{SubnetRecordBuilder, add_subnet_record}; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; + use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, test_replica_version}; use ic_types::{ batch::ValidationContext, consensus::{ @@ -924,6 +924,7 @@ mod tests { certified_height: Height::from(42), time: UNIX_EPOCH, }, + test_replica_version(), ) } diff --git a/rs/consensus/mocks/src/lib.rs b/rs/consensus/mocks/src/lib.rs index a22772e6122b..a62512700cd6 100644 --- a/rs/consensus/mocks/src/lib.rs +++ b/rs/consensus/mocks/src/lib.rs @@ -1,4 +1,5 @@ //! Contains mocks for traits internal to consensus + use ic_artifact_pool::{ canister_http_pool::CanisterHttpPoolImpl, dkg_pool::DkgPoolImpl, idkg_pool::IDkgPoolImpl, }; @@ -25,13 +26,13 @@ use ic_test_utilities_registry::{ use ic_test_utilities_time::FastForwardTimeSource; use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; use ic_types::{ - Height, RegistryVersion, SubnetId, Time, + Height, RegistryVersion, ReplicaVersion, SubnetId, Time, batch::{BatchPayload, ValidationContext}, consensus::{Payload, block_maker::SubnetRecords}, replica_config::ReplicaConfig, }; -use mockall::predicate::*; use mockall::*; +use std::str::FromStr; use std::{ collections::BTreeSet, sync::{Arc, RwLock}, @@ -178,6 +179,8 @@ impl DependenciesBuilder { replica_config: ReplicaConfig { node_id: node_test_id(0), subnet_id: subnet_records[0].1, + replica_version: ReplicaVersion::from_str(&subnet_records[0].2.replica_version_id) + .expect("Invalid replica_version_id"), }, sorted_subnet_records: subnet_records, with_state_manager_expectations: true, @@ -280,6 +283,7 @@ impl DependenciesBuilder { let pool = TestConsensusPool::new( self.replica_config.node_id, self.replica_config.subnet_id, + self.replica_config.replica_version.clone(), self.pool_config, time_source.clone(), registry.clone(), diff --git a/rs/consensus/src/consensus.rs b/rs/consensus/src/consensus.rs index 83cc40d705b1..8ce4b3d5f346 100644 --- a/rs/consensus/src/consensus.rs +++ b/rs/consensus/src/consensus.rs @@ -58,7 +58,6 @@ use ic_replicated_state::ReplicatedState; use ic_types::{ Time, artifact::ConsensusMessageId, consensus::ConsensusMessageHashable, malicious_flags::MaliciousFlags, replica_config::ReplicaConfig, - replica_version::ReplicaVersion, }; use rayon::{ThreadPool, ThreadPoolBuilder}; use std::{ @@ -100,23 +99,6 @@ enum ConsensusSubcomponent { Purger, } -/// Describe expected version and artifact version when there is a mismatch. -#[derive(Debug)] -pub(crate) struct ReplicaVersionMismatch {} - -/// The function checks if the version of the given artifact matches the default -/// protocol version and returns an error if it does not. -pub(crate) fn check_protocol_version( - version: &ReplicaVersion, -) -> Result<(), ReplicaVersionMismatch> { - let expected_version = ReplicaVersion::default(); - if version != &expected_version { - Err(ReplicaVersionMismatch {}) - } else { - Ok(()) - } -} - /// Builds a rayon thread pool with the given number of threads. pub fn build_thread_pool(num_threads: usize) -> Arc { Arc::new( diff --git a/rs/consensus/src/consensus/batch_delivery.rs b/rs/consensus/src/consensus/batch_delivery.rs index 6773ccc1bba6..0b1e629f96ad 100644 --- a/rs/consensus/src/consensus/batch_delivery.rs +++ b/rs/consensus/src/consensus/batch_delivery.rs @@ -122,7 +122,6 @@ pub(crate) fn deliver_batches_with_result_processor( ); break; }; - let replica_version = block.version().clone(); let mut block_stats = BlockStats::from(&block); debug!( every_n_seconds => 5, @@ -131,7 +130,7 @@ pub(crate) fn deliver_batches_with_result_processor( consensus => ConsensusLogEntry { height: Some(height.get()), hash: Some(block_stats.block_hash.clone()), - replica_version: Some(String::from(&replica_version)) + replica_version: Some(block.version().to_string()) } ); @@ -163,6 +162,7 @@ pub(crate) fn deliver_batches_with_result_processor( registry_client, subnet_id, pool, + block.version(), log, ) { Some(Status::Halting | Status::Halted) => { @@ -296,7 +296,7 @@ pub(crate) fn deliver_batches_with_result_processor( registry_version: block.context.registry_version, time: block.context.time, blockmaker_metrics, - replica_version, + replica_version: block.version().clone(), }; let result = message_routing.deliver_batch(batch); @@ -580,7 +580,7 @@ mod tests { use ic_crypto_test_utils_ni_dkg::dummy_transcript_for_tests; use ic_logger::replica_logger::no_op_logger; use ic_management_canister_types_private::{SetupInitialDKGResponse, VetKdCurve, VetKdKeyId}; - use ic_test_utilities_types::ids::subnet_test_id; + use ic_test_utilities_types::ids::{subnet_test_id, test_replica_version}; use ic_types::{ PrincipalId, RegistryVersion, SubnetId, batch::{BatchPayload, ValidationContext}, @@ -744,6 +744,7 @@ mod tests { certified_height: Height::from(0), time: UNIX_EPOCH, }, + test_replica_version(), ); let mut batch_stats = BatchStats::new(Height::from(1)); diff --git a/rs/consensus/src/consensus/block_maker.rs b/rs/consensus/src/consensus/block_maker.rs index c116b44615c8..9bd0ec14d5c3 100644 --- a/rs/consensus/src/consensus/block_maker.rs +++ b/rs/consensus/src/consensus/block_maker.rs @@ -379,6 +379,7 @@ impl BlockMaker { self.registry_client.as_ref(), self.replica_config.subnet_id, pool, + &self.replica_config.replica_version, &self.log, )? { // Don't propose any block if the replica is halted. @@ -438,7 +439,14 @@ impl BlockMaker { } }, ); - let block = Block::new(parent.get_hash().clone(), payload, height, rank, context); + let block = Block::new( + parent.get_hash().clone(), + payload, + height, + rank, + context, + self.replica_config.replica_version.clone(), + ); let hashed_block = hashed::Hashed::new(ic_types::crypto::crypto_hash, block); let metadata = BlockMetadata::from_block(&hashed_block, self.replica_config.subnet_id); match self @@ -735,7 +743,7 @@ mod tests { use ic_registry_keys::make_catch_up_package_contents_key; use ic_test_utilities_consensus::fake::FromParent; use ic_test_utilities_registry::{SubnetRecordBuilder, add_subnet_record}; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; + use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, test_replica_version}; use ic_types::{ consensus::{ CatchUpContent, CatchUpPackage, HasHeight, HasVersion, HashedRandomBeacon, dkg, @@ -785,6 +793,13 @@ mod tests { ), ], ) + // The block-making schedule depends on the random state set up by dependencies. + // For this test, we simulate the blockmaker running on node with ID 1. + .with_replica_config(ReplicaConfig { + node_id: node_test_id(1), + subnet_id, + replica_version: test_replica_version(), + }) .build(); pool.advance_round_normal_operation_n(4); @@ -853,6 +868,7 @@ mod tests { next_height, Rank(4), expected_context.clone(), + replica_config.replica_version.clone(), ); payload_builder @@ -875,6 +891,7 @@ mod tests { }) .unwrap(), subnet_id: replica_config.subnet_id, + replica_version: replica_config.replica_version, }; let block_maker = BlockMaker::new( @@ -1001,6 +1018,7 @@ mod tests { }) .unwrap(), subnet_id: replica_config.subnet_id, + replica_version: replica_config.replica_version, }; let block_maker = BlockMaker::new( @@ -1066,7 +1084,7 @@ mod tests { // making only empty blocks. #[test] fn test_halting_due_to_registry_instruction() { - test_halting(ReplicaVersion::default(), /*halt_at_cup_height=*/ true) + test_halting(test_replica_version(), /*halt_at_cup_height=*/ true) } fn test_halting(replica_version: ReplicaVersion, halt_at_cup_height: bool) { @@ -1172,7 +1190,7 @@ mod tests { let block_maker = BlockMaker::new( Arc::clone(&time_source) as Arc<_>, - replica_config, + replica_config.clone(), Arc::clone(®istry) as Arc, membership, crypto, @@ -1203,8 +1221,8 @@ mod tests { assert!(proposal.is_some()); let proposal = proposal.unwrap(); let block = proposal.content.as_ref(); - // blocks still uses default version, not the new version. - assert_eq!(block.version(), &ReplicaVersion::default()); + // The block still uses the old version, not the new version. + assert_eq!(block.version(), &replica_config.replica_version); // registry version 10 becomes effective. assert_eq!( PoolReader::new(&pool).registry_version(proposal.height()), diff --git a/rs/consensus/src/consensus/catchup_package_maker.rs b/rs/consensus/src/consensus/catchup_package_maker.rs index 0e0c2ca39bf5..de93ca4ebcc2 100644 --- a/rs/consensus/src/consensus/catchup_package_maker.rs +++ b/rs/consensus/src/consensus/catchup_package_maker.rs @@ -180,6 +180,7 @@ impl CatchUpPackageMaker { self.membership.registry_client.as_ref(), self.membership.subnet_id, pool, + &self.replica_config.replica_version, &self.log, ) == Some(true) }; diff --git a/rs/consensus/src/consensus/finalizer.rs b/rs/consensus/src/consensus/finalizer.rs index ccc0715d2aa2..321db49300d1 100644 --- a/rs/consensus/src/consensus/finalizer.rs +++ b/rs/consensus/src/consensus/finalizer.rs @@ -235,6 +235,7 @@ impl Finalizer { self.pick_block_to_finality_sign(pool, height)? .get_hash() .clone(), + self.replica_config.replica_version.clone(), ); let signature = self .crypto diff --git a/rs/consensus/src/consensus/malicious_consensus.rs b/rs/consensus/src/consensus/malicious_consensus.rs index 038ed62bac0b..71543a9cfaa5 100644 --- a/rs/consensus/src/consensus/malicious_consensus.rs +++ b/rs/consensus/src/consensus/malicious_consensus.rs @@ -278,7 +278,11 @@ impl ConsensusImpl { pool: &PoolReader<'_>, block: &Block, ) -> Option { - let content = FinalizationContent::new(block.height, ic_types::crypto::crypto_hash(block)); + let content = FinalizationContent::new( + block.height, + ic_types::crypto::crypto_hash(block), + self.replica_config.replica_version.clone(), + ); let signature = self .finalizer .crypto diff --git a/rs/consensus/src/consensus/notary.rs b/rs/consensus/src/consensus/notary.rs index 8f3374c6c12b..25f586d26c68 100644 --- a/rs/consensus/src/consensus/notary.rs +++ b/rs/consensus/src/consensus/notary.rs @@ -40,7 +40,7 @@ use ic_metrics::MetricsRegistry; use ic_registry_client_helpers::subnet::NotarizationDelaySettings; use ic_replicated_state::ReplicatedState; use ic_types::{ - Height, + Height, ReplicaVersion, consensus::{ BlockProposal, HasBlockHash, HasHeight, HasRank, HashedBlock, NotarizationContent, NotarizationShare, RandomBeacon, Rank, @@ -133,6 +133,7 @@ impl Notary { &self.log, height, rank, + &self.replica_config.replica_version, )?; let now_relative = self.time_source.get_relative_time(); @@ -184,7 +185,11 @@ impl Notary { block: &HashedBlock, ) -> Option { let registry_version = pool.registry_version(block.height())?; - let content = NotarizationContent::new(block.height(), block.get_hash().clone()); + let content = NotarizationContent::new( + block.height(), + block.get_hash().clone(), + self.replica_config.replica_version.clone(), + ); match self .crypto .sign(&content, self.replica_config.node_id, registry_version) @@ -244,6 +249,7 @@ fn get_adjusted_notary_delay( log: &ReplicaLogger, height: Height, rank: Rank, + replica_version: &ReplicaVersion, ) -> Option { match get_adjusted_notary_delay_from_settings( get_notarization_delay_settings( @@ -256,6 +262,7 @@ fn get_adjusted_notary_delay( state_reader, membership, rank, + replica_version, log, ) { NotaryDelay::CanNotarizeAfter(duration) => Some(duration), @@ -298,6 +305,7 @@ fn get_adjusted_notary_delay_from_settings( state_reader: &dyn StateReader, membership: &Membership, rank: Rank, + replica_version: &ReplicaVersion, logger: &ReplicaLogger, ) -> NotaryDelay { let NotarizationDelaySettings { @@ -353,6 +361,7 @@ fn get_adjusted_notary_delay_from_settings( membership.registry_client.as_ref(), membership.subnet_id, pool, + replica_version, logger, ) == Some(true) }; @@ -412,6 +421,7 @@ mod tests { } = DependenciesBuilder::new(pool_config, 1) .with_dkg_interval_length(dkg_interval_length) .build(); + let replica_version = replica_config.replica_version.clone(); state_manager .get_mut() .expect_latest_certified_height() @@ -455,6 +465,7 @@ mod tests { &no_op_logger(), Height::from(1), Rank(0), + &replica_version, ) .unwrap(), ) @@ -504,6 +515,7 @@ mod tests { &no_op_logger(), Height::from(1), Rank(9), + &replica_version, ) .unwrap(), ) @@ -521,6 +533,7 @@ mod tests { &no_op_logger(), Height::from(1), twenty_block.rank(), + &replica_version, ) .unwrap(), ) @@ -605,6 +618,7 @@ mod tests { } = DependenciesBuilder::new(pool_config, 1) .with_dkg_interval_length(dkg_interval_length) .build(); + let replica_version = replica_config.replica_version.clone(); state_manager .get_mut() .expect_latest_certified_height() @@ -646,6 +660,7 @@ mod tests { &no_op_logger(), Height::from(5), Rank(0), + &replica_version, ) .unwrap(), ); @@ -665,6 +680,7 @@ mod tests { mut pool, state_manager, membership, + replica_config, .. } = DependenciesBuilder::new(pool_config, 3) /* use large enough DKG interval to trigger notarization/CUP gap limit */ @@ -713,6 +729,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ), NotaryDelay::ReachedMaxNotarizationCertificationGap { .. } @@ -738,6 +755,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ), NotaryDelay::CanNotarizeAfter(Duration::from_secs(0)) @@ -765,6 +783,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ), NotaryDelay::ReachedMaxNotarizationCUPGap { .. } @@ -787,6 +806,7 @@ mod tests { mut pool, state_manager, membership, + replica_config, .. } = DependenciesBuilder::new(pool_config, 3) .with_dkg_interval_length(dkg_interval) @@ -805,6 +825,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ); assert_eq!( @@ -820,6 +841,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ); assert_eq!( @@ -835,6 +857,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ); assert_eq!( @@ -851,6 +874,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ); assert_eq!( @@ -869,6 +893,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ); assert_eq!( @@ -894,6 +919,7 @@ mod tests { mut pool, state_manager, membership, + replica_config, .. } = DependenciesBuilder::single_subnet( pool_config, @@ -937,6 +963,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ); assert_eq!( @@ -958,6 +985,7 @@ mod tests { state_manager.as_ref(), membership.as_ref(), Rank(0), + &replica_config.replica_version, &logger, ); assert_eq!( diff --git a/rs/consensus/src/consensus/priority.rs b/rs/consensus/src/consensus/priority.rs index 182524a362e0..2871e1dd4a90 100644 --- a/rs/consensus/src/consensus/priority.rs +++ b/rs/consensus/src/consensus/priority.rs @@ -121,7 +121,7 @@ mod tests { use super::*; use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_test_utilities_consensus::fake::FakeContent; - use ic_test_utilities_types::ids::node_test_id; + use ic_test_utilities_types::ids::{node_test_id, test_replica_version}; use ic_types::{ consensus::{ ConsensusMessageHashable, Finalization, FinalizationContent, HasHeight, Notarization, @@ -153,6 +153,7 @@ mod tests { let notarization = Notarization::fake(NotarizationContent::new( block.height(), block.content.get_hash().clone(), + test_replica_version(), )); let equivocation_proof_id = ConsensusMessageId { hash: ConsensusMessageHash::EquivocationProof(CryptoHashOf::new(CryptoHash( @@ -209,9 +210,12 @@ mod tests { // Put block into validated pool, notarization in to unvalidated pool pool.insert_validated(block.clone()); + let replica_version = test_replica_version(); + let notarization = Notarization::fake(NotarizationContent::new( block.height(), block.content.get_hash().clone(), + replica_version.clone(), )); pool.insert_unvalidated(notarization.clone()); @@ -241,6 +245,7 @@ mod tests { let finalization = Finalization::fake(FinalizationContent::new( block.height(), block.content.get_hash().clone(), + replica_version.clone(), )); pool.insert_unvalidated(finalization.clone()); @@ -265,6 +270,7 @@ mod tests { let notarization = Notarization::fake(NotarizationContent::new( block.height(), block.content.get_hash().clone(), + replica_version.clone(), )); pool.insert_validated(notarization.clone()); } @@ -275,6 +281,7 @@ mod tests { let notarization = Notarization::fake(NotarizationContent::new( block.height(), block.content.get_hash().clone(), + replica_version, )); assert!( block.height().get() diff --git a/rs/consensus/src/consensus/random_beacon_maker.rs b/rs/consensus/src/consensus/random_beacon_maker.rs index 18452712f7d5..5e5fbdc51e82 100644 --- a/rs/consensus/src/consensus/random_beacon_maker.rs +++ b/rs/consensus/src/consensus/random_beacon_maker.rs @@ -71,8 +71,11 @@ impl RandomBeaconMaker { .get_random_beacon_shares(next_height) .any(|s| s.signature.signer == my_node_id) => { - let content = - RandomBeaconContent::new(next_height, ic_types::crypto::crypto_hash(&beacon)); + let content = RandomBeaconContent::new( + next_height, + ic_types::crypto::crypto_hash(&beacon), + self.replica_config.replica_version.clone(), + ); // One might wonder whether it is appropriate to use the // dkg_id from the start_block at h to generate the // random beacon at height h. The reason this is diff --git a/rs/consensus/src/consensus/random_tape_maker.rs b/rs/consensus/src/consensus/random_tape_maker.rs index 199dc13a3b43..ed1a526d3e02 100644 --- a/rs/consensus/src/consensus/random_tape_maker.rs +++ b/rs/consensus/src/consensus/random_tape_maker.rs @@ -130,7 +130,7 @@ impl RandomTapeMaker { height: Height, pool: &PoolReader<'_>, ) -> Option { - let content = RandomTapeContent::new(height); + let content = RandomTapeContent::new(height, self.replica_config.replica_version.clone()); if let Some(dkg_id) = active_low_threshold_nidkg_id(pool.as_cache(), height) { match self @@ -266,7 +266,10 @@ mod tests { round.advance(); round.advance(); pool.insert_validated(ConsensusMessage::RandomTape(RandomTape::fake( - RandomTapeContent::new(Height::from(7)), + RandomTapeContent::new( + Height::from(7), + ic_test_utilities_types::ids::test_replica_version(), + ), ))); let shares = random_tape_maker.on_state_change(&PoolReader::new(&pool)); diff --git a/rs/consensus/src/consensus/status.rs b/rs/consensus/src/consensus/status.rs index 0f002b1d5878..3f346821c871 100644 --- a/rs/consensus/src/consensus/status.rs +++ b/rs/consensus/src/consensus/status.rs @@ -40,6 +40,7 @@ pub(crate) fn get_status( registry_client: &(impl RegistryClient + ?Sized), subnet_id: SubnetId, pool: &PoolReader<'_>, + replica_version: &ReplicaVersion, logger: &ReplicaLogger, ) -> Option { if should_halt( @@ -48,6 +49,7 @@ pub(crate) fn get_status( registry_client, subnet_id, pool, + replica_version, logger, ) .warn_if_none(logger, "Failed to check if the subnet is halting!")? @@ -60,6 +62,7 @@ pub(crate) fn get_status( registry_client, subnet_id, pool, + replica_version, logger, ) .warn_if_none(logger, "Failed to check if the subnet is halted!") @@ -80,6 +83,7 @@ pub(crate) fn should_halt( registry_client: &(impl RegistryClient + ?Sized), subnet_id: SubnetId, pool: &PoolReader<'_>, + my_replica_version: &ReplicaVersion, logger: &ReplicaLogger, ) -> Option { let registry_version = pool.registry_version(height).warn_if_none( @@ -89,7 +93,7 @@ pub(crate) fn should_halt( let should_halt_due_to_upgrading = lookup_replica_version(registry_client, subnet_id, logger, registry_version) - .map(|replica_version| replica_version != ReplicaVersion::default()) + .map(|replica_version_in_registry| &replica_version_in_registry != my_replica_version) .warn_if_none(logger, "Failed to check if the upgrade is pending!"); let should_halt_due_to_subnet_splitting = last_summary_block.map(|summary_block| { @@ -176,6 +180,7 @@ mod tests { }; use ic_types_test_utils::ids::{SUBNET_0, SUBNET_1}; use rstest::rstest; + use std::str::FromStr; use super::*; @@ -186,7 +191,7 @@ mod tests { pool_config: ArtifactPoolConfig, subnet_id: SubnetId, certified_height: Height, - replica_version: ReplicaVersion, + replica_version_in_subnet_record: ReplicaVersion, halt_at_cup_height: bool, ) -> (TestConsensusPool, Arc) { let node_ids = [node_test_id(0)]; @@ -206,7 +211,7 @@ mod tests { 10, SubnetRecordBuilder::from(&node_ids) .with_dkg_interval_length(DKG_LENGTH) - .with_replica_version(replica_version.as_ref()) + .with_replica_version(replica_version_in_subnet_record.as_ref()) .with_halt_at_cup_height(halt_at_cup_height) .build(), ), @@ -226,7 +231,7 @@ mod tests { struct TestCase { certified_height: Height, current_height: Height, - replica_version: ReplicaVersion, + replica_version_in_subnet_record: ReplicaVersion, halt_at_cup_height: bool, subnet_splitting_status: Option, subnet_id: SubnetId, @@ -237,7 +242,7 @@ mod tests { #[case::upgrade_finalized(TestCase{ certified_height: CUP_HEIGHT, current_height: CUP_HEIGHT, - replica_version: ReplicaVersion::try_from("new_replica_version").unwrap(), + replica_version_in_subnet_record: ReplicaVersion::from_str("new_replica_version").unwrap(), halt_at_cup_height: false, subnet_splitting_status: None, subnet_id: SUBNET_0, @@ -246,7 +251,7 @@ mod tests { #[case::upgrade_pending(TestCase{ certified_height: CUP_HEIGHT.decrement(), current_height: CUP_HEIGHT, - replica_version: ReplicaVersion::try_from("new_replica_version").unwrap(), + replica_version_in_subnet_record: ReplicaVersion::from_str("new_replica_version").unwrap(), halt_at_cup_height: false, subnet_splitting_status: None, subnet_id: SUBNET_0, @@ -255,7 +260,7 @@ mod tests { #[case::subnet_splitting_finalized(TestCase{ certified_height: CUP_HEIGHT, current_height: CUP_HEIGHT, - replica_version: ReplicaVersion::default(), + replica_version_in_subnet_record: ReplicaVersion::from_str("current_replica_version").unwrap(), halt_at_cup_height: false, subnet_splitting_status: Some(SubnetSplittingStatus::Scheduled(SplittingArgs { destination_subnet_id: SUBNET_1, source_subnet_id: SUBNET_0 })), subnet_id: SUBNET_0, @@ -264,7 +269,7 @@ mod tests { #[case::subnet_splitting_pending(TestCase{ certified_height: CUP_HEIGHT.decrement(), current_height: CUP_HEIGHT, - replica_version: ReplicaVersion::default(), + replica_version_in_subnet_record: ReplicaVersion::from_str("current_replica_version").unwrap(), halt_at_cup_height: false, subnet_splitting_status: Some(SubnetSplittingStatus::Scheduled(SplittingArgs { destination_subnet_id: SUBNET_1, source_subnet_id: SUBNET_0 })), subnet_id: SUBNET_0, @@ -273,7 +278,7 @@ mod tests { #[case::post_subnet_splitting_old_subnet_id(TestCase{ certified_height: CUP_HEIGHT, current_height: CUP_HEIGHT, - replica_version: ReplicaVersion::default(), + replica_version_in_subnet_record: ReplicaVersion::from_str("current_replica_version").unwrap(), halt_at_cup_height: false, subnet_splitting_status: Some(SubnetSplittingStatus::PostSplit(PostSplitArgs { new_subnet_id: SUBNET_1 })), subnet_id: SUBNET_0, @@ -282,7 +287,7 @@ mod tests { #[case::post_subnet_splitting_new_subnet_id(TestCase{ certified_height: CUP_HEIGHT, current_height: CUP_HEIGHT, - replica_version: ReplicaVersion::default(), + replica_version_in_subnet_record: ReplicaVersion::from_str("current_replica_version").unwrap(), halt_at_cup_height: false, subnet_splitting_status: Some(SubnetSplittingStatus::PostSplit(PostSplitArgs { new_subnet_id: SUBNET_1 })), subnet_id: SUBNET_1, @@ -291,7 +296,7 @@ mod tests { #[case::halt_at_cup_height(TestCase{ certified_height: CUP_HEIGHT, current_height: CUP_HEIGHT, - replica_version: ReplicaVersion::default(), + replica_version_in_subnet_record: ReplicaVersion::from_str("current_replica_version").unwrap(), halt_at_cup_height: true, subnet_splitting_status: None, subnet_id: SUBNET_0, @@ -300,7 +305,7 @@ mod tests { #[case::halting_at_cup_height(TestCase{ certified_height: CUP_HEIGHT.decrement(), current_height: CUP_HEIGHT, - replica_version: ReplicaVersion::default(), + replica_version_in_subnet_record: ReplicaVersion::from_str("current_replica_version").unwrap(), halt_at_cup_height: true, subnet_splitting_status: None, subnet_id: SUBNET_0, @@ -309,20 +314,21 @@ mod tests { #[case::running(TestCase{ certified_height: CUP_HEIGHT, current_height: CUP_HEIGHT, - replica_version: ReplicaVersion::default(), + replica_version_in_subnet_record: ReplicaVersion::from_str("current_replica_version").unwrap(), halt_at_cup_height: false, subnet_splitting_status: None, subnet_id: SUBNET_0, expected_status: Some(Status::Running), })] fn status_test(#[case] test_case: TestCase) { + let current_replica_version = ReplicaVersion::from_str("current_replica_version").unwrap(); with_test_replica_logger(|logger| { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let (pool, registry_client) = set_up( pool_config, test_case.subnet_id, test_case.certified_height, - test_case.replica_version, + test_case.replica_version_in_subnet_record, test_case.halt_at_cup_height, ); let mut last_summary_block = @@ -339,6 +345,7 @@ mod tests { registry_client.as_ref(), test_case.subnet_id, &PoolReader::new(&pool), + ¤t_replica_version, &logger, ); diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index 9223f47cb93a..2fc17cb8aa33 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2,7 +2,7 @@ //! artifacts. #![allow(clippy::result_large_err)] use crate::consensus::{ - ConsensusMessageId, check_protocol_version, + ConsensusMessageId, metrics::ValidatorMetrics, status::{self, Status}, }; @@ -794,8 +794,11 @@ impl Validator { pool_reader: &PoolReader<'_>, artifact: &S, ) -> ValidationResult { - check_protocol_version(artifact.version()) - .map_err(|_| InvalidArtifactReason::ReplicaVersionMismatch)?; + let version = artifact.version(); + let expected_version = &self.replica_config.replica_version; + if version != expected_version { + return Err(InvalidArtifactReason::ReplicaVersionMismatch.into()); + } artifact.verify_signature( self.membership.as_ref(), self.crypto.as_ref(), @@ -912,7 +915,9 @@ impl Validator { Signed: SignatureVerify + ConsensusMessageHashable + Clone, T: NotaryIssued + HasVersion, { - if check_protocol_version(notary_issued.content.version()).is_err() { + let version = notary_issued.content.version(); + let expected_version = &self.replica_config.replica_version; + if version != expected_version { return Some(ChangeAction::RemoveFromUnvalidated( notary_issued.into_message(), )); @@ -1221,6 +1226,7 @@ impl Validator { self.registry_client.as_ref(), self.replica_config.subnet_id, pool_reader, + &self.replica_config.replica_version, &self.log, ) else { return Err(ValidationFailure::FailedToGetConsensusStatus.into()); @@ -2057,6 +2063,7 @@ pub mod test { }; use ic_types_test_utils::ids::{NODE_1, NODE_2}; use rstest::rstest; + use std::str::FromStr; use std::sync::{Arc, RwLock}; pub fn assert_block_valid(results: &[ChangeAction], block: &BlockProposal) { @@ -2083,6 +2090,7 @@ pub mod test { struct ValidatorAndDependencies { validator: Validator, payload_builder: Arc, + membership: Arc, state_manager: Arc, message_routing: Arc, registry_data_provider: Arc, @@ -2146,7 +2154,7 @@ pub mod test { let validator = Validator::new( replica_config.clone(), - membership, + membership.clone(), registry.clone(), crypto, payload_builder.clone(), @@ -2162,6 +2170,7 @@ pub mod test { ValidatorAndDependencies { validator, payload_builder, + membership, state_manager, message_routing, registry_data_provider, @@ -2224,7 +2233,7 @@ pub mod test { pool.insert_unvalidated(cup_share_summary_height.clone()); let mut cup_from_old_replica_version = cup_share_summary_height.clone(); cup_from_old_replica_version.content.version = - ReplicaVersion::try_from("old_version").unwrap(); + ReplicaVersion::from_str("old_version").unwrap(); pool.insert_unvalidated(cup_from_old_replica_version.clone()); let mut cup_with_registry_version = cup_share_summary_height.clone(); cup_with_registry_version @@ -2542,7 +2551,7 @@ pub mod test { pool.insert_unvalidated(share_3.clone()); let mut share_with_old_version = share_3.clone(); share_with_old_version.content = RandomBeaconContent { - version: ReplicaVersion::try_from("old_version").unwrap(), + version: ReplicaVersion::from_str("old_version").unwrap(), height: share_3.content.height, parent: share_3.content.parent.clone(), }; @@ -2630,12 +2639,15 @@ pub mod test { // Insert a random tape of height 1 in validated pool, check if only share_2 is // validated - let tape_1 = RandomTape::fake(RandomTapeContent::new(Height::from(1))); + let tape_1 = RandomTape::fake(RandomTapeContent::new( + Height::from(1), + replica_config.replica_version.clone(), + )); pool.insert_validated(tape_1); let mut old_replica_version_share = share_2.clone(); old_replica_version_share.content.version = - ReplicaVersion::try_from("old_version").unwrap(); + ReplicaVersion::from_str("old_version").unwrap(); pool.insert_unvalidated(old_replica_version_share.clone()); let changeset = validator.on_state_change(&PoolReader::new(&pool)); @@ -2659,7 +2671,8 @@ pub mod test { pool.apply(changeset); // Insert random tape at height 4, check if it is ignored - let content = RandomTapeContent::new(Height::from(4)); + let content = + RandomTapeContent::new(Height::from(4), replica_config.replica_version.clone()); let signature = ThresholdSignature::fake(); let tape_4 = RandomTape { content, signature }; pool.insert_unvalidated(tape_4.clone()); @@ -2682,7 +2695,7 @@ pub mod test { pool.apply(changeset); // Set expected batch height to height 4, check if tape_3 is ignored - let content = RandomTapeContent::new(Height::from(3)); + let content = RandomTapeContent::new(Height::from(3), replica_config.replica_version); let signature = ThresholdSignature::fake(); let tape_3 = RandomTape { content, signature }; pool.insert_unvalidated(tape_3); @@ -2844,7 +2857,7 @@ pub mod test { test_block.context.registry_version = RegistryVersion::from(11); test_block.context.certified_height = Height::from(1); - test_block.version = ReplicaVersion::try_from("old_version").unwrap(); + test_block.version = ReplicaVersion::from_str("old_version").unwrap(); let block_proposal = BlockProposal::fake(test_block.clone(), node_id); pool.insert_unvalidated(block_proposal.clone()); @@ -2922,7 +2935,8 @@ pub mod test { registry.as_ref(), replica_config.subnet_id, &PoolReader::new(&pool), - &no_op_logger(), + &replica_config.replica_version, + &no_op_logger() ), Some(Status::Halting | Status::Halted) ); @@ -3871,6 +3885,7 @@ pub mod test { state_manager, mut pool, time_source, + replica_config, .. } = ValidatorAndDependenciesBuilder::new(pool_config, 4) .with_dkg_interval_length(cup_height.get() - 1) @@ -3891,9 +3906,10 @@ pub mod test { certified_height: Height::from(42), time: ic_types::time::UNIX_EPOCH, }, + replica_config.replica_version.clone(), ); let fake_beacon = RandomBeacon::fake(RandomBeaconContent { - version: ReplicaVersion::default(), + version: replica_config.replica_version, height: cup_height, parent: CryptoHashOf::from(CryptoHash(vec![])), }); @@ -3956,7 +3972,7 @@ pub mod test { let finalization = pool.validated().finalization().get_highest().unwrap(); let mut catch_up_package = pool.make_catch_up_package(finalization.height()); - catch_up_package.content.version = ReplicaVersion::try_from("old_version").unwrap(); + catch_up_package.content.version = ReplicaVersion::from_str("old_version").unwrap(); pool.insert_unvalidated(catch_up_package.clone()); state_manager @@ -4122,6 +4138,7 @@ pub mod test { payload_builder, state_manager, mut pool, + replica_config, .. } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); pool.advance_round_normal_operation(); @@ -4152,6 +4169,7 @@ pub mod test { let content = NotarizationContent::new( block.height(), ic_types::crypto::crypto_hash(block.as_ref()), + replica_config.replica_version, ); let mut notarization = Notarization::fake(content); notarization.signature.signers = @@ -4377,6 +4395,7 @@ pub mod test { let ValidatorAndDependencies { validator, mut pool, + replica_config, .. } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); @@ -4391,6 +4410,7 @@ pub mod test { let mut notarization = Notarization::fake(NotarizationContent::new( block.height(), block.content.get_hash().clone(), + replica_config.replica_version, )); notarization.signature.signers = vec![node_test_id(1), node_test_id(2), node_test_id(3)]; @@ -4551,7 +4571,7 @@ pub mod test { // A post-upgrade block let mut block_with_new_version = block; block_with_new_version.content.as_mut().version = - ReplicaVersion::try_from("new_version").unwrap(); + ReplicaVersion::from_str("new_version").unwrap(); block_with_new_version.update_content(); // Block proposals with replica version mismatches are simply removed @@ -4814,6 +4834,8 @@ pub mod test { time_source, payload_builder, mut pool, + replica_config, + membership, .. } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, @@ -4844,9 +4866,26 @@ pub mod test { let content = NotarizationContent::new( block.height(), block.content.get_hash().clone(), + replica_config.replica_version.clone(), ); let mut notarization = Notarization::fake(content); - notarization.signature.signers = vec![NODE_2]; + let random_beacon = PoolReader::new(&pool).get_random_beacon_tip(); + // Predict which node will be in the notarization committee and pick that + // node to sign the notarization. Otherwise, the notarization would be + // ignored. + let signer = if membership + .node_belongs_to_notarization_committee( + random_beacon.height().increment(), + &random_beacon, + NODE_1, + ) + .unwrap() + { + NODE_1 + } else { + NODE_2 + }; + notarization.signature.signers = vec![signer]; Some(notarization) } else { None diff --git a/rs/consensus/tests/framework/runner.rs b/rs/consensus/tests/framework/runner.rs index 759ed2c3f9a8..a0e8be39561c 100644 --- a/rs/consensus/tests/framework/runner.rs +++ b/rs/consensus/tests/framework/runner.rs @@ -170,8 +170,7 @@ impl<'a> ConsensusRunner<'a> { deps.message_routing.clone(), ); let dkg = ic_consensus_dkg::DkgImpl::new( - deps.replica_config.node_id, - deps.replica_config.subnet_id, + deps.replica_config.clone(), Arc::clone(&deps.registry_client), deps.state_manager.clone(), Arc::clone(&consensus_crypto), diff --git a/rs/consensus/tests/framework/test_runner.rs b/rs/consensus/tests/framework/test_runner.rs index c52787989569..7b6971b6c5f5 100644 --- a/rs/consensus/tests/framework/test_runner.rs +++ b/rs/consensus/tests/framework/test_runner.rs @@ -9,7 +9,7 @@ use ic_management_canister_types_private::MasterPublicKeyId; use ic_registry_client_fake::FakeRegistryClient; use ic_registry_proto_data_provider::ProtoRegistryDataProvider; use ic_test_utilities_time::FastForwardTimeSource; -use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; +use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, test_replica_version}; use ic_types::{Height, batch::BatchContent, crypto::CryptoHash, replica_config::ReplicaConfig}; use rand_chacha::{ChaChaRng, rand_core::SeedableRng}; use std::{cell::RefCell, rc::Rc, sync::Arc}; @@ -85,6 +85,7 @@ impl TestRunner { .map(|(index, _)| ReplicaConfig { node_id: node_test_id(index as u64), subnet_id, + replica_version: test_replica_version(), }) .collect(); let node_ids: Vec<_> = replica_configs diff --git a/rs/consensus/tests/framework/types.rs b/rs/consensus/tests/framework/types.rs index fbf17ddebe68..3ce2ce56061e 100644 --- a/rs/consensus/tests/framework/types.rs +++ b/rs/consensus/tests/framework/types.rs @@ -34,7 +34,7 @@ use ic_test_utilities::{ }; use ic_test_utilities_consensus::{IDkgStatsNoOp, batch::MockBatchPayloadBuilder}; use ic_types::{ - NodeId, SubnetId, + NodeId, ReplicaVersion, SubnetId, artifact::IdentifiableArtifact, consensus::{ CatchUpPackage, ConsensusMessage, HasHeight, certification::CertificationMessage, @@ -45,6 +45,7 @@ use ic_types::{ }; use rand_chacha::ChaChaRng; use rayon::ThreadPool; +use std::str::FromStr; use std::{ cell::{RefCell, RefMut}, cmp::Ordering, @@ -206,6 +207,7 @@ impl ConsensusDependencies { let consensus_pool = Arc::new(RwLock::new(ConsensusPoolImpl::new( replica_config.node_id, replica_config.subnet_id, + &replica_config.replica_version, cup.into(), pool_config.clone(), metrics_registry.clone(), diff --git a/rs/consensus/tests/payload.rs b/rs/consensus/tests/payload.rs index cea5a9749104..560ea28fb48f 100644 --- a/rs/consensus/tests/payload.rs +++ b/rs/consensus/tests/payload.rs @@ -27,7 +27,7 @@ use ic_test_utilities_registry::{SubnetRecordBuilder, setup_registry}; use ic_test_utilities_state::get_initial_state; use ic_test_utilities_time::FastForwardTimeSource; use ic_test_utilities_types::{ - ids::{node_test_id, subnet_test_id}, + ids::{node_test_id, subnet_test_id, test_replica_version}, messages::SignedIngressBuilder, }; use ic_types::{ @@ -107,7 +107,12 @@ fn consensus_produces_expected_batches() { let router = Arc::new(router); let node_id = node_test_id(0); let subnet_id = subnet_test_id(0); - let replica_config = ReplicaConfig { node_id, subnet_id }; + let replica_version = test_replica_version(); + let replica_config = ReplicaConfig { + node_id, + subnet_id, + replica_version, + }; let fake_crypto = CryptoReturningOk::default(); let fake_crypto = Arc::new(fake_crypto); let metrics_registry = MetricsRegistry::new(); @@ -124,8 +129,9 @@ fn consensus_produces_expected_batches() { replica_config.subnet_id, vec![( 1, - SubnetRecordBuilder::from(&[node_test_id(0)]) + SubnetRecordBuilder::from(&[node_id]) .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .with_replica_version(replica_config.replica_version.as_ref()) .build(), )], ); @@ -147,6 +153,7 @@ fn consensus_produces_expected_batches() { let consensus_pool = Arc::new(RwLock::new(consensus_pool::ConsensusPoolImpl::new( node_id, subnet_id, + &replica_config.replica_version, make_genesis(summary).into(), pool_config.clone(), MetricsRegistry::new(), @@ -187,8 +194,7 @@ fn consensus_produces_expected_batches() { let consensus_bouncer = ic_consensus::consensus::ConsensusBouncer::new(&metrics_registry, router.clone()); let dkg = ic_consensus_dkg::DkgImpl::new( - replica_config.node_id, - replica_config.subnet_id, + replica_config.clone(), Arc::clone(®istry_client) as Arc<_>, Arc::clone(&state_manager) as Arc<_>, Arc::clone(&fake_crypto) as Arc<_>, diff --git a/rs/consensus/utils/src/lib.rs b/rs/consensus/utils/src/lib.rs index 7143786d0e51..a54afdd79411 100644 --- a/rs/consensus/utils/src/lib.rs +++ b/rs/consensus/utils/src/lib.rs @@ -426,20 +426,6 @@ fn get_subnet_splitting_status_at_given_summary( } } -/// Check if the [`ReplicaVersion`] is the current version -/// -/// # Arguments -/// -/// - `version`: the [`ReplicaVersion`] to check against -/// -/// # Returns -/// -/// - `true` if `version` matches the current version -/// - `false` otherwise -pub fn is_current_protocol_version(version: &ReplicaVersion) -> bool { - version == &ReplicaVersion::default() -} - /// Get the [`SubnetRecord`] of this subnet with the specified [`RegistryVersion`] pub fn get_subnet_record( registry_client: &dyn RegistryClient, diff --git a/rs/crypto/prng/src/tests.rs b/rs/crypto/prng/src/tests.rs index 87611c24e5fe..abd3445bec8f 100644 --- a/rs/crypto/prng/src/tests.rs +++ b/rs/crypto/prng/src/tests.rs @@ -5,11 +5,12 @@ use ic_types::crypto::{ }; use ic_types::signature::ThresholdSignature; use ic_types::{ - Height, + Height, ReplicaVersion, crypto::threshold_sig::ni_dkg::{NiDkgId, NiDkgTag, NiDkgTargetSubnet}, }; use ic_types_test_utils::ids::subnet_test_id; use std::collections::BTreeSet; +use std::str::FromStr; use strum::{EnumCount, IntoEnumIterator}; #[test] @@ -103,6 +104,9 @@ fn fake_random_beacon(height: u64) -> RandomBeacon { content: RandomBeaconContent::new( Height::from(height), CryptoHashOf::new(CryptoHash(vec![])), + // The random beacon is used as a randomness source, so changing the replica version + // can affect the output. + ReplicaVersion::from_str("0.9.0").unwrap(), ), signature: ThresholdSignature { signer: fake_dkg_id(0), diff --git a/rs/crypto/prng/tests/tests.rs b/rs/crypto/prng/tests/tests.rs index fdc9ff52541b..fa61d7bd6fdf 100644 --- a/rs/crypto/prng/tests/tests.rs +++ b/rs/crypto/prng/tests/tests.rs @@ -14,34 +14,11 @@ use ic_types::{ use ic_types_test_utils::ids::subnet_test_id; use rand::RngCore; use std::collections::BTreeSet; +use std::str::FromStr; use strum::EnumCount; -/// Fix ReplicaVersion::default to 0.8.0 -/// -/// Some of the tests, namely those involving the random beacon, end -/// up incorporating the default replica version into the hash. -/// -/// This can change if the crate versions are ever modified. To make these -/// tests immunte to such changes, set ReplicaVersion::default to 0.8.0, -/// or panic if that is not successful. -fn fix_replica_version() { - let fixed_replica_version = - ReplicaVersion::try_from("0.8.0").expect("Failed to create replica version"); - - let _ = ReplicaVersion::set_default_version(fixed_replica_version.clone()); - - // Either we were able to set it, or we were not. If we were not, - // hopefully it is because we already did it previously. - // - // Either way, check that ReplicaVersion::default returns the value we need it to. - - assert_eq!(ReplicaVersion::default(), fixed_replica_version); -} - #[test] fn should_produce_deterministic_randomness_from_random_beacon_and_purpose() { - fix_replica_version(); - let random_beacon = fake_random_beacon(1); let mut rng = Csprng::from_random_beacon_and_purpose(&random_beacon, &BlockmakerRanking); @@ -51,8 +28,6 @@ fn should_produce_deterministic_randomness_from_random_beacon_and_purpose() { #[test] fn should_produce_deterministic_randomness_from_seed_and_purpose() { - fix_replica_version(); - let seed = seed(); let mut rng = Csprng::from_randomness_and_purpose(&seed, &CommitteeSampling); @@ -72,8 +47,6 @@ fn should_offer_methods_of_rng_trait() { #[test] fn should_generate_purpose_specific_randomness_for_random_beacon() { - fix_replica_version(); - let rb = random_beacon(); let mut rng_cs = Csprng::from_random_beacon_and_purpose(&rb, &CommitteeSampling); @@ -108,8 +81,6 @@ fn should_generate_purpose_specific_randomness_for_randomness_seed() { #[test] fn should_produce_different_randomness_for_same_purpose_for_different_random_beacons() { - fix_replica_version(); - let (rb1, rb2) = (random_beacon(), random_beacon_2()); assert_ne!(rb1, rb2); let purpose = CommitteeSampling; @@ -134,8 +105,6 @@ fn should_produce_different_randomness_for_same_purpose_for_different_randomness #[test] fn should_produce_different_randomness_for_different_execution_threads_for_random_beacon() { - fix_replica_version(); - let rb = random_beacon(); let (thread_1, thread_2) = (1, 2); assert_ne!(thread_1, thread_2); @@ -163,6 +132,9 @@ fn fake_random_beacon(height: u64) -> RandomBeacon { content: RandomBeaconContent::new( Height::from(height), CryptoHashOf::new(CryptoHash(vec![])), + // The random beacon is used as a randomness source, so changing the replica version + // can affect the output. + ReplicaVersion::from_str("0.8.0").unwrap(), ), signature: ThresholdSignature { signer: fake_dkg_id(0), diff --git a/rs/crypto/temp_crypto/src/lib.rs b/rs/crypto/temp_crypto/src/lib.rs index 5dbed92d4d5b..711c34330ec8 100644 --- a/rs/crypto/temp_crypto/src/lib.rs +++ b/rs/crypto/temp_crypto/src/lib.rs @@ -7,7 +7,7 @@ use ic_protobuf::registry::subnet::v1::{ use ic_protobuf::types::v1 as pb_types; use ic_registry_client_fake::FakeRegistryClient; use ic_registry_proto_data_provider::ProtoRegistryDataProvider; -use ic_types::{NodeId, ReplicaVersion, SubnetId}; +use ic_types::{NodeId, SubnetId}; use rand::rngs::OsRng; use rand::{CryptoRng, Rng}; use rand_chacha::ChaCha20Rng; @@ -1122,7 +1122,7 @@ impl EcdsaSubnetConfig { max_block_payload_size: 2 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: INITIAL_NOTARY_DELAY.as_millis() as u64, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: "replica_version_for_ecdsa_subnet".to_string(), dkg_interval_length: 59, dkg_dealings_per_block: 1, start_as_nns: false, diff --git a/rs/determinism_test/src/lib.rs b/rs/determinism_test/src/lib.rs index 8168a4f5a4ca..d970bea1d86d 100644 --- a/rs/determinism_test/src/lib.rs +++ b/rs/determinism_test/src/lib.rs @@ -11,16 +11,17 @@ use ic_management_canister_types_private::{ }; use ic_messaging::MessageRoutingImpl; use ic_state_manager::StateManagerImpl; +use ic_test_utilities_types::ids::test_replica_version; use ic_test_utilities_types::messages::SignedIngressBuilder; use ic_types::{ - CanisterId, CryptoHashOfState, Randomness, RegistryVersion, ReplicaVersion, + CanisterId, CryptoHashOfState, Randomness, RegistryVersion, batch::{Batch, BatchContent, BatchMessages, BlockmakerMetrics}, ingress::{IngressState, IngressStatus, WasmResult}, messages::{MessageId, SignedIngress}, time::UNIX_EPOCH, }; use setup::setup; -use std::{convert::TryFrom, sync::Arc, thread::sleep, time::Duration}; +use std::{sync::Arc, thread::sleep, time::Duration}; fn build_batch(message_routing: &dyn MessageRouting, msgs: Vec) -> Batch { Batch { @@ -40,7 +41,7 @@ fn build_batch(message_routing: &dyn MessageRouting, msgs: Vec) - registry_version: RegistryVersion::from(1), time: UNIX_EPOCH, blockmaker_metrics: BlockmakerMetrics::new_for_test(), - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), } } @@ -59,7 +60,7 @@ fn build_batch_with_full_state_hash(message_routing: &dyn MessageRouting) -> Bat registry_version: RegistryVersion::from(1), time: UNIX_EPOCH, blockmaker_metrics: BlockmakerMetrics::new_for_test(), - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), } } diff --git a/rs/determinism_test/src/setup.rs b/rs/determinism_test/src/setup.rs index f194515961e8..c390809612d6 100644 --- a/rs/determinism_test/src/setup.rs +++ b/rs/determinism_test/src/setup.rs @@ -22,7 +22,7 @@ use ic_test_utilities_consensus::fake::FakeVerifier; use ic_test_utilities_registry::{ SubnetRecordBuilder, add_subnet_record, insert_initial_dkg_transcript, }; -use ic_test_utilities_types::ids::subnet_test_id; +use ic_test_utilities_types::ids::{subnet_test_id, test_replica_version}; use ic_types::{ CanisterId, NodeId, PrincipalId, RegistryVersion, SubnetId, malicious_flags::MaliciousFlags, replica_config::ReplicaConfig, @@ -101,6 +101,7 @@ pub(crate) fn setup() -> ( let replica_config = ReplicaConfig { node_id: NodeId::from(PrincipalId::new_node_test_id(27)), subnet_id, + replica_version: test_replica_version(), }; let metrics_registry = MetricsRegistry::new(); diff --git a/rs/engine_controller/BUILD.bazel b/rs/engine_controller/BUILD.bazel index e069082b01ac..12a11c701fc1 100644 --- a/rs/engine_controller/BUILD.bazel +++ b/rs/engine_controller/BUILD.bazel @@ -80,6 +80,7 @@ rust_test( "//rs/registry/keys", "//rs/registry/transport", "//rs/rust_canisters/canister_test", + "//rs/test_utilities/types", "//rs/types/base_types", "//rs/types/types", "@crate_index//:candid", diff --git a/rs/engine_controller/Cargo.toml b/rs/engine_controller/Cargo.toml index 19ea95bd034e..ce31cfcc04ac 100644 --- a/rs/engine_controller/Cargo.toml +++ b/rs/engine_controller/Cargo.toml @@ -31,6 +31,7 @@ ic-nns-test-utils = { path = "../nns/test_utils" } ic-protobuf = { path = "../protobuf" } ic-registry-keys = { path = "../registry/keys" } ic-registry-transport = { path = "../registry/transport" } +ic-test-utilities-types = { path = "../test_utilities/types" } ic-types = { path = "../types/types" } pocket-ic = { path = "../../packages/pocket-ic" } prost = { workspace = true } diff --git a/rs/engine_controller/tests/tests.rs b/rs/engine_controller/tests/tests.rs index 1b207d917ab3..6b231b9f7e92 100644 --- a/rs/engine_controller/tests/tests.rs +++ b/rs/engine_controller/tests/tests.rs @@ -25,7 +25,7 @@ use ic_registry_transport::pb::v1::RegistryAtomicMutateRequest; use ic_registry_transport::pb::v1::RegistryMutation; use ic_registry_transport::pb::v1::high_capacity_registry_get_value_response::Content; use ic_registry_transport::{deserialize_get_value_response, serialize_get_value_request}; -use ic_types::ReplicaVersion; +use ic_test_utilities_types::ids::test_replica_version; use pocket_ic::PocketIcBuilder; use pocket_ic::nonblocking::PocketIc; use prost::Message; @@ -37,11 +37,6 @@ use std::convert::TryFrom; // Must match the principal hard-coded in `engine_controller`. const AUTHORIZED_CALLER: &str = "bct5z-vccu4-6q4t2-3lb6l-wm43p-ulppt-o5sqq-w6het-rthdz-qp4yn-fqe"; -/// Replica version that the registry test fixtures have already elected. -fn test_replica_version() -> String { - ReplicaVersion::default().to_string() -} - fn authorized() -> Principal { Principal::from_text(AUTHORIZED_CALLER).unwrap() } @@ -233,7 +228,7 @@ async fn create_engine_then_delete_engine_succeeds() { let create_args = CreateEngineArgs { node_ids: node_principals(&node_ids), subnet_admins: vec![], - replica_version_id: test_replica_version(), + replica_version_id: test_replica_version().to_string(), }; let new_subnet = call_create_engine(&pic, authorized(), &create_args) @@ -289,7 +284,7 @@ async fn create_engine_caller_must_be_authorized() { let args = CreateEngineArgs { node_ids: node_principals(&node_ids), subnet_admins: vec![], - replica_version_id: test_replica_version(), + replica_version_id: test_replica_version().to_string(), }; let err = call_create_engine(&pic, attacker, &args).await.unwrap_err(); assert!(err.contains("not authorized"), "unexpected error: {err}"); @@ -306,7 +301,7 @@ async fn create_engine_rejects_fewer_than_four_nodes() { &CreateEngineArgs { node_ids: nodes, subnet_admins: vec![], - replica_version_id: test_replica_version(), + replica_version_id: test_replica_version().to_string(), }, ) .await @@ -324,7 +319,7 @@ async fn create_engine_accepts_more_than_four_nodes() { let create_args = CreateEngineArgs { node_ids: node_principals(&node_ids), subnet_admins: vec![], - replica_version_id: test_replica_version(), + replica_version_id: test_replica_version().to_string(), }; let initial_subnets = subnet_list(&pic).await; @@ -351,7 +346,7 @@ async fn create_engine_rejects_duplicates() { &CreateEngineArgs { node_ids: nodes, subnet_admins: vec![], - replica_version_id: test_replica_version(), + replica_version_id: test_replica_version().to_string(), }, ) .await @@ -401,7 +396,7 @@ async fn init_arg_overrides_authorized_caller_and_survives_upgrade() { let args = CreateEngineArgs { node_ids: node_principals(&node_ids), subnet_admins: vec![], - replica_version_id: test_replica_version(), + replica_version_id: test_replica_version().to_string(), }; let err = call_create_engine(&pic, authorized(), &args) .await diff --git a/rs/execution_environment/src/canister_manager/tests.rs b/rs/execution_environment/src/canister_manager/tests.rs index fc18a9d41454..6c6752ff6427 100644 --- a/rs/execution_environment/src/canister_manager/tests.rs +++ b/rs/execution_environment/src/canister_manager/tests.rs @@ -86,7 +86,7 @@ use ic_test_utilities_types::{ }; use ic_types::{ CanisterId, CanisterTimer, ComputeAllocation, MIN_AGGREGATE_LOG_MEMORY_LIMIT, MemoryAllocation, - NumBytes, NumInstructions, SubnetId, UserId, + NumBytes, NumInstructions, ReplicaVersion, SubnetId, UserId, ingress::{IngressState, IngressStatus, WasmResult}, messages::{CanisterCall, StopCanisterCallId, StopCanisterContext}, time::UNIX_EPOCH, @@ -109,6 +109,7 @@ use std::{ io::Write, mem::size_of, path::Path, + str::FromStr, sync::Arc, }; use wirm::wasmparser; @@ -6280,6 +6281,7 @@ fn subnet_info_canister_call_succeeds() { let own_subnet_id = subnet_test_id(1); let mut test = ExecutionTestBuilder::new() .with_own_subnet_id(own_subnet_id) + .with_replica_version(ReplicaVersion::from_str("foobar").unwrap()) .build(); let uni_canister = test .universal_canister_with_cycles(Cycles::new(1_000_000_000_000)) @@ -6304,10 +6306,7 @@ fn subnet_info_canister_call_succeeds() { replica_version, registry_version, } = Decode!(&bytes, SubnetInfoResponse).unwrap(); - assert_eq!( - replica_version, - ic_types::ReplicaVersion::default().to_string() - ); + assert_eq!(replica_version, "foobar"); assert_eq!(registry_version, ic_types::RegistryVersion::default().get()); } diff --git a/rs/execution_environment/src/scheduler/test_utilities.rs b/rs/execution_environment/src/scheduler/test_utilities.rs index 4830dabc2917..b51059d29203 100644 --- a/rs/execution_environment/src/scheduler/test_utilities.rs +++ b/rs/execution_environment/src/scheduler/test_utilities.rs @@ -52,7 +52,7 @@ use ic_test_utilities::state_manager::FakeStateManager; use ic_test_utilities_execution_environment::{generate_subnets, test_registry_settings}; use ic_test_utilities_state::CanisterStateBuilder; use ic_test_utilities_types::{ - ids::{canister_test_id, subnet_test_id, user_test_id}, + ids::{canister_test_id, subnet_test_id, test_replica_version, user_test_id}, messages::{RequestBuilder, SignedIngressBuilder}, }; use ic_types::{ @@ -858,7 +858,7 @@ impl Default for SchedulerTestBuilder { master_public_key_ids: vec![], metrics_registry: MetricsRegistry::new(), round_summary: None, - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), cost_schedule: CanisterCyclesCostSchedule::Normal, subnet_admins: BTreeSet::new(), } @@ -962,13 +962,6 @@ impl SchedulerTestBuilder { } } - pub fn with_replica_version(self, replica_version: ReplicaVersion) -> Self { - Self { - replica_version, - ..self - } - } - pub fn with_cost_schedule(self, cost_schedule: CanisterCyclesCostSchedule) -> Self { Self { cost_schedule, diff --git a/rs/execution_environment/src/scheduler/tests.rs b/rs/execution_environment/src/scheduler/tests.rs index 00fe5d38bcad..065f0af2a87b 100644 --- a/rs/execution_environment/src/scheduler/tests.rs +++ b/rs/execution_environment/src/scheduler/tests.rs @@ -161,7 +161,6 @@ fn consensus_queue_is_emptied() { let ecdsa_key_id = make_ecdsa_key_id(0); let master_ecdsa_key_id = MasterPublicKeyId::Ecdsa(ecdsa_key_id.clone()); let mut test = SchedulerTestBuilder::new() - .with_replica_version(ReplicaVersion::default()) .with_chain_keys(vec![master_ecdsa_key_id.clone()]) .build(); diff --git a/rs/execution_environment/src/scheduler/tests/metrics.rs b/rs/execution_environment/src/scheduler/tests/metrics.rs index d201e83aabe0..73a5fbb7f0d1 100644 --- a/rs/execution_environment/src/scheduler/tests/metrics.rs +++ b/rs/execution_environment/src/scheduler/tests/metrics.rs @@ -885,7 +885,6 @@ fn threshold_signature_agreements_metric_is_updated() { let schnorr_key_id = make_schnorr_key_id(0); let master_schnorr_key_id = MasterPublicKeyId::Schnorr(schnorr_key_id.clone()); let mut test = SchedulerTestBuilder::new() - .with_replica_version(ReplicaVersion::default()) .with_chain_keys(vec![ master_ecdsa_key_id.clone(), master_schnorr_key_id.clone(), diff --git a/rs/http_endpoints/public/src/dashboard.rs b/rs/http_endpoints/public/src/dashboard.rs index 7136c71b7786..73ffac242242 100644 --- a/rs/http_endpoints/public/src/dashboard.rs +++ b/rs/http_endpoints/public/src/dashboard.rs @@ -36,6 +36,7 @@ pub(crate) struct DashboardService { config: Config, subnet_type: SubnetType, state_reader: Arc>, + replica_version: ReplicaVersion, } impl DashboardService { @@ -47,11 +48,13 @@ impl DashboardService { config: Config, subnet_type: SubnetType, state_reader: Arc>, + replica_version: ReplicaVersion, ) -> Router { let state = DashboardService { config, subnet_type, state_reader, + replica_version, }; Router::new().route( DashboardService::route(), @@ -65,6 +68,7 @@ async fn dashboard( config, subnet_type, state_reader, + replica_version, }): State, ) -> impl IntoResponse { let labeled_state = @@ -95,7 +99,7 @@ async fn dashboard( height: labeled_state.height(), replicated_state: labeled_state.get_ref(), canisters: &canisters, - replica_version: ReplicaVersion::default(), + replica_version, }; match dashboard.render() { diff --git a/rs/http_endpoints/public/src/lib.rs b/rs/http_endpoints/public/src/lib.rs index d33ad72071ef..d542bfbe112b 100644 --- a/rs/http_endpoints/public/src/lib.rs +++ b/rs/http_endpoints/public/src/lib.rs @@ -78,7 +78,7 @@ use ic_registry_subnet_type::SubnetType; use ic_replicated_state::ReplicatedState; use ic_tracing::ReloadHandles; use ic_types::{ - Height, NodeId, SubnetId, + Height, NodeId, ReplicaVersion, SubnetId, artifact::UnvalidatedArtifactMutation, malicious_flags::MaliciousFlags, messages::{MessageId, QueryResponseHash, ReplicaHealthStatus, SignedIngress}, @@ -265,6 +265,7 @@ pub fn start_server( ingress_verifier: Arc, node_id: NodeId, subnet_id: SubnetId, + replica_version: ReplicaVersion, nns_subnet_id: SubnetId, log: ReplicaLogger, consensus_pool_cache: Arc, @@ -385,9 +386,14 @@ pub fn start_server( Arc::clone(®istry_client), Arc::clone(&health_status), state_reader.clone(), + replica_version.clone(), + ); + let dashboard_router = DashboardService::new_router( + config.clone(), + subnet_type, + state_reader.clone(), + replica_version, ); - let dashboard_router = - DashboardService::new_router(config.clone(), subnet_type, state_reader.clone()); let catchup_router = CatchUpPackageService::new_router(consensus_pool_cache.clone()); let pprof_home_router = PprofHomeService::new_router(); diff --git a/rs/http_endpoints/public/src/status.rs b/rs/http_endpoints/public/src/status.rs index ae256f551752..08ac72dccdd9 100644 --- a/rs/http_endpoints/public/src/status.rs +++ b/rs/http_endpoints/public/src/status.rs @@ -22,6 +22,7 @@ pub(crate) struct StatusService { registry_client: Arc, replica_health_status: Arc>, state_reader: Arc>, + replica_version: ReplicaVersion, } impl StatusService { @@ -37,6 +38,7 @@ impl StatusService { registry_client: Arc, replica_health_status: Arc>, state_reader: Arc>, + replica_version: ReplicaVersion, ) -> Router { let state = Self { log, @@ -44,6 +46,7 @@ impl StatusService { registry_client, replica_health_status, state_reader, + replica_version, }; Router::new().route_service( StatusService::route(), @@ -75,7 +78,7 @@ pub(crate) async fn status(State(state): State) -> Cbor CanisterHttpResponseMetadat content_hash: crypto_hash(response), content_size: response.content.count_bytes() as u32, is_reject: response.content.is_reject(), - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), } } @@ -516,7 +516,7 @@ fn prop_random_metadata() -> impl Strategy content_hash: CryptoHashOf::new(CryptoHash(hash.to_vec())), content_size, is_reject, - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), } }) } diff --git a/rs/https_outcalls/consensus/src/payload_builder/tests.rs b/rs/https_outcalls/consensus/src/payload_builder/tests.rs index 12d91af824c3..48214b4f8850 100644 --- a/rs/https_outcalls/consensus/src/payload_builder/tests.rs +++ b/rs/https_outcalls/consensus/src/payload_builder/tests.rs @@ -39,11 +39,11 @@ use ic_test_utilities::state_manager::RefMockStateManager; use ic_test_utilities_consensus::fake::FakeContentSigner; use ic_test_utilities_registry::SubnetRecordBuilder; use ic_test_utilities_types::{ - ids::{canister_test_id, node_id_to_u64, node_test_id, subnet_test_id}, + ids::{canister_test_id, node_id_to_u64, node_test_id, subnet_test_id, test_replica_version}, messages::RequestBuilder, }; use ic_types::{ - CountBytes, Height, NodeId, NumBytes, NumberOfNodes, RegistryVersion, ReplicaVersion, + CountBytes, Height, NodeId, NumBytes, NumberOfNodes, RegistryVersion, batch::{ CanisterHttpOutOfCycles, CanisterHttpPayload, FlexibleCanisterHttpError, FlexibleCanisterHttpResponseWithProof, FlexibleCanisterHttpResponses, @@ -1774,7 +1774,7 @@ fn test_response_and_metadata_with_content( content_hash: crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: response.content.is_reject(), - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), }; (response, metadata) } @@ -7839,7 +7839,7 @@ fn metadata_share_with_content_size( content_hash: CryptoHashOf::new(CryptoHash(vec![0xAB; 32])), content_size, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), }; metadata_to_share(signer_node, &metadata) } @@ -7850,7 +7850,7 @@ fn reject_metadata_share(callback_id: u64, signer_node: u64) -> CanisterHttpResp content_hash: CryptoHashOf::new(CryptoHash(vec![0xCD; 32])), content_size: 50, is_reject: true, - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), }; metadata_to_share(signer_node, &metadata) } diff --git a/rs/https_outcalls/consensus/src/payload_builder/utils.rs b/rs/https_outcalls/consensus/src/payload_builder/utils.rs index f98a2fccc041..12673dbc80ef 100644 --- a/rs/https_outcalls/consensus/src/payload_builder/utils.rs +++ b/rs/https_outcalls/consensus/src/payload_builder/utils.rs @@ -1039,8 +1039,9 @@ fn fund_flexible_selection<'a>( mod tests { use super::*; use ic_error_types::RejectCode; + use ic_test_utilities_types::ids::test_replica_version; use ic_types::{ - CanisterId, NumberOfNodes, ReplicaVersion, + CanisterId, NumberOfNodes, canister_http::{ CANDID_OVERHEAD_RESERVE_BYTES, CanisterHttpMethod, CanisterHttpReject, MAX_CANISTER_HTTP_RESPONSE_BYTES, MAX_HTTP_OUTCALL_SPEND_FREE_SUBNET, @@ -1167,7 +1168,7 @@ mod tests { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size, is_reject, - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), }; check_content_size_within_limit(&metadata, callback_id, context) } diff --git a/rs/https_outcalls/consensus/src/pool_manager.rs b/rs/https_outcalls/consensus/src/pool_manager.rs index 84d9c5546801..7135820ad684 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -5,7 +5,6 @@ use crate::metrics::CanisterHttpPoolManagerMetrics; use ic_consensus_utils::{ crypto::ConsensusCrypto, - is_current_protocol_version, membership::{Membership, MembershipError}, }; use ic_interfaces::{ @@ -22,7 +21,7 @@ use ic_registry_client_helpers::subnet::SubnetRegistry; use ic_registry_subnet_type::SubnetType; use ic_replicated_state::ReplicatedState; use ic_types::{ - CountBytes, NodeId, ReplicaVersion, canister_http::*, crypto::Signed, messages::CallbackId, + CountBytes, NodeId, canister_http::*, crypto::Signed, messages::CallbackId, replica_config::ReplicaConfig, }; use std::{ @@ -371,7 +370,7 @@ impl CanisterHttpPoolManagerImpl { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: response.content.is_reject(), - replica_version: ReplicaVersion::default(), + replica_version: self.replica_config.replica_version.clone(), }, payment_receipt, }; @@ -453,7 +452,7 @@ impl CanisterHttpPoolManagerImpl { let share = &artifact.share; // Reject shares from different replica versions - if !is_current_protocol_version(share.content.replica_version()) { + if share.content.replica_version() != &self.replica_config.replica_version { return Some(CanisterHttpChangeAction::RemoveUnvalidated(share.clone())); } @@ -657,8 +656,9 @@ pub mod test { use ic_registry_keys::{make_api_boundary_node_record_key, make_node_record_key}; use ic_replicated_state::metadata_state::subnet_call_context_manager::SubnetCallContext; use ic_test_utilities_logger::with_test_replica_logger; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; + use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, test_replica_version}; use ic_types::CountBytes; + use ic_types::ReplicaVersion; use ic_types::crypto::crypto_hash; use ic_types::{ Height, NodeId, NumBytes, NumberOfNodes, RegistryVersion, @@ -803,7 +803,7 @@ pub mod test { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -899,7 +899,7 @@ pub mod test { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -1109,7 +1109,7 @@ pub mod test { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -1236,7 +1236,7 @@ pub mod test { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -1425,7 +1425,7 @@ pub mod test { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -1522,7 +1522,7 @@ pub mod test { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -1659,7 +1659,7 @@ pub mod test { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -1724,7 +1724,7 @@ pub mod test { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -1827,7 +1827,7 @@ pub mod test { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: true, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -1916,7 +1916,7 @@ pub mod test { content_hash: dishonest_hash, content_size: dishonest_response.content.count_bytes() as u32, is_reject: true, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -2020,7 +2020,7 @@ pub mod test { content_hash: CryptoHashOf::new(CryptoHash(vec![0xAB; 32])), content_size: limit as u32 + 1, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -2245,7 +2245,7 @@ pub mod test { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: true, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -2322,7 +2322,7 @@ pub mod test { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -2729,7 +2729,7 @@ pub mod test { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -2895,7 +2895,7 @@ pub mod test { content_hash: crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -2993,7 +2993,7 @@ pub mod test { content_hash: crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -3218,7 +3218,7 @@ pub mod test { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -3281,7 +3281,7 @@ pub mod test { content_hash: ic_types::crypto::crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -3456,7 +3456,7 @@ pub mod test { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt { spent: Cycles::new(200), @@ -3563,7 +3563,7 @@ pub mod test { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_config.replica_version.clone(), }, payment_receipt: CanisterHttpPaymentReceipt { spent: Cycles::new(200), @@ -3785,7 +3785,7 @@ pub mod test { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; @@ -3885,7 +3885,7 @@ pub mod test { content_hash: crypto_hash(&response), content_size: response.content.count_bytes() as u32, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), }, payment_receipt: CanisterHttpPaymentReceipt::default(), }; diff --git a/rs/https_outcalls/pricing/BUILD.bazel b/rs/https_outcalls/pricing/BUILD.bazel index 8ad85488577b..f60239fec02b 100644 --- a/rs/https_outcalls/pricing/BUILD.bazel +++ b/rs/https_outcalls/pricing/BUILD.bazel @@ -22,5 +22,8 @@ rust_library( rust_test( name = "pricing_test", crate = ":pricing", - deps = [], + deps = [ + # Keep sorted. + "//rs/types/types_test_utils", + ], ) diff --git a/rs/https_outcalls/pricing/Cargo.toml b/rs/https_outcalls/pricing/Cargo.toml index c3eeb858fe1e..1b9b18f7280a 100644 --- a/rs/https_outcalls/pricing/Cargo.toml +++ b/rs/https_outcalls/pricing/Cargo.toml @@ -14,3 +14,6 @@ ic-types = { path = "../../types/types" } ic-types-cycles = { path = "../../types/cycles" } prometheus = { workspace = true } slog = { workspace = true } + +[dev-dependencies] +ic-types-test-utils = { path = "../../types/types_test_utils" } diff --git a/rs/https_outcalls/pricing/src/fees.rs b/rs/https_outcalls/pricing/src/fees.rs index da4327be370c..53f22180a53e 100644 --- a/rs/https_outcalls/pricing/src/fees.rs +++ b/rs/https_outcalls/pricing/src/fees.rs @@ -519,7 +519,7 @@ mod tests { //! only compare these functions against themselves and would not.) use super::*; use ic_types::{ - NodeId, PrincipalId, ReplicaVersion, + NodeId, PrincipalId, canister_http::{ CanisterHttpPaymentReceipt, CanisterHttpResponseMetadata, CanisterHttpResponseProof, CanisterHttpResponseReceipt, CanisterHttpResponseShare, CanisterHttpResponseSignature, @@ -529,6 +529,7 @@ mod tests { signature::BasicSignature, }; use ic_types_cycles::CanisterCyclesCostSchedule; + use ic_types_test_utils::ids::test_replica_version; use std::collections::BTreeMap; fn node(i: u64) -> NodeId { @@ -541,7 +542,7 @@ mod tests { content_hash: CryptoHashOf::new(CryptoHash(vec![])), content_size, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), } } diff --git a/rs/ic_os/guest_upgrade/tests/src/lib.rs b/rs/ic_os/guest_upgrade/tests/src/lib.rs index 3f1ebc6f0236..96259e007cc4 100644 --- a/rs/ic_os/guest_upgrade/tests/src/lib.rs +++ b/rs/ic_os/guest_upgrade/tests/src/lib.rs @@ -24,6 +24,7 @@ use sev_guest::key_deriver::{Key, derive_key_from_sev_measurement}; use sev_guest_testing::{FakeAttestationReportSigner, MockSevGuestFirmwareBuilder}; use std::future::Future; use std::net::Ipv6Addr; +use std::str::FromStr; use std::sync::Arc; use std::sync::atomic::{AtomicU16, Ordering}; use std::time::Duration; @@ -561,7 +562,7 @@ async fn test_replica_version_not_in_registry() { let fixture = DiskEncryptionKeyExchangeTestFixture::new(TestConfig::default()); let result = fixture .create_server_agent(MockVSockClient::default()) - .exchange_keys(&ReplicaVersion::try_from("replica_version_missing").unwrap()) + .exchange_keys(&ReplicaVersion::from_str("replica_version_missing").unwrap()) .await .expect_err("Key exchange should fail when the target replica version is missing"); diff --git a/rs/messaging/src/message_routing/tests.rs b/rs/messaging/src/message_routing/tests.rs index 3044c2bfb641..8f1be7c1e10e 100644 --- a/rs/messaging/src/message_routing/tests.rs +++ b/rs/messaging/src/message_routing/tests.rs @@ -31,7 +31,9 @@ use ic_test_utilities_metrics::{fetch_int_counter_vec, fetch_int_gauge_vec, metr use ic_test_utilities_registry::{SubnetRecordBuilder, get_mainnet_delta_00_6d_c1}; use ic_test_utilities_state::CanisterStateBuilder; use ic_test_utilities_types::batch::BatchBuilder; -use ic_test_utilities_types::ids::{canister_test_id, node_test_id, subnet_test_id, user_test_id}; +use ic_test_utilities_types::ids::{ + canister_test_id, node_test_id, subnet_test_id, test_replica_version, user_test_id, +}; use ic_types::batch::{Batch, BatchMessages, BlockmakerMetrics}; use ic_types::crypto::AlgorithmId; use ic_types::crypto::threshold_sig::ni_dkg::{NiDkgTag, NiDkgTranscript}; @@ -39,7 +41,6 @@ use ic_types::time::Time; use ic_types::xnet::{StreamIndexedQueue, StreamSlice}; use ic_types::{ CanisterId, ExecutionRound, NodeId, NumBytes, NumInstructions, PrincipalId, Randomness, - ReplicaVersion, }; use maplit::{btreemap, btreeset}; use std::{fmt::Debug, str::FromStr, sync::Arc, time::Duration}; @@ -1046,7 +1047,7 @@ fn try_read_registry_succeeds_with_fully_specified_registry_records() { registry_version: fixture.registry.get_latest_version(), time: Time::from_nanos_since_unix_epoch(0), blockmaker_metrics: BlockmakerMetrics::new_for_test(), - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), }); let latest_state = state_manager.get_latest_state().take(); assert_eq!( @@ -2423,7 +2424,7 @@ fn process_batch_updates_subnet_metrics() { registry_version: fixture.registry.get_latest_version(), time: Time::from_nanos_since_unix_epoch(0), blockmaker_metrics: BlockmakerMetrics::new_for_test(), - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), }); let latest_state = state_manager.get_latest_state().take(); @@ -2494,7 +2495,7 @@ fn process_batch_resets_split_marker() { registry_version: fixture.registry.get_latest_version(), time: Time::from_nanos_since_unix_epoch(1), blockmaker_metrics: BlockmakerMetrics::new_for_test(), - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), }); // The subnet split marker was reset. diff --git a/rs/messaging/src/state_machine/tests.rs b/rs/messaging/src/state_machine/tests.rs index c088fcc96f8e..dd9a43ad5418 100644 --- a/rs/messaging/src/state_machine/tests.rs +++ b/rs/messaging/src/state_machine/tests.rs @@ -34,6 +34,7 @@ use ic_types_cycles::{CanisterCyclesCostSchedule, Cycles, CyclesUseCase}; use maplit::btreemap; use mockall::{Sequence, mock, predicate::*}; use std::collections::{BTreeMap, BTreeSet}; +use std::str::FromStr; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -820,7 +821,7 @@ fn test_online_split(new_subnet_id: SubnetId, other_subnet_id: SubnetId) -> Repl .checked_add(Duration::from_secs(1)) .unwrap(), blockmaker_metrics: BlockmakerMetrics::new_for_test(), - replica_version: ReplicaVersion::default(), + replica_version: ReplicaVersion::from_str("foo").unwrap(), }; let state_after_split = with_test_replica_logger(|log| { diff --git a/rs/nns/integration_tests/BUILD.bazel b/rs/nns/integration_tests/BUILD.bazel index 1d817a4e6a17..02060517bd26 100644 --- a/rs/nns/integration_tests/BUILD.bazel +++ b/rs/nns/integration_tests/BUILD.bazel @@ -62,6 +62,7 @@ BASE_DEPENDENCIES = [ "//rs/rust_canisters/xrc_mock", "//rs/state_machine_tests", "//rs/test_utilities", + "//rs/test_utilities/types", "//packages/ic-error-types", "//rs/types/management_canister_types", "//rs/types/types", diff --git a/rs/nns/integration_tests/Cargo.toml b/rs/nns/integration_tests/Cargo.toml index 26d576a79a90..9ba6b57ead9e 100644 --- a/rs/nns/integration_tests/Cargo.toml +++ b/rs/nns/integration_tests/Cargo.toml @@ -97,6 +97,7 @@ ic-sns-init = { path = "../../sns/init" } ic-sns-wasm = { path = "../sns-wasm" } ic-state-machine-tests = { path = "../../state_machine_tests" } ic-test-utilities = { path = "../../test_utilities" } +ic-test-utilities-types = { path = "../../test_utilities/types" } ic-types-cycles = { path = "../../types/cycles" } ic-types = { path = "../../types/types" } ic-types-test-utils = { path = "../../types/types_test_utils" } diff --git a/rs/nns/integration_tests/src/subnet_handler.rs b/rs/nns/integration_tests/src/subnet_handler.rs index e0f04f911a52..bad0190e8681 100644 --- a/rs/nns/integration_tests/src/subnet_handler.rs +++ b/rs/nns/integration_tests/src/subnet_handler.rs @@ -17,7 +17,7 @@ use ic_protobuf::registry::subnet::v1::{CanisterCyclesCostSchedule, SubnetRecord use ic_registry_keys::make_subnet_record_key; use ic_registry_subnet_type::SubnetType; use ic_registry_transport::{insert, pb::v1::RegistryAtomicMutateRequest}; -use ic_types::ReplicaVersion; +use ic_types_test_utils::ids::test_replica_version; use prost::Message; use registry_canister::mutations::do_update_subnet::UpdateSubnetPayload; use std::str::FromStr; @@ -40,7 +40,7 @@ fn test_submit_and_accept_update_subnet_proposal() { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: INITIAL_NOTARY_DELAY.as_millis() as u64, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, @@ -165,7 +165,7 @@ fn test_submit_and_accept_update_subnet_proposal() { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: INITIAL_NOTARY_DELAY.as_millis() as u64, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 10, dkg_dealings_per_block: 1, start_as_nns: false, 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 5bdf377481f9..19e53f416503 100644 --- a/rs/nns/integration_tests/src/update_unassigned_nodes_config.rs +++ b/rs/nns/integration_tests/src/update_unassigned_nodes_config.rs @@ -13,7 +13,7 @@ use ic_nns_test_utils::{ }; use ic_protobuf::registry::unassigned_nodes_config::v1::UnassignedNodesConfigRecord; use ic_registry_keys::make_unassigned_nodes_config_record_key; -use ic_types::ReplicaVersion; +use ic_test_utilities_types::ids::test_replica_version; use registry_canister::mutations::{ do_deploy_guestos_to_all_unassigned_nodes::DeployGuestosToAllUnassignedNodesPayload, do_update_ssh_readonly_access_for_all_unassigned_nodes::UpdateSshReadOnlyAccessForAllUnassignedNodesPayload, @@ -29,9 +29,8 @@ fn test_submit_update_ssh_readonly_access_for_all_unassigned_nodes() { let nns_canisters = NnsCanisters::set_up(&runtime, nns_init_payload).await; // 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(), + elected_replica_version: test_replica_version().to_string(), }; let proposal_id: ProposalId = submit_external_update_proposal( @@ -143,10 +142,8 @@ fn test_submit_deploy_guestos_to_all_unassigned_nodes_proposal() { .build(); let nns_canisters = NnsCanisters::set_up(&runtime, nns_init_payload).await; - let replica_version = ReplicaVersion::default().to_string(); - let payload = DeployGuestosToAllUnassignedNodesPayload { - elected_replica_version: replica_version.clone(), + elected_replica_version: test_replica_version().to_string(), }; let proposal_id: ProposalId = submit_external_update_proposal( @@ -179,7 +176,10 @@ fn test_submit_deploy_guestos_to_all_unassigned_nodes_proposal() { ) .await; - assert_eq!(unassigned_nodes_config.replica_version, replica_version); + assert_eq!( + unassigned_nodes_config.replica_version, + test_replica_version().as_ref() + ); Ok(()) }) diff --git a/rs/nns/integration_tests/src/upgrades_handler.rs b/rs/nns/integration_tests/src/upgrades_handler.rs index 6739ada5d543..56497cbcf656 100644 --- a/rs/nns/integration_tests/src/upgrades_handler.rs +++ b/rs/nns/integration_tests/src/upgrades_handler.rs @@ -19,7 +19,7 @@ use ic_protobuf::registry::replica_version::v1::{ ReplicaVersionRecord, }; use ic_registry_keys::make_replica_version_key; -use ic_types::ReplicaVersion; +use ic_test_utilities_types::ids::test_replica_version; use lazy_static::lazy_static; use registry_canister::mutations::{ do_deploy_guestos_to_all_unassigned_nodes::DeployGuestosToAllUnassignedNodesPayload, @@ -115,7 +115,7 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() { gov.update_from_sender("forward_vote", candid, input, &sender) }; - let default_version = &ReplicaVersion::default().to_string(); + let default_version = test_replica_version().to_string(); let unassigned_nodes_version = "unassigned_nodes_version"; let version_to_elect_and_unelect1 = "version_to_elect_and_unelect1"; let version_to_elect_and_unelect2 = "version_to_elect_and_unelect2"; @@ -139,14 +139,20 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() { // Check state of elected versions for version in [ - default_version, + &default_version, version_to_elect_and_unelect1, version_to_elect_and_unelect2, unassigned_nodes_version, ] { - assert!(is_elected_version(&nns_canisters.registry, version).await); + assert!( + is_elected_version(&nns_canisters.registry, version).await, + "Expected {version} to be elected" + ); } - assert!(!is_elected_version(&nns_canisters.registry, version_to_elect).await); + assert!( + !is_elected_version(&nns_canisters.registry, version_to_elect).await, + "Did not expect {version_to_elect} to be elected" + ); // update unassigned version let deploy_unassigned_payload = DeployGuestosToAllUnassignedNodesPayload { @@ -174,7 +180,7 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() { Some("Key not present"), ), ( - retire_version_payload(vec![version_to_elect_and_unelect1, default_version]), + retire_version_payload(vec![version_to_elect_and_unelect1, &default_version]), Some("Using a version that isn't elected"), ), ( @@ -245,7 +251,7 @@ fn test_submit_and_accept_update_elected_replica_versions_proposal() { } // Check state of elected versions - for version in [default_version, unassigned_nodes_version, version_to_elect] { + 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] { diff --git a/rs/nns/test_utils/src/registry.rs b/rs/nns/test_utils/src/registry.rs index 2e73426004ef..a7fc2b0ab495 100644 --- a/rs/nns/test_utils/src/registry.rs +++ b/rs/nns/test_utils/src/registry.rs @@ -52,8 +52,9 @@ use ic_registry_transport::{ serialize_get_value_request, }; use ic_test_utilities_types::ids::subnet_test_id; +use ic_test_utilities_types::ids::test_replica_version; use ic_types::{ - NodeId, ReplicaVersion, + NodeId, crypto::{ CurrentNodePublicKeys, KeyPurpose, threshold_sig::ni_dkg::{NiDkgTag, NiDkgTargetId, NiDkgTranscript}, @@ -347,7 +348,7 @@ pub fn invariant_compliant_mutation_with_subnet_id( }; const MOCK_HASH: &str = "deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef"; let release_package_url = "http://release_package.tar.zst".to_string(); - let replica_version_id = ReplicaVersion::default().to_string(); + let replica_version_id = test_replica_version().to_string(); let replica_version = ReplicaVersionRecord { replica_version_id: Some(replica_version_id.clone()), release_package_sha256_hex: MOCK_HASH.into(), @@ -607,7 +608,7 @@ pub fn initial_mutations_for_a_multinode_nns_subnet() -> Vec { add_node_mutations.append(&mut mutations); } - let replica_version_id = ReplicaVersion::default().to_string(); + let replica_version_id = test_replica_version().to_string(); const MOCK_HASH: &str = "abbaabbaabbaabbaabbaabbaabbaabbaabbaabbaabbaabbaabbaabbaabbaabba"; let release_package_url = "http://release_package.tar.zst".to_string(); let guest_launch_measurements = Some(GuestLaunchMeasurements { @@ -750,11 +751,9 @@ pub fn prepare_registry_with_two_node_sets( num_nodes_in_subnet2, ); - let replica_version = ReplicaVersion::default(); - // Subnet record 1 let subnet_record = SubnetRecord { - replica_version_id: replica_version.to_string(), + replica_version_id: test_replica_version().to_string(), membership: node_ids_and_dkg_keys_subnet_1 .keys() .map(|id| id.get().into_vec()) @@ -784,7 +783,7 @@ pub fn prepare_registry_with_two_node_sets( if assign_nodes_to_subnet2 { // Subnet record 2 let subnet2_record = SubnetRecord { - replica_version_id: replica_version.to_string(), + replica_version_id: test_replica_version().to_string(), membership: node_ids_and_dkg_keys_subnet_2 .keys() .map(|id| id.get().into_vec()) diff --git a/rs/orchestrator/registry_replicator/BUILD.bazel b/rs/orchestrator/registry_replicator/BUILD.bazel index 8627899d421c..45ea75327985 100644 --- a/rs/orchestrator/registry_replicator/BUILD.bazel +++ b/rs/orchestrator/registry_replicator/BUILD.bazel @@ -61,6 +61,7 @@ rust_test( "//rs/registry/fake", "//rs/registry/proto_data_provider", "//rs/test_utilities/logger", + "//rs/test_utilities/types", "//rs/types/types_test_utils", "@crate_index//:rand", "@crate_index//:rstest", diff --git a/rs/orchestrator/registry_replicator/Cargo.toml b/rs/orchestrator/registry_replicator/Cargo.toml index 21bd07cc80cc..a10dbaf08c91 100644 --- a/rs/orchestrator/registry_replicator/Cargo.toml +++ b/rs/orchestrator/registry_replicator/Cargo.toml @@ -43,6 +43,7 @@ ic-registry-client-fake = { path = "../../registry/fake" } ic-registry-proto-data-provider = { path = "../../registry/proto_data_provider" } ic-registry-transport = { path = "../../registry/transport" } ic-test-utilities-logger = { path = "../../test_utilities/logger" } +ic-test-utilities-types = { path = "../../test_utilities/types" } ic-types-test-utils = { path = "../../types/types_test_utils" } pocket-ic = { path = "../../../packages/pocket-ic" } rand = { workspace = true } diff --git a/rs/orchestrator/registry_replicator/src/internal_state.rs b/rs/orchestrator/registry_replicator/src/internal_state.rs index 72601639b90c..8aca863d3c0b 100644 --- a/rs/orchestrator/registry_replicator/src/internal_state.rs +++ b/rs/orchestrator/registry_replicator/src/internal_state.rs @@ -697,6 +697,7 @@ mod test { use ic_registry_proto_data_provider::ProtoRegistryDataProvider; use ic_registry_routing_table::{CanisterIdRange, RoutingTable}; use ic_test_utilities_logger::with_test_replica_logger; + use ic_test_utilities_types::ids::test_replica_version; use ic_types::{CanisterId, SubnetId}; use ic_types_test_utils::ids::{NODE_1, NODE_2, NODE_3, SUBNET_1, SUBNET_2, SUBNET_3}; use rstest::rstest; @@ -726,7 +727,7 @@ mod test { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: "test_version".to_string(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns, diff --git a/rs/orchestrator/src/registry_helper_tests.rs b/rs/orchestrator/src/registry_helper_tests.rs index 934b3fce72aa..fb7689b9a2b2 100644 --- a/rs/orchestrator/src/registry_helper_tests.rs +++ b/rs/orchestrator/src/registry_helper_tests.rs @@ -6,6 +6,7 @@ use ic_registry_keys::{make_standard_engine_replica_version_record_key, make_sub use ic_registry_proto_data_provider::ProtoRegistryDataProvider; use ic_types::PrincipalId; use lazy_static::lazy_static; +use std::str::FromStr; fn subnet_id(id: u64) -> SubnetId { SubnetId::from(PrincipalId::new_subnet_test_id(id)) @@ -52,7 +53,7 @@ fn test_get_replica_version_when_specified_directly() { // Step 3: Verify result(s). assert_eq!( result.unwrap(), - ReplicaVersion::try_from("some_version").unwrap(), + ReplicaVersion::from_str("some_version").unwrap(), ); } diff --git a/rs/orchestrator/src/upgrade.rs b/rs/orchestrator/src/upgrade.rs index 7ba3842002af..c1db8e02ed75 100644 --- a/rs/orchestrator/src/upgrade.rs +++ b/rs/orchestrator/src/upgrade.rs @@ -1161,6 +1161,7 @@ mod tests { use rand::RngCore; use rstest::rstest; use slog::Level; + use std::str::FromStr; use std::{ collections::{BTreeMap, BTreeSet}, ffi::OsStr, @@ -1233,6 +1234,7 @@ mod tests { // Helper function to create a CUP with given height and summary payload. fn make_cup_with_summary(height: Height, summary_payload: SummaryPayload) -> CatchUpPackage { + let replica_version = ReplicaVersion::from_str("replica_version_for_cup").unwrap(); let block = Block::new( CryptoHashOf::from(CryptoHash(Vec::new())), Payload::new( @@ -1246,6 +1248,7 @@ mod tests { certified_height: Height::from(42), time: UNIX_EPOCH, }, + replica_version.clone(), ); CatchUpPackage::fake(CatchUpContent::new( @@ -1255,6 +1258,7 @@ mod tests { RandomBeacon::fake(RandomBeaconContent::new( height, CryptoHashOf::from(CryptoHash(Vec::new())), + replica_version, )), ), CryptoHashOf::from(CryptoHash(Vec::new())), @@ -2049,7 +2053,7 @@ mod tests { add_replica_version_to_provider( &data_provider, max_registry_version, - &ReplicaVersion::try_from("dummy_replica_version").unwrap(), + &ReplicaVersion::from_str("dummy_replica_version").unwrap(), ); data_provider @@ -2726,7 +2730,7 @@ mod tests { async fn test_upgrade_scenarios( #[values(NODE_1)] node_id: NodeId, #[values(SubnetType::Application, SubnetType::CloudEngine)] subnet_type: SubnetType, - #[values(ReplicaVersion::try_from("replica_version_0.1").unwrap())] current_replica_version: ReplicaVersion, + #[values(ReplicaVersion::from_str("replica_version_0.1").unwrap())] current_replica_version: ReplicaVersion, #[values( None, Some(CUPScenario { @@ -2781,7 +2785,7 @@ mod tests { )] is_leaving: Option, #[values(false, true)] does_upgrade: bool, - #[values(ReplicaVersion::try_from("replica_version_0.2").unwrap())] upgrade_replica_version: ReplicaVersion, + #[values(ReplicaVersion::from_str("replica_version_0.2").unwrap())] upgrade_replica_version: ReplicaVersion, #[values( RegistryVersion::from(3), RegistryVersion::from(5), @@ -2850,7 +2854,7 @@ mod tests { let test_scenario = UpgradeTestScenario { node_id: NODE_1, subnet_type: SubnetType::System, - current_replica_version: ReplicaVersion::try_from("replica_version_0.1").unwrap(), + current_replica_version: ReplicaVersion::from_str("replica_version_0.1").unwrap(), has_local_cup: Some(CUPScenario { height: Height::from(100), // Set as the NNS subnet in `setup_registry` @@ -2861,7 +2865,7 @@ mod tests { initial_subnet_assignment: SubnetAssignment::Unknown, is_leaving: None, upgrade_to: Some(ReplicaUpgradeScenario { - replica_version: ReplicaVersion::try_from("replica_version_0.2").unwrap(), + replica_version: ReplicaVersion::from_str("replica_version_0.2").unwrap(), registry_version: RegistryVersion::from(10), is_recalled: true, has_replicated_versions_before_init: false, @@ -2893,7 +2897,7 @@ mod tests { let test_scenario = UpgradeTestScenario { node_id: NODE_1, subnet_type: SubnetType::Application, - current_replica_version: ReplicaVersion::try_from("replica_version_0.1").unwrap(), + current_replica_version: ReplicaVersion::from_str("replica_version_0.1").unwrap(), has_local_cup: Some(CUPScenario { height: Height::from(100), subnet_id: SUBNET_1, @@ -2903,7 +2907,7 @@ mod tests { initial_subnet_assignment: SubnetAssignment::Unknown, is_leaving: None, upgrade_to: Some(ReplicaUpgradeScenario { - replica_version: ReplicaVersion::try_from("replica_version_0.2").unwrap(), + replica_version: ReplicaVersion::from_str("replica_version_0.2").unwrap(), registry_version: RegistryVersion::from(10), is_recalled: true, has_replicated_versions_before_init: false, diff --git a/rs/p2p/artifact_downloader/benches/assembler.rs b/rs/p2p/artifact_downloader/benches/assembler.rs index 9c6a5b4456aa..658945aa8010 100644 --- a/rs/p2p/artifact_downloader/benches/assembler.rs +++ b/rs/p2p/artifact_downloader/benches/assembler.rs @@ -36,7 +36,7 @@ use ic_types::{ signature::{BasicSignature, BasicSignatureBatch}, time::UNIX_EPOCH, }; -use ic_types_test_utils::ids::{NODE_1, NODE_2, SUBNET_0, node_test_id}; +use ic_types_test_utils::ids::{NODE_1, NODE_2, SUBNET_0, node_test_id, test_replica_version}; use tokio::runtime::{Handle, Runtime}; struct FakeIngressPool { @@ -135,6 +135,7 @@ fn fake_block_proposal_with_ingresses_and_idkg_dealings( parent.as_ref().height.increment(), Rank(0), parent.as_ref().context.clone(), + test_replica_version(), ); ConsensusMessage::BlockProposal(BlockProposal::fake(block, NODE_1)) diff --git a/rs/p2p/artifact_downloader/src/fetch_stripped_artifact/download.rs b/rs/p2p/artifact_downloader/src/fetch_stripped_artifact/download.rs index 5c57722d2ee4..be48ee79399d 100644 --- a/rs/p2p/artifact_downloader/src/fetch_stripped_artifact/download.rs +++ b/rs/p2p/artifact_downloader/src/fetch_stripped_artifact/download.rs @@ -446,6 +446,7 @@ mod tests { use ic_metrics::MetricsRegistry; use ic_p2p_test_utils::mocks::{MockPeers, MockTransport, MockValidatedPoolReader}; use ic_test_utilities_consensus::fake::{FakeContent, FakeContentSigner}; + use ic_test_utilities_types::ids::test_replica_version; use ic_test_utilities_types::messages::SignedIngressBuilder; use ic_types::{ Height, @@ -768,9 +769,12 @@ mod tests { async fn rpc_get_idkg_dealing_from_consensus_pool_wrong_consensus_id_test() { let node_index = 1; let dealing = SignedIDkgDealing::fake(dummy_idkg_dealing_for_tests(), NODE_1); - let finalization = ConsensusMessage::Finalization(Finalization::fake( - FinalizationContent::new(Height::new(100), CryptoHashOf::from(CryptoHash(vec![]))), - )); + let finalization = + ConsensusMessage::Finalization(Finalization::fake(FinalizationContent::new( + Height::new(100), + CryptoHashOf::from(CryptoHash(vec![])), + test_replica_version(), + ))); let pools = mock_pools( PoolMessage::None, None, diff --git a/rs/p2p/artifact_downloader/src/fetch_stripped_artifact/test_utils.rs b/rs/p2p/artifact_downloader/src/fetch_stripped_artifact/test_utils.rs index 55d552dad718..361fce0f8822 100644 --- a/rs/p2p/artifact_downloader/src/fetch_stripped_artifact/test_utils.rs +++ b/rs/p2p/artifact_downloader/src/fetch_stripped_artifact/test_utils.rs @@ -1,8 +1,3 @@ -use std::{ - collections::{BTreeMap, BTreeSet}, - sync::Arc, -}; - use ic_crypto_test_utils_canister_threshold_sigs::dummy_values::dummy_idkg_dealing_for_tests; use ic_protobuf::types::v1 as pb; use ic_test_utilities_consensus::{ @@ -36,7 +31,11 @@ use ic_types::{ signature::BasicSignatureBatch, time::UNIX_EPOCH, }; -use ic_types_test_utils::ids::{NODE_1, NODE_2, SUBNET_0, node_test_id}; +use ic_types_test_utils::ids::{NODE_1, NODE_2, SUBNET_0, node_test_id, test_replica_version}; +use std::{ + collections::{BTreeMap, BTreeSet}, + sync::Arc, +}; use crate::fetch_stripped_artifact::types::{ StrippedMessage, StrippedMessageId, stripped::StrippedIDkgDealings, @@ -150,6 +149,7 @@ pub(crate) fn fake_block_proposal_with_ingresses_and_idkg( parent.as_ref().height.increment(), Rank(0), parent.as_ref().context.clone(), + test_replica_version(), ); BlockProposal::fake(block, node_test_id(0)) } diff --git a/rs/prep/BUILD.bazel b/rs/prep/BUILD.bazel index 687c460ffc1f..7f5c7264f4bd 100644 --- a/rs/prep/BUILD.bazel +++ b/rs/prep/BUILD.bazel @@ -179,6 +179,7 @@ rust_test( "//rs/registry/subnet_type", "//rs/registry/transport", "//rs/state_manager", + "//rs/test_utilities/types", "//rs/types/management_canister_types", "//rs/types/types", "@crate_index//:anyhow", diff --git a/rs/prep/Cargo.toml b/rs/prep/Cargo.toml index 90fb65e4ca3a..5260e196a46b 100644 --- a/rs/prep/Cargo.toml +++ b/rs/prep/Cargo.toml @@ -51,6 +51,7 @@ x509-cert = { workspace = true } [dev-dependencies] assert_matches = { workspace = true } +ic-test-utilities-types = { path = "../test_utilities/types" } pretty_assertions = { workspace = true } [lib] diff --git a/rs/prep/src/bin/prep.rs b/rs/prep/src/bin/prep.rs index 5ad66ccd7d5a..4e40124babe1 100644 --- a/rs/prep/src/bin/prep.rs +++ b/rs/prep/src/bin/prep.rs @@ -28,14 +28,13 @@ use ic_types::{Height, PrincipalId, ReplicaVersion}; /// the filename of the update disk image, as published on the cdn const UPD_IMG_FILENAME: &str = "update-img.tar.zst"; -/// in case the replica version id is specified on the command line, but not the -/// release package url and hash, the following url-template will be used to -/// fetch the sha256 of the corresponding image. +/// in case the release package url and hash are not specified on the command +/// line, the following url-template will be used to fetch the sha256 of the +/// corresponding image. const UPD_IMG_DEFAULT_SHA256_URL: &str = "https://download.dfinity.systems/ic//guest-os/update-img-dev/SHA256SUMS"; -/// in case the replica version id is specified on the command line, but not the -/// release package url and hash, the following url-template will be used to -/// specify the update image. +/// in case the release package url and hash are not specified on the command +/// line, the following url-template will be used to specify the update image. const UPD_IMG_DEFAULT_URL: &str = "https://download.dfinity.systems/ic//guest-os/update-img-dev/update-img.tar.zst"; const CDN_HTTP_ATTEMPTS: usize = 3; const RETRY_BACKOFF: Duration = Duration::from_secs(5); @@ -49,14 +48,13 @@ const REQUEST_TIMEOUT: Duration = Duration::from_secs(12); struct CliArgs { /// The version of the Replica being run #[clap(long)] - pub replica_version: Option, + pub replica_version: ReplicaVersion, /// The URL against which a HTTP GET request will return a release /// package that corresponds to this version. /// - /// If replica-version is specified and both release-package-download-url - /// and release-package-sha256-hex are unspecified, the - /// release-package-download-url will default to + /// If both release-package-download-url and release-package-sha256-hex are + /// unspecified, the release-package-download-url will default to /// https://download.dfinity.systems/ic//guest-os/update-img/update-img.tar.zst #[clap(long)] pub release_package_download_url: Option, @@ -65,9 +63,8 @@ struct CliArgs { /// 'release_package_url'. Must be present if release_package_url is /// present. /// - /// If replica-version is specified and both release-package-download-url - /// and release-package-sha256-hex are unspecified, the - /// release-package-download-url will downloaded from + /// If both release-package-download-url and release-package-sha256-hex are + /// unspecified, the release-package-download-url will downloaded from /// https://download.dfinity.systems/ic//guest-os/update-img-dev/SHA256SUMS #[clap(long)] pub release_package_sha256_hex: Option, @@ -169,19 +166,16 @@ fn main() -> Result<()> { let mut valid_args = CliArgs::parse().validate()?; // set replica update image if necessary - if let Some(ref replica_version_id) = valid_args.replica_version_id - && !valid_args.allow_empty_update_image - && valid_args.release_package_download_url.is_none() - { + if !valid_args.allow_empty_update_image && valid_args.release_package_download_url.is_none() { let url = Url::parse( - &UPD_IMG_DEFAULT_URL.replace("", replica_version_id.as_ref()), + &UPD_IMG_DEFAULT_URL.replace("", valid_args.replica_version.as_ref()), )?; valid_args.release_package_download_url = Some(url); - valid_args.release_package_sha256_hex = - Some(fetch_replica_version_sha256(replica_version_id.clone())?); + valid_args.release_package_sha256_hex = Some(fetch_replica_version_sha256( + valid_args.replica_version.clone(), + )?); } - let replica_version = valid_args.replica_version_id.unwrap_or_default(); let root_subnet_idx = valid_args.nns_subnet_index.unwrap_or(0); let mut topology_config = TopologyConfig::default(); for (i, (subnet_id, nodes)) in valid_args.subnets.iter().enumerate() { @@ -194,7 +188,7 @@ fn main() -> Result<()> { let subnet_configuration = SubnetConfig::new( *subnet_id, nodes.to_owned(), - replica_version.clone(), + valid_args.replica_version.clone(), valid_args.max_ingress_bytes_per_message, /*max_ingress_bytes_per_block=*/ None, /*max_ingress_messages_per_block=*/ None, @@ -225,7 +219,7 @@ fn main() -> Result<()> { let mut ic_config0 = IcConfig::new( valid_args.working_dir.as_path(), topology_config, - replica_version, + valid_args.replica_version, valid_args.generate_subnet_records, Some(root_subnet_idx), valid_args.release_package_download_url, @@ -257,7 +251,7 @@ fn main() -> Result<()> { #[derive(Clone, Eq, PartialEq, Debug)] struct ValidatedArgs { pub working_dir: PathBuf, - pub replica_version_id: Option, + pub replica_version: ReplicaVersion, pub release_package_download_url: Option, pub release_package_sha256_hex: Option, pub subnets: BTreeMap>, @@ -385,7 +379,7 @@ impl CliArgs { Ok(ValidatedArgs { working_dir, - replica_version_id: self.replica_version, + replica_version: self.replica_version, release_package_download_url: self.release_package_download_url, release_package_sha256_hex: self.release_package_sha256_hex, subnets, diff --git a/rs/prep/src/internet_computer.rs b/rs/prep/src/internet_computer.rs index c79f69cea492..2f9f87e74651 100644 --- a/rs/prep/src/internet_computer.rs +++ b/rs/prep/src/internet_computer.rs @@ -260,7 +260,7 @@ pub struct IcConfig { /// topology of the network. /// /// The version id of the initial replica. - initial_replica_version_id: ReplicaVersion, + pub initial_replica_version_id: ReplicaVersion, /// The URL of the initial release package. initial_release_package_url: Option, /// The hash of the initial release package. diff --git a/rs/prep/src/prep_state_directory.rs b/rs/prep/src/prep_state_directory.rs index e7ca05d57e8f..ed4e58ee18df 100644 --- a/rs/prep/src/prep_state_directory.rs +++ b/rs/prep/src/prep_state_directory.rs @@ -58,7 +58,7 @@ mod tests { use ic_crypto_utils_threshold_sig_der::parse_threshold_sig_key_from_der; use ic_protobuf::registry::subnet::v1::CanisterCyclesCostSchedule; use ic_registry_subnet_type::SubnetType; - use ic_types::ReplicaVersion; + use ic_test_utilities_types::ids::test_replica_version; use std::collections::BTreeMap; use std::net::SocketAddr; use std::str::FromStr; @@ -113,7 +113,7 @@ mod tests { SubnetConfig::new( 0, subnet_nodes, - ReplicaVersion::default(), + test_replica_version(), None, /*max_ingress_bytes_per_block=*/ None, None, @@ -141,7 +141,7 @@ mod tests { let ic_config = IcConfig::new( /* target_dir= */ tmp.path(), topology_config, - ReplicaVersion::default(), + test_replica_version(), /* generate_subnet_records= */ true, // see note above /* nns_subnet_index= */ Some(0), /* release_package_download_url= */ None, diff --git a/rs/prep/src/subnet_configuration.rs b/rs/prep/src/subnet_configuration.rs index 55a6776d3a6c..09d1370a1407 100644 --- a/rs/prep/src/subnet_configuration.rs +++ b/rs/prep/src/subnet_configuration.rs @@ -51,7 +51,7 @@ pub enum SubnetRunningState { /// This represents the initial configuration of an NNS subnetwork of an IC /// instance. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug)] pub struct SubnetConfig { /// The subnet id of this subnetwork. pub subnet_index: SubnetIndex, diff --git a/rs/recovery/src/replay_helper.rs b/rs/recovery/src/replay_helper.rs index a559bfdbe10c..a5bc08abc94f 100644 --- a/rs/recovery/src/replay_helper.rs +++ b/rs/recovery/src/replay_helper.rs @@ -33,6 +33,7 @@ pub async fn replay( subcmd, data_root: Some(data_root), skip_prompts, + replica_version: None, }; // Since replay output needs to be persisted anyway in case the recovery process // is restarted, we avoid declaring a return value and moving out of the diff --git a/rs/registry/admin/bin/create_subnet.rs b/rs/registry/admin/bin/create_subnet.rs index 66561c3c82f2..f35fedad9d0b 100644 --- a/rs/registry/admin/bin/create_subnet.rs +++ b/rs/registry/admin/bin/create_subnet.rs @@ -80,7 +80,7 @@ pub(crate) struct ProposeToCreateSubnetCmd { #[clap(long)] /// ID of the Replica version to run. - pub replica_version_id: Option, + pub replica_version_id: ReplicaVersion, #[clap(long)] /// The length of all DKG intervals. The DKG interval length is the number @@ -279,8 +279,6 @@ impl ProposeToCreateSubnetCmd { } // Other default parameters. { - self.replica_version_id - .get_or_insert(ReplicaVersion::default()); self.max_number_of_canisters.get_or_insert(0); self.features.get_or_insert(SubnetFeatures::default()); self.canister_cycles_cost_schedule @@ -324,11 +322,7 @@ impl ProposeToCreateSubnetCmd { max_ingress_messages_per_block: self.max_ingress_messages_per_block.unwrap_or_default(), max_ingress_bytes_per_block: self.max_ingress_bytes_per_block, max_block_payload_size: self.max_block_payload_size.unwrap_or_default(), - replica_version_id: self - .replica_version_id - .as_ref() - .expect("replica_version_id must be specified.") - .to_string(), + replica_version_id: self.replica_version_id.to_string(), unit_delay_millis: self.unit_delay_millis.unwrap_or_default(), initial_notary_delay_millis: self.initial_notary_delay_millis.unwrap_or_default(), dkg_interval_length: self.dkg_interval_length.unwrap_or_default(), @@ -414,7 +408,7 @@ mod tests { max_block_payload_size: None, unit_delay_millis: None, initial_notary_delay_millis: None, - replica_version_id: None, + replica_version_id: ReplicaVersion::from_str("").unwrap(), dkg_interval_length: None, dkg_dealings_per_block: None, initial_chain_key_configs_to_request: None, @@ -432,7 +426,7 @@ mod tests { #[test] fn cli_to_payload_conversion_works_for_chain_key_fields() { // Boilerplate stuff - let replica_version_id = ReplicaVersion::default(); + let replica_version_id = ReplicaVersion::from_str("123").unwrap(); let features = SubnetFeatures::default(); let initial_chain_key_configs_to_request = r#"[{ @@ -465,7 +459,7 @@ mod tests { idkg_key_rotation_period_ms, max_parallel_pre_signature_transcripts_in_creation, - replica_version_id: Some(replica_version_id.clone()), + replica_version_id: replica_version_id.clone(), features: Some(features), canister_cycles_cost_schedule: Some(CanisterCyclesCostSchedule::Normal), ..empty_propose_to_create_subnet_cmd() diff --git a/rs/registry/canister/src/common/test_helpers.rs b/rs/registry/canister/src/common/test_helpers.rs index 0582dc3d1f5e..713428c5f9a2 100644 --- a/rs/registry/canister/src/common/test_helpers.rs +++ b/rs/registry/canister/src/common/test_helpers.rs @@ -27,7 +27,7 @@ use ic_registry_transport::pb::v1::{ }; use ic_registry_transport::{insert, upsert}; use ic_test_utilities_types::ids::subnet_test_id; -use ic_types::ReplicaVersion; +use ic_test_utilities_types::ids::test_replica_version; use lazy_static::lazy_static; use prost::Message; use std::collections::BTreeMap; @@ -136,7 +136,7 @@ pub fn get_invariant_compliant_subnet_record(node_ids: Vec) -> SubnetRec gossip_max_duplicity: 1, gossip_max_chunk_wait_ms: 200, gossip_max_artifact_streams_per_peer: 1, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), node_ids, ..Default::default() } @@ -284,7 +284,7 @@ pub fn prepare_registry_with_cloud_engine_subnet( .collect(), subnet_type: i32::from(SubnetType::CloudEngine), canister_cycles_cost_schedule: i32::from(CanisterCyclesCostSchedule::Free), - replica_version_id: ReplicaVersion::default().to_string(), + replica_version_id: test_replica_version().to_string(), unit_delay_millis: 600, ..Default::default() }; diff --git a/rs/registry/canister/src/invariants/replica_version.rs b/rs/registry/canister/src/invariants/replica_version.rs index 70bfb41dcd81..23ddd39199b4 100644 --- a/rs/registry/canister/src/invariants/replica_version.rs +++ b/rs/registry/canister/src/invariants/replica_version.rs @@ -195,7 +195,7 @@ mod tests { GuestLaunchMeasurement, GuestLaunchMeasurementMetadata, GuestLaunchMeasurements, }; use ic_registry_transport::{delete, insert, pb::v1::RegistryMutation, upsert}; - use ic_types::ReplicaVersion; + use ic_test_utilities_types::ids::test_replica_version; use prost::Message; const MOCK_HASH: &str = "C0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEEC0FFEED00D"; @@ -527,7 +527,7 @@ mod tests { let registry = invariant_compliant_registry(0); let mutation = vec![delete( - make_replica_version_key(ReplicaVersion::default()).as_bytes(), + make_replica_version_key(test_replica_version()).as_bytes(), )]; registry.check_global_state_invariants(&mutation); } @@ -603,9 +603,10 @@ mod tests { fn check_replica_version(hash: &str, urls: Vec) { let registry = invariant_compliant_registry(0); - let key = make_replica_version_key(ReplicaVersion::default()); + let replica_version = test_replica_version().to_string(); + let key = make_replica_version_key(&replica_version); let value = ReplicaVersionRecord { - replica_version_id: Some(ReplicaVersion::default().to_string()), + replica_version_id: Some(replica_version), release_package_sha256_hex: hash.into(), release_package_urls: urls, guest_launch_measurements: Some(GuestLaunchMeasurements { @@ -653,9 +654,10 @@ mod tests { fn panic_when_measurements_are_empty() { let registry = invariant_compliant_registry(0); - let key = make_replica_version_key(ReplicaVersion::default()); + let replica_version = test_replica_version().to_string(); + let key = make_replica_version_key(&replica_version); let value = ReplicaVersionRecord { - replica_version_id: Some(ReplicaVersion::default().to_string()), + replica_version_id: Some(replica_version), release_package_sha256_hex: MOCK_HASH.into(), release_package_urls: vec![MOCK_URL.into()], guest_launch_measurements: Some(GuestLaunchMeasurements { diff --git a/rs/registry/canister/src/mutations/do_add_api_boundary_nodes.rs b/rs/registry/canister/src/mutations/do_add_api_boundary_nodes.rs index 7082324220bd..453daae1ca0e 100644 --- a/rs/registry/canister/src/mutations/do_add_api_boundary_nodes.rs +++ b/rs/registry/canister/src/mutations/do_add_api_boundary_nodes.rs @@ -102,6 +102,13 @@ impl Registry { mod tests { use std::str::FromStr; + use crate::{ + common::test_helpers::{ + add_fake_subnet, get_invariant_compliant_subnet_record, invariant_compliant_registry, + prepare_registry_with_nodes, + }, + mutations::common::test::TEST_NODE_ID, + }; use ic_base_types::{NodeId, PrincipalId, SubnetId}; use ic_nervous_system_common_test_keys::TEST_USER1_PRINCIPAL; use ic_protobuf::registry::{ @@ -111,17 +118,9 @@ mod tests { make_api_boundary_node_record_key, make_node_record_key, make_replica_version_key, }; use ic_registry_transport::{insert, update}; - use ic_types::ReplicaVersion; + use ic_test_utilities_types::ids::test_replica_version; use prost::Message; - use crate::{ - common::test_helpers::{ - add_fake_subnet, get_invariant_compliant_subnet_record, invariant_compliant_registry, - prepare_registry_with_nodes, - }, - mutations::common::test::TEST_NODE_ID, - }; - use super::AddApiBoundaryNodesPayload; #[test] @@ -162,7 +161,7 @@ mod tests { let payload = AddApiBoundaryNodesPayload { node_ids: vec![node_id], - version: ReplicaVersion::default().to_string(), + version: test_replica_version().to_string(), }; registry.maybe_apply_mutation_internal(vec![insert( diff --git a/rs/registry/canister/src/mutations/do_create_subnet.rs b/rs/registry/canister/src/mutations/do_create_subnet.rs index 91a11caf9418..0ca64de95442 100644 --- a/rs/registry/canister/src/mutations/do_create_subnet.rs +++ b/rs/registry/canister/src/mutations/do_create_subnet.rs @@ -620,8 +620,7 @@ mod test { use ic_management_canister_types_private::{EcdsaCurve, EcdsaKeyId, VetKdCurve, VetKdKeyId}; use ic_nervous_system_common_test_keys::{TEST_USER1_PRINCIPAL, TEST_USER2_PRINCIPAL}; use ic_registry_subnet_features::{ChainKeyConfig, DEFAULT_ECDSA_MAX_QUEUE_SIZE}; - use ic_test_utilities_types::ids::subnet_test_id; - use ic_types::ReplicaVersion; + use ic_test_utilities_types::ids::{subnet_test_id, test_replica_version}; // Note: this can only be unit-tested b/c it fails before we hit inter-canister calls // for DKG + ECDSA @@ -632,7 +631,7 @@ mod test { fn should_panic_if_ecdsa_keys_non_existing() { let mut registry = invariant_compliant_registry(0); let payload = CreateSubnetPayload { - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), chain_key_config: Some(InitialChainKeyConfig { key_configs: vec![KeyConfigRequest { key_config: Some(KeyConfig { @@ -697,7 +696,7 @@ mod test { // Make a request for the key from a subnet that does not have the key let payload = CreateSubnetPayload { - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), chain_key_config: Some(InitialChainKeyConfig { key_configs: vec![KeyConfigRequest { key_config: Some(KeyConfig { @@ -761,7 +760,7 @@ mod test { // Make a request for the key from a subnet that does not have the key let payload = CreateSubnetPayload { - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), chain_key_config: Some(InitialChainKeyConfig { key_configs: vec![KeyConfigRequest { key_config: Some(KeyConfig { @@ -833,7 +832,7 @@ mod test { subnet_id: Some(*TEST_USER1_PRINCIPAL), }; let payload = CreateSubnetPayload { - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), chain_key_config: Some(InitialChainKeyConfig { key_configs: vec![key_config_request; 2], signature_request_timeout_ns: None, @@ -896,7 +895,7 @@ mod test { pre_signatures_to_create_in_advance: Option, ) -> CreateSubnetPayload { CreateSubnetPayload { - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), chain_key_config: Some(InitialChainKeyConfig { key_configs: vec![KeyConfigRequest { key_config: Some(KeyConfig { diff --git a/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_subnet_nodes.rs b/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_subnet_nodes.rs index 3901908b74a2..5d1fb20173af 100644 --- a/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_subnet_nodes.rs +++ b/rs/registry/canister/src/mutations/do_deploy_guestos_to_all_subnet_nodes.rs @@ -77,8 +77,7 @@ mod tests { use ic_nns_constants::{ENGINE_CONTROLLER_CANISTER_ID, GOVERNANCE_CANISTER_ID}; use ic_protobuf::registry::subnet::v1::SubnetType as SubnetTypePb; use ic_registry_subnet_type::SubnetType; - use ic_test_utilities_types::ids::subnet_test_id; - use ic_types::ReplicaVersion; + use ic_test_utilities_types::ids::{subnet_test_id, test_replica_version}; use maplit::btreemap; /// Creates a registry with a single non-CloudEngine subnet of the given @@ -118,7 +117,7 @@ mod tests { fn deploy_payload(subnet_id: SubnetId) -> DeployGuestosToAllSubnetNodesPayload { DeployGuestosToAllSubnetNodesPayload { subnet_id: subnet_id.get(), - replica_version_id: ReplicaVersion::default().to_string(), + replica_version_id: test_replica_version().to_string(), } } @@ -136,7 +135,7 @@ mod tests { let subnet_record = registry.get_subnet_or_panic(subnet_id); assert_eq!( subnet_record.replica_version_id, - ReplicaVersion::default().to_string() + test_replica_version().to_string() ); } @@ -165,7 +164,7 @@ mod tests { let subnet_record = registry.get_subnet_or_panic(subnet_id); assert_eq!( subnet_record.replica_version_id, - ReplicaVersion::default().to_string() + test_replica_version().to_string() ); } } diff --git a/rs/registry/canister/src/mutations/do_remove_api_boundary_nodes.rs b/rs/registry/canister/src/mutations/do_remove_api_boundary_nodes.rs index e1f95e777e62..5f090410a0a8 100644 --- a/rs/registry/canister/src/mutations/do_remove_api_boundary_nodes.rs +++ b/rs/registry/canister/src/mutations/do_remove_api_boundary_nodes.rs @@ -45,7 +45,7 @@ mod tests { use ic_protobuf::registry::api_boundary_node::v1::ApiBoundaryNodeRecord; use ic_registry_keys::make_api_boundary_node_record_key; use ic_registry_transport::insert; - use ic_types::ReplicaVersion; + use ic_test_utilities_types::ids::test_replica_version; use prost::Message; use crate::{ @@ -95,7 +95,7 @@ mod tests { insert( make_api_boundary_node_record_key(node_id), // key ApiBoundaryNodeRecord { - version: ReplicaVersion::default().to_string(), + version: test_replica_version().to_string(), } .encode_to_vec(), ), diff --git a/rs/registry/canister/src/mutations/do_update_api_boundary_nodes_version.rs b/rs/registry/canister/src/mutations/do_update_api_boundary_nodes_version.rs index 7b5190021510..5c4fe02956d4 100644 --- a/rs/registry/canister/src/mutations/do_update_api_boundary_nodes_version.rs +++ b/rs/registry/canister/src/mutations/do_update_api_boundary_nodes_version.rs @@ -83,7 +83,7 @@ mod tests { }; use ic_registry_keys::{make_api_boundary_node_record_key, make_replica_version_key}; use ic_registry_transport::insert; - use ic_types::ReplicaVersion; + use ic_test_utilities_types::ids::test_replica_version; use prost::Message; use crate::{ @@ -135,7 +135,7 @@ mod tests { insert( make_api_boundary_node_record_key(node_id), // key ApiBoundaryNodeRecord { - version: ReplicaVersion::default().to_string(), + version: test_replica_version().to_string(), } .encode_to_vec(), ), diff --git a/rs/registry/canister/src/mutations/do_update_subnet.rs b/rs/registry/canister/src/mutations/do_update_subnet.rs index 9e341e827792..82c9647aff74 100644 --- a/rs/registry/canister/src/mutations/do_update_subnet.rs +++ b/rs/registry/canister/src/mutations/do_update_subnet.rs @@ -673,8 +673,8 @@ mod tests { use ic_registry_resource_limits::ResourceLimits; use ic_registry_subnet_features::DEFAULT_ECDSA_MAX_QUEUE_SIZE; use ic_registry_subnet_type::SubnetType; - use ic_test_utilities_types::ids::subnet_test_id; - use ic_types::{NumBytes, NumInstructions, PrincipalId, ReplicaVersion, SubnetId}; + use ic_test_utilities_types::ids::{subnet_test_id, test_replica_version}; + use ic_types::{NumBytes, NumInstructions, PrincipalId, SubnetId}; use maplit::btreemap; use std::str::FromStr; @@ -725,7 +725,7 @@ mod tests { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, @@ -823,7 +823,7 @@ mod tests { max_block_payload_size: 200, unit_delay_millis: 300, initial_notary_delay_millis: 200, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 8, dkg_dealings_per_block: 1, start_as_nns: true, @@ -870,7 +870,7 @@ mod tests { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, @@ -944,7 +944,7 @@ mod tests { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 100, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 2, dkg_dealings_per_block: 1, start_as_nns: false, @@ -1253,7 +1253,7 @@ mod tests { let mut registry = invariant_compliant_registry(0); add_guest_launch_measurements_to_replica_version( &mut registry, - ReplicaVersion::default().as_ref(), + test_replica_version().as_ref(), ); let (mutate_request, node_ids_and_dkg_pks) = prepare_registry_with_nodes_and_chip_id(1, 2); diff --git a/rs/registry/canister/src/mutations/node_management/do_add_node.rs b/rs/registry/canister/src/mutations/node_management/do_add_node.rs index 64ae55361fc0..72eafa386a88 100644 --- a/rs/registry/canister/src/mutations/node_management/do_add_node.rs +++ b/rs/registry/canister/src/mutations/node_management/do_add_node.rs @@ -473,7 +473,7 @@ mod tests { make_replica_version_key, }; use ic_registry_transport::{delete, insert, update}; - use ic_types::ReplicaVersion; + use ic_test_utilities_types::ids::test_replica_version; use itertools::Itertools; use lazy_static::lazy_static; use maplit::btreemap; @@ -1066,7 +1066,7 @@ mod tests { // Turn that node into an API boundary node let api_bn = ApiBoundaryNodeRecord { - version: ReplicaVersion::default().to_string(), + version: test_replica_version().to_string(), }; registry.maybe_apply_mutation_internal(vec![insert( make_api_boundary_node_record_key(old_node_id), @@ -1504,7 +1504,7 @@ mod tests { } fn add_elected_measurement_to_registry(registry: &mut Registry, measurement: &[u8]) { - let replica_version_id = ReplicaVersion::default().to_string(); + let replica_version_id = test_replica_version().to_string(); let replica_version = ReplicaVersionRecord { replica_version_id: Some(replica_version_id.clone()), release_package_sha256_hex: "".to_string(), diff --git a/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs b/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs index e4c6740d94c1..8156813bfcd7 100644 --- a/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs +++ b/rs/registry/canister/src/mutations/node_management/do_remove_node_directly.rs @@ -230,7 +230,7 @@ mod tests { }; use ic_registry_keys::{make_node_operator_record_key, make_node_record_key}; use ic_registry_transport::{insert, update}; - use ic_types::ReplicaVersion; + use ic_test_utilities_types::ids::test_replica_version; use maplit::btreemap; use prost::Message; use std::str::FromStr; @@ -287,7 +287,7 @@ mod tests { // Add API BN to registry let api_bn = ApiBoundaryNodeRecord { - version: ReplicaVersion::default().to_string(), + version: test_replica_version().to_string(), }; registry.maybe_apply_mutation_internal(vec![insert( make_api_boundary_node_record_key(node_id), @@ -306,7 +306,7 @@ mod tests { // turn first node into an API BN by adding the record to the registry let api_bn = ApiBoundaryNodeRecord { - version: ReplicaVersion::default().to_string(), + version: test_replica_version().to_string(), }; registry.maybe_apply_mutation_internal(vec![insert( make_api_boundary_node_record_key(old_node_id), @@ -367,7 +367,7 @@ mod tests { // turn first node into an API BN by adding the record to the registry let api_bn = ApiBoundaryNodeRecord { - version: ReplicaVersion::default().to_string(), + version: test_replica_version().to_string(), }; registry.maybe_apply_mutation_internal(vec![insert( make_api_boundary_node_record_key(old_node_id), diff --git a/rs/registry/canister/tests/common/test_helpers.rs b/rs/registry/canister/tests/common/test_helpers.rs index b21cfa55a2ff..3706bab584ca 100644 --- a/rs/registry/canister/tests/common/test_helpers.rs +++ b/rs/registry/canister/tests/common/test_helpers.rs @@ -35,7 +35,7 @@ use ic_registry_subnet_features::DEFAULT_ECDSA_MAX_QUEUE_SIZE; use ic_registry_subnet_type::SubnetType; use ic_registry_transport::pb::v1::RegistryAtomicMutateRequest; use ic_registry_transport::upsert; -use ic_test_utilities_types::ids::subnet_test_id; +use ic_test_utilities_types::ids::{subnet_test_id, test_replica_version}; use ic_types::ReplicaVersion; use pocket_ic::nonblocking::PocketIc; use prost::Message; @@ -61,12 +61,12 @@ pub async fn get_subnet_record(registry: &Canister<'_>, subnet_id: SubnetId) -> pub fn get_subnet_holding_chain_keys( key_ids: Vec, node_ids: Vec, + replica_version_id: ReplicaVersion, ) -> SubnetRecord { let unit_delay_millis = 10; - let replica_version_id = String::from(ReplicaVersion::default()); let mut subnet_record = SubnetRecord::from(CreateSubnetPayload { unit_delay_millis, - replica_version_id, + replica_version_id: replica_version_id.to_string(), node_ids, ..Default::default() }); @@ -237,7 +237,7 @@ pub fn prepare_registry_with_cloud_engine_subnet( let subnet_record = SubnetRecord { subnet_type: i32::from(SubnetType::CloudEngine), canister_cycles_cost_schedule: i32::from(CanisterCyclesCostSchedule::Free), - replica_version_id: ReplicaVersion::default().to_string(), + replica_version_id: test_replica_version().to_string(), unit_delay_millis: 600, ..Default::default() }; @@ -258,7 +258,7 @@ pub fn prepare_registry_with_application_subnet( let subnet_record = SubnetRecord { subnet_type: i32::from(SubnetType::Application), - replica_version_id: ReplicaVersion::default().to_string(), + replica_version_id: test_replica_version().to_string(), unit_delay_millis: 600, ..Default::default() }; diff --git a/rs/registry/canister/tests/create_subnet.rs b/rs/registry/canister/tests/create_subnet.rs index 37ca85151555..c13c24ed9cf4 100644 --- a/rs/registry/canister/tests/create_subnet.rs +++ b/rs/registry/canister/tests/create_subnet.rs @@ -41,7 +41,8 @@ use ic_registry_subnet_features::{ use ic_registry_subnet_type::SubnetType; use ic_registry_transport::{pb::v1::RegistryAtomicMutateRequest, upsert}; use ic_replica_tests::{canister_test_with_config_async, get_ic_config}; -use ic_types::{NodeId, ReplicaVersion}; +use ic_test_utilities_types::ids::test_replica_version; +use ic_types::NodeId; use pocket_ic::PocketIcBuilder; use prost::Message; use registry_canister::{ @@ -435,7 +436,7 @@ fn make_create_subnet_payload(node_ids: Vec) -> CreateSubnetPayload { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().into(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, diff --git a/rs/registry/canister/tests/recover_subnet.rs b/rs/registry/canister/tests/recover_subnet.rs index a351abf4c966..ba32728a2c46 100644 --- a/rs/registry/canister/tests/recover_subnet.rs +++ b/rs/registry/canister/tests/recover_subnet.rs @@ -34,7 +34,7 @@ use ic_registry_transport::{insert, pb::v1::RegistryAtomicMutateRequest, upsert} use ic_replica_tests::{canister_test_with_config_async, get_ic_config}; use ic_test_utilities_types::ids::subnet_test_id; use ic_types::{ - Height, RegistryVersion, ReplicaVersion, + Height, RegistryVersion, crypto::{ AlgorithmId, BasicSig, BasicSigOf, canister_threshold_sig::idkg::{ @@ -222,6 +222,7 @@ fn test_recover_subnet_with_replacement_nodes() { fn test_recover_subnet_gets_chain_keys_when_needed(key_id: MasterPublicKeyId) { let ic_config = get_ic_config(); let (config, _tmpdir) = Config::temp_config(); + let replica_version_id = ic_config.initial_replica_version_id.to_string(); canister_test_with_config_async(config, ic_config, |local_runtime| async move { let data_provider = local_runtime.registry_data_provider.clone(); let fake_client = local_runtime.registry_client.clone(); @@ -242,7 +243,7 @@ fn test_recover_subnet_gets_chain_keys_when_needed(key_id: MasterPublicKeyId) { gossip_max_duplicity: 1, gossip_max_chunk_wait_ms: 200, gossip_max_artifact_streams_per_peer: 1, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id, ..CreateSubnetPayload::default() } .into(); @@ -473,6 +474,7 @@ fn test_recover_subnet_gets_vetkd_keys_when_needed() { fn test_recover_subnet_without_chain_key_removes_it_from_signing_list(key_id: MasterPublicKeyId) { let ic_config = get_ic_config(); let (config, _tmpdir) = Config::temp_config(); + let replica_version = ic_config.initial_replica_version_id.clone(); canister_test_with_config_async(config, ic_config, |local_runtime| async move { let data_provider = local_runtime.registry_data_provider.clone(); let fake_client = local_runtime.registry_client.clone(); @@ -483,8 +485,11 @@ fn test_recover_subnet_without_chain_key_removes_it_from_signing_list(key_id: Ma let mut node_ids: Vec = node_ids_and_valid_pks.keys().cloned().collect(); let subnet_to_recover_nodes = vec![node_ids.pop().unwrap()]; - let subnet_to_recover = - get_subnet_holding_chain_keys(vec![key_id.clone()], subnet_to_recover_nodes.clone()); + let subnet_to_recover = get_subnet_holding_chain_keys( + vec![key_id.clone()], + subnet_to_recover_nodes.clone(), + replica_version, + ); // Here we discover the IC's subnet ID (from our test harness) // and then modify it to hold the key and sign for it. @@ -713,6 +718,7 @@ fn test_recover_subnet_without_vetkd_removes_it_from_signing_list() { fn test_recover_subnet_resets_the_halt_at_cup_height_flag() { let ic_config = get_ic_config(); let (config, _tmpdir) = Config::temp_config(); + let replica_version_id = ic_config.initial_replica_version_id.to_string(); canister_test_with_config_async(config, ic_config, |local_runtime| async move { let data_provider = local_runtime.registry_data_provider.clone(); let fake_client = local_runtime.registry_client.clone(); @@ -732,7 +738,7 @@ fn test_recover_subnet_resets_the_halt_at_cup_height_flag() { gossip_max_duplicity: 1, gossip_max_chunk_wait_ms: 200, gossip_max_artifact_streams_per_peer: 1, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id, node_ids: subnet_to_recover_nodes.clone(), ..Default::default() } @@ -924,6 +930,7 @@ fn dummy_initial_idkg_dealing_for_tests( fn test_recover_subnet_resets_cup_contents() { let ic_config = get_ic_config(); let (config, _tmpdir) = Config::temp_config(); + let replica_version_id = ic_config.initial_replica_version_id.to_string(); canister_test_with_config_async(config, ic_config, |local_runtime| async move { let data_provider = local_runtime.registry_data_provider.clone(); let fake_client = local_runtime.registry_client.clone(); @@ -949,7 +956,7 @@ fn test_recover_subnet_resets_cup_contents() { gossip_max_duplicity: 1, gossip_max_chunk_wait_ms: 200, gossip_max_artifact_streams_per_peer: 1, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id, ..CreateSubnetPayload::default() } .into(); diff --git a/rs/registry/canister/tests/remove_node_directly.rs b/rs/registry/canister/tests/remove_node_directly.rs index 8dd98bb06cd7..04316942e1e8 100644 --- a/rs/registry/canister/tests/remove_node_directly.rs +++ b/rs/registry/canister/tests/remove_node_directly.rs @@ -25,7 +25,8 @@ use ic_registry_keys::{ use ic_registry_transport::pb::v1::{ RegistryAtomicMutateRequest, RegistryMutation, registry_mutation, }; -use ic_types::{NodeId, ReplicaVersion}; +use ic_test_utilities_types::ids::test_replica_version; +use ic_types::NodeId; use maplit::btreemap; use prost::Message; use registry_canister::init::RegistryCanisterInitPayloadBuilder; @@ -155,7 +156,7 @@ fn node_cannot_be_removed_if_in_subnet() { let test_subnet_id = SubnetId::from(*TEST_NEURON_1_OWNER_PRINCIPAL); let test_subnet_record = SubnetRecord { membership: vec![node_id.get().to_vec()], - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), unit_delay_millis: 600, ..Default::default() }; diff --git a/rs/registry/canister/tests/swap_node_in_subnet_directly.rs b/rs/registry/canister/tests/swap_node_in_subnet_directly.rs index d6dcc1b2c64e..6e4d5c080dd9 100644 --- a/rs/registry/canister/tests/swap_node_in_subnet_directly.rs +++ b/rs/registry/canister/tests/swap_node_in_subnet_directly.rs @@ -20,7 +20,8 @@ use ic_registry_transport::{ pb::v1::{RegistryAtomicMutateRequest, RegistryMutation}, upsert, }; -use ic_types::{NodeId, ReplicaVersion, SubnetId}; +use ic_test_utilities_types::ids::test_replica_version; +use ic_types::{NodeId, SubnetId}; use pocket_ic::PocketIcBuilder; use prost::Message; use registry_canister::{ @@ -109,7 +110,7 @@ fn get_mutations_and_node_ids( .iter() .map(|vk| vk.node_id().get().to_vec()) .collect(), - replica_version_id: ReplicaVersion::default().to_string(), + replica_version_id: test_replica_version().to_string(), subnet_type: SubnetType::System as i32, ..Default::default() } diff --git a/rs/registry/canister/tests/update_subnet.rs b/rs/registry/canister/tests/update_subnet.rs index 18f9e592c4eb..d1a90b8f6266 100644 --- a/rs/registry/canister/tests/update_subnet.rs +++ b/rs/registry/canister/tests/update_subnet.rs @@ -24,7 +24,7 @@ use ic_registry_subnet_features::{ }; use ic_registry_subnet_type::SubnetType; use ic_registry_transport::{insert, pb::v1::RegistryAtomicMutateRequest}; -use ic_types::ReplicaVersion; +use ic_test_utilities_types::ids::test_replica_version; use prost::Message; use registry_canister::mutations::do_update_subnet::{ChainKeyConfig, KeyConfig}; use registry_canister::{ @@ -144,7 +144,7 @@ fn test_a_canister_other_than_the_governance_canister_cannot_update_a_subnets_co max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, @@ -274,7 +274,7 @@ fn test_the_governance_canister_can_update_a_subnets_configuration() { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, @@ -373,7 +373,7 @@ fn test_the_governance_canister_can_update_a_subnets_configuration() { max_ingress_messages_per_block: 1000, unit_delay_millis: 100, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 2, dkg_dealings_per_block: 1, start_as_nns: false, @@ -460,7 +460,7 @@ fn test_subnets_configuration_chain_key_fields_are_updated_correctly(key_id: Mas max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, diff --git a/rs/registry/canister/tests/update_subnet_admins.rs b/rs/registry/canister/tests/update_subnet_admins.rs index d458f549785a..a9115a065afb 100644 --- a/rs/registry/canister/tests/update_subnet_admins.rs +++ b/rs/registry/canister/tests/update_subnet_admins.rs @@ -15,8 +15,7 @@ use ic_protobuf::types::v1::PrincipalId as PrincipalIdPb; use ic_registry_keys::make_subnet_record_key; use ic_registry_subnet_type::SubnetType; use ic_registry_transport::{insert, pb::v1::RegistryAtomicMutateRequest}; -use ic_test_utilities_types::ids::user_test_id; -use ic_types::ReplicaVersion; +use ic_test_utilities_types::ids::{test_replica_version, user_test_id}; use prost::Message; use registry_canister::{ init::RegistryCanisterInitPayloadBuilder, @@ -158,7 +157,7 @@ fn test_subnet_admins_cannot_be_updated_for_system_subnet() { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, @@ -249,7 +248,7 @@ fn test_subnet_admins_cannot_be_updated_for_non_rented_subnet() { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, @@ -339,7 +338,7 @@ fn test_subnet_rental_canister_can_update_subnet_admins_of_rented_subnet() { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, @@ -430,7 +429,7 @@ fn test_rate_limit_subnet_admin_updates_per_subnet() { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: 1500, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 0, dkg_dealings_per_block: 1, start_as_nns: false, diff --git a/rs/registry/canister/tests/update_subnet_and_elect_replica_version.rs b/rs/registry/canister/tests/update_subnet_and_elect_replica_version.rs index 68b17e7bfc43..b11fe02f4b26 100644 --- a/rs/registry/canister/tests/update_subnet_and_elect_replica_version.rs +++ b/rs/registry/canister/tests/update_subnet_and_elect_replica_version.rs @@ -10,13 +10,12 @@ use ic_nns_test_utils::{ }; 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 ic_test_utilities_types::ids::{subnet_test_id, test_replica_version}; use assert_matches::assert_matches; use ic_protobuf::registry::replica_version::v1::{ GuestLaunchMeasurement, GuestLaunchMeasurementMetadata, GuestLaunchMeasurements, }; -use ic_types::ReplicaVersion; use lazy_static::lazy_static; use registry_canister::{ init::RegistryCanisterInitPayloadBuilder, @@ -242,7 +241,7 @@ fn test_accepted_proposal_mutates_the_registry() { // 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(), + replica_version_id: test_replica_version().to_string(), }; assert!( forward_call_via_universal_canister( @@ -260,7 +259,7 @@ fn test_accepted_proposal_mutates_the_registry() { ) .await .replica_version_id, - ReplicaVersion::default().to_string(), + test_replica_version().to_string(), ); // Try to set the subnet to an unelected version: it should fail @@ -284,7 +283,7 @@ fn test_accepted_proposal_mutates_the_registry() { ) .await .replica_version_id, - ReplicaVersion::default().to_string(), + test_replica_version().to_string(), ); Ok(()) diff --git a/rs/registry/canister/tests/update_unassigned_nodes_config.rs b/rs/registry/canister/tests/update_unassigned_nodes_config.rs index 65b3268a4f86..232484104357 100644 --- a/rs/registry/canister/tests/update_unassigned_nodes_config.rs +++ b/rs/registry/canister/tests/update_unassigned_nodes_config.rs @@ -10,7 +10,7 @@ use ic_nns_test_utils::{ }; use ic_protobuf::registry::unassigned_nodes_config::v1::UnassignedNodesConfigRecord; use ic_registry_keys::make_unassigned_nodes_config_record_key; -use ic_types::ReplicaVersion; +use ic_test_utilities_types::ids::test_replica_version; use registry_canister::{ init::RegistryCanisterInitPayloadBuilder, mutations::do_update_unassigned_nodes_config::UpdateUnassignedNodesConfigPayload, @@ -93,7 +93,7 @@ fn test_updating_unassigned_nodes_config_does_not_break_invariants() { // New payload with already-elected version payload = UpdateUnassignedNodesConfigPayload { ssh_readonly_access: None, - replica_version: Some(ReplicaVersion::default().into()), + replica_version: Some(test_replica_version().to_string()), }; assert!( @@ -114,7 +114,7 @@ fn test_updating_unassigned_nodes_config_does_not_break_invariants() { .await, UnassignedNodesConfigRecord { ssh_readonly_access: vec![], - replica_version: ReplicaVersion::default().into(), + replica_version: test_replica_version().to_string(), } ); diff --git a/rs/registry/helpers/src/subnet.rs b/rs/registry/helpers/src/subnet.rs index 5d375b95198f..3631b26e1c9f 100644 --- a/rs/registry/helpers/src/subnet.rs +++ b/rs/registry/helpers/src/subnet.rs @@ -829,7 +829,7 @@ mod tests { let subnet_id = subnet_id(4); let version = RegistryVersion::from(2); - let replica_version = ReplicaVersion::try_from("some_version").unwrap(); + let replica_version = ReplicaVersion::from_str("some_version").unwrap(); let replica_version_record = ReplicaVersionRecord::default(); let subnet_record = SubnetRecord { @@ -952,15 +952,15 @@ mod tests { // Step 3: Verify result(s). assert_eq!( low_priority_result, - Some(ReplicaVersion::try_from("new").unwrap()) + Some(ReplicaVersion::from_str("new").unwrap()) ); assert_eq!( high_priority_result, - Some(ReplicaVersion::try_from("old").unwrap()) + Some(ReplicaVersion::from_str("old").unwrap()) ); assert_eq!( override_result, - Some(ReplicaVersion::try_from("override").unwrap()) + Some(ReplicaVersion::from_str("override").unwrap()) ); } @@ -1136,7 +1136,7 @@ mod tests { let subnet_id = subnet_id(4); let version = RegistryVersion::from(2); let max_block_payload_size_bytes = 4 * 1024 * 1024; // 4MiB - let replica_version = ReplicaVersion::try_from("some_version").unwrap(); + let replica_version = ReplicaVersion::from_str("some_version").unwrap(); let subnet_record = SubnetRecord { max_block_payload_size: max_block_payload_size_bytes, diff --git a/rs/registry/regedit/src/tests.rs b/rs/registry/regedit/src/tests.rs index f85b7ba53108..80661ce7fc21 100644 --- a/rs/registry/regedit/src/tests.rs +++ b/rs/registry/regedit/src/tests.rs @@ -146,7 +146,7 @@ pub fn run_ic_prep() -> (TempDir, IcPrepStateDir) { SubnetConfig::new( SUBNET_ID, subnet_nodes, - ReplicaVersion::default(), + ReplicaVersion::try_from("test_replica_version").unwrap(), None, /*max_ingress_bytes_per_block=*/ None, None, @@ -175,7 +175,7 @@ pub fn run_ic_prep() -> (TempDir, IcPrepStateDir) { let ic_config = IcConfig::new( /* target_dir= */ temp_dir.path(), topology_config, - ReplicaVersion::default(), + ReplicaVersion::try_from("test_replica_version").unwrap(), /* generate_subnet_records= */ true, // see note above /* nns_subnet_index= */ Some(0), /* release_package_url= */ None, diff --git a/rs/replay/src/cmd.rs b/rs/replay/src/cmd.rs index 2a2c7c5e2e25..4cc142134de9 100644 --- a/rs/replay/src/cmd.rs +++ b/rs/replay/src/cmd.rs @@ -1,6 +1,6 @@ use clap::{Parser, Subcommand}; use ic_protobuf::registry::replica_version::v1::ReplicaVersionRecord; -use ic_types::{CanisterId, PrincipalId, SubnetId}; +use ic_types::{CanisterId, PrincipalId, ReplicaVersion, SubnetId}; use icp_ledger::AccountIdentifier; use std::path::PathBuf; @@ -45,6 +45,12 @@ pub struct ReplayToolArgs { #[clap(long)] /// Whether or not to skip prompts for user input. pub skip_prompts: bool, + + /// The replica version under which the extra messages of the subcommand are + /// executed. Only needed if no consensus pool is available, otherwise the version is taken from + /// its finalized tip. + #[clap(long)] + pub replica_version: Option, } #[derive(Clone, Subcommand)] diff --git a/rs/replay/src/lib.rs b/rs/replay/src/lib.rs index a132bfac6a95..68e489fb3371 100644 --- a/rs/replay/src/lib.rs +++ b/rs/replay/src/lib.rs @@ -61,6 +61,7 @@ mod validator; /// start_height: 0, /// })), /// skip_prompts: true, +/// replica_version: None, /// }; /// // Once the arguments are set well, the local store and spool directories are populated; /// // replay function could be called as follows: @@ -127,7 +128,8 @@ pub fn replay(args: ReplayToolArgs) -> ReplayResult { { let _enter_guard = rt.enter(); - let player = Player::new(cfg, subnet_id).with_replay_target_height(target_height); + let player = Player::new(cfg, subnet_id, args.replica_version) + .with_replay_target_height(target_height); if let Some(SubCommand::GetRecoveryCup(cmd)) = subcmd { cmd_get_recovery_cup(&player, cmd).unwrap(); diff --git a/rs/replay/src/player.rs b/rs/replay/src/player.rs index cdf8e5923e1a..b2dcec94ed58 100644 --- a/rs/replay/src/player.rs +++ b/rs/replay/src/player.rs @@ -176,7 +176,7 @@ impl Player { let artifact_pool_config = ArtifactPoolConfig::from(cfg.artifact_pool.clone()); let backup_dir = backup_spool_path .join(subnet_id.to_string()) - .join(replica_version.to_string()); + .join(replica_version.as_ref()); // Extract the genesis CUP and instantiate a new pool. let cup_file = backup::cup_file_name(&backup_dir, Height::from(start_height)); let initial_cup_proto = backup::read_cup_proto_file(&cup_file) @@ -185,6 +185,7 @@ impl Player { let pool = ConsensusPoolImpl::new( NodeId::from(PrincipalId::new_anonymous()), subnet_id, + &replica_version, // Note: it's important to pass the original proto which came from the command line (as // opposed to, for example, a proto which was first deserialized and then serialized // again). Since the proto file could have been produced and signed by nodes running a @@ -216,7 +217,11 @@ impl Player { /// Create and return a `Player` from a replica configuration object for /// subnet recovery. - pub(crate) fn new(cfg: Config, subnet_id: SubnetId) -> Self { + pub(crate) fn new( + cfg: Config, + subnet_id: SubnetId, + replica_version: Option, + ) -> Self { let (log, _async_log_guard) = new_replica_logger_from_config(&cfg.logger); let metrics_registry = MetricsRegistry::new(); let registry = setup_registry(cfg.clone(), Some(&metrics_registry)); @@ -243,7 +248,14 @@ impl Player { // Use the replica version from the finalized tip in the pool. PoolReader::new(pool).get_finalized_tip().version().clone() } else { - Default::default() + // Without a consensus pool, the replica version must be given. + replica_version.unwrap_or_else(|| { + panic!( + "No consensus pool found at {:?} and no replica version was given; \ + one of the two is required.", + cfg.artifact_pool.consensus_pool_path + ) + }) }; Player::new_with_params( @@ -269,11 +281,6 @@ impl Player { log: ReplicaLogger, _async_log_guard: AsyncGuard, ) -> Self { - println!("Setting default replica version {replica_version}"); - if ReplicaVersion::set_default_version(replica_version.clone()).is_err() { - println!("Failed to set default replica version"); - } - let registry_version = registry.get_latest_version(); let subnet_type = match registry.get_subnet_record(subnet_id, registry_version) { Ok(Some(record)) => { @@ -342,6 +349,7 @@ impl Player { ReplayValidator::new( cfg, subnet_id, + replica_version.clone(), crypto.clone(), crypto.clone(), verifier, @@ -742,7 +750,7 @@ impl Player { self.registry.get_latest_version(), ic_types::time::current_time(), Randomness::from([0; 32]), - ReplicaVersion::default(), + self.replica_version.clone(), ), Some(pool) => { let pool = PoolReader::new(pool); diff --git a/rs/replay/src/validator.rs b/rs/replay/src/validator.rs index 45ee65157ea9..1a374e10340d 100644 --- a/rs/replay/src/validator.rs +++ b/rs/replay/src/validator.rs @@ -27,7 +27,7 @@ use ic_metrics::MetricsRegistry; use ic_protobuf::types::v1 as pb; use ic_replicated_state::ReplicatedState; use ic_types::{ - Height, NodeId, PrincipalId, SubnetId, + Height, NodeId, PrincipalId, ReplicaVersion, SubnetId, artifact::ConsensusMessageId, consensus::{ Block, ConsensusMessage, ConsensusMessageHash, ConsensusMessageHashable, HasBlockHash, @@ -103,6 +103,7 @@ impl ReplayValidator { pub fn new( cfg: Config, subnet_id: SubnetId, + replica_version: ReplicaVersion, consensus_crypto: Arc, certification_crypto: Arc, verifier: Arc, @@ -125,7 +126,11 @@ impl ReplayValidator { Height::from(0), )); let node_id = NodeId::from(PrincipalId::new_node_test_id(1)); - let replica_cfg = ReplicaConfig::new(node_id, subnet_id); + let replica_cfg = ReplicaConfig { + node_id, + subnet_id, + replica_version, + }; let thread_pool = ThreadPoolBuilder::new() .num_threads(MAX_VALIDATION_THREADS) .build() @@ -223,6 +228,7 @@ impl ReplayValidator { let mut pool = ConsensusPoolImpl::new( self.replica_cfg.node_id, self.replica_cfg.subnet_id, + &self.replica_cfg.replica_version, cup, artifact_pool_config, MetricsRegistry::new(), diff --git a/rs/replica/bin/replica/main.rs b/rs/replica/bin/replica/main.rs index 2d10e085b989..1ced58c0d718 100644 --- a/rs/replica/bin/replica/main.rs +++ b/rs/replica/bin/replica/main.rs @@ -182,8 +182,12 @@ fn main() -> io::Result<()> { .as_ref() .map(|proto| CatchUpPackage::try_from(proto).expect("deserializing CUP failed")); - // Set the replica version and report as metric - setup::set_replica_version(&replica_args, &logger); + const UNKNOWN_REPLICA_VERSION: &str = "unknown_replica_version"; + let replica_version = replica_args.as_ref().map_or_else( + |_| ReplicaVersion::try_from(UNKNOWN_REPLICA_VERSION).unwrap(), + |args| args.replica_version.clone(), + ); + // Report replica version metric { let g = metrics_registry.int_gauge_vec( "ic_replica_info", @@ -191,7 +195,7 @@ fn main() -> io::Result<()> { &["ic_active_version", "ic_replica_binary_hash"], ); g.with_label_values(&[ - ReplicaVersion::default().as_ref(), + replica_version.as_ref(), &get_replica_binary_hash() .map(|x| x.1) .unwrap_or_else(|_| "na".to_string()), @@ -284,6 +288,7 @@ fn main() -> io::Result<()> { config.clone(), node_id, subnet_id, + replica_version, registry, crypto, cup_proto, diff --git a/rs/replica/setup_ic_network/src/lib.rs b/rs/replica/setup_ic_network/src/lib.rs index 4a59797293bc..75e1ab45b06c 100644 --- a/rs/replica/setup_ic_network/src/lib.rs +++ b/rs/replica/setup_ic_network/src/lib.rs @@ -47,7 +47,7 @@ use ic_registry_subnet_type::SubnetType; use ic_replicated_state::ReplicatedState; use ic_state_manager::state_sync::types::StateSyncMessage; use ic_types::{ - NodeId, SubnetId, + NodeId, ReplicaVersion, SubnetId, artifact::UnvalidatedArtifactMutation, canister_http::{ CanisterHttpPaymentReceipt, CanisterHttpRequest, CanisterHttpResponse, @@ -336,6 +336,7 @@ pub fn setup_consensus_and_p2p( node_id: NodeId, subnet_id: SubnetId, subnet_type: SubnetType, + replica_version: ReplicaVersion, tls_config: Arc, state_manager: Arc>, state_sync_client: Arc>, @@ -439,6 +440,7 @@ pub fn setup_consensus_and_p2p( node_id, subnet_id, subnet_type, + replica_version, artifact_pools, channels, Arc::clone(&consensus_crypto) as Arc<_>, @@ -470,6 +472,7 @@ fn start_consensus( node_id: NodeId, subnet_id: SubnetId, subnet_type: SubnetType, + replica_version: ReplicaVersion, artifact_pools: ArtifactPools, abortable_broadcast_channels: AbortableBroadcastChannels, // ConsensusCrypto is an extension of the Crypto trait and we can @@ -541,7 +544,11 @@ fn start_consensus( )); // ------------------------------------------------------------------------ - let replica_config = ReplicaConfig { node_id, subnet_id }; + let replica_config = ReplicaConfig { + node_id, + subnet_id, + replica_version: replica_version.clone(), + }; let dkg_key_manager = Arc::new(Mutex::new(ic_consensus_dkg::DkgKeyManager::new( metrics_registry.clone(), Arc::clone(&consensus_crypto), @@ -593,7 +600,7 @@ fn start_consensus( // Create the certification client. let certifier = CertifierImpl::new( - replica_config, + replica_config.clone(), Arc::clone(®istry_client), Arc::clone(&certifier_crypto), Arc::clone(&state_manager) as Arc<_>, @@ -612,8 +619,7 @@ fn start_consensus( join_handles.push(create_artifact_handler( abortable_broadcast_channels.dkg, ic_consensus_dkg::DkgImpl::new( - node_id, - subnet_id, + replica_config.clone(), Arc::clone(®istry_client), Arc::clone(&state_manager) as Arc<_>, Arc::clone(&consensus_crypto), @@ -661,7 +667,7 @@ fn start_consensus( Arc::new(Mutex::new(canister_http_adapter_client)), Arc::clone(&consensus_crypto), Arc::clone(&consensus_pool_cache), - ReplicaConfig { subnet_id, node_id }, + replica_config, subnet_type, Arc::clone(®istry_client), metrics_registry.clone(), diff --git a/rs/replica/src/setup.rs b/rs/replica/src/setup.rs index 9bd10c8f59ce..9607f392a323 100644 --- a/rs/replica/src/setup.rs +++ b/rs/replica/src/setup.rs @@ -10,9 +10,7 @@ use ic_registry_client::client::RegistryClientImpl; use ic_registry_client_helpers::subnet::{SubnetListRegistry, SubnetRegistry}; use ic_registry_local_store::LocalStoreImpl; use ic_registry_subnet_type::SubnetType; -use ic_types::{ - NodeId, RegistryVersion, ReplicaVersion, SubnetId, consensus::catchup::CatchUpPackage, -}; +use ic_types::{NodeId, RegistryVersion, SubnetId, consensus::catchup::CatchUpPackage}; use std::{env, path::PathBuf, sync::Arc}; /// Parse command-line args into `ReplicaArgs` @@ -27,24 +25,6 @@ pub fn parse_args() -> Result { }) } -/// Set the Replica version passed in via command-line -pub fn set_replica_version(args: &Result, logger: &ReplicaLogger) { - if let Ok(args) = args { - info!( - logger, - "Setting replica version to: {}", - args.replica_version.as_ref() - ); - if ReplicaVersion::set_default_version(args.replica_version.clone()).is_err() { - warn!( - logger, - "Failed to set replica version, defaulting to: {}", - ReplicaVersion::default().as_ref() - ); - } - } -} - /// Parse the catch-up package given via command-line args (if one was given) pub fn get_catch_up_package( replica_args: &Result, diff --git a/rs/replica/src/setup_ic_stack.rs b/rs/replica/src/setup_ic_stack.rs index aaad9030421a..60a5a221cd5b 100644 --- a/rs/replica/src/setup_ic_stack.rs +++ b/rs/replica/src/setup_ic_stack.rs @@ -28,7 +28,7 @@ use ic_replicated_state::{ReplicatedState, metrics::ReplicatedStateInvariants}; use ic_state_manager::{StateManagerImpl, state_sync::StateSync}; use ic_tracing::ReloadHandles; use ic_types::{ - Height, NodeId, SubnetId, + Height, NodeId, ReplicaVersion, SubnetId, artifact::UnvalidatedArtifactMutation, consensus::{CatchUpPackage, HasHeight}, messages::SignedIngress, @@ -67,6 +67,7 @@ pub fn construct_ic_stack( config: Config, node_id: NodeId, subnet_id: SubnetId, + replica_version: ReplicaVersion, registry: Arc, crypto: Arc, catch_up_package: Option, @@ -137,11 +138,13 @@ pub fn construct_ic_stack( create_consensus_pool_dir(&config); ensure_persistent_pool_replica_version_compatibility( artifact_pool_config.persistent_pool_db_path(), + &replica_version, ); let consensus_pool = Arc::new(RwLock::new(ConsensusPoolImpl::new( node_id, subnet_id, + &replica_version, // Note: it's important to pass the original proto which came from the command line (as // opposed to, for example, a proto which was first deserialized and then serialized // again). Since the proto file could have been produced and signed by nodes running a @@ -317,6 +320,7 @@ pub fn construct_ic_stack( node_id, subnet_id, subnet_type, + replica_version.clone(), Arc::clone(&crypto) as Arc<_>, Arc::clone(&state_manager) as Arc<_>, Arc::new(state_sync) as Arc<_>, @@ -354,6 +358,7 @@ pub fn construct_ic_stack( Arc::clone(&crypto) as Arc<_>, node_id, subnet_id, + replica_version, root_subnet_id, log.clone(), consensus_pool_cache, diff --git a/rs/replica_tests/src/lib.rs b/rs/replica_tests/src/lib.rs index 28cfaf27830f..42bf2bf82316 100644 --- a/rs/replica_tests/src/lib.rs +++ b/rs/replica_tests/src/lib.rs @@ -31,11 +31,11 @@ use ic_state_machine_tests::StateMachine; use ic_test_utilities::universal_canister::UNIVERSAL_CANISTER_WASM; use ic_test_utilities_logger::with_test_replica_logger; use ic_test_utilities_types::{ - ids::{node_test_id, user_anonymous_id}, + ids::{node_test_id, test_replica_version, user_anonymous_id}, messages::SignedIngressBuilder, }; use ic_types::{ - CanisterId, Height, NodeId, ReplicaVersion, Time, + CanisterId, Height, NodeId, Time, artifact::UnvalidatedArtifactMutation, ingress::{IngressState, IngressStatus, WasmResult}, messages::{Query, QuerySource, SignedIngress}, @@ -223,13 +223,15 @@ pub fn get_ic_config() -> IcConfig { }, ); + let replica_version = test_replica_version(); + let mut topology_config: TopologyConfig = TopologyConfig::default(); topology_config.insert_subnet( subnet_index, SubnetConfig::new( subnet_index, subnet_nodes, - ReplicaVersion::default(), + replica_version.clone(), /*max_ingress_bytes_per_message=*/ None, /*max_ingress_bytes_per_block=*/ None, /*max_ingress_messages_per_block=*/ None, @@ -262,7 +264,7 @@ pub fn get_ic_config() -> IcConfig { IcConfig::new( prep_dir, topology_config, - ReplicaVersion::default(), + replica_version, /* generate_subnet_records= */ true, /* nns_subnet_id= */ Some(subnet_index), /* release_package_url= */ None, @@ -290,6 +292,8 @@ where let metrics_registry = MetricsRegistry::new(); + let replica_version = ic_config.initial_replica_version_id.clone(); + let init_ic = ic_config.initialize().expect("can't fail"); let init_subnet = init_ic.initialized_topology.values().next().unwrap(); @@ -348,6 +352,7 @@ where config.clone(), temp_node, subnet_id, + replica_version, registry.clone(), crypto, None, diff --git a/rs/state_machine_tests/src/lib.rs b/rs/state_machine_tests/src/lib.rs index 2838bc958c22..aed278170f7f 100644 --- a/rs/state_machine_tests/src/lib.rs +++ b/rs/state_machine_tests/src/lib.rs @@ -140,10 +140,11 @@ use ic_test_utilities_registry::{ SubnetRecordBuilder, add_single_subnet_record, add_subnet_key_record, add_subnet_list_record, }; use ic_test_utilities_time::FastForwardTimeSource; +use ic_test_utilities_types::ids::test_replica_version; pub use ic_types::ingress::WasmResult; use ic_types::{ CanisterId, CountBytes, CryptoHashOfPartialState, CryptoHashOfState, Height, NodeId, NumBytes, - PrincipalId, Randomness, RegistryVersion, ReplicaVersion, SnapshotId, SubnetId, UserId, + PrincipalId, Randomness, RegistryVersion, SnapshotId, SubnetId, UserId, artifact::IngressMessageId, batch::{ Batch, BatchContent, BatchMessages, BatchSummary, BlockmakerMetrics, CanisterHttpSpent, @@ -352,7 +353,7 @@ pub fn add_initial_registry_records(registry_data_provider: Arc, dkg_payload_builder: Box DkgPayload>, membership: Membership, + replica_version: ReplicaVersion, } pub struct Round<'a> { @@ -178,6 +179,7 @@ impl TestConsensusPool { pub fn new( node_id: NodeId, subnet_id: SubnetId, + replica_version: ReplicaVersion, pool_config: ArtifactPoolConfig, time_source: Arc, registry_client: Arc, @@ -215,6 +217,7 @@ impl TestConsensusPool { let pool = ConsensusPoolImpl::new( node_id, subnet_id, + &replica_version, ic_test_utilities_consensus::make_genesis(summary).into(), pool_config, ic_metrics::MetricsRegistry::new(), @@ -229,6 +232,7 @@ impl TestConsensusPool { time_source, dkg_payload_builder, membership, + replica_version, } } @@ -294,6 +298,7 @@ impl TestConsensusPool { block.context.time += monotonic_block_increment; block.context.registry_version = registry_version; + block.version = self.replica_version.clone(); let idkg = block.payload.as_ref().as_idkg().cloned(); let dkg_payload = (self.dkg_payload_builder)(self, parent.clone(), &block.context); let payload = match dkg_payload { @@ -316,7 +321,10 @@ impl TestConsensusPool { pub fn make_next_tape(&self) -> RandomTape { let finalized_height = self.validated().finalization().max_height().unwrap(); - RandomTape::fake(RandomTapeContent::new(finalized_height)) + RandomTape::fake(RandomTapeContent::new( + finalized_height, + self.replica_version.clone(), + )) } /// Creates an equivocation proof for the given height and rank. Make sure @@ -325,12 +333,7 @@ impl TestConsensusPool { let signer = self.get_block_maker_by_rank(height, Some(rank)); EquivocationProof { signer, - version: self - .pool - .validated() - .highest_catch_up_package() - .content - .version, + version: self.replica_version.clone(), height, subnet_id: self.subnet_id, hash1: CryptoHashOf::new(CryptoHash(vec![1, 2, 3])), @@ -665,7 +668,11 @@ impl TestConsensusPool { // create RB shares for new blocks for i in 0..rb_shares { - let content = RandomBeaconContent::new(height, ic_types::crypto::crypto_hash(&beacon)); + let content = RandomBeaconContent::new( + height, + ic_types::crypto::crypto_hash(&beacon), + self.replica_version.clone(), + ); let share = RandomBeaconShare { signature: crypto .sign_threshold(&content, &dkg_id) @@ -686,7 +693,11 @@ impl TestConsensusPool { // create notarization shares for new blocks for i in 0..n_shares { let block = &blocks[rand_num.next().unwrap() % blocks.len()]; - let content = NotarizationContent::new(height, block.content.get_hash().clone()); + let content = NotarizationContent::new( + height, + block.content.get_hash().clone(), + self.replica_version.clone(), + ); let share = NotarizationShare { signature: crypto .sign_multi(&content, node_id, RegistryVersion::from(1)) @@ -707,7 +718,11 @@ impl TestConsensusPool { // create finalization shares for new blocks for i in 0..f_shares { let block = &blocks[rand_num.next().unwrap() % blocks.len()]; - let content = FinalizationContent::new(height, block.content.get_hash().clone()); + let content = FinalizationContent::new( + height, + block.content.get_hash().clone(), + self.replica_version.clone(), + ); let share = FinalizationShare { signature: crypto .sign_multi(&content, node_id, RegistryVersion::from(1)) @@ -728,7 +743,11 @@ impl TestConsensusPool { } pub fn notarize(&mut self, block: &BlockProposal) -> Notarization { - let content = NotarizationContent::new(block.height(), block.content.get_hash().clone()); + let content = NotarizationContent::new( + block.height(), + block.content.get_hash().clone(), + self.replica_version.clone(), + ); let notarization = Notarization::fake(content); self.insert_validated(notarization.clone()); @@ -736,7 +755,11 @@ impl TestConsensusPool { } pub fn finalize(&mut self, block: &BlockProposal) -> Finalization { - let content = FinalizationContent::new(block.height(), block.content.get_hash().clone()); + let content = FinalizationContent::new( + block.height(), + block.content.get_hash().clone(), + self.replica_version.clone(), + ); let finalization = Finalization::fake(content); self.insert_validated(finalization.clone()); @@ -744,8 +767,11 @@ impl TestConsensusPool { } pub fn finalize_block(&mut self, block: &Block) { - let content = - FinalizationContent::new(block.height(), ic_types::crypto::crypto_hash(block)); + let content = FinalizationContent::new( + block.height(), + ic_types::crypto::crypto_hash(block), + self.replica_version.clone(), + ); self.insert_validated(Finalization::fake(content)) } @@ -801,7 +827,8 @@ impl TestConsensusPool { } pub fn insert_random_tape(&mut self, height: Height) { - let msg = RandomTape::fake(RandomTapeContent::new(height)).into_message(); + let msg = RandomTape::fake(RandomTapeContent::new(height, self.replica_version.clone())) + .into_message(); let time_source = self.time_source.clone(); self.apply(vec![ChangeAction::AddToValidated( ValidatedConsensusArtifact { diff --git a/rs/test_utilities/consensus/src/fake.rs b/rs/test_utilities/consensus/src/fake.rs index b86195c074d1..233baafa0f79 100644 --- a/rs/test_utilities/consensus/src/fake.rs +++ b/rs/test_utilities/consensus/src/fake.rs @@ -1,12 +1,10 @@ -use std::collections::BTreeMap; - use ic_crypto_test_utils_ni_dkg::dummy_transcript_for_tests_with_params; use ic_crypto_tree_hash::{Digest, Witness}; use ic_interfaces::{ certification::{Verifier, VerifierError}, validation::ValidationResult, }; -use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; +use ic_test_utilities_types::ids::{node_test_id, subnet_test_id, test_replica_version}; use ic_types::{ CryptoHashOfPartialState, Height, NodeId, RegistryVersion, ReplicaVersion, SubnetId, batch::{BatchPayload, ValidationContext}, @@ -28,6 +26,7 @@ use ic_types::{ }, }; use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; pub trait Fake { fn fake() -> Self; @@ -230,7 +229,11 @@ impl FakeContentSigner impl FakeContentSigner<&Block> for NotarizationShare { fn fake(block: &Block, signer: NodeId) -> NotarizationShare { Signed { - content: NotarizationContent::new(block.height, ic_types::crypto::crypto_hash(block)), + content: NotarizationContent::new( + block.height, + ic_types::crypto::crypto_hash(block), + block.version.clone(), + ), signature: MultiSignatureShare::fake(signer), } } @@ -239,9 +242,12 @@ impl FakeContentSigner<&Block> for NotarizationShare { impl FakeContentSigner<&Block> for FinalizationShare { fn fake(block: &Block, signer: NodeId) -> FinalizationShare { let height = block.height; - let block = ic_types::crypto::crypto_hash(block); Signed { - content: FinalizationContent::new(height, block), + content: FinalizationContent::new( + height, + ic_types::crypto::crypto_hash(block), + block.version.clone(), + ), signature: MultiSignatureShare::fake(signer), } } @@ -254,7 +260,11 @@ impl FakeContentSigner<&RandomBeacon> for RandomBeaconShare { signer, }; let height = parent.content.height.increment(); - let beacon = RandomBeaconContent::new(height, ic_types::crypto::crypto_hash(parent)); + let beacon = RandomBeaconContent::new( + height, + ic_types::crypto::crypto_hash(parent), + parent.content.version.clone(), + ); Signed { content: beacon, signature, @@ -269,7 +279,7 @@ impl FakeContentSigner for RandomTapeShare { signer, }; Signed { - content: RandomTapeContent::new(height), + content: RandomTapeContent::new(height, test_replica_version()), signature, } } @@ -295,6 +305,7 @@ impl FromParent for Block { parent.height.increment(), Rank(0), parent.context.clone(), + parent.version.clone(), ) } } @@ -304,6 +315,7 @@ impl FromParent for RandomBeacon { Self::fake(RandomBeaconContent::new( parent.content.height.increment(), ic_types::crypto::crypto_hash(parent), + parent.content.version.clone(), )) } } @@ -354,6 +366,7 @@ impl Verifier for FakeVerifier { #[cfg(test)] mod tests { + use ic_test_utilities_types::ids::test_replica_version; use ic_types::consensus::HasVersion; use super::*; @@ -377,6 +390,7 @@ mod tests { certified_height: Height::from(42), time: ic_types::time::UNIX_EPOCH, }, + test_replica_version(), ); let bytes1 = bincode::serialize(&block).unwrap(); let fake_block = bincode::deserialize::(&bytes1).unwrap(); @@ -404,6 +418,7 @@ mod tests { certified_height: Height::from(42), time: ic_types::time::UNIX_EPOCH, }, + test_replica_version(), ); // fake block is binary compatible diff --git a/rs/test_utilities/consensus/src/lib.rs b/rs/test_utilities/consensus/src/lib.rs index f272790667f6..a02c96396594 100644 --- a/rs/test_utilities/consensus/src/lib.rs +++ b/rs/test_utilities/consensus/src/lib.rs @@ -18,6 +18,7 @@ use ic_replicated_state::{ }, }; use ic_test_utilities_state::ReplicatedStateBuilder; +use ic_test_utilities_types::ids::test_replica_version; use ic_types::{ Height, Time, batch::ValidationContext, @@ -228,6 +229,7 @@ pub fn make_genesis(summary: DkgSummary) -> CatchUpPackage { .unwrap() .dkg_id .clone(); + let replica_version = test_replica_version(); let block = Block::new( Id::from(CryptoHash(Vec::new())), Payload::new( @@ -244,9 +246,14 @@ pub fn make_genesis(summary: DkgSummary) -> CatchUpPackage { registry_version, time: UNIX_EPOCH, }, + replica_version.clone(), ); let random_beacon = Signed { - content: RandomBeaconContent::new(height, Id::from(CryptoHash(Vec::new()))), + content: RandomBeaconContent::new( + height, + Id::from(CryptoHash(Vec::new())), + replica_version, + ), signature: ThresholdSignature { signer: low_dkg_id, signature: CombinedThresholdSigOf::new(CombinedThresholdSig(vec![])), diff --git a/rs/test_utilities/execution_environment/src/lib.rs b/rs/test_utilities/execution_environment/src/lib.rs index 08d96f86d5c6..9888c0794e12 100644 --- a/rs/test_utilities/execution_environment/src/lib.rs +++ b/rs/test_utilities/execution_environment/src/lib.rs @@ -65,6 +65,7 @@ use ic_replicated_state::{ testing::{CanisterQueuesTesting, ReplicatedStateTesting}, }; use ic_test_utilities::state_manager::FakeStateManager; +use ic_test_utilities_types::ids::test_replica_version; use ic_test_utilities_types::messages::{IngressBuilder, RequestBuilder, SignedIngressBuilder}; use ic_types::batch::ChainKeyData; use ic_types::crypto::threshold_sig::ni_dkg::{ @@ -2474,7 +2475,7 @@ impl Default for ExecutionTestBuilder { bitcoin_get_successors_follow_up_responses: BTreeMap::default(), time: UNIX_EPOCH, current_round: ExecutionRound::new(1), - replica_version: ReplicaVersion::default(), + replica_version: test_replica_version(), precompiled_universal_canister: true, cost_schedule: CanisterCyclesCostSchedule::Normal, subnet_admins: BTreeSet::new(), diff --git a/rs/test_utilities/registry/BUILD.bazel b/rs/test_utilities/registry/BUILD.bazel index ba00da254ab3..78a9460fa22c 100644 --- a/rs/test_utilities/registry/BUILD.bazel +++ b/rs/test_utilities/registry/BUILD.bazel @@ -20,6 +20,7 @@ rust_library( "//rs/registry/resource_limits", "//rs/registry/subnet_features", "//rs/registry/subnet_type", + "//rs/test_utilities/types", "//rs/types/cycles", "//rs/types/management_canister_types", "//rs/types/types", diff --git a/rs/test_utilities/registry/Cargo.toml b/rs/test_utilities/registry/Cargo.toml index 47624fbece0b..e2807fd7ec8d 100644 --- a/rs/test_utilities/registry/Cargo.toml +++ b/rs/test_utilities/registry/Cargo.toml @@ -18,6 +18,7 @@ ic-registry-proto-data-provider = { path = "../../registry/proto_data_provider" ic-registry-resource-limits = { path = "../../registry/resource_limits" } ic-registry-subnet-features = { path = "../../registry/subnet_features" } ic-registry-subnet-type = { path = "../../registry/subnet_type" } +ic-test-utilities-types = { path = "../types" } ic-types-cycles = { path = "../../types/cycles" } ic-types = { path = "../../types/types" } ic-management-canister-types-private = { path = "../../types/management_canister_types" } diff --git a/rs/test_utilities/registry/src/lib.rs b/rs/test_utilities/registry/src/lib.rs index 3f57328e8484..dc45d17f9229 100644 --- a/rs/test_utilities/registry/src/lib.rs +++ b/rs/test_utilities/registry/src/lib.rs @@ -23,10 +23,11 @@ use ic_registry_resource_limits::ResourceLimits; use ic_registry_subnet_features::ChainKeyConfig; use ic_registry_subnet_features::SubnetFeatures; use ic_registry_subnet_type::SubnetType; +use ic_test_utilities_types::ids::test_replica_version; use ic_types::crypto::threshold_sig::ThresholdSigPublicKey; use ic_types::crypto::threshold_sig::ni_dkg::NiDkgMasterPublicKeyId; use ic_types::{ - NodeId, PrincipalId, RegistryVersion, ReplicaVersion, SubnetId, + NodeId, PrincipalId, RegistryVersion, SubnetId, crypto::threshold_sig::ni_dkg::{NiDkgTag, NiDkgTranscript}, }; use ic_types_cycles::CanisterCyclesCostSchedule; @@ -243,7 +244,7 @@ pub fn test_subnet_record() -> SubnetRecord { max_block_payload_size: 4 * 1024 * 1024, unit_delay_millis: 500, initial_notary_delay_millis: INITIAL_NOTARY_DELAY.as_millis() as u64, - replica_version_id: ReplicaVersion::default().into(), + replica_version_id: test_replica_version().to_string(), dkg_interval_length: 59, dkg_dealings_per_block: 1, start_as_nns: false, diff --git a/rs/test_utilities/types/src/batch/batch_builder.rs b/rs/test_utilities/types/src/batch/batch_builder.rs index 7ee834da8b07..ebc9cd2eee46 100644 --- a/rs/test_utilities/types/src/batch/batch_builder.rs +++ b/rs/test_utilities/types/src/batch/batch_builder.rs @@ -1,5 +1,5 @@ use ic_types::{ - Height, Randomness, RegistryVersion, ReplicaVersion, Time, + Height, Randomness, RegistryVersion, Time, batch::{Batch, BatchContent, BatchMessages, BlockmakerMetrics}, time::UNIX_EPOCH, }; @@ -26,7 +26,7 @@ impl Default for BatchBuilder { registry_version: RegistryVersion::from(1), time: UNIX_EPOCH, blockmaker_metrics: BlockmakerMetrics::new_for_test(), - replica_version: ReplicaVersion::default(), + replica_version: crate::ids::test_replica_version(), }, } } diff --git a/rs/tests/consensus/subnet_recovery/common.rs b/rs/tests/consensus/subnet_recovery/common.rs index 08bae8a1c5a8..0516a5d3a47e 100644 --- a/rs/tests/consensus/subnet_recovery/common.rs +++ b/rs/tests/consensus/subnet_recovery/common.rs @@ -70,6 +70,7 @@ use slog::{Logger, info}; use std::{ collections::{BTreeMap, HashMap}, convert::TryFrom, + str::FromStr, }; use std::{io::Read, time::Duration}; use std::{io::Write, path::Path}; @@ -907,7 +908,7 @@ fn corrupt_latest_cup( CupCorruption::CorruptedWithValidNiDkgId => { info!(logger, "Modifying CUP replica version"); cup.content.block.as_mut().version = - ReplicaVersion::try_from("invalid_version").unwrap(); + ReplicaVersion::from_str("invalid_version").unwrap(); pb::CatchUpPackage::from(cup) } CupCorruption::CorruptedIncludingInvalidNiDkgId => { diff --git a/rs/tests/testnets/mainnet_nns/src/lib.rs b/rs/tests/testnets/mainnet_nns/src/lib.rs index a476da15e2d4..f490832af04a 100644 --- a/rs/tests/testnets/mainnet_nns/src/lib.rs +++ b/rs/tests/testnets/mainnet_nns/src/lib.rs @@ -654,7 +654,7 @@ async fn test_recovered_nns(env: &TestEnv, nns_node: &IcNodeSnapshot) { ProposalWithMainnetState::elect_replica_version( nns_node, &env.topology_snapshot(), - &ReplicaVersion::try_from("1111111111111111111111111111111111111111").unwrap(), + &ReplicaVersion::from_str("1111111111111111111111111111111111111111").unwrap(), &logger, "2222222222222222222222222222222222222222222222222222222222222222".to_string(), None, diff --git a/rs/types/types/BUILD.bazel b/rs/types/types/BUILD.bazel index cc322470a8a4..c51ba0e643be 100644 --- a/rs/types/types/BUILD.bazel +++ b/rs/types/types/BUILD.bazel @@ -96,7 +96,6 @@ rust_test( "@crate_index//:rand", "@crate_index//:rand_chacha", "@crate_index//:rstest", - "@crate_index//:rusty-fork", "@crate_index//:serde", "@crate_index//:serde_bytes", "@crate_index//:serde_cbor", diff --git a/rs/types/types/Cargo.toml b/rs/types/types/Cargo.toml index bf7703866945..9941243493a8 100644 --- a/rs/types/types/Cargo.toml +++ b/rs/types/types/Cargo.toml @@ -55,7 +55,6 @@ ic-types-test-utils = { path = "../types_test_utils" } pretty_assertions = { workspace = true } rand_chacha = { workspace = true } rstest = { workspace = true } -rusty-fork = { workspace = true } [target.'cfg(not(target_arch = "wasm32"))'.dev-dependencies] proptest = { workspace = true } diff --git a/rs/types/types/src/batch/canister_http.rs b/rs/types/types/src/batch/canister_http.rs index 5be3422e593b..4f99033897bb 100644 --- a/rs/types/types/src/batch/canister_http.rs +++ b/rs/types/types/src/batch/canister_http.rs @@ -874,7 +874,7 @@ mod tests { ])), content_size: 42, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: ReplicaVersion::try_from("test_replica_version").unwrap(), }, payment_receipt: CanisterHttpPaymentReceipt { spent: Cycles::new(42), diff --git a/rs/types/types/src/canister_http.rs b/rs/types/types/src/canister_http.rs index fd9b98e63bfb..8187775802df 100644 --- a/rs/types/types/src/canister_http.rs +++ b/rs/types/types/src/canister_http.rs @@ -1435,6 +1435,7 @@ mod tests { }; use ic_types_test_utils::ids::node_test_id; use rstest::rstest; + use std::str::FromStr; use strum::IntoEnumIterator; /// The signed bytes of a [`CanisterHttpResponseReceipt`] must round-trip, for @@ -1475,7 +1476,7 @@ mod tests { content_hash: CryptoHashOf::new(CryptoHash(vec![0; 32])), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: ReplicaVersion::from_str("foobar_version").unwrap(), }, payment_receipt: CanisterHttpPaymentReceipt { spent }, } diff --git a/rs/types/types/src/consensus.rs b/rs/types/types/src/consensus.rs index 1d7a2e6ca15a..7e71f52647c8 100644 --- a/rs/types/types/src/consensus.rs +++ b/rs/types/types/src/consensus.rs @@ -287,9 +287,10 @@ impl Block { height: Height, rank: Rank, context: ValidationContext, + version: ReplicaVersion, ) -> Self { Block { - version: ReplicaVersion::default(), + version, parent, payload, height, @@ -429,9 +430,9 @@ pub struct NotarizationContent { impl NotarizationContent { /// Create a new notarization content from a height and a block hash - pub fn new(height: Height, block: CryptoHashOf) -> Self { + pub fn new(height: Height, block: CryptoHashOf, version: ReplicaVersion) -> Self { NotarizationContent { - version: ReplicaVersion::default(), + version, height, block, } @@ -529,9 +530,9 @@ pub struct FinalizationContent { } impl FinalizationContent { - pub fn new(height: Height, block: CryptoHashOf) -> Self { + pub fn new(height: Height, block: CryptoHashOf, version: ReplicaVersion) -> Self { FinalizationContent { - version: ReplicaVersion::default(), + version, height, block, } @@ -637,9 +638,13 @@ pub type HashedRandomBeacon = Hashed, RandomBeacon>; impl RandomBeaconContent { /// Create a new RandomBeaconContent with a given height and parent /// RandomBeacon - pub fn new(height: Height, parent: CryptoHashOf) -> Self { + pub fn new( + height: Height, + parent: CryptoHashOf, + version: ReplicaVersion, + ) -> Self { Self { - version: ReplicaVersion::default(), + version, height, parent, } @@ -741,11 +746,8 @@ impl SignedBytesWithoutDomainSeparator for RandomTapeContent { impl RandomTapeContent { /// Create a new RandomTapeContent from a given height - pub fn new(height: Height) -> Self { - RandomTapeContent { - version: ReplicaVersion::default(), - height, - } + pub fn new(height: Height, version: ReplicaVersion) -> Self { + RandomTapeContent { version, height } } } diff --git a/rs/types/types/src/consensus/dkg.rs b/rs/types/types/src/consensus/dkg.rs index 8abf967bd22d..56c37e0e12ad 100644 --- a/rs/types/types/src/consensus/dkg.rs +++ b/rs/types/types/src/consensus/dkg.rs @@ -88,9 +88,9 @@ pub struct DealingContent { impl DealingContent { /// Create a new DealingContent - pub fn new(dealing: NiDkgDealing, dkg_id: NiDkgId) -> Self { + pub fn new(dealing: NiDkgDealing, dkg_id: NiDkgId, version: ReplicaVersion) -> Self { DealingContent { - version: ReplicaVersion::default(), + version, dealing, dkg_id, } diff --git a/rs/types/types/src/crypto/hash/tests.rs b/rs/types/types/src/crypto/hash/tests.rs index 37a795d0a48d..4b40668a8971 100644 --- a/rs/types/types/src/crypto/hash/tests.rs +++ b/rs/types/types/src/crypto/hash/tests.rs @@ -128,6 +128,7 @@ mod crypto_hash_stability { use ic_protobuf::types::v1 as pb; use ic_types_cycles::Cycles; use std::collections::BTreeMap; + use std::str::FromStr; use std::sync::Arc; /// Helper to create a deterministic CryptoHashOf from a byte @@ -135,6 +136,12 @@ mod crypto_hash_stability { CryptoHashOf::new(CryptoHash(vec![byte; 32])) } + fn replica_version_for_stability() -> ReplicaVersion { + // The hashes in the test were calculated with this replica version, changing the value + // changes the hashes. + ReplicaVersion::from_str("0.9.0").unwrap() + } + /// Test stability of CryptoHashableTestDummy hash output #[test] fn crypto_hashable_test_dummy_stability() { @@ -174,7 +181,7 @@ mod crypto_hash_stability { /// Test stability of RandomTapeContent hash output #[test] fn random_tape_content_stability() { - let data = RandomTapeContent::new(Height::from(42)); + let data = RandomTapeContent::new(Height::from(42), replica_version_for_stability()); let hash = crypto_hash(&data); assert_eq!( hex::encode(hash.get_ref().0.as_slice()), @@ -186,7 +193,11 @@ mod crypto_hash_stability { /// Test stability of NotarizationContent hash output #[test] fn notarization_content_stability() { - let data = NotarizationContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let data = NotarizationContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); let hash = crypto_hash(&data); assert_eq!( hex::encode(hash.get_ref().0.as_slice()), @@ -198,7 +209,11 @@ mod crypto_hash_stability { /// Test stability of Notarization hash output #[test] fn notarization_stability() { - let content = NotarizationContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let content = NotarizationContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); let data: Notarization = Signed { content, signature: MultiSignature { @@ -217,7 +232,11 @@ mod crypto_hash_stability { /// Test stability of NotarizationShare hash output #[test] fn notarization_share_stability() { - let content = NotarizationContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let content = NotarizationContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); let data: NotarizationShare = Signed { content, signature: MultiSignatureShare { @@ -236,7 +255,11 @@ mod crypto_hash_stability { /// Test stability of FinalizationContent hash output #[test] fn finalization_content_stability() { - let data = FinalizationContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let data = FinalizationContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); let hash = crypto_hash(&data); assert_eq!( hex::encode(hash.get_ref().0.as_slice()), @@ -248,7 +271,11 @@ mod crypto_hash_stability { /// Test stability of Finalization hash output #[test] fn finalization_stability() { - let content = FinalizationContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let content = FinalizationContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); let data: Finalization = Signed { content, signature: MultiSignature { @@ -267,7 +294,11 @@ mod crypto_hash_stability { /// Test stability of FinalizationShare hash output #[test] fn finalization_share_stability() { - let content = FinalizationContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let content = FinalizationContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); let data: FinalizationShare = Signed { content, signature: MultiSignatureShare { @@ -286,7 +317,11 @@ mod crypto_hash_stability { /// Test stability of RandomBeaconContent hash output #[test] fn random_beacon_content_stability() { - let data = RandomBeaconContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let data = RandomBeaconContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); let hash = crypto_hash(&data); assert_eq!( hex::encode(hash.get_ref().0.as_slice()), @@ -298,7 +333,11 @@ mod crypto_hash_stability { /// Test stability of RandomBeacon hash output #[test] fn random_beacon_stability() { - let content = RandomBeaconContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let content = RandomBeaconContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); let data: RandomBeacon = Signed { content, signature: ThresholdSignature { @@ -317,7 +356,11 @@ mod crypto_hash_stability { /// Test stability of RandomBeaconShare hash output #[test] fn random_beacon_share_stability() { - let content = RandomBeaconContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let content = RandomBeaconContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); let data: RandomBeaconShare = Signed { content, signature: ThresholdSignatureShare { @@ -336,7 +379,7 @@ mod crypto_hash_stability { /// Test stability of RandomTape hash output #[test] fn random_tape_stability() { - let content = RandomTapeContent::new(Height::from(42)); + let content = RandomTapeContent::new(Height::from(42), replica_version_for_stability()); let data: RandomTape = Signed { content, signature: ThresholdSignature { @@ -355,7 +398,7 @@ mod crypto_hash_stability { /// Test stability of RandomTapeShare hash output #[test] fn random_tape_share_stability() { - let content = RandomTapeContent::new(Height::from(42)); + let content = RandomTapeContent::new(Height::from(42), replica_version_for_stability()); let data: RandomTapeShare = Signed { content, signature: ThresholdSignatureShare { @@ -377,7 +420,7 @@ mod crypto_hash_stability { let dealing = NiDkgDealing { internal_dealing: ni_dkg_csp_dealing(0x42), }; - let data = DealingContent::new(dealing, test_ni_dkg_id()); + let data = DealingContent::new(dealing, test_ni_dkg_id(), replica_version_for_stability()); let hash = crypto_hash(&data); assert_eq!( hex::encode(hash.get_ref().0.as_slice()), @@ -392,7 +435,8 @@ mod crypto_hash_stability { let dealing = NiDkgDealing { internal_dealing: ni_dkg_csp_dealing(0x42), }; - let content = DealingContent::new(dealing, test_ni_dkg_id()); + let content = + DealingContent::new(dealing, test_ni_dkg_id(), replica_version_for_stability()); let data: DkgMessage = Signed { content, signature: BasicSignature { @@ -481,7 +525,11 @@ mod crypto_hash_stability { /// Helper to create a test RandomBeacon for CatchUp content fn test_random_beacon() -> RandomBeacon { - let content = RandomBeaconContent::new(Height::from(42), test_crypto_hash_of(0x42)); + let content = RandomBeaconContent::new( + Height::from(42), + test_crypto_hash_of(0x42), + replica_version_for_stability(), + ); Signed { content, signature: ThresholdSignature { @@ -937,6 +985,7 @@ mod crypto_hash_stability { certified_height: Height::from(41), time: UNIX_EPOCH, }, + replica_version_for_stability(), ); let hash = crypto_hash(&data); assert_eq!( @@ -965,6 +1014,7 @@ mod crypto_hash_stability { certified_height: Height::from(41), time: UNIX_EPOCH, }, + replica_version_for_stability(), ) } @@ -993,7 +1043,7 @@ mod crypto_hash_stability { fn equivocation_proof_stability() { let data = EquivocationProof { signer: NodeId::from(PrincipalId::new_node_test_id(42)), - version: ReplicaVersion::default(), + version: replica_version_for_stability(), height: Height::from(42), subnet_id: SubnetId::from(PrincipalId::new_subnet_test_id(42)), hash1: test_crypto_hash_of(0x42), @@ -1073,7 +1123,7 @@ mod crypto_hash_stability { content_hash: test_crypto_hash_of(0x42), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_version_for_stability(), }; let hash = crypto_hash(&data); assert_eq!( @@ -1091,7 +1141,7 @@ mod crypto_hash_stability { content_hash: test_crypto_hash_of(0x42), content_size: 0, is_reject: false, - replica_version: ReplicaVersion::default(), + replica_version: replica_version_for_stability(), }; let receipt_share = CanisterHttpResponseReceipt { metadata, diff --git a/rs/types/types/src/replica_config.rs b/rs/types/types/src/replica_config.rs index bec633e97bff..d12f7dc28161 100644 --- a/rs/types/types/src/replica_config.rs +++ b/rs/types/types/src/replica_config.rs @@ -1,5 +1,5 @@ //! Defines the [`ReplicaConfig`]. -use crate::{NodeId, PrincipalId, SubnetId}; +use crate::{NodeId, ReplicaVersion, SubnetId}; use serde::{Deserialize, Serialize}; pub const NODE_INDEX_DEFAULT: u64 = 0; @@ -10,19 +10,5 @@ pub const SUBNET_ID_DEFAULT: u64 = 0; pub struct ReplicaConfig { pub node_id: NodeId, pub subnet_id: SubnetId, -} - -impl ReplicaConfig { - pub fn new(node_id: NodeId, subnet_id: SubnetId) -> ReplicaConfig { - Self { node_id, subnet_id } - } -} - -impl Default for ReplicaConfig { - fn default() -> Self { - ReplicaConfig::new( - NodeId::from(PrincipalId::new_node_test_id(NODE_INDEX_DEFAULT)), - SubnetId::from(PrincipalId::new_subnet_test_id(SUBNET_ID_DEFAULT)), - ) - } + pub replica_version: ReplicaVersion, } diff --git a/rs/types/types/src/replica_version.rs b/rs/types/types/src/replica_version.rs index 8f1af5c5bf1c..f32f5b6a9412 100644 --- a/rs/types/types/src/replica_version.rs +++ b/rs/types/types/src/replica_version.rs @@ -5,43 +5,17 @@ use std::convert::TryFrom; use std::error::Error; use std::fmt; use std::str::FromStr; +use std::sync::Arc; -#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Deserialize, Serialize)] -pub struct ReplicaVersion { - version_id: String, -} - -static DEFAULT_VERSION_ID: OnceCell = OnceCell::new(); pub static REPLICA_BINARY_HASH: OnceCell = OnceCell::new(); -/// The default replica version can be initialized only once to prevent -/// accidental mistakes. Otherwise its value is taken from environment -/// CARGO_PKG_VERSION at compile time. -impl ReplicaVersion { - /// Set the default value, which can only be set once. - /// Return error when it is already set. - pub fn set_default_version( - version: ReplicaVersion, - ) -> Result<(), DefaultVersionAlreadySetError> { - DEFAULT_VERSION_ID - .set(version.version_id) - .map_err(|_| DefaultVersionAlreadySetError) - } -} - -impl Default for ReplicaVersion { - fn default() -> Self { - ReplicaVersion { - version_id: DEFAULT_VERSION_ID - .get_or_init(|| env!("CARGO_PKG_VERSION").to_string()) - .clone(), - } - } +#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Deserialize, Serialize)] +pub struct ReplicaVersion { + #[serde(serialize_with = "ic_utils::serde_arc::serialize_arc")] + #[serde(deserialize_with = "ic_utils::serde_arc::deserialize_arc_str")] + version_id: Arc, } -#[derive(Eq, PartialEq, Debug)] -pub struct DefaultVersionAlreadySetError; - impl std::fmt::Display for ReplicaVersion { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { self.version_id.fmt(f) @@ -56,7 +30,7 @@ impl From for String { impl From<&ReplicaVersion> for String { fn from(version: &ReplicaVersion) -> String { - version.version_id.clone() + version.version_id.to_string() } } @@ -87,7 +61,7 @@ impl TryFrom<&str> for ReplicaVersion { Err(ReplicaVersionParseError(version_str.to_string())) } else { Ok(ReplicaVersion { - version_id: version_str.to_string(), + version_id: version_str.into(), }) } } @@ -122,31 +96,9 @@ mod test { #[test] fn test_parse_replica_version() { - assert!(ReplicaVersion::try_from("2020-09-25.0.1").is_ok()); - assert!(ReplicaVersion::try_from("1.2.1").is_ok()); - assert!(ReplicaVersion::try_from("8aefz17q_1").is_ok()); - assert!(ReplicaVersion::try_from("?+").is_err()); - assert!(ReplicaVersion::try_from(ReplicaVersion::default().as_ref()).is_ok()); - } - - #[test] - fn test_replica_default_version() { - assert_eq!( - ReplicaVersion::default().as_ref(), - env!("CARGO_PKG_VERSION") - ); - let version = ReplicaVersion::try_from("1.2.1").unwrap(); - assert!(ReplicaVersion::set_default_version(version).is_err()); - } - - use rusty_fork::rusty_fork_test; - rusty_fork_test! { - #[test] - fn test_replica_default_version_can_be_set_once() { - let version = ReplicaVersion::try_from("1.2.1").unwrap(); - assert!(ReplicaVersion::set_default_version(version.clone()).is_ok()); - assert_eq!(ReplicaVersion::default(), version); - assert!(ReplicaVersion::set_default_version(version).is_err()); - } + assert!(ReplicaVersion::from_str("2020-09-25.0.1").is_ok()); + assert!(ReplicaVersion::from_str("1.2.1").is_ok()); + assert!(ReplicaVersion::from_str("8aefz17q_1").is_ok()); + assert!(ReplicaVersion::from_str("?+").is_err()); } } diff --git a/rs/types/types_test_utils/src/ids.rs b/rs/types/types_test_utils/src/ids.rs index 1d8f759f7246..01889e56f7b3 100644 --- a/rs/types/types_test_utils/src/ids.rs +++ b/rs/types/types_test_utils/src/ids.rs @@ -1,7 +1,8 @@ use ic_types::{ - CanisterId, NodeId, PrincipalId, SubnetId, UserId, + CanisterId, NodeId, PrincipalId, ReplicaVersion, SubnetId, UserId, messages::{CallContextId, EXPECTED_MESSAGE_ID_LENGTH, MessageId}, }; +use std::str::FromStr; pub const NODE_1: NodeId = NodeId::new(PrincipalId::new( 10, @@ -153,6 +154,10 @@ pub fn node_test_id(i: u64) -> NodeId { NodeId::from(PrincipalId::new_node_test_id(i)) } +pub fn test_replica_version() -> ReplicaVersion { + ReplicaVersion::from_str("cafebabe0000ffff0000ffff0000ffff0000ffff").unwrap() +} + /// Converts a [`NodeId`] to a [`u64`]. /// /// This is meant to be used in tests only. diff --git a/rs/utils/src/serde_arc.rs b/rs/utils/src/serde_arc.rs index 36036c8960fd..cc8f0643f1b1 100644 --- a/rs/utils/src/serde_arc.rs +++ b/rs/utils/src/serde_arc.rs @@ -36,7 +36,7 @@ use std::sync::Arc; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -pub fn serialize_arc( +pub fn serialize_arc( data: &Arc, serializer: S, ) -> Result { @@ -48,3 +48,10 @@ pub fn deserialize_arc<'de, T: Deserialize<'de>, D: Deserializer<'de>>( ) -> Result, D::Error> { T::deserialize(deserializer).map(Arc::new) } + +pub fn deserialize_arc_str<'de, D: Deserializer<'de>>( + deserializer: D, +) -> Result, D::Error> { + let s = String::deserialize(deserializer)?; + Ok(Arc::from(s)) +}