Add property test strategies for V5 transactions (#2347)

Add proptest strategies that:
- set the initial block height
- set the transaction version
- make all V5 transaction network upgrade fields valid

zebra-chain/src/block/arbitrary.rs

@@ -39,6 +39,17 @@ pub struct LedgerState {
     /// To get the network upgrade, use the `network_upgrade` method.
     network_upgrade_override: Option<NetworkUpgrade>,
 
+    /// Overrides the previous block hashes in blocks generated by this ledger.
+    previous_block_hash_override: Option<block::Hash>,
+
+    /// Regardless of tip height and network, every transaction is this version.
+    transaction_version_override: Option<u32>,
+
+    /// Every V5 and later transaction has a valid `network_upgrade` field.
+    ///
+    /// If `false`, some transactions have invalid network upgrades.
+    transaction_has_valid_network_upgrade: bool,
+
     /// Generate coinbase transactions.
     ///
     /// In a block or transaction vector, make the first transaction a coinbase
@@ -47,28 +58,33 @@ pub struct LedgerState {
     /// For an individual transaction, make the transaction a coinbase
     /// transaction.
     pub(crate) has_coinbase: bool,
-
-    /// Overrides the previous block hashes in blocks generated by this ledger.
-    previous_block_hash_override: Option<block::Hash>,
 }
 
 /// 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>,
+
+    /// Regardless of tip height and network, every block has features from this
+    /// network upgrade.
+    pub network_upgrade_override: Option<NetworkUpgrade>,
+
+    /// Regardless of tip height and network, every transaction is this version.
+    pub transaction_version_override: Option<u32>,
+
+    /// Every V5 and later transaction has a valid `network_upgrade` field.
+    ///
+    /// If `false`, some transactions have invalid network upgrades.
+    pub transaction_has_valid_network_upgrade: bool,
+
+    /// Every block has exactly one coinbase transaction.
+    /// Transactions are always coinbase transactions.
+    pub always_has_coinbase: bool,
 }
 
 impl LedgerState {
@@ -81,25 +97,31 @@ impl LedgerState {
     /// 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,
+            network_upgrade_override: None,
+            transaction_version_override: None,
+            transaction_has_valid_network_upgrade: false,
+            always_has_coinbase: false,
         })
     }
 
     /// Returns a strategy for creating `LedgerState`s with features from
     /// `network_upgrade_override`.
     ///
-    /// These featues ignore the actual tip height and network).
+    /// These featues ignore the actual tip height and network.
     pub fn network_upgrade_strategy(
         network_upgrade_override: NetworkUpgrade,
+        transaction_version_override: impl Into<Option<u32>>,
+        transaction_has_valid_network_upgrade: bool,
     ) -> 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,
+            network_upgrade_override: Some(network_upgrade_override),
+            transaction_version_override: transaction_version_override.into(),
+            transaction_has_valid_network_upgrade,
+            always_has_coinbase: false,
         })
     }
 
@@ -109,12 +131,16 @@ impl LedgerState {
     /// Also applies `network_upgrade_override`, if present.
     pub fn coinbase_strategy(
         network_upgrade_override: impl Into<Option<NetworkUpgrade>>,
+        transaction_version_override: impl Into<Option<u32>>,
+        transaction_has_valid_network_upgrade: bool,
     ) -> 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,
+            network_upgrade_override: network_upgrade_override.into(),
+            transaction_version_override: transaction_version_override.into(),
+            transaction_has_valid_network_upgrade,
+            always_has_coinbase: true,
         })
     }
 
@@ -128,12 +154,36 @@ impl LedgerState {
     /// Zcash genesis features.
     pub fn genesis_strategy(
         network_upgrade_override: impl Into<Option<NetworkUpgrade>>,
+        transaction_version_override: impl Into<Option<u32>>,
+        transaction_has_valid_network_upgrade: bool,
     ) -> 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),
+            network_upgrade_override: network_upgrade_override.into(),
+            transaction_version_override: transaction_version_override.into(),
+            transaction_has_valid_network_upgrade,
+            always_has_coinbase: true,
+        })
+    }
+
+    /// Returns a strategy for creating `LedgerState`s that start at `height`.
+    ///
+    /// These strategies also have coinbase transactions, and an optional network
+    /// upgrade override.
+    pub fn height_strategy(
+        height: Height,
+        network_upgrade_override: impl Into<Option<NetworkUpgrade>>,
+        transaction_version_override: impl Into<Option<u32>>,
+        transaction_has_valid_network_upgrade: bool,
+    ) -> BoxedStrategy<Self> {
+        Self::arbitrary_with(LedgerStateOverride {
+            height_override: Some(height),
+            previous_block_hash_override: None,
+            network_upgrade_override: network_upgrade_override.into(),
+            transaction_version_override: transaction_version_override.into(),
+            transaction_has_valid_network_upgrade,
+            always_has_coinbase: true,
         })
     }
 
