Introduce mockable ChainSync object for testing (#12480)

* Introduce mockable `ChainSync` object for testing

`mockall` allows to mock `ChainSync` and to verify that the calls made
to `ChaiSync` are firstly executed at all, that they're executed in
correct order and with correct parameters.

This allows to verify, e.g., that delegating calls directly to
`ChainSync` from `NetworkService` still calls the correct functions with
correct arguments even if `Protocol` middleman is removed.

* Add Cargo.lock

* Fix tests

* Update client/network/Cargo.toml

Co-authored-by: Bastian Köcher <git@kchr.de>

* Update Cargo.lock

* Fix clippy and documentation

Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: parity-processbot <>

substrate/client/network/sync/Cargo.toml

@@ -25,6 +25,7 @@ futures = "0.3.21"
 libp2p = "0.46.1"
 log = "0.4.17"
 lru = "0.7.5"
+mockall = "0.11.2"
 prost = "0.11"
 smallvec = "1.8.0"
 thiserror = "1.0"

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

@@ -30,6 +30,7 @@
 
 pub mod block_request_handler;
 pub mod blocks;
+pub mod mock;
 mod schema;
 pub mod state;
 pub mod state_request_handler;
@@ -643,9 +644,9 @@ where
 			.extend(peers);
 	}
 
-	fn justification_requests(
-		&mut self,
-	) -> Box<dyn Iterator<Item = (PeerId, BlockRequest<B>)> + '_> {
+	fn justification_requests<'a>(
+		&'a mut self,
+	) -> Box<dyn Iterator<Item = (PeerId, BlockRequest<B>)> + 'a> {
 		let peers = &mut self.peers;
 		let mut matcher = self.extra_justifications.matcher();
 		Box::new(std::iter::from_fn(move || {
@@ -670,7 +671,9 @@ where
 		}))
 	}
 
-	fn block_requests(&mut self) -> Box<dyn Iterator<Item = (&PeerId, BlockRequest<B>)> + '_> {
+	fn block_requests<'a>(
+		&'a mut self,
+	) -> Box<dyn Iterator<Item = (PeerId, BlockRequest<B>)> + 'a> {
 		if self.mode == SyncMode::Warp {
 			return Box::new(std::iter::once(self.warp_target_block_request()).flatten())
 		}
@@ -695,8 +698,8 @@ where
 		let allowed_requests = self.allowed_requests.take();
 		let max_parallel = if major_sync { 1 } else { self.max_parallel_downloads };
 		let gap_sync = &mut self.gap_sync;
