From bc7a58f6fc0be174c3f91090fb7a482fe165a2fd Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 08:29:33 +0000 Subject: [PATCH 01/34] feat(consensus): introduce DependenciesBuilder in ic-consensus-mocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a builder for the consensus test Dependencies fixture with three constructors: new(pool_config, nodes), single_subnet(pool_config, subnet_id, records) and multiple_subnets(pool_config, records) — the latter supporting multiple subnets in the registry, as needed by subnet-splitting tests. The state manager is mocked by default and can be opted out via without_mocked_state_manager(). The legacy dependencies* functions are retargeted as thin wrappers and will be removed once all call sites are migrated. Co-Authored-By: Claude Fable 5 --- rs/consensus/mocks/src/lib.rs | 328 ++++++++++++++++++++++------------ 1 file changed, 210 insertions(+), 118 deletions(-) diff --git a/rs/consensus/mocks/src/lib.rs b/rs/consensus/mocks/src/lib.rs index 3895d75b7e71..61865aba45cc 100644 --- a/rs/consensus/mocks/src/lib.rs +++ b/rs/consensus/mocks/src/lib.rs @@ -17,7 +17,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 +31,10 @@ use ic_types::{ }; use mockall::predicate::*; use mockall::*; -use std::sync::{Arc, RwLock}; +use std::{ + collections::{BTreeMap, BTreeSet}, + sync::{Arc, RwLock}, +}; mock! { pub PayloadBuilder {} @@ -105,135 +111,221 @@ pub struct Dependencies { 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 struct DependenciesBuilder { + pool_config: ArtifactPoolConfig, + sorted_subnet_records: Vec<(u64, SubnetId, SubnetRecord)>, + replica_config: ReplicaConfig, + mocked_state_manager: bool, + #[allow(clippy::type_complexity)] + additional_registry_mutations: Vec)>>, +} + +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())], + ) + } + + /// 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." + ); + + // 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, + mocked_state_manager: true, + additional_registry_mutations: Vec::new(), + } + } + + pub fn with_replica_config(mut self, replica_config: ReplicaConfig) -> Self { + self.replica_config = replica_config; + self + } + + /// Leaves the returned `RefMockStateManager` without any expectations, so + /// that the test can set up its own `get_state_at` behavior. + pub fn without_mocked_state_manager(mut self) -> Self { + self.mocked_state_manager = false; + self + } + + pub fn add_additional_registry_mutation( + mut self, + mutation: impl FnOnce(&Arc) + 'static, + ) -> Self { + self.additional_registry_mutations.push(Box::new(mutation)); + 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 all_subnet_ids: BTreeSet = BTreeSet::default(); + let mut subnet_ids_at_version: BTreeMap> = BTreeMap::default(); + for (version, subnet_id, record) in self.sorted_subnet_records { + if all_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); + + subnet_ids_at_version + .entry(version) + .or_default() + .insert(subnet_id); + } + for (version, subnet_ids) in subnet_ids_at_version { + add_subnet_list_record(®istry_data_provider, version, Vec::from_iter(subnet_ids)); + } + + for registry_mutation in self.additional_registry_mutations { + registry_mutation(®istry_data_provider); + } + + 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.mocked_state_manager { + 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, + dkg_pool, + idkg_pool, + canister_http_pool, + } + } +} + +/// Deprecated: use `DependenciesBuilder::single_subnet(...) +/// .without_mocked_state_manager().build()` instead. pub fn dependencies_with_subnet_records_with_raw_state_manager( 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))), - ) - .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, - } + DependenciesBuilder::single_subnet(pool_config, subnet_id, records) + .without_mocked_state_manager() + .build() } -/// 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. +/// Deprecated: use `DependenciesBuilder::single_subnet(...).build()` instead. 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)), - ))); - - Dependencies { - crypto, - registry, - registry_data_provider, - membership, - time_source, - pool, - replica_config, - state_manager, - dkg_pool, - idkg_pool, - canister_http_pool, - } + DependenciesBuilder::single_subnet(pool_config, subnet_id, records).build() } -/// 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. +/// Deprecated: use `DependenciesBuilder::new(...).build()` instead. 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())], - ) + DependenciesBuilder::new(pool_config, nodes).build() } From c02383453b2dbbc2b6cf51e596a61f6b6f26bc0f Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 08:31:26 +0000 Subject: [PATCH 02/34] refactor(consensus): use DependenciesBuilder in ic-consensus-certification tests Co-Authored-By: Claude Fable 5 --- rs/consensus/certification/src/certifier.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/rs/consensus/certification/src/certifier.rs b/rs/consensus/certification/src/certifier.rs index a02af098f609..48f732ba8ba5 100644 --- a/rs/consensus/certification/src/certifier.rs +++ b/rs/consensus/certification/src/certifier.rs @@ -616,7 +616,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_consensus_mocks::{Dependencies, dependencies}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_crypto_tree_hash::{Digest, Witness, sparse_labeled_tree_from_paths}; use ic_interfaces::{ certification::CertificationPool, @@ -747,7 +747,7 @@ mod tests { crypto, state_manager, .. - } = dependencies(pool_config.clone(), 1); + } = DependenciesBuilder::new(pool_config.clone(), 1).build(); let certifier = CertifierImpl::new( replica_config, @@ -783,7 +783,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(); @@ -849,7 +849,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); @@ -986,7 +986,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(); @@ -1066,7 +1066,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); @@ -1139,7 +1139,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); @@ -1211,7 +1211,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); @@ -1384,7 +1384,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(); @@ -1482,7 +1482,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( From 4761870c976c40b683d3aa2c357accbf24674d1c Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 08:31:26 +0000 Subject: [PATCH 03/34] refactor(consensus): use DependenciesBuilder in ic-consensus-utils tests Co-Authored-By: Claude Fable 5 --- rs/consensus/utils/src/lib.rs | 5 +++-- rs/consensus/utils/src/pool_reader.rs | 22 +++++++++++++--------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/rs/consensus/utils/src/lib.rs b/rs/consensus/utils/src/lib.rs index eae36d7cae12..2699c92187c4 100644 --- a/rs/consensus/utils/src/lib.rs +++ b/rs/consensus/utils/src/lib.rs @@ -493,7 +493,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, @@ -834,7 +834,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..4fe17ce6028a 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,7 +597,7 @@ 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( + let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -608,7 +608,8 @@ pub mod test { .with_dkg_interval_length(interval_length) .build(), )], - ); + ) + .build(); // Get the finalized block after skipping exactly one DKG interval. let height = pool.advance_round_normal_operation_n(interval_length + 1); @@ -693,7 +694,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)); @@ -720,7 +721,7 @@ pub mod test { 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( + let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -729,7 +730,8 @@ pub mod test { .with_dkg_interval_length(interval_length) .build(), )], - ); + ) + .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 +757,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 +822,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 From 5704d417f4ca391a70e4605958ab887b82455bdb Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 08:32:50 +0000 Subject: [PATCH 04/34] refactor(consensus): use DependenciesBuilder in ic-consensus tests Co-Authored-By: Claude Fable 5 --- rs/consensus/src/consensus.rs | 4 +- rs/consensus/src/consensus/block_maker.rs | 34 ++-- rs/consensus/src/consensus/bounds.rs | 9 +- .../src/consensus/catchup_package_maker.rs | 26 +-- rs/consensus/src/consensus/finalizer.rs | 9 +- rs/consensus/src/consensus/notary.rs | 31 ++- rs/consensus/src/consensus/payload_builder.rs | 8 +- rs/consensus/src/consensus/priority.rs | 9 +- rs/consensus/src/consensus/proptests.rs | 12 +- rs/consensus/src/consensus/purger.rs | 14 +- .../src/consensus/random_beacon_maker.rs | 4 +- .../src/consensus/random_tape_maker.rs | 4 +- .../src/consensus/share_aggregator.rs | 9 +- rs/consensus/src/consensus/status.rs | 7 +- rs/consensus/src/consensus/validator.rs | 180 ++++++++++-------- 15 files changed, 206 insertions(+), 154 deletions(-) diff --git a/rs/consensus/src/consensus.rs b/rs/consensus/src/consensus.rs index 7563f0cee5b5..66b7781c7c6b 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; @@ -676,7 +676,7 @@ mod tests { dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params(pool_config, subnet_id, vec![(1, record)]); + } = DependenciesBuilder::single_subnet(pool_config, subnet_id, 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..c2396f21dd3f 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; @@ -766,7 +766,7 @@ mod tests { dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![ @@ -783,7 +783,8 @@ mod tests { .build(), ), ], - ); + ) + .build(); pool.advance_round_normal_operation_n(4); @@ -936,7 +937,7 @@ mod tests { dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -945,7 +946,8 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .build(); pool.advance_round_normal_operation_n(8); @@ -1090,7 +1092,7 @@ mod tests { dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config.clone(), subnet_test_id(0), vec![ @@ -1109,7 +1111,8 @@ mod tests { .build(), ), ], - ); + ) + .build(); state_manager .get_mut() @@ -1238,7 +1241,12 @@ mod tests { 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 @@ -1417,7 +1425,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 +1434,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)); @@ -1592,7 +1601,7 @@ mod tests { dkg_pool, idkg_pool, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, SOURCE_SUBNET_ID, (1..=MAX_REGISTRY_VERSION) @@ -1605,7 +1614,8 @@ mod tests { ) }) .collect(), - ); + ) + .build(); let mut payload_builder = MockPayloadBuilder::new(); payload_builder diff --git a/rs/consensus/src/consensus/bounds.rs b/rs/consensus/src/consensus/bounds.rs index 2ff4cdbc076b..d0dd527ffc02 100644 --- a/rs/consensus/src/consensus/bounds.rs +++ b/rs/consensus/src/consensus/bounds.rs @@ -186,7 +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_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_test_utilities_registry::SubnetRecordBuilder; use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; @@ -221,7 +221,12 @@ mod tests { registry, replica_config, .. - } = dependencies_with_subnet_params(pool_config, subnet_test_id(0), vec![(1, record)]); + } = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, record)], + ) + .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..7dbb3ecd3831 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; @@ -345,7 +342,7 @@ mod tests { 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( + let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -354,7 +351,8 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .build(); // Ignore state sync and state divergence deps.state_manager @@ -607,7 +605,7 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_records_with_raw_state_manager( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -616,7 +614,9 @@ mod tests { .with_dkg_interval_length(interval_length) .build(), )], - ); + ) + .without_mocked_state_manager() + .build(); let height = Height::from(0); state_manager @@ -702,7 +702,7 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -711,7 +711,8 @@ mod tests { .with_dkg_interval_length(interval_length) .build(), )], - ); + ) + .build(); pool.advance_round_normal_operation_n(5); let cup_height = PoolReader::new(&pool).get_catch_up_height(); @@ -761,7 +762,7 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -770,7 +771,8 @@ mod tests { .with_dkg_interval_length(interval_length) .build(), )], - ); + ) + .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..700117392955 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; @@ -410,7 +410,7 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -419,7 +419,8 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .build(); state_manager .get_mut() .expect_latest_certified_height() @@ -611,7 +612,7 @@ mod tests { crypto, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -620,7 +621,8 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .build(); state_manager .get_mut() .expect_latest_certified_height() @@ -688,7 +690,12 @@ mod tests { state_manager, membership, .. - } = dependencies_with_subnet_params(pool_config, subnet_test_id(0), vec![(1, record)]); + } = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, record)], + ) + .build(); let last_cup_dkg_info = PoolReader::new(&pool) .get_highest_catch_up_package() .content @@ -812,7 +819,12 @@ mod tests { state_manager, membership, .. - } = dependencies_with_subnet_params(pool_config, subnet_test_id(0), vec![(1, record)]); + } = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, record)], + ) + .build(); let certified_height = Arc::new(RwLock::new(Height::from(0))); let certified_height_clone = Arc::clone(&certified_height); @@ -917,7 +929,7 @@ mod tests { state_manager, membership, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![ @@ -935,7 +947,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..e67b7f06b950 100644 --- a/rs/consensus/src/consensus/priority.rs +++ b/rs/consensus/src/consensus/priority.rs @@ -119,7 +119,7 @@ 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}; @@ -136,7 +136,7 @@ mod tests { 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( + let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -145,7 +145,8 @@ mod tests { .with_dkg_interval_length(dkg_interval) .build(), )], - ); + ) + .build(); // Advance pool *without* producing CUP to the maximum height beyond // which we don't validate non-CUP artifacts anymore. @@ -197,7 +198,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..289f8f31b5bd 100644 --- a/rs/consensus/src/consensus/purger.rs +++ b/rs/consensus/src/consensus/purger.rs @@ -472,7 +472,7 @@ 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; @@ -493,7 +493,7 @@ mod tests { replica_config, registry, .. - } = dependencies(pool_config, 1); + } = DependenciesBuilder::new(pool_config, 1).build(); state_manager .get_mut() @@ -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); @@ -738,7 +738,7 @@ mod tests { replica_config, registry, .. - } = 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); @@ -823,7 +823,7 @@ mod tests { replica_config, registry, .. - } = dependencies(pool_config, 10); + } = DependenciesBuilder::new(pool_config, 10).build(); state_manager .get_mut() .expect_latest_state_height() 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..ad32ca8b9bd9 100644 --- a/rs/consensus/src/consensus/share_aggregator.rs +++ b/rs/consensus/src/consensus/share_aggregator.rs @@ -186,7 +186,7 @@ fn to_messages(artifacts: Vec) -> Vec 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(), - )], - )) + ValidatorAndDependencies::new( + DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![( + 1, + SubnetRecordBuilder::from(node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + )], + ) + .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(), - )], - )) + ValidatorAndDependencies::new( + DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![( + 1, + SubnetRecordBuilder::from(node_ids) + .with_dkg_interval_length(9) + .build(), + )], + ) + .without_mocked_state_manager() + .build(), + ) } #[test] @@ -2882,17 +2886,20 @@ pub mod test { mut pool, time_source, .. - } = ValidatorAndDependencies::new(dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .with_halt_at_cup_height(true) - .build(), - )], - )); + } = ValidatorAndDependencies::new( + DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![( + 1, + SubnetRecordBuilder::from(&committee) + .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 @@ -3261,20 +3268,23 @@ pub mod test { time_source, replica_config, .. - } = ValidatorAndDependencies::new(dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - (1..=block_registry_version.get()) - .map(|version| { - ( - version, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .build(), - ) - }) - .collect(), - )); + } = ValidatorAndDependencies::new( + DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + (1..=block_registry_version.get()) + .map(|version| { + ( + version, + SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(), + ) + }) + .collect(), + ) + .build(), + ); payload_builder .get_mut() .expect_validate_payload() @@ -3334,20 +3344,23 @@ pub mod test { time_source, replica_config, .. - } = ValidatorAndDependencies::new(dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - (1..=block_registry_version.get()) - .map(|version| { - ( - version, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .build(), - ) - }) - .collect(), - )); + } = ValidatorAndDependencies::new( + DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + (1..=block_registry_version.get()) + .map(|version| { + ( + version, + SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(), + ) + }) + .collect(), + ) + .build(), + ); payload_builder .get_mut() .expect_validate_payload() @@ -4540,25 +4553,28 @@ pub mod test { mut pool, replica_config, .. - } = ValidatorAndDependencies::new(dependencies_with_subnet_params( - pool_config, - subnet_test_id(0), - vec![ - ( - 1, - SubnetRecordBuilder::from(&subnet_members) - .with_dkg_interval_length(9) - .build(), - ), - ( - 10, - SubnetRecordBuilder::from(&subnet_members) - .with_dkg_interval_length(9) - .with_replica_version("new_version") - .build(), - ), - ], - )); + } = ValidatorAndDependencies::new( + DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![ + ( + 1, + SubnetRecordBuilder::from(&subnet_members) + .with_dkg_interval_length(9) + .build(), + ), + ( + 10, + SubnetRecordBuilder::from(&subnet_members) + .with_dkg_interval_length(9) + .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); From cd354818a3273cbdc8237fa83ee17f9e5ba3568b Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 08:33:44 +0000 Subject: [PATCH 05/34] refactor(consensus): use DependenciesBuilder in ic-consensus-dkg tests Co-Authored-By: Claude Fable 5 --- rs/consensus/dkg/src/dkg_key_manager.rs | 7 ++- rs/consensus/dkg/src/lib.rs | 69 ++++++++++++++--------- rs/consensus/dkg/src/payload_builder.rs | 31 +++++----- rs/consensus/dkg/src/payload_validator.rs | 22 +++++--- rs/consensus/dkg/src/utils.rs | 7 ++- 5 files changed, 80 insertions(+), 56 deletions(-) diff --git a/rs/consensus/dkg/src/dkg_key_manager.rs b/rs/consensus/dkg/src/dkg_key_manager.rs index c533205776da..16079391346e 100644 --- a/rs/consensus/dkg/src/dkg_key_manager.rs +++ b/rs/consensus/dkg/src/dkg_key_manager.rs @@ -557,7 +557,7 @@ 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; @@ -570,7 +570,7 @@ mod tests { 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( + let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(222), vec![( @@ -579,7 +579,8 @@ mod tests { .with_dkg_interval_length(dkg_interval_len) .build(), )], - ); + ) + .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..0f00d893abd3 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() @@ -780,7 +778,7 @@ mod tests { state_manager, dkg_pool, .. - } = dependencies_with_subnet_records_with_raw_state_manager( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -789,7 +787,9 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .without_mocked_state_manager() + .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); complement_state_manager_with_setup_initial_dkg_request( @@ -943,7 +943,7 @@ mod tests { registry, state_manager, .. - } = dependencies_with_subnet_records_with_raw_state_manager( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -952,7 +952,9 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .without_mocked_state_manager() + .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); complement_state_manager_with_setup_initial_dkg_request( @@ -1065,14 +1067,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() @@ -1513,7 +1515,7 @@ mod tests { let subnet_id = subnet_test_id(0); // Set pool_1 and pool_2 - let dependencies_1 = dependencies_with_subnet_records_with_raw_state_manager( + let dependencies_1 = DependenciesBuilder::single_subnet( pool_config_1, subnet_id, vec![( @@ -1522,8 +1524,10 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); - let dependencies_2 = dependencies_with_subnet_records_with_raw_state_manager( + ) + .without_mocked_state_manager() + .build(); + let dependencies_2 = DependenciesBuilder::single_subnet( pool_config_2, subnet_id, vec![( @@ -1532,7 +1536,9 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .without_mocked_state_manager() + .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 +1722,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 +1731,9 @@ mod tests { .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .build(), )], - ); + ) + .without_mocked_state_manager() + .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); complement_state_manager_with_setup_initial_dkg_request( @@ -2101,7 +2109,7 @@ mod tests { 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( + let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -2110,7 +2118,9 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .without_mocked_state_manager() + .build(); // Start without context so remote dealing validation is deferred. complement_state_manager_with_dkg_contexts( @@ -2224,7 +2234,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 +2253,9 @@ mod tests { }) .build(), )], - ); + ) + .without_mocked_state_manager() + .build(); // No contexts at the beginning complement_state_manager_with_dkg_contexts(deps.state_manager.clone(), vec![], None); @@ -2333,7 +2345,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 +2364,9 @@ mod tests { }) .build(), )], - ); + ) + .without_mocked_state_manager() + .build(); let registry_version = deps.registry.get_latest_version(); let mut contexts = vec![ @@ -2558,7 +2572,7 @@ mod tests { registry, replica_config, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -2568,7 +2582,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..eec00cd7d7d2 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_mocked_state_manager() + .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..dd30bb059cd0 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::{ @@ -306,7 +306,7 @@ mod tests { state_manager, dkg_pool, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -315,7 +315,8 @@ mod tests { .with_dkg_interval_length(dkg_interval_length) .build(), )], - ); + ) + .build(); let context = ValidationContext { registry_version: RegistryVersion::from(5), @@ -522,7 +523,7 @@ mod tests { registry, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config.clone(), SUBNET_1, vec![( @@ -531,7 +532,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 +627,7 @@ mod tests { registry, state_manager, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config.clone(), subnet_id, vec![( @@ -634,7 +636,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 +731,7 @@ mod tests { state_manager, registry_data_provider, .. - } = dependencies_with_subnet_params( + } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -737,7 +740,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..d3ce6e5f5f7b 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; @@ -317,7 +317,7 @@ mod tests { 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( + let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_id, vec![( @@ -326,7 +326,8 @@ mod tests { .with_dkg_interval_length(dkg_interval_len) .build(), )], - ); + ) + .build(); pool.advance_round_normal_operation_n(dkg_interval_len); let pool_reader = PoolReader::new(&pool); From c72e2ed8cec7b69bbc141be522a3457a53f9c740 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 08:33:44 +0000 Subject: [PATCH 06/34] refactor(consensus): use DependenciesBuilder in ic-consensus-idkg tests Co-Authored-By: Claude Fable 5 --- rs/consensus/idkg/src/lib.rs | 5 +++-- rs/consensus/idkg/src/payload_builder.rs | 14 +++++++------- .../idkg/src/payload_builder/pre_signatures.rs | 4 ++-- rs/consensus/idkg/src/test_utils.rs | 16 +++++++++------- rs/consensus/idkg/src/utils.rs | 12 ++++++------ 5 files changed, 27 insertions(+), 24 deletions(-) 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(); From 6cf3278091296eb0069bd43fefee698064913359 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 08:33:44 +0000 Subject: [PATCH 07/34] refactor(consensus): use DependenciesBuilder in ic-consensus-chain-key tests Co-Authored-By: Claude Fable 5 --- rs/consensus/chain_key/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/rs/consensus/chain_key/src/lib.rs b/rs/consensus/chain_key/src/lib.rs index 951c15e00300..1b2bb51cf566 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_mocked_state_manager() + .build(); // Enable the configured keys if let Some(config) = config From 77a0f073dd1184cfe9add92a6c393ae6e1bc27ea Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 08:35:07 +0000 Subject: [PATCH 08/34] refactor(consensus): use DependenciesBuilder in ic-https-outcalls-consensus tests Co-Authored-By: Claude Fable 5 --- .../consensus/benches/payload_validation.rs | 7 +-- .../consensus/src/payload_builder/tests.rs | 7 +-- .../consensus/src/pool_manager.rs | 50 +++++++++---------- 3 files changed, 33 insertions(+), 31 deletions(-) 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 d8b70401ebad..dc83e79e4098 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, non_flexible_initial_spent, @@ -2066,11 +2066,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 242583a68527..02377bcfc58b 100644 --- a/rs/https_outcalls/consensus/src/pool_manager.rs +++ b/rs/https_outcalls/consensus/src/pool_manager.rs @@ -661,7 +661,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}; @@ -763,7 +763,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() @@ -862,7 +862,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() @@ -971,7 +971,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() @@ -1072,7 +1072,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() @@ -1195,7 +1195,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() @@ -1380,7 +1380,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); @@ -1477,7 +1477,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() @@ -1579,7 +1579,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() @@ -1751,7 +1751,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() @@ -1857,7 +1857,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); @@ -1970,7 +1970,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() @@ -2079,7 +2079,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() @@ -2167,7 +2167,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) @@ -2258,7 +2258,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); @@ -2359,7 +2359,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. @@ -2456,7 +2456,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() @@ -2557,7 +2557,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); @@ -2646,7 +2646,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); @@ -2746,7 +2746,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); @@ -2934,7 +2934,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() @@ -3104,7 +3104,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); @@ -3196,7 +3196,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 { @@ -3297,7 +3297,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 @@ -3410,7 +3410,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 From 77c63eb1552ae7ecf1076052948cf5dbd86b9d46 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 08:35:07 +0000 Subject: [PATCH 09/34] refactor(consensus): remove legacy dependencies* functions from ic-consensus-mocks All call sites have been migrated to DependenciesBuilder. Co-Authored-By: Claude Fable 5 --- rs/consensus/mocks/src/lib.rs | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/rs/consensus/mocks/src/lib.rs b/rs/consensus/mocks/src/lib.rs index 61865aba45cc..4b95f828ba26 100644 --- a/rs/consensus/mocks/src/lib.rs +++ b/rs/consensus/mocks/src/lib.rs @@ -303,29 +303,3 @@ impl DependenciesBuilder { } } } - -/// Deprecated: use `DependenciesBuilder::single_subnet(...) -/// .without_mocked_state_manager().build()` instead. -pub fn dependencies_with_subnet_records_with_raw_state_manager( - pool_config: ArtifactPoolConfig, - subnet_id: SubnetId, - records: Vec<(u64, SubnetRecord)>, -) -> Dependencies { - DependenciesBuilder::single_subnet(pool_config, subnet_id, records) - .without_mocked_state_manager() - .build() -} - -/// Deprecated: use `DependenciesBuilder::single_subnet(...).build()` instead. -pub fn dependencies_with_subnet_params( - pool_config: ArtifactPoolConfig, - subnet_id: SubnetId, - records: Vec<(u64, SubnetRecord)>, -) -> Dependencies { - DependenciesBuilder::single_subnet(pool_config, subnet_id, records).build() -} - -/// Deprecated: use `DependenciesBuilder::new(...).build()` instead. -pub fn dependencies(pool_config: ArtifactPoolConfig, nodes: u64) -> Dependencies { - DependenciesBuilder::new(pool_config, nodes).build() -} From deb8369e0f21d7dde352cbeb747d1a21e8e4f011 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 09:01:14 +0000 Subject: [PATCH 10/34] fix: remove unused `additional_registry_mutations` --- rs/consensus/mocks/src/lib.rs | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/rs/consensus/mocks/src/lib.rs b/rs/consensus/mocks/src/lib.rs index 4b95f828ba26..e615ea9787ac 100644 --- a/rs/consensus/mocks/src/lib.rs +++ b/rs/consensus/mocks/src/lib.rs @@ -116,8 +116,6 @@ pub struct DependenciesBuilder { sorted_subnet_records: Vec<(u64, SubnetId, SubnetRecord)>, replica_config: ReplicaConfig, mocked_state_manager: bool, - #[allow(clippy::type_complexity)] - additional_registry_mutations: Vec)>>, } impl DependenciesBuilder { @@ -179,7 +177,6 @@ impl DependenciesBuilder { }, sorted_subnet_records: subnet_records, mocked_state_manager: true, - additional_registry_mutations: Vec::new(), } } @@ -195,14 +192,6 @@ impl DependenciesBuilder { self } - pub fn add_additional_registry_mutation( - mut self, - mutation: impl FnOnce(&Arc) + 'static, - ) -> Self { - self.additional_registry_mutations.push(Box::new(mutation)); - self - } - pub fn build(self) -> Dependencies { let time_source = FastForwardTimeSource::new(); let registry_data_provider = Arc::new(ProtoRegistryDataProvider::new()); @@ -233,10 +222,6 @@ impl DependenciesBuilder { add_subnet_list_record(®istry_data_provider, version, Vec::from_iter(subnet_ids)); } - for registry_mutation in self.additional_registry_mutations { - registry_mutation(®istry_data_provider); - } - let registry = Arc::new(FakeRegistryClient::new( Arc::clone(®istry_data_provider) as Arc<_> )); From b8f98f7f0016da9fa31af8eae3145ed03f98522f Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 09:44:54 +0000 Subject: [PATCH 11/34] feat(subnet-splitting): add payload_builder, message_routing and with_dkg_interval_length to consensus test Dependencies Dependencies now provides a bare RefMockPayloadBuilder and RefMockMessageRouting (no pre-armed expectations), so tests no longer need to construct these mocks themselves. DependenciesBuilder gains with_dkg_interval_length(u64), which sets the DKG interval length on every subnet record passed to the builder. Co-Authored-By: Claude Fable 5 --- Cargo.lock | 1 + rs/consensus/mocks/BUILD.bazel | 1 + rs/consensus/mocks/Cargo.toml | 1 + rs/consensus/mocks/src/lib.rs | 16 +++++++++++++++- 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index b0868fe48e91..90eb89243717 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7851,6 +7851,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/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 e615ea9787ac..cbb22e9dd7e0 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; @@ -61,7 +62,7 @@ mock! { /// Sync wrapper to allow shared modification. See [`RefMockStateManager`]. #[derive(Default)] pub struct RefMockPayloadBuilder { - pub mock: RwLock, + mock: RwLock, } impl RefMockPayloadBuilder { @@ -106,6 +107,8 @@ 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>, @@ -185,6 +188,15 @@ impl DependenciesBuilder { 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 + } + /// Leaves the returned `RefMockStateManager` without any expectations, so /// that the test can set up its own `get_state_at` behavior. pub fn without_mocked_state_manager(mut self) -> Self { @@ -282,6 +294,8 @@ impl DependenciesBuilder { 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, From 4e559a8932c8537ea1e8e586e5f18dc4e768b95c Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 09:50:52 +0000 Subject: [PATCH 12/34] refactor(consensus): remove ValidatorAndDependencies from validator tests Dependencies now carries the mocked payload builder and message routing, so the wrapper struct and its setup helpers are replaced by direct DependenciesBuilder chains plus a make_validator helper. Co-Authored-By: Claude Fable 5 --- rs/consensus/src/consensus/validator.rs | 647 ++++++++++++------------ 1 file changed, 327 insertions(+), 320 deletions(-) diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index 730b2c7d34d8..512fe08b90b1 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2000,15 +2000,12 @@ 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, DependenciesBuilder, RefMockPayloadBuilder}; - use ic_crypto_test_utils_crypto_returning_ok::CryptoReturningOk; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_interfaces::{ messaging::XNetPayloadValidationFailure, p2p::consensus::MutablePool, time_source::TimeSource, }; - use ic_interfaces_mocks::messaging::RefMockMessageRouting; use ic_logger::replica_logger::no_op_logger; use ic_metrics::MetricsRegistry; use ic_protobuf::registry::subnet::v1::{ @@ -2023,7 +2020,6 @@ pub mod test { SetupInitialDkgContext, SignWithThresholdContext, }; use ic_test_artifact_pool::consensus_pool::TestConsensusPool; - use ic_test_utilities::state_manager::RefMockStateManager; use ic_test_utilities_consensus::{ assert_changeset_matches_pattern, dkg::fake_setup_initial_dkg_context, @@ -2035,7 +2031,6 @@ pub mod test { }, }; use ic_test_utilities_registry::{SubnetRecordBuilder, add_subnet_record}; - use ic_test_utilities_time::FastForwardTimeSource; use ic_test_utilities_types::{ ids::{node_test_id, subnet_test_id}, messages::SignedIngressBuilder, @@ -2053,7 +2048,6 @@ pub mod test { BasicSig, BasicSigOf, CombinedMultiSig, CombinedMultiSigOf, CombinedThresholdSig, CombinedThresholdSigOf, CryptoHash, }, - replica_config::ReplicaConfig, signature::ThresholdSignature, subnet_id_into_protobuf, }; @@ -2079,112 +2073,35 @@ 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, - } - - impl ValidatorAndDependencies { - fn new(dependencies: Dependencies) -> Self { - let payload_builder = Arc::new(RefMockPayloadBuilder::default()); - let message_routing = Arc::new(RefMockMessageRouting::default()); - let validator = Validator::new( - dependencies.replica_config.clone(), - dependencies.membership.clone(), - dependencies.registry.clone(), - dependencies.crypto.clone(), - payload_builder.clone(), - dependencies.state_manager.clone(), - message_routing.clone(), - dependencies.dkg_pool.clone(), - build_thread_pool(MAX_CONSENSUS_THREADS), - no_op_logger(), - &MetricsRegistry::new(), - Arc::clone(&dependencies.time_source) as Arc<_>, - ); - Self { - validator, - payload_builder, - membership: dependencies.membership, - state_manager: dependencies.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, - } - } - } - - 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( - DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .build(), - ) - } - - fn setup_dependencies_with_raw_state_manager( - pool_config: ic_config::artifact_pool::ArtifactPoolConfig, - node_ids: &[NodeId], - ) -> ValidatorAndDependencies { - ValidatorAndDependencies::new( - DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(node_ids) - .with_dkg_interval_length(9) - .build(), - )], - ) - .without_mocked_state_manager() - .build(), + fn make_validator(dependencies: &Dependencies) -> Validator { + Validator::new( + dependencies.replica_config.clone(), + dependencies.membership.clone(), + dependencies.registry.clone(), + dependencies.crypto.clone(), + dependencies.payload_builder.clone(), + dependencies.state_manager.clone(), + dependencies.message_routing.clone(), + dependencies.dkg_pool.clone(), + build_thread_pool(MAX_CONSENSUS_THREADS), + no_op_logger(), + &MetricsRegistry::new(), + dependencies.time_source.clone(), ) } #[test] fn test_validate_catch_up_package_shares() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { state_manager, mut pool, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = dependencies; // The state manager is mocked and the `StateHash` is completely arbitrary. It // must just be the same as in the `CatchUpPackageShare`. @@ -2312,15 +2229,16 @@ pub mod test { ) { let expected_oldest_registry_version = RegistryVersion::from(2); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .without_mocked_state_manager() + .build(); + let validator = make_validator(&dependencies); + let Dependencies { state_manager, mut pool, .. - } = setup_dependencies_with_raw_state_manager( - pool_config, - &(0..4).map(node_test_id).collect::>(), - ); + } = dependencies; // The state manager is mocked and the `StateHash` is completely arbitrary. It // must just be the same as in the `CatchUpPackageShare`. @@ -2449,11 +2367,11 @@ pub mod test { #[test] fn test_finalization_requires_notarization() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ValidatorAndDependencies { - validator, - mut pool, - .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, .. } = dependencies; let block = pool.make_next_block(); pool.insert_validated(block.clone()); // Insert a Finalization for `block` in the unvalidated pool @@ -2530,12 +2448,15 @@ pub mod test { #[test] fn test_random_beacon_validation() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, replica_config, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = dependencies; pool.advance_round_normal_operation(); // Put a random tape share in the unvalidated pool @@ -2585,14 +2506,17 @@ pub mod test { #[test] fn test_random_tape_validation() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { state_manager, message_routing, mut pool, replica_config, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = dependencies; let mut round = pool.prepare_round().dont_finalize().dont_add_random_tape(); round.advance(); @@ -2702,17 +2626,24 @@ pub mod test { 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&committee).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, time_source, replica_config, .. - } = setup_dependencies(pool_config, &committee); + } = dependencies; payload_builder .get_mut() .expect_validate_payload() @@ -2728,12 +2659,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); @@ -2769,7 +2700,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, @@ -2785,7 +2716,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(), @@ -2806,16 +2737,23 @@ pub mod test { 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&committee).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, replica_config, .. - } = setup_dependencies(pool_config, &committee); + } = dependencies; payload_builder .get_mut() .expect_validate_payload() @@ -2831,12 +2769,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); @@ -2877,29 +2815,28 @@ pub mod test { 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![( + 1, + SubnetRecordBuilder::from(&committee) + .with_halt_at_cup_height(true) + .build(), + )], + ) + .with_dkg_interval_length(dkg_interval_length) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, - registry_client, + registry, replica_config, mut pool, time_source, .. - } = ValidatorAndDependencies::new( - DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .with_halt_at_cup_height(true) - .build(), - )], - ) - .build(), - ); + } = dependencies; // Any payload validation fails, so we can observe whether validation was attempted. payload_builder @@ -2928,7 +2865,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(), @@ -2963,17 +2900,24 @@ pub mod test { 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&committee).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, time_source, replica_config, .. - } = setup_dependencies(pool_config, &committee); + } = dependencies; payload_builder .get_mut() .expect_validate_payload() @@ -2991,13 +2935,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)); @@ -3045,14 +2989,21 @@ pub mod test { 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&committee).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, mut pool, time_source, .. - } = setup_dependencies(pool_config, &committee); + } = dependencies; payload_builder .get_mut() .expect_validate_payload() @@ -3142,16 +3093,23 @@ pub mod test { 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, replica_config, .. - } = setup_dependencies(pool_config, &subnet_members); + } = dependencies; payload_builder .get_mut() .expect_validate_payload() @@ -3162,20 +3120,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); @@ -3223,14 +3181,14 @@ pub mod test { /// 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"); @@ -3238,14 +3196,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 @@ -3258,38 +3216,31 @@ pub mod test { fn test_data_block_during_subnet_splitting(#[case] block_registry_version: RegistryVersion) { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let committee = (0..4).map(node_test_id).collect::>(); - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + (1..=block_registry_version.get()) + .map(|version| (version, SubnetRecordBuilder::from(&committee).build())) + .collect(), + ) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, pool, time_source, replica_config, .. - } = ValidatorAndDependencies::new( - DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - (1..=block_registry_version.get()) - .map(|version| { - ( - version, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .build(), - ) - }) - .collect(), - ) - .build(), - ); + } = dependencies; 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; @@ -3334,33 +3285,26 @@ pub mod test { fn test_summary_block_during_subnet_splitting(#[case] block_registry_version: RegistryVersion) { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let committee = (0..4).map(node_test_id).collect::>(); - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + (1..=block_registry_version.get()) + .map(|version| (version, SubnetRecordBuilder::from(&committee).build())) + .collect(), + ) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, - data_provider, - registry_client, + registry_data_provider, + registry, mut pool, time_source, replica_config, .. - } = ValidatorAndDependencies::new( - DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - (1..=block_registry_version.get()) - .map(|version| { - ( - version, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .build(), - ) - }) - .collect(), - ) - .build(), - ); + } = dependencies; payload_builder .get_mut() .expect_validate_payload() @@ -3389,7 +3333,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 { @@ -3418,21 +3362,24 @@ pub mod test { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let committee = (0..4).map(node_test_id).collect::>(); - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&committee).build())], + ) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { 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, - ); + } = dependencies; payload_builder .get_mut() .expect_validate_payload() @@ -3440,14 +3387,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(); @@ -3474,14 +3421,21 @@ pub mod test { 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, mut pool, time_source, .. - } = setup_dependencies(pool_config, &subnet_members); + } = dependencies; payload_builder .get_mut() @@ -3535,14 +3489,21 @@ pub mod 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, mut pool, time_source, .. - } = setup_dependencies(pool_config, &subnet_members); + } = dependencies; payload_builder .get_mut() @@ -3604,11 +3565,11 @@ pub mod test { #[test] fn test_notarization_requires_at_least_threshold_signatures() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ValidatorAndDependencies { - validator, - mut pool, - .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, .. } = dependencies; let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3655,11 +3616,11 @@ pub mod test { fn test_notarization_deduped_by_content() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { // Setup validator dependencies. - let ValidatorAndDependencies { - validator, - mut pool, - .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, .. } = dependencies; let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3710,11 +3671,11 @@ pub mod test { fn test_finalization_deduped_by_content() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { // Setup validator dependencies. - let ValidatorAndDependencies { - validator, - mut pool, - .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, .. } = dependencies; let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3892,17 +3853,16 @@ pub mod test { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let cup_height = Height::from(60); // Setup validator dependencies. - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(cup_height.get() - 1) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { state_manager, mut pool, time_source, .. - } = setup_dependencies_with_dkg_interval_length( - pool_config, - &(0..4).map(node_test_id).collect::>(), - cup_height.get() - 1, - ); + } = dependencies; pool.advance_round_normal_operation_no_cup_n(finalized_height.get()); @@ -3971,12 +3931,15 @@ pub mod test { fn test_should_not_validate_catch_up_package_when_wrong_version() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { // Setup validator dependencies. - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { state_manager, mut pool, .. - } = setup_dependencies(pool_config, &(0..4).map(node_test_id).collect::>()); + } = dependencies; pool.advance_round_normal_operation_n(9); // Create, notarize, and finalize a block at the CUP height, but don't create a CUP. @@ -4012,16 +3975,23 @@ pub mod 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, - registry_client, + registry, mut pool, time_source, replica_config, .. - } = setup_dependencies(pool_config, &subnet_members); + } = dependencies; payload_builder .get_mut() @@ -4058,7 +4028,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(), @@ -4111,7 +4081,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(), @@ -4147,13 +4117,20 @@ pub mod 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { payload_builder, state_manager, mut pool, .. - } = setup_dependencies(pool_config, &subnet_members); + } = dependencies; pool.advance_round_normal_operation(); payload_builder @@ -4215,12 +4192,19 @@ pub mod test { pool_config: ArtifactPoolConfig, ) -> (TestConsensusPool, Validator, EquivocationProof) { let subnet_members = (0..4).map(node_test_id).collect::>(); - let ValidatorAndDependencies { - validator, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, replica_config, .. - } = setup_dependencies(pool_config, &subnet_members); + } = dependencies; pool.advance_round_normal_operation(); pool.insert_validated(pool.make_next_beacon()); @@ -4406,11 +4390,15 @@ pub mod 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); + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, .. } = dependencies; pool.advance_round_normal_operation_n(9); @@ -4448,14 +4436,21 @@ pub mod 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { state_manager, time_source, payload_builder, mut pool, .. - } = setup_dependencies(pool_config, &subnet_members); + } = dependencies; payload_builder .get_mut() @@ -4513,11 +4508,15 @@ pub mod 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); + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, .. } = dependencies; let block = pool.make_next_block(); let mut block_with_malicious_signer = block.clone(); @@ -4548,33 +4547,27 @@ pub mod test { 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![ + (1, SubnetRecordBuilder::from(&subnet_members).build()), + ( + 10, + SubnetRecordBuilder::from(&subnet_members) + .with_replica_version("new_version") + .build(), + ), + ], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, replica_config, .. - } = ValidatorAndDependencies::new( - DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![ - ( - 1, - SubnetRecordBuilder::from(&subnet_members) - .with_dkg_interval_length(9) - .build(), - ), - ( - 10, - SubnetRecordBuilder::from(&subnet_members) - .with_dkg_interval_length(9) - .with_replica_version("new_version") - .build(), - ), - ], - ) - .build(), - ); + } = dependencies; // Move to the end of the DKG interval where we switch versions pool.advance_round_normal_operation_n(dkg_interval + 1); @@ -4608,14 +4601,21 @@ pub mod 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, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], + ) + .with_dkg_interval_length(9) + .build(); + let validator = make_validator(&dependencies); + let Dependencies { mut pool, time_source, payload_builder, state_manager, .. - } = setup_dependencies(pool_config, &subnet_members); + } = dependencies; payload_builder .get_mut() @@ -4846,14 +4846,21 @@ pub mod test { fn equivocation_proofs_test(#[case] test_case: EquivocationProofTestCase) { ic_test_utilities_logger::with_test_replica_logger(|logger| { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ValidatorAndDependencies { - mut validator, + let dependencies = DependenciesBuilder::single_subnet( + pool_config, + subnet_test_id(0), + vec![(1, SubnetRecordBuilder::from(&[NODE_1, NODE_2]).build())], + ) + .with_dkg_interval_length(9) + .build(); + let mut validator = make_validator(&dependencies); + let Dependencies { state_manager, time_source, payload_builder, mut pool, .. - } = setup_dependencies(pool_config, &[NODE_1, NODE_2]); + } = dependencies; validator.log = logger; payload_builder From 5187fe1d643c09d1d68c92abdd76af5acea65582 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 09:57:08 +0000 Subject: [PATCH 13/34] refactor(consensus): use DependenciesBuilder::with_dkg_interval_length in ic-consensus tests Co-Authored-By: Claude Fable 5 --- rs/consensus/src/consensus.rs | 13 +- rs/consensus/src/consensus/block_maker.rs | 42 +---- rs/consensus/src/consensus/bounds.rs | 15 +- .../src/consensus/catchup_package_maker.rs | 66 ++----- rs/consensus/src/consensus/finalizer.rs | 3 +- rs/consensus/src/consensus/notary.rs | 69 ++------ rs/consensus/src/consensus/priority.rs | 19 +- .../src/consensus/share_aggregator.rs | 5 +- rs/consensus/src/consensus/status.rs | 9 +- rs/consensus/src/consensus/validator.rs | 165 +++++------------- 10 files changed, 100 insertions(+), 306 deletions(-) diff --git a/rs/consensus/src/consensus.rs b/rs/consensus/src/consensus.rs index 66b7781c7c6b..83cc40d705b1 100644 --- a/rs/consensus/src/consensus.rs +++ b/rs/consensus/src/consensus.rs @@ -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, .. - } = DependenciesBuilder::single_subnet(pool_config, subnet_id, vec![(1, record)]).build(); + } = 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 c2396f21dd3f..cfe6a33ff082 100644 --- a/rs/consensus/src/consensus/block_maker.rs +++ b/rs/consensus/src/consensus/block_maker.rs @@ -770,20 +770,11 @@ mod tests { pool_config, subnet_id, vec![ - ( - 1, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - ), - ( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - ), + (1, SubnetRecordBuilder::from(&node_ids).build()), + (10, SubnetRecordBuilder::from(&node_ids).build()), ], ) + .with_dkg_interval_length(dkg_interval_length) .build(); pool.advance_round_normal_operation_n(4); @@ -940,13 +931,9 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![( - 1, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], + vec![(1, SubnetRecordBuilder::from(&node_ids).build())], ) + .with_dkg_interval_length(dkg_interval_length) .build(); pool.advance_round_normal_operation_n(8); @@ -1096,22 +1083,17 @@ mod tests { pool_config.clone(), subnet_test_id(0), vec![ - ( - 1, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - ), + (1, SubnetRecordBuilder::from(&node_ids).build()), ( 10, SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) .with_replica_version(replica_version.as_ref()) .with_halt_at_cup_height(halt_at_cup_height) .build(), ), ], ) + .with_dkg_interval_length(dkg_interval_length) .build(); state_manager @@ -1605,16 +1587,10 @@ mod tests { pool_config, SOURCE_SUBNET_ID, (1..=MAX_REGISTRY_VERSION) - .map(|version| { - ( - version, - SubnetRecordBuilder::from(&[NODE_1]) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .build(), - ) - }) + .map(|version| (version, SubnetRecordBuilder::from(&[NODE_1]).build())) .collect(), ) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let mut payload_builder = MockPayloadBuilder::new(); diff --git a/rs/consensus/src/consensus/bounds.rs b/rs/consensus/src/consensus/bounds.rs index d0dd527ffc02..d7fbf8bdabad 100644 --- a/rs/consensus/src/consensus/bounds.rs +++ b/rs/consensus/src/consensus/bounds.rs @@ -187,8 +187,6 @@ pub fn validated_pool_within_bounds( mod tests { use super::*; use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; - use ic_test_utilities_registry::SubnetRecordBuilder; - use ic_test_utilities_types::ids::{node_test_id, subnet_test_id}; #[test] fn test_pool_bounds() { @@ -212,21 +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, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, record)], - ) - .build(); + } = 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 7dbb3ecd3831..574b5d85c13b 100644 --- a/rs/consensus/src/consensus/catchup_package_maker.rs +++ b/rs/consensus/src/consensus/catchup_package_maker.rs @@ -309,8 +309,8 @@ 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}, @@ -341,18 +341,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 = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .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 @@ -597,7 +588,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, @@ -605,18 +595,10 @@ mod tests { crypto, state_manager, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(interval_length) - .build(), - )], - ) - .without_mocked_state_manager() - .build(); + } = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(interval_length) + .without_mocked_state_manager() + .build(); let height = Height::from(0); state_manager @@ -694,7 +676,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,17 +683,9 @@ mod tests { crypto, state_manager, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(interval_length) - .build(), - )], - ) - .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(); @@ -754,7 +727,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, @@ -762,17 +734,9 @@ mod tests { crypto, state_manager, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(interval_length) - .build(), - )], - ) - .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 ccc0715d2aa2..ca92f7aa8713 100644 --- a/rs/consensus/src/consensus/finalizer.rs +++ b/rs/consensus/src/consensus/finalizer.rs @@ -361,19 +361,18 @@ mod tests { ( 1, SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) .with_replica_version("1") .build(), ), ( 10, SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) .with_replica_version("2") .build(), ), ], ) + .with_dkg_interval_length(dkg_interval_length) .build(); let metrics_registry = MetricsRegistry::new(); let message_routing = Arc::new(FakeMessageRouting::new()); diff --git a/rs/consensus/src/consensus/notary.rs b/rs/consensus/src/consensus/notary.rs index 700117392955..ff0ffab1d12e 100644 --- a/rs/consensus/src/consensus/notary.rs +++ b/rs/consensus/src/consensus/notary.rs @@ -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,17 +409,9 @@ mod tests { crypto, state_manager, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .build(); + } = DependenciesBuilder::new(pool_config, 1) + .with_dkg_interval_length(dkg_interval_length) + .build(); state_manager .get_mut() .expect_latest_certified_height() @@ -602,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, @@ -612,17 +602,9 @@ mod tests { crypto, state_manager, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .build(); + } = DependenciesBuilder::new(pool_config, 1) + .with_dkg_interval_length(dkg_interval_length) + .build(); state_manager .get_mut() .expect_latest_certified_height() @@ -679,23 +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, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, record)], - ) - .build(); + } = 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 @@ -809,22 +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, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, record)], - ) - .build(); + } = 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); @@ -933,21 +899,16 @@ mod tests { pool_config, subnet_test_id(0), vec![ - ( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval) - .build(), - ), + (1, SubnetRecordBuilder::from(&committee).build()), ( 10, SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval) .with_replica_version("new_version") .build(), ), ], ) + .with_dkg_interval_length(dkg_interval) .build(); let certified_height = Arc::new(RwLock::new(Height::from(0))); diff --git a/rs/consensus/src/consensus/priority.rs b/rs/consensus/src/consensus/priority.rs index e67b7f06b950..a16a6f6d1ce5 100644 --- a/rs/consensus/src/consensus/priority.rs +++ b/rs/consensus/src/consensus/priority.rs @@ -121,8 +121,8 @@ mod tests { use super::*; 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,18 +135,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, .. } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(committee.as_slice()) - .with_dkg_interval_length(dkg_interval) - .build(), - )], - ) - .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. diff --git a/rs/consensus/src/consensus/share_aggregator.rs b/rs/consensus/src/consensus/share_aggregator.rs index ad32ca8b9bd9..bc02dfd7a488 100644 --- a/rs/consensus/src/consensus/share_aggregator.rs +++ b/rs/consensus/src/consensus/share_aggregator.rs @@ -332,11 +332,10 @@ mod tests { subnet_test_id(0), vec![( INITIAL_REGISTRY_VERSION, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(interval_length) - .build(), + SubnetRecordBuilder::from(&node_ids).build(), )], ) + .with_dkg_interval_length(interval_length) .build(); let message_routing = Arc::new(FakeMessageRouting::new()); let aggregator = diff --git a/rs/consensus/src/consensus/status.rs b/rs/consensus/src/consensus/status.rs index 0f002b1d5878..3c9a11edc968 100644 --- a/rs/consensus/src/consensus/status.rs +++ b/rs/consensus/src/consensus/status.rs @@ -196,22 +196,17 @@ mod tests { pool_config, subnet_id, vec![ - ( - 1, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(DKG_LENGTH) - .build(), - ), + (1, SubnetRecordBuilder::from(&node_ids).build()), ( 10, SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(DKG_LENGTH) .with_replica_version(replica_version.as_ref()) .with_halt_at_cup_height(halt_at_cup_height) .build(), ), ], ) + .with_dkg_interval_length(DKG_LENGTH) .build(); pool.advance_round_normal_operation_n(CUP_HEIGHT.get()); diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index 512fe08b90b1..cf60610a213e 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2625,14 +2625,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&committee).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -2736,14 +2731,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&committee).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -2899,14 +2889,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&committee).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -2988,14 +2973,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&committee).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -3092,14 +3072,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -3361,14 +3336,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&committee).build())], - ) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -3420,14 +3390,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -3488,14 +3453,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -3974,14 +3934,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -4116,14 +4071,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { payload_builder, @@ -4191,14 +4141,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { mut pool, @@ -4389,14 +4334,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { mut pool, .. } = dependencies; @@ -4435,14 +4375,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { state_manager, @@ -4507,14 +4442,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { mut pool, .. } = dependencies; @@ -4600,14 +4530,9 @@ 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 dependencies = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&subnet_members).build())], - ) - .with_dkg_interval_length(9) - .build(); + let dependencies = DependenciesBuilder::new(pool_config, 7) + .with_dkg_interval_length(9) + .build(); let validator = make_validator(&dependencies); let Dependencies { mut pool, From 650fae1546e5fac192880afb6278501b839e2e14 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 09:57:08 +0000 Subject: [PATCH 14/34] refactor(consensus): use DependenciesBuilder::with_dkg_interval_length in ic-consensus-utils tests Co-Authored-By: Claude Fable 5 --- rs/consensus/utils/src/pool_reader.rs | 31 ++++++--------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/rs/consensus/utils/src/pool_reader.rs b/rs/consensus/utils/src/pool_reader.rs index 4fe17ce6028a..028d8ab06919 100644 --- a/rs/consensus/utils/src/pool_reader.rs +++ b/rs/consensus/utils/src/pool_reader.rs @@ -597,19 +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, .. } = DependenciesBuilder::single_subnet( - 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(), - )], - ) - .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); @@ -719,19 +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, .. } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 1, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(interval_length) - .build(), - )], - ) - .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(); From 4a3068bbe8fd326430ce4d4eaab9ef22323db07e Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 09:57:08 +0000 Subject: [PATCH 15/34] refactor(consensus): use DependenciesBuilder::with_dkg_interval_length in ic-consensus-dkg tests Co-Authored-By: Claude Fable 5 --- rs/consensus/dkg/src/dkg_key_manager.rs | 8 +--- rs/consensus/dkg/src/lib.rs | 56 +++++++---------------- rs/consensus/dkg/src/payload_builder.rs | 16 +++---- rs/consensus/dkg/src/payload_validator.rs | 13 ++---- rs/consensus/dkg/src/utils.rs | 8 +--- 5 files changed, 30 insertions(+), 71 deletions(-) diff --git a/rs/consensus/dkg/src/dkg_key_manager.rs b/rs/consensus/dkg/src/dkg_key_manager.rs index 16079391346e..0611d2fa2af4 100644 --- a/rs/consensus/dkg/src/dkg_key_manager.rs +++ b/rs/consensus/dkg/src/dkg_key_manager.rs @@ -573,13 +573,9 @@ mod tests { let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(222), - vec![( - 1, - SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) - .build(), - )], + vec![(1, SubnetRecordBuilder::from(&nodes).build())], ) + .with_dkg_interval_length(dkg_interval_len) .build(); let csp = Arc::new(CryptoReturningOk::default()); let mut key_manager = DkgKeyManager::new( diff --git a/rs/consensus/dkg/src/lib.rs b/rs/consensus/dkg/src/lib.rs index 0f00d893abd3..9a6515bb7afc 100644 --- a/rs/consensus/dkg/src/lib.rs +++ b/rs/consensus/dkg/src/lib.rs @@ -511,11 +511,11 @@ mod tests { vec![( initial_registry_version, SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) .with_chain_key_config(test_vet_key_config()) .build(), )], ) + .with_dkg_interval_length(dkg_interval_len) .build(); state_manager .get_mut() @@ -781,13 +781,9 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], + vec![(10, SubnetRecordBuilder::from(&node_ids).build())], ) + .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); @@ -946,13 +942,9 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], + vec![(10, SubnetRecordBuilder::from(&node_ids).build())], ) + .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); @@ -1518,25 +1510,17 @@ mod tests { let dependencies_1 = DependenciesBuilder::single_subnet( pool_config_1, subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], + vec![(10, SubnetRecordBuilder::from(&node_ids).build())], ) + .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); let dependencies_2 = DependenciesBuilder::single_subnet( pool_config_2, subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], + vec![(10, SubnetRecordBuilder::from(&node_ids).build())], ) + .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); @@ -1725,13 +1709,9 @@ mod tests { let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(REMOTE_DKG_INTERVAL) - .build(), - )], + vec![(10, SubnetRecordBuilder::from(&node_ids).build())], ) + .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .without_mocked_state_manager() .build(); @@ -2112,13 +2092,9 @@ mod tests { let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], + vec![(10, SubnetRecordBuilder::from(&node_ids).build())], ) + .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); @@ -2240,7 +2216,6 @@ mod tests { vec![( 10, SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .with_chain_key_config(ChainKeyConfig { key_configs: vec![KeyConfig { key_id: MasterPublicKeyId::VetKd(key_id.clone()), @@ -2254,6 +2229,7 @@ mod tests { .build(), )], ) + .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .without_mocked_state_manager() .build(); @@ -2351,7 +2327,6 @@ mod tests { vec![( 10, SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .with_chain_key_config(ChainKeyConfig { key_configs: vec![KeyConfig { key_id: MasterPublicKeyId::VetKd(key_id.clone()), @@ -2365,6 +2340,7 @@ mod tests { .build(), )], ) + .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .without_mocked_state_manager() .build(); @@ -2578,11 +2554,11 @@ mod tests { vec![( 5, SubnetRecordBuilder::from(&committee1) - .with_dkg_interval_length(dkg_interval_length) .with_chain_key_config(test_vet_key_config()) .build(), )], ) + .with_dkg_interval_length(dkg_interval_length) .build(); // Get the latest summary block, which is the genesis block diff --git a/rs/consensus/dkg/src/payload_builder.rs b/rs/consensus/dkg/src/payload_builder.rs index eec00cd7d7d2..d4b993f845e3 100644 --- a/rs/consensus/dkg/src/payload_builder.rs +++ b/rs/consensus/dkg/src/payload_builder.rs @@ -1089,11 +1089,11 @@ mod tests { vec![( 10, SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) .with_chain_key_config(test_vet_key_config()) .build(), )], ) + .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); let registry_version = deps.registry.get_latest_version(); @@ -1283,11 +1283,11 @@ mod tests { vec![( initial_registry_version, SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) .with_chain_key_config(test_vet_key_config()) .build(), )], ) + .with_dkg_interval_length(dkg_interval_len) .build(); let cup_contents = registry .get_cup_contents(subnet_id, registry.get_latest_version()) @@ -1375,11 +1375,11 @@ mod tests { vec![( initial_registry_version, SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) .with_chain_key_config(test_vet_key_config()) .build(), )], ) + .with_dkg_interval_length(dkg_interval_len) .build(); let cup_contents = registry @@ -1477,11 +1477,11 @@ mod tests { vec![( initial_registry_version, SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) .with_chain_key_config(test_vet_key_config()) .build(), )], ) + .with_dkg_interval_length(dkg_interval_len) .build(); let cup_contents = registry .get_cup_contents(subnet_id, registry.get_latest_version()) @@ -1617,13 +1617,9 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), - vec![( - 5, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], + vec![(5, SubnetRecordBuilder::from(&committee).build())], ) + .with_dkg_interval_length(dkg_interval_length) .build(); // Get the latest summary block, which is the genesis block diff --git a/rs/consensus/dkg/src/payload_validator.rs b/rs/consensus/dkg/src/payload_validator.rs index dd30bb059cd0..e08d705e5c58 100644 --- a/rs/consensus/dkg/src/payload_validator.rs +++ b/rs/consensus/dkg/src/payload_validator.rs @@ -309,13 +309,9 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), - vec![( - 5, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], + vec![(5, SubnetRecordBuilder::from(&committee).build())], ) + .with_dkg_interval_length(dkg_interval_length) .build(); let context = ValidationContext { @@ -736,11 +732,10 @@ mod tests { subnet_id, vec![( registry_version_start.get(), - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), + SubnetRecordBuilder::from(&committee).build(), )], ) + .with_dkg_interval_length(dkg_interval_length) .build(); state_manager .get_mut() diff --git a/rs/consensus/dkg/src/utils.rs b/rs/consensus/dkg/src/utils.rs index d3ce6e5f5f7b..a3d2c3c6e418 100644 --- a/rs/consensus/dkg/src/utils.rs +++ b/rs/consensus/dkg/src/utils.rs @@ -320,13 +320,9 @@ mod tests { let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![( - 1, - SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) - .build(), - )], + vec![(1, SubnetRecordBuilder::from(&nodes).build())], ) + .with_dkg_interval_length(dkg_interval_len) .build(); pool.advance_round_normal_operation_n(dkg_interval_len); From 4bfe2293b7bf5d7b14dd00372375346d9529884d Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 11:51:42 +0000 Subject: [PATCH 16/34] refactor(consensus): add make_default_validator_and_deps to validator tests Collapses the repeated build-deps / make_validator / destructure boilerplate at the 21 test sites that use the default 4-node subnet with DKG interval length 9. Non-default sites keep building their own dependencies and calling make_validator. Co-Authored-By: Claude Fable 5 --- rs/consensus/src/consensus/validator.rs | 381 +++++++++++------------- 1 file changed, 180 insertions(+), 201 deletions(-) diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index cf60610a213e..198617225285 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2073,6 +2073,9 @@ pub mod test { }; } + /// The DKG interval length used by most tests in this module. + const DKG_INTERVAL_LENGTH: u64 = 9; + fn make_validator(dependencies: &Dependencies) -> Validator { Validator::new( dependencies.replica_config.clone(), @@ -2090,18 +2093,29 @@ pub mod test { ) } + /// Creates a validator and the default test dependencies: a subnet with + /// 4 nodes and [`DKG_INTERVAL_LENGTH`]. + fn make_default_validator_and_deps( + pool_config: ArtifactPoolConfig, + ) -> (Validator, Dependencies) { + let dependencies = DependenciesBuilder::new(pool_config, 4) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(); + let validator = make_validator(&dependencies); + (validator, dependencies) + } + #[test] fn test_validate_catch_up_package_shares() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - state_manager, - mut pool, - .. - } = dependencies; + let ( + validator, + Dependencies { + state_manager, + mut pool, + .. + }, + ) = make_default_validator_and_deps(pool_config); // The state manager is mocked and the `StateHash` is completely arbitrary. It // must just be the same as in the `CatchUpPackageShare`. @@ -2367,11 +2381,8 @@ pub mod test { #[test] fn test_finalization_requires_notarization() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { mut pool, .. } = dependencies; + let (validator, Dependencies { mut pool, .. }) = + make_default_validator_and_deps(pool_config); let block = pool.make_next_block(); pool.insert_validated(block.clone()); // Insert a Finalization for `block` in the unvalidated pool @@ -2448,15 +2459,14 @@ pub mod test { #[test] fn test_random_beacon_validation() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - mut pool, - replica_config, - .. - } = dependencies; + let ( + validator, + Dependencies { + mut pool, + replica_config, + .. + }, + ) = make_default_validator_and_deps(pool_config); pool.advance_round_normal_operation(); // Put a random tape share in the unvalidated pool @@ -2506,17 +2516,16 @@ pub mod test { #[test] fn test_random_tape_validation() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - state_manager, - message_routing, - mut pool, - replica_config, - .. - } = dependencies; + let ( + validator, + Dependencies { + state_manager, + message_routing, + mut pool, + replica_config, + .. + }, + ) = make_default_validator_and_deps(pool_config); let mut round = pool.prepare_round().dont_finalize().dont_add_random_tape(); round.advance(); @@ -2625,20 +2634,19 @@ 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 dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - mut pool, - time_source, - replica_config, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + registry_data_provider, + registry, + mut pool, + time_source, + replica_config, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() .expect_validate_payload() @@ -2731,19 +2739,18 @@ 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 dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - mut pool, - replica_config, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + registry_data_provider, + registry, + mut pool, + replica_config, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() .expect_validate_payload() @@ -2889,20 +2896,19 @@ 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 dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - mut pool, - time_source, - replica_config, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + registry_data_provider, + registry, + mut pool, + time_source, + replica_config, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() .expect_validate_payload() @@ -2973,17 +2979,16 @@ 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 dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - mut pool, - time_source, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + mut pool, + time_source, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() .expect_validate_payload() @@ -3072,19 +3077,18 @@ 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 dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - mut pool, - replica_config, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + registry_data_provider, + registry, + mut pool, + replica_config, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() .expect_validate_payload() @@ -3147,9 +3151,6 @@ 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); @@ -3390,17 +3391,16 @@ 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 dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - mut pool, - time_source, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + mut pool, + time_source, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() @@ -3453,17 +3453,16 @@ pub mod test { #[test] fn test_block_context_time() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - mut pool, - time_source, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + mut pool, + time_source, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() @@ -3525,11 +3524,8 @@ pub mod test { #[test] fn test_notarization_requires_at_least_threshold_signatures() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { mut pool, .. } = dependencies; + let (validator, Dependencies { mut pool, .. }) = + make_default_validator_and_deps(pool_config); let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3576,11 +3572,8 @@ pub mod test { fn test_notarization_deduped_by_content() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { // Setup validator dependencies. - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { mut pool, .. } = dependencies; + let (validator, Dependencies { mut pool, .. }) = + make_default_validator_and_deps(pool_config); let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3631,11 +3624,8 @@ pub mod test { fn test_finalization_deduped_by_content() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { // Setup validator dependencies. - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { mut pool, .. } = dependencies; + let (validator, Dependencies { mut pool, .. }) = + make_default_validator_and_deps(pool_config); let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3891,15 +3881,14 @@ pub mod test { fn test_should_not_validate_catch_up_package_when_wrong_version() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { // Setup validator dependencies. - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - state_manager, - mut pool, - .. - } = dependencies; + let ( + validator, + Dependencies { + state_manager, + mut pool, + .. + }, + ) = make_default_validator_and_deps(pool_config); pool.advance_round_normal_operation_n(9); // Create, notarize, and finalize a block at the CUP height, but don't create a CUP. @@ -3934,19 +3923,18 @@ pub mod test { #[test] fn test_out_of_sync_validation() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - registry, - mut pool, - time_source, - replica_config, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + registry, + mut pool, + time_source, + replica_config, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() @@ -4071,16 +4059,15 @@ pub mod test { #[test] fn test_block_validated_through_notarization() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - mut pool, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + mut pool, + .. + }, + ) = make_default_validator_and_deps(pool_config); pool.advance_round_normal_operation(); payload_builder @@ -4141,15 +4128,14 @@ pub mod test { fn setup_equivocation_proof_test( pool_config: ArtifactPoolConfig, ) -> (TestConsensusPool, Validator, EquivocationProof) { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - mut pool, - replica_config, - .. - } = dependencies; + let ( + validator, + Dependencies { + mut pool, + replica_config, + .. + }, + ) = make_default_validator_and_deps(pool_config); pool.advance_round_normal_operation(); pool.insert_validated(pool.make_next_beacon()); @@ -4334,11 +4320,8 @@ 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 dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { mut pool, .. } = dependencies; + let (validator, Dependencies { mut pool, .. }) = + make_default_validator_and_deps(pool_config); pool.advance_round_normal_operation_n(9); @@ -4375,17 +4358,16 @@ pub mod test { #[test] fn test_create_equivocation_proof() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - state_manager, - time_source, - payload_builder, - mut pool, - .. - } = dependencies; + let ( + validator, + Dependencies { + state_manager, + time_source, + payload_builder, + mut pool, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() @@ -4442,11 +4424,8 @@ pub mod test { #[test] fn test_cannot_disqualify_with_incorrect_rank() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { mut pool, .. } = dependencies; + let (validator, Dependencies { mut pool, .. }) = + make_default_validator_and_deps(pool_config); let block = pool.make_next_block(); let mut block_with_malicious_signer = block.clone(); From fbaf433a8e286687cb3209f4afb54ff8f274f3a0 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 12:22:03 +0000 Subject: [PATCH 17/34] fix: A few omissions --- rs/consensus/src/consensus/validator.rs | 43 ++++++++++++------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index 198617225285..12d65b14bc3d 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2244,7 +2244,7 @@ pub mod test { let expected_oldest_registry_version = RegistryVersion::from(2); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(9) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .without_mocked_state_manager() .build(); let validator = make_validator(&dependencies); @@ -2810,7 +2810,6 @@ 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 dependencies = DependenciesBuilder::single_subnet( pool_config, @@ -2822,7 +2821,7 @@ pub mod test { .build(), )], ) - .with_dkg_interval_length(dkg_interval_length) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let validator = make_validator(&dependencies); let Dependencies { @@ -2850,7 +2849,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(), @@ -3337,20 +3336,19 @@ pub mod test { const UNREADABLE_REGISTRY_VERSION: RegistryVersion = RegistryVersion::new(2); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - pool, - time_source, - replica_config, - .. - } = dependencies; + let ( + validator, + Dependencies { + payload_builder, + state_manager, + registry_data_provider, + registry, + pool, + time_source, + replica_config, + .. + }, + ) = make_default_validator_and_deps(pool_config); payload_builder .get_mut() .expect_validate_payload() @@ -4455,7 +4453,6 @@ 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 dependencies = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), @@ -4469,7 +4466,7 @@ pub mod test { ), ], ) - .with_dkg_interval_length(9) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let validator = make_validator(&dependencies); let Dependencies { @@ -4479,7 +4476,7 @@ pub mod test { } = dependencies; // 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 @@ -4510,7 +4507,7 @@ pub mod test { fn test_ignore_disqualified_ranks() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let dependencies = DependenciesBuilder::new(pool_config, 7) - .with_dkg_interval_length(9) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let validator = make_validator(&dependencies); let Dependencies { @@ -4755,7 +4752,7 @@ pub mod test { subnet_test_id(0), vec![(1, SubnetRecordBuilder::from(&[NODE_1, NODE_2]).build())], ) - .with_dkg_interval_length(9) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let mut validator = make_validator(&dependencies); let Dependencies { From 4e7c681768935c178d53aceabd1694e7ee337eca Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 12:25:21 +0000 Subject: [PATCH 18/34] refactor(consensus): keep with_dkg_interval_length on explicitly built subnet records in ic-consensus tests Where tests pass custom SubnetRecords to DependenciesBuilder (non-default subnet id, registry version, or membership), setting the DKG interval length via the DependenciesBuilder would silently modify the records built just above. Keep it on the SubnetRecordBuilder there; only DependenciesBuilder::new sites use the builder-level setter. Co-Authored-By: Claude Fable 5 --- rs/consensus/src/consensus/block_maker.rs | 42 +++++++++++++++---- rs/consensus/src/consensus/finalizer.rs | 3 +- rs/consensus/src/consensus/notary.rs | 9 +++- .../src/consensus/share_aggregator.rs | 5 ++- rs/consensus/src/consensus/status.rs | 9 +++- rs/consensus/src/consensus/validator.rs | 39 +++++++++++++---- 6 files changed, 82 insertions(+), 25 deletions(-) diff --git a/rs/consensus/src/consensus/block_maker.rs b/rs/consensus/src/consensus/block_maker.rs index cfe6a33ff082..c2396f21dd3f 100644 --- a/rs/consensus/src/consensus/block_maker.rs +++ b/rs/consensus/src/consensus/block_maker.rs @@ -770,11 +770,20 @@ mod tests { pool_config, subnet_id, vec![ - (1, SubnetRecordBuilder::from(&node_ids).build()), - (10, SubnetRecordBuilder::from(&node_ids).build()), + ( + 1, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + ), + ( + 10, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + ), ], ) - .with_dkg_interval_length(dkg_interval_length) .build(); pool.advance_round_normal_operation_n(4); @@ -931,9 +940,13 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![(1, SubnetRecordBuilder::from(&node_ids).build())], + vec![( + 1, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_length) .build(); pool.advance_round_normal_operation_n(8); @@ -1083,17 +1096,22 @@ mod tests { pool_config.clone(), subnet_test_id(0), vec![ - (1, SubnetRecordBuilder::from(&node_ids).build()), + ( + 1, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + ), ( 10, SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) .with_replica_version(replica_version.as_ref()) .with_halt_at_cup_height(halt_at_cup_height) .build(), ), ], ) - .with_dkg_interval_length(dkg_interval_length) .build(); state_manager @@ -1587,10 +1605,16 @@ mod tests { pool_config, SOURCE_SUBNET_ID, (1..=MAX_REGISTRY_VERSION) - .map(|version| (version, SubnetRecordBuilder::from(&[NODE_1]).build())) + .map(|version| { + ( + version, + SubnetRecordBuilder::from(&[NODE_1]) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(), + ) + }) .collect(), ) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let mut payload_builder = MockPayloadBuilder::new(); diff --git a/rs/consensus/src/consensus/finalizer.rs b/rs/consensus/src/consensus/finalizer.rs index ca92f7aa8713..ccc0715d2aa2 100644 --- a/rs/consensus/src/consensus/finalizer.rs +++ b/rs/consensus/src/consensus/finalizer.rs @@ -361,18 +361,19 @@ mod tests { ( 1, SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(dkg_interval_length) .with_replica_version("1") .build(), ), ( 10, SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(dkg_interval_length) .with_replica_version("2") .build(), ), ], ) - .with_dkg_interval_length(dkg_interval_length) .build(); let metrics_registry = MetricsRegistry::new(); let message_routing = Arc::new(FakeMessageRouting::new()); diff --git a/rs/consensus/src/consensus/notary.rs b/rs/consensus/src/consensus/notary.rs index ff0ffab1d12e..8f3374c6c12b 100644 --- a/rs/consensus/src/consensus/notary.rs +++ b/rs/consensus/src/consensus/notary.rs @@ -899,16 +899,21 @@ mod tests { pool_config, subnet_test_id(0), vec![ - (1, SubnetRecordBuilder::from(&committee).build()), + ( + 1, + SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(dkg_interval) + .build(), + ), ( 10, SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(dkg_interval) .with_replica_version("new_version") .build(), ), ], ) - .with_dkg_interval_length(dkg_interval) .build(); let certified_height = Arc::new(RwLock::new(Height::from(0))); diff --git a/rs/consensus/src/consensus/share_aggregator.rs b/rs/consensus/src/consensus/share_aggregator.rs index bc02dfd7a488..ad32ca8b9bd9 100644 --- a/rs/consensus/src/consensus/share_aggregator.rs +++ b/rs/consensus/src/consensus/share_aggregator.rs @@ -332,10 +332,11 @@ mod tests { subnet_test_id(0), vec![( INITIAL_REGISTRY_VERSION, - SubnetRecordBuilder::from(&node_ids).build(), + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(interval_length) + .build(), )], ) - .with_dkg_interval_length(interval_length) .build(); let message_routing = Arc::new(FakeMessageRouting::new()); let aggregator = diff --git a/rs/consensus/src/consensus/status.rs b/rs/consensus/src/consensus/status.rs index 3c9a11edc968..0f002b1d5878 100644 --- a/rs/consensus/src/consensus/status.rs +++ b/rs/consensus/src/consensus/status.rs @@ -196,17 +196,22 @@ mod tests { pool_config, subnet_id, vec![ - (1, SubnetRecordBuilder::from(&node_ids).build()), + ( + 1, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(DKG_LENGTH) + .build(), + ), ( 10, SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(DKG_LENGTH) .with_replica_version(replica_version.as_ref()) .with_halt_at_cup_height(halt_at_cup_height) .build(), ), ], ) - .with_dkg_interval_length(DKG_LENGTH) .build(); pool.advance_round_normal_operation_n(CUP_HEIGHT.get()); diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index 12d65b14bc3d..557f7f2a72b1 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2817,11 +2817,11 @@ pub mod test { vec![( 1, SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .with_halt_at_cup_height(true) .build(), )], ) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let validator = make_validator(&dependencies); let Dependencies { @@ -3195,10 +3195,16 @@ pub mod test { pool_config, subnet_test_id(0), (1..=block_registry_version.get()) - .map(|version| (version, SubnetRecordBuilder::from(&committee).build())) + .map(|version| { + ( + version, + SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(), + ) + }) .collect(), ) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let validator = make_validator(&dependencies); let Dependencies { @@ -3264,10 +3270,16 @@ pub mod test { pool_config, subnet_test_id(0), (1..=block_registry_version.get()) - .map(|version| (version, SubnetRecordBuilder::from(&committee).build())) + .map(|version| { + ( + version, + SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(), + ) + }) .collect(), ) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let validator = make_validator(&dependencies); let Dependencies { @@ -4457,16 +4469,21 @@ pub mod test { pool_config, subnet_test_id(0), vec![ - (1, SubnetRecordBuilder::from(&subnet_members).build()), + ( + 1, + SubnetRecordBuilder::from(&subnet_members) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(), + ), ( 10, SubnetRecordBuilder::from(&subnet_members) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .with_replica_version("new_version") .build(), ), ], ) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let validator = make_validator(&dependencies); let Dependencies { @@ -4750,9 +4767,13 @@ pub mod test { let dependencies = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), - vec![(1, SubnetRecordBuilder::from(&[NODE_1, NODE_2]).build())], + vec![( + 1, + SubnetRecordBuilder::from(&[NODE_1, NODE_2]) + .with_dkg_interval_length(DKG_INTERVAL_LENGTH) + .build(), + )], ) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) .build(); let mut validator = make_validator(&dependencies); let Dependencies { From 8fa9c5545dd8d76d20018b1ecdd841e478cf54ae Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 12:25:21 +0000 Subject: [PATCH 19/34] refactor(consensus): keep with_dkg_interval_length on explicitly built subnet records in ic-consensus-dkg tests Co-Authored-By: Claude Fable 5 --- rs/consensus/dkg/src/dkg_key_manager.rs | 8 +++- rs/consensus/dkg/src/lib.rs | 56 ++++++++++++++++------- rs/consensus/dkg/src/payload_builder.rs | 16 ++++--- rs/consensus/dkg/src/payload_validator.rs | 13 ++++-- rs/consensus/dkg/src/utils.rs | 8 +++- 5 files changed, 71 insertions(+), 30 deletions(-) diff --git a/rs/consensus/dkg/src/dkg_key_manager.rs b/rs/consensus/dkg/src/dkg_key_manager.rs index 0611d2fa2af4..16079391346e 100644 --- a/rs/consensus/dkg/src/dkg_key_manager.rs +++ b/rs/consensus/dkg/src/dkg_key_manager.rs @@ -573,9 +573,13 @@ mod tests { let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(222), - vec![(1, SubnetRecordBuilder::from(&nodes).build())], + vec![( + 1, + SubnetRecordBuilder::from(&nodes) + .with_dkg_interval_length(dkg_interval_len) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_len) .build(); let csp = Arc::new(CryptoReturningOk::default()); let mut key_manager = DkgKeyManager::new( diff --git a/rs/consensus/dkg/src/lib.rs b/rs/consensus/dkg/src/lib.rs index 9a6515bb7afc..0f00d893abd3 100644 --- a/rs/consensus/dkg/src/lib.rs +++ b/rs/consensus/dkg/src/lib.rs @@ -511,11 +511,11 @@ mod tests { vec![( initial_registry_version, SubnetRecordBuilder::from(&nodes) + .with_dkg_interval_length(dkg_interval_len) .with_chain_key_config(test_vet_key_config()) .build(), )], ) - .with_dkg_interval_length(dkg_interval_len) .build(); state_manager .get_mut() @@ -781,9 +781,13 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![(10, SubnetRecordBuilder::from(&node_ids).build())], + vec![( + 10, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); @@ -942,9 +946,13 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![(10, SubnetRecordBuilder::from(&node_ids).build())], + vec![( + 10, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); @@ -1510,17 +1518,25 @@ mod tests { let dependencies_1 = DependenciesBuilder::single_subnet( pool_config_1, subnet_id, - vec![(10, SubnetRecordBuilder::from(&node_ids).build())], + vec![( + 10, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); let dependencies_2 = DependenciesBuilder::single_subnet( pool_config_2, subnet_id, - vec![(10, SubnetRecordBuilder::from(&node_ids).build())], + vec![( + 10, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); @@ -1709,9 +1725,13 @@ mod tests { let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), - vec![(10, SubnetRecordBuilder::from(&node_ids).build())], + vec![( + 10, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(REMOTE_DKG_INTERVAL) + .build(), + )], ) - .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .without_mocked_state_manager() .build(); @@ -2092,9 +2112,13 @@ mod tests { let mut deps = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![(10, SubnetRecordBuilder::from(&node_ids).build())], + vec![( + 10, + SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); @@ -2216,6 +2240,7 @@ mod tests { vec![( 10, SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .with_chain_key_config(ChainKeyConfig { key_configs: vec![KeyConfig { key_id: MasterPublicKeyId::VetKd(key_id.clone()), @@ -2229,7 +2254,6 @@ mod tests { .build(), )], ) - .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .without_mocked_state_manager() .build(); @@ -2327,6 +2351,7 @@ mod tests { vec![( 10, SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .with_chain_key_config(ChainKeyConfig { key_configs: vec![KeyConfig { key_id: MasterPublicKeyId::VetKd(key_id.clone()), @@ -2340,7 +2365,6 @@ mod tests { .build(), )], ) - .with_dkg_interval_length(REMOTE_DKG_INTERVAL) .without_mocked_state_manager() .build(); @@ -2554,11 +2578,11 @@ mod tests { vec![( 5, SubnetRecordBuilder::from(&committee1) + .with_dkg_interval_length(dkg_interval_length) .with_chain_key_config(test_vet_key_config()) .build(), )], ) - .with_dkg_interval_length(dkg_interval_length) .build(); // Get the latest summary block, which is the genesis block diff --git a/rs/consensus/dkg/src/payload_builder.rs b/rs/consensus/dkg/src/payload_builder.rs index d4b993f845e3..eec00cd7d7d2 100644 --- a/rs/consensus/dkg/src/payload_builder.rs +++ b/rs/consensus/dkg/src/payload_builder.rs @@ -1089,11 +1089,11 @@ mod tests { vec![( 10, SubnetRecordBuilder::from(&node_ids) + .with_dkg_interval_length(dkg_interval_length) .with_chain_key_config(test_vet_key_config()) .build(), )], ) - .with_dkg_interval_length(dkg_interval_length) .without_mocked_state_manager() .build(); let registry_version = deps.registry.get_latest_version(); @@ -1283,11 +1283,11 @@ mod tests { vec![( initial_registry_version, SubnetRecordBuilder::from(&nodes) + .with_dkg_interval_length(dkg_interval_len) .with_chain_key_config(test_vet_key_config()) .build(), )], ) - .with_dkg_interval_length(dkg_interval_len) .build(); let cup_contents = registry .get_cup_contents(subnet_id, registry.get_latest_version()) @@ -1375,11 +1375,11 @@ mod tests { vec![( initial_registry_version, SubnetRecordBuilder::from(&nodes) + .with_dkg_interval_length(dkg_interval_len) .with_chain_key_config(test_vet_key_config()) .build(), )], ) - .with_dkg_interval_length(dkg_interval_len) .build(); let cup_contents = registry @@ -1477,11 +1477,11 @@ mod tests { vec![( initial_registry_version, SubnetRecordBuilder::from(&nodes) + .with_dkg_interval_length(dkg_interval_len) .with_chain_key_config(test_vet_key_config()) .build(), )], ) - .with_dkg_interval_length(dkg_interval_len) .build(); let cup_contents = registry .get_cup_contents(subnet_id, registry.get_latest_version()) @@ -1617,9 +1617,13 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), - vec![(5, SubnetRecordBuilder::from(&committee).build())], + vec![( + 5, + SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(dkg_interval_length) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_length) .build(); // Get the latest summary block, which is the genesis block diff --git a/rs/consensus/dkg/src/payload_validator.rs b/rs/consensus/dkg/src/payload_validator.rs index e08d705e5c58..dd30bb059cd0 100644 --- a/rs/consensus/dkg/src/payload_validator.rs +++ b/rs/consensus/dkg/src/payload_validator.rs @@ -309,9 +309,13 @@ mod tests { } = DependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), - vec![(5, SubnetRecordBuilder::from(&committee).build())], + vec![( + 5, + SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(dkg_interval_length) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_length) .build(); let context = ValidationContext { @@ -732,10 +736,11 @@ mod tests { subnet_id, vec![( registry_version_start.get(), - SubnetRecordBuilder::from(&committee).build(), + SubnetRecordBuilder::from(&committee) + .with_dkg_interval_length(dkg_interval_length) + .build(), )], ) - .with_dkg_interval_length(dkg_interval_length) .build(); state_manager .get_mut() diff --git a/rs/consensus/dkg/src/utils.rs b/rs/consensus/dkg/src/utils.rs index a3d2c3c6e418..d3ce6e5f5f7b 100644 --- a/rs/consensus/dkg/src/utils.rs +++ b/rs/consensus/dkg/src/utils.rs @@ -320,9 +320,13 @@ mod tests { let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( pool_config, subnet_id, - vec![(1, SubnetRecordBuilder::from(&nodes).build())], + vec![( + 1, + SubnetRecordBuilder::from(&nodes) + .with_dkg_interval_length(dkg_interval_len) + .build(), + )], ) - .with_dkg_interval_length(dkg_interval_len) .build(); pool.advance_round_normal_operation_n(dkg_interval_len); From 087764099fc0ffd332c9036294a3718f701e6fde Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 13:45:29 +0000 Subject: [PATCH 20/34] fix: address magic numbers --- rs/consensus/src/consensus/validator.rs | 28 ++++++++++++------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index 557f7f2a72b1..0bbdc386e18f 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2145,7 +2145,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); @@ -2295,7 +2295,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(); @@ -3004,18 +3004,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 { @@ -3039,13 +3039,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, @@ -3054,12 +3054,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); @@ -3900,7 +3900,7 @@ pub mod test { }, ) = make_default_validator_and_deps(pool_config); - 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(); @@ -4333,7 +4333,7 @@ pub mod test { let (validator, Dependencies { mut pool, .. }) = make_default_validator_and_deps(pool_config); - pool.advance_round_normal_operation_n(9); + pool.advance_round_normal_operation_n(DKG_INTERVAL_LENGTH); let mut block = pool.make_next_block(); From 5ac75fa700700e2963bb30c6228d0ff938b2bf61 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 14:18:38 +0000 Subject: [PATCH 21/34] refactor: revert removing ValidatorAndDependencies. Introduce its Builder that forwards to DependenciesBuilder --- rs/consensus/src/consensus/validator.rs | 562 +++++++++++++----------- 1 file changed, 303 insertions(+), 259 deletions(-) diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index 0bbdc386e18f..ff41277d5f1e 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2001,15 +2001,16 @@ pub mod test { }; use assert_matches::assert_matches; use ic_config::artifact_pool::ArtifactPoolConfig; - use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; + use ic_consensus_mocks::{Dependencies, DependenciesBuilder, RefMockPayloadBuilder}; use ic_interfaces::{ messaging::XNetPayloadValidationFailure, p2p::consensus::MutablePool, time_source::TimeSource, }; + use ic_interfaces_mocks::messaging::RefMockMessageRouting; 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; @@ -2020,6 +2021,7 @@ pub mod test { SetupInitialDkgContext, SignWithThresholdContext, }; use ic_test_artifact_pool::consensus_pool::TestConsensusPool; + use ic_test_utilities::state_manager::RefMockStateManager; use ic_test_utilities_consensus::{ assert_changeset_matches_pattern, dkg::fake_setup_initial_dkg_context, @@ -2031,6 +2033,7 @@ pub mod test { }, }; use ic_test_utilities_registry::{SubnetRecordBuilder, add_subnet_record}; + use ic_test_utilities_time::FastForwardTimeSource; use ic_test_utilities_types::{ ids::{node_test_id, subnet_test_id}, messages::SignedIngressBuilder, @@ -2048,6 +2051,7 @@ pub mod test { BasicSig, BasicSigOf, CombinedMultiSig, CombinedMultiSigOf, CombinedThresholdSig, CombinedThresholdSigOf, CryptoHash, }, + replica_config::ReplicaConfig, signature::ThresholdSignature, subnet_id_into_protobuf, }; @@ -2076,46 +2080,108 @@ pub mod test { /// The DKG interval length used by most tests in this module. const DKG_INTERVAL_LENGTH: u64 = 9; - fn make_validator(dependencies: &Dependencies) -> Validator { - Validator::new( - dependencies.replica_config.clone(), - dependencies.membership.clone(), - dependencies.registry.clone(), - dependencies.crypto.clone(), - dependencies.payload_builder.clone(), - dependencies.state_manager.clone(), - dependencies.message_routing.clone(), - dependencies.dkg_pool.clone(), - build_thread_pool(MAX_CONSENSUS_THREADS), - no_op_logger(), - &MetricsRegistry::new(), - dependencies.time_source.clone(), - ) + 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, } - /// Creates a validator and the default test dependencies: a subnet with - /// 4 nodes and [`DKG_INTERVAL_LENGTH`]. - fn make_default_validator_and_deps( - pool_config: ArtifactPoolConfig, - ) -> (Validator, Dependencies) { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .build(); - let validator = make_validator(&dependencies); - (validator, dependencies) + struct ValidatorAndDependenciesBuilder { + deps_builder: DependenciesBuilder, + } + + 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_mocked_state_manager(mut self) -> Self { + self.deps_builder = self.deps_builder.without_mocked_state_manager(); + 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( + replica_config.clone(), + membership, + registry.clone(), + crypto, + payload_builder.clone(), + state_manager.clone(), + message_routing.clone(), + dkg_pool, + build_thread_pool(MAX_CONSENSUS_THREADS), + no_op_logger(), + &MetricsRegistry::new(), + time_source.clone(), + ); + + ValidatorAndDependencies { + validator, + payload_builder, + state_manager, + message_routing, + registry_data_provider, + registry, + pool, + time_source, + replica_config, + } + } } #[test] fn test_validate_catch_up_package_shares() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ( + let ValidatorAndDependencies { validator, - Dependencies { - state_manager, - mut pool, - .. - }, - ) = make_default_validator_and_deps(pool_config); + state_manager, + mut pool, + .. + } = 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`. @@ -2243,16 +2309,14 @@ pub mod test { ) { let expected_oldest_registry_version = RegistryVersion::from(2); ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .without_mocked_state_manager() - .build(); - let validator = make_validator(&dependencies); - let Dependencies { + let ValidatorAndDependencies { + validator, state_manager, mut pool, .. - } = dependencies; + } = ValidatorAndDependenciesBuilder::new(pool_config, 4) + .without_mocked_state_manager() + .build(); // The state manager is mocked and the `StateHash` is completely arbitrary. It // must just be the same as in the `CatchUpPackageShare`. @@ -2381,8 +2445,11 @@ pub mod test { #[test] fn test_finalization_requires_notarization() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let (validator, Dependencies { mut pool, .. }) = - make_default_validator_and_deps(pool_config); + let ValidatorAndDependencies { + validator, + mut pool, + .. + } = 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 @@ -2459,14 +2526,13 @@ pub mod test { #[test] fn test_random_beacon_validation() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ( + let ValidatorAndDependencies { validator, - Dependencies { - mut pool, - replica_config, - .. - }, - ) = make_default_validator_and_deps(pool_config); + mut pool, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); + pool.advance_round_normal_operation(); // Put a random tape share in the unvalidated pool @@ -2516,16 +2582,14 @@ pub mod test { #[test] fn test_random_tape_validation() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ( + let ValidatorAndDependencies { validator, - Dependencies { - state_manager, - message_routing, - mut pool, - replica_config, - .. - }, - ) = make_default_validator_and_deps(pool_config); + state_manager, + message_routing, + mut pool, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let mut round = pool.prepare_round().dont_finalize().dont_add_random_tape(); round.advance(); @@ -2634,19 +2698,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 ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - mut pool, - time_source, - replica_config, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + registry_data_provider, + registry, + mut pool, + time_source, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -2739,18 +2801,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 ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - mut pool, - replica_config, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + registry_data_provider, + registry, + mut pool, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -2811,7 +2871,16 @@ pub mod test { fn test_summary_block_is_validated_while_halted() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let committee: Vec<_> = (0..4).map(node_test_id).collect(); - let dependencies = DependenciesBuilder::single_subnet( + let ValidatorAndDependencies { + validator, + payload_builder, + state_manager, + registry, + replica_config, + mut pool, + time_source, + .. + } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -2823,16 +2892,6 @@ pub mod test { )], ) .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - registry, - replica_config, - mut pool, - time_source, - .. - } = dependencies; // Any payload validation fails, so we can observe whether validation was attempted. payload_builder @@ -2895,19 +2954,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 ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - mut pool, - time_source, - replica_config, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + registry_data_provider, + registry, + mut pool, + time_source, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -2978,16 +3035,14 @@ 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 ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - mut pool, - time_source, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + mut pool, + time_source, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -3076,18 +3131,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 ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - mut pool, - replica_config, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + registry_data_provider, + registry, + mut pool, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -3191,7 +3244,17 @@ pub mod test { fn test_data_block_during_subnet_splitting(#[case] block_registry_version: RegistryVersion) { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let committee = (0..4).map(node_test_id).collect::>(); - let dependencies = DependenciesBuilder::single_subnet( + let ValidatorAndDependencies { + validator, + payload_builder, + state_manager, + registry_data_provider, + registry, + pool, + time_source, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), (1..=block_registry_version.get()) @@ -3206,17 +3269,6 @@ pub mod test { .collect(), ) .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - pool, - time_source, - replica_config, - .. - } = dependencies; payload_builder .get_mut() .expect_validate_payload() @@ -3266,7 +3318,17 @@ pub mod test { fn test_summary_block_during_subnet_splitting(#[case] block_registry_version: RegistryVersion) { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let committee = (0..4).map(node_test_id).collect::>(); - let dependencies = DependenciesBuilder::single_subnet( + let ValidatorAndDependencies { + validator, + payload_builder, + state_manager, + registry_data_provider, + registry, + mut pool, + time_source, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), (1..=block_registry_version.get()) @@ -3281,17 +3343,6 @@ pub mod test { .collect(), ) .build(); - let validator = make_validator(&dependencies); - let Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - mut pool, - time_source, - replica_config, - .. - } = dependencies; payload_builder .get_mut() .expect_validate_payload() @@ -3348,19 +3399,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 ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - registry_data_provider, - registry, - pool, - time_source, - replica_config, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + registry_data_provider, + registry, + pool, + time_source, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() .expect_validate_payload() @@ -3401,16 +3450,14 @@ 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 ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - mut pool, - time_source, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + mut pool, + time_source, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() @@ -3463,16 +3510,14 @@ pub mod test { #[test] fn test_block_context_time() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - mut pool, - time_source, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + mut pool, + time_source, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() @@ -3534,8 +3579,11 @@ pub mod test { #[test] fn test_notarization_requires_at_least_threshold_signatures() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let (validator, Dependencies { mut pool, .. }) = - make_default_validator_and_deps(pool_config); + let ValidatorAndDependencies { + validator, + mut pool, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3582,8 +3630,11 @@ pub mod test { fn test_notarization_deduped_by_content() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { // Setup validator dependencies. - let (validator, Dependencies { mut pool, .. }) = - make_default_validator_and_deps(pool_config); + let ValidatorAndDependencies { + validator, + mut pool, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3634,8 +3685,11 @@ pub mod test { fn test_finalization_deduped_by_content() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { // Setup validator dependencies. - let (validator, Dependencies { mut pool, .. }) = - make_default_validator_and_deps(pool_config); + let ValidatorAndDependencies { + validator, + mut pool, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let block = pool.make_next_block(); pool.insert_validated(block.clone()); @@ -3813,16 +3867,15 @@ pub mod test { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { let cup_height = Height::from(60); // Setup validator dependencies. - let dependencies = DependenciesBuilder::new(pool_config, 4) - .with_dkg_interval_length(cup_height.get() - 1) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { + let ValidatorAndDependencies { + validator, state_manager, mut pool, time_source, .. - } = dependencies; + } = 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()); @@ -3891,14 +3944,12 @@ pub mod test { fn test_should_not_validate_catch_up_package_when_wrong_version() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { // Setup validator dependencies. - let ( + let ValidatorAndDependencies { validator, - Dependencies { - state_manager, - mut pool, - .. - }, - ) = make_default_validator_and_deps(pool_config); + state_manager, + mut pool, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); 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. @@ -3933,18 +3984,16 @@ pub mod test { #[test] fn test_out_of_sync_validation() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - registry, - mut pool, - time_source, - replica_config, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + registry, + mut pool, + time_source, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() @@ -4069,15 +4118,13 @@ pub mod test { #[test] fn test_block_validated_through_notarization() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ( + let ValidatorAndDependencies { validator, - Dependencies { - payload_builder, - state_manager, - mut pool, - .. - }, - ) = make_default_validator_and_deps(pool_config); + payload_builder, + state_manager, + mut pool, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); pool.advance_round_normal_operation(); payload_builder @@ -4138,14 +4185,12 @@ pub mod test { fn setup_equivocation_proof_test( pool_config: ArtifactPoolConfig, ) -> (TestConsensusPool, Validator, EquivocationProof) { - let ( + let ValidatorAndDependencies { validator, - Dependencies { - mut pool, - replica_config, - .. - }, - ) = make_default_validator_and_deps(pool_config); + mut pool, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); pool.advance_round_normal_operation(); pool.insert_validated(pool.make_next_beacon()); @@ -4330,8 +4375,11 @@ 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 (validator, Dependencies { mut pool, .. }) = - make_default_validator_and_deps(pool_config); + let ValidatorAndDependencies { + validator, + mut pool, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); pool.advance_round_normal_operation_n(DKG_INTERVAL_LENGTH); @@ -4368,16 +4416,14 @@ pub mod test { #[test] fn test_create_equivocation_proof() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let ( + let ValidatorAndDependencies { validator, - Dependencies { - state_manager, - time_source, - payload_builder, - mut pool, - .. - }, - ) = make_default_validator_and_deps(pool_config); + state_manager, + time_source, + payload_builder, + mut pool, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); payload_builder .get_mut() @@ -4434,8 +4480,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 (validator, Dependencies { mut pool, .. }) = - make_default_validator_and_deps(pool_config); + let ValidatorAndDependencies { + validator, + mut pool, + .. + } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); let block = pool.make_next_block(); let mut block_with_malicious_signer = block.clone(); @@ -4465,7 +4514,12 @@ 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 dependencies = DependenciesBuilder::single_subnet( + let ValidatorAndDependencies { + validator, + mut pool, + replica_config, + .. + } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![ @@ -4485,12 +4539,6 @@ pub mod test { ], ) .build(); - let validator = make_validator(&dependencies); - let Dependencies { - mut pool, - replica_config, - .. - } = dependencies; // Move to the end of the DKG interval where we switch versions pool.advance_round_normal_operation_n(DKG_INTERVAL_LENGTH + 1); @@ -4523,17 +4571,14 @@ pub mod test { #[test] fn test_ignore_disqualified_ranks() { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::new(pool_config, 7) - .with_dkg_interval_length(DKG_INTERVAL_LENGTH) - .build(); - let validator = make_validator(&dependencies); - let Dependencies { + let ValidatorAndDependencies { + validator, mut pool, time_source, payload_builder, state_manager, .. - } = dependencies; + } = ValidatorAndDependenciesBuilder::new(pool_config, 7).build(); payload_builder .get_mut() @@ -4764,7 +4809,14 @@ pub mod test { fn equivocation_proofs_test(#[case] test_case: EquivocationProofTestCase) { ic_test_utilities_logger::with_test_replica_logger(|logger| { ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| { - let dependencies = DependenciesBuilder::single_subnet( + let ValidatorAndDependencies { + mut validator, + state_manager, + time_source, + payload_builder, + mut pool, + .. + } = ValidatorAndDependenciesBuilder::single_subnet( pool_config, subnet_test_id(0), vec![( @@ -4775,14 +4827,6 @@ pub mod test { )], ) .build(); - let mut validator = make_validator(&dependencies); - let Dependencies { - state_manager, - time_source, - payload_builder, - mut pool, - .. - } = dependencies; validator.log = logger; payload_builder From c448854d0ef7c3089d0646c0a2b0c9d1351ab22d Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 14:18:55 +0000 Subject: [PATCH 22/34] feat: Dependencies' fields should be non-exhaustive --- rs/consensus/mocks/src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/consensus/mocks/src/lib.rs b/rs/consensus/mocks/src/lib.rs index cbb22e9dd7e0..48ccda1f37ac 100644 --- a/rs/consensus/mocks/src/lib.rs +++ b/rs/consensus/mocks/src/lib.rs @@ -98,6 +98,7 @@ impl PayloadBuilder for RefMockPayloadBuilder { } } +#[non_exhaustive] pub struct Dependencies { pub crypto: Arc, pub registry: Arc, From 158f188be424343ab3a6f6bd36ceefaaa7f6ee35 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 14:29:59 +0000 Subject: [PATCH 23/34] ws --- rs/consensus/src/consensus/validator.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index ff41277d5f1e..84c95222e7a1 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2532,7 +2532,6 @@ pub mod test { replica_config, .. } = ValidatorAndDependenciesBuilder::new(pool_config, 4).build(); - pool.advance_round_normal_operation(); // Put a random tape share in the unvalidated pool From 8ecce954e3c70bd73011f46c921f3208b25f9902 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 15:27:01 +0000 Subject: [PATCH 24/34] fix: subnet list record --- rs/consensus/mocks/src/lib.rs | 37 +++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/rs/consensus/mocks/src/lib.rs b/rs/consensus/mocks/src/lib.rs index 48ccda1f37ac..1a2b9f10ce82 100644 --- a/rs/consensus/mocks/src/lib.rs +++ b/rs/consensus/mocks/src/lib.rs @@ -33,7 +33,7 @@ use ic_types::{ use mockall::predicate::*; use mockall::*; use std::{ - collections::{BTreeMap, BTreeSet}, + collections::BTreeSet, sync::{Arc, RwLock}, }; @@ -217,22 +217,39 @@ impl DependenciesBuilder { ) .unwrap(); - let mut all_subnet_ids: BTreeSet = BTreeSet::default(); - let mut subnet_ids_at_version: BTreeMap> = BTreeMap::default(); + let mut subnet_ids: BTreeSet = BTreeSet::default(); + let mut last_version = None; for (version, subnet_id, record) in self.sorted_subnet_records { - if all_subnet_ids.insert(subnet_id) { + // 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); - subnet_ids_at_version - .entry(version) - .or_default() - .insert(subnet_id); + last_version = Some(version); } - for (version, subnet_ids) in subnet_ids_at_version { - add_subnet_list_record(®istry_data_provider, version, Vec::from_iter(subnet_ids)); + // 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( From 23b36115e20dbcf674ba9f2590a874c2b7eeb677 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 15:35:40 +0000 Subject: [PATCH 25/34] style: ws --- rs/consensus/src/consensus/priority.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/rs/consensus/src/consensus/priority.rs b/rs/consensus/src/consensus/priority.rs index a16a6f6d1ce5..182524a362e0 100644 --- a/rs/consensus/src/consensus/priority.rs +++ b/rs/consensus/src/consensus/priority.rs @@ -121,7 +121,6 @@ mod tests { use super::*; use ic_consensus_mocks::{Dependencies, DependenciesBuilder}; use ic_test_utilities_consensus::fake::FakeContent; - use ic_test_utilities_types::ids::node_test_id; use ic_types::{ consensus::{ From 951d56335b325898e8de109ec9ba249f1e5df48c Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 16:06:34 +0000 Subject: [PATCH 26/34] feat: use payload_builder & message_routing --- rs/consensus/src/consensus/block_maker.rs | 28 +++++++++++++---------- rs/consensus/src/consensus/purger.rs | 14 +++++++----- 2 files changed, 24 insertions(+), 18 deletions(-) diff --git a/rs/consensus/src/consensus/block_maker.rs b/rs/consensus/src/consensus/block_maker.rs index c2396f21dd3f..0eea163f628e 100644 --- a/rs/consensus/src/consensus/block_maker.rs +++ b/rs/consensus/src/consensus/block_maker.rs @@ -763,6 +763,7 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, dkg_pool, idkg_pool, .. @@ -788,7 +789,6 @@ mod tests { pool.advance_round_normal_operation_n(4); - let payload_builder = MockPayloadBuilder::new(); let certified_height = Height::from(1); state_manager .get_mut() @@ -801,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(), @@ -820,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(); @@ -857,6 +856,7 @@ mod tests { ); payload_builder + .get_mut() .expect_get_payload() .withf(move |_, payloads, context, _| { matches_expected_payloads(payloads) && context == &expected_context @@ -883,7 +883,7 @@ mod tests { registry.clone(), membership, Arc::clone(&crypto) as Arc<_>, - Arc::new(payload_builder), + payload_builder.clone(), dkg_pool, idkg_pool, state_manager, @@ -934,6 +934,7 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, dkg_pool, idkg_pool, .. @@ -983,12 +984,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); @@ -1018,7 +1019,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(), @@ -1089,6 +1090,7 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, dkg_pool, idkg_pool, .. @@ -1131,8 +1133,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 = @@ -1145,7 +1147,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(), @@ -1237,6 +1239,7 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, registry_data_provider, dkg_pool, idkg_pool, @@ -1248,8 +1251,8 @@ mod tests { ) .build(); - let mut payload_builder = MockPayloadBuilder::new(); payload_builder + .get_mut() .expect_get_payload() .return_const(BatchPayload::default()); let membership = Arc::new(Membership::new( @@ -1264,7 +1267,7 @@ mod tests { Arc::clone(®istry) as Arc, membership, crypto, - Arc::new(payload_builder), + payload_builder, dkg_pool, idkg_pool, state_manager, @@ -1597,6 +1600,7 @@ mod tests { time_source, replica_config, state_manager, + payload_builder, registry_data_provider, dkg_pool, idkg_pool, @@ -1617,8 +1621,8 @@ mod tests { ) .build(); - let mut payload_builder = MockPayloadBuilder::new(); payload_builder + .get_mut() .expect_get_payload() .return_const(BatchPayload::default()); let membership = Arc::new(Membership::new( @@ -1633,7 +1637,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/purger.rs b/rs/consensus/src/consensus/purger.rs index 289f8f31b5bd..009d0305d776 100644 --- a/rs/consensus/src/consensus/purger.rs +++ b/rs/consensus/src/consensus/purger.rs @@ -474,7 +474,6 @@ mod tests { use super::*; 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,6 +491,7 @@ mod tests { state_manager, replica_config, registry, + message_routing, .. } = DependenciesBuilder::new(pool_config, 1).build(); @@ -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(), @@ -737,6 +737,7 @@ mod tests { state_manager, replica_config, registry, + message_routing, .. } = DependenciesBuilder::new(pool_config, 10).build(); @@ -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, .. } = 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(), From 669f22d8d29131d1ab94a38b3b6527087a10e546 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 16:15:32 +0000 Subject: [PATCH 27/34] refactor(consensus): use DependenciesBuilder::new at interval-only single_subnet sites in ic-consensus tests share_aggregator and block_maker built default records for subnet_test_id(0) at registry version 1 only to set the DKG interval length, which DependenciesBuilder::new supports directly. Co-Authored-By: Claude Fable 5 --- rs/consensus/src/consensus/block_maker.rs | 16 +++------------- .../src/consensus/share_aggregator.rs | 19 +++++-------------- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/rs/consensus/src/consensus/block_maker.rs b/rs/consensus/src/consensus/block_maker.rs index 0eea163f628e..70f763961975 100644 --- a/rs/consensus/src/consensus/block_maker.rs +++ b/rs/consensus/src/consensus/block_maker.rs @@ -922,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, @@ -938,17 +936,9 @@ mod tests { dkg_pool, idkg_pool, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_id, - vec![( - 1, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .build(); + } = DependenciesBuilder::new(pool_config, 13) + .with_dkg_interval_length(dkg_interval_length) + .build(); pool.advance_round_normal_operation_n(8); diff --git a/rs/consensus/src/consensus/share_aggregator.rs b/rs/consensus/src/consensus/share_aggregator.rs index ad32ca8b9bd9..4ab31e7bc376 100644 --- a/rs/consensus/src/consensus/share_aggregator.rs +++ b/rs/consensus/src/consensus/share_aggregator.rs @@ -191,8 +191,8 @@ mod tests { use ic_logger::replica_logger::no_op_logger; use ic_test_utilities::message_routing::FakeMessageRouting; use ic_test_utilities_consensus::fake::{FakeContentSigner, FakeSigner}; - 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::{ NodeId, RegistryVersion, consensus::{ @@ -320,24 +320,15 @@ mod tests { oldest_registry_version_in_use_by_replicated_state: Option, ) -> 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, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - INITIAL_REGISTRY_VERSION, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(interval_length) - .build(), - )], - ) - .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()); From fdab0ca322535303b46f843730d7316aa494095f Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 16:15:32 +0000 Subject: [PATCH 28/34] refactor(consensus): use DependenciesBuilder::new at interval-only single_subnet sites in ic-consensus-dkg tests These sites passed a custom subnet id (222, 1) or initial registry version (10, 5) that the tests never rely on: the version is consumed only through registry.get_latest_version(), and the subnet id never reaches an assertion or registry key (no consensus code special-cases the root subnet). Normalize them to the DependenciesBuilder::new defaults. In test_validate_payload the ValidationContext registry version is updated together with the record version (5 -> 1). Co-Authored-By: Claude Fable 5 --- rs/consensus/dkg/src/dkg_key_manager.rs | 17 +--- rs/consensus/dkg/src/lib.rs | 97 ++++++----------------- rs/consensus/dkg/src/payload_validator.rs | 17 +--- rs/consensus/dkg/src/utils.rs | 17 +--- 4 files changed, 36 insertions(+), 112 deletions(-) diff --git a/rs/consensus/dkg/src/dkg_key_manager.rs b/rs/consensus/dkg/src/dkg_key_manager.rs index 16079391346e..192be0ee53ce 100644 --- a/rs/consensus/dkg/src/dkg_key_manager.rs +++ b/rs/consensus/dkg/src/dkg_key_manager.rs @@ -561,26 +561,15 @@ mod tests { 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, .. } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(222), - vec![( - 1, - SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) - .build(), - )], - ) - .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 0f00d893abd3..3a13b6186333 100644 --- a/rs/consensus/dkg/src/lib.rs +++ b/rs/consensus/dkg/src/lib.rs @@ -768,28 +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, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .without_mocked_state_manager() - .build(); + } = DependenciesBuilder::new(pool_config, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_mocked_state_manager() + .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); complement_state_manager_with_setup_initial_dkg_request( @@ -804,8 +795,8 @@ mod tests { let dkg_key_manager = new_dkg_key_manager(crypto.clone(), logger.clone(), &PoolReader::new(&pool)); let dkg = DkgImpl::new( - node_test_id(1), - subnet_id, + replica_config.node_id, + replica_config.subnet_id, registry.clone(), state_manager.clone(), crypto, @@ -935,26 +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, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .without_mocked_state_manager() - .build(); + } = DependenciesBuilder::new(pool_config, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_mocked_state_manager() + .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); complement_state_manager_with_setup_initial_dkg_request( @@ -1510,35 +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 = DependenciesBuilder::single_subnet( - pool_config_1, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .without_mocked_state_manager() - .build(); - let dependencies_2 = DependenciesBuilder::single_subnet( - pool_config_2, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .without_mocked_state_manager() - .build(); + let dependencies_1 = DependenciesBuilder::new(pool_config_1, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_mocked_state_manager() + .build(); + let dependencies_2 = DependenciesBuilder::new(pool_config_2, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_mocked_state_manager() + .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 @@ -2104,23 +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 = DependenciesBuilder::single_subnet( - pool_config, - subnet_id, - vec![( - 10, - SubnetRecordBuilder::from(&node_ids) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .without_mocked_state_manager() - .build(); + let mut deps = DependenciesBuilder::new(pool_config, 2) + .with_dkg_interval_length(dkg_interval_length) + .without_mocked_state_manager() + .build(); // Start without context so remote dealing validation is deferred. complement_state_manager_with_dkg_contexts( @@ -2154,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), }; @@ -2162,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), }; diff --git a/rs/consensus/dkg/src/payload_validator.rs b/rs/consensus/dkg/src/payload_validator.rs index dd30bb059cd0..0e9b961cdd78 100644 --- a/rs/consensus/dkg/src/payload_validator.rs +++ b/rs/consensus/dkg/src/payload_validator.rs @@ -298,7 +298,6 @@ 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, @@ -306,20 +305,12 @@ mod tests { state_manager, dkg_pool, .. - } = DependenciesBuilder::single_subnet( - pool_config, - subnet_test_id(0), - vec![( - 5, - SubnetRecordBuilder::from(&committee) - .with_dkg_interval_length(dkg_interval_length) - .build(), - )], - ) - .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, }; diff --git a/rs/consensus/dkg/src/utils.rs b/rs/consensus/dkg/src/utils.rs index d3ce6e5f5f7b..1e0a26d467ec 100644 --- a/rs/consensus/dkg/src/utils.rs +++ b/rs/consensus/dkg/src/utils.rs @@ -314,20 +314,11 @@ 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 subnet_id = subnet_test_id(0); let dkg_interval_len = 10; - let Dependencies { mut pool, .. } = DependenciesBuilder::single_subnet( - pool_config, - subnet_id, - vec![( - 1, - SubnetRecordBuilder::from(&nodes) - .with_dkg_interval_length(dkg_interval_len) - .build(), - )], - ) - .build(); + let Dependencies { mut pool, .. } = 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); From ff2d309dbbc3290d4d9b8f6b411351246385d604 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Fri, 7 Aug 2026 17:00:17 +0000 Subject: [PATCH 29/34] fix: cleanups --- rs/consensus/dkg/src/payload_validator.rs | 5 +++-- rs/consensus/dkg/src/utils.rs | 12 ++++++++---- rs/consensus/src/consensus/block_maker.rs | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/rs/consensus/dkg/src/payload_validator.rs b/rs/consensus/dkg/src/payload_validator.rs index 0e9b961cdd78..20475ae15d1e 100644 --- a/rs/consensus/dkg/src/payload_validator.rs +++ b/rs/consensus/dkg/src/payload_validator.rs @@ -302,6 +302,7 @@ mod tests { crypto, mut pool, registry, + replica_config, state_manager, dkg_pool, .. @@ -327,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), @@ -355,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), diff --git a/rs/consensus/dkg/src/utils.rs b/rs/consensus/dkg/src/utils.rs index 1e0a26d467ec..5c222f89cc26 100644 --- a/rs/consensus/dkg/src/utils.rs +++ b/rs/consensus/dkg/src/utils.rs @@ -314,9 +314,12 @@ 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(0); let dkg_interval_len = 10; - let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 4) + let Dependencies { + mut pool, + replica_config, + .. + } = DependenciesBuilder::new(pool_config, 4) .with_dkg_interval_length(dkg_interval_len) .build(); @@ -325,9 +328,10 @@ mod tests { 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/src/consensus/block_maker.rs b/rs/consensus/src/consensus/block_maker.rs index 70f763961975..c116b44615c8 100644 --- a/rs/consensus/src/consensus/block_maker.rs +++ b/rs/consensus/src/consensus/block_maker.rs @@ -883,7 +883,7 @@ mod tests { registry.clone(), membership, Arc::clone(&crypto) as Arc<_>, - payload_builder.clone(), + payload_builder, dkg_pool, idkg_pool, state_manager, From fe657b81410751072eafcc8e29687a3bba0160cd Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Wed, 12 Aug 2026 08:49:51 +0000 Subject: [PATCH 30/34] fix: clippy --- Cargo.lock | 1 - rs/consensus/certification/BUILD.bazel | 1 - rs/consensus/certification/Cargo.toml | 1 - rs/consensus/certification/src/certifier.rs | 1 - 4 files changed, 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fd7bd97a61f6..514424eb55d5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7552,7 +7552,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", diff --git a/rs/consensus/certification/BUILD.bazel b/rs/consensus/certification/BUILD.bazel index 574c6f36bc72..cdc88f8955e4 100644 --- a/rs/consensus/certification/BUILD.bazel +++ b/rs/consensus/certification/BUILD.bazel @@ -55,7 +55,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..98c52f770fee 100644 --- a/rs/consensus/certification/Cargo.toml +++ b/rs/consensus/certification/Cargo.toml @@ -32,7 +32,6 @@ 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 835e94006072..f813f3ed857f 100644 --- a/rs/consensus/certification/src/certifier.rs +++ b/rs/consensus/certification/src/certifier.rs @@ -702,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}; From 471c2f5516f45bd88e74604cc42ecf223c48145c Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Wed, 12 Aug 2026 08:53:04 +0000 Subject: [PATCH 31/34] remove one more deps --- Cargo.lock | 1 - rs/consensus/certification/BUILD.bazel | 1 - rs/consensus/certification/Cargo.toml | 1 - 3 files changed, 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 514424eb55d5..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", diff --git a/rs/consensus/certification/BUILD.bazel b/rs/consensus/certification/BUILD.bazel index cdc88f8955e4..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", diff --git a/rs/consensus/certification/Cargo.toml b/rs/consensus/certification/Cargo.toml index 98c52f770fee..ee085c64d89a 100644 --- a/rs/consensus/certification/Cargo.toml +++ b/rs/consensus/certification/Cargo.toml @@ -25,7 +25,6 @@ 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" } From 343edfa15f0dd4fe687c606cd5ecbfa17fd33b38 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Wed, 12 Aug 2026 12:43:28 +0000 Subject: [PATCH 32/34] style: rename to `without_state_manager_expectations` --- rs/consensus/chain_key/src/lib.rs | 2 +- rs/consensus/dkg/src/lib.rs | 16 ++++++++-------- rs/consensus/dkg/src/payload_builder.rs | 2 +- rs/consensus/mocks/src/lib.rs | 10 +++++----- .../src/consensus/catchup_package_maker.rs | 2 +- rs/consensus/src/consensus/validator.rs | 6 +++--- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/rs/consensus/chain_key/src/lib.rs b/rs/consensus/chain_key/src/lib.rs index 1b2bb51cf566..22cf8f9b471d 100644 --- a/rs/consensus/chain_key/src/lib.rs +++ b/rs/consensus/chain_key/src/lib.rs @@ -807,7 +807,7 @@ mod tests { subnet_id, vec![(1, subnet_record_builder.build())], ) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); // Enable the configured keys diff --git a/rs/consensus/dkg/src/lib.rs b/rs/consensus/dkg/src/lib.rs index 3a13b6186333..77af175f309d 100644 --- a/rs/consensus/dkg/src/lib.rs +++ b/rs/consensus/dkg/src/lib.rs @@ -779,7 +779,7 @@ mod tests { .. } = DependenciesBuilder::new(pool_config, 2) .with_dkg_interval_length(dkg_interval_length) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); @@ -934,7 +934,7 @@ mod tests { .. } = DependenciesBuilder::new(pool_config, 2) .with_dkg_interval_length(dkg_interval_length) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); @@ -1496,11 +1496,11 @@ mod tests { // Set pool_1 and pool_2 let dependencies_1 = DependenciesBuilder::new(pool_config_1, 2) .with_dkg_interval_length(dkg_interval_length) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); let dependencies_2 = DependenciesBuilder::new(pool_config_2, 2) .with_dkg_interval_length(dkg_interval_length) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); // Return an empty call context when we create the first summary, @@ -1695,7 +1695,7 @@ mod tests { .build(), )], ) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); let target_id = NiDkgTargetId::new([0_u8; 32]); @@ -2072,7 +2072,7 @@ mod tests { let mut deps = DependenciesBuilder::new(pool_config, 2) .with_dkg_interval_length(dkg_interval_length) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); // Start without context so remote dealing validation is deferred. @@ -2207,7 +2207,7 @@ mod tests { .build(), )], ) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); // No contexts at the beginning @@ -2318,7 +2318,7 @@ mod tests { .build(), )], ) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); let registry_version = deps.registry.get_latest_version(); diff --git a/rs/consensus/dkg/src/payload_builder.rs b/rs/consensus/dkg/src/payload_builder.rs index eec00cd7d7d2..1ad5b0c3fa2f 100644 --- a/rs/consensus/dkg/src/payload_builder.rs +++ b/rs/consensus/dkg/src/payload_builder.rs @@ -1094,7 +1094,7 @@ mod tests { .build(), )], ) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); let registry_version = deps.registry.get_latest_version(); let setup_target = NiDkgTargetId::new([5_u8; 32]); diff --git a/rs/consensus/mocks/src/lib.rs b/rs/consensus/mocks/src/lib.rs index 1a2b9f10ce82..a22772e6122b 100644 --- a/rs/consensus/mocks/src/lib.rs +++ b/rs/consensus/mocks/src/lib.rs @@ -119,7 +119,7 @@ pub struct DependenciesBuilder { pool_config: ArtifactPoolConfig, sorted_subnet_records: Vec<(u64, SubnetId, SubnetRecord)>, replica_config: ReplicaConfig, - mocked_state_manager: bool, + with_state_manager_expectations: bool, } impl DependenciesBuilder { @@ -180,7 +180,7 @@ impl DependenciesBuilder { subnet_id: subnet_records[0].1, }, sorted_subnet_records: subnet_records, - mocked_state_manager: true, + with_state_manager_expectations: true, } } @@ -200,8 +200,8 @@ impl DependenciesBuilder { /// Leaves the returned `RefMockStateManager` without any expectations, so /// that the test can set up its own `get_state_at` behavior. - pub fn without_mocked_state_manager(mut self) -> Self { - self.mocked_state_manager = false; + pub fn without_state_manager_expectations(mut self) -> Self { + self.with_state_manager_expectations = false; self } @@ -293,7 +293,7 @@ impl DependenciesBuilder { self.replica_config.subnet_id, )); - if self.mocked_state_manager { + if self.with_state_manager_expectations { state_manager .get_mut() .expect_get_state_at() diff --git a/rs/consensus/src/consensus/catchup_package_maker.rs b/rs/consensus/src/consensus/catchup_package_maker.rs index 574b5d85c13b..43164ee476c0 100644 --- a/rs/consensus/src/consensus/catchup_package_maker.rs +++ b/rs/consensus/src/consensus/catchup_package_maker.rs @@ -597,7 +597,7 @@ mod tests { .. } = DependenciesBuilder::new(pool_config, 4) .with_dkg_interval_length(interval_length) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); let height = Height::from(0); diff --git a/rs/consensus/src/consensus/validator.rs b/rs/consensus/src/consensus/validator.rs index 84c95222e7a1..9223f47cb93a 100644 --- a/rs/consensus/src/consensus/validator.rs +++ b/rs/consensus/src/consensus/validator.rs @@ -2123,8 +2123,8 @@ pub mod test { self } - fn without_mocked_state_manager(mut self) -> Self { - self.deps_builder = self.deps_builder.without_mocked_state_manager(); + fn without_state_manager_expectations(mut self) -> Self { + self.deps_builder = self.deps_builder.without_state_manager_expectations(); self } @@ -2315,7 +2315,7 @@ pub mod test { mut pool, .. } = ValidatorAndDependenciesBuilder::new(pool_config, 4) - .without_mocked_state_manager() + .without_state_manager_expectations() .build(); // The state manager is mocked and the `StateHash` is completely arbitrary. It From 49e5b9af1cbede0d08d6b0e040ba786b6154d984 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Wed, 12 Aug 2026 12:44:54 +0000 Subject: [PATCH 33/34] style: ws --- rs/consensus/src/consensus/catchup_package_maker.rs | 1 - rs/consensus/src/consensus/share_aggregator.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/rs/consensus/src/consensus/catchup_package_maker.rs b/rs/consensus/src/consensus/catchup_package_maker.rs index 43164ee476c0..0e0c2ca39bf5 100644 --- a/rs/consensus/src/consensus/catchup_package_maker.rs +++ b/rs/consensus/src/consensus/catchup_package_maker.rs @@ -309,7 +309,6 @@ mod tests { fake_signature_request_context_with_registry_version, }, }; - use ic_test_utilities_types::ids::subnet_test_id; use ic_types::{ CryptoHashOfState, Height, RegistryVersion, diff --git a/rs/consensus/src/consensus/share_aggregator.rs b/rs/consensus/src/consensus/share_aggregator.rs index 4ab31e7bc376..6f5addf65831 100644 --- a/rs/consensus/src/consensus/share_aggregator.rs +++ b/rs/consensus/src/consensus/share_aggregator.rs @@ -191,7 +191,6 @@ mod tests { use ic_logger::replica_logger::no_op_logger; use ic_test_utilities::message_routing::FakeMessageRouting; use ic_test_utilities_consensus::fake::{FakeContentSigner, FakeSigner}; - use ic_test_utilities_types::ids::node_test_id; use ic_types::{ NodeId, RegistryVersion, From 54eee4e67bf20d42ca6990943b438e4d8644c988 Mon Sep 17 00:00:00 2001 From: Pierugo Pace Date: Wed, 12 Aug 2026 12:48:10 +0000 Subject: [PATCH 34/34] revert node_test_id(1) --- rs/consensus/dkg/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/consensus/dkg/src/lib.rs b/rs/consensus/dkg/src/lib.rs index 77af175f309d..be56990a6079 100644 --- a/rs/consensus/dkg/src/lib.rs +++ b/rs/consensus/dkg/src/lib.rs @@ -795,7 +795,7 @@ mod tests { let dkg_key_manager = new_dkg_key_manager(crypto.clone(), logger.clone(), &PoolReader::new(&pool)); let dkg = DkgImpl::new( - replica_config.node_id, + node_test_id(1), replica_config.subnet_id, registry.clone(), state_manager.clone(),