From f23ffb6bd10cdfd0a169221b604c925ff1724221 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Wed, 30 Oct 2024 13:40:30 -0700 Subject: [PATCH 1/9] Add a block_proposal_validation_timeout_ms config option Signed-off-by: Jacinta Ferrant --- stacks-signer/src/config.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index 6fc7c7b2dd..052817302f 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -35,6 +35,7 @@ use crate::client::SignerSlotID; const EVENT_TIMEOUT_MS: u64 = 5000; const BLOCK_PROPOSAL_TIMEOUT_MS: u64 = 600_000; +const BLOCK_PROPOSAL_VALIDATION_TIMEOUT_MS: u64 = 120_000; const DEFAULT_FIRST_PROPOSAL_BURN_BLOCK_TIMING_SECS: u64 = 60; #[derive(thiserror::Error, Debug)] @@ -158,6 +159,9 @@ pub struct GlobalConfig { pub block_proposal_timeout: Duration, /// An optional custom Chain ID pub chain_id: Option, + /// How long to wait for a response from a block proposal validation response from the node + /// before marking that block as invalid and rejecting it + pub block_proposal_validation_timeout: Duration, } /// Internal struct for loading up the config file @@ -187,6 +191,9 @@ struct RawConfigFile { pub block_proposal_timeout_ms: Option, /// An optional custom Chain ID pub chain_id: Option, + /// How long to wait for a response from a block proposal validation response from the node + /// before marking that block as invalid and rejecting it in milliseconds. + pub block_proposal_validation_timeout_ms: Option, } impl RawConfigFile { @@ -266,6 +273,11 @@ impl TryFrom for GlobalConfig { .unwrap_or(BLOCK_PROPOSAL_TIMEOUT_MS), ); + let block_proposal_validation_timeout = Duration::from_millis( + raw_data + .block_proposal_validation_timeout_ms + .unwrap_or(BLOCK_PROPOSAL_VALIDATION_TIMEOUT_MS), + ); Ok(Self { node_host: raw_data.node_host, endpoint, @@ -279,6 +291,7 @@ impl TryFrom for GlobalConfig { first_proposal_burn_block_timing, block_proposal_timeout, chain_id: raw_data.chain_id, + block_proposal_validation_timeout, }) } } From 9161e04de698c7978dd494f7b14b37fb4ddc9d14 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Wed, 30 Oct 2024 16:02:11 -0700 Subject: [PATCH 2/9] Track the last submitted block proposal and do not submit a new one if we already are processing one Signed-off-by: Jacinta Ferrant --- stacks-signer/src/chainstate.rs | 3 + stacks-signer/src/client/mod.rs | 1 + stacks-signer/src/config.rs | 2 + stacks-signer/src/runloop.rs | 1 + stacks-signer/src/tests/chainstate.rs | 1 + stacks-signer/src/v0/signer.rs | 157 +++++++++++++++++- .../src/tests/nakamoto_integrations.rs | 3 + testnet/stacks-node/src/tests/signer/v0.rs | 1 + 8 files changed, 161 insertions(+), 8 deletions(-) diff --git a/stacks-signer/src/chainstate.rs b/stacks-signer/src/chainstate.rs index 44ae11b252..926dee312a 100644 --- a/stacks-signer/src/chainstate.rs +++ b/stacks-signer/src/chainstate.rs @@ -119,6 +119,8 @@ pub struct ProposalEvalConfig { pub first_proposal_burn_block_timing: Duration, /// Time between processing a sortition and proposing a block before the block is considered invalid pub block_proposal_timeout: Duration, + /// How long to wait for a block proposal validation response + pub block_proposal_validation_timeout: Duration, } impl From<&SignerConfig> for ProposalEvalConfig { @@ -126,6 +128,7 @@ impl From<&SignerConfig> for ProposalEvalConfig { Self { first_proposal_burn_block_timing: value.first_proposal_burn_block_timing, block_proposal_timeout: value.block_proposal_timeout, + block_proposal_validation_timeout: value.block_proposal_validation_timeout, } } } diff --git a/stacks-signer/src/client/mod.rs b/stacks-signer/src/client/mod.rs index 9885182d98..7e1e388e92 100644 --- a/stacks-signer/src/client/mod.rs +++ b/stacks-signer/src/client/mod.rs @@ -411,6 +411,7 @@ pub(crate) mod tests { db_path: config.db_path.clone(), first_proposal_burn_block_timing: config.first_proposal_burn_block_timing, block_proposal_timeout: config.block_proposal_timeout, + block_proposal_validation_timeout: config.block_proposal_validation_timeout, } } diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index 052817302f..ec966539ca 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -129,6 +129,8 @@ pub struct SignerConfig { pub first_proposal_burn_block_timing: Duration, /// How much time to wait for a miner to propose a block following a sortition pub block_proposal_timeout: Duration, + /// How much time ot wait for a block proposal validation response before marking the block invalid + pub block_proposal_validation_timeout: Duration, } /// The parsed configuration for the signer diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index a0e2b739e9..de77a2a0d4 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -283,6 +283,7 @@ impl, T: StacksMessageCodec + Clone + Send + Debug> RunLo mainnet: self.config.network.is_mainnet(), db_path: self.config.db_path.clone(), block_proposal_timeout: self.config.block_proposal_timeout, + block_proposal_validation_timeout: self.config.block_proposal_validation_timeout, })) } diff --git a/stacks-signer/src/tests/chainstate.rs b/stacks-signer/src/tests/chainstate.rs index 886480f063..59cd063c9e 100644 --- a/stacks-signer/src/tests/chainstate.rs +++ b/stacks-signer/src/tests/chainstate.rs @@ -89,6 +89,7 @@ fn setup_test_environment( config: ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(30), block_proposal_timeout: Duration::from_secs(5), + block_proposal_validation_timeout: Duration::from_secs(60), }, }; diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index db84efbd7d..200db378b9 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -15,6 +15,7 @@ use std::collections::HashMap; use std::fmt::Debug; use std::sync::mpsc::Sender; +use std::time::Instant; use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader}; use blockstack_lib::net::api::postblock_proposal::{ @@ -85,6 +86,8 @@ pub struct Signer { pub signer_db: SignerDb, /// Configuration for proposal evaluation pub proposal_config: ProposalEvalConfig, + /// The current submitted block proposal and its submission time + pub submitted_block_proposal: Option<(BlockProposal, Instant)>, } impl std::fmt::Display for Signer { @@ -127,6 +130,7 @@ impl SignerTrait for Signer { if event_parity == Some(other_signer_parity) { return; } + self.check_submitted_block_proposal(); debug!("{self}: Processing event: {event:?}"); let Some(event) = event else { // No event. Do nothing. @@ -274,6 +278,7 @@ impl From for Signer { reward_cycle: signer_config.reward_cycle, signer_db, proposal_config, + submitted_block_proposal: None, } } } @@ -355,7 +360,7 @@ impl Signer { } info!( - "{self}: received a block proposal for a new block. Submit block for validation. "; + "{self}: received a block proposal for a new block."; "signer_sighash" => %signer_signature_hash, "block_id" => %block_proposal.block.block_id(), "block_height" => block_proposal.block.header.chain_length, @@ -456,14 +461,35 @@ impl Signer { Ok(_) => debug!("{self}: Block rejection accepted by stacker-db"), } } else { - // We don't know if proposal is valid, submit to stacks-node for further checks and store it locally. - // Do not store invalid blocks as this could DOS the signer. We only store blocks that are valid or unknown. - stacks_client - .submit_block_for_validation(block_info.block.clone()) - .unwrap_or_else(|e| { - warn!("{self}: Failed to submit block for validation: {e:?}"); - }); + // Just in case check if the last block validation submission timed out. + self.check_submitted_block_proposal(); + if self.submitted_block_proposal.is_none() { + // We don't know if proposal is valid, submit to stacks-node for further checks and store it locally. + info!( + "{self}: submitting block proposal for validation"; + "signer_sighash" => %signer_signature_hash, + "block_id" => %block_proposal.block.block_id(), + "block_height" => block_proposal.block.header.chain_length, + "burn_height" => block_proposal.burn_height, + ); + match stacks_client.submit_block_for_validation(block_info.block.clone()) { + Ok(_) => { + self.submitted_block_proposal = + Some((block_proposal.clone(), Instant::now())); + } + Err(e) => { + warn!("{self}: Failed to submit block for validation: {e:?}"); + } + }; + } 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. + // However, we will not be able to participate beyond this until our block submission times out or we receive a response + // from our node. + warn!("{self}: cannot submit block proposal for validation as we are already waiting for a response for a prior submission") + } + // Do not store KNOWN invalid blocks as this could DOS the signer. We only store blocks that are valid or unknown. self.signer_db .insert_block(&block_info) .unwrap_or_else(|_| panic!("{self}: Failed to insert block in DB")); @@ -493,6 +519,16 @@ impl Signer { ) -> Option { crate::monitoring::increment_block_validation_responses(true); let signer_signature_hash = block_validate_ok.signer_signature_hash; + if self + .submitted_block_proposal + .as_ref() + .map(|(proposal, _)| { + proposal.block.header.signer_signature_hash() == signer_signature_hash + }) + .unwrap_or(false) + { + self.submitted_block_proposal = None; + } // For mutability reasons, we need to take the block_info out of the map and add it back after processing let mut block_info = match self .signer_db @@ -542,6 +578,16 @@ impl Signer { ) -> Option { crate::monitoring::increment_block_validation_responses(false); let signer_signature_hash = block_validate_reject.signer_signature_hash; + if self + .submitted_block_proposal + .as_ref() + .map(|(proposal, _)| { + proposal.block.header.signer_signature_hash() == signer_signature_hash + }) + .unwrap_or(false) + { + self.submitted_block_proposal = None; + } let mut block_info = match self .signer_db .block_lookup(self.reward_cycle, &signer_signature_hash) @@ -617,6 +663,85 @@ impl Signer { } } + /// Check the current tracked submitted block proposal to see if it has timed out. + /// Broadcasts a rejection and marks the block locally rejected if it has. + fn check_submitted_block_proposal(&mut self) { + let Some((block_proposal, block_submission)) = self.submitted_block_proposal.clone() else { + // Nothing to check. + return; + }; + if block_submission.elapsed() < self.proposal_config.block_proposal_validation_timeout { + // Not expired yet. + return; + } + // Let us immediately flush, even if we later encounter an error broadcasting our responses. + // We should still attempt to handle a new proposal at this point. + self.submitted_block_proposal = None; + let signature_sighash = block_proposal.block.header.signer_signature_hash(); + // For mutability reasons, we need to take the block_info out of the map and add it back after processing + let mut block_info = match self + .signer_db + .block_lookup(self.reward_cycle, &signature_sighash) + { + Ok(Some(block_info)) => { + if block_info.state == BlockState::GloballyRejected + || block_info.state == BlockState::GloballyAccepted + { + // The block has already reached consensus. This should never really hit, but check just to be safe. + self.submitted_block_proposal = None; + return; + } + block_info + } + Ok(None) => { + // This is weird. If this is reached, its probably an error in code logic or the db was flushed. + // Why are we tracking a block submission for a block we have never seen / stored before. + error!("{self}: tracking an unknown block validation submission."; + "signer_sighash" => %signature_sighash, + "block_id" => %block_proposal.block.block_id(), + ); + self.submitted_block_proposal = None; + return; + } + Err(e) => { + error!("{self}: Failed to lookup block in signer db: {e:?}",); + self.submitted_block_proposal = None; + return; + } + }; + warn!( + "{self}: Failed to receive block validation response within {} ms. Rejecting block.", self.proposal_config.block_proposal_validation_timeout.as_millis(); + "signer_sighash" => %signature_sighash, + "block_id" => %block_proposal.block.block_id(), + ); + let rejection = BlockResponse::rejected( + block_proposal.block.header.signer_signature_hash(), + RejectCode::ConnectivityIssues, + &self.private_key, + self.mainnet, + ); + // We know proposal is invalid. Send rejection message, do not do further validation + if let Err(e) = block_info.mark_locally_rejected() { + warn!("{self}: Failed to mark block as locally rejected: {e:?}",); + }; + debug!("{self}: Broadcasting a block response to stacks node: {rejection:?}"); + let res = self + .stackerdb + .send_message_with_retry::(rejection.into()); + + match res { + Err(e) => warn!("{self}: Failed to send block rejection to stacker-db: {e:?}"), + Ok(ack) if !ack.accepted => warn!( + "{self}: Block rejection not accepted by stacker-db: {:?}", + ack.reason + ), + Ok(_) => debug!("{self}: Block rejection accepted by stacker-db"), + } + self.signer_db + .insert_block(&block_info) + .unwrap_or_else(|_| panic!("{self}: Failed to insert block in DB")); + } + /// Compute the signing weight, given a list of signatures fn compute_signature_signing_weight<'a>( &self, @@ -723,6 +848,14 @@ impl Signer { error!("{self}: Failed to update block state: {e:?}",); panic!("{self} Failed to update block state: {e}"); } + if self + .submitted_block_proposal + .as_ref() + .map(|(proposal, _)| &proposal.block.header.signer_signature_hash() == block_hash) + .unwrap_or(false) + { + self.submitted_block_proposal = None; + } } /// Handle an observed signature from another signer @@ -865,6 +998,14 @@ impl Signer { } } self.broadcast_signed_block(stacks_client, block_info.block, &addrs_to_sigs); + if self + .submitted_block_proposal + .as_ref() + .map(|(proposal, _)| &proposal.block.header.signer_signature_hash() == block_hash) + .unwrap_or(false) + { + self.submitted_block_proposal = None; + } } fn broadcast_signed_block( diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index b5140a06ee..6041c8ca28 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -6450,6 +6450,7 @@ fn signer_chainstate() { let proposal_conf = ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), + block_proposal_validation_timeout: Duration::from_secs(100), }; let mut sortitions_view = SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap(); @@ -6588,6 +6589,7 @@ fn signer_chainstate() { let proposal_conf = ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), + block_proposal_validation_timeout: Duration::from_secs(100), }; let burn_block_height = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()) .unwrap() @@ -6665,6 +6667,7 @@ fn signer_chainstate() { let proposal_conf = ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), + block_proposal_validation_timeout: Duration::from_secs(100), }; let mut sortitions_view = SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap(); let burn_block_height = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 234b73684a..18c52370c2 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -456,6 +456,7 @@ fn block_proposal_rejection() { let proposal_conf = ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), + block_proposal_validation_timeout: Duration::from_secs(100), }; let mut block = NakamotoBlock { header: NakamotoBlockHeader::empty(), From 44197c87f5459c7ebdf3cbdfba8ce115bd7f3f41 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Wed, 30 Oct 2024 17:15:19 -0700 Subject: [PATCH 3/9] WIP: broken test initial braindump Signed-off-by: Jacinta Ferrant --- .github/workflows/bitcoin-tests.yml | 1 + stacks-signer/src/chainstate.rs | 3 - stacks-signer/src/tests/chainstate.rs | 1 - stacks-signer/src/v0/signer.rs | 10 +- .../src/tests/nakamoto_integrations.rs | 3 - testnet/stacks-node/src/tests/signer/v0.rs | 150 +++++++++++++++++- 6 files changed, 157 insertions(+), 11 deletions(-) diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index 23eed46f1e..fd7c4f1df4 100644 --- a/.github/workflows/bitcoin-tests.yml +++ b/.github/workflows/bitcoin-tests.yml @@ -120,6 +120,7 @@ jobs: - tests::signer::v0::signing_in_0th_tenure_of_reward_cycle - tests::signer::v0::continue_after_tenure_extend - tests::signer::v0::multiple_miners_with_custom_chain_id + - tests::signer::v0::block_validation_response_timeout - tests::nakamoto_integrations::burn_ops_integration_test - tests::nakamoto_integrations::check_block_heights - tests::nakamoto_integrations::clarity_burn_state diff --git a/stacks-signer/src/chainstate.rs b/stacks-signer/src/chainstate.rs index 926dee312a..44ae11b252 100644 --- a/stacks-signer/src/chainstate.rs +++ b/stacks-signer/src/chainstate.rs @@ -119,8 +119,6 @@ pub struct ProposalEvalConfig { pub first_proposal_burn_block_timing: Duration, /// Time between processing a sortition and proposing a block before the block is considered invalid pub block_proposal_timeout: Duration, - /// How long to wait for a block proposal validation response - pub block_proposal_validation_timeout: Duration, } impl From<&SignerConfig> for ProposalEvalConfig { @@ -128,7 +126,6 @@ impl From<&SignerConfig> for ProposalEvalConfig { Self { first_proposal_burn_block_timing: value.first_proposal_burn_block_timing, block_proposal_timeout: value.block_proposal_timeout, - block_proposal_validation_timeout: value.block_proposal_validation_timeout, } } } diff --git a/stacks-signer/src/tests/chainstate.rs b/stacks-signer/src/tests/chainstate.rs index 59cd063c9e..886480f063 100644 --- a/stacks-signer/src/tests/chainstate.rs +++ b/stacks-signer/src/tests/chainstate.rs @@ -89,7 +89,6 @@ fn setup_test_environment( config: ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(30), block_proposal_timeout: Duration::from_secs(5), - block_proposal_validation_timeout: Duration::from_secs(60), }, }; diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 200db378b9..95769eed57 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -15,7 +15,7 @@ use std::collections::HashMap; use std::fmt::Debug; use std::sync::mpsc::Sender; -use std::time::Instant; +use std::time::{Duration, Instant}; use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader}; use blockstack_lib::net::api::postblock_proposal::{ @@ -86,6 +86,9 @@ pub struct Signer { pub signer_db: SignerDb, /// Configuration for proposal evaluation pub proposal_config: ProposalEvalConfig, + /// How long to wait for a block proposal validation response to arrive before + /// marking a submitted block as invalid + pub block_proposal_validation_timeout: Duration, /// The current submitted block proposal and its submission time pub submitted_block_proposal: Option<(BlockProposal, Instant)>, } @@ -279,6 +282,7 @@ impl From for Signer { signer_db, proposal_config, submitted_block_proposal: None, + block_proposal_validation_timeout: signer_config.block_proposal_validation_timeout, } } } @@ -670,7 +674,7 @@ impl Signer { // Nothing to check. return; }; - if block_submission.elapsed() < self.proposal_config.block_proposal_validation_timeout { + if block_submission.elapsed() < self.block_proposal_validation_timeout { // Not expired yet. return; } @@ -710,7 +714,7 @@ impl Signer { } }; warn!( - "{self}: Failed to receive block validation response within {} ms. Rejecting block.", self.proposal_config.block_proposal_validation_timeout.as_millis(); + "{self}: Failed to receive block validation response within {} ms. Rejecting block.", self.block_proposal_validation_timeout.as_millis(); "signer_sighash" => %signature_sighash, "block_id" => %block_proposal.block.block_id(), ); diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index 6041c8ca28..b5140a06ee 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -6450,7 +6450,6 @@ fn signer_chainstate() { let proposal_conf = ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), - block_proposal_validation_timeout: Duration::from_secs(100), }; let mut sortitions_view = SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap(); @@ -6589,7 +6588,6 @@ fn signer_chainstate() { let proposal_conf = ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), - block_proposal_validation_timeout: Duration::from_secs(100), }; let burn_block_height = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()) .unwrap() @@ -6667,7 +6665,6 @@ fn signer_chainstate() { let proposal_conf = ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), - block_proposal_validation_timeout: Duration::from_secs(100), }; let mut sortitions_view = SortitionsView::fetch_view(proposal_conf, &signer_client).unwrap(); let burn_block_height = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 18c52370c2..87317a78f6 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -456,7 +456,6 @@ fn block_proposal_rejection() { let proposal_conf = ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), - block_proposal_validation_timeout: Duration::from_secs(100), }; let mut block = NakamotoBlock { header: NakamotoBlockHeader::empty(), @@ -5608,3 +5607,152 @@ fn multiple_miners_with_custom_chain_id() { run_loop_2_thread.join().unwrap(); signer_test.shutdown(); } + +// Teimout a block validation response +#[test] +#[ignore] +fn block_validation_response_timeout() { + 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 timeout = Duration::from_secs(60); + let sender_sk = Secp256k1PrivateKey::new(); + let sender_addr = tests::to_addr(&sender_sk); + let send_amt = 100; + let send_fee = 180; + let recipient = PrincipalData::from(StacksAddress::burn_address(false)); + + let mut signer_test: SignerTest = SignerTest::new_with_config_modifications( + num_signers, + vec![(sender_addr.clone(), send_amt + send_fee)], + |config| { + config.block_proposal_validation_timeout = timeout; + }, + |_| {}, + None, + None, + ); + let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind); + signer_test.boot_to_epoch_3(); + + info!("------------------------- Test Mine and Verify Confirmed Nakamoto Block -------------------------"); + signer_test.mine_and_verify_confirmed_naka_block(timeout, num_signers); + info!("------------------------- Test Block Validation Stalled -------------------------"); + TEST_VALIDATE_STALL.lock().unwrap().replace(true); + let validation_stall_start = Instant::now(); + + let proposals_before = signer_test + .running_nodes + .nakamoto_blocks_proposed + .load(Ordering::SeqCst); + + // submit a tx so that the miner will attempt to mine an extra block + let sender_nonce = 0; + let transfer_tx = make_stacks_transfer( + &sender_sk, + sender_nonce, + send_fee, + signer_test.running_nodes.conf.burnchain.chain_id, + &recipient, + send_amt, + ); + submit_tx(&http_origin, &transfer_tx); + + info!("Submitted transfer tx and waiting for block proposal"); + wait_for(30, || { + Ok(signer_test + .running_nodes + .nakamoto_blocks_proposed + .load(Ordering::SeqCst) + > proposals_before) + }) + .expect("Timed out waiting for block proposal"); + + info!("------------------------- Propose Another Block -------------------------"); + let proposal_conf = ProposalEvalConfig { + first_proposal_burn_block_timing: Duration::from_secs(0), + block_proposal_timeout: Duration::from_secs(100), + }; + let mut block = NakamotoBlock { + header: NakamotoBlockHeader::empty(), + txs: vec![], + }; + + // Propose a block to the signers that passes initial checks but will not be submitted to the stacks node due to the submission stall + let view = SortitionsView::fetch_view(proposal_conf, &signer_test.stacks_client).unwrap(); + block.header.pox_treatment = BitVec::ones(1).unwrap(); + block.header.consensus_hash = view.cur_sortition.consensus_hash; + block.header.chain_length = 1; // We have mined 1 block so far + + let block_signer_signature_hash_1 = block.header.signer_signature_hash(); + let info_before = get_chain_info(&signer_test.running_nodes.conf); + signer_test.propose_block(block, Duration::from_secs(30)); + + info!("------------------------- Waiting for Timeout -------------------------"); + // Sleep the necessary timeout to make sure the validation times out. + let elapsed = validation_stall_start.elapsed(); + std::thread::sleep(timeout.saturating_sub(elapsed)); + + info!("------------------------- Wait for Block Rejection Due to Timeout -------------------------"); + // Verify the signers rejected the first block due to timeout + let start = Instant::now(); + let mut rejected_signers = vec![]; + let mut saw_connectivity_complaint = false; + while rejected_signers.len() < num_signers { + std::thread::sleep(Duration::from_secs(1)); + let chunks = test_observer::get_stackerdb_chunks(); + for chunk in chunks.into_iter().flat_map(|chunk| chunk.modified_slots) { + let Ok(message) = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()) + else { + continue; + }; + if let SignerMessage::BlockResponse(BlockResponse::Rejected(BlockRejection { + reason: _reason, + reason_code, + signer_signature_hash, + signature, + .. + })) = message + { + if signer_signature_hash == block_signer_signature_hash_1 { + rejected_signers.push(signature); + if matches!(reason_code, RejectCode::ConnectivityIssues) { + saw_connectivity_complaint = true; + } + } + } + } + assert!( + start.elapsed() <= Duration::from_secs(10), + "Timed out after waiting for response from signer" + ); + } + + assert!( + saw_connectivity_complaint, + "We did not see the expected connectity rejection reason" + ); + // Make sure our chain has still not advanced + let info_after = get_chain_info(&signer_test.running_nodes.conf); + assert_eq!(info_before, info_after); + + info!("Unpausing block validation"); + // Disable the stall and wait for the block to be processed + TEST_VALIDATE_STALL.lock().unwrap().replace(false); + + info!("------------------------- Test Mine and Verify Confirmed Nakamoto Block -------------------------"); + signer_test.mine_and_verify_confirmed_naka_block(timeout, num_signers); + + assert_eq!( + get_chain_info(&signer_test.running_nodes.conf).stacks_tip_height, + info_before.stacks_tip_height + 1, + ); +} From 39279989fd3e3cfedd295c53ea85eda3019e9629 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Thu, 31 Oct 2024 09:37:31 -0700 Subject: [PATCH 4/9] Fix block validation test and cleanup comment Signed-off-by: Jacinta Ferrant --- stacks-signer/src/config.rs | 2 +- stacks-signer/src/v0/signer.rs | 5 +-- testnet/stacks-node/src/tests/signer/v0.rs | 46 ++++++++++++---------- 3 files changed, 27 insertions(+), 26 deletions(-) diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index ec966539ca..30a44d2068 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -129,7 +129,7 @@ pub struct SignerConfig { pub first_proposal_burn_block_timing: Duration, /// How much time to wait for a miner to propose a block following a sortition pub block_proposal_timeout: Duration, - /// How much time ot wait for a block proposal validation response before marking the block invalid + /// How much time to wait for a block proposal validation response before marking the block invalid pub block_proposal_validation_timeout: Duration, } diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 95769eed57..32c4a41b7a 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -691,8 +691,7 @@ impl Signer { if block_info.state == BlockState::GloballyRejected || block_info.state == BlockState::GloballyAccepted { - // The block has already reached consensus. This should never really hit, but check just to be safe. - self.submitted_block_proposal = None; + // The block has already reached consensus. return; } block_info @@ -704,12 +703,10 @@ impl Signer { "signer_sighash" => %signature_sighash, "block_id" => %block_proposal.block.block_id(), ); - self.submitted_block_proposal = None; return; } Err(e) => { error!("{self}: Failed to lookup block in signer db: {e:?}",); - self.submitted_block_proposal = None; return; } }; diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 87317a78f6..612e3a1dd4 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -5623,7 +5623,7 @@ fn block_validation_response_timeout() { info!("------------------------- Test Setup -------------------------"); let num_signers = 5; - let timeout = Duration::from_secs(60); + let timeout = Duration::from_secs(30); let sender_sk = Secp256k1PrivateKey::new(); let sender_addr = tests::to_addr(&sender_sk); let send_amt = 100; @@ -5676,7 +5676,12 @@ fn block_validation_response_timeout() { }) .expect("Timed out waiting for block proposal"); - info!("------------------------- Propose Another Block -------------------------"); + assert!( + validation_stall_start.elapsed() < timeout, + "Test was too slow to propose another block before the timeout" + ); + + info!("------------------------- Propose Another Block Before Hitting the Timeout -------------------------"); let proposal_conf = ProposalEvalConfig { first_proposal_burn_block_timing: Duration::from_secs(0), block_proposal_timeout: Duration::from_secs(100), @@ -5686,26 +5691,27 @@ fn block_validation_response_timeout() { txs: vec![], }; + let info_before = get_chain_info(&signer_test.running_nodes.conf); // Propose a block to the signers that passes initial checks but will not be submitted to the stacks node due to the submission stall let view = SortitionsView::fetch_view(proposal_conf, &signer_test.stacks_client).unwrap(); block.header.pox_treatment = BitVec::ones(1).unwrap(); block.header.consensus_hash = view.cur_sortition.consensus_hash; - block.header.chain_length = 1; // We have mined 1 block so far + block.header.chain_length = info_before.stacks_tip_height + 1; let block_signer_signature_hash_1 = block.header.signer_signature_hash(); - let info_before = get_chain_info(&signer_test.running_nodes.conf); - signer_test.propose_block(block, Duration::from_secs(30)); + signer_test.propose_block(block, timeout); info!("------------------------- Waiting for Timeout -------------------------"); // Sleep the necessary timeout to make sure the validation times out. let elapsed = validation_stall_start.elapsed(); + let wait = timeout.saturating_sub(elapsed); + info!("Sleeping for {} ms", wait.as_millis()); std::thread::sleep(timeout.saturating_sub(elapsed)); info!("------------------------- Wait for Block Rejection Due to Timeout -------------------------"); // Verify the signers rejected the first block due to timeout - let start = Instant::now(); let mut rejected_signers = vec![]; - let mut saw_connectivity_complaint = false; + let start = Instant::now(); while rejected_signers.len() < num_signers { std::thread::sleep(Duration::from_secs(1)); let chunks = test_observer::get_stackerdb_chunks(); @@ -5714,32 +5720,30 @@ fn block_validation_response_timeout() { else { continue; }; - if let SignerMessage::BlockResponse(BlockResponse::Rejected(BlockRejection { + let SignerMessage::BlockResponse(BlockResponse::Rejected(BlockRejection { reason: _reason, reason_code, signer_signature_hash, signature, .. })) = message - { - if signer_signature_hash == block_signer_signature_hash_1 { - rejected_signers.push(signature); - if matches!(reason_code, RejectCode::ConnectivityIssues) { - saw_connectivity_complaint = true; - } + else { + continue; + }; + // We are waiting for the original block proposal which will have a diff signature to our + // second proposed block. + if signer_signature_hash != block_signer_signature_hash_1 { + rejected_signers.push(signature); + if matches!(reason_code, RejectCode::ConnectivityIssues) { + break; } } } assert!( - start.elapsed() <= Duration::from_secs(10), - "Timed out after waiting for response from signer" + start.elapsed() <= timeout, + "Timed out after waiting for ConenctivityIssues block rejection" ); } - - assert!( - saw_connectivity_complaint, - "We did not see the expected connectity rejection reason" - ); // Make sure our chain has still not advanced let info_after = get_chain_info(&signer_test.running_nodes.conf); assert_eq!(info_before, info_after); From b32df684cbf53c17250ff27a0fb302d05f2d30ef Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Thu, 31 Oct 2024 09:52:57 -0700 Subject: [PATCH 5/9] Cleanup comments and remove unnecessary clone Signed-off-by: Jacinta Ferrant --- stacks-signer/src/config.rs | 6 +++--- stacks-signer/src/v0/signer.rs | 13 +++++++------ testnet/stacks-node/src/tests/signer/v0.rs | 13 ++++++++----- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index 30a44d2068..b11dcc4a41 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -161,7 +161,7 @@ pub struct GlobalConfig { pub block_proposal_timeout: Duration, /// An optional custom Chain ID pub chain_id: Option, - /// How long to wait for a response from a block proposal validation response from the node + /// How long to wait in for a response from a block proposal validation response from the node /// before marking that block as invalid and rejecting it pub block_proposal_validation_timeout: Duration, } @@ -193,8 +193,8 @@ struct RawConfigFile { pub block_proposal_timeout_ms: Option, /// An optional custom Chain ID pub chain_id: Option, - /// How long to wait for a response from a block proposal validation response from the node - /// before marking that block as invalid and rejecting it in milliseconds. + /// How long to wait n milliseconds for a response from a block proposal validation response from the node + /// before marking that block as invalid and rejecting it pub block_proposal_validation_timeout_ms: Option, } diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 32c4a41b7a..6e965cf634 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -670,17 +670,15 @@ impl Signer { /// Check the current tracked submitted block proposal to see if it has timed out. /// Broadcasts a rejection and marks the block locally rejected if it has. fn check_submitted_block_proposal(&mut self) { - let Some((block_proposal, block_submission)) = self.submitted_block_proposal.clone() else { + let Some((block_proposal, block_submission)) = self.submitted_block_proposal.take() else { // Nothing to check. return; }; if block_submission.elapsed() < self.block_proposal_validation_timeout { - // Not expired yet. + // Not expired yet. Put it back! + self.submitted_block_proposal = Some((block_proposal, block_submission)); return; } - // Let us immediately flush, even if we later encounter an error broadcasting our responses. - // We should still attempt to handle a new proposal at this point. - self.submitted_block_proposal = None; let signature_sighash = block_proposal.block.header.signer_signature_hash(); // For mutability reasons, we need to take the block_info out of the map and add it back after processing let mut block_info = match self @@ -710,6 +708,8 @@ impl Signer { return; } }; + // We cannot determine the validity of the block, but we have not reached consensus on it yet. + // Reject it so we aren't holding up the network because of our inaction. warn!( "{self}: Failed to receive block validation response within {} ms. Rejecting block.", self.block_proposal_validation_timeout.as_millis(); "signer_sighash" => %signature_sighash, @@ -721,7 +721,6 @@ impl Signer { &self.private_key, self.mainnet, ); - // We know proposal is invalid. Send rejection message, do not do further validation if let Err(e) = block_info.mark_locally_rejected() { warn!("{self}: Failed to mark block as locally rejected: {e:?}",); }; @@ -855,6 +854,7 @@ impl Signer { .map(|(proposal, _)| &proposal.block.header.signer_signature_hash() == block_hash) .unwrap_or(false) { + // Consensus reached! No longer bother tracking its validation submission to the node as we are too late to participate in the decision anyway. self.submitted_block_proposal = None; } } @@ -1005,6 +1005,7 @@ impl Signer { .map(|(proposal, _)| &proposal.block.header.signer_signature_hash() == block_hash) .unwrap_or(false) { + // Consensus reached! No longer bother tracking its validation submission to the node as we are too late to participate in the decision anyway. self.submitted_block_proposal = None; } } diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 612e3a1dd4..3f0140c4a7 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -5608,7 +5608,9 @@ fn multiple_miners_with_custom_chain_id() { signer_test.shutdown(); } -// Teimout a block validation response +// Ensures that a signer will issue ConnectivityIssues rejections if a block submission +// times out. Also ensures that no other proposal gets submitted for validation if we +// are already waiting for a block submission response. #[test] #[ignore] fn block_validation_response_timeout() { @@ -5732,11 +5734,12 @@ fn block_validation_response_timeout() { }; // We are waiting for the original block proposal which will have a diff signature to our // second proposed block. - if signer_signature_hash != block_signer_signature_hash_1 { + assert_ne!( + signer_signature_hash, block_signer_signature_hash_1, + "Received a rejection for the wrong block" + ); + if matches!(reason_code, RejectCode::ConnectivityIssues) { rejected_signers.push(signature); - if matches!(reason_code, RejectCode::ConnectivityIssues) { - break; - } } } assert!( From 38bd6c77b28a84bb7415eed5f061a5ecd7e62786 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Fri, 8 Nov 2024 09:42:02 -0800 Subject: [PATCH 6/9] Fix comments Signed-off-by: Jacinta Ferrant --- stacks-signer/src/config.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index b11dcc4a41..f25e106d34 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -161,7 +161,7 @@ pub struct GlobalConfig { pub block_proposal_timeout: Duration, /// An optional custom Chain ID pub chain_id: Option, - /// How long to wait in for a response from a block proposal validation response from the node + /// How long to wait for a response from a block proposal validation response from the node /// before marking that block as invalid and rejecting it pub block_proposal_validation_timeout: Duration, } @@ -186,14 +186,14 @@ struct RawConfigFile { pub db_path: String, /// Metrics endpoint pub metrics_endpoint: Option, - /// How much time must pass between the first block proposal in a tenure and the next bitcoin block - /// before a subsequent miner isn't allowed to reorg the tenure + /// How much time (in secs) must pass between the first block proposal in a tenure and the next bitcoin block + /// before a subsequent miner isn't allowed to reorg the tenure pub first_proposal_burn_block_timing_secs: Option, - /// How much time to wait for a miner to propose a block following a sortition in milliseconds + /// How much time (in millisecs) to wait for a miner to propose a block following a sortition pub block_proposal_timeout_ms: Option, /// An optional custom Chain ID pub chain_id: Option, - /// How long to wait n milliseconds for a response from a block proposal validation response from the node + /// How long to wait (in millisecs) for a response from a block proposal validation response from the node /// before marking that block as invalid and rejecting it pub block_proposal_validation_timeout_ms: Option, } From b291df6e8f28e2b4dd16dd35ecbcdaf1e8c0cf54 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Fri, 8 Nov 2024 09:55:14 -0800 Subject: [PATCH 7/9] FIx poor merge Signed-off-by: Jacinta Ferrant --- stacks-signer/src/config.rs | 6 ------ testnet/stacks-node/src/tests/signer/v0.rs | 1 + 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index 026f652f12..57c90ab0eb 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -291,12 +291,6 @@ impl TryFrom for GlobalConfig { .unwrap_or(DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS), ); - let tenure_last_block_proposal_timeout = Duration::from_secs( - raw_data - .tenure_last_block_proposal_timeout_secs - .unwrap_or(DEFAULT_TENURE_LAST_BLOCK_PROPOSAL_TIMEOUT_SECS), - ); - let block_proposal_validation_timeout = Duration::from_millis( raw_data .block_proposal_validation_timeout_ms diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index d7effc2a34..07062b2de7 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -6195,6 +6195,7 @@ fn block_validation_response_timeout() { info!("------------------------- Propose Another Block Before Hitting the Timeout -------------------------"); let proposal_conf = ProposalEvalConfig { 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), }; let mut block = NakamotoBlock { From e37c8782d0ccab9711af28ffde24a95b8a312177 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Fri, 8 Nov 2024 12:39:01 -0800 Subject: [PATCH 8/9] Fix test Signed-off-by: Jacinta Ferrant --- stackslib/src/net/api/postblock_proposal.rs | 29 +++++------- testnet/stacks-node/src/tests/signer/v0.rs | 50 +++++++++++++-------- 2 files changed, 43 insertions(+), 36 deletions(-) diff --git a/stackslib/src/net/api/postblock_proposal.rs b/stackslib/src/net/api/postblock_proposal.rs index 517105515c..b67b6166aa 100644 --- a/stackslib/src/net/api/postblock_proposal.rs +++ b/stackslib/src/net/api/postblock_proposal.rs @@ -343,6 +343,17 @@ impl NakamotoBlockProposal { sortdb: &SortitionDB, chainstate: &mut StacksChainState, // not directly used; used as a handle to open other chainstates ) -> Result { + #[cfg(any(test, feature = "testing"))] + { + if *TEST_VALIDATE_STALL.lock().unwrap() == Some(true) { + // Do an extra check just so we don't log EVERY time. + warn!("Block validation is stalled due to testing directive."); + while *TEST_VALIDATE_STALL.lock().unwrap() == Some(true) { + std::thread::sleep(std::time::Duration::from_millis(10)); + } + info!("Block validation is no longer stalled due to testing directive."); + } + } let ts_start = get_epoch_time_ms(); // Measure time from start of function let time_elapsed = || get_epoch_time_ms().saturating_sub(ts_start); @@ -533,24 +544,6 @@ impl NakamotoBlockProposal { }); } - #[cfg(any(test, feature = "testing"))] - { - if *TEST_VALIDATE_STALL.lock().unwrap() == Some(true) { - // Do an extra check just so we don't log EVERY time. - warn!("Block validation is stalled due to testing directive."; - "block_id" => %block.block_id(), - "height" => block.header.chain_length, - ); - while *TEST_VALIDATE_STALL.lock().unwrap() == Some(true) { - std::thread::sleep(std::time::Duration::from_millis(10)); - } - info!("Block validation is no longer stalled due to testing directive."; - "block_id" => %block.block_id(), - "height" => block.header.chain_length, - ); - } - } - info!( "Participant: validated anchored block"; "block_header_hash" => %computed_block_header_hash, diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 35327d93c4..a58c539925 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -431,7 +431,8 @@ impl SignerTest { /// The stacks node is advanced to epoch 3.0 reward set calculation to ensure the signer set is determined. /// An invalid block proposal is forcibly written to the miner's slot to simulate the miner proposing a block. /// The signers process the invalid block by first verifying it against the stacks node block proposal endpoint. -/// The signers then broadcast a rejection of the miner's proposed block back to the respective .signers-XXX-YYY contract. +/// The signer that submitted the initial block validation request, should issue a broadcast a rejection of the +/// miner's proposed block back to the respective .signers-XXX-YYY contract. /// /// Test Assertion: /// Each signer successfully rejects the invalid block proposal. @@ -6240,8 +6241,9 @@ fn block_commit_delay() { signer_test.shutdown(); } -// Ensures that a signer will issue ConnectivityIssues rejections if a block submission -// times out. Also ensures that no other proposal gets submitted for validation if we +// Ensures that a signer that successfully submits a block to the node for validation +// will issue ConnectivityIssues rejections if a block submission times out. +// Also ensures that no other proposal gets submitted for validation if we // are already waiting for a block submission response. #[test] #[ignore] @@ -6344,11 +6346,8 @@ fn block_validation_response_timeout() { std::thread::sleep(timeout.saturating_sub(elapsed)); info!("------------------------- Wait for Block Rejection Due to Timeout -------------------------"); - // Verify the signers rejected the first block due to timeout - let mut rejected_signers = vec![]; - let start = Instant::now(); - while rejected_signers.len() < num_signers { - std::thread::sleep(Duration::from_secs(1)); + // Verify that the signer that submits the block to the node will issue a ConnectivityIssues rejection + wait_for(30, || { let chunks = test_observer::get_stackerdb_chunks(); for chunk in chunks.into_iter().flat_map(|chunk| chunk.modified_slots) { let Ok(message) = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()) @@ -6359,7 +6358,6 @@ fn block_validation_response_timeout() { reason: _reason, reason_code, signer_signature_hash, - signature, .. })) = message else { @@ -6372,27 +6370,43 @@ fn block_validation_response_timeout() { "Received a rejection for the wrong block" ); if matches!(reason_code, RejectCode::ConnectivityIssues) { - rejected_signers.push(signature); + return Ok(true); } } - assert!( - start.elapsed() <= timeout, - "Timed out after waiting for ConenctivityIssues block rejection" - ); - } + Ok(false) + }) + .expect("Timed out waiting for block proposal rejections"); // Make sure our chain has still not advanced let info_after = get_chain_info(&signer_test.running_nodes.conf); assert_eq!(info_before, info_after); - + let info_before = info_after; info!("Unpausing block validation"); - // Disable the stall and wait for the block to be processed + // Disable the stall and wait for the block to be processed successfully TEST_VALIDATE_STALL.lock().unwrap().replace(false); + wait_for(30, || { + let info = get_chain_info(&signer_test.running_nodes.conf); + Ok(info.stacks_tip_height > info_before.stacks_tip_height) + }) + .expect("Timed out waiting for block to be processed"); + let info_after = get_chain_info(&signer_test.running_nodes.conf); + assert_eq!( + info_after.stacks_tip_height, + info_before.stacks_tip_height + 1, + ); info!("------------------------- Test Mine and Verify Confirmed Nakamoto Block -------------------------"); + let info_before = info_after; signer_test.mine_and_verify_confirmed_naka_block(timeout, num_signers); + wait_for(30, || { + let info = get_chain_info(&signer_test.running_nodes.conf); + Ok(info.stacks_tip_height > info_before.stacks_tip_height) + }) + .unwrap(); + + let info_after = get_chain_info(&signer_test.running_nodes.conf); assert_eq!( - get_chain_info(&signer_test.running_nodes.conf).stacks_tip_height, + info_after.stacks_tip_height, info_before.stacks_tip_height + 1, ); } From 60bb664a50668c355591d30206b35afc2edcc560 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Fri, 8 Nov 2024 12:40:40 -0800 Subject: [PATCH 9/9] Fix clippy in stacks node Signed-off-by: Jacinta Ferrant --- testnet/stacks-node/src/tests/signer/v0.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index a58c539925..c4f00a39fc 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -2595,7 +2595,7 @@ fn empty_sortition_before_approval() { let block_proposal_timeout = Duration::from_secs(20); let mut signer_test: SignerTest = SignerTest::new_with_config_modifications( num_signers, - vec![(sender_addr.clone(), send_amt + send_fee)], + vec![(sender_addr, send_amt + send_fee)], |config| { // make the duration long enough that the miner will be marked as malicious config.block_proposal_timeout = block_proposal_timeout; @@ -2696,15 +2696,14 @@ fn empty_sortition_before_approval() { } let tx_bytes = hex_bytes(&raw_tx[2..]).unwrap(); let parsed = StacksTransaction::consensus_deserialize(&mut &tx_bytes[..]).unwrap(); - match &parsed.payload { - TransactionPayload::TenureChange(payload) => match payload.cause { + if let TransactionPayload::TenureChange(payload) = &parsed.payload { + match payload.cause { TenureChangeCause::Extended => { info!("Found tenure extend block"); return Ok(true); } TenureChangeCause::BlockFound => {} - }, - _ => {} + } }; } Ok(false) @@ -2769,7 +2768,7 @@ fn empty_sortition_before_proposal() { let block_proposal_timeout = Duration::from_secs(20); let mut signer_test: SignerTest = SignerTest::new_with_config_modifications( num_signers, - vec![(sender_addr.clone(), send_amt + send_fee)], + vec![(sender_addr, send_amt + send_fee)], |config| { // make the duration long enough that the miner will be marked as malicious config.block_proposal_timeout = block_proposal_timeout; @@ -2854,15 +2853,14 @@ fn empty_sortition_before_proposal() { } let tx_bytes = hex_bytes(&raw_tx[2..]).unwrap(); let parsed = StacksTransaction::consensus_deserialize(&mut &tx_bytes[..]).unwrap(); - match &parsed.payload { - TransactionPayload::TenureChange(payload) => match payload.cause { + if let TransactionPayload::TenureChange(payload) = &parsed.payload { + match payload.cause { TenureChangeCause::Extended => { info!("Found tenure extend block"); return Ok(true); } TenureChangeCause::BlockFound => {} - }, - _ => {} + } }; } Ok(false) @@ -6268,7 +6266,7 @@ fn block_validation_response_timeout() { let mut signer_test: SignerTest = SignerTest::new_with_config_modifications( num_signers, - vec![(sender_addr.clone(), send_amt + send_fee)], + vec![(sender_addr, send_amt + send_fee)], |config| { config.block_proposal_validation_timeout = timeout; },