Allow changing the behavior for imported blocks (#5236)

* Added option to disable default block announce

* Added on_block_imported on NetworkService

* Revert "Added on_block_imported on NetworkService"

This reverts commit ba360cad96e0cb041d7047af30df2a35eb112449.

* Do not announce block if set to not announce block

* Revert fix

* Moving default announce block to NetworkConfig

* WIP

Forked at: ad90ab7ec9
Parent branch: origin/master

* WIP

Forked at: ad90ab7ec9
Parent branch: origin/master

* Removing boolean in favor of explicit call

* Fixing tests

* WIP

Forked at: ad90ab7ec9
Parent branch: origin/master

* WIP

Forked at: ad90ab7ec9
Parent branch: origin/master

* increase spec_version

* increase spec_version

* Fixed test

* Fixing test

* Renamed should_announce_imported_blocks to announce_imported_blocks

* Updated assert_cmd

substrate/Cargo.lock

@@ -178,15 +178,16 @@ dependencies = [
 
 [[package]]
 name = "assert_cmd"
-version = "0.12.1"
+version = "1.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "35ad62275a8bda1c2c9a9303aea121eb04204272d3be0735d5dc1f49eb9ff9a9"
+checksum = "36101401a715c232f2c63a534a4b639415064b79d128d2a60d35678f8fb45204"
 dependencies = [
  "doc-comment",
  "escargot",
  "predicates",
  "predicates-core",
  "predicates-tree",
+ "wait-timeout",
 ]
 
 [[package]]
@@ -8935,6 +8936,15 @@ dependencies = [
  "glob 0.2.11",
 ]
 
+[[package]]
+name = "wait-timeout"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9f200f5b12eb75f8c1ed65abd4b2db8a6e1b138a20de009dacee265a2498f3f6"
+dependencies = [
+ "libc",
+]
+
 [[package]]
 name = "walkdir"
 version = "2.3.1"

substrate/bin/node/cli/Cargo.toml

@@ -113,7 +113,7 @@ sc-consensus-epochs = { version = "0.8.0-alpha.5", path = "../../../client/conse
 sc-service-test = { version = "2.0.0-dev", path = "../../../client/service/test" }
 futures = "0.3.4"
 tempfile = "3.1.0"
-assert_cmd = "0.12"
+assert_cmd = "1.0"
 nix = "0.17"
 serde_json = "1.0"
 

substrate/client/network/src/protocol.rs

@@ -1467,20 +1467,11 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
 		}
 	}
 
-	/// Call this when a block has been imported in the import queue and we should announce it on
-	/// the network.
-	pub fn on_block_imported(&mut self, header: &B::Header, data: Vec<u8>, is_best: bool) {
+	/// Call this when a block has been imported in the import queue
+	pub fn on_block_imported(&mut self, header: &B::Header, is_best: bool) {
 		if is_best {
 			self.sync.update_chain_info(header);
 		}
-
-		// blocks are not announced by light clients
-		if self.config.roles.is_light() {
-			return;
-		}
-
-		// send out block announcements
-		self.send_announcement(header, data, is_best, false);
 	}
 
 	/// Call this when a block has been finalized. The sync layer may have some additional

substrate/client/network/src/service.rs

@@ -403,8 +403,8 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> {
 	}
 
 	/// You must call this when a new block is imported by the client.
-	pub fn on_block_imported(&mut self, header: B::Header, data: Vec<u8>, is_best: bool) {
-		self.network_service.user_protocol_mut().on_block_imported(&header, data, is_best);
+	pub fn on_block_imported(&mut self, header: B::Header, is_best: bool) {
+		self.network_service.user_protocol_mut().on_block_imported(&header, is_best);
 	}
 
 	/// You must call this when a new block is finalized by the client.

substrate/client/network/test/src/lib.rs

@@ -281,7 +281,8 @@ impl<D> Peer<D> {
 				Default::default()
 			};
 			self.block_import.import_block(import_block, cache).expect("block_import failed");
-			self.network.on_block_imported(header, Vec::new(), true);
+			self.network.on_block_imported(header, true);
+			self.network.service().announce_block(hash, Vec::new());
 			at = hash;
 		}
 
@@ -785,9 +786,9 @@ pub trait TestNetFactory: Sized {
 				while let Poll::Ready(Some(notification)) = peer.imported_blocks_stream.as_mut().poll_next(cx) {
 					peer.network.on_block_imported(
 						notification.header,
-						Vec::new(),
 						true,
 					);
+					peer.network.service().announce_block(notification.hash, Vec::new());
 				}
 
 				// We poll `finality_notification_stream`, but we only take the last event.

substrate/client/service/src/builder.rs

@@ -1204,6 +1204,7 @@ ServiceBuilder<
 				network_status_sinks.clone(),
 				system_rpc_rx,
 				has_bootnodes,
+				config.announce_block,
 			),
 		);
 

substrate/client/service/src/config.rs

@@ -127,6 +127,8 @@ pub struct Configuration {
 	///
 	/// The default value is 8.
 	pub max_runtime_instances: usize,
+	/// Announce block automatically after they have been imported
+	pub announce_block: bool,
 }
 
 /// Configuration of the client keystore.
@@ -229,6 +231,7 @@ impl Default for Configuration {
 			tracing_targets: Default::default(),
 			tracing_receiver: Default::default(),
 			max_runtime_instances: 8,
+			announce_block: true,
 		}
 	}
 }

substrate/client/service/src/lib.rs

@@ -328,6 +328,7 @@ fn build_network_future<
 	status_sinks: Arc<Mutex<status_sinks::StatusSinks<(NetworkStatus<B>, NetworkState)>>>,
 	mut rpc_rx: mpsc::UnboundedReceiver<sc_rpc::system::Request<B>>,
 	should_have_peers: bool,
+	announce_imported_blocks: bool,
 ) -> impl Future<Output = ()> {
 	let mut imported_blocks_stream = client.import_notification_stream().fuse();
 	let mut finality_notification_stream = client.finality_notification_stream().fuse();
@@ -337,7 +338,11 @@ fn build_network_future<
 
 		// We poll `imported_blocks_stream`.
 		while let Poll::Ready(Some(notification)) = Pin::new(&mut imported_blocks_stream).poll_next(cx) {
-			network.on_block_imported(notification.header, Vec::new(), notification.is_new_best);
+			network.on_block_imported(notification.header, notification.is_new_best);
+
+			if announce_imported_blocks {
+				network.service().announce_block(notification.hash, Vec::new());
+			}
 		}
 
 		// We poll `finality_notification_stream`, but we only take the last event.

substrate/client/service/test/src/lib.rs

@@ -213,6 +213,7 @@ fn node_config<G: RuntimeGenesis + 'static, E: ChainSpecExtension + Clone + 'sta
 		tracing_targets: None,
 		tracing_receiver: Default::default(),
 		max_runtime_instances: 8,
+		announce_block: true,
 	}
 }