-		let iter = self.peers.iter_mut().filter_map(move |(id, peer)| {
-			if !peer.state.is_available() || !allowed_requests.contains(id) {
+		let iter = self.peers.iter_mut().filter_map(move |(&id, peer)| {
+			if !peer.state.is_available() || !allowed_requests.contains(&id) {
 				return None
 			}
 
@@ -725,7 +728,7 @@ where
 				};
 				Some((id, ancestry_request::<B>(current)))
 			} else if let Some((range, req)) = peer_block_request(
-				id,
+				&id,
 				peer,
 				blocks,
 				attrs,
@@ -744,7 +747,7 @@ where
 				);
 				Some((id, req))
 			} else if let Some((hash, req)) =
-				fork_sync_request(id, fork_targets, best_queued, last_finalized, attrs, |hash| {
+				fork_sync_request(&id, fork_targets, best_queued, last_finalized, attrs, |hash| {
 					if queue.contains(hash) {
 						BlockStatus::Queued
 					} else {
@@ -756,7 +759,7 @@ where
 				Some((id, req))
 			} else if let Some((range, req)) = gap_sync.as_mut().and_then(|sync| {
 				peer_gap_block_request(
-					id,
+					&id,
 					peer,
 					&mut sync.blocks,
 					attrs,
@@ -2216,7 +2219,7 @@ where
 	}
 
 	/// Generate block request for downloading of the target block body during warp sync.
-	fn warp_target_block_request(&mut self) -> Option<(&PeerId, BlockRequest<B>)> {
+	fn warp_target_block_request(&mut self) -> Option<(PeerId, BlockRequest<B>)> {
 		if let Some(sync) = &self.warp_sync {
 			if self.allowed_requests.is_empty() ||
 				sync.is_complete() ||
@@ -2234,7 +2237,7 @@ where
 						trace!(target: "sync", "New warp target block request for {}", id);
 						peer.state = PeerSyncState::DownloadingWarpTargetBlock;
 						self.allowed_requests.clear();
-						return Some((id, request))
+						return Some((*id, request))
 					}
 				}
 			}
@@ -2482,7 +2485,7 @@ fn fork_sync_request<B: BlockT>(
 		true
 	});
 	for (hash, r) in targets {
-		if !r.peers.contains(id) {
+		if !r.peers.contains(&id) {
 			continue
 		}
 		// Download the fork only if it is behind or not too far ahead our tip of the chain
@@ -2740,7 +2743,7 @@ mod test {
 
 		// we wil send block requests to these peers
 		// for these blocks we don't know about
-		assert!(sync.block_requests().all(|(p, _)| { *p == peer_id1 || *p == peer_id2 }));
+		assert!(sync.block_requests().all(|(p, _)| { p == peer_id1 || p == peer_id2 }));
 
 		// add a new peer at a known block
 		sync.new_peer(peer_id3, b1_hash, b1_number).unwrap();
@@ -2835,7 +2838,7 @@ mod test {
 		log::trace!(target: "sync", "Requests: {:?}", requests);
 
 		assert_eq!(1, requests.len());
-		assert_eq!(peer, requests[0].0);
+		assert_eq!(*peer, requests[0].0);
 
 		let request = requests[0].1.clone();
 
@@ -3065,9 +3068,9 @@ mod test {
 		send_block_announce(best_block.header().clone(), &peer_id2, &mut sync);
 
 		let (peer1_req, peer2_req) = sync.block_requests().fold((None, None), |res, req| {
-			if req.0 == &peer_id1 {
+			if req.0 == peer_id1 {
 				(Some(req.1), res.1)
-			} else if req.0 == &peer_id2 {
+			} else if req.0 == peer_id2 {
 				(res.0, Some(req.1))
 			} else {
 				panic!("Unexpected req: {:?}", req)

substrate/client/network/sync/src/mock.rs

@@ -0,0 +1,118 @@
+// This file is part of Substrate.
+
+// Copyright (C) 2022 Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: GPL-3.0-or-later WITH Classpath-exception-2.0
+
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with this program. If not, see <https://www.gnu.org/licenses/>.
+
+//! Contains a mock implementation of `ChainSync` that can be used
+//! for testing calls made to `ChainSync`.
+
+use futures::task::Poll;
+use libp2p::PeerId;
+use sc_consensus::{BlockImportError, BlockImportStatus};
+use sc_network_common::sync::{
+	message::{BlockAnnounce, BlockData, BlockRequest, BlockResponse},
+	warp::{EncodedProof, WarpProofRequest},
+	BadPeer, ChainSync as ChainSyncT, Metrics, OnBlockData, OnBlockJustification, OnStateData,
+	OpaqueBlockRequest, OpaqueBlockResponse, OpaqueStateRequest, OpaqueStateResponse, PeerInfo,
+	PollBlockAnnounceValidation, SyncStatus,
+};
+use sp_runtime::traits::{Block as BlockT, NumberFor};
+
+mockall::mock! {
+	pub ChainSync<Block: BlockT> {}
+
+	impl<Block: BlockT> ChainSyncT<Block> for ChainSync<Block> {
+		fn peer_info(&self, who: &PeerId) -> Option<PeerInfo<Block>>;
+		fn status(&self) -> SyncStatus<Block>;
+		fn num_sync_requests(&self) -> usize;
+		fn num_downloaded_blocks(&self) -> usize;
+		fn num_peers(&self) -> usize;
+		fn new_peer(
+			&mut self,
+			who: PeerId,
+			best_hash: Block::Hash,
+			best_number: NumberFor<Block>,
+		) -> Result<Option<BlockRequest<Block>>, BadPeer>;
+		fn update_chain_info(&mut self, best_hash: &Block::Hash, best_number: NumberFor<Block>);
+		fn request_justification(&mut self, hash: &Block::Hash, number: NumberFor<Block>);
+		fn clear_justification_requests(&mut self);
+		fn set_sync_fork_request(
+			&mut self,
+			peers: Vec<PeerId>,
+			hash: &Block::Hash,
+			number: NumberFor<Block>,
+		);
+		fn justification_requests<'a>(
+			&'a mut self,
+		) -> Box<dyn Iterator<Item = (PeerId, BlockRequest<Block>)> + 'a>;
+		fn block_requests<'a>(&'a mut self) -> Box<dyn Iterator<Item = (PeerId, BlockRequest<Block>)> + 'a>;
+		fn state_request(&mut self) -> Option<(PeerId, OpaqueStateRequest)>;
+		fn warp_sync_request(&mut self) -> Option<(PeerId, WarpProofRequest<Block>)>;
+		fn on_block_data(
+			&mut self,
+			who: &PeerId,
+			request: Option<BlockRequest<Block>>,
+			response: BlockResponse<Block>,
+		) -> Result<OnBlockData<Block>, BadPeer>;
+		fn on_state_data(
+			&mut self,
+			who: &PeerId,
+			response: OpaqueStateResponse,
+		) -> Result<OnStateData<Block>, BadPeer>;
+		fn on_warp_sync_data(&mut self, who: &PeerId, response: EncodedProof) -> Result<(), BadPeer>;
+		fn on_block_justification(
+			&mut self,
+			who: PeerId,
+			response: BlockResponse<Block>,
+		) -> Result<OnBlockJustification<Block>, BadPeer>;
+		fn on_blocks_processed(
+			&mut self,
+			imported: usize,
+			count: usize,
+			results: Vec<(Result<BlockImportStatus<NumberFor<Block>>, BlockImportError>, Block::Hash)>,
+		) -> Box<dyn Iterator<Item = Result<(PeerId, BlockRequest<Block>), BadPeer>>>;
+		fn on_justification_import(
+			&mut self,
+			hash: Block::Hash,
+			number: NumberFor<Block>,
+			success: bool,
+		);
+		fn on_block_finalized(&mut self, hash: &Block::Hash, number: NumberFor<Block>);
+		fn push_block_announce_validation(
+			&mut self,
+			who: PeerId,
+			hash: Block::Hash,
+			announce: BlockAnnounce<Block::Header>,
+			is_best: bool,
+		);
+		fn poll_block_announce_validation<'a>(
+			&mut self,
+			cx: &mut std::task::Context<'a>,
+		) -> Poll<PollBlockAnnounceValidation<Block::Header>>;
+		fn peer_disconnected(&mut self, who: &PeerId) -> Option<OnBlockData<Block>>;
+		fn metrics(&self) -> Metrics;
+		fn create_opaque_block_request(&self, request: &BlockRequest<Block>) -> OpaqueBlockRequest;
+		fn encode_block_request(&self, request: &OpaqueBlockRequest) -> Result<Vec<u8>, String>;
+		fn decode_block_response(&self, response: &[u8]) -> Result<OpaqueBlockResponse, String>;
+		fn block_response_into_blocks(
+			&self,
+			request: &BlockRequest<Block>,
+			response: OpaqueBlockResponse,
+		) -> Result<Vec<BlockData<Block>>, String>;
+		fn encode_state_request(&self, request: &OpaqueStateRequest) -> Result<Vec<u8>, String>;
+		fn decode_state_response(&self, response: &[u8]) -> Result<OpaqueStateResponse, String>;
+	}
+}