Don't sync on top of pruned blocks (#2079)

* Don't sync to pruned blocks

* Typo

Co-Authored-By: arkpar <arkady.paronyan@gmail.com>

substrate/core/client/db/src/lib.rs

@@ -1169,6 +1169,10 @@ impl<Block> client::backend::Backend<Block, Blake2Hasher> for Backend<Block> whe
 		}
 	}
 
+	fn have_state_at(&self, hash: &Block::Hash, number: NumberFor<Block>) -> bool {
+		!self.storage.state_db.is_pruned(hash, number.as_())
+	}
+
 	fn destroy_state(&self, mut state: Self::State) -> Result<(), client::error::Error> {
 		if let Some(hash) = state.parent_hash.clone() {
 			let is_best = || self.blockchain.meta.read().best_hash == hash;

substrate/core/client/src/backend.rs

@@ -142,6 +142,10 @@ pub trait Backend<Block, H>: AuxStore + Send + Sync where
 	fn blockchain(&self) -> &Self::Blockchain;
 	/// Returns reference to changes trie storage.
 	fn changes_trie_storage(&self) -> Option<&Self::ChangesTrieStorage>;
+	/// Returns true if state for given block is available.
+	fn have_state_at(&self, hash: &Block::Hash, _number: NumberFor<Block>) -> bool {
+		self.state_at(BlockId::Hash(hash.clone())).is_ok()
+	}
 	/// Returns state backend with post-state of given block.
 	fn state_at(&self, block: BlockId<Block>) -> error::Result<Self::State>;
 	/// Destroy state and save any useful data, such as cache.

substrate/core/client/src/client.rs

@@ -167,8 +167,10 @@ pub struct ClientInfo<Block: BlockT> {
 pub enum BlockStatus {
 	/// Added to the import queue.
 	Queued,
-	/// Already in the blockchain.
+	/// Already in the blockchain and the state is available.
-	InChain,
+	InChainWithState,
+	/// In the blockchain, but the state is not available.
+	InChainPruned,
 	/// Block or parent is known to be bad.
 	KnownBad,
 	/// Not in the queue or the blockchain.
@@ -1073,9 +1075,19 @@ impl<B, E, Block, RA> Client<B, E, Block, RA> where
 				return Ok(BlockStatus::Queued);
 			}
 		}
-		match self.backend.blockchain().header(*id).map_err(|e| error::Error::from_blockchain(Box::new(e)))?.is_some() {
+		let hash_and_number = match id.clone() {
-			true => Ok(BlockStatus::InChain),
+			BlockId::Hash(hash) => self.backend.blockchain().number(hash)?.map(|n| (hash, n)),
-			false => Ok(BlockStatus::Unknown),
+			BlockId::Number(n) => self.backend.blockchain().hash(n)?.map(|hash| (hash, n)),
+		};
+		match hash_and_number {
+			Some((hash, number)) => {
+				if self.backend().have_state_at(&hash, number) {
+					Ok(BlockStatus::InChainWithState)
+				} else {
+					Ok(BlockStatus::InChainPruned)
+				}
+			}
+			None => Ok(BlockStatus::Unknown),
 		}
 	}
 
@@ -1386,18 +1398,20 @@ impl<B, E, Block, RA> consensus::BlockImport<Block> for Client<B, E, Block, RA>
 		hash: Block::Hash,
 		parent_hash: Block::Hash,
 	) -> Result<ImportResult, Self::Error> {
-		match self.backend.blockchain().status(BlockId::Hash(parent_hash))
+		match self.block_status(&BlockId::Hash(parent_hash))
 			.map_err(|e| ConsensusError::from(ConsensusErrorKind::ClientImport(e.to_string())))?
 		{
-			blockchain::BlockStatus::InChain => {},
+			BlockStatus::InChainWithState | BlockStatus::Queued => {},
-			blockchain::BlockStatus::Unknown => return Ok(ImportResult::UnknownParent),
+			BlockStatus::Unknown | BlockStatus::InChainPruned => return Ok(ImportResult::UnknownParent),
+			BlockStatus::KnownBad => return Ok(ImportResult::KnownBad),
 		}
 
-		match self.backend.blockchain().status(BlockId::Hash(hash))
+		match self.block_status(&BlockId::Hash(hash))
 			.map_err(|e| ConsensusError::from(ConsensusErrorKind::ClientImport(e.to_string())))?
 		{
-			blockchain::BlockStatus::InChain => return Ok(ImportResult::AlreadyInChain),
+			BlockStatus::InChainWithState | BlockStatus::Queued => return Ok(ImportResult::AlreadyInChain),
-			blockchain::BlockStatus::Unknown => {},
+			BlockStatus::Unknown | BlockStatus::InChainPruned => {},
+			BlockStatus::KnownBad => return Ok(ImportResult::KnownBad),
 		}
 
 		Ok(ImportResult::imported())

substrate/core/client/src/light/blockchain.rs

@@ -109,7 +109,7 @@ impl<S, F, Block> BlockchainHeaderBackend<Block> for Blockchain<S, F> where Bloc
 				};
 
 				// if the header is from future or genesis (we never prune genesis) => return
-				if number.is_zero() || self.storage.status(BlockId::Number(number))? != BlockStatus::InChain {
+				if number.is_zero() || self.storage.status(BlockId::Number(number))? == BlockStatus::Unknown {
 					return Ok(None);
 				}
 

substrate/core/network/src/sync.rs

@@ -46,7 +46,6 @@ const JUSTIFICATION_RETRY_WAIT: Duration = Duration::from_secs(10);
 // Number of recently announced blocks to track for each peer.
 const ANNOUNCE_HISTORY_SIZE: usize = 64;
 // Max number of blocks to download for unknown forks.
-// TODO: this should take finality into account. See https://github.com/paritytech/substrate/issues/1606
 const MAX_UNKNOWN_FORK_DOWNLOAD_LEN: u32 = 32;
 
 #[derive(Debug)]
@@ -500,7 +499,7 @@ impl<B: BlockT> ChainSync<B> {
 						self.download_new(protocol, who)
 					}
 				},
-				(Ok(BlockStatus::Queued), _) | (Ok(BlockStatus::InChain), _) => {
+				(Ok(BlockStatus::Queued), _) | (Ok(BlockStatus::InChainWithState), _) | (Ok(BlockStatus::InChainPruned), _) => {
 					debug!(target:"sync", "New peer with known best hash {} ({}).", info.best_hash, info.best_number);
 					self.peers.insert(who, PeerSync {
 						common_number: info.best_number,
@@ -834,7 +833,11 @@ impl<B: BlockT> ChainSync<B> {
 			trace!(target: "sync", "Ignored invalid block announcement from {}: {}", who, hash);
 			return;
 		}
-		let known_parent = self.is_known(protocol, &header.parent_hash());
+		let parent_status = block_status(&*protocol.client(), &self.queue_blocks, header.parent_hash().clone()).ok()
+			.unwrap_or(BlockStatus::Unknown);
+		let known_parent = parent_status != BlockStatus::Unknown;
+		let ancient_parent = parent_status == BlockStatus::InChainPruned;
+
 		let known = self.is_known(protocol, &hash);
 		if let Some(ref mut peer) = self.peers.get_mut(&who) {
 			while peer.recently_announced.len() >= ANNOUNCE_HISTORY_SIZE {
@@ -862,15 +865,28 @@ impl<B: BlockT> ChainSync<B> {
 			let stale = number <= self.best_queued_number;
 			if stale {
 				if !(known_parent || self.is_already_downloading(header.parent_hash())) {
-					trace!(target: "sync", "Considering new unknown stale block announced from {}: {} {:?}", who, hash, header);
+					if protocol.client().block_status(&BlockId::Number(*header.number()))
-					self.download_unknown_stale(protocol, who, &hash);
+						.unwrap_or(BlockStatus::Unknown) == BlockStatus::InChainPruned
+					{
+						trace!(target: "sync", "Ignored unknown ancient block announced from {}: {} {:?}", who, hash, header);
+					} else {
+						trace!(target: "sync", "Considering new unknown stale block announced from {}: {} {:?}", who, hash, header);
+						self.download_unknown_stale(protocol, who, &hash);
+					}
 				} else {
-					trace!(target: "sync", "Considering new stale block announced from {}: {} {:?}", who, hash, header);
+					if ancient_parent {
-					self.download_stale(protocol, who, &hash);
+						trace!(target: "sync", "Ignored ancient stale block announced from {}: {} {:?}", who, hash, header);
+					} else {
+						self.download_stale(protocol, who, &hash);
+					}
 				}
 			} else {
-				trace!(target: "sync", "Considering new block announced from {}: {} {:?}", who, hash, header);
+				if ancient_parent {
-				self.download_new(protocol, who);
+					trace!(target: "sync", "Ignored ancient block announced from {}: {} {:?}", who, hash, header);
+				} else {
+					trace!(target: "sync", "Considering new block announced from {}: {} {:?}", who, hash, header);
+					self.download_new(protocol, who);
+				}
 			}
 		} else {
 			trace!(target: "sync", "Known block announce from {}: {}", who, hash);