diff --git a/zebra-consensus/src/block.rs b/zebra-consensus/src/block.rs index e5facd7e411..4065ae8f4b7 100644 --- a/zebra-consensus/src/block.rs +++ b/zebra-consensus/src/block.rs @@ -237,12 +237,15 @@ where let known_outpoint_hashes: Arc> = 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 index 4a4897e67ef..a6924211390 100644 --- 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, /// 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> { 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 @@ -650,7 +660,7 @@ where let mut 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, @@ -666,6 +676,8 @@ where 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 index 9f3400d1612..b323cd7f555 100644 --- 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 index b5f179e2e03..8fea9cf3433 100644 --- 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()),