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 <oxarbitrage@gmail.com>

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::<JoinSplit::<Groth16Proof>>::arbitrary(),
         joinsplit_data in TypeNameToDebug::<JoinSplitData::<Groth16Proof>>::arbitrary(),
         use_finalized_state in any::<bool>(),
@@ -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::<sapling::Spend::<PerSpendAnchor>>::arbitrary(),
         sapling_shielded_data in TypeNameToDebug::<sapling::ShieldedData::<PerSpendAnchor>>::arbitrary(),
         use_finalized_state in any::<bool>(),
@@ -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::<orchard::AuthorizedAction>::arbitrary(),
         orchard_shielded_data in TypeNameToDebug::<orchard::ShieldedData>::arbitrary(),
         use_finalized_state in any::<bool>(),
@@ -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();

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))

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]

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(