Add a test to check if min_time_between_blocks_ms config option works

Signed-off-by: Jacinta Ferrant <[email protected]>

Author: jferrant

.github/workflows/bitcoin-tests.yml

@@ -98,6 +98,7 @@ jobs:
           - tests::signer::v0::miner_forking
           - tests::signer::v0::reloads_signer_set_in
           - tests::signer::v0::signers_broadcast_signed_blocks
+          - tests::signer::v0::min_gap_between_blocks
           - tests::nakamoto_integrations::stack_stx_burn_op_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state

testnet/stacks-node/src/config.rs

@@ -86,7 +86,7 @@ pub const OP_TX_ANY_ESTIM_SIZE: u64 = fmax!(
 const DEFAULT_MAX_RBF_RATE: u64 = 150; // 1.5x
 const DEFAULT_RBF_FEE_RATE_INCREMENT: u64 = 5;
 const INV_REWARD_CYCLES_TESTNET: u64 = 6;
-const DEFAULT_MINIMUM_GAP_MS: u64 = 1000;
+const DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS: u64 = 1000;
 
 #[derive(Clone, Deserialize, Default, Debug)]
 pub struct ConfigFile {
@@ -2359,9 +2359,9 @@ pub struct MinerConfig {
     pub wait_on_signers: Duration,
     /// Whether to mock sign in Epoch 2.5 through the .miners and .signers contracts. This is used for testing purposes in Epoch 2.5 only.
     pub pre_nakamoto_mock_signing: bool,
-    /// The minimum gap to wait between blocks in milliseconds. The value must be greater than or equal to 1000 ms because if a block is mined
+    /// The minimum time to wait between mining blocks in milliseconds. The value must be greater than or equal to 1000 ms because if a block is mined
     /// within the same second as its parent, it will be rejected by the signers.
-    pub min_block_time_gap_ms: u64,
+    pub min_time_between_blocks_ms: u64,
 }
 
 impl Default for MinerConfig {
@@ -2393,7 +2393,7 @@ impl Default for MinerConfig {
             // TODO: update to a sane value based on stackerdb benchmarking
             wait_on_signers: Duration::from_secs(200),
             pre_nakamoto_mock_signing: false, // Should only default true if mining key is set
-            min_block_time_gap_ms: DEFAULT_MINIMUM_GAP_MS,
+            min_time_between_blocks_ms: DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS,
         }
     }
 }
@@ -2744,7 +2744,7 @@ pub struct MinerConfigFile {
     pub max_reorg_depth: Option<u64>,
     pub wait_on_signers_ms: Option<u64>,
     pub pre_nakamoto_mock_signing: Option<bool>,
-    pub min_block_time_gap_ms: Option<u64>,
+    pub min_time_between_blocks_ms: Option<u64>,
 }
 
 impl MinerConfigFile {
@@ -2856,12 +2856,12 @@ impl MinerConfigFile {
             pre_nakamoto_mock_signing: self
                 .pre_nakamoto_mock_signing
                 .unwrap_or(pre_nakamoto_mock_signing), // Should only default true if mining key is set
-            min_block_time_gap_ms: self.min_block_time_gap_ms.map(|ms| if ms < DEFAULT_MINIMUM_GAP_MS {
-                warn!("miner.min_block_time_gap_ms is less than the minimum allowed value of {DEFAULT_MINIMUM_GAP_MS} ms. Using the default value instead.");
-                DEFAULT_MINIMUM_GAP_MS
+                min_time_between_blocks_ms: self.min_time_between_blocks_ms.map(|ms| if ms < DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS {
+                warn!("miner.min_time_between_blocks_ms is less than the minimum allowed value of {DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS} ms. Using the default value instead.");
+                DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS
             } else {
                 ms
-            }).unwrap_or(miner_default_config.min_block_time_gap_ms),
+            }).unwrap_or(miner_default_config.min_time_between_blocks_ms),
         })
     }
 }

testnet/stacks-node/src/nakamoto_node/miner.rs

