Remove the redundant handling of stale blocks below the max reorg depth (they are rejected by the chainstate anyway)

Author: ImplOfAnImpl

build-tools/fork-detection/detector.py

@@ -56,15 +56,11 @@
 ]
 
 ENDED_UP_ON_A_FORK_FLAG_NAME = "ended_up_on_a_fork"
-STALE_BLOCK_BELOW_REORG_LIMIT_FLAG_NAME = "stale_block_below_reorg_limit"
 NO_INCOMING_BLOCKS_WHILE_ON_STALE_CHAIN_FLAG_NAME = "no_incoming_blocks_while_on_stale_chain"
 
 NODE_OUTPUT_LINE_NEW_TIP_REGEX = re.compile(
     r"NEW TIP in chainstate (?P<block_id>[0-9A-Fa-f]+) with height (?P<height>\d+), timestamp: (?P<timestamp>\d+)"
 )
-NODE_OUTPUT_LINE_STALE_BLOCK_RECEIVED_REGEX = re.compile(
-    r"Received stale block (?P<block_id>[0-9A-Fa-f]+) with height (?P<height>\d+), timestamp: (?P<timestamp>\d+)"
-)
 
 # The regex used to decide whether a node's output line should be printed to the console
 # (we want to avoid debug and info lines since they're both too noisy during sync and put extra
@@ -159,10 +155,10 @@ def do_full_sync(self):
         self.restore_peer_db()
 
         node_proc_env = os.environ.copy()
-        # Note: "chainstate_verbose_block_ids=debug" allows to catch the "Received stale block" line
-        # and also forces certain block-processing functions in chainstate to print full block ids.
-        # We avoid using the "normal" debug log, because it's too noisy, e.g. even
-        # "info,chainstate=debug" produces hundreds of megabytes of logs during the full sync.
+        # Note: "chainstate_verbose_block_ids=debug" forces certain block-processing functions
+        # in chainstate to print full block ids. We avoid using the "normal" debug log, because
+        # it's too noisy, e.g. even "info,chainstate=debug" produces hundreds of megabytes of
+        # logs during the full sync.
         node_proc_env["RUST_LOG"] = "info,chainstate_verbose_block_ids=debug"
 
         node_proc = subprocess.Popen(
@@ -236,24 +232,6 @@ def on_node_output_line_or_timeout(line: Optional[str]):
                     )
                     log_func(f"Tip reached, height = {height}{extra}")
                     return False
-
-            elif (stale_block_received_match := NODE_OUTPUT_LINE_STALE_BLOCK_RECEIVED_REGEX.search(line)) is not None:
-                block_id = stale_block_received_match.group("block_id")
-                height = int(stale_block_received_match.group("height"))
-
-                log.warn(f"Stale block received at height {height}: {block_id}")
-
-                if actual_tip_height - height >= MAX_REORG_DEPTH:
-                    # Note: this may mean 2 things:
-                    # a) If we're on the proper chain, then we've found a peer who is on
-                    #    a fork, in which case we definitely want to create this flag.
-                    # b) If we're on a fork, then the stale block may be from the proper
-                    #    chain, in which case the flag creation is redundant, because the code
-                    #    above or below should catch this situation; but it's not harmful either.
-                    self.touch_flag(
-                        STALE_BLOCK_BELOW_REORG_LIMIT_FLAG_NAME,
-                        f"height={height}, block id={block_id}"
-                    )
             else:
                 seconds_since_last_tip = (
                     cur_seconds_since_epoch - last_tip_arrival_time

chainstate/src/detail/mod.rs

@@ -94,9 +94,8 @@ type ChainstateEventHandler = EventHandler<ChainstateEvent>;
 
 pub type OrphanErrorHandler = dyn Fn(&BlockError) + Send + Sync;
 
-/// A tracing target that either forces full block ids to be printed where they're normally
-/// printed in the abbreviated form, or just makes block ids be printed where normally they won't
-/// be.
+/// A tracing target that forces full block ids to be printed in certain places where they're
+/// normally printed in the abbreviated form.
 pub const CHAINSTATE_TRACING_TARGET_VERBOSE_BLOCK_IDS: &str = "chainstate_verbose_block_ids";
 
 #[must_use]
@@ -635,21 +634,6 @@ impl<S: BlockchainStorage, V: TransactionVerificationStrategy> Chainstate<S, V>
 
             self.update_initial_block_download_flag()
                 .map_err(BlockError::BestBlockIdQueryError)?;
-        } else {
-            if tracing::event_enabled!(tracing::Level::DEBUG, CHAINSTATE_TRACING_TARGET_VERBOSE_BLOCK_IDS) {
-                // FIXME: return custom enum from attempt_to_process_block
-                let chainstate_ref = self.make_db_tx_ro().map_err(BlockError::from)?;
-                let bi = get_existing_block_index(&chainstate_ref, &block_id)?;
-
-                tracing::debug!(
-                    target: CHAINSTATE_TRACING_TARGET_VERBOSE_BLOCK_IDS,
-                    "Received stale block {:x} with height {}, timestamp: {} ({})",
-                    bi.block_id(),
-                    bi.block_height(),
-                    bi.block_timestamp(),
-                    bi.block_timestamp().into_time(),
-                )
-            }
         }
 
         Ok(result)