Add `proptest-impl` feature to `zebra-state` (#2529)

* Add `proptest-impl` feature to `zebra-state`

This prepares the `zebra-state` crate to be able to export some
test-specific helper types and functions.

* Add `arbitrary` module to `zebra-state` root

A separate module to contain the `Prepare` trait, since it's required by
some prop-test strategies and therefore can't be in the `tests` module.

* Replace usages of `tests::Prepare`

Use the same trait but placed in a new module that's accessible based on
the feature flag.

* Remove old `Prepare` trait

It was obsoleted by the new copy in the `arbitrary` module.

* Make `StateService` crate-accessible

Prepare for it to be accessible in some test modules.

* Refactor strategy function import

Import the function directly, instead of just its containing module.

* Move some strategy functions to `tests::setup`

Create a new module for the strategy functions that are only used
internally.

Co-authored-by: teor <teor@riseup.net>

zebra-state/Cargo.toml

@@ -5,7 +5,8 @@ authors = ["Zcash Foundation <zebra@zfnd.org>"]
 license = "MIT OR Apache-2.0"
 edition = "2018"
 
-# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+[features]
+proptest-impl = ["proptest", "zebra-test"]
 
 [dependencies]
 zebra-chain = { path = "../zebra-chain" }
@@ -28,6 +29,9 @@ tempdir = "0.3.7"
 chrono = "0.4.19"
 rlimit = "0.5.4"
 
+proptest = { version = "0.10.1", optional = true }
+zebra-test = { path = "../zebra-test/", optional = true }
+
 [dev-dependencies]
 zebra-chain = { path = "../zebra-chain", features = ["proptest-impl"] }
 zebra-test = { path = "../zebra-test/" }

zebra-state/src/arbitrary.rs

@@ -0,0 +1,28 @@
+use std::sync::Arc;
+
+use zebra_chain::{block::Block, transparent};
+
+use crate::PreparedBlock;
+
+/// Mocks computation done during semantic validation
+pub trait Prepare {
+    fn prepare(self) -> PreparedBlock;
+}
+
+impl Prepare for Arc<Block> {
+    fn prepare(self) -> PreparedBlock {
+        let block = self;
+        let hash = block.hash();
+        let height = block.coinbase_height().unwrap();
+        let transaction_hashes: Vec<_> = block.transactions.iter().map(|tx| tx.hash()).collect();
+        let new_outputs = transparent::new_ordered_outputs(&block, transaction_hashes.as_slice());
+
+        PreparedBlock {
+            block,
+            hash,
+            height,
+            new_outputs,
+            transaction_hashes,
+        }
+    }
+}

zebra-state/src/lib.rs

@@ -17,6 +17,8 @@
 #![deny(clippy::await_holding_lock)]
 #![forbid(unsafe_code)]
 
+#[cfg(any(test, feature = "proptest-impl"))]
+mod arbitrary;
 mod config;
 pub mod constants;
 mod error;

zebra-state/src/service.rs

@@ -46,9 +46,9 @@ pub type QueuedFinalized = (
     oneshot::Sender<Result<block::Hash, BoxError>>,
 );
 
-struct StateService {
+pub(crate) struct StateService {
     /// Holds data relating to finalized chain state.
-    disk: FinalizedState,
+    pub(crate) disk: FinalizedState,
     /// Holds data relating to non-finalized chain state.
     mem: NonFinalizedState,
     /// Blocks awaiting their parent blocks for contextual verification.
@@ -500,7 +500,7 @@ impl StateService {
     }
 }
 
-struct Iter<'a> {
+pub(crate) struct Iter<'a> {
     service: &'a StateService,
     state: IterState,
 }

zebra-state/src/service/arbitrary.rs

@@ -7,15 +7,9 @@ use proptest::{
     test_runner::TestRunner,
 };
 
-use zebra_chain::{
+use zebra_chain::{block::Block, fmt::SummaryDebug, parameters::NetworkUpgrade, LedgerState};
-    block::{Block, Height},
-    fmt::SummaryDebug,
-    parameters::{Network::*, NetworkUpgrade},
-    serialization::ZcashDeserializeInto,
-    LedgerState,
-};
 
-use crate::tests::Prepare;
+use crate::arbitrary::Prepare;
 
 use super::*;
 
@@ -93,101 +87,3 @@ impl Strategy for PreparedChain {
         })
     }
 }
