From 9851a2417868465924d551efc2a1ccc9376b5a09 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Wed, 29 Jan 2025 15:36:05 -0500 Subject: [PATCH 1/4] chore: extend the default `block_proposal_timeout` to 4 hours Until #5729 is implemented, then there is no point in rejecting a block from a miner, no matter how late it is, since no other miner will ever try to extend into its tenure. Fixes #5753 --- stacks-signer/CHANGELOG.md | 6 ++++++ stacks-signer/src/config.rs | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/stacks-signer/CHANGELOG.md b/stacks-signer/CHANGELOG.md index 2697d93508..398d4125b3 100644 --- a/stacks-signer/CHANGELOG.md +++ b/stacks-signer/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to the versioning scheme outlined in the [README.md](README.md). +## [Unreleased] + +### Changed + +- Increase default `block_proposal_timeout_ms` from 10 minutes to 4 hours. Until #5729 is implemented, there is no value in rejecting a late block from a miner, since a late block is better than no block at all. + ## [3.1.0.0.4.0] ## Added diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index a50ca7ecf8..dfcafd50e0 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -35,7 +35,7 @@ use stacks_common::util::hash::Hash160; use crate::client::SignerSlotID; const EVENT_TIMEOUT_MS: u64 = 5000; -const BLOCK_PROPOSAL_TIMEOUT_MS: u64 = 600_000; +const BLOCK_PROPOSAL_TIMEOUT_MS: u64 = 14_400_000; const BLOCK_PROPOSAL_VALIDATION_TIMEOUT_MS: u64 = 120_000; const DEFAULT_FIRST_PROPOSAL_BURN_BLOCK_TIMING_SECS: u64 = 60; const DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS: u64 = 30; From 38dbdcdd54590e143c02035b9577eb19e49e5aa3 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Thu, 30 Jan 2025 13:07:47 -0500 Subject: [PATCH 2/4] test: fix flakiness in `tests::nakamoto_integrations::skip_mining_long_tx` --- .../src/tests/nakamoto_integrations.rs | 32 +++++++++++++------ 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index f3659d5957..2884e6d1b7 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -1688,22 +1688,22 @@ fn simple_neon_integration() { assert!(tip.anchored_header.as_stacks_nakamoto().is_some()); assert!(tip.stacks_block_height >= block_height_pre_3_0 + 30); - // Check that we aren't missing burn blocks + // Check that we aren't missing burn blocks (except during the Nakamoto transition) + let epoch_3 = &naka_conf.burnchain.epochs.unwrap()[StacksEpochId::Epoch30]; let bhh = u64::from(tip.burn_header_height); let missing = test_observer::get_missing_burn_blocks(220..=bhh).unwrap(); - // This test was flakey because it was sometimes missing burn block 230, which is right at the Nakamoto transition + // This test was flaky because it was sometimes missing burn block 230, which is right at the Nakamoto transition // So it was possible to miss a burn block during the transition - // But I don't it matters at this point since the Nakamoto transition has already happened on mainnet + // But I don't think it matters at this point since the Nakamoto transition has already happened on mainnet // So just print a warning instead, don't count it as an error let missing_is_error: Vec<_> = missing .into_iter() - .filter(|i| match i { - 230 => { - warn!("Missing burn block {i}"); + .filter(|&i| { + (i != epoch_3.start_height - 1) || { + warn!("Missing burn block {} at epoch 3 transition", i); false } - _ => true, }) .collect(); @@ -9885,9 +9885,23 @@ fn skip_mining_long_tx() { assert_eq!(sender_2_nonce, 0); assert_eq!(sender_1_nonce, 4); - // Check that we aren't missing burn blocks + // Check that we aren't missing burn blocks (except during the Nakamoto transition) + let epoch_3 = &naka_conf.burnchain.epochs.unwrap()[StacksEpochId::Epoch30]; let bhh = u64::from(tip.burn_header_height); - test_observer::contains_burn_block_range(220..=bhh).unwrap(); + let missing = test_observer::get_missing_burn_blocks(220..=bhh).unwrap(); + let missing_is_error: Vec<_> = missing + .into_iter() + .filter(|&i| { + (i != epoch_3.start_height - 1) || { + warn!("Missing burn block {} at epoch 3 transition", i); + false + } + }) + .collect(); + + if !missing_is_error.is_empty() { + panic!("Missing the following burn blocks: {missing_is_error:?}"); + } check_nakamoto_empty_block_heuristics(); From 6224b7231fb18b0b34d4339142a0a2a529516ee7 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Thu, 30 Jan 2025 13:18:01 -0500 Subject: [PATCH 3/4] test: increase timeout in `block_validation_pending_table` --- testnet/stacks-node/src/tests/signer/v0.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index feb870143f..46aebd7165 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -8024,7 +8024,7 @@ fn block_validation_pending_table() { .expect("Timed out waiting for pending block validation to be submitted"); info!("----- Waiting for pending block validation to be removed -----"); - wait_for(30, || { + wait_for(60, || { let is_pending = signer_db .has_pending_block_validation(&block_signer_signature_hash) .expect("Unexpected DBError"); From fa6a6b70d30f9dc4ae6e548542f218a84a2a12e0 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Fri, 31 Jan 2025 09:50:28 -0500 Subject: [PATCH 4/4] refactor: add `check_nakamoto_no_missing_blocks` func This function can be used to check for missing burn blocks, ignoring a block missing during the transition to epoch 3.0. --- .../src/tests/nakamoto_integrations.rs | 58 ++++++++----------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index 682a7b216b..8e7132df49 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . use std::collections::{BTreeMap, HashMap, HashSet}; +use std::ops::RangeBounds; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc::{channel, Receiver, Sender}; use std::sync::{Arc, Mutex}; @@ -1488,6 +1489,26 @@ fn wait_for_first_naka_block_commit(timeout_secs: u64, naka_commits_submitted: & } } +// Check for missing burn blocks in `range`, but allow for a missed block at +// the epoch 3 transition. Panic if any other blocks are missing. +fn check_nakamoto_no_missing_blocks(conf: &Config, range: impl RangeBounds) { + let epoch_3 = &conf.burnchain.epochs.as_ref().unwrap()[StacksEpochId::Epoch30]; + let missing = test_observer::get_missing_burn_blocks(range).unwrap(); + let missing_is_error: Vec<_> = missing + .into_iter() + .filter(|&i| { + (i != epoch_3.start_height - 1) || { + warn!("Missing burn block {} at epoch 3 transition", i); + false + } + }) + .collect(); + + if !missing_is_error.is_empty() { + panic!("Missing the following burn blocks: {missing_is_error:?}"); + } +} + #[test] #[ignore] /// This test spins up a nakamoto-neon node. @@ -1689,27 +1710,8 @@ fn simple_neon_integration() { assert!(tip.stacks_block_height >= block_height_pre_3_0 + 30); // Check that we aren't missing burn blocks (except during the Nakamoto transition) - let epoch_3 = &naka_conf.burnchain.epochs.unwrap()[StacksEpochId::Epoch30]; let bhh = u64::from(tip.burn_header_height); - let missing = test_observer::get_missing_burn_blocks(220..=bhh).unwrap(); - - // This test was flaky because it was sometimes missing burn block 230, which is right at the Nakamoto transition - // So it was possible to miss a burn block during the transition - // But I don't think it matters at this point since the Nakamoto transition has already happened on mainnet - // So just print a warning instead, don't count it as an error - let missing_is_error: Vec<_> = missing - .into_iter() - .filter(|&i| { - (i != epoch_3.start_height - 1) || { - warn!("Missing burn block {} at epoch 3 transition", i); - false - } - }) - .collect(); - - if !missing_is_error.is_empty() { - panic!("Missing the following burn blocks: {missing_is_error:?}"); - } + check_nakamoto_no_missing_blocks(&naka_conf, 220..=bhh); // make sure prometheus returns an updated number of processed blocks #[cfg(feature = "monitoring_prom")] @@ -10106,22 +10108,8 @@ fn skip_mining_long_tx() { assert_eq!(sender_1_nonce, 4); // Check that we aren't missing burn blocks (except during the Nakamoto transition) - let epoch_3 = &naka_conf.burnchain.epochs.unwrap()[StacksEpochId::Epoch30]; let bhh = u64::from(tip.burn_header_height); - let missing = test_observer::get_missing_burn_blocks(220..=bhh).unwrap(); - let missing_is_error: Vec<_> = missing - .into_iter() - .filter(|&i| { - (i != epoch_3.start_height - 1) || { - warn!("Missing burn block {} at epoch 3 transition", i); - false - } - }) - .collect(); - - if !missing_is_error.is_empty() { - panic!("Missing the following burn blocks: {missing_is_error:?}"); - } + check_nakamoto_no_missing_blocks(&naka_conf, 220..=bhh); check_nakamoto_empty_block_heuristics();