Onchain scraper in dispute-coordinator will scrape SCRAPED_FINALIZED_BLOCKS_COUNT blocks before finality (#7013)

* Onchain scraper in `dispute-coordinator` will scrape `SCRAPED_FINALIZED_BLOCKS_COUNT` blocks before finality

The purpose is to make the availability of a `CandidateReceipt` for finalized candidates more likely.

For details see:  https://github.com/paritytech/polkadot/issues/7009

* Fix off by one error

* Replace `SCRAPED_FINALIZED_BLOCKS_COUNT` with `DISPUTE_CANDIDATE_LIFETIME_AFTER_FINALIZATION`

polkadot/node/core/dispute-coordinator/src/scraping/mod.rs

@@ -151,7 +151,10 @@ impl Inclusions {
 /// `process_active_leaves_update` any scraped votes.
 ///
 /// Scraped candidates are available `DISPUTE_CANDIDATE_LIFETIME_AFTER_FINALIZATION` more blocks
-/// after finalization as a precaution not to prune them prematurely.
+/// after finalization as a precaution not to prune them prematurely. Besides the newly scraped
+/// candidates `DISPUTE_CANDIDATE_LIFETIME_AFTER_FINALIZATION` finalized blocks are parsed as
+/// another precaution to have their `CandidateReceipts` available in case a dispute is raised on
+/// them,
 pub struct ChainScraper {
 	/// All candidates we have seen included, which not yet have been finalized.
 	included_candidates: candidates::ScrapedCandidates,
@@ -228,9 +231,10 @@ impl ChainScraper {
 			None => return Ok(ScrapedUpdates::new()),
 		};
 
-		// Fetch ancestry up to last finalized block.
+		// Fetch ancestry up to `SCRAPED_FINALIZED_BLOCKS_COUNT` blocks beyond
+		// the last finalized one
 		let ancestors = self
-			.get_unfinalized_block_ancestors(sender, activated.hash, activated.number)
+			.get_relevant_block_ancestors(sender, activated.hash, activated.number)
 			.await?;
 
 		// Ancestors block numbers are consecutive in the descending order.
@@ -330,10 +334,11 @@ impl ChainScraper {
 	}
 
 	/// Returns ancestors of `head` in the descending order, stopping
-	/// either at the block present in cache or at the last finalized block.
+	/// either at the block present in cache or at `SCRAPED_FINALIZED_BLOCKS_COUNT -1` blocks after
+	/// the last finalized one (called `target_ancestor`).
 	///
-	/// Both `head` and the latest finalized block are **not** included in the result.
-	async fn get_unfinalized_block_ancestors<Sender>(
+	/// Both `head` and the `target_ancestor` blocks are **not** included in the result.
+	async fn get_relevant_block_ancestors<Sender>(
 		&mut self,
 		sender: &mut Sender,
 		mut head: Hash,
@@ -342,7 +347,9 @@ impl ChainScraper {
 	where
 		Sender: overseer::DisputeCoordinatorSenderTrait,
 	{
-		let target_ancestor = get_finalized_block_number(sender).await?;
+		let target_ancestor = get_finalized_block_number(sender)
+			.await?
+			.saturating_sub(DISPUTE_CANDIDATE_LIFETIME_AFTER_FINALIZATION);
 
 		let mut ancestors = Vec::new();
 

polkadot/node/core/dispute-coordinator/src/scraping/tests.rs

@@ -410,7 +410,8 @@ fn scraper_requests_candidates_of_non_finalized_ancestors() {
 			&mut virtual_overseer,
 			&chain,
 			finalized_block_number,
-			BLOCKS_TO_SKIP - finalized_block_number as usize, // Expect the provider not to go past finalized block.
+			BLOCKS_TO_SKIP -
+				(finalized_block_number - DISPUTE_CANDIDATE_LIFETIME_AFTER_FINALIZATION) as usize, // Expect the provider not to go past finalized block.
 			get_backed_and_included_candidate_events,
 		);
 		join(process_active_leaves_update(ctx.sender(), &mut ordering, next_update), overseer_fut)

polkadot/node/primitives/src/lib.rs

@@ -65,7 +65,7 @@ pub const VALIDATION_CODE_BOMB_LIMIT: usize = (MAX_CODE_SIZE * 4u32) as usize;
 pub const POV_BOMB_LIMIT: usize = (MAX_POV_SIZE * 4u32) as usize;
 
 /// How many blocks after finalization an information about backed/included candidate should be
-/// kept.
+/// pre-loaded (when scraoing onchain votes) and kept locally (when pruning).
 ///
 /// We don't want to remove scraped candidates on finalization because we want to
 /// be sure that disputes will conclude on abandoned forks.
@@ -74,6 +74,12 @@ pub const POV_BOMB_LIMIT: usize = (MAX_POV_SIZE * 4u32) as usize;
 /// better one gets finalized the entries for the bad fork will be pruned and we
 /// might never participate in a dispute for it.
 ///
+/// Why pre-load finalized blocks? I dispute might be raised against finalized candidate. In most
+/// of the cases it will conclude valid (otherwise we are in big trouble) but never the less the
+/// node must participate. It's possible to see a vote for such dispute onchain before we have it
+/// imported by `dispute-distribution`. In this case we won't have `CandidateReceipt` and the import
+/// will fail unless we keep them preloaded.
+///
 /// This value should consider the timeout we allow for participation in approval-voting. In
 /// particular, the following condition should hold:
 ///