@@ -148,6 +198,18 @@ impl LedgerState {
             NetworkUpgrade::current(self.network, self.height)
         }
     }
+
+    /// Returns the transaction version override.
+    pub fn transaction_version_override(&self) -> Option<u32> {
+        self.transaction_version_override
+    }
+
+    /// Returns `true` if all transactions have valid network upgrade fields.
+    ///
+    /// If `false`, some transactions have invalid network upgrades.
+    pub fn transaction_has_valid_network_upgrade(&self) -> bool {
+        self.transaction_has_valid_network_upgrade
+    }
 }
 
 impl Default for LedgerState {
@@ -160,12 +222,15 @@ impl Default for LedgerState {
         let most_recent_activation_height =
             most_recent_nu.activation_height(default_network).unwrap();
 
-        Self {
+        LedgerState {
             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,
+            transaction_version_override: default_override.transaction_version_override,
+            transaction_has_valid_network_upgrade: default_override
+                .transaction_has_valid_network_upgrade,
+            has_coinbase: default_override.always_has_coinbase,
         }
     }
 }
@@ -183,10 +248,12 @@ impl Default for LedgerStateOverride {
         };
 
         LedgerStateOverride {
-            network_upgrade_override: nu5_override,
-            always_has_coinbase: true,
             height_override: None,
             previous_block_hash_override: None,
+            network_upgrade_override: nu5_override,
+            transaction_version_override: None,
+            transaction_has_valid_network_upgrade: false,
+            always_has_coinbase: true,
         }
     }
 }
