Merge pull request #1956 from blockstack/fix/1955

Fix/1955

src/net/p2p.rs

@@ -2680,25 +2680,47 @@ impl PeerNetwork {
     }
 
     /// Handle unsolicited BlocksData.
-    /// Don't (yet) validate the data, but do update our inv for the peer that sent it.
+    /// Don't (yet) validate the data, but do update our inv for the peer that sent it, if we have
+    /// an outbound connection to that peer.  Accept the blocks data either way if it corresponds
+    /// to a winning sortition -- this will cause the blocks data to be fed into the relayer, which
+    /// will then decide whether or not it needs to be stored and/or forwarded.
     /// Mask errors.
     fn handle_unsolicited_BlocksData(
         &mut self,
         sortdb: &SortitionDB,
         event_id: usize,
         new_blocks: &BlocksData,
     ) -> () {
-        let outbound_neighbor_key = match self.find_outbound_neighbor(event_id) {
-            Some(onk) => onk,
+        let (remote_neighbor_key, remote_is_authenticated) = match self.peers.get(&event_id) {
+            Some(convo) => (convo.to_neighbor_key(), convo.is_authenticated()),
             None => {
+                test_debug!(
+                    "{:?}: No such neighbor event={}",
+                    &self.local_peer,
+                    event_id
+                );
                 return;
             }
         };
 
+        if !remote_is_authenticated {
+            // drop -- a correct peer will have authenticated before sending this message
+            test_debug!(
+                "{:?}: Drop unauthenticated BlocksData from {:?}",
+                &self.local_peer,
+                &remote_neighbor_key
+            );
+            return;
+        }
+
+        let outbound_neighbor_key_opt = self.find_outbound_neighbor(event_id);
+
         debug!(
             "{:?}: Process BlocksData from {:?} with {} entries",
             &self.local_peer,
-            outbound_neighbor_key,
+            outbound_neighbor_key_opt
+                .as_ref()
+                .unwrap_or(&remote_neighbor_key),
             new_blocks.blocks.len()
         );
 
@@ -2711,39 +2733,44 @@ impl PeerNetwork {
                         continue;
                     }
                     Err(e) => {
-                        warn!(
-                            "Failed to query block snapshot for {}: {:?}",
-                            consensus_hash, &e
+                        info!(
+                            "{:?}: Failed to query block snapshot for {}: {:?}",
+                            &self.local_peer, consensus_hash, &e
                         );
                         continue;
                     }
                 };
 
             if !sn.pox_valid {
-                warn!(
-                    "Failed to query snapshot for {}: not on valid PoX fork",
-                    consensus_hash
+                info!(
+                    "{:?}: Failed to query snapshot for {}: not on the valid PoX fork",
+                    &self.local_peer, consensus_hash
                 );
                 continue;
             }
 
             if sn.winning_stacks_block_hash != block.block_hash() {
                 info!(
-                    "Ignoring block {} -- winning block was {} (sortition: {})",
+                    "{:?}: Ignoring block {} -- winning block was {} (sortition: {})",
+                    &self.local_peer,
                     block.block_hash(),
                     sn.winning_stacks_block_hash,
                     sn.sortition
                 );
                 continue;
             }
 
-            self.handle_unsolicited_inv_update(
-                sortdb,
-                event_id,
-                &outbound_neighbor_key,
-                &sn.consensus_hash,
-                false,
-            );
+            // only bother updating the inventory for this event's peer if we have an outbound
+            // connection to it.
+            if let Some(outbound_neighbor_key) = outbound_neighbor_key_opt.as_ref() {
+                self.handle_unsolicited_inv_update(
+                    sortdb,
+                    event_id,
+                    &outbound_neighbor_key,
+                    &sn.consensus_hash,
+                    false,
+                );
+            }
         }
     }
 

src/net/relay.rs

@@ -2231,10 +2231,8 @@ mod test {
         push_message(peer, dest, relay_hints, msg)
     }
 
-    #[test]
-    #[ignore]
-    fn test_get_blocks_and_microblocks_2_peers_push_blocks_and_microblocks() {
-        with_timeout(600, || {
+    fn test_get_blocks_and_microblocks_2_peers_push_blocks_and_microblocks(outbound_test: bool) {
+        with_timeout(600, move || {
             let original_blocks_and_microblocks = RefCell::new(vec![]);
             let blocks_and_microblocks = RefCell::new(vec![]);
             let idx = RefCell::new(0);
@@ -2267,7 +2265,12 @@ mod test {
                     let peer_1 = peer_configs[1].to_neighbor();
 
                     peer_configs[0].add_neighbor(&peer_1);
-                    peer_configs[1].add_neighbor(&peer_0);
+
+                    if outbound_test {
+                        // neighbor relationship is symmetric -- peer 1 has an outbound connection
+                        // to peer 0.
+                        peer_configs[1].add_neighbor(&peer_0);
+                    }
                 },
                 |num_blocks, ref mut peers| {
                     let tip = SortitionDB::get_canonical_burn_chain_tip(
@@ -2456,6 +2459,20 @@ mod test {
         })
     }
 
+    #[test]
+    #[ignore]
+    fn test_get_blocks_and_microblocks_2_peers_push_blocks_and_microblocks_outbound() {
+        // simulates node 0 pushing blocks to node 1, but node 0 is publicly routable
+        test_get_blocks_and_microblocks_2_peers_push_blocks_and_microblocks(true)
+    }
+
+    #[test]
+    #[ignore]
+    fn test_get_blocks_and_microblocks_2_peers_push_blocks_and_microblocks_inbound() {
+        // simulates node 0 pushing blocks to node 1, where node 0 is behind a NAT
+        test_get_blocks_and_microblocks_2_peers_push_blocks_and_microblocks(false)
+    }
+
     fn make_test_smart_contract_transaction(
         peer: &mut TestPeer,
         name: &str,