diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 3a68eb4b1d..96a4e67505 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -3366,9 +3366,9 @@ dependencies = [ [[package]] name = "lru" -version = "0.6.1" +version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be716eb6878ca2263eb5d00a781aa13264a794f519fe6af4fbb2668b2d5441c0" +checksum = "3aae342b73d57ad0b8b364bd12584819f2c1fe9114285dfcf8b0722607671635" dependencies = [ "hashbrown", ] @@ -7153,6 +7153,7 @@ dependencies = [ "linked-hash-map", "linked_hash_set", "log", + "lru", "nohash-hasher", "parity-scale-codec", "parking_lot 0.11.1", @@ -9811,7 +9812,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04f8ab788026715fa63b31960869617cba39117e520eb415b0139543e325ab59" dependencies = [ "cfg-if 0.1.10", - "rand 0.3.23", + "rand 0.7.3", "static_assertions", ] diff --git a/substrate/client/finality-grandpa/src/communication/mod.rs b/substrate/client/finality-grandpa/src/communication/mod.rs index 66b7f00489..d502741465 100644 --- a/substrate/client/finality-grandpa/src/communication/mod.rs +++ b/substrate/client/finality-grandpa/src/communication/mod.rs @@ -722,7 +722,7 @@ impl Sink> for OutgoingMessages ); // announce the block we voted on to our peers. - self.network.lock().announce(target_hash, Vec::new()); + self.network.lock().announce(target_hash, None); // propagate the message to peers let topic = round_topic::(self.round, self.set_id); diff --git a/substrate/client/finality-grandpa/src/communication/tests.rs b/substrate/client/finality-grandpa/src/communication/tests.rs index b2e4c405b4..4abea991ce 100644 --- a/substrate/client/finality-grandpa/src/communication/tests.rs +++ b/substrate/client/finality-grandpa/src/communication/tests.rs @@ -68,7 +68,7 @@ impl sc_network_gossip::Network for TestNetwork { let _ = self.sender.unbounded_send(Event::WriteNotification(who, message)); } - fn announce(&self, block: Hash, _associated_data: Vec) { + fn announce(&self, block: Hash, _associated_data: Option>) { let _ = self.sender.unbounded_send(Event::Announce(block)); } } diff --git a/substrate/client/network-gossip/src/bridge.rs b/substrate/client/network-gossip/src/bridge.rs index 4e8ebfda20..15451ec3cd 100644 --- a/substrate/client/network-gossip/src/bridge.rs +++ b/substrate/client/network-gossip/src/bridge.rs @@ -166,7 +166,7 @@ impl GossipEngine { /// /// Note: this method isn't strictly related to gossiping and should eventually be moved /// somewhere else. - pub fn announce(&self, block: B::Hash, associated_data: Vec) { + pub fn announce(&self, block: B::Hash, associated_data: Option>) { self.network.announce(block, associated_data); } } @@ -347,7 +347,7 @@ mod tests { unimplemented!(); } - fn announce(&self, _: B::Hash, _: Vec) { + fn announce(&self, _: B::Hash, _: Option>) { unimplemented!(); } } diff --git a/substrate/client/network-gossip/src/lib.rs b/substrate/client/network-gossip/src/lib.rs index 59c99088bd..7205533c81 100644 --- a/substrate/client/network-gossip/src/lib.rs +++ b/substrate/client/network-gossip/src/lib.rs @@ -98,7 +98,7 @@ pub trait Network { /// /// Note: this method isn't strictly related to gossiping and should eventually be moved /// somewhere else. - fn announce(&self, block: B::Hash, associated_data: Vec); + fn announce(&self, block: B::Hash, associated_data: Option>); } impl Network for Arc> { @@ -136,7 +136,7 @@ impl Network for Arc> { NetworkService::write_notification(self, who, protocol, message) } - fn announce(&self, block: B::Hash, associated_data: Vec) { + fn announce(&self, block: B::Hash, associated_data: Option>) { NetworkService::announce_block(self, block, associated_data) } } diff --git a/substrate/client/network-gossip/src/state_machine.rs b/substrate/client/network-gossip/src/state_machine.rs index 805f2e82ea..a58432d8c2 100644 --- a/substrate/client/network-gossip/src/state_machine.rs +++ b/substrate/client/network-gossip/src/state_machine.rs @@ -570,7 +570,7 @@ mod tests { unimplemented!(); } - fn announce(&self, _: B::Hash, _: Vec) { + fn announce(&self, _: B::Hash, _: Option>) { unimplemented!(); } } diff --git a/substrate/client/network/Cargo.toml b/substrate/client/network/Cargo.toml index 9c0fab84a8..64213b3f73 100644 --- a/substrate/client/network/Cargo.toml +++ b/substrate/client/network/Cargo.toml @@ -35,6 +35,7 @@ hex = "0.4.0" ip_network = "0.3.4" linked-hash-map = "0.5.2" linked_hash_set = "0.1.3" +lru = "0.6.3" log = "0.4.8" nohash-hasher = "0.2.0" parking_lot = "0.11.1" diff --git a/substrate/client/network/src/protocol.rs b/substrate/client/network/src/protocol.rs index 31ba770e93..6af5e12854 100644 --- a/substrate/client/network/src/protocol.rs +++ b/substrate/client/network/src/protocol.rs @@ -230,6 +230,8 @@ pub struct Protocol { metrics: Option, /// The `PeerId`'s of all boot nodes. boot_node_ids: HashSet, + /// A cache for the data that was associated to a block announcement. + block_announce_data_cache: lru::LruCache>, } /// Peer information @@ -491,6 +493,11 @@ impl Protocol { ) }; + let block_announce_data_cache = lru::LruCache::new( + network_config.default_peers_set.in_peers as usize + + network_config.default_peers_set.out_peers as usize, + ); + let protocol = Protocol { tick_timeout: Box::pin(interval(TICK_TIMEOUT)), propagate_timeout: Box::pin(interval(PROPAGATE_TIMEOUT)), @@ -514,6 +521,7 @@ impl Protocol { None }, boot_node_ids, + block_announce_data_cache, }; Ok((protocol, peerset_handle, known_addresses)) @@ -1069,7 +1077,7 @@ impl Protocol { /// /// In chain-based consensus, we often need to make sure non-best forks are /// at least temporarily synced. - pub fn announce_block(&mut self, hash: B::Hash, data: Vec) { + pub fn announce_block(&mut self, hash: B::Hash, data: Option>) { let header = match self.chain.header(BlockId::Hash(hash)) { Ok(Some(header)) => header, Ok(None) => { @@ -1090,6 +1098,8 @@ impl Protocol { let is_best = self.chain.info().best_hash == hash; debug!(target: "sync", "Reannouncing block {:?} is_best: {}", hash, is_best); + let data = data.or_else(|| self.block_announce_data_cache.get(&hash).cloned()).unwrap_or_default(); + for (who, ref mut peer) in self.peers.iter_mut() { let inserted = peer.known_blocks.insert(hash); if inserted { @@ -1160,9 +1170,17 @@ impl Protocol { validation_result: sync::PollBlockAnnounceValidation, ) -> CustomMessageOutcome { let (header, is_best, who) = match validation_result { - sync::PollBlockAnnounceValidation::Nothing { is_best, who, header } => { + sync::PollBlockAnnounceValidation::Skip => + return CustomMessageOutcome::None, + sync::PollBlockAnnounceValidation::Nothing { is_best, who, announce } => { self.update_peer_info(&who); + if let Some(data) = announce.data { + if !data.is_empty() { + self.block_announce_data_cache.put(announce.header.hash(), data); + } + } + // `on_block_announce` returns `OnBlockAnnounce::ImportHeader` // when we have all data required to import the block // in the BlockAnnounce message. This is only when: @@ -1170,14 +1188,21 @@ impl Protocol { // AND // 2) parent block is already imported and not pruned. if is_best { - return CustomMessageOutcome::PeerNewBest(who, *header.number()) + return CustomMessageOutcome::PeerNewBest(who, *announce.header.number()) } else { return CustomMessageOutcome::None } } - sync::PollBlockAnnounceValidation::ImportHeader { header, is_best, who } => { + sync::PollBlockAnnounceValidation::ImportHeader { announce, is_best, who } => { self.update_peer_info(&who); - (header, is_best, who) + + if let Some(data) = announce.data { + if !data.is_empty() { + self.block_announce_data_cache.put(announce.header.hash(), data); + } + } + + (announce.header, is_best, who) } sync::PollBlockAnnounceValidation::Failure { who, disconnect } => { if disconnect { diff --git a/substrate/client/network/src/protocol/sync.rs b/substrate/client/network/src/protocol/sync.rs index 70f860bdcb..03d5b64348 100644 --- a/substrate/client/network/src/protocol/sync.rs +++ b/substrate/client/network/src/protocol/sync.rs @@ -362,8 +362,8 @@ pub enum PollBlockAnnounceValidation { who: PeerId, /// Was this their new best block? is_best: bool, - /// The header of the announcement. - header: H, + /// The announcement. + announce: BlockAnnounce, }, /// The announcement header should be imported. ImportHeader { @@ -371,9 +371,11 @@ pub enum PollBlockAnnounceValidation { who: PeerId, /// Was this their new best block? is_best: bool, - /// The header of the announcement. - header: H, + /// The announcement. + announce: BlockAnnounce, }, + /// The block announcement should be skipped. + Skip, } /// Result of [`ChainSync::block_announce_validation`]. @@ -388,15 +390,6 @@ enum PreValidateBlockAnnounce { /// Should the peer be disconnected? disconnect: bool, }, - /// The announcement does not require further handling. - Nothing { - /// Who sent the processed block announcement? - who: PeerId, - /// Was this their new best block? - is_best: bool, - /// The announcement. - announce: BlockAnnounce, - }, /// The pre-validation was sucessful and the announcement should be /// further processed. Process { @@ -407,6 +400,8 @@ enum PreValidateBlockAnnounce { /// The announcement. announce: BlockAnnounce, }, + /// The block announcement should be skipped. + Skip, } /// Result of [`ChainSync::on_block_justification`]. @@ -1278,7 +1273,7 @@ impl ChainSync { who, hash, ); - PreValidateBlockAnnounce::Nothing { is_best, who, announce } + PreValidateBlockAnnounce::Skip }.boxed()); return } @@ -1295,7 +1290,7 @@ impl ChainSync { hash, who, ); - PreValidateBlockAnnounce::Nothing { is_best, who, announce } + PreValidateBlockAnnounce::Skip }.boxed()); return } @@ -1308,7 +1303,7 @@ impl ChainSync { hash, who, ); - PreValidateBlockAnnounce::Nothing { is_best, who, announce } + PreValidateBlockAnnounce::Skip }.boxed()); return } @@ -1337,7 +1332,7 @@ impl ChainSync { } Err(e) => { error!(target: "sync", "💔 Block announcement validation errored: {}", e); - PreValidateBlockAnnounce::Nothing { is_best, who, announce } + PreValidateBlockAnnounce::Skip } } }.boxed()); @@ -1393,10 +1388,6 @@ impl ChainSync { ); let (announce, is_best, who) = match pre_validation_result { - PreValidateBlockAnnounce::Nothing { is_best, who, announce } => { - self.peer_block_announce_validation_finished(&who); - return PollBlockAnnounceValidation::Nothing { is_best, who, header: announce.header } - }, PreValidateBlockAnnounce::Failure { who, disconnect } => { self.peer_block_announce_validation_finished(&who); return PollBlockAnnounceValidation::Failure { who, disconnect } @@ -1405,12 +1396,12 @@ impl ChainSync { self.peer_block_announce_validation_finished(&who); (announce, is_new_best, who) }, + PreValidateBlockAnnounce::Skip => return PollBlockAnnounceValidation::Skip, }; - let header = announce.header; - let number = *header.number(); - let hash = header.hash(); - let parent_status = self.block_status(header.parent_hash()).unwrap_or(BlockStatus::Unknown); + let number = *announce.header.number(); + let hash = announce.header.hash(); + let parent_status = self.block_status(announce.header.parent_hash()).unwrap_or(BlockStatus::Unknown); let known_parent = parent_status != BlockStatus::Unknown; let ancient_parent = parent_status == BlockStatus::InChainPruned; @@ -1419,7 +1410,7 @@ impl ChainSync { peer } else { error!(target: "sync", "💔 Called on_block_announce with a bad peer ID"); - return PollBlockAnnounceValidation::Nothing { is_best, who, header } + return PollBlockAnnounceValidation::Nothing { is_best, who, announce } }; if is_best { @@ -1430,7 +1421,7 @@ impl ChainSync { if let PeerSyncState::AncestorSearch {..} = peer.state { trace!(target: "sync", "Peer state is ancestor search."); - return PollBlockAnnounceValidation::Nothing { is_best, who, header } + return PollBlockAnnounceValidation::Nothing { is_best, who, announce } } // If the announced block is the best they have and is not ahead of us, our common number @@ -1438,7 +1429,7 @@ impl ChainSync { if is_best { if known && self.best_queued_number >= number { peer.update_common_number(number); - } else if header.parent_hash() == &self.best_queued_hash + } else if announce.header.parent_hash() == &self.best_queued_hash || known_parent && self.best_queued_number >= number { peer.update_common_number(number - One::one()); @@ -1452,37 +1443,52 @@ impl ChainSync { if let Some(target) = self.fork_targets.get_mut(&hash) { target.peers.insert(who.clone()); } - return PollBlockAnnounceValidation::Nothing { is_best, who, header } + return PollBlockAnnounceValidation::Nothing { is_best, who, announce } } if ancient_parent { - trace!(target: "sync", "Ignored ancient block announced from {}: {} {:?}", who, hash, header); - return PollBlockAnnounceValidation::Nothing { is_best, who, header } + trace!( + target: "sync", + "Ignored ancient block announced from {}: {} {:?}", + who, + hash, + announce.header, + ); + return PollBlockAnnounceValidation::Nothing { is_best, who, announce } } let requires_additional_data = !self.role.is_light() || !known_parent; if !requires_additional_data { - trace!(target: "sync", "Importing new header announced from {}: {} {:?}", who, hash, header); - return PollBlockAnnounceValidation::ImportHeader { is_best, header, who } + trace!( + target: "sync", + "Importing new header announced from {}: {} {:?}", + who, + hash, + announce.header, + ); + return PollBlockAnnounceValidation::ImportHeader { is_best, announce, who } } if number <= self.best_queued_number { trace!( target: "sync", - "Added sync target for block announced from {}: {} {:?}", who, hash, header + "Added sync target for block announced from {}: {} {:?}", + who, + hash, + announce.header, ); self.fork_targets .entry(hash.clone()) .or_insert_with(|| ForkTarget { number, - parent_hash: Some(*header.parent_hash()), + parent_hash: Some(*announce.header.parent_hash()), peers: Default::default(), }) .peers.insert(who.clone()); } trace!(target: "sync", "Announce validation result is nothing"); - PollBlockAnnounceValidation::Nothing { is_best, who, header } + PollBlockAnnounceValidation::Nothing { is_best, who, announce } } /// Call when a peer has disconnected. diff --git a/substrate/client/network/src/service.rs b/substrate/client/network/src/service.rs index 3d05d578bf..09acef62e7 100644 --- a/substrate/client/network/src/service.rs +++ b/substrate/client/network/src/service.rs @@ -858,7 +858,7 @@ impl NetworkService { /// /// In chain-based consensus, we often need to make sure non-best forks are /// at least temporarily synced. This function forces such an announcement. - pub fn announce_block(&self, hash: B::Hash, data: Vec) { + pub fn announce_block(&self, hash: B::Hash, data: Option>) { let _ = self.to_worker.unbounded_send(ServiceToWorkerMsg::AnnounceBlock(hash, data)); } @@ -1236,7 +1236,7 @@ enum ServiceToWorkerMsg { PropagateTransaction(H), PropagateTransactions, RequestJustification(B::Hash, NumberFor), - AnnounceBlock(B::Hash, Vec), + AnnounceBlock(B::Hash, Option>), GetValue(record::Key), PutValue(record::Key, Vec), AddKnownAddress(PeerId, Multiaddr), diff --git a/substrate/client/network/test/src/lib.rs b/substrate/client/network/test/src/lib.rs index ec5ab5e88d..786fddeed5 100644 --- a/substrate/client/network/test/src/lib.rs +++ b/substrate/client/network/test/src/lib.rs @@ -51,7 +51,10 @@ use sp_consensus::Error as ConsensusError; use sp_consensus::{BlockOrigin, ForkChoiceStrategy, BlockImportParams, BlockCheckParams, JustificationImport}; use futures::prelude::*; use futures::future::BoxFuture; -use sc_network::{NetworkWorker, NetworkService, config::ProtocolId}; +use sc_network::{ + NetworkWorker, NetworkService, config::{ProtocolId, MultiaddrWithPeerId, NonReservedPeerMode}, + Multiaddr, +}; use sc_network::config::{NetworkConfiguration, NonDefaultSetConfig, TransportConfig}; use libp2p::PeerId; use parking_lot::Mutex; @@ -228,6 +231,7 @@ pub struct Peer { network: NetworkWorker::Hash>, imported_blocks_stream: Pin> + Send>>, finality_notification_stream: Pin> + Send>>, + listen_addr: Multiaddr, } impl Peer { @@ -267,7 +271,7 @@ impl Peer { } /// Announces an important block on the network. - pub fn announce_block(&self, hash: ::Hash, data: Vec) { + pub fn announce_block(&self, hash: ::Hash, data: Option>) { self.network.service().announce_block(hash, data); } @@ -281,7 +285,7 @@ impl Peer { where F: FnMut(BlockBuilder) -> Block { let best_hash = self.client.info().best_hash; - self.generate_blocks_at(BlockId::Hash(best_hash), count, origin, edit_block, false, true) + self.generate_blocks_at(BlockId::Hash(best_hash), count, origin, edit_block, false, true, true) } /// Add blocks to the peer -- edit the block before adding. The chain will @@ -294,6 +298,7 @@ impl Peer { mut edit_block: F, headers_only: bool, inform_sync_about_new_best_block: bool, + announce_block: bool, ) -> H256 where F: FnMut(BlockBuilder) -> Block { let full_client = self.client.as_full() .expect("blocks could only be generated by full clients"); @@ -327,7 +332,9 @@ impl Peer { }; self.block_import.import_block(import_block, cache).expect("block_import failed"); - self.network.service().announce_block(hash, Vec::new()); + if announce_block { + self.network.service().announce_block(hash, None); + } at = hash; } @@ -337,7 +344,6 @@ impl Peer { full_client.header(&BlockId::Hash(at)).ok().flatten().unwrap().number().clone(), ); } - self.network.service().announce_block(at.clone(), Vec::new()); at } @@ -350,13 +356,13 @@ impl Peer { /// Push blocks to the peer (simplified: with or without a TX) pub fn push_headers(&mut self, count: usize) -> H256 { let best_hash = self.client.info().best_hash; - self.generate_tx_blocks_at(BlockId::Hash(best_hash), count, false, true, true) + self.generate_tx_blocks_at(BlockId::Hash(best_hash), count, false, true, true, true) } /// Push blocks to the peer (simplified: with or without a TX) starting from /// given hash. pub fn push_blocks_at(&mut self, at: BlockId, count: usize, with_tx: bool) -> H256 { - self.generate_tx_blocks_at(at, count, with_tx, false, true) + self.generate_tx_blocks_at(at, count, with_tx, false, true, true) } /// Push blocks to the peer (simplified: with or without a TX) starting from @@ -367,7 +373,18 @@ impl Peer { count: usize, with_tx: bool, ) -> H256 { - self.generate_tx_blocks_at(at, count, with_tx, false, false) + self.generate_tx_blocks_at(at, count, with_tx, false, false, true) + } + + /// Push blocks to the peer (simplified: with or without a TX) starting from + /// given hash without announcing the block. + pub fn push_blocks_at_without_announcing( + &mut self, + at: BlockId, + count: usize, + with_tx: bool, + ) -> H256 { + self.generate_tx_blocks_at(at, count, with_tx, false, true, false) } /// Push blocks/headers to the peer (simplified: with or without a TX) starting from @@ -379,6 +396,7 @@ impl Peer { with_tx: bool, headers_only: bool, inform_sync_about_new_best_block: bool, + announce_block: bool, ) -> H256 { let mut nonce = 0; if with_tx { @@ -398,6 +416,7 @@ impl Peer { }, headers_only, inform_sync_about_new_best_block, + announce_block, ) } else { self.generate_blocks_at( @@ -407,6 +426,7 @@ impl Peer { |builder| builder.build().unwrap().block, headers_only, inform_sync_about_new_best_block, + announce_block, ) } } @@ -585,6 +605,10 @@ pub struct FullPeerConfig { pub block_announce_validator: Option + Send + Sync>>, /// List of notification protocols that the network must support. pub notifications_protocols: Vec>, + /// The indices of the peers the peer should be connected to. + /// + /// If `None`, it will be connected to all other peers. + pub connect_to_peers: Option>, } pub trait TestNetFactory: Sized { @@ -689,6 +713,15 @@ pub trait TestNetFactory: Sized { set_config: Default::default() } }).collect(); + if let Some(connect_to) = config.connect_to_peers { + let addrs = connect_to.iter().map(|v| { + let peer_id = self.peer(*v).network_service().local_peer_id().clone(); + let multiaddr = self.peer(*v).listen_addr.clone(); + MultiaddrWithPeerId { peer_id, multiaddr } + }).collect(); + network_config.default_peers_set.reserved_nodes = addrs; + network_config.default_peers_set.non_reserved_mode = NonReservedPeerMode::Deny; + } let protocol_id = ProtocolId::from("test-protocol-name"); @@ -715,9 +748,12 @@ pub trait TestNetFactory: Sized { trace!(target: "test_network", "Peer identifier: {}", network.service().local_peer_id()); - self.mut_peers(|peers| { + self.mut_peers(move |peers| { for peer in peers.iter_mut() { - peer.network.add_known_address(network.service().local_peer_id().clone(), listen_addr.clone()); + peer.network.add_known_address( + network.service().local_peer_id().clone(), + listen_addr.clone(), + ); } let imported_blocks_stream = Box::pin(client.import_notification_stream().fuse()); @@ -733,6 +769,7 @@ pub trait TestNetFactory: Sized { block_import, verifier, network, + listen_addr, }); }); } @@ -813,6 +850,7 @@ pub trait TestNetFactory: Sized { imported_blocks_stream, finality_notification_stream, network, + listen_addr, }); }); } @@ -912,7 +950,7 @@ pub trait TestNetFactory: Sized { // We poll `imported_blocks_stream`. while let Poll::Ready(Some(notification)) = peer.imported_blocks_stream.as_mut().poll_next(cx) { - peer.network.service().announce_block(notification.hash, Vec::new()); + peer.network.service().announce_block(notification.hash, None); } // We poll `finality_notification_stream`, but we only take the last event. diff --git a/substrate/client/network/test/src/sync.rs b/substrate/client/network/test/src/sync.rs index 999f9fe1ee..582634fea2 100644 --- a/substrate/client/network/test/src/sync.rs +++ b/substrate/client/network/test/src/sync.rs @@ -436,7 +436,7 @@ fn can_sync_small_non_best_forks() { assert!(net.peer(0).client().header(&BlockId::Hash(small_hash)).unwrap().is_some()); assert!(!net.peer(1).client().header(&BlockId::Hash(small_hash)).unwrap().is_some()); - net.peer(0).announce_block(small_hash, Vec::new()); + net.peer(0).announce_block(small_hash, None); // after announcing, peer 1 downloads the block. @@ -452,7 +452,7 @@ fn can_sync_small_non_best_forks() { net.block_until_sync(); let another_fork = net.peer(0).push_blocks_at(BlockId::Number(35), 2, true); - net.peer(0).announce_block(another_fork, Vec::new()); + net.peer(0).announce_block(another_fork, None); block_on(futures::future::poll_fn::<(), _>(|cx| { net.poll(cx); if net.peer(1).client().header(&BlockId::Hash(another_fork)).unwrap().is_none() { @@ -500,7 +500,7 @@ fn light_peer_imports_header_from_announce() { sp_tracing::try_init_simple(); fn import_with_announce(net: &mut TestNet, hash: H256) { - net.peer(0).announce_block(hash, Vec::new()); + net.peer(0).announce_block(hash, None); block_on(futures::future::poll_fn::<(), _>(|cx| { net.poll(cx); @@ -610,7 +610,7 @@ fn does_not_sync_announced_old_best_block() { net.peer(0).push_blocks(18, true); net.peer(1).push_blocks(20, true); - net.peer(0).announce_block(old_hash, Vec::new()); + net.peer(0).announce_block(old_hash, None); block_on(futures::future::poll_fn::<(), _>(|cx| { // poll once to import announcement net.poll(cx); @@ -618,7 +618,7 @@ fn does_not_sync_announced_old_best_block() { })); assert!(!net.peer(1).is_major_syncing()); - net.peer(0).announce_block(old_hash_with_parent, Vec::new()); + net.peer(0).announce_block(old_hash_with_parent, None); block_on(futures::future::poll_fn::<(), _>(|cx| { // poll once to import announcement net.poll(cx); @@ -653,8 +653,8 @@ fn imports_stale_once() { fn import_with_announce(net: &mut TestNet, hash: H256) { // Announce twice - net.peer(0).announce_block(hash, Vec::new()); - net.peer(0).announce_block(hash, Vec::new()); + net.peer(0).announce_block(hash, None); + net.peer(0).announce_block(hash, None); block_on(futures::future::poll_fn::<(), _>(|cx| { net.poll(cx); @@ -842,3 +842,58 @@ fn sync_to_tip_when_we_sync_together_with_multiple_peers() { net.block_until_idle(); } } + +/// Ensures that when we receive a block announcement with some data attached, that we propagate +/// this data when reannouncing the block. +#[test] +fn block_announce_data_is_propagated() { + struct TestBlockAnnounceValidator; + + impl BlockAnnounceValidator for TestBlockAnnounceValidator { + fn validate( + &mut self, + _: &Header, + data: &[u8], + ) -> Pin>> + Send>> { + let correct = data.get(0) == Some(&137); + async move { + if correct { + Ok(Validation::Success { is_new_best: true }) + } else { + Ok(Validation::Failure { disconnect: false }) + } + }.boxed() + } + } + + sp_tracing::try_init_simple(); + let mut net = TestNet::new(1); + + net.add_full_peer_with_config(FullPeerConfig { + block_announce_validator: Some(Box::new(TestBlockAnnounceValidator)), + ..Default::default() + }); + + net.add_full_peer_with_config(FullPeerConfig { + block_announce_validator: Some(Box::new(TestBlockAnnounceValidator)), + connect_to_peers: Some(vec![1]), + ..Default::default() + }); + + // Wait until peer 1 is connected to both nodes. + block_on(futures::future::poll_fn::<(), _>(|cx| { + net.poll(cx); + if net.peer(1).num_peers() == 2 { + Poll::Ready(()) + } else { + Poll::Pending + } + })); + + let block_hash = net.peer(0).push_blocks_at_without_announcing(BlockId::Number(0), 1, true); + net.peer(0).announce_block(block_hash, Some(vec![137])); + + while !net.peer(1).has_block(&block_hash) || !net.peer(2).has_block(&block_hash) { + net.block_until_idle(); + } +} diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs index df1cd47db0..3033b1d09d 100644 --- a/substrate/client/service/src/lib.rs +++ b/substrate/client/service/src/lib.rs @@ -248,7 +248,7 @@ async fn build_network_future< }; if announce_imported_blocks { - network.service().announce_block(notification.hash, Vec::new()); + network.service().announce_block(notification.hash, None); } if notification.is_new_best {