From 5929e05e5290a24c851663d525df16e5a1405e35 Mon Sep 17 00:00:00 2001
From: Henry de Valence <hdevalence@hdevalence.ca>
Date: Fri, 7 Feb 2020 21:39:21 -0800
Subject: [PATCH] Remove `PushPeers` and ignore unsolicited `addr` messages.

PushPeers is more complicated to thread into the rest of our
architecture (we would need to establish a data path connecting our
service handling inbound requests to the network layer's auto-crawler),
and since we crawl the network automatically anyways, we don't actually
need to accept them in order to get updated address information.

The only possible problem with this approach is that zcashd refuses to
answer multiple address requests from the same connection, ostensibly
for fingerprinting prevention (although it's totally happy to give
exactly the same information, as long as you hang up and reconnect
first, lol).  It's unclear how this will interact with our design -- on
the one hand, it could mean that we don't get new addr information when
we ask, but on the other hand, we may have enough churn in our
connection pool that this isn't a problem anyways.
---
 zebra-network/src/peer/connection.rs          | 19 ++++---------------
 .../src/protocol/internal/request.rs          |  6 ++----
 .../src/protocol/internal/response.rs         |  3 +++
 3 files changed, 9 insertions(+), 19 deletions(-)

diff --git a/zebra-network/src/peer/connection.rs b/zebra-network/src/peer/connection.rs
index 7b4350db..e7d252f1 100644
--- a/zebra-network/src/peer/connection.rs
+++ b/zebra-network/src/peer/connection.rs
@@ -295,20 +295,6 @@ where
                 .await
                 .map_err(|e| e.into())
                 .map(|()| AwaitingResponse(Handler::GetPeers, tx)),
-            (AwaitingRequest, PushPeers(addrs)) => self
-                .peer_tx
-                .send(Message::Addr(addrs))
-                .await
-                .map_err(|e| e.into())
-                .map(|()| {
-                    // PushPeers does not have a response, so we return OK as
-                    // soon as we send the request. Sending through a oneshot
-                    // can only fail if the rx end is dropped before calling
-                    // send, which we can safely ignore (the request future was
-                    // cancelled).
-                    let _ = tx.send(Ok(Response::Ok));
-                    AwaitingRequest
-                }),
             (AwaitingRequest, Ping(nonce)) => self
                 .peer_tx
                 .send(Message::Ping(nonce))
@@ -377,7 +363,10 @@ where
         // Interpret `msg` as a request from the remote peer to our node,
         // and try to construct an appropriate request object.
         let req = match msg {
-            Message::Addr(addrs) => Some(Request::PushPeers(addrs)),
+            Message::Addr(_) => {
+                debug!("ignoring unsolicited addr message");
+                None
+            }
             Message::GetAddr => Some(Request::Peers),
             _ => {
                 debug!("unhandled message type");
diff --git a/zebra-network/src/protocol/internal/request.rs b/zebra-network/src/protocol/internal/request.rs
index 0156aab4..990fd592 100644
--- a/zebra-network/src/protocol/internal/request.rs
+++ b/zebra-network/src/protocol/internal/request.rs
@@ -1,6 +1,5 @@
 use std::collections::HashSet;
 
-use crate::meta_addr::MetaAddr;
 use zebra_chain::block::BlockHeaderHash;
 
 use super::super::types::Nonce;
@@ -10,15 +9,14 @@ use super::super::types::Nonce;
 pub enum Request {
     /// Requests additional peers from the server.
     Peers,
-    /// Advertises peers to the remote server.
-    // XXX potentially remove this -- we don't use it?
-    PushPeers(Vec<MetaAddr>),
+
     /// Heartbeats triggered on peer connection start.
     ///
     /// This is included as a bit of a hack, it should only be used
     /// internally for connection management. You should not expect to
     /// be firing or handling `Ping` requests or `Pong` responses.
     Ping(Nonce),
+
     /// Request block data by block hashes.
     ///
     /// This uses a `HashSet` rather than a `Vec` for two reasons. First, it
diff --git a/zebra-network/src/protocol/internal/response.rs b/zebra-network/src/protocol/internal/response.rs
index 13dd6a76..73e11e06 100644
--- a/zebra-network/src/protocol/internal/response.rs
+++ b/zebra-network/src/protocol/internal/response.rs
@@ -10,10 +10,13 @@ use crate::meta_addr::MetaAddr;
 pub enum Response {
     /// Generic success.
     Ok,
+
     /// Generic error.
     Error,
+
     /// A list of peers, used to respond to `GetPeers`.
     Peers(Vec<MetaAddr>),
+
     /// A list of blocks.
     Blocks(Vec<Block>),
 }