Generate test chains that pass basic chain consistency tests (#2221)

* Set the tip height and previous hash for arbitrary genesis blocks

And cleanup the ledger strategy interface.

* Generate partial chains with correct previous block hashes

* Provide the network value from the PreparedChain strategy

zebra-chain/src/block/arbitrary.rs

@@ -6,6 +6,7 @@ use proptest::{
 use std::sync::Arc;
 
 use crate::{
+    block,
     parameters::{Network, NetworkUpgrade, GENESIS_PREVIOUS_BLOCK_HASH},
     serialization,
     work::{difficulty::CompactDifficulty, equihash},
@@ -17,21 +18,21 @@ use super::*;
 #[non_exhaustive]
 /// The configuration data for proptest when generating arbitrary chains
 pub struct LedgerState {
-    /// The tip height of the block or start of the chain.
+    /// The height of the generated block, or the start height of the generated chain.
     ///
     /// To get the network upgrade, use the `network_upgrade` method.
     ///
     /// If `network_upgrade_override` is not set, the network upgrade is derived
-    /// from the height and network.
+    /// from the `height` and `network`.
-    pub tip_height: Height,
+    pub height: Height,
 
     /// The network to generate fake blocks for.
     pub network: Network,
 
-    /// Overrides the network upgrade calculated from `tip_height` and `network`.
+    /// Overrides the network upgrade calculated from `height` and `network`.
     ///
     /// To get the network upgrade, use the `network_upgrade` method.
-    pub network_upgrade_override: Option<NetworkUpgrade>,
+    network_upgrade_override: Option<NetworkUpgrade>,
 
     /// Generate coinbase transactions.
     ///
@@ -42,94 +43,181 @@ pub struct LedgerState {
     /// transaction.
     pub(crate) has_coinbase: bool,
 
-    /// Should this block have a genesis (all-zeroes) previous block hash?
+    /// Overrides the previous block hashes in blocks generated by this ledger.
-    ///
+    previous_block_hash_override: Option<block::Hash>,
-    /// In Zebra's proptests, the previous block hash can be overriden with
+}
-    /// genesis at any block height.
+
-    genesis_previous_block_hash_override: bool,
+/// Overrides for arbitrary [`LedgerState`]s.
+#[derive(Debug, Clone, Copy)]
+pub struct LedgerStateOverride {
+    /// Regardless of tip height and network, every block has features from this
+    /// network upgrade.
+    pub network_upgrade_override: Option<NetworkUpgrade>,
+
+    /// Every block has exactly one coinbase transaction.
+    /// Transactions are always coinbase transactions.
+    pub always_has_coinbase: bool,
+
+    /// Every chain starts at this block. Single blocks have this height.
+    pub height_override: Option<Height>,
+
+    /// Every chain starts with a block with this previous block hash.
+    /// Single blocks have this previous block hash.
+    pub previous_block_hash_override: Option<block::Hash>,
 }
 
 impl LedgerState {
-    /// Returns the network upgrade for this ledger state.
+    /// Returns the default strategy for creating arbitrary `LedgerState`s.
-    ///
+    pub fn default_strategy() -> BoxedStrategy<Self> {
-    /// If `network_upgrade_override` is set, it replaces the upgrade calculated
+        Self::arbitrary_with(LedgerStateOverride::default())
-    /// using `tip_height` and `network`.
-    pub fn network_upgrade(&self) -> NetworkUpgrade {
-        if let Some(network_upgrade_override) = self.network_upgrade_override {
-            network_upgrade_override
-        } else {
-            NetworkUpgrade::current(self.network, self.tip_height)
-        }
     }
 
-    /// Should this block have a genesis (all-zeroes) previous block hash?
+    /// Returns a strategy for creating arbitrary `LedgerState`s, without any
+    /// overrides.
+    pub fn no_override_strategy() -> BoxedStrategy<Self> {
+        Self::arbitrary_with(LedgerStateOverride {
+            network_upgrade_override: None,
+            always_has_coinbase: false,
+            height_override: None,
+            previous_block_hash_override: None,
+        })
+    }
+
+    /// Returns a strategy for creating `LedgerState`s with features from
+    /// `network_upgrade_override`.
     ///
-    /// In Zebra's proptests, the previous block hash can be overriden with
+    /// These featues ignore the actual tip height and network).
-    /// genesis at any block height.
+    pub fn network_upgrade_strategy(
-    pub fn use_genesis_previous_block_hash(&self) -> bool {
+        network_upgrade_override: NetworkUpgrade,
-        self.tip_height == Height(0) || self.genesis_previous_block_hash_override
+    ) -> BoxedStrategy<Self> {
+        Self::arbitrary_with(LedgerStateOverride {
+            network_upgrade_override: Some(network_upgrade_override),
+            always_has_coinbase: false,
+            height_override: None,
+            previous_block_hash_override: None,
+        })
     }
 
     /// Returns a strategy for creating `LedgerState`s that always have coinbase
     /// transactions.
-    pub fn coinbase_strategy() -> BoxedStrategy<Self> {
+    ///
-        Self::arbitrary_with(true)
+    /// Also applies `network_upgrade_override`, if present.
+    pub fn coinbase_strategy(
+        network_upgrade_override: impl Into<Option<NetworkUpgrade>>,
+    ) -> BoxedStrategy<Self> {
+        Self::arbitrary_with(LedgerStateOverride {
+            network_upgrade_override: network_upgrade_override.into(),
+            always_has_coinbase: true,
+            height_override: None,
+            previous_block_hash_override: None,
+        })
+    }
+
+    /// Returns a strategy for creating `LedgerState`s that start with a genesis
+    /// block.
+    ///
+    /// These strategies also have coinbase transactions, and an optional network
+    /// upgrade override.
+    ///
+    /// Use the `Genesis` network upgrade to get a random genesis block, with
+    /// Zcash genesis features.
+    pub fn genesis_strategy(
+        network_upgrade_override: impl Into<Option<NetworkUpgrade>>,
+    ) -> BoxedStrategy<Self> {
+        Self::arbitrary_with(LedgerStateOverride {
+            network_upgrade_override: network_upgrade_override.into(),
+            always_has_coinbase: true,
+            height_override: Some(Height(0)),
+            previous_block_hash_override: Some(GENESIS_PREVIOUS_BLOCK_HASH),
+        })
+    }
+
+    /// Returns the network upgrade for this ledger state.
+    ///
+    /// If `network_upgrade_override` is set, it replaces the upgrade calculated
+    /// using `height` and `network`.
+    pub fn network_upgrade(&self) -> NetworkUpgrade {
+        if let Some(network_upgrade_override) = self.network_upgrade_override {
+            network_upgrade_override
+        } else {
+            NetworkUpgrade::current(self.network, self.height)
+        }
     }
 }
 
 impl Default for LedgerState {
     fn default() -> Self {
-        let network = Network::Mainnet;
+        // TODO: stop having a default network
-        let most_recent_nu = NetworkUpgrade::current(network, Height::MAX);
+        let default_network = Network::default();
-        let most_recent_activation_height = most_recent_nu.activation_height(network).unwrap();
+        let default_override = LedgerStateOverride::default();
+
+        let most_recent_nu = NetworkUpgrade::current(default_network, Height::MAX);
+        let most_recent_activation_height =
+            most_recent_nu.activation_height(default_network).unwrap();
+
+        Self {
+            height: most_recent_activation_height,
+            network: default_network,
+            network_upgrade_override: default_override.network_upgrade_override,
+            has_coinbase: default_override.always_has_coinbase,
+            previous_block_hash_override: default_override.previous_block_hash_override,
+        }
+    }
+}
+
+impl Default for LedgerStateOverride {
+    fn default() -> Self {
+        let default_network = Network::default();
 
         // TODO: dynamically select any future network upgrade (#1974)
-        let nu5_activation_height = NetworkUpgrade::Nu5.activation_height(network);
+        let nu5_activation_height = NetworkUpgrade::Nu5.activation_height(default_network);
         let nu5_override = if nu5_activation_height.is_some() {
             None
         } else {
             Some(NetworkUpgrade::Nu5)
         };
 
-        Self {
+        LedgerStateOverride {
-            tip_height: most_recent_activation_height,
-            network,
             network_upgrade_override: nu5_override,
-            has_coinbase: true,
+            always_has_coinbase: true,
-            // start each chain with a genesis previous block hash, regardless of height
+            height_override: None,
-            genesis_previous_block_hash_override: true,
+            previous_block_hash_override: None,
         }
     }
 }
 
 impl Arbitrary for LedgerState {
-    type Parameters = bool;
+    type Parameters = LedgerStateOverride;
 
     /// Generate an arbitrary `LedgerState`.
     ///
-    /// The default strategy arbitrarily skips some coinbase transactions. To
+    /// The default strategy arbitrarily skips some coinbase transactions, and
-    /// override, use `LedgerState::coinbase_strategy`.
+    /// has an arbitrary start height. To override, use:
-    fn arbitrary_with(require_coinbase: Self::Parameters) -> Self::Strategy {
+    /// - [`LedgerState::coinbase_strategy`], or
+    /// - [`LedgerState::genesis_strategy`].
+    fn arbitrary_with(ledger_override: Self::Parameters) -> Self::Strategy {
         (
             any::<Height>(),
             any::<Network>(),
             any::<bool>(),
             any::<bool>(),
         )
-            .prop_map(move |(tip_height, network, nu5_override, has_coinbase)| {
+            .prop_map(move |(height, network, nu5_override, has_coinbase)| {
                 // TODO: dynamically select any future network upgrade (#1974)
-                let network_upgrade_override = if nu5_override {
+                let nu5_override = if nu5_override {
                     Some(NetworkUpgrade::Nu5)
                 } else {
                     None
                 };
 
                 LedgerState {
-                    tip_height,
+                    height: ledger_override.height_override.unwrap_or(height),
                     network,
-                    network_upgrade_override,
+                    network_upgrade_override: ledger_override
-                    has_coinbase: require_coinbase || has_coinbase,
+                        .network_upgrade_override
-                    genesis_previous_block_hash_override: true,
+                        .or(nu5_override),
+                    has_coinbase: ledger_override.always_has_coinbase || has_coinbase,
+                    previous_block_hash_override: ledger_override.previous_block_hash_override,
                 }
             })
             .boxed()
@@ -144,15 +232,10 @@ impl Arbitrary for Block {
     fn arbitrary_with(ledger_state: Self::Parameters) -> Self::Strategy {
         let transactions_strategy = Transaction::vec_strategy(ledger_state, 2);
 
-        (any::<Header>(), transactions_strategy)
+        (Header::arbitrary_with(ledger_state), transactions_strategy)
-            .prop_map(move |(mut header, transactions)| {
+            .prop_map(move |(header, transactions)| Self {
-                if ledger_state.genesis_previous_block_hash_override {
+                header,
-                    header.previous_block_hash = GENESIS_PREVIOUS_BLOCK_HASH;
+                transactions,
-                }
-                Self {
-                    header,
-                    transactions,
-                }
             })
             .boxed()
     }
@@ -164,18 +247,29 @@ impl Block {
     /// Returns a strategy for creating Vecs of blocks with increasing height of
     /// the given length.
     pub fn partial_chain_strategy(
-        init: LedgerState,
+        mut current: LedgerState,
         count: usize,
     ) -> BoxedStrategy<Vec<Arc<Self>>> {
-        let mut current = init;
         let mut vec = Vec::with_capacity(count);
+
+        // generate block strategies with the correct heights
         for _ in 0..count {
-            vec.push(Block::arbitrary_with(current).prop_map(Arc::new));
+            vec.push(Block::arbitrary_with(current));
-            current.tip_height.0 += 1;
+            current.height.0 += 1;
-            current.genesis_previous_block_hash_override = false;
         }
 
-        vec.boxed()
+        // after the vec strategy generates blocks, update the previous block hashes
+        vec.prop_map(|mut vec| {
+            let mut previous_block_hash = None;
+            for block in vec.iter_mut() {
+                if let Some(previous_block_hash) = previous_block_hash {
+                    block.header.previous_block_hash = previous_block_hash;
+                }
+                previous_block_hash = Some(block.hash());
+            }
+            vec.into_iter().map(Arc::new).collect()
+        })
+        .boxed()
     }
 }
 
@@ -203,9 +297,9 @@ impl Arbitrary for Commitment {
 }
 
 impl Arbitrary for Header {
-    type Parameters = ();
+    type Parameters = LedgerState;
 
-    fn arbitrary_with(_args: ()) -> Self::Strategy {
+    fn arbitrary_with(ledger_state: Self::Parameters) -> Self::Strategy {
         (
             // version is interpreted as i32 in the spec, so we are limited to i32::MAX here
             (4u32..(i32::MAX as u32)),
@@ -218,24 +312,34 @@ impl Arbitrary for Header {
             any::<equihash::Solution>(),
         )
             .prop_map(
-                |(
+                move |(
                     version,
-                    previous_block_hash,
+                    mut previous_block_hash,
-                    merkle_root,
-                    commitment_bytes,
-                    time,
-                    difficulty_threshold,
-                    nonce,
-                    solution,
-                )| Header {
-                    version,
-                    previous_block_hash,
                     merkle_root,
                     commitment_bytes,
                     time,
                     difficulty_threshold,
                     nonce,
                     solution,
+                )| {
+                    if let Some(previous_block_hash_override) =
+                        ledger_state.previous_block_hash_override
+                    {
+                        previous_block_hash = previous_block_hash_override;
+                    } else if ledger_state.height == Height(0) {
+                        previous_block_hash = GENESIS_PREVIOUS_BLOCK_HASH;
+                    }
+
+                    Header {
+                        version,
+                        previous_block_hash,
+                        merkle_root,
+                        commitment_bytes,
+                        time,
+                        difficulty_threshold,
+                        nonce,
+                        solution,
+                    }
                 },
             )
             .boxed()

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

@@ -51,7 +51,7 @@ proptest! {
     /// Confirm that each counted header takes at least COUNTED_HEADER_LEN bytes when serialized.
     /// This verifies that our calculated [`TrustedPreallocate::max_allocation`] is indeed an upper bound.
     #[test]
-    fn counted_header_min_length(header in Header::arbitrary_with(()), transaction_count in (0..MAX_BLOCK_BYTES)) {
+    fn counted_header_min_length(header in any::<Header>(), transaction_count in (0..MAX_BLOCK_BYTES)) {
         let header = CountedHeader {
             header,
             transaction_count: transaction_count.try_into().expect("Must run test on platform with at least 32 bit address space"),
@@ -68,7 +68,7 @@ proptest! {
     /// 1. The smallest disallowed vector of `CountedHeaders`s is too large to send via the Zcash Wire Protocol
     /// 2. The largest allowed vector is small enough to fit in a legal Zcash Wire Protocol message
     #[test]
-    fn counted_header_max_allocation(header in Header::arbitrary_with(())) {
+    fn counted_header_max_allocation(header in any::<Header>()) {
         let header = CountedHeader {
             header,
             transaction_count: 0,

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

@@ -5,7 +5,10 @@ use proptest::{arbitrary::any, prelude::*, test_runner::Config};
 use zebra_test::prelude::*;
 
 use crate::serialization::{SerializationError, ZcashDeserializeInto, ZcashSerialize};
-use crate::{parameters::Network, LedgerState};
+use crate::{
+    parameters::{Network, GENESIS_PREVIOUS_BLOCK_HASH},
+    LedgerState,
+};
 
 use super::super::{serialize::MAX_BLOCK_BYTES, *};
 
@@ -117,11 +120,15 @@ proptest! {
     }
 }
 
+/// Test [`Block::coinbase_height`].
+///
+/// Also makes sure our coinbase strategy correctly generates blocks with
+/// coinbase transactions.
 #[test]
 fn blocks_have_coinbase() -> Result<()> {
     zebra_test::init();
 
-    let strategy = LedgerState::coinbase_strategy().prop_flat_map(Block::arbitrary_with);
+    let strategy = LedgerState::coinbase_strategy(None).prop_flat_map(Block::arbitrary_with);
 
     proptest!(|(block in strategy)| {
         let has_coinbase = block.coinbase_height().is_some();
@@ -130,3 +137,42 @@ fn blocks_have_coinbase() -> Result<()> {
 
     Ok(())
 }
+
+/// Make sure our genesis strategy generates blocks with the correct coinbase
+/// height and previous block hash.
+#[test]
+fn block_genesis_strategy() -> Result<()> {
+    zebra_test::init();
+
+    let strategy = LedgerState::genesis_strategy(None).prop_flat_map(Block::arbitrary_with);
+
+    proptest!(|(block in strategy)| {
+        prop_assert_eq!(block.coinbase_height(), Some(Height(0)));
+        prop_assert_eq!(block.header.previous_block_hash, GENESIS_PREVIOUS_BLOCK_HASH);
+    });
+
+    Ok(())
+}
+
+/// Make sure our partial chain strategy generates a chain with the correct coinbase
+/// heights and previous block hashes.
+#[test]
+fn partial_chain_strategy() -> Result<()> {
+    zebra_test::init();
+
+    let strategy = LedgerState::genesis_strategy(None)
+        .prop_flat_map(|init| Block::partial_chain_strategy(init, 3));
+
+    proptest!(|(chain in strategy)| {
+        let mut height = Height(0);
+        let mut previous_block_hash = GENESIS_PREVIOUS_BLOCK_HASH;
+        for block in chain {
+            prop_assert_eq!(block.coinbase_height(), Some(height));
+            prop_assert_eq!(block.header.previous_block_hash, previous_block_hash);
+            height = Height(height.0 + 1);
+            previous_block_hash = block.hash();
+        }
+    });
+
+    Ok(())
+}

zebra-chain/src/transparent/arbitrary.rs

@@ -8,8 +8,7 @@ impl Input {
     /// Construct a strategy for creating valid-ish vecs of Inputs.
     pub fn vec_strategy(ledger_state: LedgerState, max_size: usize) -> BoxedStrategy<Vec<Self>> {
         if ledger_state.has_coinbase {
-            let height = block::Height(ledger_state.tip_height.0 + 1);
+            Self::arbitrary_with(Some(ledger_state.height))
-            Self::arbitrary_with(Some(height))
                 .prop_map(|input| vec![input])
                 .boxed()
         } else {

zebra-chain/src/transparent/prop.rs

@@ -24,7 +24,7 @@ fn input_coinbase_vecs_only_have_coinbase_input() -> Result<()> {
     zebra_test::init();
 
     let max_size = 100;
-    let strategy = LedgerState::coinbase_strategy()
+    let strategy = LedgerState::coinbase_strategy(None)
         .prop_flat_map(|ledger_state| Input::vec_strategy(ledger_state, max_size));
 
     proptest!(|(inputs in strategy)| {

zebra-state/src/service/non_finalized_state/arbitrary.rs

@@ -12,19 +12,24 @@ use crate::tests::Prepare;
 
 use super::*;
 
-const MAX_PARTIAL_CHAIN_BLOCKS: usize = 100;
+const MAX_PARTIAL_CHAIN_BLOCKS: usize = 102;
 
 #[derive(Debug)]
 pub struct PreparedChainTree {
     chain: Arc<Vec<PreparedBlock>>,
     count: BinarySearch,
+    network: Network,
 }
 
 impl ValueTree for PreparedChainTree {
-    type Value = (Arc<Vec<PreparedBlock>>, <BinarySearch as ValueTree>::Value);
+    type Value = (
+        Arc<Vec<PreparedBlock>>,
+        <BinarySearch as ValueTree>::Value,
+        Network,
+    );
 
     fn current(&self) -> Self::Value {
-        (self.chain.clone(), self.count.current())
+        (self.chain.clone(), self.count.current(), self.network)
     }
 
     fn simplify(&mut self) -> bool {
@@ -39,7 +44,7 @@ impl ValueTree for PreparedChainTree {
 #[derive(Debug, Default)]
 pub struct PreparedChain {
     // the proptests are threaded (not async), so we want to use a threaded mutex here
-    chain: std::sync::Mutex<Option<Arc<Vec<PreparedBlock>>>>,
+    chain: std::sync::Mutex<Option<(Network, Arc<Vec<PreparedBlock>>)>>,
 }
 
 impl Strategy for PreparedChain {
@@ -49,19 +54,34 @@ impl Strategy for PreparedChain {
     fn new_tree(&self, runner: &mut TestRunner) -> NewTree<Self> {
         let mut chain = self.chain.lock().unwrap();
         if chain.is_none() {
-            // Only generate blocks from the most recent network upgrade
+            // Disable NU5 for now
-            let mut ledger_state = LedgerState::default();
+            // `genesis_strategy(None)` re-enables the default Nu5 override
-            ledger_state.network_upgrade_override = None;
+            let ledger_strategy = LedgerState::genesis_strategy(Canopy);
 
-            let blocks = Block::partial_chain_strategy(ledger_state, MAX_PARTIAL_CHAIN_BLOCKS)
+            let (network, blocks) = ledger_strategy
-                .prop_map(|vec| vec.into_iter().map(|blk| blk.prepare()).collect::<Vec<_>>())
+                .prop_flat_map(|ledger| {
+                    (
+                        Just(ledger.network),
+                        Block::partial_chain_strategy(ledger, MAX_PARTIAL_CHAIN_BLOCKS),
+                    )
+                })
+                .prop_map(|(network, vec)| {
+                    (
+                        network,
+                        vec.into_iter().map(|blk| blk.prepare()).collect::<Vec<_>>(),
+                    )
+                })
                 .new_tree(runner)?
                 .current();
-            *chain = Some(Arc::new(blocks));
+            *chain = Some((network, Arc::new(blocks)));
         }
 
         let chain = chain.clone().expect("should be generated");
-        let count = (1..chain.len()).new_tree(runner)?;
+        let count = (1..chain.1.len()).new_tree(runner)?;
-        Ok(PreparedChainTree { chain, count })
+        Ok(PreparedChainTree {
+            chain: chain.1,
+            count,
+            network: chain.0,
+        })
     }
 }

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

@@ -14,7 +14,7 @@ fn forked_equals_pushed() -> Result<()> {
                                           .ok()
                                           .and_then(|v| v.parse().ok())
                                           .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)),
-        |((chain, count) in PreparedChain::default())| {
+        |((chain, count, _network) in PreparedChain::default())| {
             let fork_tip_hash = chain[count - 1].hash;
             let mut full_chain = Chain::default();
             let mut partial_chain = Chain::default();
@@ -42,7 +42,7 @@ fn finalized_equals_pushed() -> Result<()> {
                                       .ok()
                                       .and_then(|v| v.parse().ok())
                                       .unwrap_or(DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES)),
-    |((chain, end_count) in PreparedChain::default())| {
+    |((chain, end_count, _network) in PreparedChain::default())| {
         let finalized_count = chain.len() - end_count;
         let mut full_chain = Chain::default();
         let mut partial_chain = Chain::default();