Respond to inbound TransactionsById with mempool content (#2725)

* reply to inbound `TransactionsById` requests * apply style/redability suggestions and fix typo Co-authored-by: Deirdre Connolly <[email protected]> Co-authored-by: Deirdre Connolly <[email protected]> Co-authored-by: Deirdre Connolly <[email protected]>
ZcashFoundation · Sep 6, 2021 · 65e308d · 65e308d
1 parent 8e65e82
commit 65e308d
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 18 deletions.
diff --git a/zebrad/src/components/inbound.rs b/zebrad/src/components/inbound.rs
@@ -320,16 +320,14 @@ impl Service<zn::Request> for Inbound {
                     .map_ok(zn::Response::Blocks)
                     .boxed()
             }
-            zn::Request::TransactionsById(_transactions) => {
-                // `zcashd` returns a list of found transactions, followed by a
-                // `NotFound` message if any transactions are missing. `zcashd`
-                // says that Simplified Payment Verification (SPV) clients rely on
-                // this behaviour - are there any of them on the Zcash network?
-                // https://github.com/zcash/zcash/blob/e7b425298f6d9a54810cb7183f00be547e4d9415/src/main.cpp#L5632
-                // We'll implement this request once we have a mempool:
-                // https://en.bitcoin.it/wiki/Protocol_documentation#getdata
-                debug!("ignoring unimplemented request");
-                async { Ok(zn::Response::Nil) }.boxed()
+            zn::Request::TransactionsById(transactions) => {
+                let request = mempool::Request::TransactionsById(transactions);
+                self.mempool.clone().oneshot(request).map_ok(|resp| match resp {
+                    mempool::Response::Transactions(transactions) => zn::Response::Transactions(transactions),
+                    _ => unreachable!("Mempool component should always respond to a `TransactionsById` request with a `Transactions` response"),
+                })
+                .boxed()
+
             }
             zn::Request::FindBlocks { known_blocks, stop } => {
                 let request = zs::Request::FindBlockHashes { known_blocks, stop };

diff --git a/zebrad/src/components/inbound/tests.rs b/zebrad/src/components/inbound/tests.rs
@@ -1,9 +1,9 @@
-use tower::ServiceExt;
+use std::collections::HashSet;
 
 use super::mempool::{unmined_transactions_in_blocks, Mempool};
 
 use tokio::sync::oneshot;
-use tower::builder::ServiceBuilder;
+use tower::{builder::ServiceBuilder, ServiceExt};
 
 use zebra_chain::{
     parameters::Network,
@@ -14,7 +14,7 @@ use zebra_network::{Request, Response};
 use zebra_state::Config as StateConfig;
 
 #[tokio::test]
-async fn mempool_requests_for_transaction_ids() {
+async fn mempool_requests_for_transactions() {
     let network = Network::Mainnet;
     let consensus_config = ConsensusConfig::default();
     let state_config = StateConfig::ephemeral();
@@ -23,11 +23,8 @@ async fn mempool_requests_for_transaction_ids() {
     let state_service = ServiceBuilder::new().buffer(1).service(state);
     let mut mempool_service = Mempool::new(network);
 
-    let added_transaction_ids: Vec<UnminedTxId> =
-        add_some_stuff_to_mempool(&mut mempool_service, network)
-            .iter()
-            .map(|t| t.id)
-            .collect();
+    let added_transactions = add_some_stuff_to_mempool(&mut mempool_service, network);
+    let added_transaction_ids: Vec<UnminedTxId> = added_transactions.iter().map(|t| t.id).collect();
 
     let (block_verifier, transaction_verifier) =
         zebra_consensus::chain::init(consensus_config.clone(), network, state_service.clone())
@@ -45,7 +42,9 @@ async fn mempool_requests_for_transaction_ids() {
             mempool_service,
         ));
 
+    // Test `Request::MempoolTransactionIds`
     let request = inbound_service
+        .clone()
         .oneshot(Request::MempoolTransactionIds)
         .await;
     match request {
@@ -54,6 +53,21 @@ async fn mempool_requests_for_transaction_ids() {
             "`MempoolTransactionIds` requests should always respond `Ok(Vec<UnminedTxId>)`"
         ),
     };
+
+    // Test `Request::TransactionsById`
+    let hash_set = added_transaction_ids
+        .iter()
+        .copied()
+        .collect::<HashSet<_>>();
+
+    let request = inbound_service
+        .oneshot(Request::TransactionsById(hash_set))
+        .await;
+
+    match request {
+        Ok(Response::Transactions(response)) => assert_eq!(response, added_transactions),
+        _ => unreachable!("`TransactionsById` requests should always respond `Ok(Vec<UnminedTx>)`"),
+    };
 }
 
 fn add_some_stuff_to_mempool(mempool_service: &mut Mempool, network: Network) -> Vec<UnminedTx> {