From 8b6e38ae01c9591d0692ea216377b5dd47c55ccd Mon Sep 17 00:00:00 2001 From: BradleyOlson64 Date: Wed, 8 Mar 2023 14:27:19 -0800 Subject: [PATCH 1/6] First pass adding logs --- node/core/chain-selection/src/tree.rs | 4 ++++ node/core/dispute-coordinator/src/initialized.rs | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/node/core/chain-selection/src/tree.rs b/node/core/chain-selection/src/tree.rs index 7cb2527243b1..ba6b9209d0d6 100644 --- a/node/core/chain-selection/src/tree.rs +++ b/node/core/chain-selection/src/tree.rs @@ -359,6 +359,9 @@ fn apply_ancestor_reversions( // of unviability is only heavy on the first log. for revert_number in reversions { let maybe_block_entry = load_ancestor(backend, block_hash, block_number, revert_number)?; + if let Some(block_entry) = &maybe_block_entry { + gum::trace!(target: LOG_TARGET, ?revert_number, ?block_entry.block_hash, "Block marked as reverted via scraped on-chain reversions"); + } revert_single_block_entry_if_present( backend, maybe_block_entry, @@ -380,6 +383,7 @@ pub(crate) fn apply_single_reversion( revert_hash: Hash, revert_number: BlockNumber, ) -> Result<(), Error> { + gum::trace!(target: LOG_TARGET, ?revert_number, ?revert_hash, "Block marked as reverted via ChainSelectionMessage::RevertBlocks"); let maybe_block_entry = backend.load_block_entry(&revert_hash)?; revert_single_block_entry_if_present( backend, diff --git a/node/core/dispute-coordinator/src/initialized.rs b/node/core/dispute-coordinator/src/initialized.rs index 8fd3e9373d01..4e82a2e447cf 100644 --- a/node/core/dispute-coordinator/src/initialized.rs +++ b/node/core/dispute-coordinator/src/initialized.rs @@ -1029,6 +1029,15 @@ impl Initialized { // will need to mark the candidate's relay parent as reverted. if import_result.is_freshly_concluded_against() { let blocks_including = self.scraper.get_blocks_including_candidate(&candidate_hash); + for (parent_block_number, parent_block_hash) in &blocks_including { + gum::trace!( + target: LOG_TARGET, + ?candidate_hash, + ?parent_block_number, + ?parent_block_hash, + "Dispute has just concluded against the candidate hash noted. Its parent will be marked as reverted." + ); + } if blocks_including.len() > 0 { ctx.send_message(ChainSelectionMessage::RevertBlocks(blocks_including)).await; } else { From b21c8fbc7ae80f14c8ad96ade2a8a5687c90f946 Mon Sep 17 00:00:00 2001 From: BradleyOlson64 Date: Tue, 14 Mar 2023 11:15:06 -0700 Subject: [PATCH 2/6] fmt --- node/core/chain-selection/src/tree.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/node/core/chain-selection/src/tree.rs b/node/core/chain-selection/src/tree.rs index ba6b9209d0d6..25acdc539d76 100644 --- a/node/core/chain-selection/src/tree.rs +++ b/node/core/chain-selection/src/tree.rs @@ -383,7 +383,12 @@ pub(crate) fn apply_single_reversion( revert_hash: Hash, revert_number: BlockNumber, ) -> Result<(), Error> { - gum::trace!(target: LOG_TARGET, ?revert_number, ?revert_hash, "Block marked as reverted via ChainSelectionMessage::RevertBlocks"); + gum::trace!( + target: LOG_TARGET, + ?revert_number, + ?revert_hash, + "Block marked as reverted via ChainSelectionMessage::RevertBlocks" + ); let maybe_block_entry = backend.load_block_entry(&revert_hash)?; revert_single_block_entry_if_present( backend, From 00d5666048e502463060b6071fb654b3de7781b1 Mon Sep 17 00:00:00 2001 From: BradleyOlson64 Date: Tue, 14 Mar 2023 15:26:01 -0700 Subject: [PATCH 3/6] Adjustments --- node/core/chain-selection/src/tree.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/node/core/chain-selection/src/tree.rs b/node/core/chain-selection/src/tree.rs index 25acdc539d76..b714cee29101 100644 --- a/node/core/chain-selection/src/tree.rs +++ b/node/core/chain-selection/src/tree.rs @@ -360,7 +360,12 @@ fn apply_ancestor_reversions( for revert_number in reversions { let maybe_block_entry = load_ancestor(backend, block_hash, block_number, revert_number)?; if let Some(block_entry) = &maybe_block_entry { - gum::trace!(target: LOG_TARGET, ?revert_number, ?block_entry.block_hash, "Block marked as reverted via scraped on-chain reversions"); + gum::trace!( + target: LOG_TARGET, + ?revert_number, + revert_hash = ?block_entry.block_hash, + "Block marked as reverted via scraped on-chain reversions" + ); } revert_single_block_entry_if_present( backend, From 6dcf89baf760811ee1b899f3b253ec6420c1ebd9 Mon Sep 17 00:00:00 2001 From: BradleyOlson64 Date: Tue, 28 Mar 2023 11:34:38 -0700 Subject: [PATCH 4/6] Get rid of extra timers for running participations --- node/core/dispute-coordinator/src/participation/mod.rs | 5 ++++- .../dispute-coordinator/src/participation/queues/mod.rs | 9 ++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/node/core/dispute-coordinator/src/participation/mod.rs b/node/core/dispute-coordinator/src/participation/mod.rs index e366adc5facb..11fa97a77399 100644 --- a/node/core/dispute-coordinator/src/participation/mod.rs +++ b/node/core/dispute-coordinator/src/participation/mod.rs @@ -162,8 +162,11 @@ impl Participation { priority: ParticipationPriority, req: ParticipationRequest, ) -> Result<()> { - // Participation already running - we can ignore that request: + // Participation already running - we can ignore that request, discarding its timer: if self.running_participations.contains(req.candidate_hash()) { + if let Some(timer) = req.timer() { + timer.stop_and_discard(); + } return Ok(()) } // Available capacity - participate right away (if we already have a recent block): diff --git a/node/core/dispute-coordinator/src/participation/queues/mod.rs b/node/core/dispute-coordinator/src/participation/queues/mod.rs index a5a5ab962f5a..34b7bf11a462 100644 --- a/node/core/dispute-coordinator/src/participation/queues/mod.rs +++ b/node/core/dispute-coordinator/src/participation/queues/mod.rs @@ -134,6 +134,9 @@ impl ParticipationRequest { pub fn session(&self) -> SessionIndex { self.session } + pub fn timer(&self) -> &Option (CandidateHash, CandidateReceipt) { let Self { candidate_hash, candidate_receipt, .. } = self; (candidate_hash, candidate_receipt) @@ -246,7 +249,7 @@ impl Queues { // Remove any best effort entry, using it to replace our new // request. if let Some(older_request) = self.best_effort.remove(&comparator) { - if let Some(timer) = req.request_timer { + if let Some(timer) = req.timer() { timer.stop_and_discard(); } req = older_request; @@ -254,7 +257,7 @@ impl Queues { // Keeping old request if any. match self.priority.entry(comparator) { Entry::Occupied(_) => - if let Some(timer) = req.request_timer { + if let Some(timer) = req.timer() { timer.stop_and_discard(); }, Entry::Vacant(vac) => { @@ -275,7 +278,7 @@ impl Queues { // Keeping old request if any. match self.best_effort.entry(comparator) { Entry::Occupied(_) => - if let Some(timer) = req.request_timer { + if let Some(timer) = req.timer() { timer.stop_and_discard(); }, Entry::Vacant(vac) => { From 8c5051a289f76763324dcff439e698acd3141cd5 Mon Sep 17 00:00:00 2001 From: BradleyOlson64 Date: Tue, 28 Mar 2023 11:38:43 -0700 Subject: [PATCH 5/6] fmt --- node/core/dispute-coordinator/src/participation/queues/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/core/dispute-coordinator/src/participation/queues/mod.rs b/node/core/dispute-coordinator/src/participation/queues/mod.rs index 34b7bf11a462..6f36a0dfcd50 100644 --- a/node/core/dispute-coordinator/src/participation/queues/mod.rs +++ b/node/core/dispute-coordinator/src/participation/queues/mod.rs @@ -134,7 +134,7 @@ impl ParticipationRequest { pub fn session(&self) -> SessionIndex { self.session } - pub fn timer(&self) -> &Option &Option { &self.request_timer } pub fn into_candidate_info(self) -> (CandidateHash, CandidateReceipt) { From 717f55bda33db01f2fa222174b2b5e61e0a36a1d Mon Sep 17 00:00:00 2001 From: BradleyOlson64 Date: Tue, 28 Mar 2023 13:10:09 -0700 Subject: [PATCH 6/6] Handling timer discards more elegantly --- .../src/participation/mod.rs | 6 ++---- .../src/participation/queues/mod.rs | 20 +++++++------------ 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/node/core/dispute-coordinator/src/participation/mod.rs b/node/core/dispute-coordinator/src/participation/mod.rs index 11fa97a77399..a4000e626ab8 100644 --- a/node/core/dispute-coordinator/src/participation/mod.rs +++ b/node/core/dispute-coordinator/src/participation/mod.rs @@ -160,13 +160,11 @@ impl Participation { &mut self, ctx: &mut Context, priority: ParticipationPriority, - req: ParticipationRequest, + mut req: ParticipationRequest, ) -> Result<()> { // Participation already running - we can ignore that request, discarding its timer: if self.running_participations.contains(req.candidate_hash()) { - if let Some(timer) = req.timer() { - timer.stop_and_discard(); - } + req.discard_timer(); return Ok(()) } // Available capacity - participate right away (if we already have a recent block): diff --git a/node/core/dispute-coordinator/src/participation/queues/mod.rs b/node/core/dispute-coordinator/src/participation/queues/mod.rs index 6f36a0dfcd50..01950973e054 100644 --- a/node/core/dispute-coordinator/src/participation/queues/mod.rs +++ b/node/core/dispute-coordinator/src/participation/queues/mod.rs @@ -134,8 +134,10 @@ impl ParticipationRequest { pub fn session(&self) -> SessionIndex { self.session } - pub fn timer(&self) -> &Option { - &self.request_timer + pub fn discard_timer(&mut self) { + if let Some(timer) = self.request_timer.take() { + timer.stop_and_discard(); + } } pub fn into_candidate_info(self) -> (CandidateHash, CandidateReceipt) { let Self { candidate_hash, candidate_receipt, .. } = self; @@ -249,17 +251,12 @@ impl Queues { // Remove any best effort entry, using it to replace our new // request. if let Some(older_request) = self.best_effort.remove(&comparator) { - if let Some(timer) = req.timer() { - timer.stop_and_discard(); - } + req.discard_timer(); req = older_request; } // Keeping old request if any. match self.priority.entry(comparator) { - Entry::Occupied(_) => - if let Some(timer) = req.timer() { - timer.stop_and_discard(); - }, + Entry::Occupied(_) => req.discard_timer(), Entry::Vacant(vac) => { vac.insert(req); }, @@ -277,10 +274,7 @@ impl Queues { } // Keeping old request if any. match self.best_effort.entry(comparator) { - Entry::Occupied(_) => - if let Some(timer) = req.timer() { - timer.stop_and_discard(); - }, + Entry::Occupied(_) => req.discard_timer(), Entry::Vacant(vac) => { vac.insert(req); },