Set default nonempty block threshold to 80% in integration tests

crates/testing/src/test_builder.rs

@@ -156,9 +156,33 @@ pub struct TestDescription<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Ver
     pub validate_transactions: TransactionValidator,
 }
 
-pub fn nonempty_block_threshold(threshold: (u64, u64)) -> TransactionValidator {
+pub fn nonempty_block_threshold(nonempty: u64, total: u64) -> TransactionValidator {
     Arc::new(move |transactions| {
-        if matches!(threshold, (0, _)) {
+        let blocks: Vec<_> = transactions.iter().filter(|(view, _)| *view != 0).collect();
+
+        let num_blocks = blocks.len() as u64;
+        let mut num_nonempty_blocks = 0;
+
+        ensure!(num_blocks > 0, "Failed to commit any non-genesis blocks");
+
+        for (_, num_transactions) in blocks {
+            if *num_transactions > 0 {
+                num_nonempty_blocks += 1;
+            }
+        }
+
+        ensure!(
+          num_nonempty_blocks >= nonempty && num_blocks >= total,
+          "Failed to meet nonempty block threshold of {}/{}; got {num_nonempty_blocks} nonempty blocks out of a total of {num_blocks}", nonempty, total
+        );
+
+        Ok(())
+    })
+}
+
+pub fn nonempty_block_ratio(ratio: (u64, u64)) -> TransactionValidator {
+    Arc::new(move |transactions| {
+        if matches!(ratio, (0, _)) {
             return Ok(());
         }
 
@@ -176,9 +200,9 @@ pub fn nonempty_block_threshold(threshold: (u64, u64)) -> TransactionValidator {
         }
 
         ensure!(
-          // i.e. num_nonempty_blocks / num_blocks >= threshold.0 / threshold.1
-          num_nonempty_blocks * threshold.1 >= threshold.0 * num_blocks,
-          "Failed to meet nonempty block threshold of {}/{}; got {num_nonempty_blocks} nonempty blocks out of a total of {num_blocks}", threshold.0, threshold.1
+          // i.e. num_nonempty_blocks / num_blocks >= ratio.0 / ratio.1
+          num_nonempty_blocks * ratio.1 >= ratio.0 * num_blocks,
+          "Failed to meet nonempty block ratio of {}/{}; got {num_nonempty_blocks} nonempty blocks out of a total of {num_blocks}", ratio.0, ratio.1
         );
 
         Ok(())
@@ -500,7 +524,7 @@ impl<TYPES: NodeType, I: NodeImplementation<TYPES>, V: Versions> Default
             async_delay_config: DelayConfig::default(),
             upgrade_view: None,
             start_solver: true,
-            validate_transactions: Arc::new(|_| Ok(())),
+            validate_transactions: nonempty_block_ratio((40, 50)),
         }
     }
 }

crates/testing/tests/tests_4/test_marketplace.rs

@@ -15,7 +15,7 @@ use hotshot_testing::{
     block_builder::SimpleBuilderImplementation,
     completion_task::{CompletionTaskDescription, TimeBasedCompletionTaskDescription},
     test_builder::{
-        nonempty_block_limit, nonempty_block_threshold, BuilderChange, BuilderDescription,
+        nonempty_block_limit, nonempty_block_ratio, BuilderChange, BuilderDescription,
         TestDescription,
     },
 };
@@ -70,7 +70,7 @@ cross_tests!(
                                              },
                                          ),
             upgrade_view: Some(5),
-            validate_transactions: nonempty_block_threshold((40,50)),
+            validate_transactions: nonempty_block_ratio((40,50)),
             ..TestDescription::default()
         };
 
@@ -104,7 +104,7 @@ cross_tests!(
                 changes: HashMap::from([(0, BuilderChange::Down)])
               }
             ],
-            validate_transactions: nonempty_block_threshold((35,50)),
+            validate_transactions: nonempty_block_ratio((35,50)),
             ..TestDescription::default()
         };
 
@@ -140,7 +140,7 @@ cross_tests!(
               BuilderDescription {
                 changes: HashMap::from([(0, BuilderChange::Down)])
               },
-            validate_transactions: nonempty_block_threshold((40,50)),
+            validate_transactions: nonempty_block_ratio((40,50)),
             ..TestDescription::default()
         };
 

crates/testing/tests/tests_6/test_epochs.rs

@@ -20,7 +20,7 @@ use hotshot_testing::{
     completion_task::{CompletionTaskDescription, TimeBasedCompletionTaskDescription},
     overall_safety_task::OverallSafetyPropertiesDescription,
     spinning_task::{ChangeNode, NodeAction, SpinningTaskDescription},
-    test_builder::{TestDescription, TimingData},
+    test_builder::{nonempty_block_threshold, TestDescription, TimingData},
     view_sync_task::ViewSyncTaskDescription,
 };
 use hotshot_types::{data::ViewNumber, traits::node_implementation::ConsensusTime};
@@ -236,6 +236,7 @@ cross_tests!(
                                              },
                                          ),
             upgrade_view: Some(5),
+            validate_transactions: nonempty_block_threshold(40,50),
             ..TestDescription::default()
         };
 
@@ -254,7 +255,10 @@ cross_tests!(
     Versions: [EpochsTestVersions],
     Ignore: false,
     Metadata: {
-        let mut metadata = TestDescription::default_more_nodes().set_num_nodes(12,12);
+        let mut metadata = TestDescription {
+          validate_transactions: nonempty_block_threshold(15,20),
+          ..TestDescription::default_more_nodes()
+        }.set_num_nodes(12,12);
         metadata.test_config.epoch_height = 10;
         let dead_nodes = vec![
             ChangeNode {
@@ -272,7 +276,7 @@ cross_tests!(
         };
 
         // 2 nodes fail triggering view sync, expect no other timeouts
-        metadata.overall_safety_properties.num_failed_views = 6;
+        metadata.overall_safety_properties.num_failed_views = 100;
         // Make sure we keep committing rounds after the bad leaders, but not the full 50 because of the numerous timeouts
         metadata.overall_safety_properties.num_successful_views = 20;
         metadata.overall_safety_properties.expected_views_to_fail = HashMap::from([