From 114b48355eba4946bf26bf3595691375a45984ea Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Wed, 7 Oct 2020 19:25:54 -0400 Subject: [PATCH 1/3] accept BlocksData from inbound neighbors as well as outbound --- src/net/p2p.rs | 56 ++++++++++++++++++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 18 deletions(-) diff --git a/src/net/p2p.rs b/src/net/p2p.rs index d0f1c2ac5c..8c02f40f90 100644 --- a/src/net/p2p.rs +++ b/src/net/p2p.rs @@ -2680,7 +2680,10 @@ 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, @@ -2688,17 +2691,29 @@ impl PeerNetwork { 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,25 +2726,26 @@ 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 @@ -2737,13 +2753,17 @@ impl PeerNetwork { 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, + ); + } } } From 1cbeeaf2321db74bb54b0f6687369f4573f5d413 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Wed, 7 Oct 2020 19:26:45 -0400 Subject: [PATCH 2/3] add test for #1955 --- src/net/relay.rs | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/net/relay.rs b/src/net/relay.rs index c8d3827422..b78265329d 100644 --- a/src/net/relay.rs +++ b/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( @@ -2455,6 +2458,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, From d7e605039eca4925a5fd0d73ee6c88f3e54db603 Mon Sep 17 00:00:00 2001 From: Jude Nelson Date: Wed, 7 Oct 2020 20:02:56 -0400 Subject: [PATCH 3/3] cargo fmt --- src/net/p2p.rs | 21 ++++++++++++++------- src/net/relay.rs | 4 ++-- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/net/p2p.rs b/src/net/p2p.rs index 8c02f40f90..08cfa1ac29 100644 --- a/src/net/p2p.rs +++ b/src/net/p2p.rs @@ -2692,19 +2692,24 @@ impl PeerNetwork { new_blocks: &BlocksData, ) -> () { let (remote_neighbor_key, remote_is_authenticated) = match self.peers.get(&event_id) { - Some(convo) => ( - convo.to_neighbor_key(), - convo.is_authenticated() - ), + Some(convo) => (convo.to_neighbor_key(), convo.is_authenticated()), None => { - test_debug!("{:?}: No such neighbor event={}", &self.local_peer, event_id); + 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); + test_debug!( + "{:?}: Drop unauthenticated BlocksData from {:?}", + &self.local_peer, + &remote_neighbor_key + ); return; } @@ -2713,7 +2718,9 @@ impl PeerNetwork { debug!( "{:?}: Process BlocksData from {:?} with {} entries", &self.local_peer, - outbound_neighbor_key_opt.as_ref().unwrap_or(&remote_neighbor_key), + outbound_neighbor_key_opt + .as_ref() + .unwrap_or(&remote_neighbor_key), new_blocks.blocks.len() ); diff --git a/src/net/relay.rs b/src/net/relay.rs index b78265329d..5efaa3ede2 100644 --- a/src/net/relay.rs +++ b/src/net/relay.rs @@ -2458,14 +2458,14 @@ 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() {