From 236388909e1a66ecdc5a4aa0f5e982e4e2f36ba7 Mon Sep 17 00:00:00 2001 From: teor Date: Sat, 24 Jul 2021 00:27:10 +1000 Subject: [PATCH] Make nullifier tests faster and consistent with UTXO tests (#2513) * Make some NonFinalizedState methods test-only * Rename nullifier tests for clarity * Reduce test times by reducing default proptest cases The state tests should be about 4x faster after these changes. They reduce total state test "user CPU" time to 20-30 seconds on my machine. Previously it was around 2 minutes. * Replace multiple pushes with extend Co-authored-by: Alfredo Garcia --- .../src/service/check/tests/nullifier.rs | 38 +++++++++---------- .../src/service/non_finalized_state.rs | 11 +++--- .../service/non_finalized_state/tests/prop.rs | 2 +- zebra-state/src/service/tests.rs | 19 ++++------ 4 files changed, 32 insertions(+), 38 deletions(-) diff --git a/zebra-state/src/service/check/tests/nullifier.rs b/zebra-state/src/service/check/tests/nullifier.rs index 66ff781d0..735c2fa60 100644 --- a/zebra-state/src/service/check/tests/nullifier.rs +++ b/zebra-state/src/service/check/tests/nullifier.rs @@ -1,6 +1,6 @@ //! Randomised property tests for nullifier contextual validation -use std::{convert::TryInto, sync::Arc}; +use std::{convert::TryInto, env, sync::Arc}; use itertools::Itertools; use proptest::prelude::*; @@ -34,15 +34,24 @@ use crate::{ // because we're only interested in spend validation, // (and passing various other state checks). -// sprout +const DEFAULT_NULLIFIER_PROPTEST_CASES: u32 = 16; proptest! { + #![proptest_config( + proptest::test_runner::Config::with_cases(env::var("PROPTEST_CASES") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(DEFAULT_NULLIFIER_PROPTEST_CASES)) + )] + +// sprout + /// Make sure an arbitrary sprout nullifier is accepted by state contextual validation. /// /// This test makes sure there are no spurious rejections that might hide bugs in the other tests. /// (And that the test infrastructure generally works.) #[test] - fn accept_distinct_arbitrary_sprout_nullifiers( + fn accept_distinct_arbitrary_sprout_nullifiers_in_one_block( mut joinsplit in TypeNameToDebug::>::arbitrary(), joinsplit_data in TypeNameToDebug::>::arbitrary(), use_finalized_state in any::(), @@ -230,10 +239,7 @@ proptest! { block1 .transactions - .push(transaction1.into()); - block1 - .transactions - .push(transaction2.into()); + .extend([transaction1.into(), transaction2.into()]); let (mut state, genesis) = new_state_with_mainnet_genesis(); let previous_mem = state.mem.clone(); @@ -341,17 +347,15 @@ proptest! { prop_assert_eq!(Some((Height(1), block1_hash)), state.best_tip()); prop_assert!(state.mem.eq_internal_state(&previous_mem)); } -} // sapling -proptest! { /// Make sure an arbitrary sapling nullifier is accepted by state contextual validation. /// /// This test makes sure there are no spurious rejections that might hide bugs in the other tests. /// (And that the test infrastructure generally works.) #[test] - fn accept_distinct_arbitrary_sapling_nullifiers( + fn accept_distinct_arbitrary_sapling_nullifiers_in_one_block( spend in TypeNameToDebug::>::arbitrary(), sapling_shielded_data in TypeNameToDebug::>::arbitrary(), use_finalized_state in any::(), @@ -481,10 +485,7 @@ proptest! { block1 .transactions - .push(transaction1.into()); - block1 - .transactions - .push(transaction2.into()); + .extend([transaction1.into(), transaction2.into()]); let (mut state, genesis) = new_state_with_mainnet_genesis(); let previous_mem = state.mem.clone(); @@ -593,17 +594,15 @@ proptest! { prop_assert_eq!(Some((Height(1), block1_hash)), state.best_tip()); prop_assert!(state.mem.eq_internal_state(&previous_mem)); } -} // orchard -proptest! { /// Make sure an arbitrary orchard nullifier is accepted by state contextual validation. /// /// This test makes sure there are no spurious rejections that might hide bugs in the other tests. /// (And that the test infrastructure generally works.) #[test] - fn accept_distinct_arbitrary_orchard_nullifiers( + fn accept_distinct_arbitrary_orchard_nullifiers_in_one_block( authorized_action in TypeNameToDebug::::arbitrary(), orchard_shielded_data in TypeNameToDebug::::arbitrary(), use_finalized_state in any::(), @@ -733,10 +732,7 @@ proptest! { block1 .transactions - .push(transaction1.into()); - block1 - .transactions - .push(transaction2.into()); + .extend([transaction1.into(), transaction2.into()]); let (mut state, genesis) = new_state_with_mainnet_genesis(); let previous_mem = state.mem.clone(); diff --git a/zebra-state/src/service/non_finalized_state.rs b/zebra-state/src/service/non_finalized_state.rs index fa3009d63..e7a03ac42 100644 --- a/zebra-state/src/service/non_finalized_state.rs +++ b/zebra-state/src/service/non_finalized_state.rs @@ -14,13 +14,14 @@ use std::{collections::BTreeSet, mem, ops::Deref, sync::Arc}; use zebra_chain::{ block::{self, Block}, - orchard, parameters::Network, - sapling, sprout, transaction::{self, Transaction}, transparent, }; +#[cfg(test)] +use zebra_chain::{orchard, sapling, sprout}; + use crate::{FinalizedBlock, HashOrHeight, PreparedBlock, ValidateContextError}; use self::chain::Chain; @@ -309,7 +310,7 @@ impl NonFinalizedState { } /// Returns `true` if the best chain contains `sprout_nullifier`. - #[allow(dead_code)] + #[cfg(test)] pub fn best_contains_sprout_nullifier(&self, sprout_nullifier: &sprout::Nullifier) -> bool { self.best_chain() .map(|best_chain| best_chain.sprout_nullifiers.contains(sprout_nullifier)) @@ -317,7 +318,7 @@ impl NonFinalizedState { } /// Returns `true` if the best chain contains `sapling_nullifier`. - #[allow(dead_code)] + #[cfg(test)] pub fn best_contains_sapling_nullifier(&self, sapling_nullifier: &sapling::Nullifier) -> bool { self.best_chain() .map(|best_chain| best_chain.sapling_nullifiers.contains(sapling_nullifier)) @@ -325,7 +326,7 @@ impl NonFinalizedState { } /// Returns `true` if the best chain contains `orchard_nullifier`. - #[allow(dead_code)] + #[cfg(test)] pub fn best_contains_orchard_nullifier(&self, orchard_nullifier: &orchard::Nullifier) -> bool { self.best_chain() .map(|best_chain| best_chain.orchard_nullifiers.contains(orchard_nullifier)) diff --git a/zebra-state/src/service/non_finalized_state/tests/prop.rs b/zebra-state/src/service/non_finalized_state/tests/prop.rs index c7de35d20..6234c5168 100644 --- a/zebra-state/src/service/non_finalized_state/tests/prop.rs +++ b/zebra-state/src/service/non_finalized_state/tests/prop.rs @@ -14,7 +14,7 @@ use crate::{ Config, }; -const DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES: u32 = 32; +const DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES: u32 = 16; /// Check that a forked chain is the same as a chain that had the same blocks appended. #[test] diff --git a/zebra-state/src/service/tests.rs b/zebra-state/src/service/tests.rs index 40cfd619b..1da893f8e 100644 --- a/zebra-state/src/service/tests.rs +++ b/zebra-state/src/service/tests.rs @@ -175,17 +175,6 @@ fn state_behaves_when_blocks_are_committed_in_order() -> Result<()> { Ok(()) } -#[test] -fn state_behaves_when_blocks_are_committed_out_of_order() -> Result<()> { - zebra_test::init(); - - proptest!(|(blocks in out_of_order_committing_strategy())| { - populate_and_check(blocks).unwrap(); - }); - - Ok(()) -} - const DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES: u32 = 2; const BLOCKS_AFTER_NU5: u32 = 101; @@ -197,6 +186,14 @@ proptest! { .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)) )] + /// Test out of order commits of continuous block test vectors from genesis onward. + #[test] + fn state_behaves_when_blocks_are_committed_out_of_order(blocks in out_of_order_committing_strategy()) { + zebra_test::init(); + + populate_and_check(blocks).unwrap(); + } + /// Test blocks that are less than the NU5 activation height. #[test] fn some_block_less_than_network_upgrade(