From 0db014da4071a99cc5dbece6473343f4025e9004 Mon Sep 17 00:00:00 2001 From: teor Date: Tue, 20 Sep 2022 21:31:24 +1000 Subject: [PATCH] fix(testnet): look back up to 10,000 blocks on testnet for a legacy chain (#5133) * Look back 10,000 blocks on testnet for a legacy chain * Use a smaller number of legacy chain blocks in the tests * Fix test assertion error message * Clarify legacy chain check comment * cargo fmt --all Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- zebra-state/src/constants.rs | 4 +++- zebra-state/src/service.rs | 6 +++++- zebra-state/src/service/check.rs | 11 ++++++++--- zebra-state/src/service/tests.rs | 19 +++++++++++-------- 4 files changed, 27 insertions(+), 13 deletions(-) diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index 1b3f75f7f..58094797e 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -22,7 +22,9 @@ 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 = 1000; +/// +/// Zebra usually only has to check back a few blocks, but on testnet it can be a long time between v5 transactions. +pub const MAX_LEGACY_CHAIN_BLOCKS: usize = 10_000; /// The maximum number of block hashes allowed in `getblocks` responses in the Zcash network protocol. pub const MAX_FIND_BLOCK_HASHES_RESULTS: u32 = 500; diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index dec20dee0..5e7859bdb 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -38,7 +38,10 @@ use zebra_chain::{ }; use crate::{ - constants::{MAX_FIND_BLOCK_HASHES_RESULTS, MAX_FIND_BLOCK_HEADERS_RESULTS_FOR_ZEBRA}, + constants::{ + MAX_FIND_BLOCK_HASHES_RESULTS, MAX_FIND_BLOCK_HEADERS_RESULTS_FOR_ZEBRA, + MAX_LEGACY_CHAIN_BLOCKS, + }, service::{ chain_tip::{ChainTipBlock, ChainTipChange, ChainTipSender, LatestChainTip}, finalized_state::{FinalizedState, ZebraDb}, @@ -251,6 +254,7 @@ impl StateService { nu5_activation_height, state.any_ancestor_blocks(tip.1), state.network, + MAX_LEGACY_CHAIN_BLOCKS, ) { let legacy_db_path = state.disk.path().to_path_buf(); panic!( diff --git a/zebra-state/src/service/check.rs b/zebra-state/src/service/check.rs index aeb7b8281..3372b5885 100644 --- a/zebra-state/src/service/check.rs +++ b/zebra-state/src/service/check.rs @@ -12,7 +12,7 @@ use zebra_chain::{ work::difficulty::CompactDifficulty, }; -use crate::{constants, BoxError, PreparedBlock, ValidateContextError}; +use crate::{BoxError, PreparedBlock, ValidateContextError}; // use self as check use super::check; @@ -289,10 +289,15 @@ fn difficulty_threshold_is_valid( } /// Check if zebra is following a legacy chain and return an error if so. +/// +/// `nu5_activation_height` should be `NetworkUpgrade::Nu5.activation_height(network)`, and +/// `max_legacy_chain_blocks` should be [`MAX_LEGACY_CHAIN_BLOCKS`](crate::constants::MAX_LEGACY_CHAIN_BLOCKS). +/// They are only changed from the defaults for testing. pub(crate) fn legacy_chain( nu5_activation_height: block::Height, ancestors: I, network: Network, + max_legacy_chain_blocks: usize, ) -> Result<(), BoxError> where I: Iterator>, @@ -315,8 +320,8 @@ 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 index >= constants::MAX_LEGACY_CHAIN_BLOCKS { + // the last Zebra instance that updated this cached state had no NU5 activation height. + if index >= max_legacy_chain_blocks { return Err(format!( "could not find any transactions in recent blocks: \ checked {index} blocks back from {:?}", diff --git a/zebra-state/src/service/tests.rs b/zebra-state/src/service/tests.rs index 4b9e47fd8..b0987b48b 100644 --- a/zebra-state/src/service/tests.rs +++ b/zebra-state/src/service/tests.rs @@ -20,7 +20,6 @@ use zebra_test::{prelude::*, transcript::Transcript}; use crate::{ arbitrary::Prepare, - 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}, @@ -277,11 +276,14 @@ fn state_behaves_when_blocks_are_committed_in_order() -> Result<()> { const DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES: u32 = 2; +/// The legacy chain limit for tests. +const TEST_LEGACY_CHAIN_LIMIT: usize = 100; + /// Check more blocks than the legacy chain limit. -const OVER_LEGACY_CHAIN_LIMIT: u32 = constants::MAX_LEGACY_CHAIN_BLOCKS as u32 + 10; +const OVER_LEGACY_CHAIN_LIMIT: u32 = TEST_LEGACY_CHAIN_LIMIT as u32 + 10; /// Check fewer blocks than the legacy chain limit. -const UNDER_LEGACY_CHAIN_LIMIT: u32 = constants::MAX_LEGACY_CHAIN_BLOCKS as u32 - 10; +const UNDER_LEGACY_CHAIN_LIMIT: u32 = TEST_LEGACY_CHAIN_LIMIT as u32 - 10; proptest! { #![proptest_config( @@ -304,7 +306,7 @@ proptest! { fn some_block_less_than_network_upgrade( (network, nu_activation_height, chain) in partial_nu5_chain_strategy(4, true, UNDER_LEGACY_CHAIN_LIMIT, NetworkUpgrade::Canopy) ) { - let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), network) + let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), network, TEST_LEGACY_CHAIN_LIMIT) .map_err(|error| error.to_string()); prop_assert_eq!(response, Ok(())); @@ -321,13 +323,13 @@ proptest! { .coinbase_height() .expect("chain contains valid blocks"); - let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), network) + let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), network, TEST_LEGACY_CHAIN_LIMIT) .map_err(|error| error.to_string()); prop_assert_eq!( response, Err(format!( - "could not find any transactions in recent blocks: checked {MAX_LEGACY_CHAIN_BLOCKS} blocks back from {tip_height:?}", + "could not find any transactions in recent blocks: checked {TEST_LEGACY_CHAIN_LIMIT} blocks back from {tip_height:?}", )) ); } @@ -360,7 +362,8 @@ proptest! { let response = crate::service::check::legacy_chain( nu_activation_height, chain.clone().into_iter().rev(), - network + network, + TEST_LEGACY_CHAIN_LIMIT, ).map_err(|error| error.to_string()); prop_assert_eq!( @@ -377,7 +380,7 @@ proptest! { fn at_least_one_transaction_with_valid_network_upgrade( (network, nu_activation_height, chain) in partial_nu5_chain_strategy(5, true, UNDER_LEGACY_CHAIN_LIMIT, NetworkUpgrade::Canopy) ) { - let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), network) + let response = crate::service::check::legacy_chain(nu_activation_height, chain.into_iter().rev(), network, TEST_LEGACY_CHAIN_LIMIT) .map_err(|error| error.to_string()); prop_assert_eq!(response, Ok(()));