Validate spends of transparent coinbase outputs (#2525)

* Validate transparent coinbase output maturity and shielding

- Add a CoinbaseSpendRestriction enum and Transaction method
- Validate transparent coinbase spends in non-finalized chains

* Don't use genesis created UTXOs for spends in generated block chains

* Refactor out a new_transaction_ordered_outputs function

* Add Transaction::outputs_mut for tests

* Generate valid transparent spends in arbitrary block chains

* When generating blocks, fixup the block contents, then the block hash

* Test that generated chains contain at least one transparent spend

* Make generated chains long enough for reliable tests

* Add transparent and shielded input and output methods to Transaction

* Split chain generation into 3 functions

* Test that unshielded and immature transparent coinbase spends fail

* Comment punctuation

* Clarify a comment

* Clarify probability calculation

* Test that shielded mature coinbase output spends succeed

zebra-chain/src/block.rs

@@ -10,7 +10,7 @@ mod serialize;
 pub mod merkle;
 
 #[cfg(any(test, feature = "proptest-impl"))]
-mod arbitrary;
+pub mod arbitrary;
 #[cfg(any(test, feature = "bench"))]
 pub mod tests;
 

zebra-chain/src/block/arbitrary.rs

@@ -1,26 +1,45 @@
+//! Randomised property testing for [`Block`]s.
+
 use proptest::{
     arbitrary::{any, Arbitrary},
     prelude::*,
 };
 
-use std::{collections::HashSet, convert::TryInto, sync::Arc};
+use std::{collections::HashMap, sync::Arc};
 
 use crate::{
     block,
     fmt::SummaryDebug,
-    orchard,
     parameters::{
         Network,
         NetworkUpgrade::{self, *},
         GENESIS_PREVIOUS_BLOCK_HASH,
     },
     serialization,
-    transparent::Input::*,
+    transparent::{new_transaction_ordered_outputs, CoinbaseSpendRestriction},
     work::{difficulty::CompactDifficulty, equihash},
 };
 
 use super::*;
 
+/// The chain height used to test for prevout inputs.
+///
+/// This impacts the probability of `has_prevouts` failures in
+/// `arbitrary_height_partial_chain_strategy`.
+///
+/// The failure probability calculation is:
+/// ```text
+/// shielded_input = shielded_pool_count / pool_count
+/// expected_transactions = expected_inputs = MAX_ARBITRARY_ITEMS/2
+/// proptest_cases = 256
+/// 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,
+/// so that the failure probability is less than 1 in 1 million.
+pub const PREVOUTS_CHAIN_HEIGHT: usize = 20;
+
 #[derive(Debug, Clone, Copy)]
 #[non_exhaustive]
 /// The configuration data for proptest when generating arbitrary chains
@@ -313,86 +332,66 @@ impl Arbitrary for Block {
     type Strategy = BoxedStrategy<Self>;
 }
 
+/// Skip checking transparent coinbase spends in [`Block::partial_chain_strategy`].
+#[allow(clippy::result_unit_err)]
+pub fn allow_all_transparent_coinbase_spends(
+    _: transparent::OutPoint,
+    _: transparent::CoinbaseSpendRestriction,
+    _: transparent::Utxo,
+) -> Result<(), ()> {
+    Ok(())
+}
+
 impl Block {
-    /// Returns a strategy for creating Vecs of blocks with increasing height of
-    /// the given length.
-    pub fn partial_chain_strategy(
+    /// Returns a strategy for creating vectors of blocks with increasing height.
+    ///
+    /// Each vector is `count` blocks long.
+    ///
+    /// `check_transparent_coinbase_spend` is used to check if
+    /// transparent coinbase UTXOs are valid, before using them in blocks.
+    /// Use [`allow_all_transparent_coinbase_spends`] to disable this check.
+    pub fn partial_chain_strategy<F, T, E>(
         mut current: LedgerState,
         count: usize,
-    ) -> BoxedStrategy<SummaryDebug<Vec<Arc<Self>>>> {
+        check_transparent_coinbase_spend: F,
+    ) -> BoxedStrategy<SummaryDebug<Vec<Arc<Self>>>>
+    where
+        F: Fn(
+                transparent::OutPoint,
+                transparent::CoinbaseSpendRestriction,
+                transparent::Utxo,
+            ) -> Result<T, E>
+            + Copy
+            + 'static,
+    {
         let mut vec = Vec::with_capacity(count);
 
         // generate block strategies with the correct heights
         for _ in 0..count {
-            vec.push(Block::arbitrary_with(current));
+            vec.push((Just(current.height), Block::arbitrary_with(current)));
             current.height.0 += 1;
         }
 
         // after the vec strategy generates blocks, fixup invalid parts of the blocks
-        vec.prop_map(|mut vec| {
+        vec.prop_map(move |mut vec| {
             let mut previous_block_hash = None;
-            let mut utxos = HashSet::<transparent::OutPoint>::new();
+            let mut utxos = HashMap::new();
 
-            for block in vec.iter_mut() {
+            for (height, block) in vec.iter_mut() {
                 // fixup the previous block hash
                 if let Some(previous_block_hash) = previous_block_hash {
                     block.header.previous_block_hash = previous_block_hash;
                 }
-                previous_block_hash = Some(block.hash());
 
-                // fixup the transparent spends
                 let mut new_transactions = Vec::new();
-                for transaction in block.transactions.drain(..) {
-                    let mut transaction = (*transaction).clone();
-                    let mut new_inputs = Vec::new();
-
-                    for mut input in transaction.inputs_mut().drain(..) {
-                        if let PrevOut {
-                            ref mut outpoint, ..
-                        } = input
-                        {
-                            // take a UTXO if available
-                            if utxos.remove(outpoint) {
-                                new_inputs.push(input);
-                            } else if let Some(arbitrary_utxo) = utxos.clone().iter().next() {
-                                *outpoint = *arbitrary_utxo;
-                                utxos.remove(arbitrary_utxo);
-                                new_inputs.push(input);
-                            }
-                            // otherwise, drop the invalid input, it has no UTXOs to spend
-                        } else {
-                            // preserve coinbase inputs
-                            new_inputs.push(input);
-                        }
-                    }
-
-                    // delete invalid inputs
-                    *transaction.inputs_mut() = new_inputs;
-
-                    // keep transactions with valid input counts
-                    // coinbase transactions will never fail this check
-                    // this is the input check from `has_inputs_and_outputs`
-                    if !transaction.inputs().is_empty()
-                        || transaction.joinsplit_count() > 0
-                        || transaction.sapling_spends_per_anchor().count() > 0
-                        || (transaction.orchard_actions().count() > 0
-                            && transaction
-                                .orchard_flags()
-                                .unwrap_or_else(orchard::Flags::empty)
-                                .contains(orchard::Flags::ENABLE_SPENDS))
-                    {
-                        // add the created UTXOs
-                        // these outputs can be spent from the next transaction in this block onwards
-                        // see `new_outputs` for details
-                        let hash = transaction.hash();
-                        for output_index_in_transaction in 0..transaction.outputs().len() {
-                            utxos.insert(transparent::OutPoint {
-                                hash,
-                                index: output_index_in_transaction.try_into().unwrap(),
-                            });
-                        }
-
-                        // and keep the transaction
+                for (tx_index_in_block, transaction) in block.transactions.drain(..).enumerate() {
+                    if let Some(transaction) = fix_generated_transaction(
+                        (*transaction).clone(),
+                        tx_index_in_block,
+                        *height,
+                        &mut utxos,
+                        check_transparent_coinbase_spend,
+                    ) {
                         new_transactions.push(Arc::new(transaction));
                     }
                 }
@@ -400,14 +399,158 @@ impl Block {
                 // delete invalid transactions
                 block.transactions = new_transactions;
 
-                // TODO: fixup the history and authorizing data commitments, if needed
+                // TODO: if needed, fixup:
+                // - transaction output counts (currently 0..=16, consensus rules require 1..)
+                // - history and authorizing data commitments
+
+                // now that we've made all the changes, calculate our block hash,
+                // so the next block can use it
+                previous_block_hash = Some(block.hash());
             }
-            SummaryDebug(vec.into_iter().map(Arc::new).collect())
+            SummaryDebug(
+                vec.into_iter()
+                    .map(|(_height, block)| Arc::new(block))
+                    .collect(),
+            )
         })
         .boxed()
     }
 }
 
+/// Fix `transaction` so it obeys more consensus rules.
+///
+/// Spends [`OutPoint`]s from `utxos`, and adds newly created outputs.
+///
+/// If the transaction can't be fixed, returns `None`.
+pub fn fix_generated_transaction<F, T, E>(
+    mut transaction: Transaction,
+    tx_index_in_block: usize,
+    height: Height,
+    utxos: &mut HashMap<transparent::OutPoint, transparent::OrderedUtxo>,
+    check_transparent_coinbase_spend: F,
+) -> Option<Transaction>
+where
+    F: Fn(
+            transparent::OutPoint,
+            transparent::CoinbaseSpendRestriction,
+            transparent::Utxo,
+        ) -> Result<T, E>
+        + Copy
+        + 'static,
+{
+    let mut spend_restriction = transaction.coinbase_spend_restriction(height);
+    let mut new_inputs = Vec::new();
+
+    // fixup the transparent spends
+    for mut input in transaction.inputs().to_vec().into_iter() {
+        if input.outpoint().is_some() {
+            if let Some(selected_outpoint) = find_valid_utxo_for_spend(
+                &mut transaction,
+                &mut spend_restriction,
+                height,
+                utxos,
+                check_transparent_coinbase_spend,
+            ) {
+                input.set_outpoint(selected_outpoint);
+                new_inputs.push(input);
+
+                utxos.remove(&selected_outpoint);
+            }
+            // otherwise, drop the invalid input, because it has no valid UTXOs to spend
+        } else {
+            // preserve coinbase inputs
+            new_inputs.push(input.clone());
+        }
+    }
+
+    // delete invalid inputs
+    *transaction.inputs_mut() = new_inputs;
+
+    // keep transactions with valid input counts
+    // coinbase transactions will never fail this check
+    if transaction.has_transparent_or_shielded_inputs() {
+        // skip genesis created UTXOs
+        if height > Height(0) {
+            // non-coinbase outputs can be spent from the next transaction in this block onwards
+            // coinbase outputs have to wait 100 blocks, and be shielded
+            utxos.extend(new_transaction_ordered_outputs(
+                &transaction,
+                transaction.hash(),
+                tx_index_in_block,
+                height,
+            ));
+        }
+
+        Some(transaction)
+    } else {
+        None
+    }
+}
+
+/// Find a valid [`OutPoint`] in `utxos` to spend in `transaction`.
+///
+/// Modifies `transaction` and updates `spend_restriction` if needed.
+///
+/// If there is no valid output, or many search attempts have failed, returns `None`.
+pub fn find_valid_utxo_for_spend<F, T, E>(
+    transaction: &mut Transaction,
+    spend_restriction: &mut CoinbaseSpendRestriction,
+    spend_height: Height,
+    utxos: &HashMap<transparent::OutPoint, transparent::OrderedUtxo>,
+    check_transparent_coinbase_spend: F,
+) -> Option<transparent::OutPoint>
+where
+    F: Fn(
+            transparent::OutPoint,
+            transparent::CoinbaseSpendRestriction,
+            transparent::Utxo,
+        ) -> Result<T, E>
+        + Copy
+        + 'static,
+{
+    let has_shielded_outputs = transaction.has_shielded_outputs();
+    let delete_transparent_outputs = CoinbaseSpendRestriction::OnlyShieldedOutputs { spend_height };
+    let mut attempts: usize = 0;
+
+    // choose an arbitrary spendable UTXO, in hash set order
+    while let Some((candidate_outpoint, candidate_utxo)) = utxos.iter().next() {
+        let candidate_utxo = candidate_utxo.clone().utxo;
+
+        attempts += 1;
+
+        // Avoid O(n^2) algorithmic complexity by giving up early,
+        // rather than exhausively checking the entire UTXO set
+        if attempts > 100 {
+            return None;
+        }
+
+        // try the utxo as-is, then try it with deleted transparent outputs
+        if check_transparent_coinbase_spend(
+            *candidate_outpoint,
+            *spend_restriction,
+            candidate_utxo.clone(),
+        )
+        .is_ok()
+        {
+            return Some(*candidate_outpoint);
+        } else if has_shielded_outputs
+            && check_transparent_coinbase_spend(
+                *candidate_outpoint,
+                delete_transparent_outputs,
+                candidate_utxo.clone(),
+            )
+            .is_ok()
+        {
+            *transaction.outputs_mut() = Vec::new();
+            *spend_restriction = delete_transparent_outputs;
+
+            return Some(*candidate_outpoint);
+        }
+    }
+
+    None
+}
+
 impl Arbitrary for Commitment {
     type Parameters = ();
 

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

@@ -11,7 +11,11 @@ use crate::{
     LedgerState,
 };
 
-use super::super::{serialize::MAX_BLOCK_BYTES, *};
+use super::super::{
+    arbitrary::{allow_all_transparent_coinbase_spends, PREVOUTS_CHAIN_HEIGHT},
+    serialize::MAX_BLOCK_BYTES,
+    *,
+};
 
 const DEFAULT_BLOCK_ROUNDTRIP_PROPTEST_CASES: u32 = 16;
 
@@ -157,21 +161,46 @@ fn block_genesis_strategy() -> Result<()> {
     Ok(())
 }
 
-/// Make sure our partial chain strategy generates a chain with the correct coinbase
-/// heights and previous block hashes.
+/// Make sure our genesis partial chain strategy generates a chain with:
+/// - correct coinbase heights
+/// - correct previous block hashes
+/// - no transparent spends in the genesis block, because genesis transparent outputs are ignored
 #[test]
-fn partial_chain_strategy() -> Result<()> {
+fn genesis_partial_chain_strategy() -> Result<()> {
     zebra_test::init();
 
-    let strategy = LedgerState::genesis_strategy(None, None, false)
-        .prop_flat_map(|init| Block::partial_chain_strategy(init, MAX_ARBITRARY_ITEMS));
+    let strategy = LedgerState::genesis_strategy(None, None, false).prop_flat_map(|init| {
+        Block::partial_chain_strategy(
+            init,
+            MAX_ARBITRARY_ITEMS,
+            allow_all_transparent_coinbase_spends,
+        )
+    });
 
     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);
+
+            // block 1 can have spends of transparent outputs
+            // of previous transactions in the same block
+            if height == Height(0) {
+                prop_assert_eq!(
+                    block
+                        .transactions
+                        .iter()
+                        .flat_map(|t| t.inputs())
+                        .filter_map(|i| i.outpoint())
+                        .count(),
+                    0,
+                    "unexpected transparent prevout inputs at height {:?}: genesis transparent outputs are ignored",
+                    height,
+                );
+            }
+
             height = Height(height.0 + 1);
             previous_block_hash = block.hash();
         }
@@ -180,19 +209,29 @@ fn partial_chain_strategy() -> Result<()> {
     Ok(())
 }
 
-/// Make sure our block height strategy generates a chain with the correct coinbase
-/// heights and hashes.
+/// 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();
 
     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));
+        .prop_flat_map(|init| {
+            Block::partial_chain_strategy(
+                init,
+                PREVOUTS_CHAIN_HEIGHT,
+                allow_all_transparent_coinbase_spends,
+            )
+        });
 
     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() {
                 prop_assert!(block.coinbase_height().is_some());
@@ -202,8 +241,19 @@ fn arbitrary_height_partial_chain_strategy() -> Result<()> {
                 prop_assert_eq!(block.coinbase_height(), height);
                 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-chain/src/transaction.rs

@@ -28,7 +28,11 @@ use crate::{
     block, orchard,
     parameters::NetworkUpgrade,
     primitives::{Bctv14Proof, Groth16Proof},
-    sapling, sprout, transparent,
+    sapling, sprout,
+    transparent::{
+        self,
+        CoinbaseSpendRestriction::{self, *},
+    },
     value_balance::ValueBalance,
 };
 
@@ -155,6 +159,75 @@ impl Transaction {
         sighash::SigHasher::new(self, hash_type, network_upgrade, input).sighash()
     }
 
+    // other properties
+
+    /// Does this transaction have transparent or shielded inputs?
+    ///
+    /// "[Sapling onward] If effectiveVersion < 5, then at least one of tx_in_count,
+    /// nSpendsSapling, and nJoinSplit MUST be nonzero.
+    ///
+    /// [NU5 onward] If effectiveVersion ≥ 5 then this condition MUST hold:
+    /// tx_in_count > 0 or nSpendsSapling > 0 or (nActionsOrchard > 0 and enableSpendsOrchard = 1)."
+    ///
+    /// https://zips.z.cash/protocol/protocol.pdf#txnconsensus
+    pub fn has_transparent_or_shielded_inputs(&self) -> bool {
+        !self.inputs().is_empty() || self.has_shielded_inputs()
+    }
+
+    /// Does this transaction have shielded inputs?
+    ///
+    /// See [`has_transparent_or_shielded_inputs`] for details.
+    pub fn has_shielded_inputs(&self) -> bool {
+        self.joinsplit_count() > 0
+            || self.sapling_spends_per_anchor().count() > 0
+            || (self.orchard_actions().count() > 0
+                && self
+                    .orchard_flags()
+                    .unwrap_or_else(orchard::Flags::empty)
+                    .contains(orchard::Flags::ENABLE_SPENDS))
+    }
+
+    /// Does this transaction have transparent or shielded outputs?
+    ///
+    /// "[Sapling onward] If effectiveVersion < 5, then at least one of tx_out_count,
+    /// nOutputsSapling, and nJoinSplit MUST be nonzero.
+    ///
+    /// [NU5 onward] If effectiveVersion ≥ 5 then this condition MUST hold:
+    /// tx_out_count > 0 or nOutputsSapling > 0 or (nActionsOrchard > 0 and enableOutputsOrchard = 1)."
+    ///
+    /// https://zips.z.cash/protocol/protocol.pdf#txnconsensus
+    pub fn has_transparent_or_shielded_outputs(&self) -> bool {
+        !self.outputs().is_empty() || self.has_shielded_outputs()
+    }
+
+    /// Does this transaction have shielded outputs?
+    ///
+    /// See [`has_transparent_or_shielded_outputs`] for details.
+    pub fn has_shielded_outputs(&self) -> bool {
+        self.joinsplit_count() > 0
+            || self.sapling_outputs().count() > 0
+            || (self.orchard_actions().count() > 0
+                && self
+                    .orchard_flags()
+                    .unwrap_or_else(orchard::Flags::empty)
+                    .contains(orchard::Flags::ENABLE_OUTPUTS))
+    }
+
+    /// Returns the [`CoinbaseSpendRestriction`] for this transaction,
+    /// assuming it is mined at `spend_height`.
+    pub fn coinbase_spend_restriction(
+        &self,
+        spend_height: block::Height,
+    ) -> CoinbaseSpendRestriction {
+        if self.outputs().is_empty() {
+            // we know this transaction must have shielded outputs,
+            // because of other consensus rules
+            OnlyShieldedOutputs { spend_height }
+        } else {
+            SomeTransparentOutputs
+        }
+    }
+
     // header
 
     /// Return if the `fOverwintered` flag of this transaction is set.
@@ -250,6 +323,28 @@ impl Transaction {
         }
     }
 
+    /// Modify the transparent outputs of this transaction, regardless of version.
+    #[cfg(any(test, feature = "proptest-impl"))]
+    pub fn outputs_mut(&mut self) -> &mut Vec<transparent::Output> {
+        match self {
+            Transaction::V1 {
+                ref mut outputs, ..
+            } => outputs,
+            Transaction::V2 {
+                ref mut outputs, ..
+            } => outputs,
+            Transaction::V3 {
+                ref mut outputs, ..
+            } => outputs,
+            Transaction::V4 {
+                ref mut outputs, ..
+            } => outputs,
+            Transaction::V5 {
+                ref mut outputs, ..
+            } => outputs,
+        }
+    }
+
     /// Returns `true` if this transaction is a coinbase transaction.
     pub fn is_coinbase(&self) -> bool {
         self.inputs().len() == 1

zebra-chain/src/transparent.rs

@@ -9,7 +9,13 @@ mod utxo;
 
 pub use address::Address;
 pub use script::Script;
-pub use utxo::{new_ordered_outputs, new_outputs, utxos_from_ordered_utxos, OrderedUtxo, Utxo};
+pub use utxo::{
+    new_ordered_outputs, new_outputs, utxos_from_ordered_utxos, CoinbaseSpendRestriction,
+    OrderedUtxo, Utxo,
+};
+
+#[cfg(any(test, feature = "proptest-impl"))]
+pub(crate) use utxo::new_transaction_ordered_outputs;
 
 #[cfg(any(test, feature = "proptest-impl"))]
 use proptest_derive::Arbitrary;

zebra-chain/src/transparent/utxo.rs

@@ -4,7 +4,8 @@ use std::{collections::HashMap, convert::TryInto};
 
 use crate::{
     block::{self, Block},
-    transaction, transparent,
+    transaction::{self, Transaction},
+    transparent,
 };
 
 /// An unspent `transparent::Output`, with accompanying metadata.
@@ -68,6 +69,26 @@ impl OrderedUtxo {
     }
 }
 
+/// A restriction that must be checked before spending a transparent output of a
+/// coinbase transaction.
+///
+/// See [`CoinbaseSpendRestriction::check_spend`] for the consensus rules.
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+#[cfg_attr(
+    any(test, feature = "proptest-impl"),
+    derive(proptest_derive::Arbitrary)
+)]
+pub enum CoinbaseSpendRestriction {
+    /// The UTXO is spent in a transaction with one or more transparent outputs
+    SomeTransparentOutputs,
+
+    /// The UTXO is spent in a transaction which only has shielded outputs
+    OnlyShieldedOutputs {
+        /// The height at which the UTXO is spent
+        spend_height: block::Height,
+    },
+}
+
 /// Compute an index of [`Utxo`]s, given an index of [`OrderedUtxo`]s.
 pub fn utxos_from_ordered_utxos(
     ordered_utxos: HashMap<transparent::OutPoint, OrderedUtxo>,
@@ -93,29 +114,51 @@ pub fn new_ordered_outputs(
     block: &Block,
     transaction_hashes: &[transaction::Hash],
 ) -> HashMap<transparent::OutPoint, OrderedUtxo> {
-    let mut new_ordered_outputs = HashMap::default();
+    let mut new_ordered_outputs = HashMap::new();
     let height = block.coinbase_height().expect("block has coinbase height");
+
     for (tx_index_in_block, (transaction, hash)) in block
         .transactions
         .iter()
         .zip(transaction_hashes.iter().cloned())
         .enumerate()
     {
-        let from_coinbase = transaction.is_coinbase();
-        for (output_index_in_transaction, output) in
-            transaction.outputs().iter().cloned().enumerate()
-        {
-            let output_index_in_transaction = output_index_in_transaction
-                .try_into()
-                .expect("unexpectedly large number of outputs");
-            new_ordered_outputs.insert(
-                transparent::OutPoint {
-                    hash,
-                    index: output_index_in_transaction,
-                },
-                OrderedUtxo::new(output, height, from_coinbase, tx_index_in_block),
-            );
-        }
+        new_ordered_outputs.extend(new_transaction_ordered_outputs(
+            transaction,
+            hash,
+            tx_index_in_block,
+            height,
+        ));
+    }
+
+    new_ordered_outputs
+}
+
+/// Compute an index of newly created [`OrderedUtxo`]s, given a transaction,
+/// its precomputed transaction hash, the transaction's index in its block,
+/// and the block's height.
+///
+/// This function is only intended for use in tests.
+pub(crate) fn new_transaction_ordered_outputs(
+    transaction: &Transaction,
+    hash: transaction::Hash,
+    tx_index_in_block: usize,
+    height: block::Height,
+) -> HashMap<transparent::OutPoint, OrderedUtxo> {
+    let mut new_ordered_outputs = HashMap::new();
+
+    let from_coinbase = transaction.is_coinbase();
+    for (output_index_in_transaction, output) in transaction.outputs().iter().cloned().enumerate() {
+        let output_index_in_transaction = output_index_in_transaction
+            .try_into()
+            .expect("unexpectedly large number of outputs");
+        new_ordered_outputs.insert(
+            transparent::OutPoint {
+                hash,
+                index: output_index_in_transaction,
+            },
+            OrderedUtxo::new(output, height, from_coinbase, tx_index_in_block),
+        );
     }
 
     new_ordered_outputs

zebra-consensus/src/transaction/check.rs

@@ -31,28 +31,9 @@ use std::convert::TryFrom;
 ///
 /// https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus
 pub fn has_inputs_and_outputs(tx: &Transaction) -> Result<(), TransactionError> {
-    let tx_in_count = tx.inputs().len();
-    let tx_out_count = tx.outputs().len();
-    let n_joinsplit = tx.joinsplit_count();
-    let n_spends_sapling = tx.sapling_spends_per_anchor().count();
-    let n_outputs_sapling = tx.sapling_outputs().count();
-    let n_actions_orchard = tx.orchard_actions().count();
-    let flags_orchard = tx.orchard_flags().unwrap_or_else(Flags::empty);
-
-    // TODO: Improve the code to express the spec rules better #2410.
-    if tx_in_count
-        + n_spends_sapling
-        + n_joinsplit
-        + (n_actions_orchard > 0 && flags_orchard.contains(Flags::ENABLE_SPENDS)) as usize
-        == 0
-    {
+    if !tx.has_transparent_or_shielded_inputs() {
         Err(TransactionError::NoInputs)
-    } else if tx_out_count
-        + n_outputs_sapling
-        + n_joinsplit
-        + (n_actions_orchard > 0 && flags_orchard.contains(Flags::ENABLE_OUTPUTS)) as usize
-        == 0
-    {
+    } else if !tx.has_transparent_or_shielded_outputs() {
         Err(TransactionError::NoOutputs)
     } else {
         Ok(())

zebra-state/src/constants.rs

@@ -2,15 +2,6 @@
 
 /// The maturity threshold for transparent coinbase outputs.
 ///
-/// This threshold uses the relevant chain for the block being verified by the
-/// non-finalized state.
-///
-/// For the best chain, coinbase spends are only allowed from blocks at or below
-/// the finalized tip. For other chains, coinbase spends can use outputs from
-/// early non-finalized blocks, or finalized blocks. But if that chain becomes
-/// the best chain, all non-finalized blocks past the [`MAX_BLOCK_REORG_HEIGHT`]
-/// will be finalized. This includes all mature coinbase outputs.
-///
 /// "A transaction MUST NOT spend a transparent output of a coinbase transaction
 /// from a block less than 100 blocks prior to the spend. Note that transparent
 /// outputs of coinbase transactions include Founders' Reward outputs and
@@ -21,8 +12,16 @@ pub const MIN_TRANSPARENT_COINBASE_MATURITY: u32 = 100;
 /// The maximum chain reorganisation height.
 ///
 /// This threshold determines the maximum length of the best non-finalized chain.
-///
 /// Larger reorganisations would allow double-spends of coinbase transactions.
+///
+/// This threshold uses the relevant chain for the block being verified by the
+/// non-finalized state.
+///
+/// For the best chain, coinbase spends are only allowed from blocks at or below
+/// the finalized tip. For other chains, coinbase spends can use outputs from
+/// early non-finalized blocks, or finalized blocks. But if that chain becomes
+/// the best chain, all non-finalized blocks past the [`MAX_BLOCK_REORG_HEIGHT`]
+/// will be finalized. This includes all mature coinbase outputs.
 pub const MAX_BLOCK_REORG_HEIGHT: u32 = MIN_TRANSPARENT_COINBASE_MATURITY - 1;
 
 /// The database format version, incremented each time the database format changes.

zebra-state/src/error.rs

@@ -7,6 +7,8 @@ use zebra_chain::{
     block, orchard, sapling, sprout, transparent, work::difficulty::CompactDifficulty,
 };
 
+use crate::constants::MIN_TRANSPARENT_COINBASE_MATURITY;
+
 /// A wrapper for type erased errors that is itself clonable and implements the
 /// Error trait
 #[derive(Debug, Error, Clone)]
@@ -95,6 +97,28 @@ pub enum ValidateContextError {
     #[non_exhaustive]
     EarlyTransparentSpend { outpoint: transparent::OutPoint },
 
+    #[error(
+        "unshielded transparent coinbase spend: {outpoint:?} \
+         must be spent in a transaction which only has shielded outputs"
+    )]
+    #[non_exhaustive]
+    UnshieldedTransparentCoinbaseSpend { outpoint: transparent::OutPoint },
+
+    #[error(
+        "immature transparent coinbase spend: \
+        attempt to spend {outpoint:?} at {spend_height:?}, \
+        but spends are invalid before {min_spend_height:?}, \
+        which is {MIN_TRANSPARENT_COINBASE_MATURITY:?} blocks \
+        after it was created at {created_height:?}"
+    )]
+    #[non_exhaustive]
+    ImmatureTransparentCoinbaseSpend {
+        outpoint: transparent::OutPoint,
+        spend_height: block::Height,
+        min_spend_height: block::Height,
+        created_height: block::Height,
+    },
+
     #[error("sprout double-spend: duplicate nullifier: {nullifier:?}, in finalized state: {in_finalized_state:?}")]
     #[non_exhaustive]
     DuplicateSproutNullifier {

zebra-state/src/lib.rs

@@ -27,7 +27,6 @@ mod response;
 mod service;
 mod util;
 
-// TODO: move these to integration tests.
 #[cfg(test)]
 mod tests;
 

zebra-state/src/service.rs

@@ -28,12 +28,14 @@ use crate::{
     PreparedBlock, Request, Response, ValidateContextError,
 };
 
-#[cfg(any(test, feature = "proptest-impl"))]
-pub mod arbitrary;
-mod check;
+pub(crate) mod check;
 mod finalized_state;
 mod non_finalized_state;
 mod pending_utxos;
+
+#[cfg(any(test, feature = "proptest-impl"))]
+pub mod arbitrary;
+
 #[cfg(test)]
 mod tests;
 

zebra-state/src/service/arbitrary.rs

@@ -7,13 +7,24 @@ use proptest::{
     test_runner::TestRunner,
 };
 
-use zebra_chain::{block::Block, fmt::SummaryDebug, parameters::NetworkUpgrade, LedgerState};
+use zebra_chain::{
+    block::{self, Block},
+    fmt::SummaryDebug,
+    parameters::NetworkUpgrade,
+    LedgerState,
+};
 
-use crate::arbitrary::Prepare;
+use crate::{arbitrary::Prepare, constants};
 
 use super::*;
 
-const MAX_PARTIAL_CHAIN_BLOCKS: usize = 102;
+/// The chain length for state proptests.
+///
+/// Shorter lengths increase the probability of proptest failures.
+///
+/// See [`block::arbitrary::PREVOUTS_CHAIN_HEIGHT`] for details.
+pub const MAX_PARTIAL_CHAIN_BLOCKS: usize =
+    constants::MIN_TRANSPARENT_COINBASE_MATURITY as usize + block::arbitrary::PREVOUTS_CHAIN_HEIGHT;
 
 #[derive(Debug)]
 pub struct PreparedChainTree {
@@ -62,7 +73,11 @@ impl Strategy for PreparedChain {
                 .prop_flat_map(|ledger| {
                     (
                         Just(ledger.network),
-                        Block::partial_chain_strategy(ledger, MAX_PARTIAL_CHAIN_BLOCKS),
+                        Block::partial_chain_strategy(
+                            ledger,
+                            MAX_PARTIAL_CHAIN_BLOCKS,
+                            check::utxo::transparent_coinbase_spend,
+                        ),
                     )
                 })
                 .prop_map(|(network, vec)| {

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

@@ -1,27 +1,126 @@
-//! Randomised property tests for UTXO contextual validation
+//! Test vectors and randomised property tests for UTXO contextual validation
 
 use std::{convert::TryInto, env, sync::Arc};
 
 use proptest::prelude::*;
 
 use zebra_chain::{
+    amount::Amount,
     block::{Block, Height},
     fmt::TypeNameToDebug,
     serialization::ZcashDeserializeInto,
-    transaction::{LockTime, Transaction},
+    transaction::{self, LockTime, Transaction},
     transparent,
 };
 
 use crate::{
     arbitrary::Prepare,
+    constants::MIN_TRANSPARENT_COINBASE_MATURITY,
+    service::check,
     service::StateService,
     tests::setup::{new_state_with_mainnet_genesis, transaction_v4_from_coinbase},
     FinalizedBlock,
     ValidateContextError::{
-        DuplicateTransparentSpend, EarlyTransparentSpend, MissingTransparentOutput,
+        DuplicateTransparentSpend, EarlyTransparentSpend, ImmatureTransparentCoinbaseSpend,
+        MissingTransparentOutput, UnshieldedTransparentCoinbaseSpend,
     },
 };
 
+/// Check that shielded, mature spends of coinbase transparent outputs succeed.
+///
+/// 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_shielded_mature_coinbase_utxo_spend() {
+    zebra_test::init();
+
+    let created_height = Height(1);
+    let outpoint = transparent::OutPoint {
+        hash: transaction::Hash([0u8; 32]),
+        index: 0,
+    };
+    let output = transparent::Output {
+        value: Amount::zero(),
+        lock_script: transparent::Script::new(&[]),
+    };
+    let utxo = transparent::Utxo {
+        output,
+        height: created_height,
+        from_coinbase: true,
+    };
+
+    let min_spend_height = Height(created_height.0 + MIN_TRANSPARENT_COINBASE_MATURITY);
+    let spend_restriction = transparent::CoinbaseSpendRestriction::OnlyShieldedOutputs {
+        spend_height: min_spend_height,
+    };
+
+    let result = check::utxo::transparent_coinbase_spend(outpoint, spend_restriction, utxo.clone());
+    assert_eq!(result, Ok(utxo));
+}
+
+/// Check that non-shielded spends of coinbase transparent outputs fail.
+#[test]
+fn reject_unshielded_coinbase_utxo_spend() {
+    zebra_test::init();
+
+    let created_height = Height(1);
+    let outpoint = transparent::OutPoint {
+        hash: transaction::Hash([0u8; 32]),
+        index: 0,
+    };
+    let output = transparent::Output {
+        value: Amount::zero(),
+        lock_script: transparent::Script::new(&[]),
+    };
+    let utxo = transparent::Utxo {
+        output,
+        height: created_height,
+        from_coinbase: true,
+    };
+
+    let spend_restriction = transparent::CoinbaseSpendRestriction::SomeTransparentOutputs;
+
+    let result = check::utxo::transparent_coinbase_spend(outpoint, spend_restriction, utxo);
+    assert_eq!(result, Err(UnshieldedTransparentCoinbaseSpend { outpoint }));
+}
+
+/// Check that early spends of coinbase transparent outputs fail.
+#[test]
+fn reject_immature_coinbase_utxo_spend() {
+    zebra_test::init();
+
+    let created_height = Height(1);
+    let outpoint = transparent::OutPoint {
+        hash: transaction::Hash([0u8; 32]),
+        index: 0,
+    };
+    let output = transparent::Output {
+        value: Amount::zero(),
+        lock_script: transparent::Script::new(&[]),
+    };
+    let utxo = transparent::Utxo {
+        output,
+        height: created_height,
+        from_coinbase: true,
+    };
+
+    let min_spend_height = Height(created_height.0 + MIN_TRANSPARENT_COINBASE_MATURITY);
+    let spend_height = Height(min_spend_height.0 - 1);
+    let spend_restriction =
+        transparent::CoinbaseSpendRestriction::OnlyShieldedOutputs { spend_height };
+
+    let result = check::utxo::transparent_coinbase_spend(outpoint, spend_restriction, utxo);
+    assert_eq!(
+        result,
+        Err(ImmatureTransparentCoinbaseSpend {
+            outpoint,
+            spend_height,
+            min_spend_height,
+            created_height
+        })
+    );
+}
+
 // These tests use the `Arbitrary` trait to easily generate complex types,
 // then modify those types to cause an error (or to ensure success).
 //

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

@@ -2,40 +2,36 @@
 
 use std::collections::{HashMap, HashSet};
 
-use zebra_chain::transparent;
+use zebra_chain::{
+    block,
+    transparent::{self, CoinbaseSpendRestriction::*},
+};
 
 use crate::{
+    constants::MIN_TRANSPARENT_COINBASE_MATURITY,
     service::finalized_state::FinalizedState,
     PreparedBlock,
     ValidateContextError::{
-        self, DuplicateTransparentSpend, EarlyTransparentSpend, MissingTransparentOutput,
+        self, DuplicateTransparentSpend, EarlyTransparentSpend, ImmatureTransparentCoinbaseSpend,
+        MissingTransparentOutput, UnshieldedTransparentCoinbaseSpend,
     },
 };
 
-/// Reject double-spends of transparent outputs:
+/// Reject invalid spends of transparent outputs.
+///
+/// Double-spends:
 /// - duplicate spends that are both in this block,
+/// - spends of an output that was spent by a previous block,
+///
+/// Missing spends:
 /// - spends of an output that hasn't been created yet,
-///   (in linear chain and transaction order), and
-/// - spends of an output that was spent by a previous block.
+///   (in linear chain and transaction order),
+/// - spends of UTXOs that were never created in this chain,
 ///
-/// Also rejects attempts to spend UTXOs that were never created (in this chain).
-///
-/// "each output of a particular transaction
-/// can only be used as an input once in the block chain.
-/// Any subsequent reference is a forbidden double spend-
-/// an attempt to spend the same satoshis twice."
-///
-/// https://developer.bitcoin.org/devguide/block_chain.html#introduction
-///
-/// "Any input within this block can spend an output which also appears in this block
-/// (assuming the spend is otherwise valid).
-/// However, the TXID corresponding to the output must be placed at some point
-/// before the TXID corresponding to the input.
-/// This ensures that any program parsing block chain transactions linearly
-/// will encounter each output before it is used as an input."
-///
-/// https://developer.bitcoin.org/reference/block_chain.html#merkle-trees
-pub fn transparent_double_spends(
+/// Invalid spends:
+/// - spends of an immature transparent coinbase output,
+/// - unshielded spends of a transparent coinbase output.
+pub fn transparent_spend(
     prepared: &PreparedBlock,
     non_finalized_chain_unspent_utxos: &HashMap<transparent::OutPoint, transparent::Utxo>,
     non_finalized_chain_spent_utxos: &HashSet<transparent::OutPoint>,
@@ -52,6 +48,7 @@ pub fn transparent_double_spends(
         });
 
         for spend in spends {
+            // see `transparent_spend_chain_order` for the consensus rule
             if !block_spends.insert(*spend) {
                 // reject in-block duplicate spends
                 return Err(DuplicateTransparentSpend {
@@ -60,56 +57,150 @@ pub fn transparent_double_spends(
                 });
             }
 
-            // check spends occur in chain order
+            let utxo = transparent_spend_chain_order(
+                *spend,
+                spend_tx_index_in_block,
+                &prepared.new_outputs,
+                non_finalized_chain_unspent_utxos,
+                non_finalized_chain_spent_utxos,
+                finalized_state,
+            )?;
+
+            // The state service returns UTXOs from pending blocks,
+            // which can be rejected by later contextual checks.
+            // This is a particular issue for v5 transactions,
+            // because their authorizing data is only bound to the block data
+            // during contextual validation (#2336).
             //
-            // because we are in the non-finalized state, we need to check spends within the same block,
-            // spent non-finalized UTXOs, and unspent non-finalized and finalized UTXOs.
+            // We don't want to use UTXOs from invalid pending blocks,
+            // so we check transparent coinbase maturity and shielding
+            // using known valid UTXOs during non-finalized chain validation.
 
-            if let Some(output) = prepared.new_outputs.get(spend) {
-                // reject the spend if it uses an output from this block,
-                // but the output was not created by an earlier transaction
-                //
-                // we know the spend is invalid, because transaction IDs are unique
-                //
-                // (transaction IDs also commit to transaction inputs,
-                // so it should be cryptographically impossible for a transaction
-                // to spend its own outputs)
-                if output.tx_index_in_block >= spend_tx_index_in_block {
-                    return Err(EarlyTransparentSpend { outpoint: *spend });
-                } else {
-                    // a unique spend of a previous transaction's output is ok
-                    continue;
-                }
-            }
-
-            if non_finalized_chain_spent_utxos.contains(spend) {
-                // reject the spend if its UTXO is already spent in the
-                // non-finalized parent chain
-                return Err(DuplicateTransparentSpend {
-                    outpoint: *spend,
-                    location: "the non-finalized chain",
-                });
-            }
-
-            if !non_finalized_chain_unspent_utxos.contains_key(spend)
-                && finalized_state.utxo(spend).is_none()
-            {
-                // we don't keep spent UTXOs in the finalized state,
-                // so all we can say is that it's missing from both
-                // the finalized and non-finalized chains
-                // (it might have been spent in the finalized state,
-                // or it might never have existed in this chain)
-                return Err(MissingTransparentOutput {
-                    outpoint: *spend,
-                    location: "the non-finalized and finalized chain",
-                });
-            }
+            let spend_restriction = transaction.coinbase_spend_restriction(prepared.height);
+            transparent_coinbase_spend(*spend, spend_restriction, utxo)?;
         }
     }
 
     Ok(())
 }
 
+/// Check that transparent spends occur in chain order.
+///
+/// Because we are in the non-finalized state, we need to check spends within the same block,
+/// spent non-finalized UTXOs, and unspent non-finalized and finalized UTXOs.
+///
+/// "Any input within this block can spend an output which also appears in this block
+/// (assuming the spend is otherwise valid).
+/// However, the TXID corresponding to the output must be placed at some point
+/// before the TXID corresponding to the input.
+/// This ensures that any program parsing block chain transactions linearly
+/// will encounter each output before it is used as an input."
+///
+/// https://developer.bitcoin.org/reference/block_chain.html#merkle-trees
+///
+/// "each output of a particular transaction
+/// can only be used as an input once in the block chain.
+/// Any subsequent reference is a forbidden double spend-
+/// an attempt to spend the same satoshis twice."
+///
+/// https://developer.bitcoin.org/devguide/block_chain.html#introduction
+fn transparent_spend_chain_order(
+    spend: transparent::OutPoint,
+    spend_tx_index_in_block: usize,
+    block_new_outputs: &HashMap<transparent::OutPoint, transparent::OrderedUtxo>,
+    non_finalized_chain_unspent_utxos: &HashMap<transparent::OutPoint, transparent::Utxo>,
+    non_finalized_chain_spent_utxos: &HashSet<transparent::OutPoint>,
+    finalized_state: &FinalizedState,
+) -> Result<transparent::Utxo, ValidateContextError> {
+    if let Some(output) = block_new_outputs.get(&spend) {
+        // reject the spend if it uses an output from this block,
+        // but the output was not created by an earlier transaction
+        //
+        // we know the spend is invalid, because transaction IDs are unique
+        //
+        // (transaction IDs also commit to transaction inputs,
+        // so it should be cryptographically impossible for a transaction
+        // to spend its own outputs)
+        if output.tx_index_in_block >= spend_tx_index_in_block {
+            return Err(EarlyTransparentSpend { outpoint: spend });
+        } else {
+            // a unique spend of a previous transaction's output is ok
+            return Ok(output.utxo.clone());
+        }
+    }
+
+    if non_finalized_chain_spent_utxos.contains(&spend) {
+        // reject the spend if its UTXO is already spent in the
+        // non-finalized parent chain
+        return Err(DuplicateTransparentSpend {
+            outpoint: spend,
+            location: "the non-finalized chain",
+        });
+    }
+
+    match (
+        non_finalized_chain_unspent_utxos.get(&spend),
+        finalized_state.utxo(&spend),
+    ) {
+        (None, None) => {
+            // we don't keep spent UTXOs in the finalized state,
+            // so all we can say is that it's missing from both
+            // the finalized and non-finalized chains
+            // (it might have been spent in the finalized state,
+            // or it might never have existed in this chain)
+            Err(MissingTransparentOutput {
+                outpoint: spend,
+                location: "the non-finalized and finalized chain",
+            })
+        }
+
+        (Some(utxo), _) => Ok(utxo.clone()),
+        (_, Some(utxo)) => Ok(utxo),
+    }
+}
+
+/// Check that `utxo` is spendable, based on the coinbase `spend_restriction`.
+///
+/// "A transaction with one or more transparent inputs from coinbase transactions
+/// MUST have no transparent outputs (i.e.tx_out_count MUST be 0)."
+///
+/// "A transaction MUST NOT spend a transparent output of a coinbase transaction
+/// from a block less than 100 blocks prior to the spend.
+///
+/// Note that transparent outputs of coinbase transactions include Founders’
+/// Reward outputs and transparent funding stream outputs."
+///
+/// https://zips.z.cash/protocol/protocol.pdf#txnencodingandconsensus
+pub fn transparent_coinbase_spend(
+    outpoint: transparent::OutPoint,
+    spend_restriction: transparent::CoinbaseSpendRestriction,
+    utxo: transparent::Utxo,
+) -> Result<transparent::Utxo, ValidateContextError> {
+    if !utxo.from_coinbase {
+        return Ok(utxo);
+    }
+
+    match spend_restriction {
+        OnlyShieldedOutputs { spend_height } => {
+            let min_spend_height = utxo.height + block::Height(MIN_TRANSPARENT_COINBASE_MATURITY);
+            // TODO: allow full u32 range of block heights (#1113)
+            let min_spend_height =
+                min_spend_height.expect("valid UTXOs have coinbase heights far below Height::MAX");
+            if spend_height >= min_spend_height {
+                Ok(utxo)
+            } else {
+                Err(ImmatureTransparentCoinbaseSpend {
+                    outpoint,
+                    spend_height,
+                    min_spend_height,
+                    created_height: utxo.height,
+                })
+            }
+        }
+        SomeTransparentOutputs => Err(UnshieldedTransparentCoinbaseSpend { outpoint }),
+    }
+}
+
 /// Reject negative remaining transaction value.
 ///
 /// Consensus rule: The remaining value in the transparent transaction value pool MUST be nonnegative.

zebra-state/src/service/non_finalized_state.rs

@@ -172,7 +172,7 @@ impl NonFinalizedState {
         prepared: PreparedBlock,
         finalized_state: &FinalizedState,
     ) -> Result<Chain, ValidateContextError> {
-        check::utxo::transparent_double_spends(
+        check::utxo::transparent_spend(
             &prepared,
             &parent_chain.unspent_utxos(),
             &parent_chain.spent_utxos,

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

@@ -2,7 +2,12 @@ use std::{env, sync::Arc};
 
 use zebra_test::prelude::*;
 
-use zebra_chain::{block::Block, fmt::DisplayToDebug, parameters::NetworkUpgrade::*, LedgerState};
+use zebra_chain::{
+    block::{self, Block},
+    fmt::DisplayToDebug,
+    parameters::NetworkUpgrade::*,
+    LedgerState,
+};
 
 use crate::{
     arbitrary::Prepare,
@@ -17,6 +22,10 @@ use crate::{
 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.
+///
+/// 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();
@@ -30,12 +39,37 @@ fn forked_equals_pushed() -> Result<()> {
             let fork_tip_hash = chain[fork_at_count - 1].hash;
             let mut full_chain = Chain::default();
             let mut partial_chain = Chain::default();
+            let mut has_prevouts = false;
 
             for block in chain.iter().take(fork_at_count) {
                 partial_chain = partial_chain.push(block.clone())?;
             }
             for block in chain.iter() {
                 full_chain = full_chain.push(block.clone())?;
+
+                // check some other properties of generated chains
+                if block.height == block::Height(0) {
+                    prop_assert_eq!(
+                        block
+                            .block
+                            .transactions
+                            .iter()
+                            .flat_map(|t| t.inputs())
+                            .filter_map(|i| i.outpoint())
+                            .count(),
+                        0,
+                        "unexpected transparent prevout inputs at height {:?}: genesis transparent outputs are ignored",
+                        block.height,
+                    );
+                }
+
+                has_prevouts |= block
+                    .block
+                    .transactions
+                    .iter()
+                    .flat_map(|t| t.inputs())
+                    .find_map(|i| i.outpoint())
+                    .is_some();
             }
 
             let forked = full_chain.fork(fork_tip_hash).expect("fork works").expect("hash is present");
@@ -43,6 +77,10 @@ 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(())

zebra-state/src/tests/setup.rs

@@ -12,7 +12,10 @@ use zebra_chain::{
     transaction::Transaction,
 };
 
-use crate::{service::StateService, Config, FinalizedBlock};
+use crate::{
+    service::{check, StateService},
+    Config, FinalizedBlock,
+};
 
 /// Generate a chain that allows us to make tests for the legacy chain rules.
 ///
@@ -63,7 +66,11 @@ pub(crate) fn partial_nu5_chain_strategy(
                 transaction_has_valid_network_upgrade,
             )
             .prop_flat_map(move |init| {
-                Block::partial_chain_strategy(init, blocks_after_nu_activation as usize)
+                Block::partial_chain_strategy(
+                    init,
+                    blocks_after_nu_activation as usize,
+                    check::utxo::transparent_coinbase_spend,
+                )
             })
             .prop_map(move |partial_chain| (network, nu_activation, partial_chain))
         })