From 23a2b7b5b33067fa2d7cadc4e98379b2d660d9d2 Mon Sep 17 00:00:00 2001 From: eskimor <eskimor@users.noreply.github.com> Date: Fri, 1 Sep 2023 20:56:09 +0200 Subject: [PATCH] Fix wrong ref counting (#1358) * Fix wrong ref counting in case of multiple inserts at same height. * More warn --------- Co-authored-by: eskimor <eskimor@no-such-url.com> --- .../node/core/dispute-coordinator/src/initialized.rs | 4 ++-- .../dispute-coordinator/src/scraping/candidates.rs | 11 ++++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/polkadot/node/core/dispute-coordinator/src/initialized.rs b/polkadot/node/core/dispute-coordinator/src/initialized.rs index dd7aef19f6e..2d8e15064e4 100644 --- a/polkadot/node/core/dispute-coordinator/src/initialized.rs +++ b/polkadot/node/core/dispute-coordinator/src/initialized.rs @@ -1271,7 +1271,7 @@ impl Initialized { if import_result.has_fresh_byzantine_threshold_against() { let blocks_including = self.scraper.get_blocks_including_candidate(&candidate_hash); for (parent_block_number, parent_block_hash) in &blocks_including { - gum::trace!( + gum::warn!( target: LOG_TARGET, ?candidate_hash, ?parent_block_number, @@ -1282,7 +1282,7 @@ impl Initialized { if blocks_including.len() > 0 { ctx.send_message(ChainSelectionMessage::RevertBlocks(blocks_including)).await; } else { - gum::debug!( + gum::warn!( target: LOG_TARGET, ?candidate_hash, ?session, diff --git a/polkadot/node/core/dispute-coordinator/src/scraping/candidates.rs b/polkadot/node/core/dispute-coordinator/src/scraping/candidates.rs index 38956700545..b1870164cb8 100644 --- a/polkadot/node/core/dispute-coordinator/src/scraping/candidates.rs +++ b/polkadot/node/core/dispute-coordinator/src/scraping/candidates.rs @@ -135,11 +135,14 @@ impl ScrapedCandidates { } pub fn insert(&mut self, block_number: BlockNumber, candidate_hash: CandidateHash) { - self.candidates.insert(candidate_hash); - self.candidates_by_block_number + if self + .candidates_by_block_number .entry(block_number) .or_default() - .insert(candidate_hash); + .insert(candidate_hash) + { + self.candidates.insert(candidate_hash); + } } // Used only for tests to verify the pruning doesn't leak data. @@ -159,6 +162,8 @@ mod scraped_candidates_tests { let mut candidates = ScrapedCandidates::new(); let target = CandidateHash(BlakeTwo256::hash(&vec![1, 2, 3])); candidates.insert(1, target); + // Repeated inserts at same height should be fine: + candidates.insert(1, target); assert!(candidates.contains(&target)); -- GitLab