Remove unreliable generated chain prevouts tests (#2548)

And adjust the chain lengths for better coverage.

zebra-chain/src/block/arbitrary.rs

@@ -22,23 +22,25 @@ use crate::{
 
 use super::*;
 
-/// The chain height used to test for prevout inputs.
+/// The chain length for zebra-chain proptests.
 ///
-/// This impacts the probability of `has_prevouts` failures in
+/// Most generated chains will contain transparent spends at or before this height.
-/// `arbitrary_height_partial_chain_strategy`.
 ///
-/// The failure probability calculation is:
+/// This height was chosen a tradeoff between chains with no spends,
+/// and chains which spend outputs created by previous spends.
+///
+/// The raw probability of having no spends during a test run is:
 /// ```text
 /// shielded_input = shielded_pool_count / pool_count
 /// expected_transactions = expected_inputs = MAX_ARBITRARY_ITEMS/2
-/// proptest_cases = 256
+/// shielded_input^(expected_transactions * expected_inputs * (PREVOUTS_CHAIN_HEIGHT - 1))
-/// number_of_proptests = 5 as of July 2021 (PREVOUTS_CHAIN_HEIGHT and PartialChain tests)
-/// shielded_input^(expected_transactions * expected_inputs * PREVOUTS_CHAIN_HEIGHT) * proptest_cases * number_of_proptests
 /// ```
 ///
-/// `PREVOUTS_CHAIN_HEIGHT` should be increased, and `proptest_cases` should be reduced,
+/// This probability is approximately 3%. However, proptest generation and
-/// so that the failure probability is less than 1 in 1 million.
+/// minimisation strategies can create additional chains with no transparent spends.
-pub const PREVOUTS_CHAIN_HEIGHT: usize = 20;
+///
+/// To increase the proportion of test runs with proptest spends, increase `PREVOUTS_CHAIN_HEIGHT`.
+pub const PREVOUTS_CHAIN_HEIGHT: usize = 4;
 
 #[derive(Debug, Clone, Copy)]
 #[non_exhaustive]
@@ -400,8 +402,10 @@ impl Block {
                 block.transactions = new_transactions;
 
                 // TODO: if needed, fixup:
-                // - transaction output counts (currently 0..=16, consensus rules require 1..)
+                // - transparent values and shielded value balances
+                // - transaction outputs (currently 0..=16 outputs, consensus rules require 1..)
                 // - history and authorizing data commitments
+                // - the transaction merkle root
 
                 // now that we've made all the changes, calculate our block hash,
                 // so the next block can use it

zebra-chain/src/block/tests/prop.rs

@@ -7,7 +7,6 @@ use zebra_test::prelude::*;
 use crate::{
     parameters::{Network, GENESIS_PREVIOUS_BLOCK_HASH},
     serialization::{SerializationError, ZcashDeserializeInto, ZcashSerialize},
-    transaction::arbitrary::MAX_ARBITRARY_ITEMS,
     LedgerState,
 };
 
@@ -172,7 +171,7 @@ fn genesis_partial_chain_strategy() -> Result<()> {
     let strategy = LedgerState::genesis_strategy(None, None, false).prop_flat_map(|init| {
         Block::partial_chain_strategy(
             init,
-            MAX_ARBITRARY_ITEMS,
+            PREVOUTS_CHAIN_HEIGHT,
             allow_all_transparent_coinbase_spends,
         )
     });
@@ -212,7 +211,6 @@ fn genesis_partial_chain_strategy() -> Result<()> {
 /// Make sure our block height strategy generates a chain with:
 /// - correct coinbase heights
 /// - correct previous block hashes
-/// - at least one transparent PrevOut input in the entire chain
 #[test]
 fn arbitrary_height_partial_chain_strategy() -> Result<()> {
     zebra_test::init();
@@ -230,7 +228,6 @@ fn arbitrary_height_partial_chain_strategy() -> Result<()> {
     proptest!(|(chain in strategy)| {
         let mut height = None;
         let mut previous_block_hash = None;
-        let mut has_prevouts = false;
 
         for block in chain {
             if height.is_none() {
@@ -242,18 +239,8 @@ fn arbitrary_height_partial_chain_strategy() -> Result<()> {
                 prop_assert_eq!(Some(block.header.previous_block_hash), previous_block_hash);
             }
 
-            has_prevouts |= block
-                .transactions
-                .iter()
-                .flat_map(|t| t.inputs())
-                .find_map(|i| i.outpoint())
-                .is_some();
-
             previous_block_hash = Some(block.hash());
         }
-
-        // this assertion checks that we're covering transparent spends
-        prop_assert!(has_prevouts, "unexpected missing prevouts in entire chain");
     });
 
     Ok(())

zebra-state/src/service/arbitrary.rs

@@ -20,7 +20,10 @@ use super::*;
 
 /// The chain length for state proptests.
 ///
-/// Shorter lengths increase the probability of proptest failures.
+/// Most generated chains will contain transparent spends at or before this height.
+///
+/// This height was chosen as a tradeoff between chains with no transparent spends,
+/// and chains which spend outputs created by previous spends.
 ///
 /// See [`block::arbitrary::PREVOUTS_CHAIN_HEIGHT`] for details.
 pub const MAX_PARTIAL_CHAIN_BLOCKS: usize =

zebra-state/src/service/non_finalized_state/tests/prop.rs

@@ -25,7 +25,6 @@ const DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES: u32 = 1;
 ///
 /// Also check for:
 /// - no transparent spends in the genesis block, because genesis transparent outputs are ignored
-/// - at least one transparent PrevOut input in the entire chain
 #[test]
 fn forked_equals_pushed() -> Result<()> {
     zebra_test::init();
@@ -40,7 +39,6 @@ fn forked_equals_pushed() -> Result<()> {
 
             let mut full_chain = Chain::new(Default::default(), Default::default());
             let mut partial_chain = Chain::new(Default::default(), Default::default());
-            let mut has_prevouts = false;
 
             for block in chain.iter().take(fork_at_count) {
                 partial_chain = partial_chain.push(block.clone())?;
@@ -59,18 +57,12 @@ fn forked_equals_pushed() -> Result<()> {
                             .filter_map(|i| i.outpoint())
                             .count(),
                         0,
-                        "unexpected transparent prevout inputs at height {:?}: genesis transparent outputs are ignored",
+                        "unexpected transparent prevout input at height {:?}: \
+                         genesis transparent outputs must be ignored, \
+                         so there can not be any spends in the genesis block",
                         block.height,
                     );
                 }
-
-                has_prevouts |= block
-                    .block
-                    .transactions
-                    .iter()
-                    .flat_map(|t| t.inputs())
-                    .find_map(|i| i.outpoint())
-                    .is_some();
             }
 
             let forked = full_chain
@@ -85,10 +77,6 @@ fn forked_equals_pushed() -> Result<()> {
             // the first check is redundant, but it's useful for debugging
             prop_assert_eq!(forked.blocks.len(), partial_chain.blocks.len());
             prop_assert!(forked.eq_internal_state(&partial_chain));
-
-            // this assertion checks that we're still generating some transparent spends,
-            // after proptests remove unshielded and immature transparent coinbase spends
-            prop_assert!(has_prevouts, "no blocks in chain had prevouts");
         });
 
     Ok(())