@@ -194,12 +261,11 @@ impl Default for LedgerStateOverride {
 impl Arbitrary for LedgerState {
     type Parameters = LedgerStateOverride;
 
-    /// Generate an arbitrary `LedgerState`.
+    /// Generate an arbitrary [`LedgerState`].
     ///
     /// The default strategy arbitrarily skips some coinbase transactions, and
-    /// has an arbitrary start height. To override, use:
+    /// has an arbitrary start height. To override, use a specific [`LegderState`]
-    /// - [`LedgerState::coinbase_strategy`], or
+    /// strategy method.
-    /// - [`LedgerState::genesis_strategy`].
     fn arbitrary_with(ledger_override: Self::Parameters) -> Self::Strategy {
         (
             any::<Height>(),
@@ -207,20 +273,21 @@ impl Arbitrary for LedgerState {
             any::<bool>(),
             any::<bool>(),
         )
-            .prop_map(move |(height, network, nu5_override, has_coinbase)| {
+            .prop_map(
-                // TODO: dynamically select any future network upgrade (#1974)
+                move |(height, network, transaction_has_valid_network_upgrade, has_coinbase)| {
-                let nu5_override = if nu5_override { Some(Nu5) } else { None };
+                    LedgerState {
-
+                        height: ledger_override.height_override.unwrap_or(height),
-                LedgerState {
+                        network,
-                    height: ledger_override.height_override.unwrap_or(height),
+                        network_upgrade_override: ledger_override.network_upgrade_override,
-                    network,
+                        previous_block_hash_override: ledger_override.previous_block_hash_override,
-                    network_upgrade_override: ledger_override
+                        transaction_version_override: ledger_override.transaction_version_override,
-                        .network_upgrade_override
+                        transaction_has_valid_network_upgrade: ledger_override
-                        .or(nu5_override),
+                            .transaction_has_valid_network_upgrade
-                    has_coinbase: ledger_override.always_has_coinbase || has_coinbase,
+                            || transaction_has_valid_network_upgrade,
-                    previous_block_hash_override: ledger_override.previous_block_hash_override,
+                        has_coinbase: ledger_override.always_has_coinbase || has_coinbase,
-                }
+                    }
-            })
+                },
+            )
             .boxed()
     }
 

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

@@ -1,12 +1,13 @@
-use std::env;
+use std::{env, io::ErrorKind};
-use std::io::ErrorKind;
 
 use proptest::{arbitrary::any, prelude::*, test_runner::Config};
+
 use zebra_test::prelude::*;
 
-use crate::serialization::{SerializationError, ZcashDeserializeInto, ZcashSerialize};
 use crate::{
     parameters::{Network, GENESIS_PREVIOUS_BLOCK_HASH},
+    serialization::{SerializationError, ZcashDeserializeInto, ZcashSerialize},
+    transaction::arbitrary::MAX_ARBITRARY_ITEMS,
     LedgerState,
 };
 
@@ -128,7 +129,8 @@ proptest! {
 fn blocks_have_coinbase() -> Result<()> {
     zebra_test::init();
 
-    let strategy = LedgerState::coinbase_strategy(None).prop_flat_map(Block::arbitrary_with);
+    let strategy =
+        LedgerState::coinbase_strategy(None, None, false).prop_flat_map(Block::arbitrary_with);
 
     proptest!(|(block in strategy)| {
         let has_coinbase = block.coinbase_height().is_some();
@@ -144,7 +146,8 @@ fn blocks_have_coinbase() -> Result<()> {
 fn block_genesis_strategy() -> Result<()> {
     zebra_test::init();
 
-    let strategy = LedgerState::genesis_strategy(None).prop_flat_map(Block::arbitrary_with);
+    let strategy =
+        LedgerState::genesis_strategy(None, None, false).prop_flat_map(Block::arbitrary_with);
 
     proptest!(|(block in strategy)| {
         prop_assert_eq!(block.coinbase_height(), Some(Height(0)));
@@ -160,8 +163,8 @@ fn block_genesis_strategy() -> Result<()> {
 fn partial_chain_strategy() -> Result<()> {
     zebra_test::init();
 
-    let strategy = LedgerState::genesis_strategy(None)
+    let strategy = LedgerState::genesis_strategy(None, None, false)
-        .prop_flat_map(|init| Block::partial_chain_strategy(init, 3));
+        .prop_flat_map(|init| Block::partial_chain_strategy(init, MAX_ARBITRARY_ITEMS));
 
     proptest!(|(chain in strategy)| {
         let mut height = Height(0);
@@ -176,3 +179,32 @@ fn partial_chain_strategy() -> Result<()> {
 
     Ok(())
 }
+
+/// Make sure our block height strategy generates a chain with the correct coinbase
+/// heights and hashes.
+#[test]
+fn arbitrary_height_partial_chain_strategy() -> Result<()> {
+    zebra_test::init();
+
+    let strategy = any::<Height>()
+        .prop_flat_map(|height| LedgerState::height_strategy(height, None, None, false))
+        .prop_flat_map(|init| Block::partial_chain_strategy(init, MAX_ARBITRARY_ITEMS));
+
+    proptest!(|(chain in strategy)| {
+        let mut height = None;
+        let mut previous_block_hash = None;
+        for block in chain {
+            if height.is_none() {
+                prop_assert!(block.coinbase_height().is_some());
+                height = block.coinbase_height();
+            } else {
+                height = Some(Height(height.unwrap().0 + 1));
+                prop_assert_eq!(block.coinbase_height(), height);
+                prop_assert_eq!(Some(block.header.previous_block_hash), previous_block_hash);
+            }
+            previous_block_hash = Some(block.hash());
+        }
+    });
+
+    Ok(())
+}

zebra-chain/src/transaction/arbitrary.rs

@@ -16,7 +16,7 @@ use crate::{
         redpallas::{Binding, Signature},
         Bctv14Proof, Groth16Proof, Halo2Proof, ZkSnarkProof,
     },
-    sapling,
+    sapling::{self, AnchorVariant, PerSpendAnchor, SharedAnchor},
     serialization::{ZcashDeserialize, ZcashDeserializeInto},
     sprout, transparent, LedgerState,
 };
@@ -24,7 +24,6 @@ use crate::{
 use itertools::Itertools;
 
 use super::{FieldNotPresent, JoinSplitData, LockTime, Memo, Transaction};
-use sapling::{AnchorVariant, PerSpendAnchor, SharedAnchor};
 
 /// The maximum number of arbitrary transactions, inputs, or outputs.
 ///
@@ -130,7 +129,7 @@ impl Transaction {
             option::of(any::<orchard::ShieldedData>()),
         )
             .prop_map(
-                |(
+                move |(
                     network_upgrade,
                     lock_time,
                     expiry_height,
@@ -140,7 +139,11 @@ impl Transaction {
                     orchard_shielded_data,
                 )| {
                     Transaction::V5 {
-                        network_upgrade,
+                        network_upgrade: if ledger_state.transaction_has_valid_network_upgrade() {
+                            ledger_state.network_upgrade()
+                        } else {
+                            network_upgrade
+                        },
                         lock_time,
                         expiry_height,
                         inputs,
@@ -393,6 +396,16 @@ impl Arbitrary for Transaction {
     type Parameters = LedgerState;
 
     fn arbitrary_with(ledger_state: Self::Parameters) -> Self::Strategy {
+        match ledger_state.transaction_version_override() {
+            Some(1) => return Self::v1_strategy(ledger_state),
+            Some(2) => return Self::v2_strategy(ledger_state),
+            Some(3) => return Self::v3_strategy(ledger_state),
+            Some(4) => return Self::v4_strategy(ledger_state),
+            Some(5) => return Self::v5_strategy(ledger_state),
+            Some(_) => unreachable!("invalid transaction version in override"),
+            None => {}
+        }
+
         match ledger_state.network_upgrade() {
             NetworkUpgrade::Genesis | NetworkUpgrade::BeforeOverwinter => {
                 Self::v1_strategy(ledger_state)

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

@@ -1,9 +1,19 @@
+//! Randomised property tests for transactions.
+
 use proptest::prelude::*;
+
 use std::io::Cursor;
 
+use zebra_test::prelude::*;
+
 use super::super::*;
 
-use crate::serialization::{ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize};
+use crate::{
+    block::Block,
+    serialization::{ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize},
+    transaction::arbitrary::MAX_ARBITRARY_ITEMS,
+    LedgerState,
+};
 
 proptest! {
     #[test]
@@ -44,3 +54,57 @@ proptest! {
         prop_assert_eq![locktime, other_locktime];
     }
 }
+
+/// Make sure a transaction version override generates transactions with the specified
+/// transaction versions.
+#[test]
+fn arbitrary_transaction_version_strategy() -> Result<()> {
+    zebra_test::init();
+
+    // Update with new transaction versions as needed
+    let strategy = (1..5u32)
+        .prop_flat_map(|transaction_version| {
+            LedgerState::coinbase_strategy(None, transaction_version, false)
+        })
+        .prop_flat_map(|ledger_state| Transaction::vec_strategy(ledger_state, MAX_ARBITRARY_ITEMS));
+
+    proptest!(|(transactions in strategy)| {
+        let mut version = None;
+        for t in transactions {
+            if version.is_none() {
+                version = Some(t.version());
+            } else {
+                prop_assert_eq!(Some(t.version()), version);
+            }
+        }
+    });
+
+    Ok(())
+}
+
+/// Make sure a transaction valid network upgrade strategy generates transactions
+/// with valid network upgrades.
+#[test]
+fn transaction_valid_network_upgrade_strategy() -> Result<()> {
+    zebra_test::init();
+
+    // Update with new transaction versions as needed
+    let strategy = LedgerState::coinbase_strategy(None, 5, true).prop_flat_map(|ledger_state| {
+        (
+            Just(ledger_state.network),
+            Block::arbitrary_with(ledger_state),
+        )
+    });
+
+    proptest!(|((network, block) in strategy)| {
+        // TODO: replace with check_transaction_network_upgrade from #2343
+        let block_network_upgrade = NetworkUpgrade::current(network, block.coinbase_height().unwrap());
+        for transaction in block.transactions {
+            if let Transaction::V5 { network_upgrade, .. } = transaction.as_ref() {
+                prop_assert_eq!(network_upgrade, &block_network_upgrade);
+            }
+        }
+    });
+
+    Ok(())
+}

zebra-chain/src/transparent/prop.rs

@@ -27,7 +27,7 @@ fn coinbase_has_height() -> Result<()> {
 fn input_coinbase_vecs_only_have_coinbase_input() -> Result<()> {
     zebra_test::init();
 
-    let strategy = LedgerState::coinbase_strategy(None)
+    let strategy = LedgerState::coinbase_strategy(None, None, false)
         .prop_flat_map(|ledger_state| Input::vec_strategy(ledger_state, MAX_ARBITRARY_ITEMS));
 
     proptest!(|(inputs in strategy.prop_map(SummaryDebug))| {

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

@@ -55,7 +55,7 @@ impl Strategy for PreparedChain {
         let mut chain = self.chain.lock().unwrap();
         if chain.is_none() {
             // TODO: use the latest network upgrade (#1974)
-            let ledger_strategy = LedgerState::genesis_strategy(Nu5);
+            let ledger_strategy = LedgerState::genesis_strategy(Nu5, None, false);
 
             let (network, blocks) = ledger_strategy
                 .prop_flat_map(|ledger| {