change(mempool): Contextually validates mempool transactions in best chain (#5716)

* updates comments

* adds check nullifier no dup fns for transactions

* Adds:
- check::anchors fn for tx iter
- TODO comments for unifying nullifiers and anchors checks
- new state request

Updates unknown anchor errors to accomodate tx-only check

Calls new state fn from transaction verifier

* updates check::anchors fns to use transactions

updates TransactionContextualValidity request to check sprout anchors

adds comment mentioning TransactionContextualValidity ignores UTXOs

* conditions new state req call on is_mempool

updates tests

* fix doc link / lint error

* checks for duplicate nullifiers with closures

* Update zebra-state/src/service/check/nullifier.rs

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

* documents find_duplicate_nullifier params

moves if let statement into for loop

* renames new state req/res

* asserts correct response variant in tx verifier

* adds CheckBestChainTipShieldedSpends call in tx verifier to async checks

* re-adds tracing instrumentation to check::anchors fn

renames transaction_in_state to transaction_in_chain

* adds block/tx wrapper fns for anchors checks

* uses UnminedTx instead of transaction.hash()

deletes broken test

* updates new state req/res name

* updates tests and uses par_iter for anchors checks

* Updates check::anchors pub fn docs.

* Adds:
- comments / docs
- a TransactionError variant for ValidateContextError

* Apply suggestions from code review

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

* moves downcast to From impl

rustfmt

* moves the ValidateContextError into an Arc

updates comments and naming

* leaves par_iter for another PR

* puts io::Error in an Arc

* updates anchors tests to call tx_anchors check

* updates tests to call tx_no_duplicates_in_chain

slightly improves formatting

* Update zebra-consensus/src/error.rs

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

* moves Arc from HistoryError to ValidateContextError

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

zebra-chain/src/block/commitment.rs

@@ -332,7 +332,7 @@ impl FromHex for ChainHistoryBlockTxAuthCommitmentHash {
 /// implement, and ensures that we don't reject blocks or transactions
 /// for a non-enumerated reason.
 #[allow(missing_docs)]
-#[derive(Error, Debug, PartialEq, Eq)]
+#[derive(Error, Clone, Debug, PartialEq, Eq)]
 pub enum CommitmentError {
     #[error(
         "invalid final sapling root: expected {:?}, actual: {:?}",

zebra-consensus/src/error.rs

@@ -9,6 +9,7 @@ use chrono::{DateTime, Utc};
 use thiserror::Error;
 
 use zebra_chain::{amount, block, orchard, sapling, sprout, transparent};
+use zebra_state::ValidateContextError;
 
 use crate::{block::MAX_BLOCK_SIGOPS, BoxError};
 
@@ -180,6 +181,10 @@ pub enum TransactionError {
 
     #[error("could not find a mempool transaction input UTXO in the best chain")]
     TransparentInputNotFound,
+
+    #[error("could not validate nullifiers and anchors on best chain")]
+    #[cfg_attr(any(test, feature = "proptest-impl"), proptest(skip))]
+    ValidateNullifiersAndAnchorsError(#[from] ValidateContextError),
 }
 
 impl From<BoxError> for TransactionError {
@@ -190,6 +195,11 @@ impl From<BoxError> for TransactionError {
             Err(e) => err = e,
         }
 
+        match err.downcast::<ValidateContextError>() {
+            Ok(e) => return (*e).into(),
+            Err(e) => err = e,
+        }
+
         // buffered transaction verifier service error
         match err.downcast::<TransactionError>() {
             Ok(e) => return *e,

zebra-consensus/src/transaction.rs

@@ -175,10 +175,10 @@ impl Request {
     }
 
     /// The unverified mempool transaction, if this is a mempool request.
-    pub fn into_mempool_transaction(self) -> Option<UnminedTx> {
+    pub fn mempool_transaction(&self) -> Option<UnminedTx> {
         match self {
             Request::Block { .. } => None,
-            Request::Mempool { transaction, .. } => Some(transaction),
+            Request::Mempool { transaction, .. } => Some(transaction.clone()),
         }
     }
 
@@ -357,15 +357,16 @@ where
 
             // Load spent UTXOs from state.
             // TODO: Make this a method of `Request` and replace `tx.clone()` with `self.transaction()`?
-            let (spent_utxos, spent_outputs) =
+            let load_spent_utxos_fut =
-                Self::spent_utxos(tx.clone(), req.known_utxos(), req.is_mempool(), state).await?;
+                Self::spent_utxos(tx.clone(), req.known_utxos(), req.is_mempool(), state.clone());
+            let (spent_utxos, spent_outputs) = load_spent_utxos_fut.await?;
 
             let cached_ffi_transaction =
                 Arc::new(CachedFfiTransaction::new(tx.clone(), spent_outputs));
 
             tracing::trace!(?tx_id, "got state UTXOs");
 
-            let async_checks = match tx.as_ref() {
+            let mut async_checks = match tx.as_ref() {
                 Transaction::V1 { .. } | Transaction::V2 { .. } | Transaction::V3 { .. } => {
                     tracing::debug!(?tx, "got transaction with wrong version");
                     return Err(TransactionError::WrongVersion);
@@ -396,6 +397,21 @@ where
                 )?,
             };
 
+            if let Some(unmined_tx) = req.mempool_transaction() {
+                let check_anchors_and_revealed_nullifiers_query = state
+                    .clone()
+                    .oneshot(zs::Request::CheckBestChainTipNullifiersAndAnchors(
+                        unmined_tx,
+                    ))
+                    .map(|res| {
+                        assert!(res? == zs::Response::ValidBestChainTipNullifiersAndAnchors, "unexpected response to CheckBestChainTipNullifiersAndAnchors request");
+                        Ok(())
+                    }
+                );
+
+                async_checks.push(check_anchors_and_revealed_nullifiers_query);
+            }
+
             tracing::trace!(?tx_id, "awaiting async checks...");
 
             // If the Groth16 parameter download hangs,

zebra-consensus/src/transaction/tests.rs

@@ -189,17 +189,28 @@ async fn mempool_request_with_missing_input_is_rejected() {
         .find(|(_, tx)| !(tx.is_coinbase() || tx.inputs().is_empty()))
         .expect("At least one non-coinbase transaction with transparent inputs in test vectors");
 
-    let expected_state_request = zebra_state::Request::UnspentBestChainUtxo(match tx.inputs()[0] {
+    let input_outpoint = match tx.inputs()[0] {
         transparent::Input::PrevOut { outpoint, .. } => outpoint,
         transparent::Input::Coinbase { .. } => panic!("requires a non-coinbase transaction"),
-    });
+    };
 
     tokio::spawn(async move {
         state
-            .expect_request(expected_state_request)
+            .expect_request(zebra_state::Request::UnspentBestChainUtxo(input_outpoint))
             .await
             .expect("verifier should call mock state service")
             .respond(zebra_state::Response::UnspentBestChainUtxo(None));
+
+        state
+            .expect_request_that(|req| {
+                matches!(
+                    req,
+                    zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_)
+                )
+            })
+            .await
+            .expect("verifier should call mock state service")
+            .respond(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors);
     });
 
     let verifier_response = verifier
@@ -251,6 +262,17 @@ async fn mempool_request_with_present_input_is_accepted() {
                     .get(&input_outpoint)
                     .map(|utxo| utxo.utxo.clone()),
             ));
+
+        state
+            .expect_request_that(|req| {
+                matches!(
+                    req,
+                    zebra_state::Request::CheckBestChainTipNullifiersAndAnchors(_)
+                )
+            })
+            .await
+            .expect("verifier should call mock state service")
+            .respond(zebra_state::Response::ValidBestChainTipNullifiersAndAnchors);
     });
 
     let verifier_response = verifier

zebra-state/src/error.rs

@@ -47,7 +47,7 @@ pub type BoxError = Box<dyn std::error::Error + Send + Sync + 'static>;
 pub struct CommitBlockError(#[from] ValidateContextError);
 
 /// An error describing why a block failed contextual validation.
-#[derive(Debug, Error, PartialEq, Eq)]
+#[derive(Debug, Error, Clone, PartialEq, Eq)]
 #[non_exhaustive]
 #[allow(missing_docs)]
 pub enum ValidateContextError {
@@ -224,7 +224,7 @@ pub enum ValidateContextError {
     NoteCommitmentTreeError(#[from] zebra_chain::parallel::tree::NoteCommitmentTreeError),
 
     #[error("error building the history tree")]
-    HistoryTreeError(#[from] HistoryTreeError),
+    HistoryTreeError(#[from] Arc<HistoryTreeError>),
 
     #[error("block contains an invalid commitment")]
     InvalidBlockCommitment(#[from] block::CommitmentError),
@@ -236,8 +236,8 @@ pub enum ValidateContextError {
     #[non_exhaustive]
     UnknownSproutAnchor {
         anchor: sprout::tree::Root,
-        height: block::Height,
+        height: Option<block::Height>,
-        tx_index_in_block: usize,
+        tx_index_in_block: Option<usize>,
         transaction_hash: transaction::Hash,
     },
 
@@ -248,8 +248,8 @@ pub enum ValidateContextError {
     #[non_exhaustive]
     UnknownSaplingAnchor {
         anchor: sapling::tree::Root,
-        height: block::Height,
+        height: Option<block::Height>,
-        tx_index_in_block: usize,
+        tx_index_in_block: Option<usize>,
         transaction_hash: transaction::Hash,
     },
 
@@ -260,8 +260,8 @@ pub enum ValidateContextError {
     #[non_exhaustive]
     UnknownOrchardAnchor {
         anchor: orchard::tree::Root,
-        height: block::Height,
+        height: Option<block::Height>,
-        tx_index_in_block: usize,
+        tx_index_in_block: Option<usize>,
         transaction_hash: transaction::Hash,
     },
 }

zebra-state/src/request.rs

@@ -14,7 +14,8 @@ use zebra_chain::{
     parallel::tree::NoteCommitmentTrees,
     sapling,
     serialization::SerializationError,
-    sprout, transaction,
+    sprout,
+    transaction::{self, UnminedTx},
     transparent::{self, utxos_from_ordered_utxos},
     value_balance::{ValueBalance, ValueBalanceError},
 };
@@ -539,6 +540,11 @@ pub enum Request {
         /// Optionally, the hash of the last header to request.
         stop: Option<block::Hash>,
     },
+
+    /// Contextually validates anchors and nullifiers of a transaction on the best chain
+    ///
+    /// Returns [`Response::ValidBestChainTipNullifiersAndAnchors`]
+    CheckBestChainTipNullifiersAndAnchors(UnminedTx),
 }
 
 impl Request {
@@ -555,6 +561,9 @@ impl Request {
             Request::Block(_) => "block",
             Request::FindBlockHashes { .. } => "find_block_hashes",
             Request::FindBlockHeaders { .. } => "find_block_headers",
+            Request::CheckBestChainTipNullifiersAndAnchors(_) => {
+                "best_chain_tip_nullifiers_anchors"
+            }
         }
     }
 
@@ -736,6 +745,11 @@ pub enum ReadRequest {
     /// Returns a type with found utxos and transaction information.
     UtxosByAddresses(HashSet<transparent::Address>),
 
+    /// Contextually validates anchors and nullifiers of a transaction on the best chain
+    ///
+    /// Returns [`ReadResponse::ValidBestChainTipNullifiersAndAnchors`].
+    CheckBestChainTipNullifiersAndAnchors(UnminedTx),
+
     #[cfg(feature = "getblocktemplate-rpcs")]
     /// Looks up a block hash by height in the current best chain.
     ///
@@ -772,6 +786,9 @@ impl ReadRequest {
             ReadRequest::AddressBalance { .. } => "address_balance",
             ReadRequest::TransactionIdsByAddresses { .. } => "transaction_ids_by_addesses",
             ReadRequest::UtxosByAddresses(_) => "utxos_by_addesses",
+            ReadRequest::CheckBestChainTipNullifiersAndAnchors(_) => {
+                "best_chain_tip_nullifiers_anchors"
+            }
             #[cfg(feature = "getblocktemplate-rpcs")]
             ReadRequest::BestChainBlockHash(_) => "best_chain_block_hash",
             #[cfg(feature = "getblocktemplate-rpcs")]
@@ -815,6 +832,10 @@ impl TryFrom<Request> for ReadRequest {
                 Ok(ReadRequest::FindBlockHeaders { known_blocks, stop })
             }
 
+            Request::CheckBestChainTipNullifiersAndAnchors(tx) => {
+                Ok(ReadRequest::CheckBestChainTipNullifiersAndAnchors(tx))
+            }
+
             Request::CommitBlock(_) | Request::CommitFinalizedBlock(_) => {
                 Err("ReadService does not write blocks")
             }

zebra-state/src/response.rs

@@ -54,6 +54,11 @@ pub enum Response {
 
     /// The response to a `FindBlockHeaders` request.
     BlockHeaders(Vec<block::CountedHeader>),
+
+    /// Response to [`Request::CheckBestChainTipNullifiersAndAnchors`].
+    ///
+    /// Does not check transparent UTXO inputs
+    ValidBestChainTipNullifiersAndAnchors,
 }
 
 #[derive(Clone, Debug, PartialEq, Eq)]
@@ -114,6 +119,11 @@ pub enum ReadResponse {
     /// Response to [`ReadRequest::UtxosByAddresses`] with found utxos and transaction data.
     AddressUtxos(AddressUtxos),
 
+    /// Response to [`ReadRequest::CheckBestChainTipNullifiersAndAnchors`].
+    ///
+    /// Does not check transparent UTXO inputs
+    ValidBestChainTipNullifiersAndAnchors,
+
     #[cfg(feature = "getblocktemplate-rpcs")]
     /// Response to [`ReadRequest::BestChainBlockHash`](crate::ReadRequest::BestChainBlockHash) with the
     /// specified block hash.
@@ -171,6 +181,8 @@ impl TryFrom<ReadResponse> for Response {
             ReadResponse::BlockHashes(hashes) => Ok(Response::BlockHashes(hashes)),
             ReadResponse::BlockHeaders(headers) => Ok(Response::BlockHeaders(headers)),
 
+            ReadResponse::ValidBestChainTipNullifiersAndAnchors => Ok(Response::ValidBestChainTipNullifiersAndAnchors),
+
             ReadResponse::TransactionIdsForBlock(_)
             | ReadResponse::SaplingTree(_)
             | ReadResponse::OrchardTree(_)

zebra-state/src/service.rs

@@ -1024,7 +1024,8 @@ impl Service<Request> for StateService {
             | Request::UnspentBestChainUtxo(_)
             | Request::Block(_)
             | Request::FindBlockHashes { .. }
-            | Request::FindBlockHeaders { .. } => {
+            | Request::FindBlockHeaders { .. }
+            | Request::CheckBestChainTipNullifiersAndAnchors(_) => {
                 // Redirect the request to the concurrent ReadStateService
                 let read_service = self.read_service.clone();
 
@@ -1217,7 +1218,6 @@ impl Service<ReadRequest> for ReadStateService {
                 .boxed()
             }
 
-            // Currently unused.
             ReadRequest::UnspentBestChainUtxo(outpoint) => {
                 let timer = CodeTimer::start();
 
@@ -1519,6 +1519,39 @@ impl Service<ReadRequest> for ReadStateService {
                 .boxed()
             }
 
+            ReadRequest::CheckBestChainTipNullifiersAndAnchors(unmined_tx) => {
+                let timer = CodeTimer::start();
+
+                let state = self.clone();
+
+                let span = Span::current();
+                tokio::task::spawn_blocking(move || {
+                    span.in_scope(move || {
+                        let latest_non_finalized_best_chain =
+                            state.latest_non_finalized_state().best_chain().cloned();
+
+                        check::nullifier::tx_no_duplicates_in_chain(
+                            &state.db,
+                            latest_non_finalized_best_chain.as_ref(),
+                            &unmined_tx.transaction,
+                        )?;
+
+                        check::anchors::tx_anchors_refer_to_final_treestates(
+                            &state.db,
+                            latest_non_finalized_best_chain.as_ref(),
+                            &unmined_tx,
+                        )?;
+
+                        // The work is done in the future.
+                        timer.finish(module_path!(), line!(), "ReadRequest::UnspentBestChainUtxo");
+
+                        Ok(ReadResponse::ValidBestChainTipNullifiersAndAnchors)
+                    })
+                })
+                .map(|join_result| join_result.expect("panic in ReadRequest::UnspentBestChainUtxo"))
+                .boxed()
+            }
+
             // Used by get_block_hash RPC.
             #[cfg(feature = "getblocktemplate-rpcs")]
             ReadRequest::BestChainBlockHash(height) => {

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

@@ -5,7 +5,8 @@ use std::{collections::HashMap, sync::Arc};
 
 use zebra_chain::{
     block::{Block, Height},
-    sprout, transaction,
+    sprout,
+    transaction::{Hash as TransactionHash, Transaction, UnminedTx},
 };
 
 use crate::{
@@ -13,18 +14,19 @@ use crate::{
     PreparedBlock, ValidateContextError,
 };
 
-/// Checks the final Sapling and Orchard anchors specified by transactions in this
+/// Checks the final Sapling and Orchard anchors specified by `transaction`
-/// `prepared` block.
 ///
 /// This method checks for anchors computed from the final treestate of each block in
 /// the `parent_chain` or `finalized_state`.
-#[tracing::instrument(skip(finalized_state, parent_chain, prepared))]
+#[tracing::instrument(skip(finalized_state, parent_chain, transaction))]
-pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates(
+fn sapling_orchard_anchors_refer_to_final_treestates(
     finalized_state: &ZebraDb,
-    parent_chain: &Chain,
+    parent_chain: Option<&Arc<Chain>>,
-    prepared: &PreparedBlock,
+    transaction: &Arc<Transaction>,
+    transaction_hash: TransactionHash,
+    tx_index_in_block: Option<usize>,
+    height: Option<Height>,
 ) -> Result<(), ValidateContextError> {
-    for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() {
     // Sapling Spends
     //
     // MUST refer to some earlier block’s final Sapling treestate.
@@ -47,18 +49,20 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates(
             ?anchor,
             ?anchor_index_in_tx,
             ?tx_index_in_block,
-                height = ?prepared.height,
+            ?height,
             "observed sapling anchor",
         );
 
-            if !parent_chain.sapling_anchors.contains(&anchor)
+        if !parent_chain
+            .map(|chain| chain.sapling_anchors.contains(&anchor))
+            .unwrap_or(false)
             && !finalized_state.contains_sapling_anchor(&anchor)
         {
             return Err(ValidateContextError::UnknownSaplingAnchor {
                 anchor,
-                    height: prepared.height,
+                height,
                 tx_index_in_block,
-                    transaction_hash: prepared.transaction_hashes[tx_index_in_block],
+                transaction_hash,
             });
         }
 
@@ -66,7 +70,7 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates(
             ?anchor,
             ?anchor_index_in_tx,
             ?tx_index_in_block,
-                height = ?prepared.height,
+            ?height,
             "validated sapling anchor",
         );
     }
@@ -86,31 +90,34 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates(
         tracing::debug!(
             ?orchard_shielded_data.shared_anchor,
             ?tx_index_in_block,
-                height = ?prepared.height,
+            ?height,
             "observed orchard anchor",
         );
 
         if !parent_chain
+            .map(|chain| {
+                chain
                     .orchard_anchors
                     .contains(&orchard_shielded_data.shared_anchor)
+            })
+            .unwrap_or(false)
             && !finalized_state.contains_orchard_anchor(&orchard_shielded_data.shared_anchor)
         {
             return Err(ValidateContextError::UnknownOrchardAnchor {
                 anchor: orchard_shielded_data.shared_anchor,
-                    height: prepared.height,
+                height,
                 tx_index_in_block,
-                    transaction_hash: prepared.transaction_hashes[tx_index_in_block],
+                transaction_hash,
             });
         }
 
         tracing::debug!(
             ?orchard_shielded_data.shared_anchor,
             ?tx_index_in_block,
-                height = ?prepared.height,
+            ?height,
             "validated orchard anchor",
         );
     }
-    }
 
     Ok(())
 }
@@ -122,31 +129,28 @@ pub(crate) fn sapling_orchard_anchors_refer_to_final_treestates(
 /// Sprout anchors may also refer to the interstitial output treestate of any prior
 /// `JoinSplit` _within the same transaction_; these are created on the fly
 /// in [`sprout_anchors_refer_to_treestates()`].
-#[tracing::instrument(skip(finalized_state, parent_chain, prepared))]
+#[tracing::instrument(skip(sprout_final_treestates, finalized_state, parent_chain, transaction))]
-pub(crate) fn fetch_sprout_final_treestates(
+fn fetch_sprout_final_treestates(
+    sprout_final_treestates: &mut HashMap<
+        sprout::tree::Root,
+        Arc<sprout::tree::NoteCommitmentTree>,
+    >,
     finalized_state: &ZebraDb,
-    parent_chain: &Chain,
+    parent_chain: Option<&Arc<Chain>>,
-    prepared: &PreparedBlock,
+    transaction: &Arc<Transaction>,
-) -> HashMap<sprout::tree::Root, Arc<sprout::tree::NoteCommitmentTree>> {
+    tx_index_in_block: Option<usize>,
-    let mut sprout_final_treestates = HashMap::new();
+    height: Option<Height>,
-
+) {
-    for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() {
     // Fetch and return Sprout JoinSplit final treestates
-        for (joinsplit_index_in_tx, joinsplit) in
+    for (joinsplit_index_in_tx, joinsplit) in transaction.sprout_groth16_joinsplits().enumerate() {
-            transaction.sprout_groth16_joinsplits().enumerate()
-        {
         // Avoid duplicate fetches
         if sprout_final_treestates.contains_key(&joinsplit.anchor) {
             continue;
         }
 
         let input_tree = parent_chain
-                .sprout_trees_by_anchor
+            .and_then(|chain| chain.sprout_trees_by_anchor.get(&joinsplit.anchor).cloned())
-                .get(&joinsplit.anchor)
+            .or_else(|| finalized_state.sprout_note_commitment_tree_by_anchor(&joinsplit.anchor));
-                .cloned()
-                .or_else(|| {
-                    finalized_state.sprout_note_commitment_tree_by_anchor(&joinsplit.anchor)
-                });
 
         if let Some(input_tree) = input_tree {
             /* TODO:
@@ -172,24 +176,21 @@ pub(crate) fn fetch_sprout_final_treestates(
                 ?joinsplit.anchor,
                 ?joinsplit_index_in_tx,
                 ?tx_index_in_block,
-                    height = ?prepared.height,
+                ?height,
                 "observed sprout final treestate anchor",
             );
         }
     }
-    }
 
     tracing::trace!(
         sprout_final_treestate_count = ?sprout_final_treestates.len(),
         ?sprout_final_treestates,
-        height = ?prepared.height,
+        ?height,
         "returning sprout final treestate anchors",
     );
-
-    sprout_final_treestates
 }
 
-/// Checks the Sprout anchors specified by transactions in `block`.
+/// Checks the Sprout anchors specified by `transactions`.
 ///
 /// Sprout anchors may refer to some earlier block's final treestate (like
 /// Sapling and Orchard do exclusively) _or_ to the interstitial output
@@ -199,33 +200,21 @@ pub(crate) fn fetch_sprout_final_treestates(
 /// (which must be populated with all treestates pointed to in the `prepared` block;
 /// see [`fetch_sprout_final_treestates()`]); or in the interstitial
 /// treestates which are computed on the fly in this function.
-#[tracing::instrument(skip(sprout_final_treestates, block, transaction_hashes))]
+#[tracing::instrument(skip(sprout_final_treestates, transaction))]
-pub(crate) fn sprout_anchors_refer_to_treestates(
+fn sprout_anchors_refer_to_treestates(
-    sprout_final_treestates: HashMap<sprout::tree::Root, Arc<sprout::tree::NoteCommitmentTree>>,
+    sprout_final_treestates: &HashMap<sprout::tree::Root, Arc<sprout::tree::NoteCommitmentTree>>,
-    block: Arc<Block>,
+    transaction: &Arc<Transaction>,
-    // Only used for debugging
+    transaction_hash: TransactionHash,
-    height: Height,
+    tx_index_in_block: Option<usize>,
-    transaction_hashes: Arc<[transaction::Hash]>,
+    height: Option<Height>,
 ) -> Result<(), ValidateContextError> {
-    tracing::trace!(
-        sprout_final_treestate_count = ?sprout_final_treestates.len(),
-        ?sprout_final_treestates,
-        ?height,
-        "received sprout final treestate anchors",
-    );
-
-    for (tx_index_in_block, transaction) in block.transactions.iter().enumerate() {
     // Sprout JoinSplits, with interstitial treestates to check as well.
-        let mut interstitial_trees: HashMap<
+    let mut interstitial_trees: HashMap<sprout::tree::Root, Arc<sprout::tree::NoteCommitmentTree>> =
-            sprout::tree::Root,
+        HashMap::new();
-            Arc<sprout::tree::NoteCommitmentTree>,
-        > = HashMap::new();
 
     let joinsplit_count = transaction.sprout_groth16_joinsplits().count();
 
-        for (joinsplit_index_in_tx, joinsplit) in
+    for (joinsplit_index_in_tx, joinsplit) in transaction.sprout_groth16_joinsplits().enumerate() {
-            transaction.sprout_groth16_joinsplits().enumerate()
-        {
         // Check all anchor sets, including the one for interstitial
         // anchors.
         //
@@ -288,7 +277,7 @@ pub(crate) fn sprout_anchors_refer_to_treestates(
                     anchor: joinsplit.anchor,
                     height,
                     tx_index_in_block,
-                        transaction_hash: transaction_hashes[tx_index_in_block],
+                    transaction_hash,
                 });
             }
         };
@@ -326,7 +315,159 @@ pub(crate) fn sprout_anchors_refer_to_treestates(
             "observed sprout interstitial anchor",
         );
     }
-    }
+
+    Ok(())
+}
+
+/// Accepts a [`ZebraDb`], [`Chain`], and [`PreparedBlock`].
+///
+/// Iterates over the transactions in the [`PreparedBlock`] checking the final Sapling and Orchard anchors.
+///
+/// This method checks for anchors computed from the final treestate of each block in
+/// the `parent_chain` or `finalized_state`.
+#[tracing::instrument(skip_all)]
+pub(crate) fn block_sapling_orchard_anchors_refer_to_final_treestates(
+    finalized_state: &ZebraDb,
+    parent_chain: &Arc<Chain>,
+    prepared: &PreparedBlock,
+) -> Result<(), ValidateContextError> {
+    prepared.block.transactions.iter().enumerate().try_for_each(
+        |(tx_index_in_block, transaction)| {
+            sapling_orchard_anchors_refer_to_final_treestates(
+                finalized_state,
+                Some(parent_chain),
+                transaction,
+                prepared.transaction_hashes[tx_index_in_block],
+                Some(tx_index_in_block),
+                Some(prepared.height),
+            )
+        },
+    )
+}
+
+/// Accepts a [`ZebraDb`], [`Arc<Chain>`](Chain), and [`PreparedBlock`].
+///
+/// Iterates over the transactions in the [`PreparedBlock`], and fetches the Sprout final treestates
+/// from the state.
+///
+/// Returns a `HashMap` of the Sprout final treestates from the state for [`sprout_anchors_refer_to_treestates()`]
+/// to check Sprout final and interstitial treestates without accessing the disk.
+///
+/// Sprout anchors may also refer to the interstitial output treestate of any prior
+/// `JoinSplit` _within the same transaction_; these are created on the fly
+/// in [`sprout_anchors_refer_to_treestates()`].
+#[tracing::instrument(skip_all)]
+pub(crate) fn block_fetch_sprout_final_treestates(
+    finalized_state: &ZebraDb,
+    parent_chain: &Arc<Chain>,
+    prepared: &PreparedBlock,
+) -> HashMap<sprout::tree::Root, Arc<sprout::tree::NoteCommitmentTree>> {
+    let mut sprout_final_treestates = HashMap::new();
+
+    for (tx_index_in_block, transaction) in prepared.block.transactions.iter().enumerate() {
+        fetch_sprout_final_treestates(
+            &mut sprout_final_treestates,
+            finalized_state,
+            Some(parent_chain),
+            transaction,
+            Some(tx_index_in_block),
+            Some(prepared.height),
+        );
+    }
+
+    sprout_final_treestates
+}
+
+/// Accepts a [`ZebraDb`], [`Arc<Chain>`](Chain), [`Arc<Block>`](Block), and an
+/// [`Arc<[transaction::Hash]>`](TransactionHash) of hashes corresponding to the transactions in [`Block`]
+///
+/// Iterates over the transactions in the [`Block`] checking the final Sprout anchors.
+///
+/// Sprout anchors may refer to some earlier block's final treestate (like
+/// Sapling and Orchard do exclusively) _or_ to the interstitial output
+/// treestate of any prior `JoinSplit` _within the same transaction_.
+///
+/// This method searches for anchors in the supplied `sprout_final_treestates`
+/// (which must be populated with all treestates pointed to in the `prepared` block;
+/// see [`fetch_sprout_final_treestates()`]); or in the interstitial
+/// treestates which are computed on the fly in this function.
+#[tracing::instrument(skip(sprout_final_treestates, block, transaction_hashes))]
+pub(crate) fn block_sprout_anchors_refer_to_treestates(
+    sprout_final_treestates: HashMap<sprout::tree::Root, Arc<sprout::tree::NoteCommitmentTree>>,
+    block: Arc<Block>,
+    // Only used for debugging
+    transaction_hashes: Arc<[TransactionHash]>,
+    height: Height,
+) -> Result<(), ValidateContextError> {
+    tracing::trace!(
+        sprout_final_treestate_count = ?sprout_final_treestates.len(),
+        ?sprout_final_treestates,
+        ?height,
+        "received sprout final treestate anchors",
+    );
+
+    block
+        .transactions
+        .iter()
+        .enumerate()
+        .try_for_each(|(tx_index_in_block, transaction)| {
+            sprout_anchors_refer_to_treestates(
+                &sprout_final_treestates,
+                transaction,
+                transaction_hashes[tx_index_in_block],
+                Some(tx_index_in_block),
+                Some(height),
+            )?;
+
+            Ok(())
+        })
+}
+
+/// Accepts a [`ZebraDb`], an optional [`Option<Arc<Chain>>`](Chain), and an [`UnminedTx`].
+///
+/// Checks the final Sprout, Sapling and Orchard anchors specified in the [`UnminedTx`].
+///
+/// This method checks for anchors computed from the final treestate of each block in
+/// the `parent_chain` or `finalized_state`.
+#[tracing::instrument(skip_all)]
+pub(crate) fn tx_anchors_refer_to_final_treestates(
+    finalized_state: &ZebraDb,
+    parent_chain: Option<&Arc<Chain>>,
+    unmined_tx: &UnminedTx,
+) -> Result<(), ValidateContextError> {
+    sapling_orchard_anchors_refer_to_final_treestates(
+        finalized_state,
+        parent_chain,
+        &unmined_tx.transaction,
+        unmined_tx.id.mined_id(),
+        None,
+        None,
+    )?;
+
+    let mut sprout_final_treestates = HashMap::new();
+
+    fetch_sprout_final_treestates(
+        &mut sprout_final_treestates,
+        finalized_state,
+        parent_chain,
+        &unmined_tx.transaction,
+        None,
+        None,
+    );
+
+    tracing::trace!(
+        sprout_final_treestate_count = ?sprout_final_treestates.len(),
+        ?sprout_final_treestates,
+        "received sprout final treestate anchors",
+    );
+
+    sprout_anchors_refer_to_treestates(
+        &sprout_final_treestates,
+        &unmined_tx.transaction,
+        unmined_tx.id.mined_id(),
+        None,
+        None,
+    )?;
 
     Ok(())
 }

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

@@ -1,12 +1,14 @@
 //! Checks for nullifier uniqueness.
 
-use std::collections::HashSet;
+use std::{collections::HashSet, sync::Arc};
 
 use tracing::trace;
+use zebra_chain::transaction::Transaction;
 
 use crate::{
-    error::DuplicateNullifierError, service::finalized_state::ZebraDb, PreparedBlock,
+    error::DuplicateNullifierError,
-    ValidateContextError,
+    service::{finalized_state::ZebraDb, non_finalized_state::Chain},
+    PreparedBlock, ValidateContextError,
 };
 
 // Tidy up some doc links
@@ -54,6 +56,73 @@ pub(crate) fn no_duplicates_in_finalized_chain(
     Ok(())
 }
 
+/// Accepts an iterator of revealed nullifiers, a predicate fn for checking if a nullifier is in
+/// in the finalized chain, and a predicate fn for checking if the nullifier is in the non-finalized chain
+///
+/// Returns `Err(DuplicateNullifierError)` if any of the `revealed_nullifiers` are found in the
+/// non-finalized or finalized chains.
+///
+/// Returns `Ok(())` if all the `revealed_nullifiers` have not been seen in either chain.
+fn find_duplicate_nullifier<'a, NullifierT, FinalizedStateContainsFn, NonFinalizedStateContainsFn>(
+    revealed_nullifiers: impl IntoIterator<Item = &'a NullifierT>,
+    finalized_chain_contains: FinalizedStateContainsFn,
+    non_finalized_chain_contains: Option<NonFinalizedStateContainsFn>,
+) -> Result<(), ValidateContextError>
+where
+    NullifierT: DuplicateNullifierError + 'a,
+    FinalizedStateContainsFn: Fn(&'a NullifierT) -> bool,
+    NonFinalizedStateContainsFn: Fn(&'a NullifierT) -> bool,
+{
+    for nullifier in revealed_nullifiers {
+        if let Some(true) = non_finalized_chain_contains.as_ref().map(|f| f(nullifier)) {
+            Err(nullifier.duplicate_nullifier_error(false))?
+        } else if finalized_chain_contains(nullifier) {
+            Err(nullifier.duplicate_nullifier_error(true))?
+        }
+    }
+
+    Ok(())
+}
+
+/// Reject double-spends of nullifiers:
+/// - one from this [`Transaction`], and the other already committed to the
+///   provided non-finalized [`Chain`] or [`ZebraDb`].
+///
+/// # Consensus
+///
+/// > A nullifier MUST NOT repeat either within a transaction,
+/// > or across transactions in a valid blockchain.
+/// > Sprout and Sapling and Orchard nullifiers are considered disjoint,
+/// > even if they have the same bit pattern.
+///
+/// <https://zips.z.cash/protocol/protocol.pdf#nullifierset>
+#[tracing::instrument(skip_all)]
+pub(crate) fn tx_no_duplicates_in_chain(
+    finalized_chain: &ZebraDb,
+    non_finalized_chain: Option<&Arc<Chain>>,
+    transaction: &Arc<Transaction>,
+) -> Result<(), ValidateContextError> {
+    find_duplicate_nullifier(
+        transaction.sprout_nullifiers(),
+        |nullifier| finalized_chain.contains_sprout_nullifier(nullifier),
+        non_finalized_chain.map(|chain| |nullifier| chain.sprout_nullifiers.contains(nullifier)),
+    )?;
+
+    find_duplicate_nullifier(
+        transaction.sapling_nullifiers(),
+        |nullifier| finalized_chain.contains_sapling_nullifier(nullifier),
+        non_finalized_chain.map(|chain| |nullifier| chain.sapling_nullifiers.contains(nullifier)),
+    )?;
+
+    find_duplicate_nullifier(
+        transaction.orchard_nullifiers(),
+        |nullifier| finalized_chain.contains_orchard_nullifier(nullifier),
+        non_finalized_chain.map(|chain| |nullifier| chain.orchard_nullifiers.contains(nullifier)),
+    )?;
+
+    Ok(())
+}
+
 /// Reject double-spends of nullifers:
 /// - both within the same `JoinSplit` (sprout only),
 /// - from different `JoinSplit`s, [`sapling::Spend`][2]s or

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

@@ -8,14 +8,17 @@ use zebra_chain::{
     primitives::Groth16Proof,
     serialization::ZcashDeserializeInto,
     sprout::JoinSplit,
-    transaction::{JoinSplitData, LockTime, Transaction},
+    transaction::{JoinSplitData, LockTime, Transaction, UnminedTx},
 };
 
 use crate::{
     arbitrary::Prepare,
-    service::write::validate_and_commit_non_finalized,
+    service::{
+        check::anchors::tx_anchors_refer_to_final_treestates,
+        write::validate_and_commit_non_finalized,
+    },
     tests::setup::{new_state_with_mainnet_genesis, transaction_v4_from_coinbase},
-    PreparedBlock,
+    PreparedBlock, ValidateContextError,
 };
 
 // Sprout
@@ -41,13 +44,6 @@ fn check_sprout_anchors() {
     // Add initial transactions to [`block_1`].
     let block_1 = prepare_sprout_block(block_1, block_395);
 
-    // Validate and commit [`block_1`]. This will add an anchor referencing the
-    // empty note commitment tree to the state.
-    assert!(
-        validate_and_commit_non_finalized(&finalized_state, &mut non_finalized_state, block_1)
-            .is_ok()
-    );
-
     // Bootstrap a block at height == 2 that references the Sprout note commitment tree state
     // from [`block_1`].
     let block_2 = zebra_test::vectors::BLOCK_MAINNET_2_BYTES
@@ -63,6 +59,43 @@ fn check_sprout_anchors() {
     // Add the transactions with the first anchors to [`block_2`].
     let block_2 = prepare_sprout_block(block_2, block_396);
 
+    let unmined_txs: Vec<_> = block_2
+        .block
+        .transactions
+        .iter()
+        .map(UnminedTx::from)
+        .collect();
+
+    let check_unmined_tx_anchors_result = unmined_txs.iter().try_for_each(|unmined_tx| {
+        tx_anchors_refer_to_final_treestates(
+            &finalized_state.db,
+            non_finalized_state.best_chain(),
+            unmined_tx,
+        )
+    });
+
+    assert!(matches!(
+        check_unmined_tx_anchors_result,
+        Err(ValidateContextError::UnknownSproutAnchor { .. })
+    ));
+
+    // Validate and commit [`block_1`]. This will add an anchor referencing the
+    // empty note commitment tree to the state.
+    assert!(
+        validate_and_commit_non_finalized(&finalized_state, &mut non_finalized_state, block_1)
+            .is_ok()
+    );
+
+    let check_unmined_tx_anchors_result = unmined_txs.iter().try_for_each(|unmined_tx| {
+        tx_anchors_refer_to_final_treestates(
+            &finalized_state.db,
+            non_finalized_state.best_chain(),
+            unmined_tx,
+        )
+    });
+
+    assert!(check_unmined_tx_anchors_result.is_ok());
+
     // Validate and commit [`block_2`]. This will also check the anchors.
     assert_eq!(
         validate_and_commit_non_finalized(&finalized_state, &mut non_finalized_state, block_2),
@@ -188,10 +221,6 @@ fn check_sapling_anchors() {
         });
 
     let block1 = Arc::new(block1).prepare();
-    assert!(
-        validate_and_commit_non_finalized(&finalized_state, &mut non_finalized_state, block1)
-            .is_ok()
-    );
 
     // Bootstrap a block at height == 2 that references the Sapling note commitment tree state
     // from earlier block
@@ -238,6 +267,42 @@ fn check_sapling_anchors() {
         });
 
     let block2 = Arc::new(block2).prepare();
+
+    let unmined_txs: Vec<_> = block2
+        .block
+        .transactions
+        .iter()
+        .map(UnminedTx::from)
+        .collect();
+
+    let check_unmined_tx_anchors_result = unmined_txs.iter().try_for_each(|unmined_tx| {
+        tx_anchors_refer_to_final_treestates(
+            &finalized_state.db,
+            non_finalized_state.best_chain(),
+            unmined_tx,
+        )
+    });
+
+    assert!(matches!(
+        check_unmined_tx_anchors_result,
+        Err(ValidateContextError::UnknownSaplingAnchor { .. })
+    ));
+
+    assert!(
+        validate_and_commit_non_finalized(&finalized_state, &mut non_finalized_state, block1)
+            .is_ok()
+    );
+
+    let check_unmined_tx_anchors_result = unmined_txs.iter().try_for_each(|unmined_tx| {
+        tx_anchors_refer_to_final_treestates(
+            &finalized_state.db,
+            non_finalized_state.best_chain(),
+            unmined_tx,
+        )
+    });
+
+    assert!(check_unmined_tx_anchors_result.is_ok());
+
     assert_eq!(
         validate_and_commit_non_finalized(&finalized_state, &mut non_finalized_state, block2),
         Ok(())

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

@@ -19,7 +19,9 @@ use zebra_chain::{
 
 use crate::{
     arbitrary::Prepare,
-    service::{read, write::validate_and_commit_non_finalized},
+    service::{
+        check::nullifier::tx_no_duplicates_in_chain, read, write::validate_and_commit_non_finalized,
+    },
     tests::setup::{new_state_with_mainnet_genesis, transaction_v4_from_coinbase},
     FinalizedBlock,
     ValidateContextError::{
@@ -155,7 +157,9 @@ proptest! {
         let block1 = Arc::new(block1).prepare();
         let commit_result = validate_and_commit_non_finalized(
             &finalized_state,
-            &mut non_finalized_state,  block1);
+            &mut non_finalized_state,
+            block1
+        );
 
         // if the random proptest data produces other errors,
         // we might need to just check `is_err()` here
@@ -214,7 +218,9 @@ proptest! {
         let block1 = Arc::new(block1).prepare();
         let commit_result = validate_and_commit_non_finalized(
             &finalized_state,
-            &mut non_finalized_state,  block1);
+            &mut non_finalized_state,
+            block1
+        );
 
         prop_assert_eq!(
             commit_result,
@@ -318,13 +324,13 @@ proptest! {
         let duplicate_nullifier = joinsplit1.nullifiers[0];
         joinsplit2.nullifiers[0] = duplicate_nullifier;
 
-        let transaction1 = transaction_v4_with_joinsplit_data(joinsplit_data1.0, [joinsplit1.0]);
+        let transaction1 = Arc::new(transaction_v4_with_joinsplit_data(joinsplit_data1.0, [joinsplit1.0]));
         let transaction2 = transaction_v4_with_joinsplit_data(joinsplit_data2.0, [joinsplit2.0]);
 
         block1.transactions[0] = transaction_v4_from_coinbase(&block1.transactions[0]).into();
         block2.transactions[0] = transaction_v4_from_coinbase(&block2.transactions[0]).into();
 
-        block1.transactions.push(transaction1.into());
+        block1.transactions.push(transaction1.clone());
         block2.transactions.push(transaction2.into());
 
         let (mut finalized_state, mut non_finalized_state, _genesis) = new_state_with_mainnet_genesis();
@@ -335,6 +341,11 @@ proptest! {
 
         let mut previous_mem = non_finalized_state.clone();
 
+        // makes sure there are no spurious rejections that might hide bugs in `tx_no_duplicates_in_chain`
+        let check_tx_no_duplicates_in_chain =
+            tx_no_duplicates_in_chain(&finalized_state.db, non_finalized_state.best_chain(), &transaction1);
+        prop_assert!(check_tx_no_duplicates_in_chain.is_ok());
+
         let block1_hash;
         // randomly choose to commit the next block to the finalized or non-finalized state
         if duplicate_in_finalized_state {
@@ -354,7 +365,9 @@ proptest! {
             let block1 = Arc::new(block1).prepare();
             let commit_result = validate_and_commit_non_finalized(
                 &finalized_state,
-            &mut non_finalized_state,  block1.clone());
+                &mut non_finalized_state,
+                block1.clone()
+            );
 
             prop_assert_eq!(commit_result, Ok(()));
             prop_assert_eq!(Some((Height(1), block1.hash)), read::best_tip(&non_finalized_state, &finalized_state.db));
@@ -371,7 +384,9 @@ proptest! {
         let block2 = Arc::new(block2).prepare();
         let commit_result = validate_and_commit_non_finalized(
             &finalized_state,
-            &mut non_finalized_state,  block2);
+            &mut non_finalized_state,
+            block2
+        );
 
         prop_assert_eq!(
             commit_result,
@@ -381,6 +396,18 @@ proptest! {
             }
             .into())
         );
+
+        let check_tx_no_duplicates_in_chain =
+            tx_no_duplicates_in_chain(&finalized_state.db, non_finalized_state.best_chain(), &transaction1);
+
+        prop_assert_eq!(
+            check_tx_no_duplicates_in_chain,
+            Err(DuplicateSproutNullifier {
+                nullifier: duplicate_nullifier,
+                in_finalized_state: duplicate_in_finalized_state,
+            })
+        );
+
         prop_assert_eq!(Some((Height(1), block1_hash)), read::best_tip(&non_finalized_state, &finalized_state.db));
         prop_assert!(non_finalized_state.eq_internal_state(&previous_mem));
     }
@@ -534,7 +561,9 @@ proptest! {
         let block1 = Arc::new(block1).prepare();
         let commit_result = validate_and_commit_non_finalized(
             &finalized_state,
-            &mut non_finalized_state,  block1);
+            &mut non_finalized_state,
+            block1
+        );
 
         prop_assert_eq!(
             commit_result,
@@ -572,14 +601,14 @@ proptest! {
         spend2.nullifier = duplicate_nullifier;
 
         let transaction1 =
-            transaction_v4_with_sapling_shielded_data(sapling_shielded_data1.0, [spend1.0]);
+            Arc::new(transaction_v4_with_sapling_shielded_data(sapling_shielded_data1.0, [spend1.0]));
         let transaction2 =
             transaction_v4_with_sapling_shielded_data(sapling_shielded_data2.0, [spend2.0]);
 
         block1.transactions[0] = transaction_v4_from_coinbase(&block1.transactions[0]).into();
         block2.transactions[0] = transaction_v4_from_coinbase(&block2.transactions[0]).into();
 
-        block1.transactions.push(transaction1.into());
+        block1.transactions.push(transaction1.clone());
         block2.transactions.push(transaction2.into());
 
         let (mut finalized_state, mut non_finalized_state, _genesis) = new_state_with_mainnet_genesis();
@@ -590,6 +619,11 @@ proptest! {
 
         let mut previous_mem = non_finalized_state.clone();
 
+        // makes sure there are no spurious rejections that might hide bugs in `tx_no_duplicates_in_chain`
+        let check_tx_no_duplicates_in_chain =
+            tx_no_duplicates_in_chain(&finalized_state.db, non_finalized_state.best_chain(), &transaction1);
+        prop_assert!(check_tx_no_duplicates_in_chain.is_ok());
+
         let block1_hash;
         // randomly choose to commit the next block to the finalized or non-finalized state
         if duplicate_in_finalized_state {
@@ -632,6 +666,18 @@ proptest! {
             }
             .into())
         );
+
+        let check_tx_no_duplicates_in_chain =
+            tx_no_duplicates_in_chain(&finalized_state.db, non_finalized_state.best_chain(), &transaction1);
+
+        prop_assert_eq!(
+            check_tx_no_duplicates_in_chain,
+            Err(DuplicateSaplingNullifier {
+                nullifier: duplicate_nullifier,
+                in_finalized_state: duplicate_in_finalized_state,
+            })
+        );
+
         prop_assert_eq!(Some((Height(1), block1_hash)), read::best_tip(&non_finalized_state, &finalized_state.db));
         prop_assert!(non_finalized_state.eq_internal_state(&previous_mem));
     }
@@ -829,10 +875,10 @@ proptest! {
         let duplicate_nullifier = authorized_action1.action.nullifier;
         authorized_action2.action.nullifier = duplicate_nullifier;
 
-        let transaction1 = transaction_v5_with_orchard_shielded_data(
+        let transaction1 = Arc::new(transaction_v5_with_orchard_shielded_data(
             orchard_shielded_data1.0,
             [authorized_action1.0],
-        );
+        ));
         let transaction2 = transaction_v5_with_orchard_shielded_data(
             orchard_shielded_data2.0,
             [authorized_action2.0],
@@ -841,7 +887,7 @@ proptest! {
         block1.transactions[0] = transaction_v4_from_coinbase(&block1.transactions[0]).into();
         block2.transactions[0] = transaction_v4_from_coinbase(&block2.transactions[0]).into();
 
-        block1.transactions.push(transaction1.into());
+        block1.transactions.push(transaction1.clone());
         block2.transactions.push(transaction2.into());
 
     let (mut finalized_state, mut non_finalized_state, _genesis) = new_state_with_mainnet_genesis();
@@ -852,6 +898,11 @@ proptest! {
 
         let mut previous_mem = non_finalized_state.clone();
 
+        // makes sure there are no spurious rejections that might hide bugs in `tx_no_duplicates_in_chain`
+        let check_tx_no_duplicates_in_chain =
+            tx_no_duplicates_in_chain(&finalized_state.db, non_finalized_state.best_chain(), &transaction1);
+        prop_assert!(check_tx_no_duplicates_in_chain.is_ok());
+
         let block1_hash;
         // randomly choose to commit the next block to the finalized or non-finalized state
         if duplicate_in_finalized_state {
@@ -893,6 +944,18 @@ proptest! {
             }
             .into())
         );
+
+        let check_tx_no_duplicates_in_chain =
+            tx_no_duplicates_in_chain(&finalized_state.db, non_finalized_state.best_chain(), &transaction1);
+
+        prop_assert_eq!(
+            check_tx_no_duplicates_in_chain,
+            Err(DuplicateOrchardNullifier {
+                nullifier: duplicate_nullifier,
+                in_finalized_state: duplicate_in_finalized_state,
+            })
+        );
+
         prop_assert_eq!(Some((Height(1), block1_hash)), read::best_tip(&non_finalized_state, &finalized_state.db));
         prop_assert!(non_finalized_state.eq_internal_state(&previous_mem));
     }

zebra-state/src/service/non_finalized_state.rs

@@ -226,15 +226,18 @@ impl NonFinalizedState {
         )?;
 
         // Reads from disk
-        check::anchors::sapling_orchard_anchors_refer_to_final_treestates(
+        check::anchors::block_sapling_orchard_anchors_refer_to_final_treestates(
             finalized_state,
             &new_chain,
             &prepared,
         )?;
 
         // Reads from disk
-        let sprout_final_treestates =
+        let sprout_final_treestates = check::anchors::block_fetch_sprout_final_treestates(
-            check::anchors::fetch_sprout_final_treestates(finalized_state, &new_chain, &prepared);
+            finalized_state,
+            &new_chain,
+            &prepared,
+        );
 
         // Quick check that doesn't read from disk
         let contextual = ContextuallyValidBlock::with_block_and_spent_utxos(
@@ -285,11 +288,12 @@ impl NonFinalizedState {
             });
 
             scope.spawn_fifo(|_scope| {
-                sprout_anchor_result = Some(check::anchors::sprout_anchors_refer_to_treestates(
+                sprout_anchor_result =
+                    Some(check::anchors::block_sprout_anchors_refer_to_treestates(
                         sprout_final_treestates,
                         block2,
-                    height,
                         transaction_hashes,
+                        height,
                     ));
             });
 

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

@@ -105,11 +105,11 @@ pub struct Chain {
     pub(crate) orchard_anchors_by_height: BTreeMap<block::Height, orchard::tree::Root>,
 
     /// The Sprout nullifiers revealed by `blocks`.
-    pub(super) sprout_nullifiers: HashSet<sprout::Nullifier>,
+    pub(crate) sprout_nullifiers: HashSet<sprout::Nullifier>,
     /// The Sapling nullifiers revealed by `blocks`.
-    pub(super) sapling_nullifiers: HashSet<sapling::Nullifier>,
+    pub(crate) sapling_nullifiers: HashSet<sapling::Nullifier>,
     /// The Orchard nullifiers revealed by `blocks`.
-    pub(super) orchard_nullifiers: HashSet<orchard::Nullifier>,
+    pub(crate) orchard_nullifiers: HashSet<orchard::Nullifier>,
 
     /// Partial transparent address index data from `blocks`.
     pub(super) partial_transparent_transfers: HashMap<transparent::Address, TransparentTransfers>,
@@ -410,12 +410,14 @@ impl Chain {
                 .expect("Orchard anchors must exist for pre-fork blocks");
 
             let history_tree_mut = Arc::make_mut(&mut self.history_tree);
-            history_tree_mut.push(
+            history_tree_mut
+                .push(
                     self.network,
                     block.block.clone(),
                     *sapling_root,
                     *orchard_root,
-            )?;
+                )
+                .map_err(Arc::new)?;
         }
 
         Ok(())
@@ -909,12 +911,14 @@ impl Chain {
 
         // TODO: update the history trees in a rayon thread, if they show up in CPU profiles
         let history_tree_mut = Arc::make_mut(&mut self.history_tree);
-        history_tree_mut.push(
+        history_tree_mut
+            .push(
                 self.network,
                 contextually_valid.block.clone(),
                 sapling_root,
                 orchard_root,
-        )?;
+            )
+            .map_err(Arc::new)?;
 
         self.history_trees_by_height
             .insert(height, self.history_tree.clone());

zebrad/src/components/inbound/tests/fake_peer_set.rs

@@ -163,7 +163,7 @@ async fn mempool_push_transaction() -> Result<(), crate::BoxError> {
         let transaction = responder
             .request()
             .clone()
-            .into_mempool_transaction()
+            .mempool_transaction()
             .expect("unexpected non-mempool request");
 
         // Set a dummy fee and sigops.
@@ -267,7 +267,7 @@ async fn mempool_advertise_transaction_ids() -> Result<(), crate::BoxError> {
         let transaction = responder
             .request()
             .clone()
-            .into_mempool_transaction()
+            .mempool_transaction()
             .expect("unexpected non-mempool request");
 
         // Set a dummy fee and sigops.
@@ -368,7 +368,7 @@ async fn mempool_transaction_expiration() -> Result<(), crate::BoxError> {
         let transaction = responder
             .request()
             .clone()
-            .into_mempool_transaction()
+            .mempool_transaction()
             .expect("unexpected non-mempool request");
 
         // Set a dummy fee and sigops.
@@ -502,7 +502,7 @@ async fn mempool_transaction_expiration() -> Result<(), crate::BoxError> {
         let transaction = responder
             .request()
             .clone()
-            .into_mempool_transaction()
+            .mempool_transaction()
             .expect("unexpected non-mempool request");
 
         // Set a dummy fee and sigops.

zebrad/src/components/mempool/downloads.rs

@@ -271,10 +271,10 @@ where
         let mut state = self.state.clone();
 
         let fut = async move {
-            // Don't download/verify if the transaction is already in the state.
+            // Don't download/verify if the transaction is already in the best chain.
-            Self::transaction_in_state(&mut state, txid).await?;
+            Self::transaction_in_best_chain(&mut state, txid).await?;
 
-            trace!(?txid, "transaction is not in state");
+            trace!(?txid, "transaction is not in best chain");
 
             let next_height = match state.oneshot(zs::Request::Tip).await {
                 Ok(zs::Response::Tip(None)) => Ok(Height(0)),
@@ -442,12 +442,11 @@ where
         self.pending.len()
     }
 
-    /// Check if transaction is already in the state.
+    /// Check if transaction is already in the best chain.
-    async fn transaction_in_state(
+    async fn transaction_in_best_chain(
         state: &mut ZS,
         txid: UnminedTxId,
     ) -> Result<(), TransactionDownloadVerifyError> {
-        // Check if the transaction is already in the state.
         match state
             .ready()
             .await