-
-/// Generate a chain that allows us to make tests for the legacy chain rules.
-///
-/// Arguments:
-/// - `transaction_version_override`: See `LedgerState::height_strategy` for details.
-/// - `transaction_has_valid_network_upgrade`: See `LedgerState::height_strategy` for details.
-///    Note: `false` allows zero or more invalid network upgrades.
-/// - `blocks_after_nu_activation`: The number of blocks the strategy will generate
-/// after the provided `network_upgrade`.
-/// - `network_upgrade` - The network upgrade that we are using to simulate from where the
-/// legacy chain checks should start to apply.
-///
-/// Returns:
-/// A generated arbitrary strategy for the provided arguments.
-pub(crate) fn partial_nu5_chain_strategy(
-    transaction_version_override: u32,
-    transaction_has_valid_network_upgrade: bool,
-    blocks_after_nu_activation: u32,
-    // TODO: This argument can be removed and just use Nu5 after we have an activation height #1841
-    network_upgrade: NetworkUpgrade,
-) -> impl Strategy<
-    Value = (
-        Network,
-        Height,
-        zebra_chain::fmt::SummaryDebug<Vec<Arc<Block>>>,
-    ),
-> {
-    (
-        any::<Network>(),
-        NetworkUpgrade::reduced_branch_id_strategy(),
-    )
-        .prop_flat_map(move |(network, random_nu)| {
-            // TODO: update this to Nu5 after we have a height #1841
-            let mut nu = network_upgrade;
-            let nu_activation = nu.activation_height(network).unwrap();
-            let height = Height(nu_activation.0 + blocks_after_nu_activation);
-
-            // The `network_upgrade_override` will not be enough as when it is `None`,
-            // current network upgrade will be used (`NetworkUpgrade::Canopy`) which will be valid.
-            if !transaction_has_valid_network_upgrade {
-                nu = random_nu;
-            }
-
-            zebra_chain::block::LedgerState::height_strategy(
-                height,
-                Some(nu),
-                Some(transaction_version_override),
-                transaction_has_valid_network_upgrade,
-            )
-            .prop_flat_map(move |init| {
-                Block::partial_chain_strategy(init, blocks_after_nu_activation as usize)
-            })
-            .prop_map(move |partial_chain| (network, nu_activation, partial_chain))
-        })
-}
-
-/// Return a new `StateService` containing the mainnet genesis block.
-/// Also returns the finalized genesis block itself.
-pub(super) fn new_state_with_mainnet_genesis() -> (StateService, FinalizedBlock) {
-    let genesis = zebra_test::vectors::BLOCK_MAINNET_GENESIS_BYTES
-        .zcash_deserialize_into::<Arc<Block>>()
-        .expect("block should deserialize");
-
-    let mut state = StateService::new(Config::ephemeral(), Mainnet);
-
-    assert_eq!(None, state.best_tip());
-
-    let genesis = FinalizedBlock::from(genesis);
-    state
-        .disk
-        .commit_finalized_direct(genesis.clone(), "test")
-        .expect("unexpected invalid genesis block test vector");
-
-    assert_eq!(Some((Height(0), genesis.hash)), state.best_tip());
-
-    (state, genesis)
-}
-
-/// Return a `Transaction::V4` with the coinbase data from `coinbase`.
-///
-/// Used to convert a coinbase transaction to a version that the non-finalized state will accept.
-pub(super) fn transaction_v4_from_coinbase(coinbase: &Transaction) -> Transaction {
-    assert!(
-        !coinbase.has_sapling_shielded_data(),
-        "conversion assumes sapling shielded data is None"
-    );
-
-    Transaction::V4 {
-        inputs: coinbase.inputs().to_vec(),
-        outputs: coinbase.outputs().to_vec(),
-        lock_time: coinbase.lock_time(),
-        // `Height(0)` means that the expiry height is ignored
-        expiry_height: coinbase.expiry_height().unwrap_or(Height(0)),
-        // invalid for coinbase transactions
-        joinsplit_data: None,
-        sapling_shielded_data: None,
-    }
-}

