Warp sync: Warp proof block import should not mark as leaf

.github/zombienet-tests/zombienet_cumulus_tests.yml

@@ -82,7 +82,7 @@
   needs-wasm-binary: true
 
 - job-name: "zombienet-cumulus-0015-parachain-runtime-upgrade"
-  test-filter: "functional::parachain_runtime_upgrade::parachain_runtime_upgrade_test"
+  test-filter: "zombie_ci::parachain_runtime_upgrade_slot_duration_18s::parachain_runtime_upgrade_slot_duration_18s"
   runner-type: "default"
   cumulus-image: "test-parachain"
   use-zombienet-sdk: true

prdoc/pr_11152.prdoc

@@ -0,0 +1,18 @@
+title: 'Warp sync: Warp proof block import should not mark as leaf'
+doc:
+- audience: Node Dev
+  description: |-
+    While warp syncing a node, I saw some huge stalls. What happens:
+
+    - During warp sync we store warp proofs
+    - After warp sync we start gap sync
+    - Problem: Once gap sync finishes, node starts looking for displaced leaves from all the warp proof blocks, which was over 2500 in my observed case. This led to a 30minutes stall.
+
+    In this PR I propose to import the blocks during warp sync with a Disconnected state, which does not add them as leaves. This fixes the downtime.
+crates:
+- name: sc-client-api
+  bump: major
+- name: sc-client-db
+  bump: major
+- name: sc-service
+  bump: major

substrate/client/api/src/backend.rs

@@ -175,13 +175,24 @@ pub trait BlockImportOperation<Block: BlockT> {
 	fn state(&self) -> sp_blockchain::Result<Option<&Self::State>>;
 
 	/// Append block data to the transaction.
+	///
+	/// - `header`: The block header.
+	/// - `body`: The block body (extrinsics), if available.
+	/// - `indexed_body`: Raw extrinsic data to be stored in the transaction index, keyed by their
+	///   hash.
+	/// - `justifications`: Block justifications, e.g. finality proofs.
+	/// - `state`: Whether this is a normal block, the new best block, or a newly finalized block.
+	/// - `register_as_leaf`: Whether to add the block to the leaf set. Blocks imported during warp
+	///   sync are stored in the database but should not be registered as leaves, since they are
+	///   historical blocks and not candidates for chain progression.
 	fn set_block_data(
 		&mut self,
 		header: Block::Header,
 		body: Option<Vec<Block::Extrinsic>>,
 		indexed_body: Option<Vec<Vec<u8>>>,
 		justifications: Option<Justifications>,
 		state: NewBlockState,
+		register_as_leaf: bool,
 	) -> sp_blockchain::Result<()>;
 
 	/// Inject storage data into the database.

substrate/client/api/src/in_mem.rs

@@ -48,6 +48,7 @@ use crate::{
 struct PendingBlock<B: BlockT> {
 	block: StoredBlock<B>,
 	state: NewBlockState,
+	register_as_leaf: bool,
 }
 
 #[derive(PartialEq, Eq, Clone)]
@@ -159,6 +160,7 @@ impl<Block: BlockT> Blockchain<Block> {
 		justifications: Option<Justifications>,
 		body: Option<Vec<<Block as BlockT>::Extrinsic>>,
 		new_state: NewBlockState,
+		register_as_leaf: bool,
 	) -> sp_blockchain::Result<()> {
 		let number = *header.number();
 		if new_state.is_best() {
@@ -167,7 +169,9 @@ impl<Block: BlockT> Blockchain<Block> {
 
 		{
 			let mut storage = self.storage.write();
-			storage.leaves.import(hash, number, *header.parent_hash());
+			if register_as_leaf {
+				storage.leaves.import(hash, number, *header.parent_hash());
+			}
 			storage.blocks.insert(hash, StoredBlock::new(header, body, justifications));
 
 			if let NewBlockState::Final = new_state {
@@ -515,10 +519,14 @@ impl<Block: BlockT> backend::BlockImportOperation<Block> for BlockImportOperatio
 		_indexed_body: Option<Vec<Vec<u8>>>,
 		justifications: Option<Justifications>,
 		state: NewBlockState,
+		register_as_leaf: bool,
 	) -> sp_blockchain::Result<()> {
 		assert!(self.pending_block.is_none(), "Only one block per operation is allowed");
-		self.pending_block =
-			Some(PendingBlock { block: StoredBlock::new(header, body, justifications), state });
+		self.pending_block = Some(PendingBlock {
+			block: StoredBlock::new(header, body, justifications),
+			state,
+			register_as_leaf,
+		});
 		Ok(())
 	}
 
@@ -692,7 +700,14 @@ impl<Block: BlockT> backend::Backend<Block> for Backend<Block> {
 
 			self.states.write().insert(hash, new_state);
 
-			self.blockchain.insert(hash, header, justification, body, pending_block.state)?;
+			self.blockchain.insert(
+				hash,
+				header,
+				justification,
+				body,
+				pending_block.state,
+				pending_block.register_as_leaf,
+			)?;
 		}
 
 		if !operation.aux.is_empty() {
@@ -832,16 +847,16 @@ mod tests {
 		let just2 = None;
 		let just3 = Some(Justifications::from((ID1, vec![3])));
 		blockchain
-			.insert(header(0).hash(), header(0), just0, None, NewBlockState::Final)
+			.insert(header(0).hash(), header(0), just0, None, NewBlockState::Final, true)
 			.unwrap();
 		blockchain
-			.insert(header(1).hash(), header(1), just1, None, NewBlockState::Final)
+			.insert(header(1).hash(), header(1), just1, None, NewBlockState::Final, true)
 			.unwrap();
 		blockchain
-			.insert(header(2).hash(), header(2), just2, None, NewBlockState::Best)
+			.insert(header(2).hash(), header(2), just2, None, NewBlockState::Best, true)
 			.unwrap();
 		blockchain
-			.insert(header(3).hash(), header(3), just3, None, NewBlockState::Final)
+			.insert(header(3).hash(), header(3), just3, None, NewBlockState::Final, true)
 			.unwrap();
 		blockchain
 	}

substrate/client/db/benches/state_access.rs

@@ -57,7 +57,7 @@ fn insert_blocks(db: &Backend<Block>, storage: Vec<(Vec<u8>, Vec<u8>)>) -> H256
 		)
 		.unwrap();
 
-	op.set_block_data(header.clone(), Some(vec![]), None, None, NewBlockState::Best)
+	op.set_block_data(header.clone(), Some(vec![]), None, None, NewBlockState::Best, true)
 		.unwrap();
 
 	db.commit_operation(op).unwrap();
@@ -95,7 +95,7 @@ fn insert_blocks(db: &Backend<Block>, storage: Vec<(Vec<u8>, Vec<u8>)>) -> H256
 		op.update_db_storage(tx).unwrap();
 		op.update_storage(changes.clone(), Default::default()).unwrap();
 
-		op.set_block_data(header.clone(), Some(vec![]), None, None, NewBlockState::Best)
+		op.set_block_data(header.clone(), Some(vec![]), None, None, NewBlockState::Best, true)
 			.unwrap();
 
 		db.commit_operation(op).unwrap();