fix(consensus): Increase the number of blocks checked for legacy transactions (#4804)

* Increase the number of legacy chain check blocks, and improve logging

* Automatically adjust test message when MAX_LEGACY_CHAIN_BLOCKS changes

zebra-state/src/constants.rs

@@ -22,7 +22,7 @@ pub const DATABASE_FORMAT_VERSION: u32 = 25;
 
 /// The maximum number of blocks to check for NU5 transactions,
 /// before we assume we are on a pre-NU5 legacy chain.
-pub const MAX_LEGACY_CHAIN_BLOCKS: usize = 100;
+pub const MAX_LEGACY_CHAIN_BLOCKS: usize = 1000;
 
 use lazy_static::lazy_static;
 use regex::Regex;

zebra-state/src/service.rs

@@ -206,27 +206,28 @@ impl StateService {
         let timer = CodeTimer::start();
 
         if let Some(tip) = state.best_tip() {
-            if let Some(nu5_activation_height) = NetworkUpgrade::Nu5.activation_height(network) {
+            let nu5_activation_height = NetworkUpgrade::Nu5
-                if check::legacy_chain(
+                .activation_height(network)
-                    nu5_activation_height,
+                .expect("NU5 activation height is set");
-                    state.any_ancestor_blocks(tip.1),
+
-                    state.network,
+            if let Err(error) = check::legacy_chain(
-                )
+                nu5_activation_height,
-                .is_err()
+                state.any_ancestor_blocks(tip.1),
-                {
+                state.network,
-                    let legacy_db_path = Some(state.disk.path().to_path_buf());
+            ) {
-                    panic!(
+                let legacy_db_path = state.disk.path().to_path_buf();
-                        "Cached state contains a legacy chain. \
+                panic!(
-                        An outdated Zebra version did not know about a recent network upgrade, \
+                    "Cached state contains a legacy chain.\n\
-                        so it followed a legacy chain using outdated rules. \
+                     An outdated Zebra version did not know about a recent network upgrade,\n\
-                        Hint: Delete your database, and restart Zebra to do a full sync. \
+                     so it followed a legacy chain using outdated consensus branch rules.\n\
-                        Database path: {:?}",
+                     Hint: Delete your database, and restart Zebra to do a full sync.\n\
-                        legacy_db_path,
+                     Database path: {legacy_db_path:?}\n\
-                    );
+                     Error: {error:?}",
-                }
+                );
             }
         }
-        tracing::info!("no legacy chain found");
+
+        tracing::info!("cached state consensus branch is valid: no legacy chain found");
         timer.finish(module_path!(), line!(), "legacy chain check");
 
         (state, read_only_service, latest_chain_tip, chain_tip_change)

zebra-state/src/service/check.rs

@@ -297,7 +297,10 @@ pub(crate) fn legacy_chain<I>(
 where
     I: Iterator<Item = Arc<Block>>,
 {
-    for (count, block) in ancestors.enumerate() {
+    let mut ancestors = ancestors.peekable();
+    let tip_height = ancestors.peek().and_then(|block| block.coinbase_height());
+
+    for (index, block) in ancestors.enumerate() {
         // Stop checking if the chain reaches Canopy. We won't find any more V5 transactions,
         // so the rest of our checks are useless.
         //
@@ -313,8 +316,13 @@ where
 
         // If we are past our NU5 activation height, but there are no V5 transactions in recent blocks,
         // the Zebra instance that verified those blocks had no NU5 activation height.
-        if count >= constants::MAX_LEGACY_CHAIN_BLOCKS {
+        if index >= constants::MAX_LEGACY_CHAIN_BLOCKS {
-            return Err("giving up after checking too many blocks".into());
+            return Err(format!(
+                "could not find any transactions in recent blocks: \
+                 checked {index} blocks back from {:?}",
+                tip_height.expect("database contains valid blocks"),
+            )
+            .into());
         }
 
         // If a transaction `network_upgrade` field is different from the network upgrade calculated
@@ -322,7 +330,9 @@ where
         // network upgrade heights.
         block
             .check_transaction_network_upgrade_consistency(network)
-            .map_err(|_| "inconsistent network upgrade found in transaction")?;
+            .map_err(|error| {
+                format!("inconsistent network upgrade found in transaction: {error:?}")
+            })?;
 
         // If we find at least one transaction with a valid `network_upgrade` field, the Zebra instance that
         // verified those blocks used the same network upgrade heights. (Up to this point in the chain.)

zebra-state/src/service/tests.rs

@@ -20,7 +20,8 @@ use zebra_test::{prelude::*, transcript::Transcript};
 
 use crate::{
     arbitrary::Prepare,
-    constants, init_test,
+    constants::{self, MAX_LEGACY_CHAIN_BLOCKS},
+    init_test,
     service::{arbitrary::populated_state, chain_tip::TipAction, StateService},
     tests::setup::{partial_nu5_chain_strategy, transaction_v4_from_coinbase},
     BoxError, Config, FinalizedBlock, PreparedBlock, Request, Response,
@@ -314,12 +315,20 @@ proptest! {
     fn no_transaction_with_network_upgrade(
         (network, nu_activation_height, chain) in partial_nu5_chain_strategy(4, true, OVER_LEGACY_CHAIN_LIMIT, NetworkUpgrade::Canopy)
     ) {
+        let tip_height = chain
+            .last()
+            .expect("chain contains at least one block")
+            .coinbase_height()
+            .expect("chain contains valid blocks");
+
         let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), network)
             .map_err(|error| error.to_string());
 
         prop_assert_eq!(
             response,
-            Err("giving up after checking too many blocks".into())
+            Err(format!(
+                "could not find any transactions in recent blocks: checked {MAX_LEGACY_CHAIN_BLOCKS} blocks back from {tip_height:?}",
+            ))
         );
     }
 
@@ -356,7 +365,7 @@ proptest! {
 
         prop_assert_eq!(
             response,
-            Err("inconsistent network upgrade found in transaction".into()),
+            Err("inconsistent network upgrade found in transaction: WrongTransactionConsensusBranchId".into()),
             "first: {:?}, last: {:?}",
             chain.first().map(|block| block.coinbase_height()),
             chain.last().map(|block| block.coinbase_height()),