Allow import withouth state verification (#4031)

* Allow import without state verification

* Explicit None

Co-Authored-By: Robert Habermeier <rphmeier@gmail.com>

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

@@ -480,13 +480,7 @@ impl<B: BlockT> ChainSync<B> {
 			return;
 		}
 
-		let block_status = self.client.block_status(&BlockId::Number(number - One::one()))
-			.unwrap_or(BlockStatus::Unknown);
-		if block_status == BlockStatus::InChainPruned {
-			trace!(target: "sync", "Refusing to sync ancient block {:?}", hash);
-			return;
-		}
-
+		trace!(target: "sync", "Downloading requested old fork {:?}", hash);
 		self.is_idle = false;
 		for peer_id in &peers {
 			if let Some(peer) = self.peers.get_mut(peer_id) {
@@ -571,7 +565,7 @@ impl<B: BlockT> ChainSync<B> {
 		let major_sync = self.status().state == SyncState::Downloading;
 		let blocks = &mut self.blocks;
 		let attrs = &self.required_block_attributes;
-		let fork_targets = &self.fork_targets;
+		let fork_targets = &mut self.fork_targets;
 		let mut have_requests = false;
 		let last_finalized = self.client.info().chain.finalized_number;
 		let best_queued = self.best_queued_number;
@@ -646,6 +640,7 @@ impl<B: BlockT> ChainSync<B> {
 									body: block_data.block.body,
 									justification: block_data.block.justification,
 									origin: block_data.origin,
+									allow_missing_state: false,
 								}
 							}).collect()
 					}
@@ -658,14 +653,15 @@ impl<B: BlockT> ChainSync<B> {
 								body: b.body,
 								justification: b.justification,
 								origin: Some(who.clone()),
+								allow_missing_state: true,
 							}
 						}).collect()
 					}
 					PeerSyncState::AncestorSearch(num, state) => {
-						let block_hash_match = match (blocks.get(0), self.client.block_hash(*num)) {
+						let matching_hash = match (blocks.get(0), self.client.block_hash(*num)) {
 							(Some(block), Ok(maybe_our_block_hash)) => {
 								trace!(target: "sync", "Got ancestry block #{} ({}) from peer {}", num, block.hash, who);
-								maybe_our_block_hash.map_or(false, |x| x == block.hash)
+								maybe_our_block_hash.filter(|x| x == &block.hash)
 							},
 							(None, _) => {
 								debug!(target: "sync", "Invalid response when searching for ancestor from {}", who);
@@ -676,27 +672,34 @@ impl<B: BlockT> ChainSync<B> {
 								return Err(BadPeer(who, ANCESTRY_BLOCK_ERROR_REPUTATION_CHANGE))
 							}
 						};
-						if block_hash_match && peer.common_number < *num {
+						if matching_hash.is_some() && peer.common_number < *num {
 							peer.common_number = *num;
 						}
-						if !block_hash_match && num.is_zero() {
+						if matching_hash.is_none() && num.is_zero() {
 							trace!(target:"sync", "Ancestry search: genesis mismatch for peer {}", who);
 							return Err(BadPeer(who, GENESIS_MISMATCH_REPUTATION_CHANGE))
 						}
-						if let Some((next_state, next_num)) = handle_ancestor_search_state(state, *num, block_hash_match) {
+						if let Some((next_state, next_num)) = handle_ancestor_search_state(state, *num, matching_hash.is_some()) {
 							peer.state = PeerSyncState::AncestorSearch(next_num, next_state);
 							return Ok(OnBlockData::Request(who, ancestry_request::<B>(next_num)))
 						} else {
 							// Ancestry search is complete. Check if peer is on a stale fork unknown to us and
 							// add it to sync targets if necessary.
-							trace!(target: "sync", "Ancestry search complete. Ours={} ({}), Theirs={} ({}), Common={}",
+							trace!(target: "sync", "Ancestry search complete. Ours={} ({}), Theirs={} ({}), Common={:?} ({})",
 								self.best_queued_hash,
 								self.best_queued_number,
 								peer.best_hash,
 								peer.best_number,
-								peer.common_number
+								matching_hash,
+								peer.common_number,
 							);
-							if peer.common_number < peer.best_number && peer.best_number < self.best_queued_number {
+							let client = &self.client;
+							if peer.common_number < peer.best_number
+								&& peer.best_number < self.best_queued_number
+								&& matching_hash.and_then(
+									|h| client.block_status(&BlockId::Hash(h)).ok()
+								).unwrap_or(BlockStatus::Unknown) != BlockStatus::InChainPruned
+							{
 								trace!(target: "sync", "Added fork target {} for {}" , peer.best_hash, who);
 								self.fork_targets
 									.entry(peer.best_hash.clone())
@@ -1250,25 +1253,31 @@ fn peer_block_request<B: BlockT>(
 /// Get pending fork sync targets for a peer.
 fn fork_sync_request<B: BlockT>(
 	id: &PeerId,
-	targets: &HashMap<B::Hash, ForkTarget<B>>,
+	targets: &mut HashMap<B::Hash, ForkTarget<B>>,
 	best_num: NumberFor<B>,
 	finalized: NumberFor<B>,
 	attributes: &message::BlockAttributes,
 	check_block: impl Fn(&B::Hash) -> BlockStatus,
 ) -> Option<(B::Hash, BlockRequest<B>)>
 {
+	targets.retain(|hash, r| if r.number > finalized {
+		true
+	} else {
+		trace!(target: "sync", "Removed expired fork sync request {:?} (#{})", hash, r.number);
+		false
+	});
 	for (hash, r) in targets {
 		if !r.peers.contains(id) {
 			continue
 		}
 		if r.number <= best_num {
-			trace!(target: "sync", "Downloading requested fork {:?} from {}", hash, id);
 			let parent_status = r.parent_hash.as_ref().map_or(BlockStatus::Unknown, check_block);
 			let mut count = (r.number - finalized).saturated_into::<u32>(); // up to the last finalized block
 			if parent_status != BlockStatus::Unknown {
 				// request only single block
 				count = 1;
 			}
+			trace!(target: "sync", "Downloading requested fork {:?} from {}, {} blocks", hash, id, count);
 			return Some((hash.clone(), message::generic::BlockRequest {
 				id: 0,
 				fields: attributes.clone(),

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

@@ -37,9 +37,10 @@ fn prepare_good_block() -> (TestClient, Hash, u64, PeerId, IncomingBlock<Block>)
 	(client, hash, number, peer_id.clone(), IncomingBlock {
 		hash,
 		header,
-		body: None,
+		body: Some(Vec::new()),
 		justification,
-		origin: Some(peer_id.clone())
+		origin: Some(peer_id.clone()),
+		allow_missing_state: false,
 	})
 }
 
@@ -53,7 +54,7 @@ fn import_single_good_block_works() {
 	match import_single_block(&mut test_client::new(), BlockOrigin::File, block, &mut PassThroughVerifier(true)) {
 		Ok(BlockImportResult::ImportedUnknown(ref num, ref aux, ref org))
 			if *num == number && *aux == expected_aux && *org == Some(peer_id) => {}
-		_ => panic!()
+		r @ _ => panic!("{:?}", r)
 	}
 }
 

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

@@ -96,6 +96,7 @@ impl<B: BlockT> Verifier<B> for PassThroughVerifier {
 			post_digests: vec![],
 			auxiliary: Vec::new(),
 			fork_choice: ForkChoiceStrategy::LongestChain,
+			allow_missing_state: false,
 		}, maybe_keys))
 	}
 }
@@ -374,16 +375,10 @@ impl<D, S: NetworkSpecialization<Block>> Peer<D, S> {
 		}
 	}
 
-	/// Count the current number of known blocks. Note that:
-	///  1. this might be expensive as it creates an in-memory-copy of the chain
-	///     to count the blocks, thus if you have a different way of testing this
-	///     (e.g. `info.best_hash`) - use that.
-	///  2. This is not always increasing nor accurate, as the
-	///     orphaned and proven-to-never-finalized blocks may be pruned at any time.
-	///     Therefore, this number can drop again.
-	pub fn blocks_count(&self) -> usize {
+	/// Count the total number of imported blocks.
+	pub fn blocks_count(&self) -> u64 {
 		self.backend.as_ref().map(
-			|backend| backend.as_in_memory().blockchain().blocks_count()
+			|backend| backend.blocks_count()
 		).unwrap_or(0)
 	}
 }
@@ -526,9 +521,16 @@ pub trait TestNetFactory: Sized {
 		net
 	}
 
-	/// Add a full peer.
 	fn add_full_peer(&mut self, config: &ProtocolConfig) {
-		let test_client_builder = TestClientBuilder::with_default_backend();
+		self.add_full_peer_with_states(config, None)
+	}
+
+	/// Add a full peer.
+	fn add_full_peer_with_states(&mut self, config: &ProtocolConfig, keep_blocks: Option<u32>) {
+		let test_client_builder = match keep_blocks {
+			Some(keep_blocks) => TestClientBuilder::with_pruning_window(keep_blocks),
+			None => TestClientBuilder::with_default_backend(),
+		};
 		let backend = test_client_builder.backend();
 		let (c, longest_chain) = test_client_builder.build_with_longest_chain();
 		let client = Arc::new(c);
@@ -686,7 +688,7 @@ pub trait TestNetFactory: Sized {
 			if peer.is_major_syncing() || peer.network.num_queued_blocks() != 0 {
 				return Async::NotReady
 			}
-			match (highest, peer.client.info().chain.best_number) {
+			match (highest, peer.client.info().chain.best_hash) {
 				(None, b) => highest = Some(b),
 				(Some(ref a), ref b) if a == b => {},
 				(Some(_), _) => return Async::NotReady,

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

@@ -236,7 +236,14 @@ fn sync_no_common_longer_chain_fails() {
 	let mut net = TestNet::new(3);
 	net.peer(0).push_blocks(20, true);
 	net.peer(1).push_blocks(20, false);
-	net.block_until_sync(&mut runtime);
+	runtime.block_on(futures::future::poll_fn::<(), (), _>(|| -> Result<_, ()> {
+		net.poll();
+		if net.peer(0).is_major_syncing() {
+			Ok(Async::NotReady)
+		} else {
+			Ok(Async::Ready(()))
+		}
+	})).unwrap();
 	let peer1 = &net.peers()[1];
 	assert!(!net.peers()[0].blockchain_canon_equals(peer1));
 }
@@ -592,3 +599,37 @@ fn can_sync_explicit_forks() {
 		Ok(Async::Ready(()))
 	})).unwrap();
 }
+
+#[test]
+fn syncs_header_only_forks() {
+	let _ = ::env_logger::try_init();
+	let mut runtime = current_thread::Runtime::new().unwrap();
+	let mut net = TestNet::new(0);
+	let config = ProtocolConfig::default();
+	net.add_full_peer_with_states(&config, None);
+	net.add_full_peer_with_states(&config, Some(3));
+	net.peer(0).push_blocks(2, false);
+	net.peer(1).push_blocks(2, false);
+
+	net.peer(0).push_blocks(2, true);
+	let small_hash = net.peer(0).client().info().chain.best_hash;
+	let small_number = net.peer(0).client().info().chain.best_number;
+	net.peer(1).push_blocks(4, false);
+
+	net.block_until_sync(&mut runtime);
+	// Peer 1 won't sync the small fork because common block state is missing
+	assert_eq!(9, net.peer(0).blocks_count());
+	assert_eq!(7, net.peer(1).blocks_count());
+
+	// Request explicit header-only sync request for the ancient fork.
+	let first_peer_id = net.peer(0).id();
+	net.peer(1).set_sync_fork_request(vec![first_peer_id], small_hash, small_number);
+	runtime.block_on(futures::future::poll_fn::<(), (), _>(|| -> Result<_, ()> {
+		net.poll();
+		if net.peer(1).client().header(&BlockId::Hash(small_hash)).unwrap().is_none() {
+			return Ok(Async::NotReady)
+		}
+		Ok(Async::Ready(()))
+	})).unwrap();
+}
+