zebra-state/src/service/check/tests/nullifier.rs

@@ -18,8 +18,8 @@ use zebra_chain::{
 };
 
 use crate::{
-    service::arbitrary::{new_state_with_mainnet_genesis, transaction_v4_from_coinbase},
+    arbitrary::Prepare,
-    tests::Prepare,
+    tests::setup::{new_state_with_mainnet_genesis, transaction_v4_from_coinbase},
     FinalizedBlock,
     ValidateContextError::{
         DuplicateOrchardNullifier, DuplicateSaplingNullifier, DuplicateSproutNullifier,

zebra-state/src/service/check/tests/utxo.rs

@@ -13,11 +13,9 @@ use zebra_chain::{
 };
 
 use crate::{
-    service::{
+    arbitrary::Prepare,
-        arbitrary::{new_state_with_mainnet_genesis, transaction_v4_from_coinbase},
+    service::StateService,
-        StateService,
+    tests::setup::{new_state_with_mainnet_genesis, transaction_v4_from_coinbase},
-    },
-    tests::Prepare,
     FinalizedBlock,
     ValidateContextError::{
         DuplicateTransparentSpend, EarlyTransparentSpend, MissingTransparentOutput,

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

@@ -164,7 +164,7 @@ mod tests {
     use zebra_chain::{block::Block, serialization::ZcashDeserializeInto};
     use zebra_test::prelude::*;
 
-    use crate::tests::{FakeChainHelper, Prepare};
+    use crate::{arbitrary::Prepare, tests::FakeChainHelper};
 
     use self::assert_eq;
     use super::*;

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

@@ -5,12 +5,12 @@ use zebra_test::prelude::*;
 use zebra_chain::{block::Block, fmt::DisplayToDebug, parameters::NetworkUpgrade::*, LedgerState};
 
 use crate::{
+    arbitrary::Prepare,
     service::{
         arbitrary::PreparedChain,
         finalized_state::FinalizedState,
         non_finalized_state::{Chain, NonFinalizedState},
     },
-    tests::Prepare,
     Config,
 };
 

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

@@ -4,11 +4,12 @@ use zebra_chain::{block::Block, parameters::Network, serialization::ZcashDeseria
 use zebra_test::prelude::*;
 
 use crate::{
+    arbitrary::Prepare,
     service::{
         finalized_state::FinalizedState,
         non_finalized_state::{Chain, NonFinalizedState},
     },
-    tests::{FakeChainHelper, Prepare},
+    tests::FakeChainHelper,
     Config,
 };
 

zebra-state/src/service/tests.rs

@@ -10,7 +10,7 @@ use zebra_chain::{
 };
 use zebra_test::{prelude::*, transcript::Transcript};
 
-use crate::{init, service::arbitrary, BoxError, Config, Request, Response};
+use crate::{init, tests::setup::partial_nu5_chain_strategy, BoxError, Config, Request, Response};
 
 const LAST_BLOCK_HEIGHT: u32 = 10;
 
@@ -197,7 +197,7 @@ proptest! {
     /// Test blocks that are less than the NU5 activation height.
     #[test]
     fn some_block_less_than_network_upgrade(
-        (network, nu_activation_height, chain) in arbitrary::partial_nu5_chain_strategy(4, true, BLOCKS_AFTER_NU5/2, NetworkUpgrade::Canopy)
+        (network, nu_activation_height, chain) in partial_nu5_chain_strategy(4, true, BLOCKS_AFTER_NU5/2, NetworkUpgrade::Canopy)
     ) {
         let response = crate::service::legacy_chain_check(nu_activation_height, chain.into_iter().rev(), network)
             .map_err(|error| error.to_string());
@@ -208,7 +208,7 @@ proptest! {
     /// Test the maximum amount of blocks to check before chain is declared to be legacy.
     #[test]
     fn no_transaction_with_network_upgrade(
-        (network, nu_activation_height, chain) in arbitrary::partial_nu5_chain_strategy(4, true, BLOCKS_AFTER_NU5, NetworkUpgrade::Canopy)
+        (network, nu_activation_height, chain) in partial_nu5_chain_strategy(4, true, BLOCKS_AFTER_NU5, NetworkUpgrade::Canopy)
     ) {
         let response = crate::service::legacy_chain_check(nu_activation_height, chain.into_iter().rev(), network)
             .map_err(|error| error.to_string());
@@ -222,7 +222,7 @@ proptest! {
     /// Test the `Block.check_transaction_network_upgrade()` error inside the legacy check.
     #[test]
     fn at_least_one_transaction_with_inconsistent_network_upgrade(
-        (network, nu_activation_height, chain) in arbitrary::partial_nu5_chain_strategy(5, false, BLOCKS_AFTER_NU5, NetworkUpgrade::Canopy)
+        (network, nu_activation_height, chain) in partial_nu5_chain_strategy(5, false, BLOCKS_AFTER_NU5, NetworkUpgrade::Canopy)
     ) {
         // this test requires that an invalid block is encountered
         // before a valid block (and before the check gives up),
@@ -262,7 +262,7 @@ proptest! {
     /// Test there is at least one transaction with a valid `network_upgrade` in the legacy check.
     #[test]
     fn at_least_one_transaction_with_valid_network_upgrade(
-        (network, nu_activation_height, chain) in arbitrary::partial_nu5_chain_strategy(5, true, BLOCKS_AFTER_NU5/2, NetworkUpgrade::Canopy)
+        (network, nu_activation_height, chain) in partial_nu5_chain_strategy(5, true, BLOCKS_AFTER_NU5/2, NetworkUpgrade::Canopy)
     ) {
         let response = crate::service::legacy_chain_check(nu_activation_height, chain.into_iter().rev(), network)
             .map_err(|error| error.to_string());

zebra-state/src/tests.rs

@@ -10,28 +10,7 @@ use zebra_chain::{
 
 use super::*;
 
-/// Mocks computation done during semantic validation
+pub mod setup;
-pub trait Prepare {
-    fn prepare(self) -> PreparedBlock;
-}
-
-impl Prepare for Arc<Block> {
-    fn prepare(self) -> PreparedBlock {
-        let block = self;
-        let hash = block.hash();
-        let height = block.coinbase_height().unwrap();
-        let transaction_hashes: Vec<_> = block.transactions.iter().map(|tx| tx.hash()).collect();
-        let new_outputs = transparent::new_ordered_outputs(&block, transaction_hashes.as_slice());
-
-        PreparedBlock {
-            block,
-            hash,
-            height,
-            new_outputs,
-            transaction_hashes,
-        }
-    }
-}
 
 /// Helper trait for constructing "valid" looking chains of blocks
 pub trait FakeChainHelper {

zebra-state/src/tests/setup.rs

@@ -0,0 +1,113 @@
+use std::sync::Arc;
+
+use proptest::prelude::*;
+
+use zebra_chain::{
+    block::{Block, Height},
+    parameters::{
+        Network::{self, *},
+        NetworkUpgrade,
+    },
+    serialization::ZcashDeserializeInto,
+    transaction::Transaction,
+};
+
+use crate::{service::StateService, Config, FinalizedBlock};
+
+/// Generate a chain that allows us to make tests for the legacy chain rules.
+///
+/// Arguments:
+/// - `transaction_version_override`: See `LedgerState::height_strategy` for details.
+/// - `transaction_has_valid_network_upgrade`: See `LedgerState::height_strategy` for details.
+///    Note: `false` allows zero or more invalid network upgrades.
+/// - `blocks_after_nu_activation`: The number of blocks the strategy will generate
+/// after the provided `network_upgrade`.
+/// - `network_upgrade` - The network upgrade that we are using to simulate from where the
+/// legacy chain checks should start to apply.
+///
+/// Returns:
+/// A generated arbitrary strategy for the provided arguments.
+pub(crate) fn partial_nu5_chain_strategy(
+    transaction_version_override: u32,
+    transaction_has_valid_network_upgrade: bool,
+    blocks_after_nu_activation: u32,
+    // TODO: This argument can be removed and just use Nu5 after we have an activation height #1841
+    network_upgrade: NetworkUpgrade,
+) -> impl Strategy<
+    Value = (
+        Network,
+        Height,
+        zebra_chain::fmt::SummaryDebug<Vec<Arc<Block>>>,
+    ),
+> {
+    (
+        any::<Network>(),
+        NetworkUpgrade::reduced_branch_id_strategy(),
+    )
+        .prop_flat_map(move |(network, random_nu)| {
+            // TODO: update this to Nu5 after we have a height #1841
+            let mut nu = network_upgrade;
+            let nu_activation = nu.activation_height(network).unwrap();
+            let height = Height(nu_activation.0 + blocks_after_nu_activation);
+
+            // The `network_upgrade_override` will not be enough as when it is `None`,
+            // current network upgrade will be used (`NetworkUpgrade::Canopy`) which will be valid.
+            if !transaction_has_valid_network_upgrade {
+                nu = random_nu;
+            }
+
+            zebra_chain::block::LedgerState::height_strategy(
+                height,
+                Some(nu),
+                Some(transaction_version_override),
+                transaction_has_valid_network_upgrade,
+            )
+            .prop_flat_map(move |init| {
+                Block::partial_chain_strategy(init, blocks_after_nu_activation as usize)
+            })
+            .prop_map(move |partial_chain| (network, nu_activation, partial_chain))
+        })
+}
+
+/// Return a new `StateService` containing the mainnet genesis block.
+/// Also returns the finalized genesis block itself.
+pub(crate) fn new_state_with_mainnet_genesis() -> (StateService, FinalizedBlock) {
+    let genesis = zebra_test::vectors::BLOCK_MAINNET_GENESIS_BYTES
+        .zcash_deserialize_into::<Arc<Block>>()
+        .expect("block should deserialize");
+
+    let mut state = StateService::new(Config::ephemeral(), Mainnet);
+
+    assert_eq!(None, state.best_tip());
+
+    let genesis = FinalizedBlock::from(genesis);
+    state
+        .disk
+        .commit_finalized_direct(genesis.clone(), "test")
+        .expect("unexpected invalid genesis block test vector");
+
+    assert_eq!(Some((Height(0), genesis.hash)), state.best_tip());
+
+    (state, genesis)
+}
+
+/// Return a `Transaction::V4` with the coinbase data from `coinbase`.
+///
+/// Used to convert a coinbase transaction to a version that the non-finalized state will accept.
+pub(crate) fn transaction_v4_from_coinbase(coinbase: &Transaction) -> Transaction {
+    assert!(
+        !coinbase.has_sapling_shielded_data(),
+        "conversion assumes sapling shielded data is None"
+    );
+
+    Transaction::V4 {
+        inputs: coinbase.inputs().to_vec(),
+        outputs: coinbase.outputs().to_vec(),
+        lock_time: coinbase.lock_time(),
+        // `Height(0)` means that the expiry height is ignored
+        expiry_height: coinbase.expiry_height().unwrap_or(Height(0)),
+        // invalid for coinbase transactions
+        joinsplit_data: None,
+        sapling_shielded_data: None,
+    }
+}