From 4f96a4bb403ac23351446230fcdb2016954ee884 Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 30 Jul 2021 09:22:10 +1000 Subject: [PATCH] Increase coverage for generated chains and proptests (#2540) * Make legacy chain limit clearer That way, it doesn't get confused with the coinbase maturity limit. * Allow 1-5 transactions in each generated block, not always 5 * rustfmt Co-authored-by: Alfredo Garcia --- zebra-chain/src/transaction/arbitrary.rs | 2 +- zebra-state/src/constants.rs | 4 ++++ zebra-state/src/service.rs | 8 +++----- zebra-state/src/service/tests.rs | 19 +++++++++++++------ 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/zebra-chain/src/transaction/arbitrary.rs b/zebra-chain/src/transaction/arbitrary.rs index d3cde508..6046f7dd 100644 --- a/zebra-chain/src/transaction/arbitrary.rs +++ b/zebra-chain/src/transaction/arbitrary.rs @@ -166,7 +166,7 @@ impl Transaction { ledger_state.has_coinbase = false; let remainder = vec( Transaction::arbitrary_with(ledger_state).prop_map(Arc::new), - len, + 0..=len, ); (coinbase, remainder) diff --git a/zebra-state/src/constants.rs b/zebra-state/src/constants.rs index e4e2bb1b..334761a8 100644 --- a/zebra-state/src/constants.rs +++ b/zebra-state/src/constants.rs @@ -27,6 +27,10 @@ pub const MAX_BLOCK_REORG_HEIGHT: u32 = MIN_TRANSPARENT_COINBASE_MATURITY - 1; /// The database format version, incremented each time the database format changes. pub const DATABASE_FORMAT_VERSION: u32 = 6; +/// 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; + use lazy_static::lazy_static; use regex::Regex; diff --git a/zebra-state/src/service.rs b/zebra-state/src/service.rs index a6932759..9982223e 100644 --- a/zebra-state/src/service.rs +++ b/zebra-state/src/service.rs @@ -24,8 +24,8 @@ use zebra_chain::{ }; use crate::{ - request::HashOrHeight, BoxError, CloneError, CommitBlockError, Config, FinalizedBlock, - PreparedBlock, Request, Response, ValidateContextError, + constants, request::HashOrHeight, BoxError, CloneError, CommitBlockError, Config, + FinalizedBlock, PreparedBlock, Request, Response, ValidateContextError, }; pub(crate) mod check; @@ -769,8 +769,6 @@ fn legacy_chain_check( where I: Iterator>, { - const MAX_BLOCKS_TO_CHECK: usize = 100; - for (count, 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. @@ -787,7 +785,7 @@ 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 >= MAX_BLOCKS_TO_CHECK { + if count >= constants::MAX_LEGACY_CHAIN_BLOCKS { return Err("giving up after checking too many blocks".into()); } diff --git a/zebra-state/src/service/tests.rs b/zebra-state/src/service/tests.rs index 0813a4fb..d1b34d4b 100644 --- a/zebra-state/src/service/tests.rs +++ b/zebra-state/src/service/tests.rs @@ -10,7 +10,9 @@ use zebra_chain::{ }; use zebra_test::{prelude::*, transcript::Transcript}; -use crate::{init_test, tests::setup::partial_nu5_chain_strategy, BoxError, Request, Response}; +use crate::{ + constants, init_test, tests::setup::partial_nu5_chain_strategy, BoxError, Request, Response, +}; const LAST_BLOCK_HEIGHT: u32 = 10; @@ -174,7 +176,12 @@ fn state_behaves_when_blocks_are_committed_in_order() -> Result<()> { } const DEFAULT_PARTIAL_CHAIN_PROPTEST_CASES: u32 = 2; -const BLOCKS_AFTER_NU5: u32 = 101; + +/// Check more blocks than the legacy chain limit. +const OVER_LEGACY_CHAIN_LIMIT: u32 = constants::MAX_LEGACY_CHAIN_BLOCKS 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; proptest! { #![proptest_config( @@ -195,7 +202,7 @@ proptest! { /// Test blocks that are less than the NU5 activation height. #[test] fn some_block_less_than_network_upgrade( - (network, nu_activation_height, chain) in partial_nu5_chain_strategy(4, true, BLOCKS_AFTER_NU5/2, NetworkUpgrade::Canopy) + (network, nu_activation_height, chain) in partial_nu5_chain_strategy(4, true, UNDER_LEGACY_CHAIN_LIMIT, NetworkUpgrade::Canopy) ) { let response = crate::service::legacy_chain_check(nu_activation_height, chain.into_iter().rev(), network) .map_err(|error| error.to_string()); @@ -206,7 +213,7 @@ proptest! { /// Test the maximum amount of blocks to check before chain is declared to be legacy. #[test] fn no_transaction_with_network_upgrade( - (network, nu_activation_height, chain) in partial_nu5_chain_strategy(4, true, BLOCKS_AFTER_NU5, NetworkUpgrade::Canopy) + (network, nu_activation_height, chain) in partial_nu5_chain_strategy(4, true, OVER_LEGACY_CHAIN_LIMIT, NetworkUpgrade::Canopy) ) { let response = crate::service::legacy_chain_check(nu_activation_height, chain.into_iter().rev(), network) .map_err(|error| error.to_string()); @@ -220,7 +227,7 @@ proptest! { /// Test the `Block.check_transaction_network_upgrade()` error inside the legacy check. #[test] fn at_least_one_transaction_with_inconsistent_network_upgrade( - (network, nu_activation_height, chain) in partial_nu5_chain_strategy(5, false, BLOCKS_AFTER_NU5, NetworkUpgrade::Canopy) + (network, nu_activation_height, chain) in partial_nu5_chain_strategy(5, false, OVER_LEGACY_CHAIN_LIMIT, NetworkUpgrade::Canopy) ) { // this test requires that an invalid block is encountered // before a valid block (and before the check gives up), @@ -260,7 +267,7 @@ proptest! { /// Test there is at least one transaction with a valid `network_upgrade` in the legacy check. #[test] fn at_least_one_transaction_with_valid_network_upgrade( - (network, nu_activation_height, chain) in partial_nu5_chain_strategy(5, true, BLOCKS_AFTER_NU5/2, NetworkUpgrade::Canopy) + (network, nu_activation_height, chain) in partial_nu5_chain_strategy(5, true, UNDER_LEGACY_CHAIN_LIMIT, NetworkUpgrade::Canopy) ) { let response = crate::service::legacy_chain_check(nu_activation_height, chain.into_iter().rev(), network) .map_err(|error| error.to_string());