adds a test

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));

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::<TransactionError>()
+        .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"
     );
 }