From 7d8e6014e0a1f6a595d726c37bc692783eb3dd3c Mon Sep 17 00:00:00 2001 From: Brice Dobry <232827048+brice-stacks@users.noreply.github.com> Date: Wed, 5 Aug 2026 14:40:21 -0400 Subject: [PATCH 1/3] fix: avoid livelock after mining delay If, somehow, we arrive in a scenario where the miner's proposals are not reaching signers (or at least not 70% of them) for `block_proposal_max_age_secs`, then finally, the proposals arrive at the signers, they would previously silently drop this proposal, neither approving or rejecting it. The miner, continuing to wait for approval or rejection would be permanently stuck in the `propose_block` loop, only ever reproposing the same block. This commit changes the signer behavior so that instead of silently ignoring the block, the reject it with a new reason, `ProposalTooOld`, which, when it receives >= 30% of these rejections, will trigger the miner to exit that loop and mine a new block. --- libsigner/src/v0/messages.rs | 17 + stacks-node/src/tests/signer/v0/mod.rs | 28 +- .../signer/v0/proposal_replication_void.rs | 302 ++++++++++++++++++ .../changelog.d/replication-void-fix.changed | 1 + stacks-signer/src/v0/signer.rs | 13 +- 5 files changed, 352 insertions(+), 9 deletions(-) create mode 100644 stacks-node/src/tests/signer/v0/proposal_replication_void.rs create mode 100644 stacks-signer/changelog.d/replication-void-fix.changed diff --git a/libsigner/src/v0/messages.rs b/libsigner/src/v0/messages.rs index cf2f33d97e9..565977a18c0 100644 --- a/libsigner/src/v0/messages.rs +++ b/libsigner/src/v0/messages.rs @@ -1072,6 +1072,7 @@ impl From<&RejectReason> for RejectReasonPrefix { RejectReason::NoSignerConsensus => RejectReasonPrefix::NoSignerConsensus, RejectReason::ConsensusHashMismatch { .. } => RejectReasonPrefix::ConsensusHashMismatch, RejectReason::ProblematicTransactions => RejectReasonPrefix::ProblematicTransactions, + RejectReason::ProposalTooOld => RejectReasonPrefix::ProposalTooOld, RejectReason::Unknown(_) => RejectReasonPrefix::Unknown, RejectReason::NotRejected => RejectReasonPrefix::NotRejected, } @@ -1160,6 +1161,9 @@ pub enum RejectReason { /// The block marks one or more transactions as problematic, which signers /// do not yet allow ProblematicTransactions, + /// The block proposal's header timestamp is older than the signer's + /// configured `block_proposal_max_age_secs` + ProposalTooOld, /// The block was approved, no rejection details needed NotRejected, /// Handle unknown codes gracefully @@ -1210,6 +1214,9 @@ pub enum RejectReasonPrefix { /// The block marks one or more transactions as problematic, which signers /// do not yet allow ProblematicTransactions = 17, + /// The block proposal's header timestamp is older than the signer's + /// configured `block_proposal_max_age_secs` + ProposalTooOld = 18, /// Unknown reject code, for forward compatibility Unknown = 254, /// The block was approved, no rejection details needed @@ -1238,6 +1245,7 @@ impl RejectReasonPrefix { Self::NoSignerConsensus => 15, Self::ConsensusHashMismatch => 16, Self::ProblematicTransactions => 17, + Self::ProposalTooOld => 18, Self::Unknown => 254, Self::NotRejected => 255, } @@ -1265,6 +1273,7 @@ impl From for RejectReasonPrefix { 15 => Self::NoSignerConsensus, 16 => Self::ConsensusHashMismatch, 17 => Self::ProblematicTransactions, + 18 => Self::ProposalTooOld, 255 => Self::NotRejected, // For forward compatibility, all other values are unknown _ => Self::Unknown, @@ -1930,6 +1939,7 @@ impl StacksMessageCodec for RejectReason { | RejectReason::IrrecoverablePubkeyHash | RejectReason::NoSignerConsensus | RejectReason::ProblematicTransactions + | RejectReason::ProposalTooOld | RejectReason::Unknown(_) | RejectReason::NotRejected => { // No additional data to serialize / deserialize @@ -1975,6 +1985,7 @@ impl StacksMessageCodec for RejectReason { RejectReason::ConsensusHashMismatch { expected, actual } } RejectReasonPrefix::ProblematicTransactions => RejectReason::ProblematicTransactions, + RejectReasonPrefix::ProposalTooOld => RejectReason::ProposalTooOld, RejectReasonPrefix::Unknown => RejectReason::Unknown(type_prefix_byte), RejectReasonPrefix::NotRejected => RejectReason::NotRejected, }; @@ -2081,6 +2092,12 @@ impl std::fmt::Display for RejectReason { "The block has an irrecoverable associated miner public key hash." ) } + RejectReason::ProposalTooOld => { + write!( + f, + "The block proposal's header timestamp is older than the maximum proposal age." + ) + } RejectReason::NoSignerConsensus => { write!(f, "No signer consensus reached.") } diff --git a/stacks-node/src/tests/signer/v0/mod.rs b/stacks-node/src/tests/signer/v0/mod.rs index c16aa5304ba..496c39933d0 100644 --- a/stacks-node/src/tests/signer/v0/mod.rs +++ b/stacks-node/src/tests/signer/v0/mod.rs @@ -126,6 +126,7 @@ pub mod failed_txs; pub mod late_block_proposal; pub mod missing_burn_block_proposal; pub mod problematic_txs; +pub mod proposal_replication_void; pub mod reorg; pub mod signers_consider_consensus_blocks; pub mod signers_consider_late_proposals; @@ -2529,6 +2530,7 @@ pub fn wait_for_block_rejections_from_signers( })?; Ok(result) } + /// Waits for at least 70% of the provided signers to send an update for a block with the specificed burn block height and parent tenure stacks block height and message version pub fn wait_for_state_machine_update( timeout_secs: u64, @@ -5979,8 +5981,10 @@ fn block_validation_pending_table() { #[test] #[ignore] -/// Test the block_proposal_max_age_secs signer configuration option. It should reject blocks that are -/// invalid but within the max age window, otherwise it should simply drop the block without further processing. +/// Test the block_proposal_max_age_secs signer configuration option. Blocks that are +/// invalid but within the max age window are rejected after validation; blocks past the +/// max age window are rejected immediately (without validation) with ProposalTooOld so +/// the miner learns to re-mine instead of re-sending the same stale block forever. /// /// Test Setup: /// The test spins up five stacks signers, one miner Nakamoto node, and a corresponding bitcoind. @@ -5990,11 +5994,12 @@ fn block_validation_pending_table() { /// An invalid block proposal with a recent timestamp is forcibly written to the miner's slot to simulate the miner proposing a block. /// The signers process the invalid block and broadcast a block response rejection to the respective .signers-XXX-YYY contract. /// A second block proposal with an outdated timestamp is then submitted to the miner's slot to simulate the miner proposing a very old block. -/// The test confirms no further block rejection response is submitted to the .signers-XXX-YYY contract. +/// The test confirms the stale proposal is also rejected (with ProposalTooOld), and never accepted. /// /// Test Assertion: /// - Each signer successfully rejects the recent invalid block proposal. -/// - No signer submits a block proposal response for the outdated block proposal. +/// - Each signer rejects the outdated block proposal with the ProposalTooOld reason. +/// - No signer accepts either block. /// - The stacks tip does not advance fn block_proposal_max_age_rejections() { if env::var("BITCOIND_TEST") != Ok("1".into()) { @@ -6054,8 +6059,16 @@ fn block_proposal_max_age_rejections() { match message { SignerMessage::BlockResponse(BlockResponse::Rejected(BlockRejection { signer_signature_hash, + response_data, .. })) => { + if signer_signature_hash == block_signer_signature_hash_1 { + assert_eq!( + response_data.reject_reason, + RejectReason::ProposalTooOld, + "Stale proposal must be rejected as ProposalTooOld" + ); + } let entry = status_map.entry(signer_signature_hash).or_insert((0, 0)); entry.0 += 1; } @@ -6073,7 +6086,10 @@ fn block_proposal_max_age_rejections() { .get(&block_signer_signature_hash_1) .cloned() .unwrap_or((0, 0)); - assert_eq!(block_1_status, (0, 0)); + assert_eq!( + block_1_status.1, 0, + "Block 1 (stale) must never be accepted" + ); let block_2_status = status_map .get(&block_signer_signature_hash_2) @@ -6084,7 +6100,7 @@ fn block_proposal_max_age_rejections() { info!("Block 2 status"; "accepted" => %block_2_status.1, "rejected" => %block_2_status.0 ); - Ok(block_2_status.0 > num_signers * 7 / 10) + Ok(block_2_status.0 > num_signers * 7 / 10 && block_1_status.0 > num_signers * 7 / 10) }) .expect("Timed out waiting for block rejections"); diff --git a/stacks-node/src/tests/signer/v0/proposal_replication_void.rs b/stacks-node/src/tests/signer/v0/proposal_replication_void.rs new file mode 100644 index 00000000000..ac1c34f24d6 --- /dev/null +++ b/stacks-node/src/tests/signer/v0/proposal_replication_void.rs @@ -0,0 +1,302 @@ +// Copyright (C) 2026 Stacks Open Internet Foundation +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . +use std::env; +use std::time::Duration; + +use libsigner::v0::messages::RejectReason; +use pinny::tag; +use stacks::core::test_util::to_addr; +use stacks::types::chainstate::StacksPublicKey; +use stacks::util::get_epoch_time_secs; +use stacks::util::secp256k1::Secp256k1PrivateKey; +use stacks_signer::v0::tests::TEST_IGNORE_ALL_BLOCK_PROPOSALS; +use stacks_signer::v0::SpawnedSigner; +use tracing_subscriber::prelude::*; +use tracing_subscriber::{fmt, EnvFilter}; + +use super::SignerTest; +use crate::tests::nakamoto_integrations::wait_for; +use crate::tests::neon_integrations::{get_chain_info, test_observer}; +use crate::tests::signer::v0::{ + wait_for_block_proposal, wait_for_block_pushed_and_tip, wait_for_block_rejections_from_signers, +}; + +#[tag(bitcoind)] +#[test] +#[ignore] +/// Reproduce a "replication void", where the miner's block proposal reaches no +/// signer at all, for less time than `block_proposal_max_age_secs`. +/// +/// Test Setup: +/// Five signers, one miner Nakamoto node, bitcoind. The miner's +/// block_rejection_timeout is shrunk to 20s so the resend loop is observable +/// quickly. +/// +/// Test Execution: +/// 1. All signers are set to ignore incoming proposals (the void). +/// 2. A transfer tx forces the miner to mine and propose block N. +/// 3. Wait for the first re-send of the proposal and assert it is verbatim: +/// identical signer_signature_hash and header timestamp, and the chain tip +/// has not advanced. +/// 4. Lift the void. The signers accept the *original* proposal. +/// +/// Test Assertion: +/// The block that finally advances the tip is the original block N proposal — +/// same hash, same (now old) header timestamp. This proves that a void +/// shorter than block_proposal_max_age_secs ends with an old-timestamped +/// block on chain, NOT a freshly mined one. +fn proposal_void_shorter_than_max_age_recovers_with_original_block() { + 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::random(); + let sender_addr = to_addr(&sender_sk); + let send_amt = 100; + let send_fee = 180; + let signer_test: SignerTest = SignerTest::new_with_config_modifications( + num_signers, + vec![(sender_addr, send_amt + send_fee)], + |_| {}, + |config| { + // make the miner's SignatureTimeout resend loop fast enough to observe + config.miner.block_rejection_timeout_steps = [(0, Duration::from_secs(20))].into(); + }, + None, + None, + ); + signer_test.boot_to_epoch_3(); + + let conf = signer_test.running_nodes.conf.clone(); + let miner_sk = conf.miner.mining_key.clone().unwrap(); + let miner_pk = StacksPublicKey::from_private(&miner_sk); + let all_signers = signer_test.signer_test_pks(); + + info!("------------------------- Open the Void: All Signers Ignore Proposals -------------------------"); + test_observer::clear(); + TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(all_signers); + + let info_before = get_chain_info(&conf); + info!("------------------------- Force Miner to Propose Block N -------------------------"); + signer_test + .submit_transfer_tx(&sender_sk, send_fee, send_amt) + .expect("Failed to submit transfer tx"); + + let proposal_1 = wait_for_block_proposal(30, info_before.stacks_tip_height + 1, &miner_pk) + .expect("Timed out waiting for the initial proposal of block N"); + let sighash_1 = proposal_1.block.header.signer_signature_hash(); + let timestamp_1 = proposal_1.block.header.timestamp; + + info!("------------------------- Wait for the Verbatim Re-Send -------------------------"; + "signer_signature_hash" => %sighash_1, + "timestamp" => timestamp_1, + ); + test_observer::clear(); + let proposal_2 = wait_for_block_proposal(60, info_before.stacks_tip_height + 1, &miner_pk) + .expect("Timed out waiting for the miner to re-send the proposal into the void"); + assert_eq!( + proposal_2.block.header.signer_signature_hash(), + sighash_1, + "Miner should re-send the SAME proposal on SignatureTimeout, not re-mine" + ); + assert_eq!( + proposal_2.block.header.timestamp, timestamp_1, + "Re-sent proposal must keep the original header timestamp" + ); + let info_during = get_chain_info(&conf); + assert_eq!( + info_during.stacks_tip_height, info_before.stacks_tip_height, + "Chain must not advance while the proposal reaches no signer" + ); + + info!("------------------------- Lift the Void -------------------------"); + TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(vec![]); + + let block_n = + wait_for_block_pushed_and_tip(60, info_before.stacks_tip_height + 1, &miner_pk, || { + get_chain_info(&conf).stacks_tip + }) + .expect("Block N was not accepted after the void was lifted"); + assert_eq!( + block_n.header.signer_signature_hash(), + sighash_1, + "The block that ends the stall must be the ORIGINAL proposal" + ); + assert_eq!( + block_n.header.timestamp, timestamp_1, + "The accepted block must carry the original (old) header timestamp" + ); + // the accepted timestamp is genuinely old relative to acceptance time + assert!( + get_epoch_time_secs() >= timestamp_1 + 20, + "Test expected at least one full resend cycle to elapse before acceptance" + ); + signer_test.shutdown(); +} + +#[tag(bitcoind)] +#[test] +#[ignore] +/// Verify that a "replication void" longer than `block_proposal_max_age_secs` +/// no longer livelocks the tenure. +/// +/// Historically, signers silently dropped proposals whose header timestamp +/// was older than `block_proposal_max_age_secs`, broadcasting no rejection. +/// The miner's resend loop exits only on rejections reaching 30% weight, a +/// burn/stacks tip change, or the block appearing in the staging DB — none of +/// which can happen when every signer stays silent — so the miner re-sent the +/// same stale block forever and the tenure livelocked until the next +/// sortition. Signers now reject stale proposals with +/// `RejectReason::ProposalTooOld`, which trips the miner's rejection +/// threshold and makes it re-mine a fresh block. +/// +/// Test Setup: +/// Five signers with block_proposal_max_age_secs = 30, one miner with a 15s +/// rejection timeout. +/// +/// Test Execution: +/// 1. All signers ignore proposals (the void); the miner proposes block N. +/// 2. Hold the void for > 30s so the proposal goes stale, then lift it. +/// 3. The miner re-sends the stale proposal; every signer rejects it with +/// ProposalTooOld. +/// 4. The miner re-mines and the chain advances — with NO new bitcoin block. +/// +/// Test Assertion: +/// - All signers reject the stale proposal with reason ProposalTooOld. +/// - The chain recovers within the same tenure (no new sortition needed) and +/// the recovery block is a fresh re-mine: different signer_signature_hash +/// and a newer header timestamp. +fn proposal_void_longer_than_max_age_recovers_by_rejection_and_remine() { + 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::random(); + let sender_addr = to_addr(&sender_sk); + let send_amt = 100; + let send_fee = 180; + let max_age_secs = 30; + let signer_test: SignerTest = SignerTest::new_with_config_modifications( + num_signers, + vec![(sender_addr, send_amt + send_fee)], + |config| { + config.block_proposal_max_age_secs = max_age_secs; + }, + |config| { + config.miner.block_rejection_timeout_steps = [(0, Duration::from_secs(15))].into(); + }, + None, + None, + ); + signer_test.boot_to_epoch_3(); + + let conf = signer_test.running_nodes.conf.clone(); + let miner_sk = conf.miner.mining_key.clone().unwrap(); + let miner_pk = StacksPublicKey::from_private(&miner_sk); + let all_signers = signer_test.signer_test_pks(); + + info!("------------------------- Open the Void: All Signers Ignore Proposals -------------------------"); + test_observer::clear(); + TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(all_signers.clone()); + + let info_before = get_chain_info(&conf); + signer_test + .submit_transfer_tx(&sender_sk, send_fee, send_amt) + .expect("Failed to submit transfer tx"); + + let proposal_1 = wait_for_block_proposal(30, info_before.stacks_tip_height + 1, &miner_pk) + .expect("Timed out waiting for the initial proposal of block N"); + let sighash_1 = proposal_1.block.header.signer_signature_hash(); + let timestamp_1 = proposal_1.block.header.timestamp; + + info!("------------------------- Hold the Void Until the Proposal Is Stale -------------------------"; + "signer_signature_hash" => %sighash_1, + "timestamp" => timestamp_1, + "max_age_secs" => max_age_secs, + ); + // wait until the proposal is comfortably past max age (measured from its + // own header timestamp), while the miner keeps re-sending into the void + wait_for(max_age_secs * 3, || { + Ok(get_epoch_time_secs() > timestamp_1 + max_age_secs + 5) + }) + .expect("Timed out waiting for wall clock to pass proposal max age"); + + info!( + "------------------------- Lift the Void; Proposal Is Now Stale -------------------------" + ); + test_observer::clear(); + TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(vec![]); + + // the miner must still be re-sending the same stale block + let proposal_stale = wait_for_block_proposal(60, info_before.stacks_tip_height + 1, &miner_pk) + .expect("Timed out waiting for the miner to re-send the stale proposal"); + assert_eq!( + proposal_stale.block.header.signer_signature_hash(), + sighash_1, + "Miner should still be re-sending the SAME stale proposal" + ); + + info!("------------------------- Signers Reject the Stale Proposal -------------------------"); + let rejections = wait_for_block_rejections_from_signers(60, &sighash_1, &all_signers) + .expect("Timed out waiting for ProposalTooOld rejections from all signers"); + for rejection in &rejections { + assert_eq!( + rejection.response_data.reject_reason, + RejectReason::ProposalTooOld, + "Stale proposal must be rejected as ProposalTooOld" + ); + } + + info!( + "------------------------- Miner Re-Mines Within the Same Tenure -------------------------" + ); + // no new bitcoin block: recovery must come from the miner re-mining after + // the rejections trip its threshold + let burn_height_before = get_chain_info(&conf).burn_block_height; + let recovery_block = + wait_for_block_pushed_and_tip(120, info_before.stacks_tip_height + 1, &miner_pk, || { + get_chain_info(&conf).stacks_tip + }) + .expect("Chain did not recover via re-mine after the stale proposal was rejected"); + assert_eq!( + get_chain_info(&conf).burn_block_height, + burn_height_before, + "Recovery must not depend on a new sortition" + ); + assert_ne!( + recovery_block.header.signer_signature_hash(), + sighash_1, + "Recovery block must be freshly mined, not the stale proposal" + ); + assert!( + recovery_block.header.timestamp > timestamp_1, + "Recovery block must carry a fresh header timestamp" + ); + signer_test.shutdown(); +} diff --git a/stacks-signer/changelog.d/replication-void-fix.changed b/stacks-signer/changelog.d/replication-void-fix.changed new file mode 100644 index 00000000000..7cc85880e6c --- /dev/null +++ b/stacks-signer/changelog.d/replication-void-fix.changed @@ -0,0 +1 @@ +Instead of silently ignoring old block proposals, reject them with the new `ProposalTooOld` reason. This allows the miner to break out of its `propose_block` loop and mine a new block instead of being stuck in a live lock until the next Bitcoin block arrives. diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 38f66a832ba..b44f5868fb2 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -1262,14 +1262,20 @@ impl Signer { .saturating_add(self.block_proposal_max_age_secs) < get_epoch_time_secs() { - // Block is too old. Drop it with a warning. Don't even bother broadcasting to the node. - warn!("{self}: Received a block proposal that is more than {} secs old. Ignoring...", self.block_proposal_max_age_secs; + // Block is too old. Reject it (without validating) rather than silently + // dropping it: the miner's proposal loop re-sends the same block until it + // accumulates rejection weight, so a silent drop from the whole signer set + // would livelock the tenure until the next sortition. + warn!("{self}: Received a block proposal that is more than {} secs old. Rejecting...", self.block_proposal_max_age_secs; "signer_signature_hash" => %block_proposal.block.header.signer_signature_hash(), "block_id" => %block_proposal.block.block_id(), "block_height" => block_proposal.block.header.chain_length, "burn_height" => block_proposal.burn_height, "timestamp" => block_proposal.block.header.timestamp, ); + let rejection = + self.create_block_rejection(RejectReason::ProposalTooOld, &block_proposal.block); + self.send_block_response(&block_proposal.block, rejection.into()); return; } @@ -2361,7 +2367,8 @@ fn should_reevaluate_reject_reason(block_info: &BlockInfo) -> bool { | RejectReason::InvalidParentBlock | RejectReason::DuplicateBlockFound | RejectReason::IrrecoverablePubkeyHash - | RejectReason::ProblematicTransactions => { + | RejectReason::ProblematicTransactions + | RejectReason::ProposalTooOld => { // No need to re-validate these types of rejections. false } From 6fe49976fd42bc9cc2a445db52afa9c55b9845dd Mon Sep 17 00:00:00 2001 From: Brice Dobry <232827048+brice-stacks@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:22:20 -0400 Subject: [PATCH 2/3] fix: do NOT revert accept to reject based on proposal age --- .../signer/v0/proposal_replication_void.rs | 171 +++++++++++++++++- .../changelog.d/replication-void-fix.changed | 2 +- stacks-signer/src/v0/signer.rs | 45 ++--- 3 files changed, 193 insertions(+), 25 deletions(-) diff --git a/stacks-node/src/tests/signer/v0/proposal_replication_void.rs b/stacks-node/src/tests/signer/v0/proposal_replication_void.rs index ac1c34f24d6..abcbb5555ae 100644 --- a/stacks-node/src/tests/signer/v0/proposal_replication_void.rs +++ b/stacks-node/src/tests/signer/v0/proposal_replication_void.rs @@ -15,13 +15,15 @@ use std::env; use std::time::Duration; -use libsigner::v0::messages::RejectReason; +use libsigner::v0::messages::{BlockResponse, RejectReason, SignerMessage}; use pinny::tag; use stacks::core::test_util::to_addr; use stacks::types::chainstate::StacksPublicKey; use stacks::util::get_epoch_time_secs; use stacks::util::secp256k1::Secp256k1PrivateKey; -use stacks_signer::v0::tests::TEST_IGNORE_ALL_BLOCK_PROPOSALS; +use stacks_signer::v0::tests::{ + TEST_IGNORE_ALL_BLOCK_PROPOSALS, TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST, +}; use stacks_signer::v0::SpawnedSigner; use tracing_subscriber::prelude::*; use tracing_subscriber::{fmt, EnvFilter}; @@ -30,7 +32,9 @@ use super::SignerTest; use crate::tests::nakamoto_integrations::wait_for; use crate::tests::neon_integrations::{get_chain_info, test_observer}; use crate::tests::signer::v0::{ - wait_for_block_proposal, wait_for_block_pushed_and_tip, wait_for_block_rejections_from_signers, + get_stackerdb_signer_messages, wait_for_block_acceptance_from_signers, + wait_for_block_pre_commits_from_signers, wait_for_block_proposal, + wait_for_block_pushed_and_tip, wait_for_block_rejections_from_signers, }; #[tag(bitcoind)] @@ -300,3 +304,164 @@ fn proposal_void_longer_than_max_age_recovers_by_rejection_and_remine() { ); signer_test.shutdown(); } + +#[tag(bitcoind)] +#[test] +#[ignore] +/// Verify that a signer which has already decided on a block does not flip its +/// decision when the same proposal is re-sent after +/// `block_proposal_max_age_secs`. +/// +/// `ProposalTooOld` is only appropriate when the signer has nothing to report. +/// If the signer already accepted the block, overwriting that acceptance with a +/// rejection would leave the miner and the other signers with divergent views +/// of this signer's vote, depending on which of the two responses each of them +/// observed (the miner keeps the acceptance, since approvals are sticky, while +/// a signer that only saw the rejection would count it toward the rejection +/// threshold). Resending the prior acceptance is also what actually unsticks +/// the miner: it is re-proposing precisely because it never heard the +/// acceptance. +/// +/// Test Setup: +/// Five signers with block_proposal_max_age_secs = 30, one miner with a 15s +/// rejection timeout. +/// +/// Test Execution: +/// 1. Suppress the signers' acceptance broadcasts (note that this testing hook +/// suppresses acceptances only -- a rejection would still be broadcast), so +/// the signers validate and locally accept block N while the miner hears +/// nothing and stays in its resend loop. +/// 2. Hold that state for > 30s so block N's proposal goes stale, then let the +/// acceptances flow again. +/// 3. The miner re-sends the stale proposal, and every signer resends its +/// acceptance instead of rejecting it as too old. +/// +/// Test Assertion: +/// - All signers respond to the stale proposal with an acceptance. +/// - No signer ever rejects block N (in particular, not with ProposalTooOld). +/// - The original block N -- same hash, same old header timestamp -- is the +/// block that advances the tip. +fn stale_proposal_of_accepted_block_resends_acceptance() { + 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::random(); + let sender_addr = to_addr(&sender_sk); + let send_amt = 100; + let send_fee = 180; + let max_age_secs = 30; + let signer_test: SignerTest = SignerTest::new_with_config_modifications( + num_signers, + vec![(sender_addr, send_amt + send_fee)], + |config| { + config.block_proposal_max_age_secs = max_age_secs; + }, + |config| { + config.miner.block_rejection_timeout_steps = [(0, Duration::from_secs(15))].into(); + }, + None, + None, + ); + signer_test.boot_to_epoch_3(); + + let conf = signer_test.running_nodes.conf.clone(); + let miner_sk = conf.miner.mining_key.clone().unwrap(); + let miner_pk = StacksPublicKey::from_private(&miner_sk); + let all_signers = signer_test.signer_test_pks(); + + info!("------------------------- Suppress the Signers' Acceptances -------------------------"); + test_observer::clear(); + TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST.set(all_signers.clone()); + + let info_before = get_chain_info(&conf); + signer_test + .submit_transfer_tx(&sender_sk, send_fee, send_amt) + .expect("Failed to submit transfer tx"); + + let proposal = wait_for_block_proposal(30, info_before.stacks_tip_height + 1, &miner_pk) + .expect("Timed out waiting for the initial proposal of block N"); + let sighash = proposal.block.header.signer_signature_hash(); + let timestamp = proposal.block.header.timestamp; + + // pre-commits are still broadcast, so they are our proof that every signer + // validated block N and holds a decision to report, even though the miner + // never sees an acceptance + wait_for_block_pre_commits_from_signers(60, &sighash, &all_signers) + .expect("Timed out waiting for all signers to pre-commit to block N"); + + info!("------------------------- Hold Until the Proposal Is Stale -------------------------"; + "signer_signature_hash" => %sighash, + "timestamp" => timestamp, + "max_age_secs" => max_age_secs, + ); + wait_for(max_age_secs * 3, || { + Ok(get_epoch_time_secs() > timestamp + max_age_secs + 5) + }) + .expect("Timed out waiting for wall clock to pass proposal max age"); + + assert_eq!( + get_chain_info(&conf).stacks_tip_height, + info_before.stacks_tip_height, + "Chain must not advance while the acceptances are suppressed" + ); + + info!("------------------------- Let the Acceptances Flow -------------------------"); + test_observer::clear(); + TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST.set(vec![]); + + // the miner is still re-sending the same, now stale, proposal + let proposal_stale = wait_for_block_proposal(60, info_before.stacks_tip_height + 1, &miner_pk) + .expect("Timed out waiting for the miner to re-send the stale proposal"); + assert_eq!( + proposal_stale.block.header.signer_signature_hash(), + sighash, + "Miner should still be re-sending the SAME stale proposal" + ); + + info!("------------------------- Signers Resend Their Acceptance -------------------------"); + let acceptances = wait_for_block_acceptance_from_signers(60, &sighash, &all_signers) + .expect("Timed out waiting for the signers to resend their acceptance of block N"); + assert_eq!(acceptances.len(), num_signers); + + // no signer flipped its already-made decision to a rejection + let rejections: Vec<_> = get_stackerdb_signer_messages() + .into_iter() + .filter_map(|(_chunk, message)| match message { + SignerMessage::BlockResponse(BlockResponse::Rejected(rejection)) + if rejection.signer_signature_hash == sighash => + { + Some(rejection.response_data.reject_reason) + } + _ => None, + }) + .collect(); + assert!( + rejections.is_empty(), + "A signer that already accepted block N must not reject the stale re-proposal, got: {rejections:?}" + ); + + info!("------------------------- The Original Block N Lands -------------------------"); + let block_n = + wait_for_block_pushed_and_tip(60, info_before.stacks_tip_height + 1, &miner_pk, || { + get_chain_info(&conf).stacks_tip + }) + .expect("Block N was not accepted after the acceptances were unblocked"); + assert_eq!( + block_n.header.signer_signature_hash(), + sighash, + "The block that ends the stall must be the ORIGINAL proposal" + ); + assert_eq!( + block_n.header.timestamp, timestamp, + "The accepted block must carry the original (old) header timestamp" + ); + signer_test.shutdown(); +} diff --git a/stacks-signer/changelog.d/replication-void-fix.changed b/stacks-signer/changelog.d/replication-void-fix.changed index 7cc85880e6c..eb32a112e9e 100644 --- a/stacks-signer/changelog.d/replication-void-fix.changed +++ b/stacks-signer/changelog.d/replication-void-fix.changed @@ -1 +1 @@ -Instead of silently ignoring old block proposals, reject them with the new `ProposalTooOld` reason. This allows the miner to break out of its `propose_block` loop and mine a new block instead of being stuck in a live lock until the next Bitcoin block arrives. +Instead of silently ignoring old block proposals, reject them with the new `ProposalTooOld` reason. This allows the miner to break out of its `propose_block` loop and mine a new block instead of being stuck in a live lock until the next Bitcoin block arrives. Proposals for blocks that we have already decided on are unaffected: the signer resends its prior decision rather than flipping it. diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index b44f5868fb2..a430900b360 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -1255,6 +1255,16 @@ impl Signer { return; } + let signer_signature_hash = block_proposal.block.header.signer_signature_hash(); + let prior_block_info = self.block_lookup_by_reward_cycle(&signer_signature_hash); + if let Some(block_info) = &prior_block_info { + // If we have already decided on this block, resend that decision (or ignore + // the proposal) rather than evaluating it again. + if !self.should_reevaluate_block(block_info, block_proposal) { + return; + } + } + if block_proposal .block .header @@ -1267,7 +1277,7 @@ impl Signer { // accumulates rejection weight, so a silent drop from the whole signer set // would livelock the tenure until the next sortition. warn!("{self}: Received a block proposal that is more than {} secs old. Rejecting...", self.block_proposal_max_age_secs; - "signer_signature_hash" => %block_proposal.block.header.signer_signature_hash(), + "signer_signature_hash" => %signer_signature_hash, "block_id" => %block_proposal.block.block_id(), "block_height" => block_proposal.block.header.chain_length, "burn_height" => block_proposal.burn_height, @@ -1279,25 +1289,18 @@ impl Signer { return; } - // TODO: should add a check to ignore an old burn block height if we know its outdated. Would require us to store the burn block height we last saw on the side. - // the signer needs to be able to determine whether or not the block they're about to sign would conflict with an already-signed Stacks block - let signer_signature_hash = block_proposal.block.header.signer_signature_hash(); - let pending_responses = - if let Some(block_info) = self.block_lookup_by_reward_cycle(&signer_signature_hash) { - if !self.should_reevaluate_block(&block_info, block_proposal) { - return; - } - PendingBlockResponses::empty() - } else { - info!( - "{self}: received a block proposal for a new block."; - "signer_signature_hash" => %signer_signature_hash, - "block_id" => %block_proposal.block.block_id(), - "block_height" => block_proposal.block.header.chain_length, - "burn_height" => block_proposal.burn_height, - "consensus_hash" => %block_proposal.block.header.consensus_hash, - ); - self.signer_db + let pending_responses = if prior_block_info.is_some() { + PendingBlockResponses::empty() + } else { + info!( + "{self}: received a block proposal for a new block."; + "signer_signature_hash" => %signer_signature_hash, + "block_id" => %block_proposal.block.block_id(), + "block_height" => block_proposal.block.header.chain_length, + "burn_height" => block_proposal.burn_height, + "consensus_hash" => %block_proposal.block.header.consensus_hash, + ); + self.signer_db .drain_pending_block_responses(&signer_signature_hash) .unwrap_or_else(|e| { warn!( @@ -1307,7 +1310,7 @@ impl Signer { ); PendingBlockResponses::empty() }) - }; + }; crate::monitoring::actions::increment_block_proposals_received(); // Creating a new proposal will overwrite any prior proposal info on the block if it exists, e.g. validity, signed_timestamps, etc. let mut block_info = BlockInfo::from(block_proposal.clone()); From 9ccfb92c6613e8ce28d1506f9703ecd249ff8e01 Mon Sep 17 00:00:00 2001 From: Brice Dobry <232827048+brice-stacks@users.noreply.github.com> Date: Mon, 24 Aug 2026 11:44:05 -0400 Subject: [PATCH 3/3] chore: update comments --- sample/conf/signer/mainnet-signer-conf.toml | 4 +++- stacks-node/src/tests/signer/v0/mod.rs | 3 ++- stacks-signer/changelog.d/replication-void-fix.changed | 2 +- stacks-signer/src/config.rs | 4 +++- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/sample/conf/signer/mainnet-signer-conf.toml b/sample/conf/signer/mainnet-signer-conf.toml index 61432d5aceb..143a87eb042 100644 --- a/sample/conf/signer/mainnet-signer-conf.toml +++ b/sample/conf/signer/mainnet-signer-conf.toml @@ -88,7 +88,9 @@ db_path = "/var/lib/stacks-signer/signerdb.sqlite" # block_proposal_validation_timeout_ms = 120000 # Maximum age of a block proposal that will be processed. -# Proposals older than this are silently dropped. +# Proposals older than this are rejected (without validation) with a +# `ProposalTooOld` response, unless the signer has already decided on +# the block, in which case it resends its prior decision. # Default: 600 # Units: seconds # block_proposal_max_age_secs = 600 diff --git a/stacks-node/src/tests/signer/v0/mod.rs b/stacks-node/src/tests/signer/v0/mod.rs index 14157cf3e35..927e02f25b6 100644 --- a/stacks-node/src/tests/signer/v0/mod.rs +++ b/stacks-node/src/tests/signer/v0/mod.rs @@ -6050,7 +6050,8 @@ fn block_proposal_max_age_rejections() { signer_test.propose_block(block, short_timeout); info!("------------------------- Test Block Proposal Rejected -------------------------"); - // Verify the signers rejected only the SECOND block proposal. The first was not even processed. + // Verify the signers reject both proposals: the first (stale) with reason + // `ProposalTooOld` and without validation, the second after validation. wait_for(120, || { let mut status_map = HashMap::new(); for (_chunk, message) in get_stackerdb_signer_messages() { diff --git a/stacks-signer/changelog.d/replication-void-fix.changed b/stacks-signer/changelog.d/replication-void-fix.changed index eb32a112e9e..2c3ff1509de 100644 --- a/stacks-signer/changelog.d/replication-void-fix.changed +++ b/stacks-signer/changelog.d/replication-void-fix.changed @@ -1 +1 @@ -Instead of silently ignoring old block proposals, reject them with the new `ProposalTooOld` reason. This allows the miner to break out of its `propose_block` loop and mine a new block instead of being stuck in a live lock until the next Bitcoin block arrives. Proposals for blocks that we have already decided on are unaffected: the signer resends its prior decision rather than flipping it. +Instead of silently ignoring old block proposals, reject them with the new `ProposalTooOld` reason. This allows the miner to break out of its `propose_block` loop and mine a new block instead of being stuck in a livelock until the next Bitcoin block arrives. Proposals for blocks that we have already decided on are unaffected: the signer retains its prior decision, resending it when appropriate rather than flipping it. diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index 9f18223fd0b..ca31c5e3055 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -417,7 +417,9 @@ struct RawConfigFile { /// - Increase if signer and miner clocks are poorly synchronized. pub tenure_idle_timeout_buffer_secs: Option, /// The maximum age of a block proposal that will be processed by the signer. - /// Proposals older than this are ignored. + /// Proposals older than this are rejected (without validation) with a + /// `ProposalTooOld` response, unless the signer has already decided on the + /// block, in which case it resends its prior decision. /// --- /// @default: `600` /// @units: seconds