Update some comments

paritytech · eskimor · Nov 11, 2022 · Oct 5, 2022 · Oct 5, 2022 · Oct 7, 2022
commit 40b41157f3d6a2d99ac827ba33d8d847c0dbf83d
diff --git a/node/core/dispute-coordinator/src/scraping/mod.rs b/node/core/dispute-coordinator/src/scraping/mod.rs
@@ -57,24 +57,16 @@ const LRU_OBSERVED_BLOCKS_CAPACITY: NonZeroUsize = match NonZeroUsize::new(20) {
 ///
 /// Concretely:
 ///
-/// - Monitors for `CandidateIncluded` events to keep track of candidates that have been included on chains.
+/// - Monitors for `CandidateIncluded` events to keep track of candidates that have been
+///   included on chains.
 /// - Monitors for `CandidateBacked` events to keep track of all backed candidates.
 /// - Calls `FetchOnChainVotes` for each block to gather potentially missed votes from chain.
 ///
 /// With this information it provides a `CandidateComparator` and as a return value of
 /// `process_active_leaves_update` any scraped votes.
 ///
-/// If a candidate was backed and included - it will be removed once the block at its height
-/// is finalized. However a candidate can be backed and never included. To avoid
-/// leaking memory in such cases we keep track at which block height the candidate was backed.
-/// We remove backed (and
-/// not included) candidates when `CANDIDATE_MAX_LIFETIME` more blocks are
-/// finalized after the block they were initially backed in.
-/// E.g. if `CANDIDATE_MAX_LIFETIME = 2` when block 4 is finalized we will remove
-/// all candidates backed in block 2 which were not already cleaned up on finalization.
-///
-/// Please note that if a candidate is backed AND included it will be cleaned up on
-/// finalization and won't be kept for `CANDIDATE_MAX_LIFETIME` blocks.
+/// Scraped candidates are available `CANDIDATE_LIFETIME_AFTER_FINALIZATION` more blocks
+/// after finalization as a precaution not to clean them prematurely.
 pub struct ChainScraper {
 	/// All candidates we have seen included, which not yet have been finalized.
 	included_candidates: candidates::ScrapedCandidates,
@@ -95,9 +87,9 @@ impl ChainScraper {
 	/// As long as we have `MAX_FINALITY_LAG` this makes sense as a value.
 	pub(crate) const ANCESTRY_SIZE_LIMIT: u32 = MAX_FINALITY_LAG;
 
-	/// How many blocks after finalization a backed and not included candidate should
-	/// be kept.
-	pub(crate) const CANDIDATE_MAX_LIFETIME: BlockNumber = 2;
+	/// How many blocks after finalization a backed and not included candidate
+	/// should be kept.
+	pub(crate) const CANDIDATE_LIFETIME_AFTER_FINALIZATION: BlockNumber = 2;
 
 	/// Create a properly initialized `OrderingProvider`.
 	///
@@ -177,19 +169,20 @@ impl ChainScraper {
 
 	/// Prune finalized candidates.
 	///
-	/// Once a candidate lives in a relay chain block that's behind `CANDIDATE_MAX_LIFETIME` blocks
-	/// after finalization we can treat it as low priority.
+	/// Once a candidate lives in a relay chain block that's behind `CANDIDATE_LIFETIME_AFTER_FINALIZATION`
+	/// blocks after finalization we can treat it as low priority.
 	pub fn process_finalized_block(&mut self, finalized_block_number: &BlockNumber) {
-		// `CANDIDATE_MAX_LIFETIME - 1` because `finalized_block_number` counts to the candidate lifetime.
+		// `CANDIDATE_LIFETIME_AFTER_FINALIZATION - 1` because `finalized_block_number`
+		// counts to the candidate lifetime.
 		// Example:
-		// finalized_block_number = 4; CANDIDATE_MAX_LIFETIME = 2;
+		// finalized_block_number = 4; CANDIDATE_LIFETIME_AFTER_FINALIZATION = 2;
 		// key_to_clean = 4 - (2 - 1) = 3
 		// After `remove_all_at_block_height` at 3:
 		// 0, 1, 2 will be removed
 		// 3, 4 will be kept
 		// => We keep candidates in the last two finalized blocks
 		finalized_block_number
-			.checked_sub(Self::CANDIDATE_MAX_LIFETIME - 1)
+			.checked_sub(Self::CANDIDATE_LIFETIME_AFTER_FINALIZATION - 1)
 			.map(|key_to_clean| {
 				self.backed_candidates.remove_up_to_height(&key_to_clean);
 				self.included_candidates.remove_up_to_height(&key_to_clean);

diff --git a/node/core/dispute-coordinator/src/scraping/tests.rs b/node/core/dispute-coordinator/src/scraping/tests.rs
@@ -451,7 +451,7 @@ fn scraper_cleans_finalized_candidates() {
 		// Finalize the next block and verify that candidate is neither backed nor included.
 		// Backed AND Included candidates should be cleaned on finalization.
 		// `scraper_handles_backed_but_not_included_candidate` covers Backed and NOT Included candidates.
-		finalized_block_number += ChainScraper::CANDIDATE_MAX_LIFETIME + 1;
+		finalized_block_number += ChainScraper::CANDIDATE_LIFETIME_AFTER_FINALIZATION + 1;
 		process_finalized_block(&mut scraper, &finalized_block_number);
 
 		assert!(!scraper.is_candidate_backed(&candidate.hash()));
@@ -492,15 +492,16 @@ fn scraper_handles_backed_but_not_included_candidate() {
 		// Finalize blocks to enforce pruning of scraped events
 		process_finalized_block(&mut scraper, &finalized_block_number);
 
-		// `FIRST_TEST_BLOCK` is finalized, which is within `BACKED_CANDIDATE_MAX_LIFETIME` window.
+		// `FIRST_TEST_BLOCK` is finalized, which is within `BACKED_CANDIDATE_LIFETIME_AFTER_FINALIZATION` window.
 		// The candidate should still be backed.
 		let candidate = make_candidate_receipt(get_block_number_hash(BLOCK_WITH_EVENTS as u32));
 		assert!(!scraper.is_candidate_included(&candidate.hash()));
 		assert!(scraper.is_candidate_backed(&candidate.hash()));
 
-		// Bump the finalized block outside `BACKED_CANDIDATE_MAX_LIFETIME`.
+		// Bump the finalized block outside `BACKED_CANDIDATE_LIFETIME_AFTER_FINALIZATION`.
 		// The candidate should be removed.
-		finalized_block_number += FIRST_TEST_BLOCK + ChainScraper::CANDIDATE_MAX_LIFETIME + 1;
+		finalized_block_number +=
+			FIRST_TEST_BLOCK + ChainScraper::CANDIDATE_LIFETIME_AFTER_FINALIZATION + 1;
 		process_finalized_block(&mut scraper, &finalized_block_number);
 
 		assert!(!scraper.is_candidate_included(&candidate.hash()));
@@ -537,7 +538,7 @@ fn scraper_handles_the_same_candidate_incuded_in_two_different_block_heights() {
 
 		// Finalize blocks to enforce pruning of scraped events.
 		// The magic candidate was added twice, so it shouldn't be removed if we finalize two more blocks.
-		finalized_block_number += ChainScraper::CANDIDATE_MAX_LIFETIME + 1; //BLOCKS_TO_SKIP as u32 - finalized_block_number;
+		finalized_block_number += ChainScraper::CANDIDATE_LIFETIME_AFTER_FINALIZATION + 1; //BLOCKS_TO_SKIP as u32 - finalized_block_number;
 		process_finalized_block(&mut scraper, &finalized_block_number);
 
 		let magic_candidate = make_candidate_receipt(get_magic_candidate_hash());