From b68116c9be7178a040ce23e0e3ab6f244156b3ca Mon Sep 17 00:00:00 2001
From: Alexey Shekhirin <a.shekhirin@gmail.com>
Date: Sun, 9 Jul 2023 18:16:53 +0100
Subject: [PATCH] fix(tree): update metrics only on canonical/side chain
 changes (#3671)

---
 crates/blockchain-tree/src/blockchain_tree.rs |  8 ++++++--
 crates/blockchain-tree/src/shareable.rs       | 16 ++++++++--------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/crates/blockchain-tree/src/blockchain_tree.rs b/crates/blockchain-tree/src/blockchain_tree.rs
index 0fad707991..92c821064c 100644
--- a/crates/blockchain-tree/src/blockchain_tree.rs
+++ b/crates/blockchain-tree/src/blockchain_tree.rs
@@ -1084,8 +1084,12 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTree<DB, C, EF>
         }
     }
 
-    /// Update blockchain tree and sync metrics
-    pub(crate) fn update_metrics(&mut self) {
+    /// Update blockchain tree chains (canonical and sidechains) and sync metrics.
+    ///
+    /// NOTE: this method should not be called during the pipeline sync, because otherwise the sync
+    /// checkpoint metric will get overwritten. Buffered blocks metrics are updated in
+    /// [BlockBuffer] during the pipeline sync.
+    pub(crate) fn update_chains_metrics(&mut self) {
         let height = self.canonical_chain().tip().number;
 
         self.metrics.sidechains.set(self.chains.len() as f64);
diff --git a/crates/blockchain-tree/src/shareable.rs b/crates/blockchain-tree/src/shareable.rs
index 663c2e03ba..a63b18dd26 100644
--- a/crates/blockchain-tree/src/shareable.rs
+++ b/crates/blockchain-tree/src/shareable.rs
@@ -43,9 +43,9 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
 {
     fn buffer_block(&self, block: SealedBlockWithSenders) -> Result<(), InsertBlockError> {
         let mut tree = self.tree.write();
-        let res = tree.buffer_block(block);
-        tree.update_metrics();
-        res
+        // Blockchain tree metrics shouldn't be updated here, see
+        // `BlockchainTree::update_chains_metrics` documentation.
+        tree.buffer_block(block)
     }
 
     fn insert_block(
@@ -55,7 +55,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
         trace!(target: "blockchain_tree", hash=?block.hash, number=block.number, parent_hash=?block.parent_hash, "Inserting block");
         let mut tree = self.tree.write();
         let res = tree.insert_block(block);
-        tree.update_metrics();
+        tree.update_chains_metrics();
         res
     }
 
@@ -63,14 +63,14 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
         trace!(target: "blockchain_tree", ?finalized_block, "Finalizing block");
         let mut tree = self.tree.write();
         tree.finalize_block(finalized_block);
-        tree.update_metrics();
+        tree.update_chains_metrics();
     }
 
     fn restore_canonical_hashes(&self, last_finalized_block: BlockNumber) -> Result<(), Error> {
         trace!(target: "blockchain_tree", ?last_finalized_block, "Restoring canonical hashes for last finalized block");
         let mut tree = self.tree.write();
         let res = tree.restore_canonical_hashes(last_finalized_block);
-        tree.update_metrics();
+        tree.update_chains_metrics();
         res
     }
 
@@ -78,7 +78,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
         trace!(target: "blockchain_tree", ?block_hash, "Making block canonical");
         let mut tree = self.tree.write();
         let res = tree.make_canonical(block_hash);
-        tree.update_metrics();
+        tree.update_chains_metrics();
         res
     }
 
@@ -86,7 +86,7 @@ impl<DB: Database, C: Consensus, EF: ExecutorFactory> BlockchainTreeEngine
         trace!(target: "blockchain_tree", ?unwind_to, "Unwinding to block number");
         let mut tree = self.tree.write();
         let res = tree.unwind(unwind_to);
-        tree.update_metrics();
+        tree.update_chains_metrics();
         res
     }
 }