From 7b6ea485f568f836e2b6e0099f5f53c04d7d1c18 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Fri, 7 Apr 2023 23:34:09 +0300 Subject: [PATCH] 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` --- .../dispute-coordinator/src/scraping/mod.rs | 21 ++++++++++++------- .../dispute-coordinator/src/scraping/tests.rs | 3 ++- node/primitives/src/lib.rs | 8 ++++++- 3 files changed, 23 insertions(+), 9 deletions(-) diff --git a/node/core/dispute-coordinator/src/scraping/mod.rs b/node/core/dispute-coordinator/src/scraping/mod.rs index 0e3c4ecc29ba..554f79bd0b84 100644 --- a/node/core/dispute-coordinator/src/scraping/mod.rs +++ b/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( + /// Both `head` and the `target_ancestor` blocks are **not** included in the result. + async fn get_relevant_block_ancestors( &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(); diff --git a/node/core/dispute-coordinator/src/scraping/tests.rs b/node/core/dispute-coordinator/src/scraping/tests.rs index b7183739d8f8..da76a93b7353 100644 --- a/node/core/dispute-coordinator/src/scraping/tests.rs +++ b/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) diff --git a/node/primitives/src/lib.rs b/node/primitives/src/lib.rs index 18b7fa18a0c8..c113769563ad 100644 --- a/node/primitives/src/lib.rs +++ b/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: ///