refactor(consensus): introduce a builder pattern for dependencies in consensus' unit tests

rs/consensus/certification/BUILD.bazel

@@ -39,7 +39,6 @@ rust_test(
         "//rs/artifact_pool",
         "//rs/canonical_state",
         "//rs/canonical_state/tree_hash",
-        "//rs/config",
         "//rs/consensus/mocks",
         "//rs/consensus/utils",
         "//rs/crypto/test_utils/crypto_returning_ok",
@@ -55,7 +54,6 @@ rust_test(
         "//rs/test_utilities/artifact_pool",
         "//rs/test_utilities/consensus",
         "//rs/test_utilities/logger",
-        "//rs/test_utilities/registry",
         "//rs/test_utilities/types",
         "//rs/types/types",
         "//rs/types/types_test_utils",

rs/consensus/certification/Cargo.toml

@@ -25,14 +25,12 @@ slog = { workspace = true }
 assert_matches = { workspace = true }
 ic-artifact-pool = { path = "../../artifact_pool" }
 ic-consensus-mocks = { path = "../mocks" }
-ic-config = { path = "../../config" }
 ic-crypto-test-utils-crypto-returning-ok = { path = "../../crypto/test_utils/crypto_returning_ok" }
 ic-registry-subnet-type = { path = "../../registry/subnet_type" }
 ic-test-artifact-pool = { path = "../../test_utilities/artifact_pool" }
 ic-test-utilities = { path = "../../test_utilities" }
 ic-test-utilities-consensus = { path = "../../test_utilities/consensus" }
 ic-test-utilities-logger = { path = "../../test_utilities/logger" }
-ic-test-utilities-registry = { path = "../../test_utilities/registry" }
 ic-test-utilities-types = { path = "../../test_utilities/types" }
 ic-types-test-utils = { path = "../../types/types_test_utils" }
 mockall = { workspace = true }

rs/consensus/certification/src/certifier.rs

@@ -691,8 +691,7 @@ mod tests {
     use ic_canonical_state::lazy_tree_conversion::replicated_state_as_lazy_tree;
     use ic_canonical_state_tree_hash::hash_tree::hash_lazy_tree;
     use ic_canonical_state_tree_hash::lazy_tree::materialize::materialize_partial;
-    use ic_config::artifact_pool::ArtifactPoolConfig;
-    use ic_consensus_mocks::{Dependencies, dependencies, dependencies_with_subnet_params};
+    use ic_consensus_mocks::{Dependencies, DependenciesBuilder};
     use ic_crypto_tree_hash::{Digest, Witness, sparse_labeled_tree_from_paths};
     use ic_interfaces::{
         certification::CertificationPool,
@@ -703,7 +702,6 @@ mod tests {
     use ic_test_artifact_pool::consensus_pool::TestConsensusPool;
     use ic_test_utilities_consensus::fake::*;
     use ic_test_utilities_logger::with_test_replica_logger;
-    use ic_test_utilities_registry::SubnetRecordBuilder;
     use ic_test_utilities_types::ids::{node_test_id, subnet_test_id};
     use ic_types::backwards_compatibility::BackwardsCompatible;
     use ic_types::consensus::{BlockPayload, HashedBlock, Payload, dkg::SplittingArgs};
@@ -827,7 +825,7 @@ mod tests {
                     crypto,
                     state_manager,
                     ..
-                } = dependencies(pool_config.clone(), 1);
+                } = DependenciesBuilder::new(pool_config.clone(), 1).build();
 
                 let certifier = CertifierImpl::new(
                     replica_config,
@@ -863,7 +861,7 @@ mod tests {
                     crypto,
                     state_manager,
                     ..
-                } = dependencies(pool_config.clone(), 4);
+                } = DependenciesBuilder::new(pool_config.clone(), 4).build();
                 pool.advance_round_normal_operation();
                 add_expectations(state_manager.clone(), 1, 4);
                 let metrics_registry = MetricsRegistry::new();
@@ -929,7 +927,7 @@ mod tests {
                     crypto,
                     state_manager,
                     ..
-                } = dependencies(pool_config.clone(), 4);
+                } = DependenciesBuilder::new(pool_config.clone(), 4).build();
 
                 pool.advance_round_normal_operation_n(6);
                 add_expectations(state_manager.clone(), 1, 4);
@@ -1066,7 +1064,7 @@ mod tests {
                 crypto,
                 state_manager,
                 ..
-            } = dependencies(pool_config.clone(), 6);
+            } = DependenciesBuilder::new(pool_config.clone(), 6).build();
             // make the mock state manager return empty hashes for heights 3, 4 and 5
             add_expectations(state_manager.clone(), 3, 5);
             let metrics_registry = MetricsRegistry::new();
@@ -1146,7 +1144,7 @@ mod tests {
                 crypto,
                 state_manager,
                 ..
-            } = dependencies(pool_config.clone(), 7);
+            } = DependenciesBuilder::new(pool_config.clone(), 7).build();
             pool.insert_beacon_chain(&pool.make_next_beacon(), Height::from(10));
             // make the mock state manager return empty hashes for heights 3, 4 and 5
             add_expectations(state_manager.clone(), 3, 5);
@@ -1219,7 +1217,7 @@ mod tests {
                 crypto,
                 state_manager,
                 ..
-            } = dependencies(pool_config.clone(), 4);
+            } = DependenciesBuilder::new(pool_config.clone(), 4).build();
             pool.advance_round_normal_operation_n(10);
             // make the mock state manager return empty hashes for heights 3, 4 and 5
             add_expectations(state_manager.clone(), 3, 5);
