From dcd06357455a67843ba2bfe2b479308d1690fc9e Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Tue, 25 Jun 2024 16:25:08 +0100 Subject: [PATCH] TEST: Turn each twins scenario into a separate test case (#133) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What ❔ Instead of running multiple Twins scenarios in a loop inside a single test, we use the `#[test_casing]` macro to reify scenarios as test cases, except for the test which runs up to 100 scenarios looking for a counter example (which for me came after ~25 scenarios) and splitting wouldn't make sense. ## Why ❔ So that scenarios take individually less time, and are potentially easier to reproduce (eg. jump to scenario 10 which failed). ### Example ```console ❯ cargo nextest run -p zksync_consensus_bft twins Compiling zksync_consensus_bft v0.1.0 (/Users/aakoshh/Work/matter-labs/era-consensus/node/actors/bft) Finished `test` profile [unoptimized + debuginfo] target(s) in 2.52s Starting 76 tests across 1 binary (67 skipped; run ID: 9352ff1c-ce88-4a10-b15f-0efdf3f3a250, nextest profile: default) PASS [ 0.023s] zksync_consensus_bft testonly::twins::tests::prop_twin PASS [ 0.025s] zksync_consensus_bft testonly::twins::tests::prop_cluster PASS [ 0.023s] zksync_consensus_bft testonly::twins::tests::test_splits PASS [ 0.029s] zksync_consensus_bft testonly::twins::tests::test_scenario_generator PASS [ 0.390s] zksync_consensus_bft testonly::twins::tests::prop_splits PASS [ 0.588s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_00 PASS [ 0.613s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_04 PASS [ 0.653s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_09 PASS [ 0.700s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_05 PASS [ 0.731s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_06 PASS [ 0.806s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_07 PASS [ 0.942s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_01 PASS [ 1.082s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_11 PASS [ 1.191s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_08 PASS [ 1.282s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_02 PASS [ 0.706s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_14 PASS [ 0.847s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_15 PASS [ 0.933s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_13 PASS [ 0.712s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_18 PASS [ 1.720s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_03 PASS [ 1.355s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_12 PASS [ 1.116s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_16 PASS [ 1.815s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_10 PASS [ 1.107s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_17 PASS [ 0.798s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_21 PASS [ 0.963s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_20 PASS [ 1.349s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_19 PASS [ 1.125s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_23 PASS [ 1.070s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_24 PASS [ 0.891s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_27 PASS [ 1.111s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_25 PASS [ 0.968s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_28 PASS [ 0.916s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_29 PASS [ 1.313s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_26 PASS [ 1.348s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_30 PASS [ 1.183s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_33 PASS [ 1.405s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_31 PASS [ 1.007s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_34 PASS [ 2.051s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_22 PASS [ 0.989s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_37 PASS [ 1.078s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_36 PASS [ 1.103s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_39 PASS [ 1.454s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_35 PASS [ 1.289s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_38 PASS [ 1.476s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_41 PASS [ 2.318s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_32 PASS [ 1.435s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_42 PASS [ 1.404s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_45 PASS [ 1.529s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_43 PASS [ 1.507s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_46 PASS [ 2.316s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_40 PASS [ 0.400s] zksync_consensus_bft tests::twins_network_wo_twins_w_partitions::case_0 PASS [ 1.545s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_47 PASS [ 1.395s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_49 PASS [ 2.030s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_44 PASS [ 0.557s] zksync_consensus_bft tests::twins_network_wo_twins_w_partitions::case_1 PASS [ 0.511s] zksync_consensus_bft tests::twins_network_wo_twins_w_partitions::case_2 PASS [ 0.326s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_0 PASS [ 0.323s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_1 PASS [ 0.445s] zksync_consensus_bft tests::twins_network_wo_twins_w_partitions::case_4 PASS [ 0.324s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_2 PASS [ 0.284s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_3 PASS [ 0.563s] zksync_consensus_bft tests::twins_network_wo_twins_w_partitions::case_3 PASS [ 1.974s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_48 PASS [ 0.317s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_4 PASS [ 0.325s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_5 PASS [ 0.332s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_6 PASS [ 0.332s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_7 PASS [ 0.315s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_8 PASS [ 0.310s] zksync_consensus_bft tests::twins_network_wo_twins_wo_partitions::case_9 PASS [ 3.078s] zksync_consensus_bft tests::twins_network_w2_twins_w_partitions::case_0 PASS [ 3.160s] zksync_consensus_bft tests::twins_network_w2_twins_w_partitions::case_3 PASS [ 3.632s] zksync_consensus_bft tests::twins_network_w2_twins_w_partitions::case_1 PASS [ 3.463s] zksync_consensus_bft tests::twins_network_w2_twins_w_partitions::case_4 PASS [ 3.758s] zksync_consensus_bft tests::twins_network_w2_twins_w_partitions::case_2 PASS [ 10.296s] zksync_consensus_bft tests::twins_network_to_fail ------------ Summary [ 10.304s] 76 tests run: 76 passed, 67 skipped ❯ cargo nextest run -p zksync_consensus_bft twins_network_w1_twins_w_partitions::case_37 Compiling zksync_consensus_bft v0.1.0 (/Users/aakoshh/Work/matter-labs/era-consensus/node/actors/bft) Finished `test` profile [unoptimized + debuginfo] target(s) in 0.89s Starting 1 test across 1 binary (142 skipped; run ID: 596b7941-05f4-4821-a679-1f7a37cd346b, nextest profile: default) PASS [ 0.572s] zksync_consensus_bft tests::twins_network_w1_twins_w_partitions::case_37 ------------ Summary [ 0.573s] 1 test run: 1 passed, 142 skipped ``` --- node/actors/bft/src/tests.rs | 72 ++++++++++++++++++++++++++++-------- 1 file changed, 56 insertions(+), 16 deletions(-) diff --git a/node/actors/bft/src/tests.rs b/node/actors/bft/src/tests.rs index d498f424..7ee4088a 100644 --- a/node/actors/bft/src/tests.rs +++ b/node/actors/bft/src/tests.rs @@ -5,7 +5,7 @@ use crate::testonly::{ }; use assert_matches::assert_matches; use std::collections::HashMap; -use test_casing::test_casing; +use test_casing::{cases, test_casing, TestCases}; use zksync_concurrency::{ctx, scope, time}; use zksync_consensus_network::testonly::new_configs_for_validators; use zksync_consensus_roles::validator::{ @@ -185,11 +185,14 @@ async fn non_proposing_leader() { /// /// This should be a simple sanity check that the network works and consensus /// is achieved under the most favourable conditions. +#[test_casing(10,0..10)] #[tokio::test] -async fn twins_network_wo_twins_wo_partitions() { +async fn twins_network_wo_twins_wo_partitions(num_reseeds: usize) { tokio::time::pause(); // n<6 implies f=0 and q=n - run_twins(5, 0, 10).await.unwrap(); + run_twins(5, 0, TwinsScenarios::Reseeds(num_reseeds)) + .await + .unwrap(); } /// Run Twins scenarios without actual twins, but enough replicas that partitions @@ -198,31 +201,44 @@ async fn twins_network_wo_twins_wo_partitions() { /// /// This should be a sanity check that without Byzantine behaviour the consensus /// is resilient to temporary network partitions. +#[test_casing(5,0..5)] #[tokio::test] -async fn twins_network_wo_twins_w_partitions() { +async fn twins_network_wo_twins_w_partitions(num_reseeds: usize) { tokio::time::pause(); // n=6 implies f=1 and q=5; 6 is the minimum where partitions are possible. - run_twins(6, 0, 5).await.unwrap(); + run_twins(6, 0, TwinsScenarios::Reseeds(num_reseeds)) + .await + .unwrap(); } +/// Test cases with 1 twin, with 6-10 replicas, 10 scenarios each. +const CASES_TWINS_1: TestCases<(usize, usize)> = cases! { + (6..=10).flat_map(|num_replicas| (0..10).map(move |num_reseeds| (num_replicas, num_reseeds))) +}; + /// Run Twins scenarios with random number of nodes and 1 twin. -#[test_casing(5, 6..=10)] +#[test_casing(50, CASES_TWINS_1)] #[tokio::test] -async fn twins_network_w1_twins_w_partitions(num_replicas: usize) { +async fn twins_network_w1_twins_w_partitions(num_replicas: usize, num_reseeds: usize) { tokio::time::pause(); // n>=6 implies f>=1 and q=n-f // let num_honest = validator::threshold(num_replicas as u64) as usize; // let max_faulty = num_replicas - num_honest; // let num_twins = rng.gen_range(1..=max_faulty); - run_twins(num_replicas, 1, 10).await.unwrap(); + run_twins(num_replicas, 1, TwinsScenarios::Reseeds(num_reseeds)) + .await + .unwrap(); } /// Run Twins scenarios with higher number of nodes and 2 twins. +#[test_casing(5,0..5)] #[tokio::test] -async fn twins_network_w2_twins_w_partitions() { +async fn twins_network_w2_twins_w_partitions(num_reseeds: usize) { tokio::time::pause(); // n>=11 implies f>=2 and q=n-f - run_twins(11, 2, 8).await.unwrap(); + run_twins(11, 2, TwinsScenarios::Reseeds(num_reseeds)) + .await + .unwrap(); } /// Run Twins scenario with more twins than tolerable and expect it to fail. @@ -230,21 +246,35 @@ async fn twins_network_w2_twins_w_partitions() { async fn twins_network_to_fail() { tokio::time::pause(); // With n=5 f=0, so 1 twin means more faulty nodes than expected. - assert_matches!(run_twins(5, 1, 100).await, Err(TestError::BlockConflict)); + assert_matches!( + run_twins(5, 1, TwinsScenarios::Multiple(100)).await, + Err(TestError::BlockConflict) + ); +} + +/// Govern how many scenarios to execute in the test. +enum TwinsScenarios { + /// Execute N scenarios in a loop. + /// + /// Use this when looking for a counter example, ie. a scenario where consensus fails. + Multiple(usize), + /// Execute 1 scenario after doing N reseeds of the RNG. + /// + /// Use this with the `#[test_casing]` macro to turn scenarios into separate test cases. + Reseeds(usize), } -/// Create network configuration for a given number of replicas and twins and run [Test]. +/// Create network configuration for a given number of replicas and twins and run [Test], async fn run_twins( num_replicas: usize, num_twins: usize, - num_scenarios: usize, + scenarios: TwinsScenarios, ) -> Result<(), TestError> { zksync_concurrency::testonly::abort_on_panic(); - // Use a single timeout for all scenarios to finish. // A single scenario with 11 replicas took 3-5 seconds. // Panic on timeout; works with `cargo nextest` and the `abort_on_panic` above. - let _guard = zksync_concurrency::testonly::set_timeout(time::Duration::seconds(60)); + let _guard = zksync_concurrency::testonly::set_timeout(time::Duration::seconds(30)); let ctx = &ctx::test_root(&ctx::RealClock); #[derive(PartialEq, Debug)] @@ -272,7 +302,17 @@ async fn run_twins( } } - let rng = &mut ctx.rng(); + let (num_scenarios, num_reseeds) = match scenarios { + TwinsScenarios::Multiple(n) => (n, 0), + TwinsScenarios::Reseeds(n) => (1, n), + }; + + // Keep scenarios separate by generating a different RNG many times. + let mut rng = ctx.rng(); + for _ in 0..num_reseeds { + rng = ctx.rng(); + } + let rng = &mut rng; // The existing test machinery uses the number of finalized blocks as an exit criteria. let blocks_to_finalize = 3;