From bdda2cd17a42cd1b55c93eeb9734dc6b8d2de8e8 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 18 Apr 2025 14:56:15 -0500 Subject: [PATCH 1/6] add configurable `proposal_wait_time_ms` to signer --- stacks-signer/src/chainstate.rs | 3 +++ stacks-signer/src/client/mod.rs | 1 + stacks-signer/src/config.rs | 9 +++++++++ stacks-signer/src/runloop.rs | 1 + stacks-signer/src/tests/chainstate.rs | 1 + stacks-signer/src/v0/signer.rs | 2 ++ testnet/stacks-node/src/tests/nakamoto_integrations.rs | 3 +++ testnet/stacks-node/src/tests/signer/v0.rs | 5 +++++ 8 files changed, 25 insertions(+) diff --git a/stacks-signer/src/chainstate.rs b/stacks-signer/src/chainstate.rs index 0f51c05fc5..a753d95b73 100644 --- a/stacks-signer/src/chainstate.rs +++ b/stacks-signer/src/chainstate.rs @@ -141,6 +141,8 @@ pub struct ProposalEvalConfig { /// Time following the last block of the previous tenure's global acceptance that a signer will consider an attempt by /// the new miner to reorg it as valid towards miner activity pub reorg_attempts_activity_timeout: Duration, + /// Time to wait before submitting a block proposal to the stacks-node + pub proposal_wait_time: Duration, } impl From<&SignerConfig> for ProposalEvalConfig { @@ -152,6 +154,7 @@ impl From<&SignerConfig> for ProposalEvalConfig { tenure_idle_timeout: value.tenure_idle_timeout, reorg_attempts_activity_timeout: value.reorg_attempts_activity_timeout, tenure_idle_timeout_buffer: value.tenure_idle_timeout_buffer, + proposal_wait_time: value.proposal_wait_time, } } } diff --git a/stacks-signer/src/client/mod.rs b/stacks-signer/src/client/mod.rs index 59b8309d53..42460cd815 100644 --- a/stacks-signer/src/client/mod.rs +++ b/stacks-signer/src/client/mod.rs @@ -418,6 +418,7 @@ pub(crate) mod tests { tenure_idle_timeout_buffer: config.tenure_idle_timeout_buffer, block_proposal_max_age_secs: config.block_proposal_max_age_secs, reorg_attempts_activity_timeout: config.reorg_attempts_activity_timeout, + proposal_wait_time: config.proposal_wait_time, } } diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index 4b4c990c85..9897858535 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -175,6 +175,8 @@ pub struct SignerConfig { pub reorg_attempts_activity_timeout: Duration, /// The running mode for the signer (dry-run or normal) pub signer_mode: SignerConfigMode, + /// Time to wait before submitting a block proposal to the stacks-node + pub proposal_wait_time: Duration, } /// The parsed configuration for the signer @@ -221,6 +223,8 @@ pub struct GlobalConfig { /// Time following the last block of the previous tenure's global acceptance that a signer will consider an attempt by /// the new miner to reorg it as valid towards miner activity pub reorg_attempts_activity_timeout: Duration, + /// Time to wait before submitting a block proposal to the stacks-node + pub proposal_wait_time: Duration, /// Is this signer binary going to be running in dry-run mode? pub dry_run: bool, } @@ -268,6 +272,8 @@ struct RawConfigFile { /// Time (in millisecs) following a block's global acceptance that a signer will consider an attempt by a miner /// to reorg the block as valid towards miner activity pub reorg_attempts_activity_timeout_ms: Option, + /// Time to wait (in millisecs) before submitting a block proposal to the stacks-node + pub proposal_wait_time_ms: Option, /// Is this signer binary going to be running in dry-run mode? pub dry_run: Option, } @@ -385,6 +391,8 @@ impl TryFrom for GlobalConfig { .unwrap_or(DEFAULT_TENURE_IDLE_TIMEOUT_BUFFER_SECS), ); + let proposal_wait_time = Duration::from_millis(raw_data.proposal_wait_time_ms.unwrap_or(0)); + Ok(Self { node_host: raw_data.node_host, endpoint, @@ -405,6 +413,7 @@ impl TryFrom for GlobalConfig { reorg_attempts_activity_timeout, dry_run, tenure_idle_timeout_buffer, + proposal_wait_time, }) } } diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 0bb8cd651b..883416ae8a 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -320,6 +320,7 @@ impl, T: StacksMessageCodec + Clone + Send + Debug> RunLo tenure_idle_timeout_buffer: self.config.tenure_idle_timeout_buffer, block_proposal_max_age_secs: self.config.block_proposal_max_age_secs, reorg_attempts_activity_timeout: self.config.reorg_attempts_activity_timeout, + proposal_wait_time: self.config.proposal_wait_time, })) } diff --git a/stacks-signer/src/tests/chainstate.rs b/stacks-signer/src/tests/chainstate.rs index 17450501c6..c2e7b9b12d 100644 --- a/stacks-signer/src/tests/chainstate.rs +++ b/stacks-signer/src/tests/chainstate.rs @@ -91,6 +91,7 @@ fn setup_test_environment( tenure_idle_timeout: Duration::from_secs(300), tenure_idle_timeout_buffer: Duration::from_secs(2), reorg_attempts_activity_timeout: Duration::from_secs(3), + proposal_wait_time: Duration::from_secs(0), }, }; diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 4442df6d67..9648a86d0f 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -17,6 +17,7 @@ use std::fmt::Debug; use std::sync::mpsc::Sender; #[cfg(any(test, feature = "testing"))] use std::sync::LazyLock; +use std::thread; use std::time::{Duration, Instant}; use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader}; @@ -1407,6 +1408,7 @@ impl Signer { /// is busy with a previous request. fn submit_block_for_validation(&mut self, stacks_client: &StacksClient, block: &NakamotoBlock) { let signer_signature_hash = block.header.signer_signature_hash(); + thread::sleep(self.proposal_config.proposal_wait_time); match stacks_client.submit_block_for_validation(block.clone()) { Ok(_) => { self.submitted_block_proposal = Some((signer_signature_hash, Instant::now())); diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index b3472fad66..edc269f4cc 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -6577,6 +6577,7 @@ fn signer_chainstate() { // this config disallows any reorg due to poorly timed block commits let proposal_conf = ProposalEvalConfig { + proposal_wait_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -6703,6 +6704,7 @@ fn signer_chainstate() { // this config disallows any reorg due to poorly timed block commits let proposal_conf = ProposalEvalConfig { + proposal_wait_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -6780,6 +6782,7 @@ fn signer_chainstate() { // this config disallows any reorg due to poorly timed block commits let proposal_conf = ProposalEvalConfig { + proposal_wait_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index ee17e04941..2b2094a8d5 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -1467,6 +1467,7 @@ fn block_proposal_rejection() { info!("------------------------- Send Block Proposal To Signers -------------------------"); let proposal_conf = ProposalEvalConfig { + proposal_wait_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -7326,6 +7327,7 @@ fn block_validation_response_timeout() { info!("------------------------- Propose Another Block Before Hitting the Timeout -------------------------"); let proposal_conf = ProposalEvalConfig { + proposal_wait_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), tenure_last_block_proposal_timeout: Duration::from_secs(30), block_proposal_timeout: Duration::from_secs(100), @@ -7614,6 +7616,7 @@ fn block_validation_pending_table() { info!("----- Proposing a concurrent block -----"); let proposal_conf = ProposalEvalConfig { + proposal_wait_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -8902,6 +8905,7 @@ fn incoming_signers_ignore_block_proposals() { no_next_signer_messages(); let proposal_conf = ProposalEvalConfig { + proposal_wait_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -9082,6 +9086,7 @@ fn outgoing_signers_ignore_block_proposals() { old_signers_ignore_block_proposals(new_signature_hash); let proposal_conf = ProposalEvalConfig { + proposal_wait_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), From 96dcd3fe79b8f2e7669f36f82f03481da81b460b Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Mon, 21 Apr 2025 16:03:47 -0500 Subject: [PATCH 2/6] feat: check if a proposals parent has been processed before submit --- stacks-signer/src/config.rs | 16 +- stacks-signer/src/lib.rs | 2 + stacks-signer/src/runloop.rs | 13 + stacks-signer/src/signerdb.rs | 35 ++- stacks-signer/src/v0/signer.rs | 172 ++++++++++-- testnet/stacks-node/src/tests/signer/v0.rs | 297 ++++++++++++++++++++- 6 files changed, 496 insertions(+), 39 deletions(-) diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index 9897858535..e0065d7cbb 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -45,6 +45,10 @@ const DEFAULT_REORG_ATTEMPTS_ACTIVITY_TIMEOUT_MS: u64 = 200_000; /// Default number of seconds to add to the tenure extend time, after computing the idle timeout, /// to allow for clock skew between the signer and the miner const DEFAULT_TENURE_IDLE_TIMEOUT_BUFFER_SECS: u64 = 2; +/// Default time (in ms) to wait before submitting a proposal if we +/// cannot determine that our stacks-node has processed the parent +/// block +const DEFAULT_PROPOSAL_WAIT_TIME_MS: u64 = 15_000; #[derive(thiserror::Error, Debug)] /// An error occurred parsing the provided configuration @@ -175,7 +179,8 @@ pub struct SignerConfig { pub reorg_attempts_activity_timeout: Duration, /// The running mode for the signer (dry-run or normal) pub signer_mode: SignerConfigMode, - /// Time to wait before submitting a block proposal to the stacks-node + /// Time to wait before submitting a block proposal to the stacks-node if we cannot + /// determine that the stacks-node has processed the parent pub proposal_wait_time: Duration, } @@ -223,7 +228,8 @@ pub struct GlobalConfig { /// Time following the last block of the previous tenure's global acceptance that a signer will consider an attempt by /// the new miner to reorg it as valid towards miner activity pub reorg_attempts_activity_timeout: Duration, - /// Time to wait before submitting a block proposal to the stacks-node + /// Time to wait before submitting a block proposal to the stacks-node if we cannot + /// determine that the stacks-node has processed the parent pub proposal_wait_time: Duration, /// Is this signer binary going to be running in dry-run mode? pub dry_run: bool, @@ -391,7 +397,11 @@ impl TryFrom for GlobalConfig { .unwrap_or(DEFAULT_TENURE_IDLE_TIMEOUT_BUFFER_SECS), ); - let proposal_wait_time = Duration::from_millis(raw_data.proposal_wait_time_ms.unwrap_or(0)); + let proposal_wait_time = Duration::from_millis( + raw_data + .proposal_wait_time_ms + .unwrap_or(DEFAULT_PROPOSAL_WAIT_TIME_MS), + ); Ok(Self { node_host: raw_data.node_host, diff --git a/stacks-signer/src/lib.rs b/stacks-signer/src/lib.rs index 1c69132e79..ba2fe31daf 100644 --- a/stacks-signer/src/lib.rs +++ b/stacks-signer/src/lib.rs @@ -78,6 +78,8 @@ pub trait Signer: Debug + Display { fn has_unprocessed_blocks(&self) -> bool; /// Get a reference to the local state machine of the signer fn get_local_state_machine(&self) -> &LocalStateMachine; + /// Get a reference to the local state machine of the signer + fn get_pending_proposals_count(&self) -> u64; } /// A wrapper around the running signer type for the signer diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 883416ae8a..828ffc2faa 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -56,6 +56,8 @@ pub struct StateInfo { /// The local state machines for the running signers /// as a pair of (reward-cycle, state-machine) pub signer_state_machines: Vec<(u64, Option)>, + /// The number of pending block proposals for this signer + pub pending_proposals_count: u64, } /// The signer result that can be sent across threads @@ -524,6 +526,17 @@ impl, T: StacksMessageCodec + Clone + Send + Debug> ) }) .collect(), + pending_proposals_count: self + .stacks_signers + .values() + .find_map(|signer| { + if let ConfiguredSigner::RegisteredSigner(signer) = signer { + Some(signer.get_pending_proposals_count()) + } else { + None + } + }) + .unwrap_or(0), }; info!("Signer status check requested: {state_info:?}"); diff --git a/stacks-signer/src/signerdb.rs b/stacks-signer/src/signerdb.rs index 1be7a40f40..1191f03285 100644 --- a/stacks-signer/src/signerdb.rs +++ b/stacks-signer/src/signerdb.rs @@ -1180,12 +1180,18 @@ impl SignerDb { /// If found, remove it from the pending table. pub fn get_and_remove_pending_block_validation( &self, - ) -> Result, DBError> { - let qry = "DELETE FROM block_validations_pending WHERE signer_signature_hash = (SELECT signer_signature_hash FROM block_validations_pending ORDER BY added_time ASC LIMIT 1) RETURNING signer_signature_hash"; + ) -> Result, DBError> { + let qry = "DELETE FROM block_validations_pending WHERE signer_signature_hash = (SELECT signer_signature_hash FROM block_validations_pending ORDER BY added_time ASC LIMIT 1) RETURNING signer_signature_hash, added_time"; let args = params![]; let mut stmt = self.db.prepare(qry)?; - let sighash: Option = stmt.query_row(args, |row| row.get(0)).optional()?; - Ok(sighash.and_then(|sighash| Sha512Trunc256Sum::from_hex(&sighash).ok())) + let result: Option<(String, i64)> = stmt + .query_row(args, |row| Ok((row.get(0)?, row.get(1)?))) + .optional()?; + Ok(result.and_then(|(sighash, ts_i64)| { + let signer_sighash = Sha512Trunc256Sum::from_hex(&sighash).ok()?; + let ts = u64::try_from(ts_i64).ok()?; + Some((signer_sighash, ts)) + })) } /// Remove a pending block validation @@ -2093,20 +2099,29 @@ mod tests { db.insert_pending_block_validation(&Sha512Trunc256Sum([0x03; 32]), 3000) .unwrap(); - let pending_hash = db.get_and_remove_pending_block_validation().unwrap(); - assert_eq!(pending_hash, Some(Sha512Trunc256Sum([0x01; 32]))); + let (pending_hash, _) = db + .get_and_remove_pending_block_validation() + .unwrap() + .unwrap(); + assert_eq!(pending_hash, Sha512Trunc256Sum([0x01; 32])); let pendings = db.get_all_pending_block_validations().unwrap(); assert_eq!(pendings.len(), 2); - let pending_hash = db.get_and_remove_pending_block_validation().unwrap(); - assert_eq!(pending_hash, Some(Sha512Trunc256Sum([0x02; 32]))); + let (pending_hash, _) = db + .get_and_remove_pending_block_validation() + .unwrap() + .unwrap(); + assert_eq!(pending_hash, Sha512Trunc256Sum([0x02; 32])); let pendings = db.get_all_pending_block_validations().unwrap(); assert_eq!(pendings.len(), 1); - let pending_hash = db.get_and_remove_pending_block_validation().unwrap(); - assert_eq!(pending_hash, Some(Sha512Trunc256Sum([0x03; 32]))); + let (pending_hash, _) = db + .get_and_remove_pending_block_validation() + .unwrap() + .unwrap(); + assert_eq!(pending_hash, Sha512Trunc256Sum([0x03; 32])); let pendings = db.get_all_pending_block_validations().unwrap(); assert!(pendings.is_empty()); diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 9648a86d0f..2738887abc 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -17,7 +17,6 @@ use std::fmt::Debug; use std::sync::mpsc::Sender; #[cfg(any(test, feature = "testing"))] use std::sync::LazyLock; -use std::thread; use std::time::{Duration, Instant}; use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader}; @@ -26,9 +25,9 @@ use blockstack_lib::net::api::postblock_proposal::{ TOO_MANY_REQUESTS_STATUS, }; use blockstack_lib::util_lib::db::Error as DBError; -use clarity::types::chainstate::StacksPrivateKey; #[cfg(any(test, feature = "testing"))] use clarity::types::chainstate::StacksPublicKey; +use clarity::types::chainstate::{StacksBlockId, StacksPrivateKey}; use clarity::types::{PrivateKey, StacksEpochId}; use clarity::util::hash::{MerkleHashFunc, Sha512Trunc256Sum}; use clarity::util::secp256k1::Secp256k1PublicKey; @@ -74,6 +73,12 @@ pub enum SignerMode { }, } +/// Track N most recently processed block identifiers +pub struct RecentlyProcessedBlocks { + blocks: Vec, + write_head: usize, +} + /// The stacks signer registered for the reward cycle #[derive(Debug)] pub struct Signer { @@ -110,6 +115,8 @@ pub struct Signer { pub block_proposal_max_age_secs: u64, /// The signer's local state machine used in signer set agreement pub local_state_machine: LocalStateMachine, + /// Cache of stacks block IDs for blocks recently processed by our stacks-node + recently_processed: RecentlyProcessedBlocks<100>, } impl std::fmt::Display for SignerMode { @@ -127,6 +134,44 @@ impl std::fmt::Display for Signer { } } +impl std::fmt::Debug for RecentlyProcessedBlocks { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "RecentlyProcessed({:?})", self.blocks) + } +} + +impl RecentlyProcessedBlocks { + /// Construct a new recently processed blocks cache + pub fn new() -> Self { + Self { + blocks: Vec::with_capacity(N), + write_head: 0, + } + } + /// Is `block` known to have been processed by our stacks-node? + pub fn is_processed(&self, block: &StacksBlockId) -> bool { + self.blocks.contains(block) + } + + /// Add a block that we know has been processed by our stacks-node + pub fn add_block(&mut self, block: StacksBlockId) { + if self.blocks.len() < N { + self.blocks.push(block); + return; + } + let Some(location) = self.blocks.get_mut(self.write_head) else { + warn!( + "Failed to cache processing information about {block}, write_head {} was improperly set for cache size {N} with blocks length {}", + self.write_head, + self.blocks.len() + ); + return; + }; + *location = block; + self.write_head = (self.write_head + 1) % self.blocks.len(); + } +} + impl SignerTrait for Signer { /// Create a new signer from the given configuration fn new(stacks_client: &StacksClient, signer_config: SignerConfig) -> Self { @@ -162,6 +207,7 @@ impl SignerTrait for Signer { block_proposal_validation_timeout: signer_config.block_proposal_validation_timeout, block_proposal_max_age_secs: signer_config.block_proposal_max_age_secs, local_state_machine: signer_state, + recently_processed: RecentlyProcessedBlocks::new(), } } @@ -195,6 +241,7 @@ impl SignerTrait for Signer { return; } self.check_submitted_block_proposal(); + self.check_pending_block_validations(stacks_client); debug!("{self}: Processing event: {event:?}"); let Some(event) = event else { // No event. Do nothing. @@ -353,6 +400,7 @@ impl SignerTrait for Signer { debug!("{self}: received a new block event for a pre-nakamoto block, no processing necessary"); return; }; + self.recently_processed.add_block(*block_id); debug!( "{self}: Received a new block event."; "block_id" => %block_id, @@ -383,6 +431,7 @@ impl SignerTrait for Signer { } } } + if prior_state != self.local_state_machine { self.local_state_machine .send_signer_update_message(&mut self.stackerdb); @@ -402,6 +451,19 @@ impl SignerTrait for Signer { fn get_local_state_machine(&self) -> &LocalStateMachine { &self.local_state_machine } + + #[cfg(not(any(test, feature = "testing")))] + fn get_pending_proposals_count(&self) -> u64 { + 0 + } + + #[cfg(any(test, feature = "testing"))] + fn get_pending_proposals_count(&self) -> u64 { + self.signer_db + .get_all_pending_block_validations() + .map(|results| u64::try_from(results.len()).unwrap()) + .unwrap_or(0) + } } impl Signer { @@ -460,6 +522,31 @@ impl Signer { ) } + /// Check some heuristics to see if our stacks-node has processed the parent of `block`. + /// Note: this can be wrong in both directions. It may return false for some blocks that + /// have been processed, and it may return true for some blocks that have not been processed. + /// The caller should not depend on this being 100% accurate. + fn maybe_processed_parent(&self, client: &StacksClient, block: &NakamotoBlock) -> bool { + let parent_block_id = &block.header.parent_block_id; + if self.recently_processed.is_processed(parent_block_id) { + return true; + } + let Ok(peer_info) = client.get_peer_info().inspect_err(|e| { + warn!( + "Failed to fetch stacks-node peer info, assuming block not processed yet"; + "error" => ?e + ) + }) else { + return false; + }; + + if peer_info.stacks_tip_height >= block.header.chain_length.saturating_sub(1) { + true + } else { + false + } + } + /// Check if block should be rejected based on sortition state /// Will return a BlockResponse::Rejection if the block is invalid, none otherwise. fn check_block_against_sortition_state( @@ -679,7 +766,11 @@ impl Signer { #[cfg(any(test, feature = "testing"))] self.test_stall_block_validation_submission(); - self.submit_block_for_validation(stacks_client, &block_proposal.block); + self.submit_block_for_validation( + stacks_client, + &block_proposal.block, + get_epoch_time_secs(), + ); } else { // Still store the block but log we can't submit it for validation. We may receive enough signatures/rejections // from other signers to push the proposed block into a global rejection/acceptance regardless of our participation. @@ -1009,24 +1100,38 @@ impl Signer { }; // Check if there is a pending block validation that we need to submit to the node - match self.signer_db.get_and_remove_pending_block_validation() { - Ok(Some(signer_sig_hash)) => { - info!("{self}: Found a pending block validation: {signer_sig_hash:?}"); - match self.signer_db.block_lookup(&signer_sig_hash) { - Ok(Some(block_info)) => { - self.submit_block_for_validation(stacks_client, &block_info.block); - } - Ok(None) => { - // This should never happen - error!( - "{self}: Pending block validation not found in DB: {signer_sig_hash:?}" - ); - } - Err(e) => error!("{self}: Failed to get block info: {e:?}"), + self.check_pending_block_validations(stacks_client); + } + + /// Check if we can submit a block validation, and do so if we have pending block proposals + fn check_pending_block_validations(&mut self, stacks_client: &StacksClient) { + // if we're already waiting on a submitted block proposal, we cannot submit yet. + if self.submitted_block_proposal.is_some() { + return; + } + + let (signer_sig_hash, insert_ts) = + match self.signer_db.get_and_remove_pending_block_validation() { + Ok(Some(x)) => x, + Ok(None) => { + return; } + Err(e) => { + warn!("{self}: Failed to get pending block validation: {e:?}"); + return; + } + }; + + info!("{self}: Found a pending block validation: {signer_sig_hash:?}"); + match self.signer_db.block_lookup(&signer_sig_hash) { + Ok(Some(block_info)) => { + self.submit_block_for_validation(stacks_client, &block_info.block, insert_ts); + } + Ok(None) => { + // This should never happen + error!("{self}: Pending block validation not found in DB: {signer_sig_hash:?}"); } - Ok(None) => {} - Err(e) => warn!("{self}: Failed to get pending block validation: {e:?}"), + Err(e) => error!("{self}: Failed to get block info: {e:?}"), } } @@ -1406,9 +1511,29 @@ impl Signer { /// Submit a block for validation, and mark it as pending if the node /// is busy with a previous request. - fn submit_block_for_validation(&mut self, stacks_client: &StacksClient, block: &NakamotoBlock) { + fn submit_block_for_validation( + &mut self, + stacks_client: &StacksClient, + block: &NakamotoBlock, + added_epoch_time: u64, + ) { let signer_signature_hash = block.header.signer_signature_hash(); - thread::sleep(self.proposal_config.proposal_wait_time); + if !self.maybe_processed_parent(stacks_client, block) { + let time_elapsed = get_epoch_time_secs().saturating_sub(added_epoch_time); + if Duration::from_secs(time_elapsed) < self.proposal_config.proposal_wait_time { + info!("{self}: Have not processed parent of block proposal yet, inserting pending block validation and will try again later"; + "signer_signature_hash" => %signer_signature_hash, + ); + self.signer_db + .insert_pending_block_validation(&signer_signature_hash, added_epoch_time) + .unwrap_or_else(|e| { + warn!("{self}: Failed to insert pending block validation: {e:?}") + }); + return; + } else { + debug!("{self}: Cannot confirm that we have processed parent, but we've waiting proposal_wait_time, will submit proposal"); + } + } match stacks_client.submit_block_for_validation(block.clone()) { Ok(_) => { self.submitted_block_proposal = Some((signer_signature_hash, Instant::now())); @@ -1419,10 +1544,7 @@ impl Signer { "signer_signature_hash" => %signer_signature_hash, ); self.signer_db - .insert_pending_block_validation( - &signer_signature_hash, - get_epoch_time_secs(), - ) + .insert_pending_block_validation(&signer_signature_hash, added_epoch_time) .unwrap_or_else(|e| { warn!("{self}: Failed to insert pending block validation: {e:?}") }); diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 2b2094a8d5..0bc4535dda 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -56,7 +56,7 @@ use stacks::net::api::postblock_proposal::{ BlockValidateResponse, ValidateRejectCode, TEST_VALIDATE_DELAY_DURATION_SECS, TEST_VALIDATE_STALL, }; -use stacks::net::relay::fault_injection::set_ignore_block; +use stacks::net::relay::fault_injection::{clear_ignore_block, set_ignore_block}; use stacks::types::chainstate::{ BlockHeaderHash, StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey, }; @@ -1666,6 +1666,301 @@ fn mine_2_nakamoto_reward_cycles() { signer_test.shutdown(); } +#[test] +#[ignore] +fn revalidate_unknown_parent() { + if env::var("BITCOIND_TEST") != Ok("1".into()) { + return; + } + + let num_signers = 5; + let max_nakamoto_tenures = 30; + let inter_blocks_per_tenure = 5; + + // setup sender + recipient for a test stx transfer + let sender_sk = Secp256k1PrivateKey::random(); + let sender_addr = tests::to_addr(&sender_sk); + let send_amt = 1000; + let send_fee = 180; + + let btc_miner_1_seed = vec![1, 1, 1, 1]; + let btc_miner_2_seed = vec![2, 2, 2, 2]; + let btc_miner_1_pk = Keychain::default(btc_miner_1_seed.clone()).get_pub_key(); + let btc_miner_2_pk = Keychain::default(btc_miner_2_seed.clone()).get_pub_key(); + + let node_1_rpc = gen_random_port(); + let node_1_p2p = gen_random_port(); + let node_2_rpc = gen_random_port(); + let node_2_p2p = gen_random_port(); + + let localhost = "127.0.0.1"; + let node_1_rpc_bind = format!("{localhost}:{node_1_rpc}"); + + // All signers are listening to node 1 + let mut signer_test: SignerTest = SignerTest::new_with_config_modifications( + num_signers, + vec![( + sender_addr, + (send_amt + send_fee) * max_nakamoto_tenures * inter_blocks_per_tenure, + )], + |signer_config| { + signer_config.node_host = node_1_rpc_bind.clone(); + signer_config.first_proposal_burn_block_timing = Duration::from_secs(0); + // rely on actually checking that the block is processed + signer_config.proposal_wait_time = Duration::from_secs(600); + }, + |config| { + config.node.rpc_bind = format!("{localhost}:{node_1_rpc}"); + config.node.p2p_bind = format!("{localhost}:{node_1_p2p}"); + config.node.data_url = format!("http://{localhost}:{node_1_rpc}"); + config.node.p2p_address = format!("{localhost}:{node_1_p2p}"); + config.node.pox_sync_sample_secs = 30; + config.miner.block_commit_delay = Duration::from_secs(0); + + config.node.seed = btc_miner_1_seed.clone(); + config.node.local_peer_seed = btc_miner_1_seed.clone(); + config.burnchain.local_mining_public_key = Some(btc_miner_1_pk.to_hex()); + config.miner.mining_key = Some(Secp256k1PrivateKey::from_seed(&[1])); + + // Increase the reward cycle length to avoid missing a prepare phase + // while we are intentionally forking. + config.burnchain.pox_reward_length = Some(40); + config.burnchain.pox_prepare_length = Some(10); + + // Move epoch 2.5 and 3.0 earlier, so we have more time for the + // test before re-stacking is required. + if let Some(epochs) = config.burnchain.epochs.as_mut() { + epochs[StacksEpochId::Epoch24].end_height = 131; + epochs[StacksEpochId::Epoch25].start_height = 131; + epochs[StacksEpochId::Epoch25].end_height = 166; + epochs[StacksEpochId::Epoch30].start_height = 166; + } else { + panic!("Expected epochs to be set"); + } + }, + Some(vec![btc_miner_1_pk, btc_miner_2_pk]), + None, + ); + + let conf = signer_test.running_nodes.conf.clone(); + let mut conf_node_2 = conf.clone(); + conf_node_2.node.rpc_bind = format!("{localhost}:{node_2_rpc}"); + conf_node_2.node.p2p_bind = format!("{localhost}:{node_2_p2p}"); + conf_node_2.node.data_url = format!("http://{localhost}:{node_2_rpc}"); + conf_node_2.node.p2p_address = format!("{localhost}:{node_2_p2p}"); + conf_node_2.node.seed = btc_miner_2_seed.clone(); + conf_node_2.burnchain.local_mining_public_key = Some(btc_miner_2_pk.to_hex()); + conf_node_2.node.local_peer_seed = btc_miner_2_seed; + conf_node_2.miner.mining_key = Some(Secp256k1PrivateKey::from_seed(&[2])); + conf_node_2.node.miner = true; + conf_node_2.events_observers.clear(); + + let node_1_sk = Secp256k1PrivateKey::from_seed(&conf.node.local_peer_seed); + let node_1_pk = StacksPublicKey::from_private(&node_1_sk); + + conf_node_2.node.working_dir = format!("{}-1", conf_node_2.node.working_dir); + + conf_node_2.node.set_bootstrap_nodes( + format!("{}@{}", &node_1_pk.to_hex(), conf.node.p2p_bind), + conf.burnchain.chain_id, + conf.burnchain.peer_version, + ); + + let mining_pk_1 = StacksPublicKey::from_private(&conf.miner.mining_key.unwrap()); + let mining_pk_2 = StacksPublicKey::from_private(&conf_node_2.miner.mining_key.unwrap()); + let mining_pkh_1 = Hash160::from_node_public_key(&mining_pk_1); + let mining_pkh_2 = Hash160::from_node_public_key(&mining_pk_2); + debug!("The mining key for miner 1 is {mining_pkh_1}"); + debug!("The mining key for miner 2 is {mining_pkh_2}"); + + let http_origin = format!("http://{}", &conf.node.rpc_bind); + + let mut run_loop_2 = boot_nakamoto::BootRunLoop::new(conf_node_2.clone()).unwrap(); + let rl2_coord_channels = run_loop_2.coordinator_channels(); + let run_loop_stopper_2 = run_loop_2.get_termination_switch(); + let Counters { + naka_skip_commit_op: rl2_skip_commit_op, + .. + } = run_loop_2.counters(); + let rl2_counters = run_loop_2.counters(); + let rl1_counters = signer_test.running_nodes.counters.clone(); + + signer_test.boot_to_epoch_3(); + + // Pause block commits from miner 2 to make sure + // miner 1 wins the first block + rl2_skip_commit_op.set(true); + + let run_loop_2_thread = thread::Builder::new() + .name("run_loop_2".into()) + .spawn(move || run_loop_2.start(None, 0)) + .unwrap(); + + wait_for(200, || { + let Some(node_1_info) = get_chain_info_opt(&conf) else { + return Ok(false); + }; + let Some(node_2_info) = get_chain_info_opt(&conf_node_2) else { + return Ok(false); + }; + Ok(node_1_info.stacks_tip_height == node_2_info.stacks_tip_height) + }) + .expect("Timed out waiting for follower to catch up to the miner"); + + info!("------------------------- Reached Epoch 3.0 -------------------------"); + + let rl1_skip_commit_op = signer_test + .running_nodes + .counters + .naka_skip_commit_op + .clone(); + + let sortdb = SortitionDB::open( + &conf.get_burn_db_file_path(), + false, + conf.get_burnchain().pox_constants, + ) + .unwrap(); + + info!("-------- Waiting miner 2 to catch up to miner 1 --------"); + + // Wait for miner 2 to catch up to miner 1 + // (note: use a high timeout to avoid potential failing on github workflow) + wait_for(600, || { + let info_1 = get_chain_info(&conf); + let info_2 = get_chain_info(&conf_node_2); + Ok(info_1.stacks_tip_height == info_2.stacks_tip_height) + }) + .expect("Timed out waiting for miner 2 to catch up to miner 1"); + + info!("-------- Miner 2 caught up to miner 1 --------"); + + let info_before = get_chain_info(&conf); + + info!("-------- Miner 1 starting next tenure --------"); + + wait_for(60, || { + Ok(rl1_counters.naka_submitted_commit_last_burn_height.get() + >= info_before.burn_block_height) + }) + .unwrap(); + info!("-------- Blocking Miner 1 so that Miner 2 will win the next next tenure --------"); + rl1_skip_commit_op.set(true); + + // Mine the first block + signer_test.mine_bitcoin_block(); + signer_test.check_signer_states_normal(); + + let tip_sn = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()).unwrap(); + assert_eq!(tip_sn.miner_pk_hash, Some(mining_pkh_1)); + + info!("------- Unblocking Miner 2 ------"); + rl2_skip_commit_op.set(false); + wait_for(60, || { + Ok(rl2_counters.naka_submitted_commit_last_burn_height.get() + > info_before.burn_block_height + && rl2_counters.naka_submitted_commit_last_stacks_tip.get() + > info_before.stacks_tip_height) + }) + .unwrap(); + let peer_info_before = signer_test.get_peer_info(); + info!("------- Miner 2 wins first tenure ------"); + signer_test.mine_bitcoin_block(); + signer_test.check_signer_states_normal(); + let tip_sn = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()).unwrap(); + assert_eq!(tip_sn.miner_pk_hash, Some(mining_pkh_2)); + + // Setup miner 1 to ignore a block in this tenure + let ignore_block = peer_info_before.stacks_tip_height + 2; + set_ignore_block(ignore_block, &conf.node.working_dir); + + // wait for the tenure to start (i.e., the tenure change block to be produced, + // which should be mined and not ignored) + wait_for(60, || { + Ok(signer_test.get_peer_info().stacks_tip_height == ignore_block - 1) + }) + .unwrap(); + + info!( + "Mining 1st interim block in Miner 2's first tenure"; + ); + + let (_, sender_nonce) = signer_test + .submit_transfer_tx(&sender_sk, send_fee, send_amt) + .unwrap(); + + wait_for(60, || { + let http_origin = &conf_node_2.node.data_url; + Ok(get_account(http_origin, &sender_addr).nonce > sender_nonce) + }) + .unwrap(); + + // should not have updated yet in node 1 + assert_eq!(get_account(&http_origin, &sender_addr).nonce, sender_nonce); + + info!( + "Mining 2nd interim block in Miner 2's first tenure"; + ); + + let sender_nonce = get_account(&conf_node_2.node.data_url, &sender_addr).nonce; + let recipient = PrincipalData::from(StacksAddress::burn_address(false)); + let transfer_tx = make_stacks_transfer( + &sender_sk, + sender_nonce, + send_fee, + conf.burnchain.chain_id, + &recipient, + send_amt, + ); + + // should be no pending proposals yet. + signer_test + .get_all_states() + .iter() + .for_each(|state| assert_eq!(state.pending_proposals_count, 0)); + + submit_tx_fallible(&http_origin, &transfer_tx).unwrap(); + + wait_for(60, || { + Ok(signer_test.get_all_states().iter().all(|state| { + info!( + "State: pending_proposal_count = {}", + state.pending_proposals_count + ); + state.pending_proposals_count == 1 + })) + }) + .unwrap(); + + // sleep to make sure that the pending proposal isn't just temporarily pending + thread::sleep(Duration::from_secs(5)); + + signer_test + .get_all_states() + .iter() + .for_each(|state| assert_eq!(state.pending_proposals_count, 1)); + assert_eq!( + get_account(&http_origin, &sender_addr).nonce, + sender_nonce - 1 + ); + + // clear the block ignore and make sure that the proposal gets processed by miner 1 + clear_ignore_block(); + + wait_for(60, || { + Ok(get_account(&http_origin, &sender_addr).nonce > sender_nonce) + }) + .unwrap(); + + rl2_coord_channels + .lock() + .expect("Mutex poisoned") + .stop_chains_coordinator(); + run_loop_stopper_2.store(false, Ordering::SeqCst); + run_loop_2_thread.join().unwrap(); + signer_test.shutdown(); +} + #[test] #[ignore] /// This test is a regression test for issue #5858 in which the signer runloop From c68e1deab41e48d1046cb49dfd2f0cdf3b908b88 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 22 Apr 2025 10:06:10 -0500 Subject: [PATCH 3/6] chore: update changelog, rename config option --- stacks-signer/CHANGELOG.md | 6 ++++++ stacks-signer/src/chainstate.rs | 4 ++-- stacks-signer/src/client/mod.rs | 2 +- stacks-signer/src/config.rs | 16 ++++++++-------- stacks-signer/src/runloop.rs | 2 +- stacks-signer/src/tests/chainstate.rs | 2 +- stacks-signer/src/v0/signer.rs | 6 ++++-- .../src/tests/nakamoto_integrations.rs | 6 +++--- testnet/stacks-node/src/tests/signer/v0.rs | 12 ++++++------ 9 files changed, 32 insertions(+), 24 deletions(-) diff --git a/stacks-signer/CHANGELOG.md b/stacks-signer/CHANGELOG.md index ca471474a4..3c64b6077d 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). +## [3.1.0.0.8.1] + +### Added + +- The signer will now check if their associated stacks-node has processed the parent block for a block proposal before submitting that block proposal. If it cannot confirm that the parent block has been processed, it waits a default time of 15s, configurable via `proposal_wait_for_parent_time_secs` in the signer config.toml. + ## [3.1.0.0.8.0] ### Changed diff --git a/stacks-signer/src/chainstate.rs b/stacks-signer/src/chainstate.rs index a753d95b73..614a4be4ae 100644 --- a/stacks-signer/src/chainstate.rs +++ b/stacks-signer/src/chainstate.rs @@ -142,7 +142,7 @@ pub struct ProposalEvalConfig { /// the new miner to reorg it as valid towards miner activity pub reorg_attempts_activity_timeout: Duration, /// Time to wait before submitting a block proposal to the stacks-node - pub proposal_wait_time: Duration, + pub proposal_wait_for_parent_time: Duration, } impl From<&SignerConfig> for ProposalEvalConfig { @@ -154,7 +154,7 @@ impl From<&SignerConfig> for ProposalEvalConfig { tenure_idle_timeout: value.tenure_idle_timeout, reorg_attempts_activity_timeout: value.reorg_attempts_activity_timeout, tenure_idle_timeout_buffer: value.tenure_idle_timeout_buffer, - proposal_wait_time: value.proposal_wait_time, + proposal_wait_for_parent_time: value.proposal_wait_for_parent_time, } } } diff --git a/stacks-signer/src/client/mod.rs b/stacks-signer/src/client/mod.rs index 42460cd815..c1f6d5e1d6 100644 --- a/stacks-signer/src/client/mod.rs +++ b/stacks-signer/src/client/mod.rs @@ -418,7 +418,7 @@ pub(crate) mod tests { tenure_idle_timeout_buffer: config.tenure_idle_timeout_buffer, block_proposal_max_age_secs: config.block_proposal_max_age_secs, reorg_attempts_activity_timeout: config.reorg_attempts_activity_timeout, - proposal_wait_time: config.proposal_wait_time, + proposal_wait_for_parent_time: config.proposal_wait_for_parent_time, } } diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index e0065d7cbb..e642288593 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -48,7 +48,7 @@ const DEFAULT_TENURE_IDLE_TIMEOUT_BUFFER_SECS: u64 = 2; /// Default time (in ms) to wait before submitting a proposal if we /// cannot determine that our stacks-node has processed the parent /// block -const DEFAULT_PROPOSAL_WAIT_TIME_MS: u64 = 15_000; +const DEFAULT_PROPOSAL_WAIT_TIME_FOR_PARENT_SECS: u64 = 15; #[derive(thiserror::Error, Debug)] /// An error occurred parsing the provided configuration @@ -181,7 +181,7 @@ pub struct SignerConfig { pub signer_mode: SignerConfigMode, /// Time to wait before submitting a block proposal to the stacks-node if we cannot /// determine that the stacks-node has processed the parent - pub proposal_wait_time: Duration, + pub proposal_wait_for_parent_time: Duration, } /// The parsed configuration for the signer @@ -230,7 +230,7 @@ pub struct GlobalConfig { pub reorg_attempts_activity_timeout: Duration, /// Time to wait before submitting a block proposal to the stacks-node if we cannot /// determine that the stacks-node has processed the parent - pub proposal_wait_time: Duration, + pub proposal_wait_for_parent_time: Duration, /// Is this signer binary going to be running in dry-run mode? pub dry_run: bool, } @@ -279,7 +279,7 @@ struct RawConfigFile { /// to reorg the block as valid towards miner activity pub reorg_attempts_activity_timeout_ms: Option, /// Time to wait (in millisecs) before submitting a block proposal to the stacks-node - pub proposal_wait_time_ms: Option, + pub proposal_wait_for_parent_time_secs: Option, /// Is this signer binary going to be running in dry-run mode? pub dry_run: Option, } @@ -397,10 +397,10 @@ impl TryFrom for GlobalConfig { .unwrap_or(DEFAULT_TENURE_IDLE_TIMEOUT_BUFFER_SECS), ); - let proposal_wait_time = Duration::from_millis( + let proposal_wait_for_parent_time = Duration::from_secs( raw_data - .proposal_wait_time_ms - .unwrap_or(DEFAULT_PROPOSAL_WAIT_TIME_MS), + .proposal_wait_for_parent_time_secs + .unwrap_or(DEFAULT_PROPOSAL_WAIT_TIME_FOR_PARENT_SECS), ); Ok(Self { @@ -423,7 +423,7 @@ impl TryFrom for GlobalConfig { reorg_attempts_activity_timeout, dry_run, tenure_idle_timeout_buffer, - proposal_wait_time, + proposal_wait_for_parent_time, }) } } diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 828ffc2faa..9526f368a8 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -322,7 +322,7 @@ impl, T: StacksMessageCodec + Clone + Send + Debug> RunLo tenure_idle_timeout_buffer: self.config.tenure_idle_timeout_buffer, block_proposal_max_age_secs: self.config.block_proposal_max_age_secs, reorg_attempts_activity_timeout: self.config.reorg_attempts_activity_timeout, - proposal_wait_time: self.config.proposal_wait_time, + proposal_wait_for_parent_time: self.config.proposal_wait_for_parent_time, })) } diff --git a/stacks-signer/src/tests/chainstate.rs b/stacks-signer/src/tests/chainstate.rs index c2e7b9b12d..422b0c84d4 100644 --- a/stacks-signer/src/tests/chainstate.rs +++ b/stacks-signer/src/tests/chainstate.rs @@ -91,7 +91,7 @@ fn setup_test_environment( tenure_idle_timeout: Duration::from_secs(300), tenure_idle_timeout_buffer: Duration::from_secs(2), reorg_attempts_activity_timeout: Duration::from_secs(3), - proposal_wait_time: Duration::from_secs(0), + proposal_wait_for_parent_time: Duration::from_secs(0), }, }; diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 2738887abc..ddde95509a 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -1520,7 +1520,9 @@ impl Signer { let signer_signature_hash = block.header.signer_signature_hash(); if !self.maybe_processed_parent(stacks_client, block) { let time_elapsed = get_epoch_time_secs().saturating_sub(added_epoch_time); - if Duration::from_secs(time_elapsed) < self.proposal_config.proposal_wait_time { + if Duration::from_secs(time_elapsed) + < self.proposal_config.proposal_wait_for_parent_time + { info!("{self}: Have not processed parent of block proposal yet, inserting pending block validation and will try again later"; "signer_signature_hash" => %signer_signature_hash, ); @@ -1531,7 +1533,7 @@ impl Signer { }); return; } else { - debug!("{self}: Cannot confirm that we have processed parent, but we've waiting proposal_wait_time, will submit proposal"); + debug!("{self}: Cannot confirm that we have processed parent, but we've waiting proposal_wait_for_parent_time, will submit proposal"); } } match stacks_client.submit_block_for_validation(block.clone()) { diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index edc269f4cc..6da10ee7e4 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -6577,7 +6577,7 @@ fn signer_chainstate() { // this config disallows any reorg due to poorly timed block commits let proposal_conf = ProposalEvalConfig { - proposal_wait_time: Duration::from_secs(0), + proposal_wait_for_parent_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -6704,7 +6704,7 @@ fn signer_chainstate() { // this config disallows any reorg due to poorly timed block commits let proposal_conf = ProposalEvalConfig { - proposal_wait_time: Duration::from_secs(0), + proposal_wait_for_parent_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -6782,7 +6782,7 @@ fn signer_chainstate() { // this config disallows any reorg due to poorly timed block commits let proposal_conf = ProposalEvalConfig { - proposal_wait_time: Duration::from_secs(0), + proposal_wait_for_parent_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 0bc4535dda..dcfc8c129e 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -1467,7 +1467,7 @@ fn block_proposal_rejection() { info!("------------------------- Send Block Proposal To Signers -------------------------"); let proposal_conf = ProposalEvalConfig { - proposal_wait_time: Duration::from_secs(0), + proposal_wait_for_parent_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -1707,7 +1707,7 @@ fn revalidate_unknown_parent() { signer_config.node_host = node_1_rpc_bind.clone(); signer_config.first_proposal_burn_block_timing = Duration::from_secs(0); // rely on actually checking that the block is processed - signer_config.proposal_wait_time = Duration::from_secs(600); + signer_config.proposal_wait_for_parent_time = Duration::from_secs(600); }, |config| { config.node.rpc_bind = format!("{localhost}:{node_1_rpc}"); @@ -7622,7 +7622,7 @@ fn block_validation_response_timeout() { info!("------------------------- Propose Another Block Before Hitting the Timeout -------------------------"); let proposal_conf = ProposalEvalConfig { - proposal_wait_time: Duration::from_secs(0), + proposal_wait_for_parent_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), tenure_last_block_proposal_timeout: Duration::from_secs(30), block_proposal_timeout: Duration::from_secs(100), @@ -7911,7 +7911,7 @@ fn block_validation_pending_table() { info!("----- Proposing a concurrent block -----"); let proposal_conf = ProposalEvalConfig { - proposal_wait_time: Duration::from_secs(0), + proposal_wait_for_parent_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -9200,7 +9200,7 @@ fn incoming_signers_ignore_block_proposals() { no_next_signer_messages(); let proposal_conf = ProposalEvalConfig { - proposal_wait_time: Duration::from_secs(0), + proposal_wait_for_parent_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), @@ -9381,7 +9381,7 @@ fn outgoing_signers_ignore_block_proposals() { old_signers_ignore_block_proposals(new_signature_hash); let proposal_conf = ProposalEvalConfig { - proposal_wait_time: Duration::from_secs(0), + proposal_wait_for_parent_time: Duration::from_secs(0), first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), tenure_last_block_proposal_timeout: Duration::from_secs(30), From ac370c06ed04b318dc754677e01190539485a15b Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 22 Apr 2025 12:17:35 -0500 Subject: [PATCH 4/6] address PR reviews --- stacks-signer/CHANGELOG.md | 2 +- stacks-signer/src/lib.rs | 2 +- stacks-signer/src/v0/signer.rs | 10 ++++------ 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/stacks-signer/CHANGELOG.md b/stacks-signer/CHANGELOG.md index 3c64b6077d..4b5639c770 100644 --- a/stacks-signer/CHANGELOG.md +++ b/stacks-signer/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE ### Added -- The signer will now check if their associated stacks-node has processed the parent block for a block proposal before submitting that block proposal. If it cannot confirm that the parent block has been processed, it waits a default time of 15s, configurable via `proposal_wait_for_parent_time_secs` in the signer config.toml. +- The signer will now check if their associated stacks-node has processed the parent block for a block proposal before submitting that block proposal. If it cannot confirm that the parent block has been processed, it waits a default time of 15s before submitting, configurable via `proposal_wait_for_parent_time_secs` in the signer config.toml. ## [3.1.0.0.8.0] diff --git a/stacks-signer/src/lib.rs b/stacks-signer/src/lib.rs index ba2fe31daf..71727653d8 100644 --- a/stacks-signer/src/lib.rs +++ b/stacks-signer/src/lib.rs @@ -78,7 +78,7 @@ pub trait Signer: Debug + Display { fn has_unprocessed_blocks(&self) -> bool; /// Get a reference to the local state machine of the signer fn get_local_state_machine(&self) -> &LocalStateMachine; - /// Get a reference to the local state machine of the signer + /// Get the number of pending block proposals fn get_pending_proposals_count(&self) -> u64; } diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index ddde95509a..43ce180e64 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -540,11 +540,9 @@ impl Signer { return false; }; - if peer_info.stacks_tip_height >= block.header.chain_length.saturating_sub(1) { - true - } else { - false - } + // if our stacks node has processed block height >= block proposal's parent + // return true + peer_info.stacks_tip_height >= block.header.chain_length.saturating_sub(1) } /// Check if block should be rejected based on sortition state @@ -1533,7 +1531,7 @@ impl Signer { }); return; } else { - debug!("{self}: Cannot confirm that we have processed parent, but we've waiting proposal_wait_for_parent_time, will submit proposal"); + debug!("{self}: Cannot confirm that we have processed parent, but we've waited proposal_wait_for_parent_time, will submit proposal"); } } match stacks_client.submit_block_for_validation(block.clone()) { From 615f8b5cc8888c4b010ff539df3f24ba9a4e304b Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 22 Apr 2025 12:59:56 -0500 Subject: [PATCH 5/6] make signer runloop run the "after-event" code on all invocations --- libsigner/src/events.rs | 1 + stacks-signer/src/v0/signer.rs | 153 ++++++++++++++++++--------------- 2 files changed, 86 insertions(+), 68 deletions(-) diff --git a/libsigner/src/events.rs b/libsigner/src/events.rs index ad0583a057..97d19ac407 100644 --- a/libsigner/src/events.rs +++ b/libsigner/src/events.rs @@ -589,6 +589,7 @@ struct BlockEvent { #[serde(with = "prefix_hex")] index_block_hash: StacksBlockId, #[serde(with = "prefix_opt_hex")] + #[serde(default)] signer_signature_hash: Option, #[serde(with = "prefix_hex")] consensus_hash: ConsensusHash, diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 43ce180e64..348023bc2c 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -266,6 +266,91 @@ impl SignerTrait for Signer { .unwrap_or_else(|e| error!("{self}: failed to update local state machine for pending update"; "err" => ?e)); } + self.handle_event_match(stacks_client, sortition_state, event, current_reward_cycle); + + self.check_submitted_block_proposal(); + self.check_pending_block_validations(stacks_client); + + if prior_state != self.local_state_machine { + self.local_state_machine + .send_signer_update_message(&mut self.stackerdb); + } + } + + fn has_unprocessed_blocks(&self) -> bool { + self.signer_db + .has_unprocessed_blocks(self.reward_cycle) + .unwrap_or_else(|e| { + error!("{self}: Failed to check for pending blocks: {e:?}",); + // Assume we have pending blocks to prevent premature cleanup + true + }) + } + + fn get_local_state_machine(&self) -> &LocalStateMachine { + &self.local_state_machine + } + + #[cfg(not(any(test, feature = "testing")))] + fn get_pending_proposals_count(&self) -> u64 { + 0 + } + + #[cfg(any(test, feature = "testing"))] + fn get_pending_proposals_count(&self) -> u64 { + self.signer_db + .get_all_pending_block_validations() + .map(|results| u64::try_from(results.len()).unwrap()) + .unwrap_or(0) + } +} + +impl Signer { + /// Determine this signers response to a proposed block + /// Returns a BlockResponse if we have already validated the block + /// Returns None otherwise + fn determine_response(&mut self, block_info: &BlockInfo) -> Option { + let valid = block_info.valid?; + let response = if valid { + debug!("{self}: Accepting block {}", block_info.block.block_id()); + self.create_block_acceptance(&block_info.block) + } else { + debug!("{self}: Rejecting block {}", block_info.block.block_id()); + self.create_block_rejection(RejectReason::RejectedInPriorRound, &block_info.block) + }; + Some(response) + } + + /// Create a block acceptance response for a block + pub fn create_block_acceptance(&self, block: &NakamotoBlock) -> BlockResponse { + let signature = self + .private_key + .sign(block.header.signer_signature_hash().bits()) + .expect("Failed to sign block"); + BlockResponse::accepted( + block.header.signer_signature_hash(), + signature, + self.signer_db.calculate_tenure_extend_timestamp( + self.proposal_config + .tenure_idle_timeout + .saturating_add(self.proposal_config.tenure_idle_timeout_buffer), + block, + true, + ), + ) + } + + /// The actual switch-on-event processing of an event. + /// This is separated from the Signer trait implementation of process_event + /// so that the "do on every event" functionality can run after every event processing + /// (i.e. even if the event_match does an early return). + fn handle_event_match( + &mut self, + stacks_client: &StacksClient, + sortition_state: &mut Option, + event: &SignerEvent, + current_reward_cycle: u64, + ) { match event { SignerEvent::BlockValidationResponse(block_validate_response) => { debug!("{self}: Received a block proposal result from the stacks node..."); @@ -431,74 +516,6 @@ impl SignerTrait for Signer { } } } - - if prior_state != self.local_state_machine { - self.local_state_machine - .send_signer_update_message(&mut self.stackerdb); - } - } - - fn has_unprocessed_blocks(&self) -> bool { - self.signer_db - .has_unprocessed_blocks(self.reward_cycle) - .unwrap_or_else(|e| { - error!("{self}: Failed to check for pending blocks: {e:?}",); - // Assume we have pending blocks to prevent premature cleanup - true - }) - } - - fn get_local_state_machine(&self) -> &LocalStateMachine { - &self.local_state_machine - } - - #[cfg(not(any(test, feature = "testing")))] - fn get_pending_proposals_count(&self) -> u64 { - 0 - } - - #[cfg(any(test, feature = "testing"))] - fn get_pending_proposals_count(&self) -> u64 { - self.signer_db - .get_all_pending_block_validations() - .map(|results| u64::try_from(results.len()).unwrap()) - .unwrap_or(0) - } -} - -impl Signer { - /// Determine this signers response to a proposed block - /// Returns a BlockResponse if we have already validated the block - /// Returns None otherwise - fn determine_response(&mut self, block_info: &BlockInfo) -> Option { - let valid = block_info.valid?; - let response = if valid { - debug!("{self}: Accepting block {}", block_info.block.block_id()); - self.create_block_acceptance(&block_info.block) - } else { - debug!("{self}: Rejecting block {}", block_info.block.block_id()); - self.create_block_rejection(RejectReason::RejectedInPriorRound, &block_info.block) - }; - Some(response) - } - - /// Create a block acceptance response for a block - pub fn create_block_acceptance(&self, block: &NakamotoBlock) -> BlockResponse { - let signature = self - .private_key - .sign(block.header.signer_signature_hash().bits()) - .expect("Failed to sign block"); - BlockResponse::accepted( - block.header.signer_signature_hash(), - signature, - self.signer_db.calculate_tenure_extend_timestamp( - self.proposal_config - .tenure_idle_timeout - .saturating_add(self.proposal_config.tenure_idle_timeout_buffer), - block, - true, - ), - ) } /// Create a block rejection response for a block with the given reject code From fb6b3855c8859cac400566023a905bb3c0e11ca6 Mon Sep 17 00:00:00 2001 From: wileyj <2847772+wileyj@users.noreply.github.com> Date: Mon, 28 Apr 2025 08:42:01 -0700 Subject: [PATCH 6/6] bump version for signer in versions.toml --- versions.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/versions.toml b/versions.toml index cef4db013a..fc1401db85 100644 --- a/versions.toml +++ b/versions.toml @@ -1,4 +1,4 @@ # Update these values when a new release is created. # `stacks-common/build.rs` will automatically update `versions.rs` with these values. stacks_node_version = "3.1.0.0.8" -stacks_signer_version = "3.1.0.0.8.0" +stacks_signer_version = "3.1.0.0.8.1"