@@ -1291,7 +1289,7 @@ mod tests {
                     crypto,
                     state_manager,
                     ..
-                } = dependencies(pool_config.clone(), 1);
+                } = DependenciesBuilder::new(pool_config.clone(), 1).build();
                 pool.advance_round_normal_operation_n(10);
                 // make the mock state manager return empty hashes for heights 3, 4 and 5
                 add_expectations(state_manager.clone(), 3, 5);
@@ -1464,7 +1462,7 @@ mod tests {
                 crypto,
                 state_manager,
                 ..
-            } = dependencies(pool_config.clone(), 4);
+            } = DependenciesBuilder::new(pool_config.clone(), 4).build();
             // make the mock state manager return empty hashes for heights 4 and 5
             add_expectations(state_manager.clone(), 4, 5);
             let metrics_registry = MetricsRegistry::new();
@@ -1562,7 +1560,7 @@ mod tests {
                     crypto,
                     state_manager,
                     ..
-                } = dependencies(pool_config.clone(), 4);
+                } = DependenciesBuilder::new(pool_config.clone(), 4).build();
 
                 let metrics_registry = MetricsRegistry::new();
                 let cert_pool = CertificationPoolImpl::new(
@@ -1641,23 +1639,6 @@ mod tests {
     // DKG interval length used for subnet-splitting tests.
     const TEST_DKG_INTERVAL: u64 = 9;
 
-    fn dependencies_for_splitting_tests(
-        pool_config: ArtifactPoolConfig,
-        nodes: u64,
-    ) -> Dependencies {
-        let committee = (0..nodes).map(node_test_id).collect::<Vec<_>>();
-        dependencies_with_subnet_params(
-            pool_config,
-            subnet_test_id(0),
-            vec![(
-                1,
-                SubnetRecordBuilder::from(&committee)
-                    .with_dkg_interval_length(TEST_DKG_INTERVAL)
-                    .build(),
-            )],
-        )
-    }
-
     // Advances `pool` by TEST_DKG_INTERVAL rounds so the next block is a DKG
     // summary block, then inserts and finalizes that summary block after setting
     // its subnet-splitting status to `status`.
@@ -1754,7 +1735,9 @@ mod tests {
                     crypto,
                     state_manager,
                     ..
-                } = dependencies_for_splitting_tests(pool_config.clone(), 4);
+                } = DependenciesBuilder::new(pool_config.clone(), 4)
+                    .with_dkg_interval_length(TEST_DKG_INTERVAL)
+                    .build();
 
                 let metrics_registry = MetricsRegistry::new();
                 let cert_pool = CertificationPoolImpl::new(
@@ -1816,7 +1799,9 @@ mod tests {
                     crypto,
                     state_manager,
                     ..
-                } = dependencies_for_splitting_tests(pool_config.clone(), 4);
+                } = DependenciesBuilder::new(pool_config.clone(), 4)
+                    .with_dkg_interval_length(TEST_DKG_INTERVAL)
+                    .build();
 
                 let metrics_registry = MetricsRegistry::new();
                 let cert_pool = CertificationPoolImpl::new(
@@ -1883,7 +1868,9 @@ mod tests {
                     crypto,
                     state_manager,
                     ..
-                } = dependencies_for_splitting_tests(pool_config.clone(), 1);
+                } = DependenciesBuilder::new(pool_config.clone(), 1)
+                    .with_dkg_interval_length(TEST_DKG_INTERVAL)
+                    .build();
 
                 let certifier = CertifierImpl::new(
                     replica_config,

rs/consensus/chain_key/src/lib.rs

@@ -657,9 +657,7 @@ fn reject_if_invalid(
 mod tests {
     use assert_matches::assert_matches;
     use core::{convert::From, iter::Iterator, time::Duration};
-    use ic_consensus_mocks::{
-        Dependencies, dependencies_with_subnet_records_with_raw_state_manager,
-    };
+    use ic_consensus_mocks::{Dependencies, DependenciesBuilder};
     use ic_crypto_temp_crypto::TempCryptoComponent;
     use ic_interfaces::consensus::{InvalidPayloadReason, PayloadValidationFailure};
     use ic_interfaces::idkg::IDkgChangeAction;
@@ -804,11 +802,13 @@ mod tests {
                 registry,
                 registry_data_provider,
                 ..
-            } = dependencies_with_subnet_records_with_raw_state_manager(
+            } = DependenciesBuilder::single_subnet(
                 pool_config,
                 subnet_id,
                 vec![(1, subnet_record_builder.build())],
-            );
+            )
+            .without_state_manager_expectations()
+            .build();
 
             // Enable the configured keys
             if let Some(config) = config

rs/consensus/dkg/src/dkg_key_manager.rs

@@ -557,29 +557,19 @@ fn dkg_id_log_msg(id: &NiDkgId) -> String {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use ic_consensus_mocks::{Dependencies, dependencies_with_subnet_params};
+    use ic_consensus_mocks::{Dependencies, DependenciesBuilder};
     use ic_crypto_test_utils_crypto_returning_ok::CryptoReturningOk;
     use ic_metrics::MetricsRegistry;
     use ic_test_utilities_logger::with_test_replica_logger;
-    use ic_test_utilities_registry::SubnetRecordBuilder;
-    use ic_test_utilities_types::ids::{node_test_id, subnet_test_id};
 
     #[test]
     fn test_transcripts_get_loaded_and_retained() {
         ic_test_utilities::artifact_pool_config::with_test_pool_config(|pool_config| {
             with_test_replica_logger(|logger| {
-                let nodes: Vec<_> = (0..1).map(node_test_id).collect();
                 let dkg_interval_len = 3;
-                let Dependencies { mut pool, .. } = dependencies_with_subnet_params(
-                    pool_config,
-                    subnet_test_id(222),
-                    vec![(
-                        1,
-                        SubnetRecordBuilder::from(&nodes)
-                            .with_dkg_interval_length(dkg_interval_len)
-                            .build(),
-                    )],
-                );
+                let Dependencies { mut pool, .. } = DependenciesBuilder::new(pool_config, 1)
+                    .with_dkg_interval_length(dkg_interval_len)
+                    .build();
                 let csp = Arc::new(CryptoReturningOk::default());
                 let mut key_manager = DkgKeyManager::new(
                     MetricsRegistry::new(),