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 <oxarbitrage@gmail.com>

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)

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;
 

zebra-state/src/service.rs

@@ -24,8 +24,8 @@ use zebra_chain::{
 };
 
 use crate::{
-    request::HashOrHeight, BoxError, CloneError, CommitBlockError, Config, FinalizedBlock,
+    constants, request::HashOrHeight, BoxError, CloneError, CommitBlockError, Config,
-    PreparedBlock, Request, Response, ValidateContextError,
+    FinalizedBlock, PreparedBlock, Request, Response, ValidateContextError,
 };
 
 pub(crate) mod check;
@@ -769,8 +769,6 @@ fn legacy_chain_check<I>(
 where
     I: Iterator<Item = Arc<Block>>,
 {
-    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());
         }
 

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());