fix(rpc): Omit transactions with transparent coinbase spends that are…

… immature at the next block height from block templates (#6510)

* Adds `maturity_height` to VerifiedUnminedTx

Filters out transactions that are invalid at next_block_height in getblocktemplate method

* Adds unit testing

* rustfmt

* rejects txs with immature coinbase spends from mempool

* Condenses fns for transparent coinbase spend check

* Updates calls to VerifiedUnminedTx::new()

* Update zebra-chain/src/transparent/utxo.rs

* Applies suggestions from code review

zebra-chain/src/block/arbitrary.rs

@@ -373,7 +373,7 @@ impl Arbitrary for Block {
 pub fn allow_all_transparent_coinbase_spends(
     _: transparent::OutPoint,
     _: transparent::CoinbaseSpendRestriction,
-    _: transparent::OrderedUtxo,
+    _: &transparent::Utxo,
 ) -> Result<(), ()> {
     Ok(())
 }
@@ -390,7 +390,7 @@ impl Block {
     /// `generate_valid_commitments` specifies if the generated blocks
     /// should have valid commitments. This makes it much slower so it's better
     /// to enable only when needed.
-    pub fn partial_chain_strategy<F, T, E>(
+    pub fn partial_chain_strategy<F, E>(
         mut current: LedgerState,
         count: usize,
         check_transparent_coinbase_spend: F,
@@ -400,8 +400,8 @@ impl Block {
         F: Fn(
                 transparent::OutPoint,
                 transparent::CoinbaseSpendRestriction,
-                transparent::OrderedUtxo,
-            ) -> Result<T, E>
+                &transparent::Utxo,
+            ) -> Result<(), E>
             + Copy
             + 'static,
     {
@@ -558,7 +558,7 @@ impl Block {
 /// Spends [`transparent::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>(
+pub fn fix_generated_transaction<F, E>(
     mut transaction: Transaction,
     tx_index_in_block: usize,
     height: Height,
@@ -570,8 +570,8 @@ where
     F: Fn(
             transparent::OutPoint,
             transparent::CoinbaseSpendRestriction,
-            transparent::OrderedUtxo,
-        ) -> Result<T, E>
+            &transparent::Utxo,
+        ) -> Result<(), E>
         + Copy
         + 'static,
 {
@@ -640,7 +640,7 @@ where
 /// 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>(
+pub fn find_valid_utxo_for_spend<F, E>(
     transaction: &mut Transaction,
     spend_restriction: &mut CoinbaseSpendRestriction,
     spend_height: Height,
@@ -651,8 +651,8 @@ where
     F: Fn(
             transparent::OutPoint,
             transparent::CoinbaseSpendRestriction,
-            transparent::OrderedUtxo,
-        ) -> Result<T, E>
+            &transparent::Utxo,
+        ) -> Result<(), E>
         + Copy
         + 'static,
 {
@@ -674,7 +674,7 @@ where
         if check_transparent_coinbase_spend(
             *candidate_outpoint,
             *spend_restriction,
-            candidate_utxo.clone(),
+            candidate_utxo.as_ref(),
         )
         .is_ok()
         {
@@ -683,7 +683,7 @@ where
             && check_transparent_coinbase_spend(
                 *candidate_outpoint,
                 delete_transparent_outputs,
-                candidate_utxo.clone(),
+                candidate_utxo.as_ref(),
             )
             .is_ok()
         {

zebra-chain/src/transparent/utxo.rs

@@ -54,6 +54,12 @@ pub struct OrderedUtxo {
     pub tx_index_in_block: usize,
 }
 
+impl AsRef<Utxo> for OrderedUtxo {
+    fn as_ref(&self) -> &Utxo {
+        &self.utxo
+    }
+}
+
 impl Utxo {
     /// Create a new UTXO from its fields.
     pub fn new(output: transparent::Output, height: block::Height, from_coinbase: bool) -> Utxo {

zebra-consensus/src/error.rs

@@ -8,7 +8,10 @@
 use chrono::{DateTime, Utc};
 use thiserror::Error;
 
-use zebra_chain::{amount, block, orchard, sapling, sprout, transparent};
+use zebra_chain::{
+    amount, block, orchard, sapling, sprout,
+    transparent::{self, MIN_TRANSPARENT_COINBASE_MATURITY},
+};
 use zebra_state::ValidateContextError;
 
 use crate::{block::MAX_BLOCK_SIGOPS, BoxError};
@@ -187,17 +190,42 @@ pub enum TransactionError {
     #[error("could not validate nullifiers and anchors on best chain")]
     #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))]
     // This error variant is at least 128 bytes
-    ValidateNullifiersAndAnchorsError(Box<ValidateContextError>),
+    ValidateContextError(Box<ValidateContextError>),
 
     #[error("could not validate mempool transaction lock time on best chain")]
     #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))]
     // TODO: turn this into a typed error
     ValidateMempoolLockTimeError(String),
+
+    #[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(
+        "unshielded transparent coinbase spend: {outpoint:?} \
+         must be spent in a transaction which only has shielded outputs"
+    )]
+    #[non_exhaustive]
+    UnshieldedTransparentCoinbaseSpend {
+        outpoint: transparent::OutPoint,
+        min_spend_height: block::Height,
+    },
 }
 
 impl From<ValidateContextError> for TransactionError {
     fn from(err: ValidateContextError) -> Self {
-        TransactionError::ValidateNullifiersAndAnchorsError(Box::new(err))
+        TransactionError::ValidateContextError(Box::new(err))
     }
 }
 

zebra-consensus/src/transaction.rs

@@ -224,10 +224,7 @@ impl Request {
 
     /// Returns true if the request is a mempool request.
     pub fn is_mempool(&self) -> bool {
-        match self {
-            Request::Block { .. } => false,
-            Request::Mempool { .. } => true,
-        }
+        matches!(self, Request::Mempool { .. })
     }
 }
 
@@ -377,6 +374,12 @@ where
                 Self::spent_utxos(tx.clone(), req.known_utxos(), req.is_mempool(), state.clone());
             let (spent_utxos, spent_outputs) = load_spent_utxos_fut.await?;
 
+            // WONTFIX: Return an error for Request::Block as well to replace this check in
+            //       the state once #2336 has been implemented?
+            if req.is_mempool() {
+                Self::check_maturity_height(&req, &spent_utxos)?;
+            }
+
             let cached_ffi_transaction =
                 Arc::new(CachedFfiTransaction::new(tx.clone(), spent_outputs));
 
@@ -420,7 +423,10 @@ where
                         unmined_tx,
                     ))
                     .map(|res| {
-                        assert!(res? == zs::Response::ValidBestChainTipNullifiersAndAnchors, "unexpected response to CheckBestChainTipNullifiersAndAnchors request");
+                        assert!(
+                            res? == zs::Response::ValidBestChainTipNullifiersAndAnchors,
+                            "unexpected response to CheckBestChainTipNullifiersAndAnchors request"
+                        );
                         Ok(())
                     }
                 );
@@ -567,6 +573,28 @@ where
         Ok((spent_utxos, spent_outputs))
     }
 
+    /// Accepts `request`, a transaction verifier [`&Request`](Request),
+    /// and `spent_utxos`, a HashMap of UTXOs in the chain that are spent by this transaction.
+    ///
+    /// Gets the `transaction`, `height`, and `known_utxos` for the request and checks calls
+    /// [`check::tx_transparent_coinbase_spends_maturity`] to verify that every transparent
+    /// coinbase output spent by the transaction will have matured by `height`.
+    ///
+    /// Returns `Ok(())` if every transparent coinbase output spent by the transaction is
+    /// mature and valid for the request height, or a [`TransactionError`] if the transaction
+    /// spends transparent coinbase outputs that are immature and invalid for the request height.
+    pub fn check_maturity_height(
+        request: &Request,
+        spent_utxos: &HashMap<transparent::OutPoint, transparent::Utxo>,
+    ) -> Result<(), TransactionError> {
+        check::tx_transparent_coinbase_spends_maturity(
+            request.transaction(),
+            request.height(),
+            request.known_utxos(),
+            spent_utxos,
+        )
+    }
+
     /// Verify a V4 transaction.
     ///
     /// Returns a set of asynchronous checks that must all succeed for the transaction to be

zebra-consensus/src/transaction/check.rs

@@ -2,7 +2,12 @@
 //!
 //! Code in this file can freely assume that no pre-V4 transactions are present.
 
-use std::{borrow::Cow, collections::HashSet, convert::TryFrom, hash::Hash};
+use std::{
+    borrow::Cow,
+    collections::{HashMap, HashSet},
+    hash::Hash,
+    sync::Arc,
+};
 
 use chrono::{DateTime, Utc};
 
@@ -13,6 +18,7 @@ use zebra_chain::{
     parameters::{Network, NetworkUpgrade},
     primitives::zcash_note_encryption,
     transaction::{LockTime, Transaction},
+    transparent,
 };
 
 use crate::error::TransactionError;
@@ -462,3 +468,29 @@ fn validate_expiry_height_mined(
 
     Ok(())
 }
+
+/// Accepts a transaction, block height, block UTXOs, and
+/// the transaction's spent UTXOs from the chain.
+///
+/// Returns `Ok(())` if spent transparent coinbase outputs are
+/// valid for the block height, or a [`Err(TransactionError)`](TransactionError)
+pub fn tx_transparent_coinbase_spends_maturity(
+    tx: Arc<Transaction>,
+    height: Height,
+    block_new_outputs: Arc<HashMap<transparent::OutPoint, transparent::OrderedUtxo>>,
+    spent_utxos: &HashMap<transparent::OutPoint, transparent::Utxo>,
+) -> Result<(), TransactionError> {
+    for spend in tx.spent_outpoints() {
+        let utxo = block_new_outputs
+            .get(&spend)
+            .map(|ordered_utxo| ordered_utxo.utxo.clone())
+            .or_else(|| spent_utxos.get(&spend).cloned())
+            .expect("load_spent_utxos_fut.await should return an error if a utxo is missing");
+
+        let spend_restriction = tx.coinbase_spend_restriction(height);
+
+        zebra_state::check::transparent_coinbase_spend(spend, spend_restriction, &utxo)?;
+    }
+
+    Ok(())
+}

zebra-consensus/src/transaction/tests.rs

@@ -564,6 +564,104 @@ async fn mempool_request_with_past_lock_time_is_accepted() {
     );
 }
 
+/// Tests that calls to the transaction verifier with a mempool request that spends
+/// immature coinbase outputs will return an error.
+#[tokio::test]
+async fn mempool_request_with_immature_spent_is_rejected() {
+    let _init_guard = zebra_test::init();
+
+    let mut state: MockService<_, _, _, _> = MockService::build().for_prop_tests();
+    let verifier = Verifier::new(Network::Mainnet, state.clone());
+
+    let height = NetworkUpgrade::Canopy
+        .activation_height(Network::Mainnet)
+        .expect("Canopy activation height is specified");
+    let fund_height = (height - 1).expect("fake source fund block height is too small");
+    let (input, output, known_utxos) = mock_transparent_transfer(fund_height, true, 0);
+
+    // Create a non-coinbase V4 tx with the last valid expiry height.
+    let tx = Transaction::V4 {
+        inputs: vec![input],
+        outputs: vec![output],
+        lock_time: LockTime::min_lock_time_timestamp(),
+        expiry_height: height,
+        joinsplit_data: None,
+        sapling_shielded_data: None,
+    };
+
+    let input_outpoint = match tx.inputs()[0] {
+        transparent::Input::PrevOut { outpoint, .. } => outpoint,
+        transparent::Input::Coinbase { .. } => panic!("requires a non-coinbase transaction"),
+    };
+
+    let spend_restriction = tx.coinbase_spend_restriction(height);
+
+    let coinbase_spend_height = Height(5);
+
+    let utxo = known_utxos
+        .get(&input_outpoint)
+        .map(|utxo| {
+            let mut utxo = utxo.utxo.clone();
+            utxo.height = coinbase_spend_height;
+            utxo.from_coinbase = true;
+            utxo
+        })
+        .expect("known_utxos should contain the outpoint");
+
+    let expected_error =
+        zebra_state::check::transparent_coinbase_spend(input_outpoint, spend_restriction, &utxo)
+            .map_err(Box::new)
+            .map_err(TransactionError::ValidateContextError)
+            .expect_err("check should fail");
+
+    tokio::spawn(async move {
+        state
+            .expect_request(zebra_state::Request::BestChainNextMedianTimePast)
+            .await
+            .expect("verifier should call mock state service with correct request")
+            .respond(zebra_state::Response::BestChainNextMedianTimePast(
+                DateTime32::MAX,
+            ));
+
+        state
+            .expect_request(zebra_state::Request::UnspentBestChainUtxo(input_outpoint))
+            .await
+            .expect("verifier should call mock state service with correct request")
+            .respond(zebra_state::Response::UnspentBestChainUtxo(
+                known_utxos.get(&input_outpoint).map(|utxo| {
+                    let mut utxo = utxo.utxo.clone();
+                    utxo.height = coinbase_spend_height;
+                    utxo.from_coinbase = true;
+                    utxo
+                }),
+            ));
+
+        state
+            .expect_request_that(|req| {
+                matches!(
+                    req,
+                    zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_)
+                )
+            })
+            .await
+            .expect("verifier should call mock state service with correct request")
+            .respond(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors);
+    });
+
+    let verifier_response = verifier
+        .oneshot(Request::Mempool {
+            transaction: tx.into(),
+            height,
+        })
+        .await
+        .expect_err("verification of transaction with immature spend should fail");
+
+    assert_eq!(
+        verifier_response, expected_error,
+        "expected to fail verification, got: {verifier_response:?}"
+    );
+}
+
 #[test]
 fn v5_transaction_with_no_outputs_fails_validation() {
     let transaction = fake_v5_transactions_for_network(