Remove set_finality_proof_request_builder (#3087)

* Remove set_finality_proof_request_builder

* Fix Babe

* Fix Grandpa

* Fix service doctests

substrate/core/network/src/config.rs

@@ -54,6 +54,11 @@ pub struct Params<B: BlockT, S, H: ExHashT> {
 	/// from us.
 	pub finality_proof_provider: Option<Arc<dyn FinalityProofProvider<B>>>,
 
+	/// How to build requests for proofs of finality.
+	///
+	/// This object, if `Some`, is used when we need a proof of finality from another node.
+	pub finality_proof_request_builder: Option<BoxFinalityProofRequestBuilder<B>>,
+
 	/// The `OnDemand` object acts as a "receiver" for block data requests from the client.
 	/// If `Some`, the network worker will process these requests and answer them.
 	/// Normally used only for light clients.
@@ -116,6 +121,25 @@ impl parity_codec::Decode for Roles {
 	}
 }
 
+/// Finality proof request builder.
+pub trait FinalityProofRequestBuilder<B: BlockT>: Send {
+	/// Build data blob, associated with the request.
+	fn build_request_data(&mut self, hash: &B::Hash) -> Vec<u8>;
+}
+
+/// Implementation of `FinalityProofRequestBuilder` that builds a dummy empty request.
+#[derive(Debug, Default)]
+pub struct DummyFinalityProofRequestBuilder;
+
+impl<B: BlockT> FinalityProofRequestBuilder<B> for DummyFinalityProofRequestBuilder {
+	fn build_request_data(&mut self, _: &B::Hash) -> Vec<u8> {
+		Vec::new()
+	}
+}
+
+/// Shared finality proof request builder struct used by the queue.
+pub type BoxFinalityProofRequestBuilder<B> = Box<dyn FinalityProofRequestBuilder<B> + Send + Sync>;
+
 /// Name of a protocol, transmitted on the wire. Should be unique for each chain.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct ProtocolId(smallvec::SmallVec<[u8; 6]>);

substrate/core/network/src/protocol.rs

@@ -28,7 +28,6 @@ use runtime_primitives::traits::{
 	Block as BlockT, Header as HeaderT, NumberFor, One, Zero,
 	CheckedSub, SaturatedConversion
 };
-use consensus::import_queue::BoxFinalityProofRequestBuilder;
 use message::{
 	BlockRequest as BlockRequestMessage,
 	FinalityProofRequest as FinalityProofRequestMessage, Message,
@@ -41,7 +40,7 @@ use on_demand::{OnDemandCore, OnDemandNetwork, RequestData};
 use specialization::NetworkSpecialization;
 use sync::{ChainSync, Context as SyncContext, SyncState};
 use crate::service::{TransactionPool, ExHashT};
-use crate::config::Roles;
+use crate::config::{BoxFinalityProofRequestBuilder, Roles};
 use rustc_hex::ToHex;
 use std::collections::{BTreeMap, HashMap};
 use std::sync::Arc;
@@ -390,11 +389,12 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Protocol<B, S, H> {
 		specialization: S,
 		transaction_pool: Arc<dyn TransactionPool<H, B>>,
 		finality_proof_provider: Option<Arc<dyn FinalityProofProvider<B>>>,
+		finality_proof_request_builder: Option<BoxFinalityProofRequestBuilder<B>>,
 		protocol_id: ProtocolId,
 		peerset_config: peerset::PeersetConfig,
 	) -> error::Result<(Protocol<B, S, H>, peerset::PeersetHandle)> {
 		let info = chain.info();
-		let sync = ChainSync::new(config.roles, &info);
+		let sync = ChainSync::new(config.roles, &info, finality_proof_request_builder);
 		let (peerset, peerset_handle) = peerset::Peerset::from_config(peerset_config);
 		let versions = &((MIN_VERSION as u8)..=(CURRENT_VERSION as u8)).collect::<Vec<u8>>();
 		let behaviour = CustomProto::new(protocol_id, versions, peerset);
@@ -1251,10 +1251,6 @@ impl<B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Protocol<B, S, H> {
 		self.sync.block_imported(hash, number)
 	}
 
-	pub fn set_finality_proof_request_builder(&mut self, request_builder: BoxFinalityProofRequestBuilder<B>) {
-		self.sync.set_finality_proof_request_builder(request_builder)
-	}
-
 	/// Call this when a justification has been processed by the import queue, with or without
 	/// errors.
 	pub fn justification_import_result(&mut self, hash: B::Hash, number: NumberFor<B>, success: bool) {

substrate/core/network/src/protocol/sync.rs

@@ -37,7 +37,7 @@ use log::{debug, trace, warn, info, error};
 use crate::protocol::PeerInfo as ProtocolPeerInfo;
 use libp2p::PeerId;
 use client::{BlockStatus, ClientInfo};
-use consensus::{BlockOrigin, import_queue::{IncomingBlock, BoxFinalityProofRequestBuilder}};
+use consensus::{BlockOrigin, import_queue::IncomingBlock};
 use client::error::Error as ClientError;
 use blocks::BlockCollection;
 use extra_requests::ExtraRequests;
@@ -47,7 +47,7 @@ use runtime_primitives::traits::{
 };
 use runtime_primitives::{Justification, generic::BlockId};
 use crate::message;
-use crate::config::Roles;
+use crate::config::{Roles, BoxFinalityProofRequestBuilder};
 use std::collections::HashSet;
 
 mod blocks;
@@ -196,7 +196,11 @@ pub struct Status<B: BlockT> {
 
 impl<B: BlockT> ChainSync<B> {
 	/// Create a new instance. Pass the initial known state of the chain.
-	pub(crate) fn new(role: Roles, info: &ClientInfo<B>) -> Self {
+	pub(crate) fn new(
+		role: Roles,
+		info: &ClientInfo<B>,
+		request_builder: Option<BoxFinalityProofRequestBuilder<B>>
+	) -> Self {
 		let mut required_block_attributes =
 			message::BlockAttributes::HEADER | message::BlockAttributes::JUSTIFICATION;
 
@@ -215,7 +219,7 @@ impl<B: BlockT> ChainSync<B> {
 			required_block_attributes,
 			queue_blocks: Default::default(),
 			best_importing_number: Zero::zero(),
-			request_builder: None,
+			request_builder,
 		}
 	}
 
@@ -715,10 +719,6 @@ impl<B: BlockT> ChainSync<B> {
 		self.extra_finality_proofs.try_finalize_root(request_block, finalization_result, true);
 	}
 
-	pub fn set_finality_proof_request_builder(&mut self, builder: BoxFinalityProofRequestBuilder<B>) {
-		self.request_builder = Some(builder)
-	}
-
 	/// Log that a block has been successfully imported
 	pub fn block_imported(&mut self, hash: &B::Hash, number: NumberFor<B>) {
 		trace!(target: "sync", "Block imported successfully {} ({})", number, hash);

substrate/core/network/src/service.rs

@@ -28,7 +28,7 @@
 use std::{collections::HashMap, fs, marker::PhantomData, io, path::Path};
 use std::sync::{Arc, atomic::{AtomicBool, AtomicUsize, Ordering}};
 
-use consensus::import_queue::{ImportQueue, Link, BoxFinalityProofRequestBuilder};
+use consensus::import_queue::{ImportQueue, Link};
 use futures::{prelude::*, sync::mpsc};
 use log::{warn, error, info};
 use libp2p::core::{swarm::NetworkBehaviour, transport::boxed::Boxed, muxing::StreamMuxerBox};
@@ -169,6 +169,7 @@ impl<B: BlockT + 'static, S: NetworkSpecialization<B>, H: ExHashT> NetworkWorker
 			params.specialization,
 			params.transaction_pool,
 			params.finality_proof_provider,
+			params.finality_proof_request_builder,
 			params.protocol_id,
 			peerset_config,
 		)?;
@@ -683,7 +684,4 @@ impl<'a, B: BlockT, S: NetworkSpecialization<B>, H: ExHashT> Link<B> for Network
 	fn restart(&mut self) {
 		self.protocol.user_protocol_mut().restart()
 	}
-	fn set_finality_proof_request_builder(&mut self, builder: BoxFinalityProofRequestBuilder<B>) {
-		self.protocol.user_protocol_mut().set_finality_proof_request_builder(builder)
-	}
 }

substrate/core/network/src/test/block_import.rs

@@ -75,7 +75,7 @@ fn async_import_queue_drops() {
 	// Perform this test multiple times since it exhibits non-deterministic behavior.
 	for _ in 0..100 {
 		let verifier = Arc::new(PassThroughVerifier(true));
-		let queue = BasicQueue::new(verifier, Box::new(test_client::new()), None, None, None);
+		let queue = BasicQueue::new(verifier, Box::new(test_client::new()), None, None);
 		drop(queue);
 	}
 }

substrate/core/network/src/test/mod.rs

@@ -35,14 +35,13 @@ use crate::config::Roles;
 use consensus::import_queue::BasicQueue;
 use consensus::import_queue::{
 	BoxBlockImport, BoxJustificationImport, Verifier, BoxFinalityProofImport,
-	BoxFinalityProofRequestBuilder,
 };
 use consensus::block_import::{BlockImport, ImportResult};
 use consensus::{Error as ConsensusError, well_known_cache_keys::{self, Id as CacheKeyId}};
 use consensus::{BlockOrigin, ForkChoiceStrategy, ImportBlock, JustificationImport};
 use futures::prelude::*;
 use crate::{NetworkWorker, NetworkService, config::ProtocolId};
-use crate::config::{NetworkConfiguration, TransportConfig};
+use crate::config::{NetworkConfiguration, TransportConfig, BoxFinalityProofRequestBuilder};
 use libp2p::PeerId;
 use parking_lot::Mutex;
 use primitives::{H256, Blake2Hasher};
@@ -457,7 +456,6 @@ pub trait TestNetFactory: Sized {
 			Box::new(block_import.clone()),
 			justification_import,
 			finality_proof_import,
-			finality_proof_request_builder,
 		));
 
 		let listen_addr = build_multiaddr![Memory(rand::random::<u64>())];
@@ -471,6 +469,7 @@ pub trait TestNetFactory: Sized {
 			},
 			chain: client.clone(),
 			finality_proof_provider: self.make_finality_proof_provider(PeersClient::Full(client.clone())),
+			finality_proof_request_builder,
 			on_demand: None,
 			transaction_pool: Arc::new(EmptyTransactionPool),
 			protocol_id: ProtocolId::from(&b"test-protocol-name"[..]),
@@ -514,7 +513,6 @@ pub trait TestNetFactory: Sized {
 			Box::new(block_import.clone()),
 			justification_import,
 			finality_proof_import,
-			finality_proof_request_builder,
 		));
 
 		let listen_addr = build_multiaddr![Memory(rand::random::<u64>())];
@@ -528,6 +526,7 @@ pub trait TestNetFactory: Sized {
 			},
 			chain: client.clone(),
 			finality_proof_provider: self.make_finality_proof_provider(PeersClient::Light(client.clone())),
+			finality_proof_request_builder,
 			on_demand: None,
 			transaction_pool: Arc::new(EmptyTransactionPool),
 			protocol_id: ProtocolId::from(&b"test-protocol-name"[..]),