Remove --legacy-network-protocol CLI flag (#6411)

Author: tomaka

client/cli/src/params/network_params.rs

@@ -102,11 +102,6 @@ pub struct NetworkParams {
 	/// By default this option is true for `--dev` and false otherwise.
 	#[structopt(long)]
 	pub discover_local: bool,
-
-	/// Use the legacy "pre-mainnet-launch" networking protocol. Enable if things seem broken.
-	/// This option will be removed in the future.
-	#[structopt(long)]
-	pub legacy_network_protocol: bool,
 }
 
 impl NetworkParams {
@@ -165,7 +160,6 @@ impl NetworkParams {
 			},
 			max_parallel_downloads: self.max_parallel_downloads,
 			allow_non_globals_in_dht: self.discover_local || is_dev,
-			use_new_block_requests_protocol: !self.legacy_network_protocol,
 		}
 	}
 }

client/network/src/config.rs

@@ -425,9 +425,6 @@ pub struct NetworkConfiguration {
 	pub max_parallel_downloads: u32,
 	/// Should we insert non-global addresses into the DHT?
 	pub allow_non_globals_in_dht: bool,
-	/// If true, uses the `/<chainid>/block-requests/<version>` experimental protocol rather than
-	/// the legacy substream. This option is meant to be hard-wired to `true` in the future.
-	pub use_new_block_requests_protocol: bool,
 }
 
 impl NetworkConfiguration {
@@ -459,7 +456,6 @@ impl NetworkConfiguration {
 			},
 			max_parallel_downloads: 5,
 			allow_non_globals_in_dht: false,
-			use_new_block_requests_protocol: true,
 		}
 	}
 }

client/network/src/protocol.rs

@@ -250,9 +250,6 @@ pub struct Protocol<B: BlockT, H: ExHashT> {
 	metrics: Option<Metrics>,
 	/// The `PeerId`'s of all boot nodes.
 	boot_node_ids: Arc<HashSet<PeerId>>,
-	/// If true, we send back requests as `CustomMessageOutcome` events. If false, we directly
-	/// dispatch requests using the legacy substream.
-	use_new_block_requests_protocol: bool,
 }
 
 #[derive(Default)]
@@ -374,7 +371,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
 		block_announce_validator: Box<dyn BlockAnnounceValidator<B> + Send>,
 		metrics_registry: Option<&Registry>,
 		boot_node_ids: Arc<HashSet<PeerId>>,
-		use_new_block_requests_protocol: bool,
 		queue_size_report: Option<HistogramVec>,
 	) -> error::Result<(Protocol<B, H>, sc_peerset::PeersetHandle)> {
 		let info = chain.info();
@@ -458,7 +454,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
 				None
 			},
 			boot_node_ids,
-			use_new_block_requests_protocol,
 		};
 
 		Ok((protocol, peerset_handle))
@@ -655,16 +650,6 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
 		CustomMessageOutcome::None
 	}
 
