From 49aa41544d1e0e38dd14c4c2fdfc0292baccc51d Mon Sep 17 00:00:00 2001
From: Henry de Valence <hdevalence@hdevalence.ca>
Date: Wed, 22 Jul 2020 17:39:33 -0700
Subject: [PATCH] sync: try to ignore spurious inv messages.

Closes #697.

per  https://github.com/ZcashFoundation/zebra/issues/697#issuecomment-662742971

The response to a getblocks message is an inv message with the hashes of the
following blocks. However, inv messages are also sent unsolicited to gossip new
blocks across the network. Normally, this wouldn't be a problem, because for
every other request we filter only for the messages that are relevant to us.
But because the response to a getblocks message is an inv, the network layer
doesn't (and can't) distinguish between the response inv and the unsolicited
inv.

But there is a mitigation we can do. In our sync algorithm we have two phases:
(1) "ObtainTips" to get a set of tips to chase down, (2) repeatedly call
"ExtendTips" to extend those as far as possible. The unsolicited inv messages
have length 1, but when extending tips we expect to get more than one hash. So
we could reject responses in ExtendTips that have length 1 in order to ignore
these messages. This way we automatically ignore gossip messages during initial
block sync (while we're extending a tip) but we don't ignore length-1 responses
while trying to obtain tips (while querying the network for new tips).
---
 zebrad/src/commands/start/sync.rs | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/zebrad/src/commands/start/sync.rs b/zebrad/src/commands/start/sync.rs
index 689a3d6f..40be7681 100644
--- a/zebrad/src/commands/start/sync.rs
+++ b/zebrad/src/commands/start/sync.rs
@@ -261,16 +261,20 @@ where
                         // It indicates that the remote peer does not have any blocks
                         // following the prospective tip.
                         // TODO(jlusby): reject both main and test net genesis blocks
-                        match hashes.first() {
-                            Some(&super::GENESIS) => {
-                                tracing::debug!("skipping response, peer could not extend the tip");
-                                continue;
-                            }
-                            None => {
+                        match (hashes.first(), hashes.len()) {
+                            (_, 0) => {
                                 tracing::debug!("skipping empty response");
                                 continue;
                             }
-                            Some(_) => {}
+                            (_, 1) => {
+                                tracing::debug!("skipping length-1 response, in case it's an unsolicited inv message");
+                                continue;
+                            }
+                            (Some(&super::GENESIS), _) => {
+                                tracing::debug!("skipping response, peer could not extend the tip");
+                                continue;
+                            }
+                            _ => {}
                         }
 
                         let new_tip = hashes.pop().expect("expected: hashes must have len > 0");