Block verifier now passes transaction ids to the tx verifier

ZcashFoundation · Oct 19, 2024 · 837c077 · 837c077
1 parent ee23d3d
commit 837c077
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 16 deletions.
diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs
@@ -237,12 +237,15 @@ where
             let known_outpoint_hashes: Arc<HashSet<transaction::Hash>> =
                 Arc::new(known_utxos.keys().map(|outpoint| outpoint.hash).collect());
 
-            for transaction in &block.transactions {
+            for (&transaction_hash, transaction) in
+                transaction_hashes.iter().zip(block.transactions.iter())
+            {
                 let rsp = transaction_verifier
                     .ready()
                     .await
                     .expect("transaction verifier is always ready")
                     .call(tx::Request::Block {
+                        transaction_hash,
                         transaction: transaction.clone(),
                         known_outpoint_hashes: known_outpoint_hashes.clone(),
                         known_utxos: known_utxos.clone(),

diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs
@@ -146,6 +146,8 @@ where
 pub enum Request {
     /// Verify the supplied transaction as part of a block.
     Block {
+        /// The transaction hash.
+        transaction_hash: transaction::Hash,
         /// The transaction itself.
         transaction: Arc<Transaction>,
         /// Set of transaction hashes that create new transparent outputs.
@@ -261,6 +263,16 @@ impl Request {
         }
     }
 
+    /// The mined transaction ID for the transaction in this request.
+    pub fn tx_mined_id(&self) -> transaction::Hash {
+        match self {
+            Request::Block {
+                transaction_hash, ..
+            } => *transaction_hash,
+            Request::Mempool { transaction, .. } => transaction.id.mined_id(),
+        }
+    }
+
     /// The set of additional known unspent transaction outputs that's in this request.
     pub fn known_utxos(&self) -> Arc<HashMap<transparent::OutPoint, transparent::OrderedUtxo>> {
         match self {
@@ -390,16 +402,14 @@ where
         async move {
             tracing::trace!(?tx_id, ?req, "got tx verify request");
 
-            if !req.is_mempool() {
-                if let Some(result) = Self::try_find_verified_unmined_tx(&req, mempool.clone()).await {
-                    let verified_tx = result?;
+            if let Some(result) = Self::try_find_verified_unmined_tx(&req, mempool.clone()).await {
+                let verified_tx = result?;
 
-                    return Ok(Response::Block {
-                        tx_id: verified_tx.transaction.id,
-                        miner_fee: Some(verified_tx.miner_fee),
-                        legacy_sigop_count: verified_tx.legacy_sigop_count
-                    });
-                }
+                return Ok(Response::Block {
+                    tx_id: verified_tx.transaction.id,
+                    miner_fee: Some(verified_tx.miner_fee),
+                    legacy_sigop_count: verified_tx.legacy_sigop_count
+                });
             }
 
             // Do quick checks first
@@ -648,24 +658,23 @@ where
             return None;
         }
 
-        let mut mempool = mempool?;
+        let mempool = mempool?;
         let known_outpoint_hashes = req.known_outpoint_hashes();
-        let tx_id = req.transaction().hash();
+        let tx_id = req.tx_mined_id();
 
         let mempool::Response::TransactionWithDeps {
             transaction,
             dependencies,
         } = mempool
-            .ready()
-            .await
-            .ok()?
-            .call(mempool::Request::TransactionWithDepsByMinedId(tx_id))
+            .oneshot(mempool::Request::TransactionWithDepsByMinedId(tx_id))
             .await
             .ok()?
         else {
             panic!("unexpected response to TransactionWithDepsByMinedId request");
         };
 
+        // Note: This does not verify that the spends are in order, this should be
+        //       done during contextual validation in zebra-state.
         let has_all_tx_deps = dependencies
             .into_iter()
             .all(|dependency_id| known_outpoint_hashes.contains(&dependency_id));

diff --git a/zebra-consensus/src/transaction/tests.rs b/zebra-consensus/src/transaction/tests.rs
@@ -985,6 +985,7 @@ async fn v5_transaction_is_rejected_before_nu5_activation() {
 
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1046,6 +1047,7 @@ fn v5_transaction_is_accepted_after_nu5_activation_for_network(network: Network)
 
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1102,6 +1104,7 @@ async fn v4_transaction_with_transparent_transfer_is_accepted() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1147,6 +1150,7 @@ async fn v4_transaction_with_last_valid_expiry_height() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction.clone()),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1193,6 +1197,7 @@ async fn v4_coinbase_transaction_with_low_expiry_height() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction.clone()),
             known_utxos: Arc::new(HashMap::new()),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1241,6 +1246,7 @@ async fn v4_transaction_with_too_low_expiry_height() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction.clone()),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1292,6 +1298,7 @@ async fn v4_transaction_with_exceeding_expiry_height() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction.clone()),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1346,6 +1353,7 @@ async fn v4_coinbase_transaction_with_exceeding_expiry_height() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction.clone()),
             known_utxos: Arc::new(HashMap::new()),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1398,6 +1406,7 @@ async fn v4_coinbase_transaction_is_accepted() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(HashMap::new()),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1454,6 +1463,7 @@ async fn v4_transaction_with_transparent_transfer_is_rejected_by_the_script() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1510,6 +1520,7 @@ async fn v4_transaction_with_conflicting_transparent_spend_is_rejected() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1582,6 +1593,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_inside_joinsplit_is_rejected
 
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1659,6 +1671,7 @@ fn v4_transaction_with_conflicting_sprout_nullifier_across_joinsplits_is_rejecte
 
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1719,6 +1732,7 @@ async fn v5_transaction_with_transparent_transfer_is_accepted() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1766,6 +1780,7 @@ async fn v5_transaction_with_last_valid_expiry_height() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction.clone()),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1812,6 +1827,7 @@ async fn v5_coinbase_transaction_expiry_height() {
     let result = verifier
         .clone()
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction.clone()),
             known_utxos: Arc::new(HashMap::new()),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1834,6 +1850,7 @@ async fn v5_coinbase_transaction_expiry_height() {
     let result = verifier
         .clone()
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(new_transaction.clone()),
             known_utxos: Arc::new(HashMap::new()),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1864,6 +1881,7 @@ async fn v5_coinbase_transaction_expiry_height() {
     let result = verifier
         .clone()
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(new_transaction.clone()),
             known_utxos: Arc::new(HashMap::new()),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1896,6 +1914,7 @@ async fn v5_coinbase_transaction_expiry_height() {
     let result = verifier
         .clone()
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(new_transaction.clone()),
             known_utxos: Arc::new(HashMap::new()),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1946,6 +1965,7 @@ async fn v5_transaction_with_too_low_expiry_height() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction.clone()),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -1998,6 +2018,7 @@ async fn v5_transaction_with_exceeding_expiry_height() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction.clone()),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2053,6 +2074,7 @@ async fn v5_coinbase_transaction_is_accepted() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2111,6 +2133,7 @@ async fn v5_transaction_with_transparent_transfer_is_rejected_by_the_script() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2169,6 +2192,7 @@ async fn v5_transaction_with_conflicting_transparent_spend_is_rejected() {
 
     let result = verifier
         .oneshot(Request::Block {
+            transaction_hash: transaction.hash(),
             transaction: Arc::new(transaction),
             known_utxos: Arc::new(known_utxos),
             known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2214,6 +2238,7 @@ fn v4_with_signed_sprout_transfer_is_accepted() {
         // Test the transaction verifier
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction,
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2304,6 +2329,7 @@ async fn v4_with_joinsplit_is_rejected_for_modification(
         let result = verifier
             .clone()
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction: transaction.clone(),
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2351,6 +2377,7 @@ fn v4_with_sapling_spends() {
         // Test the transaction verifier
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction,
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2394,6 +2421,7 @@ fn v4_with_duplicate_sapling_spends() {
         // Test the transaction verifier
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction,
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2439,6 +2467,7 @@ fn v4_with_sapling_outputs_and_no_spends() {
         // Test the transaction verifier
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction,
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2488,6 +2517,7 @@ fn v5_with_sapling_spends() {
         // Test the transaction verifier
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2532,6 +2562,7 @@ fn v5_with_duplicate_sapling_spends() {
         // Test the transaction verifier
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),
@@ -2595,6 +2626,7 @@ fn v5_with_duplicate_orchard_action() {
         // Test the transaction verifier
         let result = verifier
             .oneshot(Request::Block {
+                transaction_hash: transaction.hash(),
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(HashMap::new()),
                 known_outpoint_hashes: Arc::new(HashSet::new()),

diff --git a/zebra-consensus/src/transaction/tests/prop.rs b/zebra-consensus/src/transaction/tests/prop.rs
@@ -455,11 +455,13 @@ fn validate(
             tower::service_fn(|_| async { unreachable!("State service should not be called") });
         let verifier = transaction::Verifier::new_for_tests(&network, state_service);
         let verifier = Buffer::new(verifier, 10);
+        let transaction_hash = transaction.hash();
 
         // Test the transaction verifier
         verifier
             .clone()
             .oneshot(transaction::Request::Block {
+                transaction_hash,
                 transaction: Arc::new(transaction),
                 known_utxos: Arc::new(known_utxos),
                 known_outpoint_hashes: Arc::new(HashSet::new()),