-	fn send_request(&mut self, who: &PeerId, message: Message<B>) {
-		send_request::<B, H>(
-			&mut self.behaviour,
-			&mut self.context_data.stats,
-			&mut self.context_data.peers,
-			who,
-			message,
-		);
-	}
-
 	fn send_message(
 		&mut self,
 		who: &PeerId,
@@ -896,15 +881,10 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
 				Ok(sync::OnBlockData::Import(origin, blocks)) =>
 					CustomMessageOutcome::BlockImport(origin, blocks),
 				Ok(sync::OnBlockData::Request(peer, mut req)) => {
-					if self.use_new_block_requests_protocol {
-						self.update_peer_request(&peer, &mut req);
-						CustomMessageOutcome::BlockRequest {
-							target: peer,
-							request: req,
-						}
-					} else {
-						self.send_request(&peer, GenericMessage::BlockRequest(req));
-						CustomMessageOutcome::None
+					self.update_peer_request(&peer, &mut req);
+					CustomMessageOutcome::BlockRequest {
+						target: peer,
+						request: req,
 					}
 				}
 				Err(sync::BadPeer(id, repu)) => {
@@ -1077,15 +1057,11 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
 			match self.sync.new_peer(who.clone(), info.best_hash, info.best_number) {
 				Ok(None) => (),
 				Ok(Some(mut req)) => {
-					if self.use_new_block_requests_protocol {
-						self.update_peer_request(&who, &mut req);
-						self.pending_messages.push_back(CustomMessageOutcome::BlockRequest {
-							target: who.clone(),
-							request: req,
-						});
-					} else {
-						self.send_request(&who, GenericMessage::BlockRequest(req))
-					}
+					self.update_peer_request(&who, &mut req);
+					self.pending_messages.push_back(CustomMessageOutcome::BlockRequest {
+						target: who.clone(),
+						request: req,
+					});
 				},
 				Err(sync::BadPeer(id, repu)) => {
 					self.behaviour.disconnect_peer(&id);
@@ -1415,15 +1391,10 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
 				CustomMessageOutcome::BlockImport(origin, blocks)
 			},
 			Ok(sync::OnBlockData::Request(peer, mut req)) => {
-				if self.use_new_block_requests_protocol {
-					self.update_peer_request(&peer, &mut req);
-					CustomMessageOutcome::BlockRequest {
-						target: peer,
-						request: req,
-					}
-				} else {
-					self.send_request(&peer, GenericMessage::BlockRequest(req));
-					CustomMessageOutcome::None
+				self.update_peer_request(&peer, &mut req);
+				CustomMessageOutcome::BlockRequest {
+					target: peer,
+					request: req,
 				}
 			}
 			Err(sync::BadPeer(id, repu)) => {
@@ -1523,22 +1494,11 @@ impl<B: BlockT, H: ExHashT> Protocol<B, H> {
 		for result in results {
 			match result {
 				Ok((id, mut req)) => {
-					if self.use_new_block_requests_protocol {
-						update_peer_request(&mut self.context_data.peers, &id, &mut req);
-						self.pending_messages.push_back(CustomMessageOutcome::BlockRequest {
-							target: id,
-							request: req,
-						});
-					} else {
-						let msg = GenericMessage::BlockRequest(req);
-						send_request(
-							&mut self.behaviour,
-							&mut self.context_data.stats,
-							&mut self.context_data.peers,
-							&id,
-							msg
-						)
-					}
+					update_peer_request(&mut self.context_data.peers, &id, &mut req);
+					self.pending_messages.push_back(CustomMessageOutcome::BlockRequest {
+						target: id,
+						request: req,
+					});
 				}
 				Err(sync::BadPeer(id, repu)) => {
 					self.behaviour.disconnect_peer(&id);
@@ -1917,27 +1877,6 @@ pub enum CustomMessageOutcome<B: BlockT> {
 	None,
 }
 
-fn send_request<B: BlockT, H: ExHashT>(
-	behaviour: &mut GenericProto,
-	stats: &mut HashMap<&'static str, PacketStats>,
-	peers: &mut HashMap<PeerId, Peer<B, H>>,
-	who: &PeerId,
-	mut message: Message<B>,
-) {
-	if let GenericMessage::BlockRequest(ref mut r) = message {
-		if let Some(ref mut peer) = peers.get_mut(who) {
-			r.id = peer.next_request_id;
-			peer.next_request_id += 1;
-			if let Some((timestamp, request)) = peer.block_request.take() {
-				trace!(target: "sync", "Request {} for {} is now obsolete.", request.id, who);
-				peer.obsolete_requests.insert(request.id, timestamp);
-			}
-			peer.block_request = Some((Instant::now(), r.clone()));
-		}
-	}
-	send_message::<B>(behaviour, stats, who, None, message)
-}
-
 fn update_peer_request<B: BlockT, H: ExHashT>(
 	peers: &mut HashMap<PeerId, Peer<B, H>>,
 	who: &PeerId,
@@ -2032,58 +1971,28 @@ impl<B: BlockT, H: ExHashT> NetworkBehaviour for Protocol<B, H> {
 		}
 
 		for (id, mut r) in self.sync.block_requests() {
-			if self.use_new_block_requests_protocol {
-				update_peer_request(&mut self.context_data.peers, &id, &mut r);
-				let event = CustomMessageOutcome::BlockRequest {
-					target: id.clone(),
-					request: r,
-				};
-				self.pending_messages.push_back(event);
-			} else {
-				send_request(
-					&mut self.behaviour,
-					&mut self.context_data.stats,
-					&mut self.context_data.peers,
-					&id,
-					GenericMessage::BlockRequest(r),
-				)
-			}
+			update_peer_request(&mut self.context_data.peers, &id, &mut r);
+			let event = CustomMessageOutcome::BlockRequest {
+				target: id.clone(),
+				request: r,
+			};
+			self.pending_messages.push_back(event);
 		}
 		for (id, mut r) in self.sync.justification_requests() {
-			if self.use_new_block_requests_protocol {
-				update_peer_request(&mut self.context_data.peers, &id, &mut r);
-				let event = CustomMessageOutcome::BlockRequest {
-					target: id,
-					request: r,
-				};
-				self.pending_messages.push_back(event);
-			} else {
-				send_request(
-					&mut self.behaviour,
-					&mut self.context_data.stats,
-					&mut self.context_data.peers,
-					&id,
-					GenericMessage::BlockRequest(r),
-				)
-			}
+			update_peer_request(&mut self.context_data.peers, &id, &mut r);
+			let event = CustomMessageOutcome::BlockRequest {
+				target: id,
+				request: r,
+			};
+			self.pending_messages.push_back(event);
 		}
 		for (id, r) in self.sync.finality_proof_requests() {
-			if self.use_new_block_requests_protocol {
-				let event = CustomMessageOutcome::FinalityProofRequest {
-					target: id,
-					block_hash: r.block,
-					request: r.request,
-				};
-				self.pending_messages.push_back(event);
-			} else {
-				send_request(
-					&mut self.behaviour,
-					&mut self.context_data.stats,
-					&mut self.context_data.peers,
-					&id,
-					GenericMessage::FinalityProofRequest(r),
-				)
-			}
+			let event = CustomMessageOutcome::FinalityProofRequest {
+				target: id,
+				block_hash: r.block,
+				request: r.request,
+			};
+			self.pending_messages.push_back(event);
 		}
 		if let Poll::Ready(Some((peer_id, result))) = self.pending_transactions.poll_next_unpin(cx) {
 			self.on_handle_extrinsic_import(peer_id, result);
@@ -2237,7 +2146,6 @@ mod tests {
 			Box::new(DefaultBlockAnnounceValidator::new(client.clone())),
 			None,
 			Default::default(),
-			true,
 			None,
 		).unwrap();
 

client/network/src/service.rs

@@ -224,7 +224,6 @@ impl<B: BlockT + 'static, H: ExHashT> NetworkWorker<B, H> {
 			params.block_announce_validator,
 			params.metrics_registry.as_ref(),
 			boot_node_ids.clone(),
-			params.network_config.use_new_block_requests_protocol,
 			metrics.as_ref().map(|m| m.notifications_queues_size.clone()),
 		)?;