diff --git a/Cargo.lock b/Cargo.lock index 979fcd921782..28f3e9059472 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7536,7 +7536,6 @@ dependencies = [ "ic-artifact-pool", "ic-canonical-state", "ic-canonical-state-tree-hash", - "ic-config", "ic-consensus-mocks", "ic-consensus-utils", "ic-crypto-test-utils-crypto-returning-ok", @@ -7552,7 +7551,6 @@ dependencies = [ "ic-test-utilities", "ic-test-utilities-consensus", "ic-test-utilities-logger", - "ic-test-utilities-registry", "ic-test-utilities-types", "ic-types", "ic-types-test-utils", @@ -7744,6 +7742,7 @@ dependencies = [ "ic-consensus-utils", "ic-crypto-test-utils-crypto-returning-ok", "ic-interfaces", + "ic-interfaces-mocks", "ic-interfaces-state-manager", "ic-logger", "ic-metrics", diff --git a/rs/consensus/certification/BUILD.bazel b/rs/consensus/certification/BUILD.bazel index 574c6f36bc72..abd6b2dcf7bd 100644 --- a/rs/consensus/certification/BUILD.bazel +++ b/rs/consensus/certification/BUILD.bazel @@ -39,7 +39,6 @@ rust_test( "//rs/artifact_pool", "//rs/canonical_state", "//rs/canonical_state/tree_hash", - "//rs/config", "//rs/consensus/mocks", "//rs/consensus/utils", "//rs/crypto/test_utils/crypto_returning_ok", @@ -55,7 +54,6 @@ rust_test( "//rs/test_utilities/artifact_pool", "//rs/test_utilities/consensus", "//rs/test_utilities/logger", - "//rs/test_utilities/registry", "//rs/test_utilities/types", "//rs/types/types", "//rs/types/types_test_utils", diff --git a/rs/consensus/certification/Cargo.toml b/rs/consensus/certification/Cargo.toml index 6163c3b3bfff..ee085c64d89a 100644 --- a/rs/consensus/certification/Cargo.toml +++ b/rs/consensus/certification/Cargo.toml @@ -25,14 +25,12 @@ slog = { workspace = true } assert_matches = { workspace = true } ic-artifact-pool = { path = "../../artifact_pool" } ic-consensus-mocks = { path = "../mocks" } -ic-config = { path = "../../config" } ic-crypto-test-utils-crypto-returning-ok = { path = "../../crypto/test_utils/crypto_returning_ok" } ic-registry-subnet-type = { path = "../../registry/subnet_type" } ic-test-artifact-pool = { path = "../../test_utilities/artifact_pool" } ic-test-utilities = { path = "../../test_utilities" } ic-test-utilities-consensus = { path = "../../test_utilities/consensus" } ic-test-utilities-logger = { path = "../../test_utilities/logger" } -ic-test-utilities-registry = { path = "../../test_utilities/registry" } ic-test-utilities-types = { path = "../../test_utilities/types" } ic-types-test-utils = { path = "../../types/types_test_utils" } mockall = { workspace = true } diff --git a/rs/consensus/certification/src/certifier.rs b/rs/consensus/certification/src/certifier.rs index 69886d36567c..f813f3ed857f 100644 --- a/rs/consensus/certification/src/certifier.rs +++ b/rs/consensus/certification/src/certifier.rs @@ -691,8 +691,7 @@ mod tests { use ic_canonical_state::lazy_tree_conversion::replicated_state_as_lazy_tree; use ic_canonical_state_tree_hash::hash_tree::hash_lazy_tree; use ic_canonical_state_tree_hash::lazy_tree::materialize::materialize_partial; - use ic_config::artifact_pool::ArtifactPoolConfig; - use ic_consensus_mocks::{Dependencies, dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_tree_hash::{Digest, Witness, sparse_labeled_tree_from_paths}; use ic_interfaces::{ certification::CertificationPool, @@ -703,7 +702,6 @@ mod tests { use ic_test_artifact_pool::consensus_pool::TestConsensusPool; use ic_test_utilities_consensus::fake::*; use ic_test_utilities_logger::with_test_replica_logger; - use ic_test_utilities_registry::SubnetRecordBuilder; use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; use ic_types::backwards_compatibility::BackwardsCompatible; use ic_types::consensus::{BlockPayload, HashedBlock, Payload, dkg::SplittingArgs}; @@ -827,7 +825,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 1); + } = DependenciesBuilder::new(pool_config.clone(), 1).build(); let certifier = CertifierImpl::new( replica_config, @@ -863,7 +861,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); pool.advance_round_normal_operation(); add_expectations(state_manager.clone(), 1, 4); let metrics_registry = MetricsRegistry::new(); @@ -929,7 +927,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); pool.advance_round_normal_operation_n(6); add_expectations(state_manager.clone(), 1, 4); @@ -1066,7 +1064,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 6); + } = DependenciesBuilder::new(pool_config.clone(), 6).build(); // make the mock state manager return empty hashes for heights 3, 4 and 5 add_expectations(state_manager.clone(), 3, 5); let metrics_registry = MetricsRegistry::new(); @@ -1146,7 +1144,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 7); + } = DependenciesBuilder::new(pool_config.clone(), 7).build(); pool.insert_beacon_chain(&pool.make_next_beacon(), Height::from(10)); // make the mock state manager return empty hashes for heights 3, 4 and 5 add_expectations(state_manager.clone(), 3, 5); @@ -1219,7 +1217,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); pool.advance_round_normal_operation_n(10); // make the mock state manager return empty hashes for heights 3, 4 and 5 add_expectations(state_manager.clone(), 3, 5); @@ -1291,7 +1289,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 1); + } = DependenciesBuilder::new(pool_config.clone(), 1).build(); pool.advance_round_normal_operation_n(10); // make the mock state manager return empty hashes for heights 3, 4 and 5 add_expectations(state_manager.clone(), 3, 5); @@ -1464,7 +1462,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); // make the mock state manager return empty hashes for heights 4 and 5 add_expectations(state_manager.clone(), 4, 5); let metrics_registry = MetricsRegistry::new(); @@ -1562,7 +1560,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); let metrics_registry = MetricsRegistry::new(); let cert_pool = CertificationPoolImpl::new( @@ -1641,23 +1639,6 @@ mod tests { // DKG interval length used for subnet-splitting tests. const TEST_DKG_INTERVAL: u64 = 9; - fn dependencies_for_splitting_tests( - pool_config: ArtifactPoolConfig, - nodes: u64, - ) -> Dependencies { - let committee = (0..nodes).map(node_test_id).collect::>(); - dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(TEST_DKG_INTERVAL) - .build(), - )], - ) - } - // Advances `pool` by TEST_DKG_INTERVAL rounds so the next block is a DKG // summary block, then inserts and finalizes that summary block after setting // its subnet-splitting status to `status`. @@ -1754,7 +1735,9 @@ mod tests { crypto, state_manager, .. - } = dependencies_for_splitting_tests(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4) + .with_dkg_interval_length(TEST_DKG_INTERVAL) + .build(); let metrics_registry = MetricsRegistry::new(); let cert_pool = CertificationPoolImpl::new( @@ -1816,7 +1799,9 @@ mod tests { crypto, state_manager, .. - } = dependencies_for_splitting_tests(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4) + .with_dkg_interval_length(TEST_DKG_INTERVAL) + .build(); let metrics_registry = MetricsRegistry::new(); let cert_pool = CertificationPoolImpl::new( @@ -1883,7 +1868,9 @@ mod tests { crypto, state_manager, .. - } = dependencies_for_splitting_tests(pool_config.clone(), 1); + } = DependenciesBuilder::new(pool_config.clone(), 1) + .with_dkg_interval_length(TEST_DKG_INTERVAL) + .build(); let certifier = CertifierImpl::new( replica_config, diff --git a/rs/consensus/chain_key/src/lib.rs b/rs/consensus/chain_key/src/lib.rs index 951c15e00300..22cf8f9b471d 100644 --- a/rs/consensus/chain_key/src/lib.rs +++ b/rs/consensus/chain_key/src/lib.rs @@ -657,9 +657,7 @@ fn reject_if_invalid( mod tests { use assert_matches::assert_matches; use core::{convert::From, iter::Iterator, time::Duration}; - use ic_consensus_mocks::{ - Dependencies, dependencies_with_subnet_records_with_raw_state_manager, - }; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_temp_crypto::TempCryptoComponent; use ic_interfaces::consensus::{InvalidPayloadReason, PayloadValidationFailure}; use ic_interfaces::idkg::IDkgChangeAction; @@ -804,11 +802,13 @@ mod tests { registry, registry_data_provider, .. - } = dependencies_with_subnet_records_with_raw_state_manager( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![(1, subnet_record_builder.build())], - ); + ) + .without_state_manager_expectations() + .build(); // Enable the configured keys if let Some(config) = config diff --git a/rs/consensus/dkg/src/dkg_key_manager.rs b/rs/consensus/dkg/src/dkg_key_manager.rs index c533205776da..192be0ee53ce 100644 --- a/rs/consensus/dkg/src/dkg_key_manager.rs +++ b/rs/consensus/dkg/src/dkg_key_manager.rs @@ -557,29 +557,19 @@ fn dkg_id_log_msg(id: &NiDkgId) -> String { #[cfg(test)] mod tests { use super::*; - use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_test_utils_crypto_returning_ok::CryptoReturningOk; use ic_metrics::MetricsRegistry; use ic_test_utilities_logger::with_test_replica_logger; - use ic_test_utilities_registry::SubnetRecordBuilder; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; #[test] fn test_transcripts_get_loaded_and_retained() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { with_test_replica_logger(|logger| { - let nodes: Vec<_> = (0..1).map(node_test_id).collect(); let dkg_interval_len = 3; - let Dependencies { mut pool, .. } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(222), - vec![( - 1, - SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) - .build(), - )], - ); + let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 1) + .with_dkg_interval_length(dkg_interval_len) + .build(); let csp = Arc::new(CryptoReturningOk::default()); let mut key_manager = DkgKeyManager::new( MetricsRegistry::new(), diff --git a/rs/consensus/dkg/src/lib.rs b/rs/consensus/dkg/src/lib.rs index bdf33dfc803a..be56990a6079 100644 --- a/rs/consensus/dkg/src/lib.rs +++ b/rs/consensus/dkg/src/lib.rs @@ -419,10 +419,7 @@ mod tests { }; use core::panic; use ic_artifact_pool::dkg_pool::DkgPoolImpl; - use ic_consensus_mocks::{ - Dependencies, dependencies, dependencies_with_subnet_params, - dependencies_with_subnet_records_with_raw_state_manager, - }; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_consensus_utils::pool_reader::PoolReader; use ic_crypto_test_utils_crypto_returning_ok::CryptoReturningOk; use ic_crypto_test_utils_ni_dkg::dummy_dealing; @@ -508,7 +505,7 @@ mod tests { registry, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -518,7 +515,8 @@ mod tests { .with_chain_key_config(test_vet_key_config()) .build(), )], - ); + ) + .build(); state_manager .get_mut() .expect_get_latest_certified_state() @@ -676,7 +674,7 @@ mod tests { state_manager, replica_config, .. - } = dependencies(pool_config.clone(), 2); + } = DependenciesBuilder::new(pool_config.clone(), 2).build(); state_manager .get_mut() .expect_get_latest_certified_state() @@ -770,26 +768,19 @@ mod tests { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { use ic_types::crypto::threshold_sig::ni_dkg::*; with_test_replica_logger(|logger| { - let node_ids = vec![node_test_id(0), node_test_id(1)]; let dkg_interval_length = 99; - let subnet_id = subnet_test_id(0); let Dependencies { mut pool, crypto, registry, + replica_config, state_manager, dkg_pool, .. - } = dependencies_with_subnet_records_with_raw_state_manager( - pool_config, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_state_manager_expectations() + .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); complement_state_manager_with_setup_initial_dkg_request( @@ -805,7 +796,7 @@ mod tests { new_dkg_key_manager(crypto.clone(), logger.clone(), &PoolReader::new(&pool)); let dkg = DkgImpl::new( node_test_id(1), - subnet_id, + replica_config.subnet_id, registry.clone(), state_manager.clone(), crypto, @@ -935,24 +926,16 @@ mod tests { fn test_config_generation_failures_are_added_to_data_blocks() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { use ic_types::crypto::threshold_sig::ni_dkg::*; - let node_ids = vec![node_test_id(0), node_test_id(1)]; let dkg_interval_length = 99; - let subnet_id = subnet_test_id(0); let Dependencies { mut pool, registry, state_manager, .. - } = dependencies_with_subnet_records_with_raw_state_manager( - pool_config, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_state_manager_expectations() + .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); complement_state_manager_with_setup_initial_dkg_request( @@ -1065,14 +1048,14 @@ mod tests { state_manager: state_manager_1, replica_config: replica_config_1, .. - } = dependencies(pool_config_1, 2); + } = DependenciesBuilder::new(pool_config_1, 2).build(); let Dependencies { pool: consensus_pool_2, registry: registry_2, state_manager: state_manager_2, replica_config: replica_config_2, .. - } = dependencies(pool_config_2, 2); + } = DependenciesBuilder::new(pool_config_2, 2).build(); for state_manager in [&state_manager_1, &state_manager_2] { state_manager .get_mut() @@ -1508,31 +1491,17 @@ mod tests { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config_2| { use ic_types::crypto::threshold_sig::ni_dkg::*; with_test_replica_logger(|logger| { - let node_ids = vec![node_test_id(0), node_test_id(1)]; let dkg_interval_length = 99; - let subnet_id = subnet_test_id(0); // Set pool_1 and pool_2 - let dependencies_1 = dependencies_with_subnet_records_with_raw_state_manager( - pool_config_1, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); - let dependencies_2 = dependencies_with_subnet_records_with_raw_state_manager( - pool_config_2, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); + let dependencies_1 = DependenciesBuilder::new(pool_config_1, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_state_manager_expectations() + .build(); + let dependencies_2 = DependenciesBuilder::new(pool_config_2, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_state_manager_expectations() + .build(); // Return an empty call context when we create the first summary, // so that we later test the case where remote dealing has a different @@ -1716,7 +1685,7 @@ mod tests { ) -> (Dependencies, NiDkgTargetId, Vec) { let node_ids = (1..8).map(node_test_id).collect::>(); - let mut deps = dependencies_with_subnet_records_with_raw_state_manager( + let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -1725,7 +1694,9 @@ mod tests { .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .build(), )], - ); + ) + .without_state_manager_expectations() + .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); complement_state_manager_with_setup_initial_dkg_request( @@ -2096,21 +2067,13 @@ mod tests { fn test_remote_dealing_validation_is_deferred_until_context_exists() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { with_test_replica_logger(|logger| { - let node_ids = vec![node_test_id(0), node_test_id(1)]; let dkg_interval_length = 99; - let subnet_id = subnet_test_id(0); let target_id = NiDkgTargetId::new([9_u8; 32]); - let mut deps = dependencies_with_subnet_records_with_raw_state_manager( - pool_config, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); + let mut deps = DependenciesBuilder::new(pool_config, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_state_manager_expectations() + .build(); // Start without context so remote dealing validation is deferred. complement_state_manager_with_dkg_contexts( @@ -2144,7 +2107,7 @@ mod tests { DkgPoolImpl::new(MetricsRegistry::new(), no_op_logger(), start_height); let remote_dkg_id = NiDkgId { start_block_height: start_height, - dealer_subnet: subnet_id, + dealer_subnet: deps.replica_config.subnet_id, dkg_tag: NiDkgTag::LowThreshold, target_subnet: NiDkgTargetSubnet::Remote(target_id), }; @@ -2152,7 +2115,7 @@ mod tests { let other_target_id = NiDkgTargetId::new([10_u8; 32]); let deferred_remote_dkg_id = NiDkgId { start_block_height: start_height, - dealer_subnet: subnet_id, + dealer_subnet: deps.replica_config.subnet_id, dkg_tag: NiDkgTag::LowThreshold, target_subnet: NiDkgTargetSubnet::Remote(other_target_id), }; @@ -2224,7 +2187,7 @@ mod tests { }; let target_id = NiDkgTargetId::new([0_u8; 32]); - let mut deps = dependencies_with_subnet_records_with_raw_state_manager( + let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -2243,7 +2206,9 @@ mod tests { }) .build(), )], - ); + ) + .without_state_manager_expectations() + .build(); // No contexts at the beginning complement_state_manager_with_dkg_contexts(deps.state_manager.clone(), vec![], None); @@ -2333,7 +2298,7 @@ mod tests { let setup_target_id = NiDkgTargetId::new([1_u8; 32]); let reshare_target_id = NiDkgTargetId::new([2_u8; 32]); - let mut deps = dependencies_with_subnet_records_with_raw_state_manager( + let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -2352,7 +2317,9 @@ mod tests { }) .build(), )], - ); + ) + .without_state_manager_expectations() + .build(); let registry_version = deps.registry.get_latest_version(); let mut contexts = vec![ @@ -2558,7 +2525,7 @@ mod tests { registry, replica_config, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -2568,7 +2535,8 @@ mod tests { .with_chain_key_config(test_vet_key_config()) .build(), )], - ); + ) + .build(); // Get the latest summary block, which is the genesis block let cup = PoolReader::new(&pool).get_highest_catch_up_package(); diff --git a/rs/consensus/dkg/src/payload_builder.rs b/rs/consensus/dkg/src/payload_builder.rs index cd4f4fa8cc95..1ad5b0c3fa2f 100644 --- a/rs/consensus/dkg/src/payload_builder.rs +++ b/rs/consensus/dkg/src/payload_builder.rs @@ -882,10 +882,7 @@ mod tests { }, *, }; - use ic_consensus_mocks::{ - Dependencies, dependencies_with_subnet_params, - dependencies_with_subnet_records_with_raw_state_manager, - }; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_test_utils_ni_dkg::dummy_transcript_for_tests_with_params; use ic_logger::replica_logger::no_op_logger; use ic_management_canister_types_private::{VetKdCurve, VetKdKeyId}; @@ -1086,7 +1083,7 @@ mod tests { let subnet_id = subnet_test_id(0); let vet_key_config = test_vet_key_config(); let key_id = vet_key_config.key_configs[0].key_id.clone(); - let mut deps = dependencies_with_subnet_records_with_raw_state_manager( + let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -1096,7 +1093,9 @@ mod tests { .with_chain_key_config(test_vet_key_config()) .build(), )], - ); + ) + .without_state_manager_expectations() + .build(); let registry_version = deps.registry.get_latest_version(); let setup_target = NiDkgTargetId::new([5_u8; 32]); let reshare_target = NiDkgTargetId::new([6_u8; 32]); @@ -1278,7 +1277,7 @@ mod tests { mut pool, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -1288,7 +1287,8 @@ mod tests { .with_chain_key_config(test_vet_key_config()) .build(), )], - ); + ) + .build(); let cup_contents = registry .get_cup_contents(subnet_id, registry.get_latest_version()) .expect("Failed to retreive the DKG transcripts from registry"); @@ -1369,7 +1369,7 @@ mod tests { let initial_registry_version = 145; let dkg_interval_len = 66; let subnet_id = subnet_test_id(222); - let Dependencies { registry, .. } = dependencies_with_subnet_params( + let Dependencies { registry, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -1379,7 +1379,8 @@ mod tests { .with_chain_key_config(test_vet_key_config()) .build(), )], - ); + ) + .build(); let cup_contents = registry .get_cup_contents(subnet_id, registry.get_latest_version()) @@ -1470,7 +1471,7 @@ mod tests { let initial_registry_version = 112; let Dependencies { registry, mut pool, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -1480,7 +1481,8 @@ mod tests { .with_chain_key_config(test_vet_key_config()) .build(), )], - ); + ) + .build(); let cup_contents = registry .get_cup_contents(subnet_id, registry.get_latest_version()) .expect("Failed to retreive the DKG transcripts from registry"); @@ -1612,7 +1614,7 @@ mod tests { registry, replica_config, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -1621,7 +1623,8 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .build(); // Get the latest summary block, which is the genesis block let cup = PoolReader::new(&pool).get_highest_catch_up_package(); diff --git a/rs/consensus/dkg/src/payload_validator.rs b/rs/consensus/dkg/src/payload_validator.rs index aaba630b5695..20475ae15d1e 100644 --- a/rs/consensus/dkg/src/payload_validator.rs +++ b/rs/consensus/dkg/src/payload_validator.rs @@ -255,7 +255,7 @@ mod tests { use super::*; use crate::{DkgImpl, DkgKeyManager}; use ic_artifact_pool::dkg_pool::DkgPoolImpl; - use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_temp_crypto::{NodeKeysToGenerate, TempCryptoComponent}; use ic_crypto_test_utils_ni_dkg::{dummy_dealing, dummy_transcript_for_tests}; use ic_interfaces::{ @@ -298,27 +298,20 @@ mod tests { fn test_validate_payload() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let dkg_interval_length = 4; - let committee = (0..4).map(node_test_id).collect::>(); let Dependencies { crypto, mut pool, registry, + replica_config, state_manager, dkg_pool, .. - } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 5, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(dkg_interval_length) + .build(); let context = ValidationContext { - registry_version: RegistryVersion::from(5), + registry_version: RegistryVersion::from(1), certified_height: Height::from(0), time: ic_types::time::UNIX_EPOCH, }; @@ -335,7 +328,7 @@ mod tests { .unwrap(); assert!( validate_payload( - subnet_test_id(0), + replica_config.subnet_id, registry.as_ref(), crypto.as_ref(), &PoolReader::new(&pool), @@ -363,7 +356,7 @@ mod tests { .unwrap(); assert!( validate_payload( - subnet_test_id(0), + replica_config.subnet_id, registry.as_ref(), crypto.as_ref(), &PoolReader::new(&pool), @@ -522,7 +515,7 @@ mod tests { registry, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config.clone(), SUBNET_1, vec![( @@ -531,7 +524,8 @@ mod tests { .with_dkg_dealings_per_block(1) .build(), )], - ); + ) + .build(); let mut parent = Block::from(pool.make_next_block()); parent.payload = Payload::new( @@ -625,7 +619,7 @@ mod tests { registry, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config.clone(), subnet_id, vec![( @@ -634,7 +628,8 @@ mod tests { .with_dkg_dealings_per_block(max_dealings_per_payload) .build(), )], - ); + ) + .build(); let mut parent = Block::from(pool.make_next_block()); parent.payload = Payload::new( @@ -728,7 +723,7 @@ mod tests { state_manager, registry_data_provider, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -737,7 +732,8 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .build(); state_manager .get_mut() .expect_get_latest_certified_state() diff --git a/rs/consensus/dkg/src/utils.rs b/rs/consensus/dkg/src/utils.rs index f6027da0a147..5c222f89cc26 100644 --- a/rs/consensus/dkg/src/utils.rs +++ b/rs/consensus/dkg/src/utils.rs @@ -189,7 +189,7 @@ mod tests { use super::{get_dealers_from_chain, get_dkg_dealings}; use crate::test_utils::create_dealing; use crate::utils::vetkd_key_ids_for_subnet; - use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_consensus_utils::pool_reader::PoolReader; use ic_crypto_test_utils_ni_dkg::dummy_transcript_for_tests; use ic_interfaces_registry::RegistryValue; @@ -314,28 +314,24 @@ mod tests { #[test] fn test_get_dkg_dealings_included_and_excluded_by_transcript() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let subnet_id = subnet_test_id(1); - let nodes: Vec<_> = (0..4).map(node_test_id).collect(); let dkg_interval_len = 10; - let Dependencies { mut pool, .. } = dependencies_with_subnet_params( - pool_config, - subnet_id, - vec![( - 1, - SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) - .build(), - )], - ); + let Dependencies { + mut pool, + replica_config, + .. + } = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(dkg_interval_len) + .build(); pool.advance_round_normal_operation_n(dkg_interval_len); let pool_reader = PoolReader::new(&pool); let tip = pool_reader.get_finalized_tip(); // DKG id that has a transcript in this block -> its dealings should be excluded. - let dkg_id_with_transcript = dkg_id(subnet_id, NiDkgTag::HighThreshold); + let dkg_id_with_transcript = dkg_id(replica_config.subnet_id, NiDkgTag::HighThreshold); // DKG id with no transcript -> its dealings should be included. - let dkg_id_without_transcript = dkg_id(subnet_id, NiDkgTag::LowThreshold); + let dkg_id_without_transcript = + dkg_id(replica_config.subnet_id, NiDkgTag::LowThreshold); // Dealings per dealer (0..4) for each DKG id. let dealings_excluded: Vec<_> = (0..4) diff --git a/rs/consensus/idkg/src/lib.rs b/rs/consensus/idkg/src/lib.rs index 7dc4e21a0205..3b2ddf7b2184 100644 --- a/rs/consensus/idkg/src/lib.rs +++ b/rs/consensus/idkg/src/lib.rs @@ -627,7 +627,7 @@ mod tests { use self::test_utils::TestIDkgBlockReader; use super::*; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_logger::no_op_logger; use ic_management_canister_types_private::MasterPublicKeyId; use ic_test_utilities::state_manager::RefMockStateManager; @@ -758,7 +758,8 @@ mod tests { const EXPECTED_CERTIFIED_HEIGHT: u64 = 10; const EXPECTED_FINALIZED_HEIGHT: u64 = 12; ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { mut pool, .. } = dependencies(pool_config.clone(), 1); + let Dependencies { mut pool, .. } = + DependenciesBuilder::new(pool_config.clone(), 1).build(); let state_manager = Arc::new(RefMockStateManager::default()); state_manager diff --git a/rs/consensus/idkg/src/payload_builder.rs b/rs/consensus/idkg/src/payload_builder.rs index 1194f2a67190..909c9a2a8b4a 100644 --- a/rs/consensus/idkg/src/payload_builder.rs +++ b/rs/consensus/idkg/src/payload_builder.rs @@ -709,7 +709,7 @@ mod tests { use super::*; use crate::{test_utils::*, utils::block_chain_reader}; use assert_matches::assert_matches; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_test_utils_canister_threshold_sigs::{ CanisterThresholdSigTestEnvironment, IDkgParticipants, dummy_values::dummy_initial_idkg_dealing_for_tests, generate_tecdsa_protocol_inputs, @@ -902,7 +902,7 @@ mod tests { fn test_update_summary_refs(key_id: IDkgMasterPublicKeyId) { let mut rng = reproducible_rng(); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { mut pool, .. } = dependencies(pool_config, 1); + let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 1).build(); let subnet_id = subnet_test_id(1); let mut expected_transcripts = BTreeSet::new(); let transcript_builder = TestIDkgTranscriptBuilder::new(); @@ -1163,7 +1163,7 @@ mod tests { fn test_summary_proto_conversion(key_id: IDkgMasterPublicKeyId) { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let mut rng = reproducible_rng(); - let Dependencies { mut pool, .. } = dependencies(pool_config, 1); + let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 1).build(); let subnet_id = subnet_test_id(1); let transcript_builder = TestIDkgTranscriptBuilder::new(); // Create a summary block with transcripts @@ -1419,7 +1419,7 @@ mod tests { registry, registry_data_provider, .. - } = dependencies(pool_config, 1); + } = DependenciesBuilder::new(pool_config, 1).build(); let subnet_id = subnet_test_id(1); let mut block_reader = TestIDkgBlockReader::new(); @@ -1559,7 +1559,7 @@ mod tests { registry, registry_data_provider, .. - } = dependencies(pool_config, 1); + } = DependenciesBuilder::new(pool_config, 1).build(); let subnet_id = subnet_test_id(1); let mut block_reader = TestIDkgBlockReader::new(); @@ -1896,7 +1896,7 @@ mod tests { registry, registry_data_provider, .. - } = dependencies(pool_config, 1); + } = DependenciesBuilder::new(pool_config, 1).build(); let subnet_id = subnet_test_id(1); let node_ids = vec![node_test_id(0)]; let subnet_record = SubnetRecordBuilder::from(&node_ids) @@ -2099,7 +2099,7 @@ mod tests { registry, registry_data_provider, .. - } = dependencies(pool_config, 1); + } = DependenciesBuilder::new(pool_config, 1).build(); let subnet_id = subnet_test_id(1); let node_ids = vec![node_test_id(0)]; let subnet_record = SubnetRecordBuilder::from(&node_ids) diff --git a/rs/consensus/idkg/src/payload_builder/pre_signatures.rs b/rs/consensus/idkg/src/payload_builder/pre_signatures.rs index 2a56d78a09d4..4eab374cc312 100644 --- a/rs/consensus/idkg/src/payload_builder/pre_signatures.rs +++ b/rs/consensus/idkg/src/payload_builder/pre_signatures.rs @@ -584,7 +584,7 @@ pub(super) mod tests { }, utils::block_chain_reader, }; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_consensus_utils::pool_reader::PoolReader; use ic_crypto_test_utils_canister_threshold_sigs::{ CanisterThresholdSigTestEnvironment, IDkgParticipants, mock_transcript, @@ -641,7 +641,7 @@ pub(super) mod tests { mut pool, replica_config, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); // Advance 4 rounds without IDkg let mut height = pool.advance_round_normal_operation_n(4); diff --git a/rs/consensus/idkg/src/test_utils.rs b/rs/consensus/idkg/src/test_utils.rs index 4dfaa69ee76e..9fb576286cfc 100644 --- a/rs/consensus/idkg/src/test_utils.rs +++ b/rs/consensus/idkg/src/test_utils.rs @@ -7,7 +7,7 @@ use crate::{ }; use ic_artifact_pool::idkg_pool::IDkgPoolImpl; use ic_config::artifact_pool::ArtifactPoolConfig; -use ic_consensus_mocks::{Dependencies, dependencies}; +use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_consensus_utils::crypto::ConsensusCrypto; use ic_crypto_temp_crypto::TempCryptoComponent; use ic_crypto_test_utils_canister_threshold_sigs::{ @@ -398,7 +398,8 @@ pub(crate) fn create_pre_signer_dependencies_with_crypto_and_threads( threads: usize, ) -> (IDkgPoolImpl, IDkgPreSignerImpl) { let metrics_registry = MetricsRegistry::new(); - let Dependencies { pool, crypto, .. } = dependencies(pool_config.clone(), 1); + let Dependencies { pool, crypto, .. } = + DependenciesBuilder::new(pool_config.clone(), 1).build(); // need to make sure subnet matches the transcript let pre_signer = IDkgPreSignerImpl::new( @@ -420,7 +421,8 @@ pub(crate) fn create_pre_signer_dependencies_and_pool( logger: ReplicaLogger, ) -> (IDkgPoolImpl, IDkgPreSignerImpl, TestConsensusPool) { let metrics_registry = MetricsRegistry::new(); - let Dependencies { pool, crypto, .. } = dependencies(pool_config.clone(), 1); + let Dependencies { pool, crypto, .. } = + DependenciesBuilder::new(pool_config.clone(), 1).build(); let pre_signer = IDkgPreSignerImpl::new( NODE_1, @@ -464,7 +466,7 @@ pub(crate) fn create_signer_dependencies_with_crypto_and_threads( crypto, state_manager, .. - } = dependencies(pool_config.clone(), 1); + } = DependenciesBuilder::new(pool_config.clone(), 1).build(); let signer = ThresholdSignerImpl::new( NODE_1, @@ -517,7 +519,7 @@ pub(crate) fn create_signer_dependencies_and_state_manager( crypto, state_manager, .. - } = dependencies(pool_config.clone(), 1); + } = DependenciesBuilder::new(pool_config.clone(), 1).build(); let signer = ThresholdSignerImpl::new( NODE_1, @@ -545,7 +547,7 @@ pub(crate) fn create_complaint_dependencies_with_crypto_and_node_id( crypto, state_manager, .. - } = dependencies(pool_config.clone(), 1); + } = DependenciesBuilder::new(pool_config.clone(), 1).build(); let complaint_handler = IDkgComplaintHandlerImpl::new( node_id, @@ -571,7 +573,7 @@ pub(crate) fn create_complaint_dependencies_and_pool( crypto, state_manager, .. - } = dependencies(pool_config.clone(), 1); + } = DependenciesBuilder::new(pool_config.clone(), 1).build(); state_manager .get_mut() diff --git a/rs/consensus/idkg/src/utils.rs b/rs/consensus/idkg/src/utils.rs index ae1aa9f8bce7..82f0a6ed8e1a 100644 --- a/rs/consensus/idkg/src/utils.rs +++ b/rs/consensus/idkg/src/utils.rs @@ -519,7 +519,7 @@ mod tests { }; use assert_matches::assert_matches; use ic_config::artifact_pool::ArtifactPoolConfig; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_test_utils_canister_threshold_sigs::{ IDkgParticipants, dummy_values::dummy_initial_idkg_dealing_for_tests, generate_key_transcript, @@ -687,7 +687,7 @@ mod tests { registry, registry_data_provider, .. - } = dependencies(pool_config, 1); + } = DependenciesBuilder::new(pool_config, 1).build(); let subnet_id = subnet_test_id(1); let registry_version = RegistryVersion::from(10); @@ -995,7 +995,7 @@ mod tests { let block = make_block(Some(idkg_payload), height); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { pool, .. } = dependencies(pool_config, 1); + let Dependencies { pool, .. } = DependenciesBuilder::new(pool_config, 1).build(); let log = no_op_logger(); let pool_reader = PoolReader::new(&pool); let mut stats = IDkgPayloadStats::default(); @@ -1060,7 +1060,7 @@ mod tests { let block = make_block(Some(idkg_payload), block_height); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { pool, .. } = dependencies(pool_config, 1); + let Dependencies { pool, .. } = DependenciesBuilder::new(pool_config, 1).build(); let log = no_op_logger(); let pool_reader = PoolReader::new(&pool); let mut stats = IDkgPayloadStats::default(); @@ -1080,7 +1080,7 @@ mod tests { #[test] fn test_block_without_idkg_should_not_deliver_data() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { pool, .. } = dependencies(pool_config, 1); + let Dependencies { pool, .. } = DependenciesBuilder::new(pool_config, 1).build(); let log = no_op_logger(); let pool_reader = PoolReader::new(&pool); let block = make_block(None, Height::from(100)); @@ -1140,7 +1140,7 @@ mod tests { let block = make_block(Some(idkg_payload), height); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { pool, .. } = dependencies(pool_config, 1); + let Dependencies { pool, .. } = DependenciesBuilder::new(pool_config, 1).build(); let log = no_op_logger(); let pool_reader = PoolReader::new(&pool); let mut stats = IDkgPayloadStats::default(); diff --git a/rs/consensus/mocks/BUILD.bazel b/rs/consensus/mocks/BUILD.bazel index a0bcb4865e06..3436f7af6397 100644 --- a/rs/consensus/mocks/BUILD.bazel +++ b/rs/consensus/mocks/BUILD.bazel @@ -22,6 +22,7 @@ rust_library( "//rs/consensus/utils", "//rs/crypto/test_utils/crypto_returning_ok", "//rs/interfaces", + "//rs/interfaces/mocks", "//rs/interfaces/state_manager", "//rs/monitoring/logger", "//rs/monitoring/metrics", diff --git a/rs/consensus/mocks/Cargo.toml b/rs/consensus/mocks/Cargo.toml index ff0f87c0c572..2b3ba3abd399 100644 --- a/rs/consensus/mocks/Cargo.toml +++ b/rs/consensus/mocks/Cargo.toml @@ -12,6 +12,7 @@ ic-config = { path = "../../config" } ic-consensus-utils = { path = "../utils" } ic-crypto-test-utils-crypto-returning-ok = { path = "../../crypto/test_utils/crypto_returning_ok" } ic-interfaces = { path = "../../interfaces" } +ic-interfaces-mocks = { path = "../../interfaces/mocks" } ic-interfaces-state-manager = { path = "../../interfaces/state_manager" } ic-logger = { path = "../../monitoring/logger" } ic-metrics = { path = "../../monitoring/metrics" } diff --git a/rs/consensus/mocks/src/lib.rs b/rs/consensus/mocks/src/lib.rs index 3895d75b7e71..a22772e6122b 100644 --- a/rs/consensus/mocks/src/lib.rs +++ b/rs/consensus/mocks/src/lib.rs @@ -10,6 +10,7 @@ use ic_interfaces::{ consensus::{PayloadBuilder, PayloadValidationError}, validation::ValidationResult, }; +use ic_interfaces_mocks::messaging::RefMockMessageRouting; use ic_protobuf::registry::subnet::v1::SubnetRecord; use ic_registry_client_fake::FakeRegistryClient; use ic_registry_keys::ROOT_SUBNET_ID_KEY; @@ -17,7 +18,10 @@ use ic_registry_proto_data_provider::ProtoRegistryDataProvider; use ic_test_artifact_pool::consensus_pool::TestConsensusPool; use ic_test_utilities::state_manager::RefMockStateManager; use ic_test_utilities_consensus::IDkgStatsNoOp; -use ic_test_utilities_registry::{SubnetRecordBuilder, setup_registry_non_final}; +use ic_test_utilities_registry::{ + SubnetRecordBuilder, add_single_subnet_record, add_subnet_list_record, + insert_initial_dkg_transcript, +}; use ic_test_utilities_time::FastForwardTimeSource; use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; use ic_types::{ @@ -28,7 +32,10 @@ use ic_types::{ }; use mockall::predicate::*; use mockall::*; -use std::sync::{Arc, RwLock}; +use std::{ + collections::BTreeSet, + sync::{Arc, RwLock}, +}; mock! { pub PayloadBuilder {} @@ -55,7 +62,7 @@ mock! { /// Sync wrapper to allow shared modification. See [`RefMockStateManager`]. #[derive(Default)] pub struct RefMockPayloadBuilder { - pub mock: RwLock, + mock: RwLock, } impl RefMockPayloadBuilder { @@ -91,6 +98,7 @@ impl PayloadBuilder for RefMockPayloadBuilder { } } +#[non_exhaustive] pub struct Dependencies { pub crypto: Arc, pub registry: Arc, @@ -100,140 +108,215 @@ pub struct Dependencies { pub pool: TestConsensusPool, pub replica_config: ReplicaConfig, pub state_manager: Arc, + pub payload_builder: Arc, + pub message_routing: Arc, pub dkg_pool: Arc>, pub idkg_pool: Arc>, pub canister_http_pool: Arc>, } -/// Creates most common consensus components used for testing. All components -/// share the same mocked registry with the provided records, so they refer to -/// the identical registry content at any time. The MockStateManager instance -/// that is returned contains no expectations. -pub fn dependencies_with_subnet_records_with_raw_state_manager( +pub struct DependenciesBuilder { pool_config: ArtifactPoolConfig, - subnet_id: SubnetId, - records: Vec<(u64, SubnetRecord)>, -) -> Dependencies { - let time_source = FastForwardTimeSource::new(); - let registry_version = RegistryVersion::from(records[0].clone().0); - let (registry_data_provider, registry) = setup_registry_non_final(subnet_id, records); - registry_data_provider - .add( - ROOT_SUBNET_ID_KEY, - registry_version, - Some(ic_types::subnet_id_into_protobuf(subnet_test_id(0))), + sorted_subnet_records: Vec<(u64, SubnetId, SubnetRecord)>, + replica_config: ReplicaConfig, + with_state_manager_expectations: bool, +} + +impl DependenciesBuilder { + /// Creates a builder for the most common consensus components used for + /// testing. All components share the same mocked registry with one + /// registry version holding the subnet record for the specified number of + /// nodes with all other parameters set to their default values. + pub fn new(pool_config: ArtifactPoolConfig, nodes: u64) -> Self { + let committee = (0..nodes).map(node_test_id).collect::>(); + Self::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&committee).build())], ) - .unwrap(); - registry.update_to_latest_version(); - let replica_config = ReplicaConfig { - subnet_id, - node_id: node_test_id(0), - }; - let crypto = Arc::new(CryptoReturningOk::default()); - let state_manager = Arc::new(RefMockStateManager::default()); - let log = ic_logger::replica_logger::no_op_logger(); - let dkg_pool = Arc::new(RwLock::new(DkgPoolImpl::new( - ic_metrics::MetricsRegistry::new(), - log.clone(), - Height::from(0), - ))); - let idkg_pool = Arc::new(RwLock::new(IDkgPoolImpl::new( - replica_config.node_id, - pool_config.clone(), - log.clone(), - ic_metrics::MetricsRegistry::new(), - Box::new(IDkgStatsNoOp {}), - ))); - let canister_http_pool = Arc::new(RwLock::new(CanisterHttpPoolImpl::new( - ic_metrics::MetricsRegistry::new(), - log, - ))); - let pool = TestConsensusPool::new( - replica_config.node_id, - subnet_id, - pool_config, - time_source.clone(), - registry.clone(), - crypto.clone(), - state_manager.clone(), - Some(dkg_pool.clone()), - ); - let membership = Arc::new(Membership::new( - pool.get_cache(), - registry.clone(), - subnet_id, - )); - Dependencies { - crypto, - registry, - registry_data_provider, - membership, - time_source, - pool, - replica_config, - state_manager, - dkg_pool, - idkg_pool, - canister_http_pool, } -} -/// Creates most common consensus components used for testing. All components -/// share the same mocked registry with the provided records, so they refer to -/// the identical registry content at any time. This constructor should be used, -/// if specific subnet parameters are required. -pub fn dependencies_with_subnet_params( - pool_config: ArtifactPoolConfig, - subnet_id: SubnetId, - records: Vec<(u64, SubnetRecord)>, -) -> Dependencies { - let Dependencies { - time_source, - registry_data_provider, - registry, - membership, - crypto, - pool, - replica_config, - state_manager, - dkg_pool, - idkg_pool, - canister_http_pool, - .. - } = dependencies_with_subnet_records_with_raw_state_manager(pool_config, subnet_id, records); - - state_manager - .get_mut() - .expect_get_state_at() - .return_const(Ok(ic_interfaces_state_manager::Labeled::new( - Height::new(0), - Arc::new(ic_test_utilities_state::get_initial_state(0, 0)), - ))); + /// Creates a builder for the most common consensus components used for + /// testing. All components share the same mocked registry with the + /// provided records, so they refer to the identical registry content at + /// any time. This constructor should be used, if specific subnet + /// parameters are required. + pub fn single_subnet( + pool_config: ArtifactPoolConfig, + subnet_id: SubnetId, + subnet_records: Vec<(u64, SubnetRecord)>, + ) -> Self { + Self::multiple_subnets( + pool_config, + subnet_records + .into_iter() + .map(|(version, record)| (version, subnet_id, record)) + .collect(), + ) + } + + /// Creates a builder for the most common consensus components used for + /// testing. All components share the same mocked registry with the + /// provided records, so they refer to the identical registry content at + /// any time. This constructor should be used, if records for multiple + /// subnets are required. + pub fn multiple_subnets( + pool_config: ArtifactPoolConfig, + mut subnet_records: Vec<(u64, SubnetId, SubnetRecord)>, + ) -> Self { + assert!( + !subnet_records.is_empty(), + "Cannot setup a registry without records." + ); - Dependencies { - crypto, - registry, - registry_data_provider, - membership, - time_source, - pool, - replica_config, - state_manager, - dkg_pool, - idkg_pool, - canister_http_pool, + // Sort the records by registry version to ensure we are iterating on them in the correct + // order when inserting them into the registry. + subnet_records.sort_by_key(|(version, _, _)| *version); + + Self { + pool_config, + replica_config: ReplicaConfig { + node_id: node_test_id(0), + subnet_id: subnet_records[0].1, + }, + sorted_subnet_records: subnet_records, + with_state_manager_expectations: true, + } + } + + pub fn with_replica_config(mut self, replica_config: ReplicaConfig) -> Self { + self.replica_config = replica_config; + self + } + + /// Sets the DKG interval length on every subnet record passed to the + /// builder. + pub fn with_dkg_interval_length(mut self, length: u64) -> Self { + for (_, _, record) in self.sorted_subnet_records.iter_mut() { + record.dkg_interval_length = length; + } + self } -} -/// Creates most common consensus components used for testing. All components -/// share the same mocked registry with one registry version holding the subnet -/// record for the specified number of nodes with all other parameters set to -/// their default values. -pub fn dependencies(pool_config: ArtifactPoolConfig, nodes: u64) -> Dependencies { - let committee = (0..nodes).map(node_test_id).collect::>(); - dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&committee).build())], - ) + /// Leaves the returned `RefMockStateManager` without any expectations, so + /// that the test can set up its own `get_state_at` behavior. + pub fn without_state_manager_expectations(mut self) -> Self { + self.with_state_manager_expectations = false; + self + } + + pub fn build(self) -> Dependencies { + let time_source = FastForwardTimeSource::new(); + let registry_data_provider = Arc::new(ProtoRegistryDataProvider::new()); + + registry_data_provider + .add( + ROOT_SUBNET_ID_KEY, + RegistryVersion::from(self.sorted_subnet_records[0].0), + Some(ic_types::subnet_id_into_protobuf(subnet_test_id(0))), + ) + .unwrap(); + + let mut subnet_ids: BTreeSet = BTreeSet::default(); + let mut last_version = None; + for (version, subnet_id, record) in self.sorted_subnet_records { + // Update the subnet list record for the previous version when the version changes. + // This ensures that the subnet list record is updated only once per version, even if + // there are multiple subnet records for that version (we assume the records are + // sorted). + if let Some(last_version) = last_version + && last_version != version + { + add_subnet_list_record( + ®istry_data_provider, + last_version, + Vec::from_iter(subnet_ids.clone()), + ); + } + + if subnet_ids.insert(subnet_id) { + insert_initial_dkg_transcript(version, subnet_id, &record, ®istry_data_provider); + } + + add_single_subnet_record(®istry_data_provider, version, subnet_id, record); + + last_version = Some(version); + } + // The last iterated version never had its subnet list record updated, so we need to do it + // here. + if let Some(last_version) = last_version { + add_subnet_list_record( + ®istry_data_provider, + last_version, + Vec::from_iter(subnet_ids), + ); + } + + let registry = Arc::new(FakeRegistryClient::new( + Arc::clone(®istry_data_provider) as Arc<_> + )); + + registry.update_to_latest_version(); + + let crypto = Arc::new(CryptoReturningOk::default()); + let state_manager = Arc::new(RefMockStateManager::default()); + let log = ic_logger::replica_logger::no_op_logger(); + let dkg_pool = Arc::new(RwLock::new(DkgPoolImpl::new( + ic_metrics::MetricsRegistry::new(), + log.clone(), + Height::from(0), + ))); + let idkg_pool = Arc::new(RwLock::new(IDkgPoolImpl::new( + self.replica_config.node_id, + self.pool_config.clone(), + log.clone(), + ic_metrics::MetricsRegistry::new(), + Box::new(IDkgStatsNoOp {}), + ))); + let canister_http_pool = Arc::new(RwLock::new(CanisterHttpPoolImpl::new( + ic_metrics::MetricsRegistry::new(), + log, + ))); + let pool = TestConsensusPool::new( + self.replica_config.node_id, + self.replica_config.subnet_id, + self.pool_config, + time_source.clone(), + registry.clone(), + crypto.clone(), + state_manager.clone(), + Some(dkg_pool.clone()), + ); + let membership = Arc::new(Membership::new( + pool.get_cache(), + registry.clone(), + self.replica_config.subnet_id, + )); + + if self.with_state_manager_expectations { + state_manager + .get_mut() + .expect_get_state_at() + .return_const(Ok(ic_interfaces_state_manager::Labeled::new( + Height::new(0), + Arc::new(ic_test_utilities_state::get_initial_state(0, 0)), + ))); + } + + Dependencies { + crypto, + registry, + registry_data_provider, + membership, + time_source, + pool, + replica_config: self.replica_config, + state_manager, + payload_builder: Arc::new(RefMockPayloadBuilder::default()), + message_routing: Arc::new(RefMockMessageRouting::default()), + dkg_pool, + idkg_pool, + canister_http_pool, + } + } } diff --git a/rs/consensus/src/consensus.rs b/rs/consensus/src/consensus.rs index 7563f0cee5b5..83cc40d705b1 100644 --- a/rs/consensus/src/consensus.rs +++ b/rs/consensus/src/consensus.rs @@ -636,7 +636,7 @@ impl BouncerFactory for Consensus mod tests { use super::*; use ic_config::artifact_pool::ArtifactPoolConfig; - use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_https_outcalls_consensus::test_utils::FakeCanisterHttpPayloadBuilder; use ic_logger::replica_logger::no_op_logger; use ic_metrics::MetricsRegistry; @@ -651,20 +651,12 @@ mod tests { use ic_test_utilities_registry::SubnetRecordBuilder; use ic_test_utilities_time::FastForwardTimeSource; use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; - use ic_types::{CryptoHashOfState, Height, SubnetId, crypto::CryptoHash}; + use ic_types::{CryptoHashOfState, Height, crypto::CryptoHash}; use std::sync::Arc; fn set_up_consensus_with_subnet_record( record: SubnetRecord, pool_config: ArtifactPoolConfig, - ) -> (ConsensusImpl, TestConsensusPool, Arc) { - set_up_consensus_with_subnet_record_and_subnet_id(record, pool_config, subnet_test_id(0)) - } - - fn set_up_consensus_with_subnet_record_and_subnet_id( - record: SubnetRecord, - pool_config: ArtifactPoolConfig, - subnet_id: SubnetId, ) -> (ConsensusImpl, TestConsensusPool, Arc) { let Dependencies { pool, @@ -676,7 +668,8 @@ mod tests { dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params(pool_config, subnet_id, vec![(1, record)]); + } = DependenciesBuilder::single_subnet(pool_config, subnet_test_id(0), vec![(1, record)]) + .build(); state_manager .get_mut() .expect_latest_certified_height() diff --git a/rs/consensus/src/consensus/block_maker.rs b/rs/consensus/src/consensus/block_maker.rs index 3edf4519422c..c116b44615c8 100644 --- a/rs/consensus/src/consensus/block_maker.rs +++ b/rs/consensus/src/consensus/block_maker.rs @@ -725,7 +725,7 @@ pub(super) fn is_time_to_make_block( mod tests { use super::*; - use ic_consensus_mocks::{Dependencies, MockPayloadBuilder, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder, MockPayloadBuilder}; use ic_interfaces::consensus_pool::ConsensusPool; use ic_logger::replica_logger::no_op_logger; use ic_metrics::MetricsRegistry; @@ -763,10 +763,11 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![ @@ -783,11 +784,11 @@ mod tests { .build(), ), ], - ); + ) + .build(); pool.advance_round_normal_operation_n(4); - let payload_builder = MockPayloadBuilder::new(); let certified_height = Height::from(1); state_manager .get_mut() @@ -800,7 +801,7 @@ mod tests { Arc::clone(®istry) as Arc, membership.clone(), crypto.clone(), - Arc::new(payload_builder), + payload_builder.clone(), dkg_pool.clone(), idkg_pool.clone(), state_manager.clone(), @@ -819,7 +820,6 @@ mod tests { // Check that block creation works properly. pool.advance_round_normal_operation_n(4); - let mut payload_builder = MockPayloadBuilder::new(); let start = pool.validated().block_proposal().get_highest().unwrap(); let next_height = start.height().increment(); let start_hash = start.content.get_hash(); @@ -856,6 +856,7 @@ mod tests { ); payload_builder + .get_mut() .expect_get_payload() .withf(move |_, payloads, context, _| { matches_expected_payloads(payloads) && context == &expected_context @@ -882,7 +883,7 @@ mod tests { registry.clone(), membership, Arc::clone(&crypto) as Arc<_>, - Arc::new(payload_builder), + payload_builder, dkg_pool, idkg_pool, state_manager, @@ -921,9 +922,7 @@ mod tests { #[test] fn test_build_batch_payload() { - let subnet_id = subnet_test_id(0); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let node_ids: Vec<_> = (0..13).map(node_test_id).collect(); let dkg_interval_length = 9; let Dependencies { mut pool, @@ -933,19 +932,13 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params( - pool_config, - subnet_id, - vec![( - 1, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 13) + .with_dkg_interval_length(dkg_interval_length) + .build(); pool.advance_round_normal_operation_n(8); @@ -981,12 +974,12 @@ mod tests { pool.insert_validated(summary); // Payload builder always returns a batch payload with some canister HTTP data - let mut payload_builder = MockPayloadBuilder::new(); let expected_payload = BatchPayload { canister_http: vec![1; 64], ..Default::default() }; payload_builder + .get_mut() .expect_get_payload() .return_const(expected_payload.clone()); let certified_height = Height::from(1); @@ -1016,7 +1009,7 @@ mod tests { Arc::clone(®istry) as Arc, membership.clone(), crypto.clone(), - Arc::new(payload_builder), + payload_builder.clone(), dkg_pool.clone(), idkg_pool.clone(), state_manager.clone(), @@ -1087,10 +1080,11 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config.clone(), subnet_test_id(0), vec![ @@ -1109,7 +1103,8 @@ mod tests { .build(), ), ], - ); + ) + .build(); state_manager .get_mut() @@ -1128,8 +1123,8 @@ mod tests { Arc::new(ic_test_utilities_state::get_initial_state(0, 0)), ))); - let mut payload_builder = MockPayloadBuilder::new(); payload_builder + .get_mut() .expect_get_payload() .return_const(BatchPayload::default()); let membership = @@ -1142,7 +1137,7 @@ mod tests { Arc::clone(®istry) as Arc, membership.clone(), crypto.clone(), - Arc::new(payload_builder), + payload_builder.clone(), dkg_pool.clone(), idkg_pool.clone(), state_manager.clone(), @@ -1234,14 +1229,20 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, registry_data_provider, dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params(pool_config, subnet_id, vec![(1, record.clone())]); + } = DependenciesBuilder::single_subnet( + pool_config, + subnet_id, + vec![(1, record.clone())], + ) + .build(); - let mut payload_builder = MockPayloadBuilder::new(); payload_builder + .get_mut() .expect_get_payload() .return_const(BatchPayload::default()); let membership = Arc::new(Membership::new( @@ -1256,7 +1257,7 @@ mod tests { Arc::clone(®istry) as Arc, membership, crypto, - Arc::new(payload_builder), + payload_builder, dkg_pool, idkg_pool, state_manager, @@ -1417,7 +1418,7 @@ mod tests { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let Dependencies { mut pool, registry, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -1426,7 +1427,8 @@ mod tests { .with_unit_delay(unit_delay) .build(), )], - ); + ) + .build(); for rank in past_block_ranks { pool.advance_round_with_block(&pool.make_next_block_with_rank(*rank)); @@ -1588,11 +1590,12 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, registry_data_provider, dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, SOURCE_SUBNET_ID, (1..=MAX_REGISTRY_VERSION) @@ -1605,10 +1608,11 @@ mod tests { ) }) .collect(), - ); + ) + .build(); - let mut payload_builder = MockPayloadBuilder::new(); payload_builder + .get_mut() .expect_get_payload() .return_const(BatchPayload::default()); let membership = Arc::new(Membership::new( @@ -1623,7 +1627,7 @@ mod tests { Arc::clone(®istry) as Arc, membership, crypto, - Arc::new(payload_builder), + payload_builder, dkg_pool, idkg_pool, state_manager, diff --git a/rs/consensus/src/consensus/bounds.rs b/rs/consensus/src/consensus/bounds.rs index 2ff4cdbc076b..d7fbf8bdabad 100644 --- a/rs/consensus/src/consensus/bounds.rs +++ b/rs/consensus/src/consensus/bounds.rs @@ -186,9 +186,7 @@ pub fn validated_pool_within_bounds( #[cfg(test)] mod tests { use super::*; - use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; - use ic_test_utilities_registry::SubnetRecordBuilder; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; #[test] fn test_pool_bounds() { @@ -212,16 +210,14 @@ mod tests { // Simple check: advance pool without purging, until we have too many // finalized blocks. ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let committee = (0..40).map(node_test_id).collect::>(); - let record = SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(499) - .build(); let Dependencies { mut pool, registry, replica_config, .. - } = dependencies_with_subnet_params(pool_config, subnet_test_id(0), vec![(1, record)]); + } = DependenciesBuilder::new(pool_config, 40) + .with_dkg_interval_length(499) + .build(); // Still within bounds. pool.advance_round_normal_operation_n(max_counts.finalization as u64); diff --git a/rs/consensus/src/consensus/catchup_package_maker.rs b/rs/consensus/src/consensus/catchup_package_maker.rs index a98e40af549b..0e0c2ca39bf5 100644 --- a/rs/consensus/src/consensus/catchup_package_maker.rs +++ b/rs/consensus/src/consensus/catchup_package_maker.rs @@ -293,10 +293,7 @@ impl CatchUpPackageMaker { mod tests { //! CatchUpPackageMaker unit tests use super::*; - use ic_consensus_mocks::{ - Dependencies, dependencies_with_subnet_params, - dependencies_with_subnet_records_with_raw_state_manager, - }; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_logger::replica_logger::no_op_logger; use ic_protobuf::registry::subnet::v1::SubnetRecord; use ic_registry_client_helpers::subnet::SubnetRegistry; @@ -312,8 +309,7 @@ mod tests { fake_signature_request_context_with_registry_version, }, }; - use ic_test_utilities_registry::SubnetRecordBuilder; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; + use ic_test_utilities_types::ids::subnet_test_id; use ic_types::{ CryptoHashOfState, Height, RegistryVersion, consensus::{BlockPayload, BlockProposal, Payload, SummaryPayload, idkg::PreSigId}, @@ -344,17 +340,9 @@ mod tests { fn with_cup_maker_setup(run: impl FnOnce(CatchUpPackageMaker, u64, Dependencies) -> T) -> T { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let dkg_interval_length = 5; - let committee: Vec<_> = (0..4).map(node_test_id).collect(); - let mut deps = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); + let mut deps = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(dkg_interval_length) + .build(); // Ignore state sync and state divergence deps.state_manager @@ -599,7 +587,6 @@ mod tests { ) { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let interval_length = 5; - let committee: Vec<_> = (0..4).map(node_test_id).collect(); let Dependencies { mut pool, membership, @@ -607,16 +594,10 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_records_with_raw_state_manager( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(interval_length) + .without_state_manager_expectations() + .build(); let height = Height::from(0); state_manager @@ -694,7 +675,6 @@ mod tests { fn test_invoke_state_sync() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let interval_length = 3; - let committee: Vec<_> = (0..5).map(node_test_id).collect(); let Dependencies { mut pool, membership, @@ -702,16 +682,9 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 5) + .with_dkg_interval_length(interval_length) + .build(); pool.advance_round_normal_operation_n(5); let cup_height = PoolReader::new(&pool).get_catch_up_height(); @@ -753,7 +726,6 @@ mod tests { fn test_state_divergence_report() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let interval_length = 3; - let committee: Vec<_> = (0..5).map(node_test_id).collect(); let Dependencies { mut pool, membership, @@ -761,16 +733,9 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 5) + .with_dkg_interval_length(interval_length) + .build(); state_manager .get_mut() diff --git a/rs/consensus/src/consensus/finalizer.rs b/rs/consensus/src/consensus/finalizer.rs index f69cc144b7e3..ccc0715d2aa2 100644 --- a/rs/consensus/src/consensus/finalizer.rs +++ b/rs/consensus/src/consensus/finalizer.rs @@ -252,7 +252,7 @@ impl Finalizer { mod tests { //! Finalizer unit tests use super::*; - use ic_consensus_mocks::{Dependencies, dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_logger::replica_logger::no_op_logger; use ic_metrics::MetricsRegistry; use ic_test_utilities::{ @@ -277,7 +277,7 @@ mod tests { registry, crypto, .. - } = dependencies(pool_config, 1); + } = DependenciesBuilder::new(pool_config, 1).build(); let message_routing = FakeMessageRouting::new(); assert_eq!(pool.advance_round_normal_operation(), Height::from(1)); @@ -354,7 +354,7 @@ mod tests { registry, crypto, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![ @@ -373,7 +373,8 @@ mod tests { .build(), ), ], - ); + ) + .build(); let metrics_registry = MetricsRegistry::new(); let message_routing = Arc::new(FakeMessageRouting::new()); *message_routing.next_batch_height.write().unwrap() = Height::from(2); diff --git a/rs/consensus/src/consensus/notary.rs b/rs/consensus/src/consensus/notary.rs index 56101736c910..8f3374c6c12b 100644 --- a/rs/consensus/src/consensus/notary.rs +++ b/rs/consensus/src/consensus/notary.rs @@ -384,7 +384,7 @@ mod tests { //! Notary unit tests use super::*; use assert_matches::assert_matches; - use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_interfaces::{consensus_pool::ConsensusPool, time_source::TimeSource}; use ic_logger::replica_logger::no_op_logger; use ic_metrics::MetricsRegistry; @@ -400,7 +400,6 @@ mod tests { #[test] fn test_notary_behavior() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let committee = vec![node_test_id(0)]; let dkg_interval_length = 30; let Dependencies { mut pool, @@ -410,16 +409,9 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 1) + .with_dkg_interval_length(dkg_interval_length) + .build(); state_manager .get_mut() .expect_latest_certified_height() @@ -601,7 +593,6 @@ mod tests { #[test] fn test_out_of_sync_notarization() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let committee = vec![node_test_id(0)]; let dkg_interval_length = 30; let Dependencies { mut pool, @@ -611,16 +602,9 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 1) + .with_dkg_interval_length(dkg_interval_length) + .build(); state_manager .get_mut() .expect_latest_certified_height() @@ -677,18 +661,15 @@ mod tests { unit_delay: Duration::from_secs(1), initial_notary_delay: Duration::from_secs(0), }; - let committee = (0..3).map(node_test_id).collect::>(); - /* use large enough DKG interval to trigger notarization/CUP gap limit */ - let record = SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(ACCEPTABLE_NOTARIZATION_CUP_GAP + 30) - .build(); - let Dependencies { mut pool, state_manager, membership, .. - } = dependencies_with_subnet_params(pool_config, subnet_test_id(0), vec![(1, record)]); + } = DependenciesBuilder::new(pool_config, 3) + /* use large enough DKG interval to trigger notarization/CUP gap limit */ + .with_dkg_interval_length(ACCEPTABLE_NOTARIZATION_CUP_GAP + 30) + .build(); let last_cup_dkg_info = PoolReader::new(&pool) .get_highest_catch_up_package() .content @@ -802,17 +783,14 @@ mod tests { unit_delay: Duration::from_secs(1), initial_notary_delay, }; - let committee = (0..3).map(node_test_id).collect::>(); - let record = SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval) - .build(); - let Dependencies { mut pool, state_manager, membership, .. - } = dependencies_with_subnet_params(pool_config, subnet_test_id(0), vec![(1, record)]); + } = DependenciesBuilder::new(pool_config, 3) + .with_dkg_interval_length(dkg_interval) + .build(); let certified_height = Arc::new(RwLock::new(Height::from(0))); let certified_height_clone = Arc::clone(&certified_height); @@ -917,7 +895,7 @@ mod tests { state_manager, membership, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![ @@ -935,7 +913,8 @@ mod tests { .build(), ), ], - ); + ) + .build(); let certified_height = Arc::new(RwLock::new(Height::from(0))); let certified_height_clone = Arc::clone(&certified_height); diff --git a/rs/consensus/src/consensus/payload_builder.rs b/rs/consensus/src/consensus/payload_builder.rs index 25acad2542c1..22ccfe686f34 100644 --- a/rs/consensus/src/consensus/payload_builder.rs +++ b/rs/consensus/src/consensus/payload_builder.rs @@ -217,7 +217,7 @@ pub(crate) mod test { use ic_btc_replica_types::{ BitcoinAdapterResponse, BitcoinAdapterResponseWrapper, GetSuccessorsResponseComplete, }; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_tree_hash::{Digest, Witness}; use ic_https_outcalls_consensus::test_utils::FakeCanisterHttpPayloadBuilder; use ic_interfaces::consensus::PayloadWithSizeEstimate; @@ -338,7 +338,7 @@ pub(crate) mod test { provided_canister_http_responses: Vec, ) { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { registry, .. } = dependencies(pool_config, 1); + let Dependencies { registry, .. } = DependenciesBuilder::new(pool_config, 1).build(); let payload_builder = make_test_payload_impl( registry, vec![provided_ingress_messages.clone()], @@ -428,7 +428,7 @@ pub(crate) mod test { // 5. query_stats fn test_get_payload_respect_limits() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { registry, .. } = dependencies(pool_config, 1); + let Dependencies { registry, .. } = DependenciesBuilder::new(pool_config, 1).build(); const MAX_BLOCK_SIZE: NumBytes = NumBytes::new(ic_limits::MAX_BLOCK_PAYLOAD_SIZE); const XNET_PAYLOAD_SIZE: NumBytes = NumBytes::new(64 * KB); @@ -508,7 +508,7 @@ pub(crate) mod test { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { const ZERO_BYTES: NumBytes = NumBytes::new(0); - let Dependencies { registry, .. } = dependencies(pool_config, 1); + let Dependencies { registry, .. } = DependenciesBuilder::new(pool_config, 1).build(); let settings = MocksSettings { ingress_payload_size_to_return: NumBytes::from(ingress_payload_size), diff --git a/rs/consensus/src/consensus/priority.rs b/rs/consensus/src/consensus/priority.rs index c467ecd31afa..182524a362e0 100644 --- a/rs/consensus/src/consensus/priority.rs +++ b/rs/consensus/src/consensus/priority.rs @@ -119,10 +119,9 @@ fn compute_bouncer( #[cfg(test)] mod tests { use super::*; - use ic_consensus_mocks::{Dependencies, dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_test_utilities_consensus::fake::FakeContent; - use ic_test_utilities_registry::SubnetRecordBuilder; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; + use ic_test_utilities_types::ids::node_test_id; use ic_types::{ consensus::{ ConsensusMessageHashable, Finalization, FinalizationContent, HasHeight, Notarization, @@ -135,17 +134,9 @@ mod tests { fn test_bouncer_for_validation_gap() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let dkg_interval = 499; - let committee = (0..4).map(node_test_id).collect::>(); - let Dependencies { mut pool, .. } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(committee.as_slice()) - .with_dkg_interval_length(dkg_interval) - .build(), - )], - ); + let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(dkg_interval) + .build(); // Advance pool *without* producing CUP to the maximum height beyond // which we don't validate non-CUP artifacts anymore. @@ -197,7 +188,7 @@ mod tests { #[test] fn test_bouncer_function() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { mut pool, .. } = dependencies(pool_config, 1); + let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 1).build(); pool.advance_round_normal_operation_n(2); let expected_batch_height = Height::from(1); diff --git a/rs/consensus/src/consensus/proptests.rs b/rs/consensus/src/consensus/proptests.rs index 2657ab7f058e..37292ebcf784 100644 --- a/rs/consensus/src/consensus/proptests.rs +++ b/rs/consensus/src/consensus/proptests.rs @@ -1,5 +1,5 @@ use crate::consensus::payload_builder::test::make_test_payload_impl; -use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; +use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_tree_hash::{Digest, Witness}; use ic_interfaces::{batch_payload::ProposalContext, consensus::PayloadBuilder}; use ic_interfaces_registry::RegistryClient; @@ -41,11 +41,12 @@ fn proptest_payload_size_validation() { .with_max_ingress_bytes_per_message(MAX_BLOCK_SIZE as u64) .build(); - let Dependencies { registry, .. } = dependencies_with_subnet_params( + let Dependencies { registry, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![(1, subnet_record.clone())], - ); + ) + .build(); proptest!( ProptestConfig { @@ -172,11 +173,12 @@ fn regression1() { .with_max_ingress_bytes_per_message(MAX_BLOCK_SIZE as u64) .build(); - let Dependencies { registry, .. } = dependencies_with_subnet_params( + let Dependencies { registry, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![(1, subnet_record.clone())], - ); + ) + .build(); let ingress = vec![make_ingress(965988), make_ingress(1019914)]; let mut xnet = BTreeMap::new(); xnet.insert(subnet_test_id(0), make_xnet_slice(1389926)); diff --git a/rs/consensus/src/consensus/purger.rs b/rs/consensus/src/consensus/purger.rs index f1a31e60005e..009d0305d776 100644 --- a/rs/consensus/src/consensus/purger.rs +++ b/rs/consensus/src/consensus/purger.rs @@ -472,9 +472,8 @@ fn get_pending_cup_heights(pool: &PoolReader<'_>) -> BTreeSet { #[cfg(test)] mod tests { use super::*; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_interfaces::p2p::consensus::MutablePool; - use ic_interfaces_mocks::messaging::MockMessageRouting; use ic_logger::replica_logger::no_op_logger; use ic_metrics::MetricsRegistry; use ic_test_utilities::message_routing::FakeMessageRouting; @@ -492,8 +491,9 @@ mod tests { state_manager, replica_config, registry, + message_routing, .. - } = dependencies(pool_config, 1); + } = DependenciesBuilder::new(pool_config, 1).build(); state_manager .get_mut() @@ -537,17 +537,17 @@ mod tests { .withf(move |height| *height == *checkpoint_purge_height_clone.read().unwrap()) .return_const(()); - let mut message_routing = MockMessageRouting::new(); let expected_batch_height = Arc::new(RwLock::new(Height::from(0))); let expected_batch_height_clone = Arc::clone(&expected_batch_height); message_routing + .get_mut() .expect_expected_batch_height() .returning(move || *expected_batch_height_clone.read().unwrap()); let purger = Purger::new( replica_config, state_manager, - Arc::new(message_routing), + message_routing, registry, no_op_logger(), MetricsRegistry::new(), @@ -631,7 +631,7 @@ mod tests { replica_config, registry, .. - } = dependencies(pool_config, 3); + } = DependenciesBuilder::new(pool_config, 3).build(); state_manager .get_mut() .expect_latest_state_height() @@ -667,7 +667,7 @@ mod tests { replica_config, registry, .. - } = dependencies(pool_config, 3); + } = DependenciesBuilder::new(pool_config, 3).build(); state_manager .get_mut() .expect_latest_state_height() @@ -706,7 +706,7 @@ mod tests { #[test] fn test_get_purge_height() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { mut pool, .. } = dependencies(pool_config, 1); + let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 1).build(); // Initial purge height is None. assert_eq!(get_purge_height(&PoolReader::new(&pool)), None); @@ -737,8 +737,9 @@ mod tests { state_manager, replica_config, registry, + message_routing, .. - } = dependencies(pool_config, 10); + } = DependenciesBuilder::new(pool_config, 10).build(); let expected_extra_heights = Arc::new(RwLock::new(BTreeSet::new())); let extra_heights_clone = Arc::clone(&expected_extra_heights); @@ -759,7 +760,7 @@ mod tests { let purger = Purger::new( replica_config, state_manager.clone(), - Arc::new(MockMessageRouting::new()), + message_routing, registry, no_op_logger(), MetricsRegistry::new(), @@ -822,20 +823,21 @@ mod tests { state_manager, replica_config, registry, + message_routing, .. - } = dependencies(pool_config, 10); + } = DependenciesBuilder::new(pool_config, 10).build(); state_manager .get_mut() .expect_latest_state_height() .returning(|| Height::new(0)); - let mut message_routing = MockMessageRouting::new(); message_routing + .get_mut() .expect_expected_batch_height() .returning(|| Height::new(0)); let purger = Purger::new( replica_config, state_manager, - Arc::new(message_routing), + message_routing, registry, no_op_logger(), MetricsRegistry::new(), diff --git a/rs/consensus/src/consensus/random_beacon_maker.rs b/rs/consensus/src/consensus/random_beacon_maker.rs index e414bf0507d5..18452712f7d5 100644 --- a/rs/consensus/src/consensus/random_beacon_maker.rs +++ b/rs/consensus/src/consensus/random_beacon_maker.rs @@ -105,7 +105,7 @@ impl RandomBeaconMaker { mod tests { //! BeaconMaker unit tests use super::*; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_interfaces::consensus_pool::ConsensusPool; use ic_logger::replica_logger::no_op_logger; @@ -118,7 +118,7 @@ mod tests { replica_config, crypto, .. - } = dependencies(pool_config, 1); + } = DependenciesBuilder::new(pool_config, 1).build(); let beacon_maker = RandomBeaconMaker::new(replica_config, membership, crypto, no_op_logger()); diff --git a/rs/consensus/src/consensus/random_tape_maker.rs b/rs/consensus/src/consensus/random_tape_maker.rs index c00ffa170082..199dc13a3b43 100644 --- a/rs/consensus/src/consensus/random_tape_maker.rs +++ b/rs/consensus/src/consensus/random_tape_maker.rs @@ -187,7 +187,7 @@ impl RandomTapeMaker { mod tests { use super::*; use crate::consensus::add_all_to_validated; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_interfaces::{p2p::consensus::MutablePool, time_source::TimeSource}; use ic_logger::replica_logger::no_op_logger; use ic_test_utilities::message_routing::FakeMessageRouting; @@ -210,7 +210,7 @@ mod tests { time_source, crypto, .. - } = dependencies(pool_config, 4); + } = DependenciesBuilder::new(pool_config, 4).build(); let message_routing = Arc::new(FakeMessageRouting::new()); pool.advance_round_normal_operation(); *message_routing.next_batch_height.write().unwrap() = Height::from(2); diff --git a/rs/consensus/src/consensus/share_aggregator.rs b/rs/consensus/src/consensus/share_aggregator.rs index 3c26940ed4a5..6f5addf65831 100644 --- a/rs/consensus/src/consensus/share_aggregator.rs +++ b/rs/consensus/src/consensus/share_aggregator.rs @@ -186,13 +186,12 @@ fn to_messages(artifacts: Vec) -> Vec, ) -> CatchUpPackage { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let node_ids: Vec<_> = (0..3).map(node_test_id).collect(); let interval_length = 3; let Dependencies { mut pool, membership, crypto, .. - } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - INITIAL_REGISTRY_VERSION, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(interval_length) - .build(), - )], - ); + } = DependenciesBuilder::new(pool_config, 3) + .with_dkg_interval_length(interval_length) + .build(); let message_routing = Arc::new(FakeMessageRouting::new()); let aggregator = ShareAggregator::new(membership, message_routing, crypto, no_op_logger()); diff --git a/rs/consensus/src/consensus/status.rs b/rs/consensus/src/consensus/status.rs index 08f47ebb0cc3..0f002b1d5878 100644 --- a/rs/consensus/src/consensus/status.rs +++ b/rs/consensus/src/consensus/status.rs @@ -162,7 +162,7 @@ mod tests { use std::sync::Arc; use ic_config::artifact_pool::ArtifactPoolConfig; - use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_registry_client_fake::FakeRegistryClient; use ic_test_artifact_pool::consensus_pool::{Round, TestConsensusPool}; use ic_test_utilities_logger::with_test_replica_logger; @@ -192,7 +192,7 @@ mod tests { let node_ids = [node_test_id(0)]; let Dependencies { mut pool, registry, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![ @@ -211,7 +211,8 @@ mod tests { .build(), ), ], - ); + ) + .build(); pool.advance_round_normal_operation_n(CUP_HEIGHT.get()); Round::new(&mut pool) diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index 6b61fdda0605..9223f47cb93a 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2000,13 +2000,8 @@ pub mod test { MAX_CONSENSUS_THREADS, block_maker::get_block_maker_delay, build_thread_pool, }; use assert_matches::assert_matches; - use ic_artifact_pool::dkg_pool::DkgPoolImpl; use ic_config::artifact_pool::ArtifactPoolConfig; - use ic_consensus_mocks::{ - Dependencies, RefMockPayloadBuilder, dependencies_with_subnet_params, - dependencies_with_subnet_records_with_raw_state_manager, - }; - use ic_crypto_test_utils_crypto_returning_ok::CryptoReturningOk; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder, RefMockPayloadBuilder}; use ic_interfaces::{ messaging::XNetPayloadValidationFailure, p2p::consensus::MutablePool, time_source::TimeSource, @@ -2015,7 +2010,7 @@ pub mod test { use ic_logger::replica_logger::no_op_logger; use ic_metrics::MetricsRegistry; use ic_protobuf::registry::subnet::v1::{ - CatchUpPackageContents, SubnetSplittingArgs as SubnetSplittingArgsProto, + CatchUpPackageContents, SubnetRecord, SubnetSplittingArgs as SubnetSplittingArgsProto, catch_up_package_contents::CupType, }; use ic_registry_client_fake::FakeRegistryClient; @@ -2082,96 +2077,102 @@ pub mod test { }; } - pub struct ValidatorAndDependencies { - pub validator: Validator, - pub payload_builder: Arc, - pub membership: Arc, - pub state_manager: Arc, - pub message_routing: Arc, - pub crypto: Arc, - pub data_provider: Arc, - pub registry_client: Arc, - pub pool: TestConsensusPool, - pub dkg_pool: Arc>, - pub time_source: Arc, - pub replica_config: ReplicaConfig, + /// The DKG interval length used by most tests in this module. + const DKG_INTERVAL_LENGTH: u64 = 9; + + struct ValidatorAndDependencies { + validator: Validator, + payload_builder: Arc, + state_manager: Arc, + message_routing: Arc, + registry_data_provider: Arc, + registry: Arc, + pool: TestConsensusPool, + time_source: Arc, + replica_config: ReplicaConfig, + } + + struct ValidatorAndDependenciesBuilder { + deps_builder: DependenciesBuilder, } - impl ValidatorAndDependencies { - fn new(dependencies: Dependencies) -> Self { - let payload_builder = Arc::new(RefMockPayloadBuilder::default()); - let message_routing = Arc::new(RefMockMessageRouting::default()); + impl ValidatorAndDependenciesBuilder { + fn new(pool_config: ArtifactPoolConfig, nodes: u64) -> Self { + Self { + deps_builder: DependenciesBuilder::new(pool_config, nodes) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH), + } + } + + fn single_subnet( + pool_config: ArtifactPoolConfig, + subnet_id: SubnetId, + subnet_records: Vec<(u64, SubnetRecord)>, + ) -> Self { + Self { + deps_builder: DependenciesBuilder::single_subnet( + pool_config, + subnet_id, + subnet_records, + ), + } + } + + fn with_dkg_interval_length(mut self, length: u64) -> Self { + self.deps_builder = self.deps_builder.with_dkg_interval_length(length); + self + } + + fn without_state_manager_expectations(mut self) -> Self { + self.deps_builder = self.deps_builder.without_state_manager_expectations(); + self + } + + fn build(self) -> ValidatorAndDependencies { + let Dependencies { + payload_builder, + membership, + state_manager, + message_routing, + crypto, + registry_data_provider, + registry, + pool, + dkg_pool, + time_source, + replica_config, + .. + } = self.deps_builder.build(); + let validator = Validator::new( - dependencies.replica_config.clone(), - dependencies.membership.clone(), - dependencies.registry.clone(), - dependencies.crypto.clone(), + replica_config.clone(), + membership, + registry.clone(), + crypto, payload_builder.clone(), - dependencies.state_manager.clone(), + state_manager.clone(), message_routing.clone(), - dependencies.dkg_pool.clone(), + dkg_pool, build_thread_pool(MAX_CONSENSUS_THREADS), no_op_logger(), &MetricsRegistry::new(), - Arc::clone(&dependencies.time_source) as Arc<_>, + time_source.clone(), ); - Self { + + ValidatorAndDependencies { validator, payload_builder, - membership: dependencies.membership, - state_manager: dependencies.state_manager, + state_manager, message_routing, - crypto: dependencies.crypto, - data_provider: dependencies.registry_data_provider, - registry_client: dependencies.registry, - pool: dependencies.pool, - dkg_pool: dependencies.dkg_pool, - time_source: dependencies.time_source, - replica_config: dependencies.replica_config, + registry_data_provider, + registry, + pool, + time_source, + replica_config, } } } - fn setup_dependencies( - pool_config: ic_config::artifact_pool::ArtifactPoolConfig, - node_ids: &[NodeId], - ) -> ValidatorAndDependencies { - setup_dependencies_with_dkg_interval_length(pool_config, node_ids, 9) - } - - fn setup_dependencies_with_dkg_interval_length( - pool_config: ic_config::artifact_pool::ArtifactPoolConfig, - node_ids: &[NodeId], - dkg_interval_length: u64, - ) -> ValidatorAndDependencies { - ValidatorAndDependencies::new(dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - )) - } - - fn setup_dependencies_with_raw_state_manager( - pool_config: ic_config::artifact_pool::ArtifactPoolConfig, - node_ids: &[NodeId], - ) -> ValidatorAndDependencies { - ValidatorAndDependencies::new(dependencies_with_subnet_records_with_raw_state_manager( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(node_ids) - .with_dkg_interval_length(9) - .build(), - )], - )) - } - #[test] fn test_validate_catch_up_package_shares() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { @@ -2180,7 +2181,7 @@ pub mod test { state_manager, mut pool, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); // The state manager is mocked and the `StateHash` is completely arbitrary. It // must just be the same as in the `CatchUpPackageShare`. @@ -2210,7 +2211,7 @@ pub mod test { }; // Skip to two heights before Summary height - pool.advance_round_normal_operation_no_cup_n(8); + pool.advance_round_normal_operation_no_cup_n(DKG_INTERVAL_LENGTH - 1); let cup_share_data_height = make_next_cup_share(&pool); pool.advance_round_normal_operation_no_cup_n(1); @@ -2313,10 +2314,9 @@ pub mod test { state_manager, mut pool, .. - } = setup_dependencies_with_raw_state_manager( - pool_config, - &(0..4).map(node_test_id).collect::>(), - ); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4) + .without_state_manager_expectations() + .build(); // The state manager is mocked and the `StateHash` is completely arbitrary. It // must just be the same as in the `CatchUpPackageShare`. @@ -2359,7 +2359,7 @@ pub mod test { }; // Skip to Summary height - pool.advance_round_normal_operation_no_cup_n(9); + pool.advance_round_normal_operation_no_cup_n(DKG_INTERVAL_LENGTH); let mut proposal = pool.make_next_block(); let block = proposal.content.as_mut(); @@ -2449,7 +2449,7 @@ pub mod test { validator, mut pool, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let block = pool.make_next_block(); pool.insert_validated(block.clone()); // Insert a Finalization for `block` in the unvalidated pool @@ -2531,7 +2531,7 @@ pub mod test { mut pool, replica_config, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); pool.advance_round_normal_operation(); // Put a random tape share in the unvalidated pool @@ -2588,7 +2588,7 @@ pub mod test { mut pool, replica_config, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let mut round = pool.prepare_round().dont_finalize().dont_add_random_tape(); round.advance(); @@ -2697,18 +2697,17 @@ pub mod test { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let prior_height = Height::from(5); let certified_height = Height::from(1); - let committee: Vec<_> = (0..4).map(node_test_id).collect(); let ValidatorAndDependencies { validator, payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, time_source, replica_config, .. - } = setup_dependencies(pool_config, &committee); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -2724,12 +2723,12 @@ pub mod test { .return_const(certified_height); add_subnet_record( - &data_provider, + ®istry_data_provider, 11, replica_config.subnet_id, SubnetRecordBuilder::from(&[]).build(), ); - registry_client.update_to_latest_version(); + registry.update_to_latest_version(); // Create a block chain with some length that will not be finalized pool.insert_beacon_chain(&pool.make_next_beacon(), prior_height); @@ -2765,7 +2764,7 @@ pub mod test { ); // Time between blocks increases by at least initial_notary_delay + 1ns - let monotonic_block_increment = registry_client + let monotonic_block_increment = registry .get_notarization_delay_settings( replica_config.subnet_id, test_block.context.registry_version, @@ -2781,7 +2780,7 @@ pub mod test { let delay = monotonic_block_increment + get_block_maker_delay( &no_op_logger(), - registry_client.as_ref(), + registry.as_ref(), replica_config.subnet_id, &pool_reader, parent.clone(), @@ -2801,17 +2800,16 @@ pub mod test { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let prior_height = Height::from(5); let certified_height = Height::from(1); - let committee: Vec<_> = (0..4).map(node_test_id).collect(); let ValidatorAndDependencies { validator, payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, replica_config, .. - } = setup_dependencies(pool_config, &committee); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -2827,12 +2825,12 @@ pub mod test { .return_const(certified_height); add_subnet_record( - &data_provider, + ®istry_data_provider, 11, replica_config.subnet_id, SubnetRecordBuilder::from(&[]).build(), ); - registry_client.update_to_latest_version(); + registry.update_to_latest_version(); // Create a block chain with some length that will not be finalized pool.insert_beacon_chain(&pool.make_next_beacon(), prior_height); @@ -2871,28 +2869,28 @@ pub mod test { #[test] fn test_summary_block_is_validated_while_halted() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dkg_interval_length = 9; let committee: Vec<_> = (0..4).map(node_test_id).collect(); let ValidatorAndDependencies { validator, payload_builder, state_manager, - registry_client, + registry, replica_config, mut pool, time_source, .. - } = ValidatorAndDependencies::new(dependencies_with_subnet_params( + } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( 1, SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .with_halt_at_cup_height(true) .build(), )], - )); + ) + .build(); // Any payload validation fails, so we can observe whether validation was attempted. payload_builder @@ -2909,7 +2907,7 @@ pub mod test { // Advance to the block right before the *next* summary height and build the summary // block proposal at the DKG boundary. - pool.advance_round_normal_operation_n(dkg_interval_length); + pool.advance_round_normal_operation_n(DKG_INTERVAL_LENGTH); let summary_proposal = pool.make_next_block(); assert!( summary_proposal.content.as_ref().payload.is_summary(), @@ -2921,7 +2919,7 @@ pub mod test { status::get_status( summary_proposal.height(), &PoolReader::new(&pool).get_highest_finalized_summary_block(), - registry_client.as_ref(), + registry.as_ref(), replica_config.subnet_id, &PoolReader::new(&pool), &no_op_logger(), @@ -2955,18 +2953,17 @@ pub mod test { fn test_block_validation_without_notarized_parent() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let certified_height = Height::from(1); - let committee = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, time_source, replica_config, .. - } = setup_dependencies(pool_config, &committee); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -2984,13 +2981,13 @@ pub mod test { ))); add_subnet_record( - &data_provider, + ®istry_data_provider, 11, replica_config.subnet_id, SubnetRecordBuilder::from(&[]).build(), ); - registry_client.update_to_latest_version(); + registry.update_to_latest_version(); pool.insert_beacon_chain(&pool.make_next_beacon(), Height::from(3)); @@ -3037,7 +3034,6 @@ pub mod test { fn test_block_validation_with_missing_past_payload() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let certified_height = Height::from(1); - let committee = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, payload_builder, @@ -3045,7 +3041,7 @@ pub mod test { mut pool, time_source, .. - } = setup_dependencies(pool_config, &committee); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -3062,18 +3058,18 @@ pub mod test { Arc::new(ic_test_utilities_state::get_initial_state(0, 0)), ))); - pool.advance_round_normal_operation_n(8); + pool.advance_round_normal_operation_n(DKG_INTERVAL_LENGTH - 1); - // Make block at height 9 + // Make block at height `DKG_INTERVAL_LENGTH` let next_proposal = pool.make_next_block(); - assert_eq!(next_proposal.height().get(), 9); - // Make and insert beacon at height 9 + assert_eq!(next_proposal.height().get(), DKG_INTERVAL_LENGTH); + // Make and insert beacon at height `DKG_INTERVAL_LENGTH` let next_beacon = pool.make_next_beacon(); pool.insert_validated(next_beacon.clone()); - // Make and insert beacon at height 10 + // Make and insert beacon at height `DKG_INTERVAL_LENGTH + 1` let summary_beacon = RandomBeacon::from_parent(&next_beacon); pool.insert_validated(summary_beacon.clone()); - // Make summary at height 10 + // Make summary at height `DKG_INTERVAL_LENGTH + 1` let summary = pool.make_next_block_from_parent(next_proposal.content.get_value(), Rank(0)); let cup = CatchUpPackage { @@ -3097,13 +3093,13 @@ pub mod test { pool.insert_validated(summary); let test_block = pool.make_next_block(); - assert_eq!(test_block.height().get(), 11); + assert_eq!(test_block.height().get(), DKG_INTERVAL_LENGTH + 2); // Forward time correctly time_source .set_time(test_block.content.as_ref().context.time) .unwrap(); - // Validation should fail, since the payload at height 9 is missing + // Validation should fail, since the payload at height `DKG_INTERVAL_LENGTH` is missing let result = validator.check_block_validity(&PoolReader::new(&pool), &test_block); assert_matches!( result, @@ -3112,12 +3108,12 @@ pub mod test { )) ); - // Insert the missing proposal at height 9, the payload should be validated as expected + // Insert the missing proposal at height `DKG_INTERVAL_LENGTH`, the payload should be validated as expected pool.insert_validated(next_proposal); let result = validator.check_block_validity(&PoolReader::new(&pool), &test_block); assert_matches!(result, Ok(())); - // Insert the cup at height 10, the payload validation should fail + // Insert the cup at height `DKG_INTERVAL_LENGTH + 1`, the payload validation should fail // Since payloads below the CUP are not returned pool.insert_validated(cup); let result = validator.check_block_validity(&PoolReader::new(&pool), &test_block); @@ -3134,17 +3130,16 @@ pub mod test { fn test_block_validation_with_registry_versions() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let certified_height = Height::from(1); - let subnet_members = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, replica_config, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -3155,20 +3150,20 @@ pub mod test { .return_const(certified_height); add_subnet_record( - &data_provider, + ®istry_data_provider, 11, replica_config.subnet_id, SubnetRecordBuilder::from(&[]).build(), ); add_subnet_record( - &data_provider, + ®istry_data_provider, 12, replica_config.subnet_id, SubnetRecordBuilder::from(&[]).build(), ); - registry_client.update_to_latest_version(); + registry.update_to_latest_version(); let mut parent_block = make_next_block(&pool); parent_block.content.as_mut().context.registry_version = RegistryVersion::from(12); @@ -3207,23 +3202,20 @@ pub mod test { next_block } - /// The DKG interval length used by the subnet splitting tests below. - const DKG_INTERVAL_LENGTH: u64 = 9; - /// The registry version at which the subnet split is scheduled in the tests below. const SUBNET_SPLIT_REGISTRY_VERSION: RegistryVersion = RegistryVersion::new(3); /// Schedules a subnet split at [`SUBNET_SPLIT_REGISTRY_VERSION`] by overwriting the subnet's /// CUP contents record with one carrying a [`CupType::SubnetSplitting`], the way the NNS would. fn schedule_subnet_split( - data_provider: &Arc, - registry_client: &FakeRegistryClient, + registry_data_provider: &Arc, + registry: &FakeRegistryClient, subnet_id: SubnetId, ) { // Keep everything but the CUP type as it is at genesis, so that the scheduled split is the // only difference between the two records. - let mut cup_contents = registry_client - .get_cup_contents(subnet_id, registry_client.get_latest_version()) + let mut cup_contents = registry + .get_cup_contents(subnet_id, registry.get_latest_version()) .expect("Failed to get the CUP contents") .value .expect("The CUP contents should be in the registry"); @@ -3231,14 +3223,14 @@ pub mod test { destination_subnet_id: Some(subnet_id_into_protobuf(subnet_test_id(1))), })); - data_provider + registry_data_provider .add( &make_catch_up_package_contents_key(subnet_id), SUBNET_SPLIT_REGISTRY_VERSION, Some(cup_contents), ) .expect("Failed to add the CUP contents"); - registry_client.reload(); + registry.reload(); } /// Until the summary block starting the split is reached, data blocks must keep their registry @@ -3255,13 +3247,13 @@ pub mod test { validator, payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, pool, time_source, replica_config, .. - } = ValidatorAndDependencies::new(dependencies_with_subnet_params( + } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), (1..=block_registry_version.get()) @@ -3274,12 +3266,13 @@ pub mod test { ) }) .collect(), - )); + ) + .build(); payload_builder .get_mut() .expect_validate_payload() .returning(|_, _, _, _| Ok(())); - schedule_subnet_split(&data_provider, ®istry_client, replica_config.subnet_id); + schedule_subnet_split(®istry_data_provider, ®istry, replica_config.subnet_id); let mut test_block = make_next_block(&pool); test_block.content.as_mut().context.registry_version = block_registry_version; @@ -3328,13 +3321,13 @@ pub mod test { validator, payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, time_source, replica_config, .. - } = ValidatorAndDependencies::new(dependencies_with_subnet_params( + } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), (1..=block_registry_version.get()) @@ -3347,7 +3340,8 @@ pub mod test { ) }) .collect(), - )); + ) + .build(); payload_builder .get_mut() .expect_validate_payload() @@ -3376,7 +3370,7 @@ pub mod test { .return_const(context.certified_height); time_source.set_time(context.time).unwrap(); - schedule_subnet_split(&data_provider, ®istry_client, replica_config.subnet_id); + schedule_subnet_split(®istry_data_provider, ®istry, replica_config.subnet_id); let result = validator.check_block_validity(&PoolReader::new(&pool), &summary_proposal); if block_registry_version <= SUBNET_SPLIT_REGISTRY_VERSION { @@ -3404,22 +3398,17 @@ pub mod test { const UNREADABLE_REGISTRY_VERSION: RegistryVersion = RegistryVersion::new(2); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let committee = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, pool, time_source, replica_config, .. - } = setup_dependencies_with_dkg_interval_length( - pool_config, - &committee, - DKG_INTERVAL_LENGTH, - ); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -3427,14 +3416,14 @@ pub mod test { // Delete the subnet's CUP contents record at `UNREADABLE_REGISTRY_VERSION`, so that the // subnet splitting status can no longer be determined at that version. - data_provider + registry_data_provider .add::( &make_catch_up_package_contents_key(replica_config.subnet_id), UNREADABLE_REGISTRY_VERSION, None, ) .expect("Failed to delete the CUP contents"); - registry_client.update_to_latest_version(); + registry.update_to_latest_version(); let test_block = make_next_block(&pool); let context = test_block.content.as_ref().context.clone(); @@ -3460,7 +3449,6 @@ pub mod test { #[allow(clippy::cognitive_complexity)] fn test_certified_height_change() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let subnet_members = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, payload_builder, @@ -3468,7 +3456,7 @@ pub mod test { mut pool, time_source, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() @@ -3521,7 +3509,6 @@ pub mod test { #[test] fn test_block_context_time() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let subnet_members = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, payload_builder, @@ -3529,7 +3516,7 @@ pub mod test { mut pool, time_source, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() @@ -3595,7 +3582,7 @@ pub mod test { validator, mut pool, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3646,7 +3633,7 @@ pub mod test { validator, mut pool, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3701,7 +3688,7 @@ pub mod test { validator, mut pool, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3885,11 +3872,9 @@ pub mod test { mut pool, time_source, .. - } = setup_dependencies_with_dkg_interval_length( - pool_config, - &(0..4).map(node_test_id).collect::>(), - cup_height.get() - 1, - ); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(cup_height.get() - 1) + .build(); pool.advance_round_normal_operation_no_cup_n(finalized_height.get()); @@ -3963,9 +3948,9 @@ pub mod test { state_manager, mut pool, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); - pool.advance_round_normal_operation_n(9); + pool.advance_round_normal_operation_n(DKG_INTERVAL_LENGTH); // Create, notarize, and finalize a block at the CUP height, but don't create a CUP. pool.prepare_round().dont_add_catch_up_package().advance(); @@ -3998,17 +3983,16 @@ pub mod test { #[test] fn test_out_of_sync_validation() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let subnet_members = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, payload_builder, state_manager, - registry_client, + registry, mut pool, time_source, replica_config, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() @@ -4045,7 +4029,7 @@ pub mod test { let pool_reader = PoolReader::new(&pool); let delay = get_block_maker_delay( &no_op_logger(), - registry_client.as_ref(), + registry.as_ref(), replica_config.subnet_id, &pool_reader, parent.clone(), @@ -4098,7 +4082,7 @@ pub mod test { let pool_reader = PoolReader::new(&pool); let delay = get_block_maker_delay( &no_op_logger(), - registry_client.as_ref(), + registry.as_ref(), replica_config.subnet_id, &pool_reader, parent.clone(), @@ -4133,14 +4117,13 @@ pub mod test { #[test] fn test_block_validated_through_notarization() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let subnet_members = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, payload_builder, state_manager, mut pool, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); pool.advance_round_normal_operation(); payload_builder @@ -4201,13 +4184,12 @@ pub mod test { fn setup_equivocation_proof_test( pool_config: ArtifactPoolConfig, ) -> (TestConsensusPool, Validator, EquivocationProof) { - let subnet_members = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, mut pool, replica_config, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); pool.advance_round_normal_operation(); pool.insert_validated(pool.make_next_beacon()); @@ -4392,14 +4374,13 @@ pub mod test { #[test] fn test_validator_rejects_incorrect_signature_in_notarization_fast_path() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let subnet_members = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, mut pool, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); - pool.advance_round_normal_operation_n(9); + pool.advance_round_normal_operation_n(DKG_INTERVAL_LENGTH); let mut block = pool.make_next_block(); @@ -4434,7 +4415,6 @@ pub mod test { #[test] fn test_create_equivocation_proof() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let subnet_members = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, state_manager, @@ -4442,7 +4422,7 @@ pub mod test { payload_builder, mut pool, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() @@ -4499,12 +4479,11 @@ pub mod test { #[test] fn test_cannot_disqualify_with_incorrect_rank() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let subnet_members = (0..4).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, mut pool, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let block = pool.make_next_block(); let mut block_with_malicious_signer = block.clone(); @@ -4534,34 +4513,34 @@ pub mod test { fn test_cannot_disqualify_with_proposal_from_different_version() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let subnet_members = (0..4).map(node_test_id).collect::>(); - let dkg_interval = 9; let ValidatorAndDependencies { validator, mut pool, replica_config, .. - } = ValidatorAndDependencies::new(dependencies_with_subnet_params( + } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![ ( 1, SubnetRecordBuilder::from(&subnet_members) - .with_dkg_interval_length(9) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(), ), ( 10, SubnetRecordBuilder::from(&subnet_members) - .with_dkg_interval_length(9) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .with_replica_version("new_version") .build(), ), ], - )); + ) + .build(); // Move to the end of the DKG interval where we switch versions - pool.advance_round_normal_operation_n(dkg_interval + 1); + pool.advance_round_normal_operation_n(DKG_INTERVAL_LENGTH + 1); assert!(pool.get_cache().finalized_block().payload.is_summary()); // An empty block created before the update @@ -4591,7 +4570,6 @@ pub mod test { #[test] fn test_ignore_disqualified_ranks() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let subnet_members = (0..7).map(node_test_id).collect::>(); let ValidatorAndDependencies { validator, mut pool, @@ -4599,7 +4577,7 @@ pub mod test { payload_builder, state_manager, .. - } = setup_dependencies(pool_config, &subnet_members); + } = ValidatorAndDependenciesBuilder::new(pool_config, 7).build(); payload_builder .get_mut() @@ -4837,7 +4815,17 @@ pub mod test { payload_builder, mut pool, .. - } = setup_dependencies(pool_config, &[NODE_1, NODE_2]); + } = ValidatorAndDependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![( + 1, + SubnetRecordBuilder::from(&[NODE_1, NODE_2]) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(), + )], + ) + .build(); validator.log = logger; payload_builder diff --git a/rs/consensus/utils/src/lib.rs b/rs/consensus/utils/src/lib.rs index 1f96c42ed810..7143786d0e51 100644 --- a/rs/consensus/utils/src/lib.rs +++ b/rs/consensus/utils/src/lib.rs @@ -517,7 +517,7 @@ mod tests { }; use super::*; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_management_canister_types_private::MasterPublicKeyId; use ic_replicated_state::metadata_state::subnet_call_context_manager::{ SetupInitialDkgContext, SignWithThresholdContext, @@ -858,7 +858,8 @@ mod tests { fn test_ignore_disqualified_ranks() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { const SUBNET_SIZE: u64 = 10; - let Dependencies { mut pool, .. } = dependencies(pool_config, SUBNET_SIZE); + let Dependencies { mut pool, .. } = + DependenciesBuilder::new(pool_config, SUBNET_SIZE).build(); let height = Height::new(1); diff --git a/rs/consensus/utils/src/pool_reader.rs b/rs/consensus/utils/src/pool_reader.rs index cc53af781a50..028d8ab06919 100644 --- a/rs/consensus/utils/src/pool_reader.rs +++ b/rs/consensus/utils/src/pool_reader.rs @@ -588,7 +588,7 @@ where #[cfg(test)] pub mod test { use super::*; - use ic_consensus_mocks::{Dependencies, dependencies, dependencies_with_subnet_params}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_interfaces_registry::RegistryClient; use ic_test_utilities_registry::{SubnetRecordBuilder, add_subnet_record}; use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; @@ -597,18 +597,9 @@ pub mod test { fn test_get_dkg_summary_block() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let interval_length = 3; - let Dependencies { mut pool, .. } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from( - (0..1).map(node_test_id).collect::>().as_slice(), - ) - .with_dkg_interval_length(interval_length) - .build(), - )], - ); + let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 1) + .with_dkg_interval_length(interval_length) + .build(); // Get the finalized block after skipping exactly one DKG interval. let height = pool.advance_round_normal_operation_n(interval_length + 1); @@ -693,7 +684,7 @@ pub mod test { #[test] fn test_get_finalized_block_at_height_without_finalization() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let Dependencies { mut pool, .. } = dependencies(pool_config, 1); + let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 1).build(); let start = pool.make_next_block(); pool.insert_beacon_chain(&pool.make_next_beacon(), Height::from(10)); pool.insert_block_chain_with(start.clone(), Height::from(10)); @@ -718,18 +709,10 @@ pub mod test { #[test] fn test_get_notarized_finalized_height() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let committee = vec![node_test_id(0)]; let interval_length = 4; - let Dependencies { mut pool, .. } = dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(interval_length) - .build(), - )], - ); + let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 1) + .with_dkg_interval_length(interval_length) + .build(); pool.advance_round_normal_operation_n(4); pool.prepare_round().dont_add_catch_up_package().advance(); let block = pool.latest_notarized_blocks().next().unwrap(); @@ -755,7 +738,8 @@ pub mod test { let replicas = 10; let f = 3; let block_proposals_per_round = f + 1; - let Dependencies { mut pool, .. } = dependencies(pool_config, replicas); + let Dependencies { mut pool, .. } = + DependenciesBuilder::new(pool_config, replicas).build(); // Because `TestConsensusPool::advance_round` alternates between // putting blocks in validated and unvalidated pools for each rank, @@ -819,11 +803,12 @@ pub mod test { registry, replica_config, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![(1, record.clone())], - ); + ) + .build(); let subnet_id = replica_config.subnet_id; let pool_reader = PoolReader::new(&pool); // Right now we only have the genesis block. For the genesis interval and the diff --git a/rs/https_outcalls/consensus/benches/payload_validation.rs b/rs/https_outcalls/consensus/benches/payload_validation.rs index 1544cb980495..8331ac19d1af 100644 --- a/rs/https_outcalls/consensus/benches/payload_validation.rs +++ b/rs/https_outcalls/consensus/benches/payload_validation.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use criterion::{BenchmarkId, Criterion, black_box, criterion_group, criterion_main}; -use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; +use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_temp_crypto::{NodeKeysToGenerate, TempCryptoComponent}; use ic_https_outcalls_consensus::payload_builder::CanisterHttpPayloadBuilderImpl; use ic_https_outcalls_pricing::fees::{flexible_initial_spent, non_flexible_initial_spent}; @@ -178,11 +178,12 @@ fn build_target( }) .build(); - let deps = dependencies_with_subnet_params( + let deps = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![(REGISTRY_VERSION.get(), subnet_record)], - ); + ) + .build(); let registry_client: Arc = deps.registry.clone(); diff --git a/rs/https_outcalls/consensus/src/payload_builder/tests.rs b/rs/https_outcalls/consensus/src/payload_builder/tests.rs index c8e44ad2e12f..20f77f4f1d25 100644 --- a/rs/https_outcalls/consensus/src/payload_builder/tests.rs +++ b/rs/https_outcalls/consensus/src/payload_builder/tests.rs @@ -11,7 +11,7 @@ use crate::payload_builder::{ use assert_matches::assert_matches; use candid::{Decode, Encode}; use ic_artifact_pool::canister_http_pool::CanisterHttpPoolImpl; -use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params}; +use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_error_types::RejectCode; use ic_https_outcalls_pricing::fees::{ consensus_cost_coefficient, flexible_initial_spent, min_flexible_consensus_cost, @@ -2112,11 +2112,12 @@ pub(crate) fn test_config_with_http_feature( canister_http_pool, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![(1, subnet_record)], - ); + ) + .build(); let payload_builder = CanisterHttpPayloadBuilderImpl::new( canister_http_pool.clone(), diff --git a/rs/https_outcalls/consensus/src/pool_manager.rs b/rs/https_outcalls/consensus/src/pool_manager.rs index f8d4cf348c03..9952ed700ae0 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -630,7 +630,7 @@ pub mod test { use super::*; use assert_matches::assert_matches; use ic_artifact_pool::canister_http_pool::CanisterHttpPoolImpl; - use ic_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_consensus_utils::crypto::SignVerify; use ic_error_types::RejectCode; use ic_interfaces::p2p::consensus::{MutablePool, UnvalidatedArtifact}; @@ -732,7 +732,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -831,7 +831,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -940,7 +940,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -1041,7 +1041,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -1164,7 +1164,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -1349,7 +1349,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); // Define the delegated node and a different, incorrect signer. let delegated_node_id = ic_test_utilities_types::ids::node_test_id(1); @@ -1446,7 +1446,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -1548,7 +1548,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -1720,7 +1720,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -1826,7 +1826,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); // This is the ID of the dishonest replica sending the artifact let delegated_node_id = ic_test_utilities_types::ids::node_test_id(1); @@ -1940,7 +1940,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let peer_id = node_test_id(1); let callback_id = CallbackId::from(0); @@ -2035,7 +2035,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); let callback_id = CallbackId::from(0); state_manager @@ -2127,7 +2127,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -2236,7 +2236,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -2324,7 +2324,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); // There are 2 contexts in the replicated state. let contexts = (3..5) @@ -2415,7 +2415,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); let stale_callback_id = CallbackId::from(3); let active_callback_id = CallbackId::from(4); @@ -2516,7 +2516,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); // Delegate the request to this node so it is the authorized signer // and creates a share for the injected response. @@ -2613,7 +2613,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -2714,7 +2714,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); let committee_member_1_self = replica_config.node_id; let committee_member_2 = ic_test_utilities_types::ids::node_test_id(1); @@ -2803,7 +2803,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); // Define the delegated node and a different, incorrect signer. let committee_member = ic_test_utilities_types::ids::node_test_id(1); @@ -2903,7 +2903,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let committee_member = replica_config.node_id; let callback_id = CallbackId::from(0); @@ -3091,7 +3091,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); let mut shim_mock = MockNonBlockingChannel::::new(); shim_mock .expect_try_receive() @@ -3261,7 +3261,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 4); + } = DependenciesBuilder::new(pool_config.clone(), 4).build(); let dummy_node_id = replica_config.node_id; // irrelevant for this test let callback_id = CallbackId::from(5); @@ -3353,7 +3353,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); // Use a context with a small per-replica allowance. let request = CanisterHttpRequestContext { @@ -3454,7 +3454,7 @@ pub mod test { state_manager, registry, .. - } = dependencies(pool_config.clone(), 5); + } = DependenciesBuilder::new(pool_config.clone(), 5).build(); // A free subnet grants a zero per-replica allowance (nothing is // charged), yet the reported spend is still accumulated for cost @@ -3567,7 +3567,7 @@ pub mod test { registry, registry_data_provider, .. - } = dependencies(pool_config.clone(), 1); + } = DependenciesBuilder::new(pool_config.clone(), 1).build(); // Register a handful of API boundary nodes, each with a distinct // HTTP endpoint. `get_{system,app}_api_boundary_node_ids` splits the