From ba1061a043e5216ac5e83bca0106248004215fb2 Mon Sep 17 00:00:00 2001
From: Emilia Hane <elsaemiliaevahane@gmail.com>
Date: Sun, 3 Mar 2024 15:31:28 +0100
Subject: [PATCH] Return fetch error on response that fails verification or
 validation (#6864)

---
 .../net/network/src/transactions/fetcher.rs   | 60 +++++++++++++++++--
 crates/net/network/src/transactions/mod.rs    |  3 +
 2 files changed, 57 insertions(+), 6 deletions(-)

diff --git a/crates/net/network/src/transactions/fetcher.rs b/crates/net/network/src/transactions/fetcher.rs
index 326c8c2b0c..bffa6fd29d 100644
--- a/crates/net/network/src/transactions/fetcher.rs
+++ b/crates/net/network/src/transactions/fetcher.rs
@@ -843,6 +843,23 @@ impl TransactionFetcher {
 
         match result {
             Ok(Ok(transactions)) => {
+                //
+                // 1. peer has failed to serve any of the hashes it has announced to us that we,
+                // as a follow, have requested
+                //
+                if transactions.is_empty() {
+                    trace!(target: "net::tx",
+                        peer_id=format!("{peer_id:#}"),
+                        requested_hashes_len=requested_hashes.len(),
+                        "received empty `PooledTransactions` response from peer, peer failed to serve hashes it announced"
+                    );
+
+                    return FetchEvent::EmptyResponse { peer_id }
+                }
+
+                //
+                // 2. filter out hashes that we didn't request
+                //
                 let payload = UnverifiedPooledTransactions::new(transactions);
 
                 let unverified_len = payload.len();
@@ -858,25 +875,40 @@ impl TransactionFetcher {
                         "received `PooledTransactions` response from peer with entries that didn't verify against request, filtered out transactions"
                     );
                 }
+                // peer has only sent hashes that we didn't request
+                if verified_payload.is_empty() {
+                    return FetchEvent::FetchError { peer_id, error: RequestError::BadResponse }
+                }
 
+                //
+                // 3. stateless validation of payload, e.g. dedup
+                //
                 let unvalidated_payload_len = verified_payload.len();
 
-                // todo: report peer for sending invalid response
-                // <https://github.com/paradigmxyz/reth/issues/6529>
-
                 let (validation_outcome, valid_payload) =
                     self.filter_valid_message.partially_filter_valid_entries(verified_payload);
 
+                // todo: validate based on announced tx size/type and report peer for sending
+                // invalid response <https://github.com/paradigmxyz/reth/issues/6529>. requires
+                // passing the rlp encoded length down from active session along with the decoded
+                // tx.
+
                 if let FilterOutcome::ReportPeer = validation_outcome {
                     trace!(target: "net::tx",
                         peer_id=format!("{peer_id:#}"),
                         unvalidated_payload_len=unvalidated_payload_len,
                         valid_payload_len=valid_payload.len(),
-                        "received invalid `PooledTransactions` response from peer, filtered out invalid entries"
+                        "received invalid `PooledTransactions` response from peer, filtered out duplicate entries"
                     );
                 }
+                // valid payload will have at least one transaction at this point. even if the tx
+                // size/type announced by the peer is different to the actual tx size/type, pass on
+                // to pending pool imports pipeline for validation.
 
-                // clear received hashes
+                //
+                // 4. clear received hashes
+                //
+                let requested_hashes_len = requested_hashes.len();
                 let mut fetched = Vec::with_capacity(valid_payload.len());
                 requested_hashes.retain(|requested_hash| {
                     if valid_payload.contains_key(requested_hash) {
@@ -888,7 +920,18 @@ impl TransactionFetcher {
                 });
                 fetched.shrink_to_fit();
 
-                // buffer left over hashes
+                if fetched.len() < requested_hashes_len {
+                    trace!(target: "net::tx",
+                        peer_id=format!("{peer_id:#}"),
+                        requested_hashes_len=requested_hashes_len,
+                        fetched_len=fetched.len(),
+                        "peer failed to serve hashes it announced"
+                    );
+                }
+
+                //
+                // 5. buffer left over hashes
+                //
                 self.try_buffer_hashes_for_retry(requested_hashes, &peer_id);
 
                 let transactions =
@@ -994,6 +1037,11 @@ pub enum FetchEvent {
         /// The specific error that occurred while fetching.
         error: RequestError,
     },
+    /// An empty response was received.
+    EmptyResponse {
+        /// The ID of the sender.
+        peer_id: PeerId,
+    },
 }
 
 /// An inflight request for [`PooledTransactions`] from a peer.
diff --git a/crates/net/network/src/transactions/mod.rs b/crates/net/network/src/transactions/mod.rs
index 7879954c4e..a69aac19fe 100644
--- a/crates/net/network/src/transactions/mod.rs
+++ b/crates/net/network/src/transactions/mod.rs
@@ -1280,6 +1280,9 @@ where
                                 trace!(target: "net::tx", ?peer_id, %error, "requesting transactions from peer failed");
                                 this.on_request_error(peer_id, error);
                             }
+                            FetchEvent::EmptyResponse { peer_id } => {
+                                trace!(target: "net::tx", ?peer_id, "peer returned empty response");
+                            }
                         }
                         some_ready = true;
                     }