diff --git a/zebra-consensus/src/transaction.rs b/zebra-consensus/src/transaction.rs index e25b3b8d28e..02f97fb7778 100644 --- a/zebra-consensus/src/transaction.rs +++ b/zebra-consensus/src/transaction.rs @@ -406,7 +406,7 @@ where let verified_tx = result?; return Ok(Response::Block { - tx_id: verified_tx.transaction.id, + tx_id, miner_fee: Some(verified_tx.miner_fee), legacy_sigop_count: verified_tx.legacy_sigop_count }); @@ -673,8 +673,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. + // Note: This does not verify that the spends are in order, the spend order + // should be verified 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 b323cd7f555..ab4318b159b 100644 --- a/zebra-consensus/src/transaction/tests.rs +++ b/zebra-consensus/src/transaction/tests.rs @@ -696,13 +696,179 @@ async fn mempool_request_with_unmined_output_spends_is_accepted() { ); tokio::time::sleep(POLL_MEMPOOL_DELAY * 2).await; + // polled before AwaitOutput request and after a mempool transaction with transparent outputs + // is successfully verified assert_eq!( mempool.poll_count(), 2, - "the mempool service should have been polled twice, \ - first before being called with an AwaitOutput request, \ - then again shortly after a mempool transaction with transparent outputs \ - is successfully verified" + "the mempool service should have been polled twice" + ); +} + +#[tokio::test] +async fn skips_verification_of_block_transactions_in_mempool() { + let mut state: MockService<_, _, _, _> = MockService::build().for_prop_tests(); + let mempool: MockService<_, _, _, _> = MockService::build().for_prop_tests(); + let (mempool_setup_tx, mempool_setup_rx) = tokio::sync::oneshot::channel(); + let verifier = Verifier::new(&Network::Mainnet, state.clone(), mempool_setup_rx); + let verifier = Buffer::new(verifier, 1); + + mempool_setup_tx + .send(mempool.clone()) + .ok() + .expect("send should succeed"); + + 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, + Amount::try_from(10001).expect("invalid value"), + ); + + // 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 tx_hash = tx.hash(); + 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(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(None)); + + 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 mut mempool_clone = mempool.clone(); + tokio::spawn(async move { + mempool_clone + .expect_request(mempool::Request::AwaitOutput(input_outpoint)) + .await + .expect("verifier should call mock state service with correct request") + .respond(mempool::Response::UnspentOutput( + known_utxos + .get(&input_outpoint) + .expect("input outpoint should exist in known_utxos") + .utxo + .output + .clone(), + )); + }); + + let verifier_response = verifier + .clone() + .oneshot(Request::Mempool { + transaction: tx.clone().into(), + height, + }) + .await; + + assert!( + verifier_response.is_ok(), + "expected successful verification, got: {verifier_response:?}" + ); + + let crate::transaction::Response::Mempool { + transaction, + spent_mempool_outpoints, + } = verifier_response.expect("already checked that response is ok") + else { + panic!("unexpected response variant from transaction verifier for Mempool request") + }; + + assert_eq!( + spent_mempool_outpoints, + vec![input_outpoint], + "spent_mempool_outpoints in tx verifier response should match input_outpoint" + ); + + let mut mempool_clone = mempool.clone(); + tokio::spawn(async move { + for _ in 0..2 { + mempool_clone + .expect_request(mempool::Request::TransactionWithDepsByMinedId(tx_hash)) + .await + .expect("verifier should call mock state service with correct request") + .respond(mempool::Response::TransactionWithDeps { + transaction: transaction.clone(), + dependencies: [input_outpoint.hash].into(), + }); + } + }); + + let make_request = |known_outpoint_hashes| Request::Block { + transaction_hash: tx_hash, + transaction: Arc::new(tx), + known_outpoint_hashes, + known_utxos: Arc::new(HashMap::new()), + height, + time: Utc::now(), + }; + + let crate::transaction::Response::Block { .. } = verifier + .clone() + .oneshot(make_request.clone()(Arc::new([input_outpoint.hash].into()))) + .await + .expect("should return Ok without calling state service") + else { + panic!("unexpected response variant from transaction verifier for Block request") + }; + + let verifier_response_err = *verifier + .clone() + .oneshot(make_request(Arc::new(HashSet::new()))) + .await + .expect_err("should return Err without calling state service") + .downcast::() + .expect("tx verifier error type should be TransactionError"); + + assert_eq!( + verifier_response_err, + TransactionError::TransparentInputNotFound, + "should be a transparent input not found error" + ); + + tokio::time::sleep(POLL_MEMPOOL_DELAY * 2).await; + // polled before AwaitOutput request, after a mempool transaction with transparent outputs, + // is successfully verified, and twice more when checking if a transaction in a block is + // already the mempool. + assert_eq!( + mempool.poll_count(), + 4, + "the mempool service should have been polled 4 times" ); }