From 65e308d2e14d94d97214fc634637f11040699734 Mon Sep 17 00:00:00 2001
From: Alfredo Garcia <oxarbitrage@gmail.com>
Date: Mon, 6 Sep 2021 19:55:17 -0300
Subject: [PATCH] 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 <durumcrustulum@gmail.com>

Co-authored-by: Deirdre Connolly <durumcrustulum@gmail.com>
Co-authored-by: Deirdre Connolly <deirdre@zfnd.org>
---
 zebrad/src/components/inbound.rs       | 18 +++++++---------
 zebrad/src/components/inbound/tests.rs | 30 +++++++++++++++++++-------
 2 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/zebrad/src/components/inbound.rs b/zebrad/src/components/inbound.rs
index f2e8aabb4ff..ece90903a11 100644
--- 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
index a4416ea45c4..bced22afb58 100644
--- 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> {