diff --git a/substrate/client/network/src/protocol.rs b/substrate/client/network/src/protocol.rs
index 597031b90182c812a5e8851b3e088e4feeef6d0e..f794b106da24d1b2875aa857296e2ca116ac20eb 100644
--- a/substrate/client/network/src/protocol.rs
+++ b/substrate/client/network/src/protocol.rs
@@ -528,10 +528,9 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
self.sync.num_sync_requests()
}
- /// Sync local state with the blockchain state.
- pub fn update_chain(&mut self) {
- let info = self.context_data.chain.info();
- self.sync.update_chain_info(&info.best_hash, info.best_number);
+ /// Inform sync about new best imported block.
+ pub fn new_best_block_imported(&mut self, hash: B::Hash, number: NumberFor<B>) {
+ self.sync.update_chain_info(&hash, number);
self.behaviour.set_legacy_handshake_message(
build_status_message(&self.config, &self.context_data.chain),
);
@@ -541,11 +540,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
);
}
- /// Inform sync about an own imported block.
- pub fn own_block_imported(&mut self, hash: B::Hash, number: NumberFor<B>) {
- self.sync.update_chain_info(&hash, number);
- }
-
fn update_peer_info(&mut self, who: &PeerId) {
if let Some(info) = self.sync.peer_info(who) {
if let Some(ref mut peer) = self.context_data.peers.get_mut(who) {
@@ -1258,18 +1252,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
count: usize,
results: Vec<(Result<BlockImportResult<NumberFor<B>>, BlockImportError>, B::Hash)>
) {
- let new_best = results.iter().rev().find_map(|r| match r {
- (Ok(BlockImportResult::ImportedUnknown(n, aux, _)), hash) if aux.is_new_best => Some((*n, hash.clone())),
- _ => None,
- });
- if let Some((best_num, best_hash)) = new_best {
- self.sync.update_chain_info(&best_hash, best_num);
- self.behaviour.set_legacy_handshake_message(build_status_message(&self.config, &self.context_data.chain));
- self.behaviour.set_notif_protocol_handshake(
- &self.block_announces_protocol,
- BlockAnnouncesHandshake::build(&self.config, &self.context_data.chain).encode()
- );
- }
let results = self.sync.on_blocks_processed(
imported,
count,
diff --git a/substrate/client/network/src/service.rs b/substrate/client/network/src/service.rs
index 8ef76d4850699a774a09efc0e792b2506e744fcc..0a87c37703d89b15083c4a1e148cd5d1a0c7bb74 100644
--- a/substrate/client/network/src/service.rs
+++ b/substrate/client/network/src/service.rs
@@ -484,11 +484,9 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> {
self.network_service.user_protocol_mut().on_block_finalized(hash, &header);
}
- /// This should be called when blocks are added to the
- /// chain by something other than the import queue.
- /// Currently this is only useful for tests.
- pub fn update_chain(&mut self) {
- self.network_service.user_protocol_mut().update_chain();
+ /// Inform the network service about new best imported block.
+ pub fn new_best_block_imported(&mut self, hash: B::Hash, number: NumberFor<B>) {
+ self.network_service.user_protocol_mut().new_best_block_imported(hash, number);
}
/// Returns the local `PeerId`.
@@ -1012,21 +1010,11 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkService<B, H> {
self.num_connected.load(Ordering::Relaxed)
}
- /// This function should be called when blocks are added to the chain by something other
- /// than the import queue.
- ///
- /// > **Important**: This function is a hack and can be removed at any time. Do **not** use it.
- pub fn update_chain(&self) {
- let _ = self
- .to_worker
- .unbounded_send(ServiceToWorkerMsg::UpdateChain);
- }
-
- /// Inform the network service about an own imported block.
- pub fn own_block_imported(&self, hash: B::Hash, number: NumberFor<B>) {
+ /// Inform the network service about new best imported block.
+ pub fn new_best_block_imported(&self, hash: B::Hash, number: NumberFor<B>) {
let _ = self
.to_worker
- .unbounded_send(ServiceToWorkerMsg::OwnBlockImported(hash, number));
+ .unbounded_send(ServiceToWorkerMsg::NewBestBlockImported(hash, number));
}
/// Utility function to extract `PeerId` from each `Multiaddr` for priority group updates.
@@ -1181,8 +1169,7 @@ enum ServiceToWorkerMsg<B: BlockT, H: ExHashT> {
protocol_name: Cow<'static, str>,
},
DisconnectPeer(PeerId),
- UpdateChain,
- OwnBlockImported(B::Hash, NumberFor<B>),
+ NewBestBlockImported(B::Hash, NumberFor<B>),
}
/// Main network worker. Must be polled in order for the network to advance.
@@ -1319,10 +1306,8 @@ impl<B: BlockT + 'static, H: ExHashT> Future for NetworkWorker<B, H> {
this.network_service.register_notifications_protocol(protocol_name),
ServiceToWorkerMsg::DisconnectPeer(who) =>
this.network_service.user_protocol_mut().disconnect_peer(&who),
- ServiceToWorkerMsg::UpdateChain =>
- this.network_service.user_protocol_mut().update_chain(),
- ServiceToWorkerMsg::OwnBlockImported(hash, number) =>
- this.network_service.user_protocol_mut().own_block_imported(hash, number),
+ ServiceToWorkerMsg::NewBestBlockImported(hash, number) =>
+ this.network_service.user_protocol_mut().new_best_block_imported(hash, number),
}
}
diff --git a/substrate/client/network/test/src/lib.rs b/substrate/client/network/test/src/lib.rs
index 6950ada4f8458c1413d0ee6ad3ada0b658d99038..a70ecb4fb0484d1b9d6972b74743c30e570b1844 100644
--- a/substrate/client/network/test/src/lib.rs
+++ b/substrate/client/network/test/src/lib.rs
@@ -279,7 +279,7 @@ impl<D> Peer<D> {
where F: FnMut(BlockBuilder<Block, PeersFullClient, substrate_test_runtime_client::Backend>) -> Block
{
let best_hash = self.client.info().best_hash;
- self.generate_blocks_at(BlockId::Hash(best_hash), count, origin, edit_block, false)
+ self.generate_blocks_at(BlockId::Hash(best_hash), count, origin, edit_block, false, true)
}
/// Add blocks to the peer -- edit the block before adding. The chain will
@@ -291,6 +291,7 @@ impl<D> Peer<D> {
origin: BlockOrigin,
mut edit_block: F,
headers_only: bool,
+ inform_sync_about_new_best_block: bool,
) -> H256 where F: FnMut(BlockBuilder<Block, PeersFullClient, substrate_test_runtime_client::Backend>) -> Block {
let full_client = self.client.as_full()
.expect("blocks could only be generated by full clients");
@@ -328,7 +329,12 @@ impl<D> Peer<D> {
at = hash;
}
- self.network.update_chain();
+ if inform_sync_about_new_best_block {
+ self.network.new_best_block_imported(
+ at,
+ full_client.header(&BlockId::Hash(at)).ok().flatten().unwrap().number().clone(),
+ );
+ }
self.network.service().announce_block(at.clone(), Vec::new());
at
}
@@ -342,18 +348,36 @@ impl<D> Peer<D> {
/// 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)
+ self.generate_tx_blocks_at(BlockId::Hash(best_hash), count, false, 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<Block>, count: usize, with_tx: bool) -> H256 {
- self.generate_tx_blocks_at(at, count, with_tx, false)
+ self.generate_tx_blocks_at(at, count, with_tx, false, true)
+ }
+
+ /// Push blocks to the peer (simplified: with or without a TX) starting from
+ /// given hash without informing the sync protocol about the new best block.
+ pub fn push_blocks_at_without_informing_sync(
+ &mut self,
+ at: BlockId<Block>,
+ count: usize,
+ with_tx: bool,
+ ) -> H256 {
+ self.generate_tx_blocks_at(at, count, with_tx, false, false)
}
/// Push blocks/headers to the peer (simplified: with or without a TX) starting from
/// given hash.
- fn generate_tx_blocks_at(&mut self, at: BlockId<Block>, count: usize, with_tx: bool, headers_only:bool) -> H256 {
+ fn generate_tx_blocks_at(
+ &mut self,
+ at: BlockId<Block>,
+ count: usize,
+ with_tx: bool,
+ headers_only: bool,
+ inform_sync_about_new_best_block: bool,
+ ) -> H256 {
let mut nonce = 0;
if with_tx {
self.generate_blocks_at(
@@ -370,7 +394,8 @@ impl<D> Peer<D> {
nonce = nonce + 1;
builder.build().unwrap().block
},
- headers_only
+ headers_only,
+ inform_sync_about_new_best_block,
)
} else {
self.generate_blocks_at(
@@ -379,6 +404,7 @@ impl<D> Peer<D> {
BlockOrigin::File,
|builder| builder.build().unwrap().block,
headers_only,
+ inform_sync_about_new_best_block,
)
}
}
diff --git a/substrate/client/network/test/src/sync.rs b/substrate/client/network/test/src/sync.rs
index 64985871d85e0737ca16a755381310b94a0357d1..9a488ae4fa49c32b2e3ec62f8e43b97a5eafb8a3 100644
--- a/substrate/client/network/test/src/sync.rs
+++ b/substrate/client/network/test/src/sync.rs
@@ -779,3 +779,38 @@ fn wait_until_deferred_block_announce_validation_is_ready() {
net.block_until_idle();
}
}
+
+/// When we don't inform the sync protocol about the best block, a node will not sync from us as the
+/// handshake is not does not contain our best block.
+#[test]
+fn sync_to_tip_requires_that_sync_protocol_is_informed_about_best_block() {
+ sp_tracing::try_init_simple();
+ log::trace!(target: "sync", "Test");
+ let mut net = TestNet::new(1);
+
+ // Produce some blocks
+ let block_hash = net.peer(0).push_blocks_at_without_informing_sync(BlockId::Number(0), 3, true);
+
+ // Add a node and wait until they are connected
+ net.add_full_peer_with_config(Default::default());
+ net.block_until_connected();
+ net.block_until_idle();
+
+ // The peer should not have synced the block.
+ assert!(!net.peer(1).has_block(&block_hash));
+
+ // Make sync protocol aware of the best block
+ net.peer(0).network_service().new_best_block_imported(block_hash, 3);
+ net.block_until_idle();
+
+ // Connect another node that should now sync to the tip
+ net.add_full_peer_with_config(Default::default());
+ net.block_until_connected();
+
+ while !net.peer(2).has_block(&block_hash) {
+ net.block_until_idle();
+ }
+
+ // However peer 1 should still not have the block.
+ assert!(!net.peer(1).has_block(&block_hash));
+}
diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs
index fd5ad9ebac917e15bef4bf20faf662aca7eb7292..fdccbde6a0206395fdf1f9842051ed914bd0d042 100644
--- a/substrate/client/service/src/lib.rs
+++ b/substrate/client/service/src/lib.rs
@@ -250,8 +250,8 @@ async fn build_network_future<
network.service().announce_block(notification.hash, Vec::new());
}
- if let sp_consensus::BlockOrigin::Own = notification.origin {
- network.service().own_block_imported(
+ if notification.is_new_best {
+ network.service().new_best_block_imported(
notification.hash,
notification.header.number().clone(),
);
diff --git a/substrate/client/service/test/src/lib.rs b/substrate/client/service/test/src/lib.rs
index 8a9f0ace171d88302a34509d2edcb159e649f83e..28930473f0a0a00a8055190d7f7c4c2d4f9d747f 100644
--- a/substrate/client/service/test/src/lib.rs
+++ b/substrate/client/service/test/src/lib.rs
@@ -542,7 +542,8 @@ pub fn sync<G, E, Fb, F, Lb, L, B, ExF, U>(
make_block_and_import(&first_service, first_user_data);
}
- network.full_nodes[0].1.network().update_chain();
+ let info = network.full_nodes[0].1.client().info();
+ network.full_nodes[0].1.network().new_best_block_imported(info.best_hash, info.best_number);
network.full_nodes[0].3.clone()
};
diff --git a/substrate/primitives/consensus/common/src/import_queue.rs b/substrate/primitives/consensus/common/src/import_queue.rs
index 3ad8c7c92e07f80028de8d052406e28ae5386c27..b32ca0133d9954f539313d54b7b0186ec17f9dde 100644
--- a/substrate/primitives/consensus/common/src/import_queue.rs
+++ b/substrate/primitives/consensus/common/src/import_queue.rs
@@ -136,7 +136,7 @@ pub trait Link<B: BlockT>: Send {
/// Block import successful result.
#[derive(Debug, PartialEq)]
-pub enum BlockImportResult<N: ::std::fmt::Debug + PartialEq> {
+pub enum BlockImportResult<N: std::fmt::Debug + PartialEq> {
/// Imported known block.
ImportedKnown(N),
/// Imported unknown block.