From c291c484366ebf72f3005066cbfb4a959470218f Mon Sep 17 00:00:00 2001 From: Antioch Peverell Date: Tue, 18 Sep 2018 18:51:37 +0100 Subject: [PATCH] Simplify block broadcast (#1549) * removed random "block vs compact_block" --- p2p/src/peers.rs | 35 ++++++---------------------------- p2p/src/protocol.rs | 26 ++++++------------------- servers/src/common/adapters.rs | 31 +++--------------------------- 3 files changed, 15 insertions(+), 77 deletions(-) diff --git a/p2p/src/peers.rs b/p2p/src/peers.rs index 5b2b4662..a804ec97 100644 --- a/p2p/src/peers.rs +++ b/p2p/src/peers.rs @@ -130,8 +130,7 @@ impl Peers { .filter(|x| match x.try_read() { Ok(peer) => peer.info.direction == Direction::Outbound, Err(_) => false, - }) - .collect::>(); + }).collect::>(); res } @@ -165,8 +164,7 @@ impl Peers { .filter(|x| match x.try_read() { Ok(peer) => peer.info.total_difficulty > total_difficulty, Err(_) => false, - }) - .collect::>(); + }).collect::>(); thread_rng().shuffle(&mut max_peers); max_peers @@ -190,8 +188,7 @@ impl Peers { && peer.info.capabilities.contains(Capabilities::FULL_HIST) } Err(_) => false, - }) - .collect::>(); + }).collect::>(); thread_rng().shuffle(&mut max_peers); max_peers @@ -220,8 +217,7 @@ impl Peers { .map(|x| match x.try_read() { Ok(peer) => peer.info.total_difficulty.clone(), Err(_) => Difficulty::zero(), - }) - .max() + }).max() .unwrap(); let mut max_peers = peers @@ -229,8 +225,7 @@ impl Peers { .filter(|x| match x.try_read() { Ok(peer) => peer.info.total_difficulty == max_total_difficulty, Err(_) => false, - }) - .collect::>(); + }).collect::>(); thread_rng().shuffle(&mut max_peers); max_peers @@ -303,23 +298,6 @@ impl Peers { count } - /// Broadcasts the provided block to PEER_PREFERRED_COUNT of our peers. - /// We may be connected to PEER_MAX_COUNT peers so we only - /// want to broadcast to a random subset of peers. - /// A peer implementation may drop the broadcast request - /// if it knows the remote peer already has the block. - pub fn broadcast_block(&self, b: &core::Block) { - let count = self.broadcast("block", |p| p.send_block(b)); - debug!( - LOGGER, - "broadcast_block: {} @ {} [{}] was sent to {} peers.", - b.header.pow.total_difficulty, - b.header.height, - b.hash(), - count, - ); - } - /// Broadcasts the provided compact block to PEER_PREFERRED_COUNT of our peers. /// We may be connected to PEER_MAX_COUNT peers so we only /// want to broadcast to a random subset of peers. @@ -489,8 +467,7 @@ impl Peers { .map(|x| { let p = x.read().unwrap(); p.info.addr.clone() - }) - .collect::>() + }).collect::>() }; // now remove them taking a short-lived write lock each time diff --git a/p2p/src/protocol.rs b/p2p/src/protocol.rs index 03daa951..e6006182 100644 --- a/p2p/src/protocol.rs +++ b/p2p/src/protocol.rs @@ -27,7 +27,6 @@ use msg::{ read_exact, BanReason, GetPeerAddrs, Headers, Locator, PeerAddrs, Ping, Pong, SockAddr, TxHashSetArchive, TxHashSetRequest, Type, }; -use rand::{self, Rng}; use types::{Error, NetAdapter}; use util::LOGGER; @@ -107,7 +106,10 @@ impl MessageHandler for Protocol { Type::GetBlock => { let h: Hash = msg.body()?; - trace!(LOGGER, "handle_payload: GetBlock {}", h); + debug!( + LOGGER, + "handle_payload: Getblock: {}, msg_len: {}", h, msg.header.msg_len, + ); let bo = adapter.get_block(h); if let Some(b) = bo { @@ -129,25 +131,9 @@ impl MessageHandler for Protocol { Type::GetCompactBlock => { let h: Hash = msg.body()?; - if let Some(b) = adapter.get_block(h) { - // if we have txs in the block send a compact block - // but if block is empty - - // to allow us to test all code paths, randomly choose to send - // either the block or the compact block - let mut rng = rand::thread_rng(); - - if b.kernels().len() == 1 && rng.gen() { - debug!( - LOGGER, - "handle_payload: GetCompactBlock: empty block, sending full block", - ); - - Ok(Some(msg.respond(Type::Block, b))) - } else { - let cb: CompactBlock = b.into(); - Ok(Some(msg.respond(Type::CompactBlock, cb))) - } + let cb: CompactBlock = b.into(); + Ok(Some(msg.respond(Type::CompactBlock, cb))) } else { Ok(None) } diff --git a/servers/src/common/adapters.rs b/servers/src/common/adapters.rs index 0d418bac..3fc4a6ba 100644 --- a/servers/src/common/adapters.rs +++ b/servers/src/common/adapters.rs @@ -15,7 +15,6 @@ //! Adapters connecting new block, new transaction, and accepted transaction //! events to consumers of those events. -use rand::{self, Rng}; use std::fs::File; use std::net::SocketAddr; use std::ops::Deref; @@ -643,40 +642,16 @@ impl ChainAdapter for ChainToPoolAndNetAdapter { ); } - // If we mined the block then we want to broadcast the block itself. - // If block is empty then broadcast the block. - // If block contains txs then broadcast the compact block. + // If we mined the block then we want to broadcast the compact block. // If we received the block from another node then broadcast "header first" // to minimize network traffic. - if opts.contains(Options::MINE) { // propagate compact block out if we mined the block - // but broadcast full block if we have no txs let cb: CompactBlock = b.clone().into(); - if cb.kern_ids().is_empty() { - // In the interest of exercising all code paths - // randomly decide how we send an empty block out. - let mut rng = rand::thread_rng(); - if rng.gen() { - wo(&self.peers).broadcast_block(&b); - } else { - wo(&self.peers).broadcast_compact_block(&cb); - } - } else { - wo(&self.peers).broadcast_compact_block(&cb); - } + wo(&self.peers).broadcast_compact_block(&cb); } else { // "header first" propagation if we are not the originator of this block - // again randomly chose between "header first" or "compact block" propagation - // to ensure we test a wide variety of code paths - - let mut rng = rand::thread_rng(); - if rng.gen() { - wo(&self.peers).broadcast_header(&b.header); - } else { - let cb = b.clone().into(); - wo(&self.peers).broadcast_compact_block(&cb); - } + wo(&self.peers).broadcast_header(&b.header); } } }