@@ -1054,13 +1054,17 @@ impl BlockMinerThread {
         let time_since_parent_ms = get_epoch_time_secs()
             .saturating_sub(parent_block_info.stacks_parent_header.burn_header_timestamp)
             / 1000;
-        if time_since_parent_ms < self.config.miner.min_block_time_gap_ms {
+        if time_since_parent_ms < self.config.miner.min_time_between_blocks_ms {
             let wait_ms = self
                 .config
                 .miner
-                .min_block_time_gap_ms
+                .min_time_between_blocks_ms
                 .saturating_sub(time_since_parent_ms);
-            info!("Waiting {wait_ms} ms before mining a new block.");
+            info!("Parent block mined {} ms ago, waiting {} ms before mining a new block", time_since_parent_ms, wait_ms;
+                "parent_block_id" => %parent_block_info.stacks_parent_header.index_block_hash(),
+                "parent_block_height" => parent_block_info.stacks_parent_header.stacks_block_height,
+                "parent_block_timestamp" => parent_block_info.stacks_parent_header.burn_header_timestamp,
+            );
             sleep_ms(wait_ms);
         }
         Ok(())

testnet/stacks-node/src/tests/signer/v0.rs

@@ -2788,7 +2788,8 @@ fn signer_set_rollover() {
         .running_nodes
         .btc_regtest_controller
         .get_burnchain()
-        .reward_cycle_to_block_height(next_reward_cycle);
+        .reward_cycle_to_block_height(next_reward_cycle)
+        .saturating_add(1);
 
     info!("---- Mining to next reward set calculation -----");
     signer_test.run_until_burnchain_height_nakamoto(
@@ -2847,3 +2848,117 @@ fn signer_set_rollover() {
         assert!(signer.stop().is_none());
     }
 }
+
+#[test]
+#[ignore]
+/// This test checks that the signers will broadcast a block once they receive enough signatures.
+fn min_gap_between_blocks() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
+    tracing_subscriber::registry()
+        .with(fmt::layer())
+        .with(EnvFilter::from_default_env())
+        .init();
+
+    info!("------------------------- Test Setup -------------------------");
+    let num_signers = 5;
+    let sender_sk = Secp256k1PrivateKey::new();
+    let sender_addr = tests::to_addr(&sender_sk);
+    let send_amt = 100;
+    let send_fee = 180;
+    let recipient = PrincipalData::from(StacksAddress::burn_address(false));
+    let time_between_blocks_ms = 10_000;
+    let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
+        num_signers,
+        vec![(sender_addr.clone(), send_amt + send_fee)],
+        Some(Duration::from_secs(15)),
+        |_config| {},
+        |config| {
+            config.miner.min_time_between_blocks_ms = time_between_blocks_ms;
+        },
+        &[],
+    );
+
+    let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
+
+    signer_test.boot_to_epoch_3();
+
+    let proposals_before = signer_test
+        .running_nodes
+        .nakamoto_blocks_proposed
+        .load(Ordering::SeqCst);
+
+    let blocks_before = signer_test
+        .running_nodes
+        .nakamoto_blocks_mined
+        .load(Ordering::SeqCst);
+
+    let info_before = get_chain_info(&signer_test.running_nodes.conf);
+
+    // submit a tx so that the miner will mine a block
+    let sender_nonce = 0;
+    let transfer_tx =
+        make_stacks_transfer(&sender_sk, sender_nonce, send_fee, &recipient, send_amt);
+    submit_tx(&http_origin, &transfer_tx);
+
+    info!("Submitted transfer tx and waiting for block proposal. Ensure it does not arrive before the gap is exceeded");
+    let start_time = Instant::now();
+    while start_time.elapsed().as_millis() < (time_between_blocks_ms - 1000).into() {
+        let blocks_proposed = signer_test
+            .running_nodes
+            .nakamoto_blocks_proposed
+            .load(Ordering::SeqCst);
+        assert_eq!(
+            blocks_proposed, proposals_before,
+            "Block proposed before gap was exceeded"
+        );
+        std::thread::sleep(Duration::from_millis(100));
+    }
+
+    let start_time = Instant::now();
+    loop {
+        let blocks_proposed = signer_test
+            .running_nodes
+            .nakamoto_blocks_proposed
+            .load(Ordering::SeqCst);
+        if blocks_proposed > proposals_before {
+            break;
+        }
+        assert!(
+            start_time.elapsed().as_secs() < 30,
+            "Block not proposed after gap was exceeded within timeout"
+        );
+        std::thread::sleep(Duration::from_millis(100));
+    }
+
+    debug!("Ensure that the block is mined after the gap is exceeded");
+
+    let start = Instant::now();
+    let duration = 30;
+    loop {
+        let blocks_mined = signer_test
+            .running_nodes
+            .nakamoto_blocks_mined
+            .load(Ordering::SeqCst);
+
+        let info = get_chain_info(&signer_test.running_nodes.conf);
+        if blocks_mined > blocks_before && info.stacks_tip_height > info_before.stacks_tip_height {
+            break;
+        }
+
+        debug!(
+            "blocks_mined: {},{}, stacks_tip_height: {},{}",
+            blocks_mined, blocks_before, info_before.stacks_tip_height, info.stacks_tip_height
+        );
+
+        std::thread::sleep(Duration::from_millis(100));
+        assert!(
+            start.elapsed() < Duration::from_secs(duration),
+            "Block not mined within timeout"
+        );
+    }
+
+    signer_test.shutdown();
+}