From ef0a368c351ded9919f2f4f6f7c0de937fb95c58 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Tue, 8 Oct 2024 08:46:45 -0500 Subject: [PATCH 1/9] feat: naka miner waits for outstanding parent proposals * adds new config option `miner.wait_for_proposals_secs: Option` to control the maximum time that a miner will wait for any of its parent's outstanding block proposals to be confirmed. Default value is 10 seconds. --- libsigner/src/v0/messages.rs | 9 ++ testnet/stacks-node/src/config.rs | 28 +++- .../stacks-node/src/nakamoto_node/miner.rs | 138 +++++++++++++++++- 3 files changed, 168 insertions(+), 7 deletions(-) diff --git a/libsigner/src/v0/messages.rs b/libsigner/src/v0/messages.rs index 47d317992d..5c3685303d 100644 --- a/libsigner/src/v0/messages.rs +++ b/libsigner/src/v0/messages.rs @@ -26,6 +26,7 @@ use std::fmt::{Debug, Display}; use std::io::{Read, Write}; use std::net::{SocketAddr, TcpListener, TcpStream}; +use std::ops::Range; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::mpsc::Sender; use std::sync::Arc; @@ -90,6 +91,14 @@ MinerSlotID { BlockPushed = 1 }); +impl MinerSlotID { + /// Return the u32 slot id for messages of this type from a given miner's + /// slot range in the .miners contract + pub fn get_slot_for_miner(&self, miner_range: &Range) -> u32 { + miner_range.start.saturating_add(self.to_u8().into()) + } +} + impl MessageSlotIDTrait for MessageSlotID { fn stacker_db_contract(&self, mainnet: bool, reward_cycle: u64) -> QualifiedContractIdentifier { NakamotoSigners::make_signers_db_contract_id(reward_cycle, self.to_u32(), mainnet) diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs index 3852bf4224..bc669a1f43 100644 --- a/testnet/stacks-node/src/config.rs +++ b/testnet/stacks-node/src/config.rs @@ -87,6 +87,7 @@ const DEFAULT_MAX_RBF_RATE: u64 = 150; // 1.5x const DEFAULT_RBF_FEE_RATE_INCREMENT: u64 = 5; const INV_REWARD_CYCLES_TESTNET: u64 = 6; const DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS: u64 = 1000; +const DEFAULT_WAIT_FOR_PROPOSALS_SECS: u64 = 10; #[derive(Clone, Deserialize, Default, Debug)] pub struct ConfigFile { @@ -2385,6 +2386,10 @@ pub struct MinerConfig { /// The minimum time to wait between mining blocks in milliseconds. The value must be greater than or equal to 1000 ms because if a block is mined /// within the same second as its parent, it will be rejected by the signers. pub min_time_between_blocks_ms: u64, + /// How much time (in seconds) to wait for an outstanding block + /// proposal from a parent tenure to get confirmed before + /// building a child block of that tenure. + pub wait_for_proposals_secs: u64, } impl Default for MinerConfig { @@ -2415,6 +2420,7 @@ impl Default for MinerConfig { max_reorg_depth: 3, pre_nakamoto_mock_signing: false, // Should only default true if mining key is set min_time_between_blocks_ms: DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS, + wait_for_proposals_secs: DEFAULT_WAIT_FOR_PROPOSALS_SECS, } } } @@ -2773,6 +2779,10 @@ pub struct MinerConfigFile { pub max_reorg_depth: Option, pub pre_nakamoto_mock_signing: Option, pub min_time_between_blocks_ms: Option, + /// How much time (in seconds) to wait for an outstanding block + /// proposal from a parent tenure to get confirmed before + /// building a child block of that tenure. + pub wait_for_proposals_secs: Option, } impl MinerConfigFile { @@ -2880,12 +2890,18 @@ impl MinerConfigFile { pre_nakamoto_mock_signing: self .pre_nakamoto_mock_signing .unwrap_or(pre_nakamoto_mock_signing), // Should only default true if mining key is set - min_time_between_blocks_ms: self.min_time_between_blocks_ms.map(|ms| if ms < DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS { - warn!("miner.min_time_between_blocks_ms is less than the minimum allowed value of {DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS} ms. Using the default value instead."); - DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS - } else { - ms - }).unwrap_or(miner_default_config.min_time_between_blocks_ms), + min_time_between_blocks_ms: self + .min_time_between_blocks_ms + .map(|ms| if ms < DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS { + warn!("miner.min_time_between_blocks_ms is less than the minimum allowed value of {DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS} ms. Using the default value instead."); + DEFAULT_MIN_TIME_BETWEEN_BLOCKS_MS + } else { + ms + }) + .unwrap_or(miner_default_config.min_time_between_blocks_ms), + wait_for_proposals_secs: self + .wait_for_proposals_secs + .unwrap_or(miner_default_config.wait_for_proposals_secs), }) } } diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index af539db5b1..38f21b180b 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -19,7 +19,7 @@ use std::time::{Duration, Instant}; use clarity::boot_util::boot_code_id; use clarity::vm::types::PrincipalData; -use libsigner::v0::messages::{MinerSlotID, SignerMessage}; +use libsigner::v0::messages::{MinerSlotID, SignerMessage, SignerMessageTypePrefix}; use libsigner::StackerDBSession; use rand::{thread_rng, Rng}; use stacks::burnchains::Burnchain; @@ -37,6 +37,7 @@ use stacks::chainstate::stacks::{ TenureChangeCause, TenureChangePayload, TransactionAnchorMode, TransactionPayload, TransactionVersion, }; +use stacks::codec::StacksMessageCodec; use stacks::net::p2p::NetworkHandle; use stacks::net::stackerdb::StackerDBs; use stacks::net::{NakamotoBlocksData, StacksMessageType}; @@ -150,6 +151,8 @@ pub struct BlockMinerThread { /// Handle to the p2p thread for block broadcast p2p_handle: NetworkHandle, signer_set_cache: Option, + /// UNIX epoch in seconds that the miner thread started + start_time: u64, } impl BlockMinerThread { @@ -176,6 +179,7 @@ impl BlockMinerThread { reason, p2p_handle: rt.get_p2p_handle(), signer_set_cache: None, + start_time: get_epoch_time_secs(), } } @@ -264,6 +268,114 @@ impl BlockMinerThread { Ok(()) } + /// See if there's an outstanding block proposal in the .miners stackerdb + /// from our parent tenure + fn check_outstanding_block_proposal( + parent_tenure_election_ch: &ConsensusHash, + stacks_parent_header: &StacksHeaderInfo, + sortdb: &SortitionDB, + stackerdbs: &StackerDBs, + is_mainnet: bool, + wait_for_parent_proposals_secs: u64, + my_start_time: u64, + ) -> Result<(), NakamotoNodeError> { + let cur_burn_chain_tip = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()) + .expect("FATAL: failed to query sortition DB for canonical burn chain tip"); + + let parent_proposal_slot = match NakamotoChainState::get_miner_slot( + sortdb, + &cur_burn_chain_tip, + parent_tenure_election_ch, + ) { + Ok(Some(parent_slots)) => MinerSlotID::BlockProposal.get_slot_for_miner(&parent_slots), + Ok(None) => { + debug!("Parent tenure no longer has a miner slot, will not check for outstanding proposals"); + return Ok(()); + } + Err(e) => { + info!( + "Failed to lookup miner slots for parent tenure, will not check for outstanding proposals"; + "err" => ?e + ); + return Ok(()); + } + }; + let miners_contract = boot_code_id(MINERS_NAME, is_mainnet); + let latest_chunk = match stackerdbs.get_latest_chunk(&miners_contract, parent_proposal_slot) + { + Ok(Some(chunk)) => chunk, + Ok(None) => { + debug!("Parent tenure slots have no proposal data"); + return Ok(()); + } + Err(e) => { + info!( + "Failed to read miner slot for parent tenure, will not check for outstanding proposals"; + "err" => ?e + ); + return Ok(()); + } + }; + + let latest_proposal = match SignerMessage::consensus_deserialize( + &mut latest_chunk.as_slice(), + ) { + Ok(SignerMessage::BlockProposal(proposal)) => proposal, + Ok(d) => { + info!( + "Parent tenure's miner slot contained data other than a proposal, will not check for outstanding proposals"; + "message.type_prefix" => ?SignerMessageTypePrefix::from(&d), + ); + return Ok(()); + } + Err(e) => { + info!( + "Failed to parse message in parent tenure's miner slot, will not check for outstanding proposals"; + "err" => ?e + ); + return Ok(()); + } + }; + + if latest_proposal.block.header.chain_length <= stacks_parent_header.stacks_block_height { + debug!("Parent block proposal found, and its the block we are building on top of"); + return Ok(()); + } + + let proposal_timestamp = latest_proposal.block.header.timestamp; + let current_time = get_epoch_time_secs(); + + if proposal_timestamp > current_time { + // don't wait for insanity + debug!( + "Found outstanding parent block proposal, but its timestamp is in the future, ignoring."; + ); + return Ok(()); + } + + // if enough time has passed, this proposal should have been accepted already, so just ignore it + if current_time.saturating_sub(proposal_timestamp) > wait_for_parent_proposals_secs { + debug!( + "Found outstanding parent block proposal, but enough time has passed that this proposal should be ignored."; + ); + return Ok(()); + } + + // if enough time has passed since the miner thread itself started, just ignore the proposal + if current_time.saturating_sub(my_start_time) > wait_for_parent_proposals_secs { + debug!( + "Found outstanding parent block proposal, but enough time has passed since miner thread started that this proposal should be ignored."; + ); + return Ok(()); + } + + // otherwise, there *is* an outstanding proposal, which the signer set could still be considering + // signal the miner thread to abort and retry + return Err(NakamotoNodeError::MiningFailure( + ChainstateError::MinerAborted, + )); + } + pub fn run_miner( mut self, prior_miner: Option>>, @@ -1039,6 +1151,30 @@ impl BlockMinerThread { return Err(NakamotoNodeError::ParentNotFound); }; + if self.last_block_mined.is_none() { + let Some(ParentTenureInfo { + ref parent_tenure_consensus_hash, + .. + }) = parent_block_info.parent_tenure + else { + warn!( + "Miner should be starting a new tenure, but failed to load parent tenure info" + ); + return Err(NakamotoNodeError::ParentNotFound); + }; + let stackerdbs = StackerDBs::connect(&self.config.get_stacker_db_file_path(), false) + .map_err(|e| NakamotoNodeError::MiningFailure(ChainstateError::NetError(e)))?; + Self::check_outstanding_block_proposal( + parent_tenure_consensus_hash, + &parent_block_info.stacks_parent_header, + &burn_db, + &stackerdbs, + self.config.is_mainnet(), + self.config.miner.wait_for_proposals_secs, + self.start_time, + )?; + } + // create our coinbase if this is the first block we've mined this tenure let tenure_start_info = self.make_tenure_start_info( &chain_state, From 7522653aad9bc3ba2db72c8c57f3cb857e001357 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Wed, 9 Oct 2024 08:31:08 -0500 Subject: [PATCH 2/9] ci: skip mutants on integration covered components, cover mutant for get_slot_for_miner --- libsigner/src/v0/messages.rs | 15 +++++++++++++++ testnet/stacks-node/src/config.rs | 1 + testnet/stacks-node/src/nakamoto_node/miner.rs | 1 + 3 files changed, 17 insertions(+) diff --git a/libsigner/src/v0/messages.rs b/libsigner/src/v0/messages.rs index 5c3685303d..bf7b78a473 100644 --- a/libsigner/src/v0/messages.rs +++ b/libsigner/src/v0/messages.rs @@ -1131,4 +1131,19 @@ mod test { .expect("Failed to deserialize MockSignData"); assert_eq!(mock_block, deserialized_data); } + + #[test] + fn get_slot_for_miner() { + let miner_range = std::ops::Range { start: 7, end: 10 }; + assert_eq!( + MinerSlotID::BlockProposal.get_slot_for_miner(&miner_range), + 7, + "Block proposals should be in the first slot assigned to a miner" + ); + assert_eq!( + MinerSlotID::BlockPushed.get_slot_for_miner(&miner_range), + 8, + "Block pushes should be in the second slot assigned to a miner" + ); + } } diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs index bc669a1f43..6e1074b3be 100644 --- a/testnet/stacks-node/src/config.rs +++ b/testnet/stacks-node/src/config.rs @@ -2786,6 +2786,7 @@ pub struct MinerConfigFile { } impl MinerConfigFile { + #[cfg_attr(test, mutants::skip)] fn into_config_default(self, miner_default_config: MinerConfig) -> Result { let mining_key = self .mining_key diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index 38f21b180b..acb67bf4f4 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -270,6 +270,7 @@ impl BlockMinerThread { /// See if there's an outstanding block proposal in the .miners stackerdb /// from our parent tenure + #[cfg_attr(test, mutants::skip)] fn check_outstanding_block_proposal( parent_tenure_election_ch: &ConsensusHash, stacks_parent_header: &StacksHeaderInfo, From de9b51ea68640cd7e9adb943d476939813397a43 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Mon, 7 Oct 2024 15:31:10 -0700 Subject: [PATCH 3/9] Fix metric breakage Signed-off-by: Jacinta Ferrant --- .../src/tests/nakamoto_integrations.rs | 56 ++++++++++++------- testnet/stacks-node/src/tests/signer/v0.rs | 28 ++++++---- 2 files changed, 51 insertions(+), 33 deletions(-) diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index 049df9f83a..5868111047 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -1488,16 +1488,19 @@ fn simple_neon_integration() { // query for prometheus metrics #[cfg(feature = "monitoring_prom")] { - let prom_http_origin = format!("http://{}", prom_bind); - let client = reqwest::blocking::Client::new(); - let res = client - .get(&prom_http_origin) - .send() - .unwrap() - .text() - .unwrap(); - let expected_result = format!("stacks_node_stacks_tip_height {block_height_pre_3_0}"); - assert!(res.contains(&expected_result)); + wait_for(10, || { + let prom_http_origin = format!("http://{}", prom_bind); + let client = reqwest::blocking::Client::new(); + let res = client + .get(&prom_http_origin) + .send() + .unwrap() + .text() + .unwrap(); + let expected_result = format!("stacks_node_stacks_tip_height {block_height_pre_3_0}"); + Ok(res.contains(&expected_result)) + }) + .expect("Prometheus metrics did not update"); } info!("Nakamoto miner started..."); @@ -1599,19 +1602,30 @@ fn simple_neon_integration() { let bhh = u64::from(tip.burn_header_height); test_observer::contains_burn_block_range(220..=bhh).unwrap(); - // make sure prometheus returns an updated height + // make sure prometheus returns an updated number of processed blocks #[cfg(feature = "monitoring_prom")] { - let prom_http_origin = format!("http://{}", prom_bind); - let client = reqwest::blocking::Client::new(); - let res = client - .get(&prom_http_origin) - .send() - .unwrap() - .text() - .unwrap(); - let expected_result = format!("stacks_node_stacks_tip_height {}", tip.stacks_block_height); - assert!(res.contains(&expected_result)); + wait_for(10, || { + let prom_http_origin = format!("http://{}", prom_bind); + let client = reqwest::blocking::Client::new(); + let res = client + .get(&prom_http_origin) + .send() + .unwrap() + .text() + .unwrap(); + let expected_result_1 = format!( + "stacks_node_stx_blocks_processed_total {}", + tip.stacks_block_height + ); + + let expected_result_2 = format!( + "stacks_node_stacks_tip_height {}", + tip.stacks_block_height - 1 + ); + Ok(res.contains(&expected_result_1) && res.contains(&expected_result_2)) + }) + .expect("Prometheus metrics did not update"); } check_nakamoto_empty_block_heuristics(); diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 6bffea2749..2e1a448375 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -558,18 +558,22 @@ fn miner_gather_signatures() { // Test prometheus metrics response #[cfg(feature = "monitoring_prom")] { - let metrics_response = signer_test.get_signer_metrics(); - - // Because 5 signers are running in the same process, the prometheus metrics - // are incremented once for every signer. This is why we expect the metric to be - // `5`, even though there is only one block proposed. - let expected_result = format!("stacks_signer_block_proposals_received {}", num_signers); - assert!(metrics_response.contains(&expected_result)); - let expected_result = format!( - "stacks_signer_block_responses_sent{{response_type=\"accepted\"}} {}", - num_signers - ); - assert!(metrics_response.contains(&expected_result)); + wait_for(30, || { + let metrics_response = signer_test.get_signer_metrics(); + + // Because 5 signers are running in the same process, the prometheus metrics + // are incremented once for every signer. This is why we expect the metric to be + // `10`, even though there are only two blocks proposed. + let expected_result_1 = + format!("stacks_signer_block_proposals_received {}", num_signers * 2); + let expected_result_2 = format!( + "stacks_signer_block_responses_sent{{response_type=\"accepted\"}} {}", + num_signers * 2 + ); + Ok(metrics_response.contains(&expected_result_1) + && metrics_response.contains(&expected_result_2)) + }) + .expect("Failed to advance prometheus metrics"); } } From 23d1bf5331ad278922a59db3581e7c7c2508c014 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Fri, 18 Oct 2024 15:21:52 -0700 Subject: [PATCH 4/9] wip: wait for proposals test --- stacks-signer/src/v0/signer.rs | 4 + .../stacks-node/src/nakamoto_node/miner.rs | 2 + testnet/stacks-node/src/tests/signer/v0.rs | 194 +++++++++++++++++- 3 files changed, 198 insertions(+), 2 deletions(-) diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 5d068fad0f..f309825a4d 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -63,6 +63,10 @@ pub static TEST_PAUSE_BLOCK_BROADCAST: std::sync::Mutex> = std::syn /// Skip broadcasting the block to the network pub static TEST_SKIP_BLOCK_BROADCAST: std::sync::Mutex> = std::sync::Mutex::new(None); +#[cfg(any(test, feature = "testing"))] +/// Skip checking a block proposal's sortition +pub static TEST_SKIP_SORTITION_CHECK: std::sync::Mutex> = std::sync::Mutex::new(None); + /// The stacks signer registered for the reward cycle #[derive(Debug)] pub struct Signer { diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index acb67bf4f4..bdcf1e9e34 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -370,6 +370,8 @@ impl BlockMinerThread { return Ok(()); } + info!("Found outstanding parent block proposal, which the signer set could still be considering"); + // otherwise, there *is* an outstanding proposal, which the signer set could still be considering // signal the miner thread to abort and retry return Err(NakamotoNodeError::MiningFailure( diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 89192d274d..f183d50760 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -42,7 +42,7 @@ use stacks::net::api::postblock_proposal::{ValidateRejectCode, TEST_VALIDATE_STA use stacks::net::relay::fault_injection::set_ignore_block; use stacks::types::chainstate::{StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey}; use stacks::types::PublicKey; -use stacks::util::hash::MerkleHashFunc; +use stacks::util::hash::{to_hex, MerkleHashFunc}; use stacks::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; use stacks::util_lib::boot::boot_code_id; use stacks::util_lib::signed_structured_data::pox4::{ @@ -65,7 +65,9 @@ use tracing_subscriber::{fmt, EnvFilter}; use super::SignerTest; use crate::config::{EventKeyType, EventObserverConfig}; use crate::event_dispatcher::MinedNakamotoBlockEvent; -use crate::nakamoto_node::miner::{TEST_BLOCK_ANNOUNCE_STALL, TEST_BROADCAST_STALL}; +use crate::nakamoto_node::miner::{ + TEST_BLOCK_ANNOUNCE_STALL, TEST_BROADCAST_STALL, TEST_MINE_STALL, +}; use crate::nakamoto_node::sign_coordinator::TEST_IGNORE_SIGNERS; use crate::neon::Counters; use crate::run_loop::boot_nakamoto; @@ -578,6 +580,194 @@ fn miner_gather_signatures() { } } +/// Test that a miner can wait for a proposal from a previous +/// miner's tenure +#[test] +#[ignore] +fn miner_wait_for_proposal() { + if env::var("BITCOIND_TEST") != Ok("1".into()) { + return; + } + + tracing_subscriber::registry() + .with(fmt::layer()) + .with(EnvFilter::from_default_env()) + .init(); + + info!("------------------------- Test Setup -------------------------"); + let num_signers = 5; + let sender_sk = Secp256k1PrivateKey::new(); + let sender_addr = tests::to_addr(&sender_sk); + let recipient_sk = Secp256k1PrivateKey::new(); + let recipient_addr = tests::to_addr(&recipient_sk); + let send_amt = 100; + let send_fee = 180; + let mut signer_test: SignerTest = SignerTest::new_with_config_modifications( + num_signers, + vec![(sender_addr.clone(), (send_amt + send_fee) * 5)], + |_signer_conf| {}, + |neon_conf| { + neon_conf.miner.wait_for_proposals_secs = 20; + }, + None, + None, + ); + let naka_conf = &signer_test.running_nodes.conf.clone(); + let http_origin = format!("http://{}", &naka_conf.node.rpc_bind); + let timeout = Duration::from_secs(30); + let burnchain = naka_conf.get_burnchain(); + let sortdb = burnchain.open_sortition_db(true).unwrap(); + + 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); + + // *TEST_MINE_STALL.lock().unwrap() = Some(true); + + 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![], + }; + + 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; + + let all_signers: Vec<_> = signer_test + .signer_stacks_private_keys + .iter() + .map(StacksPublicKey::from_private) + .collect(); + + let short_timeout = Duration::from_secs(30); + + // // Make more than >70% of the signers ignore the block proposal to ensure it it is not globally accepted/rejected + // let ignoring_signers: Vec<_> = all_signers + // .iter() + // .cloned() + // .take(num_signers * 7 / 10) + // .collect(); + TEST_IGNORE_ALL_BLOCK_PROPOSALS + .lock() + .unwrap() + .replace(all_signers); + + let proposals_before = signer_test + .running_nodes + .nakamoto_blocks_proposed + .load(Ordering::SeqCst); + + // submit a tx so that the miner will 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_addr.into(), + send_amt, + ); + submit_tx(&http_origin, &transfer_tx); + + info!("Submitted transfer tx and waiting for block proposal"); + let start_time = Instant::now(); + while signer_test + .running_nodes + .nakamoto_blocks_proposed + .load(Ordering::SeqCst) + <= proposals_before + { + assert!( + start_time.elapsed() <= short_timeout, + "Timed out waiting for block proposal" + ); + std::thread::sleep(Duration::from_millis(100)); + } + + let block_proposal = test_observer::get_stackerdb_chunks() + .into_iter() + .flat_map(|chunk| chunk.modified_slots) + .filter_map(|chunk| SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()).ok()) + .filter_map(|message| match message { + SignerMessage::BlockProposal(proposal) => Some(proposal), + _ => None, + }) + .last() + .expect("Expected a block proposal"); + + // *TEST_MINE_STALL.lock().unwrap() = Some(true); + + let (chainstate, _) = StacksChainState::open( + naka_conf.is_mainnet(), + naka_conf.burnchain.chain_id, + &naka_conf.get_chainstate_path_str(), + None, + ) + .unwrap(); + + let tip = NakamotoChainState::get_canonical_block_header(chainstate.db(), &sortdb) + .unwrap() + .unwrap(); + + // let blocks = test_observer::get_blocks(); + // let last_block = blocks.last().unwrap(); + + let last_stacks_height = tip.stacks_block_height; + // block.header.chain_length = last_stacks_height + 1; + // block.header.parent_block_id = tip.anchored_header.as_stacks_nakamoto().unwrap().block_id(); + + let block = block_proposal.block; + + // First propose a block to the signers that does not have the correct consensus hash or BitVec. This should be rejected BEFORE + // the block is submitted to the node for validation. + let block_signer_signature_hash_1 = block.header.signer_signature_hash(); + info!( + "------ Proposing Block ------"; + "signer_sig_hash" => %block_signer_signature_hash_1, + ); + TEST_IGNORE_ALL_BLOCK_PROPOSALS.lock().unwrap().take(); + signer_test.propose_block(block.clone(), short_timeout); + info!("------ Mining BTC Block ------"); + next_block_and_wait( + &mut signer_test.running_nodes.btc_regtest_controller, + &signer_test.running_nodes.blocks_processed, + ); + + *TEST_MINE_STALL.lock().unwrap() = Some(false); + + wait_for(30, || { + let tip = NakamotoChainState::get_canonical_block_header(chainstate.db(), &sortdb) + .unwrap() + .unwrap(); + Ok(tip.stacks_block_height > last_stacks_height) + }) + .expect("Timed out waiting for new block to be mined"); + + let blocks = test_observer::get_blocks(); + let last_block = blocks.last().unwrap(); + + let block_hash = &last_block + .as_object() + .unwrap() + .get("block_hash") + .unwrap() + .as_str() + .unwrap()[2..]; + + assert_eq!( + block_hash, + to_hex(&block_signer_signature_hash_1.0).as_str() + ); + + signer_test.shutdown(); +} + #[test] #[ignore] /// Test that signers can handle a transition between Nakamoto reward cycles From 89675e1aa8f25205bc9fd6264d2e48bff9679155 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Fri, 18 Oct 2024 15:23:34 -0700 Subject: [PATCH 5/9] fix: bad merge --- testnet/stacks-node/src/tests/signer/v0.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 9b40a364ff..400a1a7da7 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -43,9 +43,7 @@ use stacks::net::api::postblock_proposal::{ValidateRejectCode, TEST_VALIDATE_STA use stacks::net::relay::fault_injection::set_ignore_block; use stacks::types::chainstate::{StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey}; use stacks::types::PublicKey; -use stacks::util::hash::{ - hex_bytes, {to_hex, MerkleHashFunc}, -}; +use stacks::util::hash::{hex_bytes, to_hex, MerkleHashFunc}; use stacks::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; use stacks::util_lib::boot::boot_code_id; use stacks::util_lib::signed_structured_data::pox4::{ From d699e857d8bae9882392b4dacef3a5d3c94c9776 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Sun, 20 Oct 2024 14:47:15 -0700 Subject: [PATCH 6/9] feat: working test of miner waiting for previous proposal --- .../stacks-node/src/nakamoto_node/miner.rs | 10 +- testnet/stacks-node/src/tests/signer/mod.rs | 19 +- testnet/stacks-node/src/tests/signer/v0.rs | 203 ++++++++++-------- 3 files changed, 136 insertions(+), 96 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index aed4a13dce..ed23eec729 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -330,10 +330,12 @@ impl BlockMinerThread { return Ok(()); } Err(e) => { - info!( - "Failed to parse message in parent tenure's miner slot, will not check for outstanding proposals"; - "err" => ?e - ); + if latest_chunk.len() > 0 { + info!( + "Failed to parse message in parent tenure's miner slot, will not check for outstanding proposals"; + "err" => ?e + ); + } return Ok(()); } }; diff --git a/testnet/stacks-node/src/tests/signer/mod.rs b/testnet/stacks-node/src/tests/signer/mod.rs index 2e67234285..eb44b0f753 100644 --- a/testnet/stacks-node/src/tests/signer/mod.rs +++ b/testnet/stacks-node/src/tests/signer/mod.rs @@ -14,7 +14,7 @@ // along with this program. If not, see . mod v0; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; // Copyright (C) 2020-2024 Stacks Open Internet Foundation // // This program is free software: you can redistribute it and/or modify @@ -512,6 +512,23 @@ impl + Send + 'static, T: SignerEventTrait + 'static> SignerTest>() } + /// Get the slot id for each signer + fn get_slot_per_signer(&self, reward_cycle: u64) -> HashMap { + let slots = self + .get_signer_slots(reward_cycle) + .expect("FATAL: failed to get signer slots from stackerdb"); + let mut signer_to_slot_id = HashMap::new(); + for signer_config in self.signer_configs.iter() { + let pk = Secp256k1PublicKey::from_private(&signer_config.stacks_private_key); + for (slot_id, (address, _)) in slots.iter().enumerate() { + if address == &signer_config.stacks_address { + signer_to_slot_id.insert(pk, SignerSlotID(u32::try_from(slot_id).unwrap())); + } + } + } + signer_to_slot_id + } + /// Get the signer public keys for the given reward cycle fn get_signer_public_keys(&self, reward_cycle: u64) -> Vec { let entries = self.get_reward_set_signers(reward_cycle); diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 400a1a7da7..864637b9c2 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -42,8 +42,8 @@ use stacks::net::api::getsigner::GetSignerResponse; use stacks::net::api::postblock_proposal::{ValidateRejectCode, TEST_VALIDATE_STALL}; use stacks::net::relay::fault_injection::set_ignore_block; use stacks::types::chainstate::{StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey}; -use stacks::types::PublicKey; -use stacks::util::hash::{hex_bytes, to_hex, MerkleHashFunc}; +use stacks::types::{PrivateKey, PublicKey}; +use stacks::util::hash::{hex_bytes, MerkleHashFunc}; use stacks::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; use stacks::util_lib::boot::boot_code_id; use stacks::util_lib::signed_structured_data::pox4::{ @@ -66,9 +66,7 @@ use tracing_subscriber::{fmt, EnvFilter}; use super::SignerTest; use crate::config::{EventKeyType, EventObserverConfig}; use crate::event_dispatcher::MinedNakamotoBlockEvent; -use crate::nakamoto_node::miner::{ - TEST_BLOCK_ANNOUNCE_STALL, TEST_BROADCAST_STALL, TEST_MINE_STALL, -}; +use crate::nakamoto_node::miner::{TEST_BLOCK_ANNOUNCE_STALL, TEST_BROADCAST_STALL}; use crate::nakamoto_node::sign_coordinator::TEST_IGNORE_SIGNERS; use crate::neon::Counters; use crate::run_loop::boot_nakamoto; @@ -582,7 +580,21 @@ fn miner_gather_signatures() { } /// Test that a miner can wait for a proposal from a previous -/// miner's tenure +/// miner's tenure. Due to the way signers and miners handle changing +/// sortitions, there is some "manual labor" done here to trigger the +/// scenario we're testing. +/// +/// During Tenure A, we trigger a miner to propose a nakamoto block, but +/// we've configured signers to ignore it. We then grab this block proposal +/// from StackerDB and save it. +/// +/// We then advance to Tenure B, and this new miner will see that a recently proposed +/// block is in StackerDB, and the new miner will wait for some reaction to that +/// block. +/// +/// Because signers will reject blocks from a previous sortition, we manually sign +/// and broadcast BlockAccepted responses from the signers. The signers will then +/// broadcast the block themselves once they've received enough signatures. #[test] #[ignore] fn miner_wait_for_proposal() { @@ -616,29 +628,12 @@ fn miner_wait_for_proposal() { let naka_conf = &signer_test.running_nodes.conf.clone(); let http_origin = format!("http://{}", &naka_conf.node.rpc_bind); let timeout = Duration::from_secs(30); - let burnchain = naka_conf.get_burnchain(); - let sortdb = burnchain.open_sortition_db(true).unwrap(); 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); - // *TEST_MINE_STALL.lock().unwrap() = Some(true); - - 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![], - }; - - 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; + info!("----- Test Start for miner_wait_for_proposal -----"); let all_signers: Vec<_> = signer_test .signer_stacks_private_keys @@ -648,16 +643,20 @@ fn miner_wait_for_proposal() { let short_timeout = Duration::from_secs(30); - // // Make more than >70% of the signers ignore the block proposal to ensure it it is not globally accepted/rejected - // let ignoring_signers: Vec<_> = all_signers - // .iter() - // .cloned() - // .take(num_signers * 7 / 10) - // .collect(); + // Make all but 1 signers ignore. We need 1 signer to view it so that they save + // it in their SignerDB. If a signer has never seen a block, they won't + // handle other signer's block responses, which is what causes the signer to + // eventually broadcast the block. + let ignoring_signers: Vec<_> = all_signers.iter().cloned().take(num_signers - 1).collect(); + info!( + "----- Ignoring block proposals from {} of {} signers -----", + ignoring_signers.len(), + all_signers.len() + ); TEST_IGNORE_ALL_BLOCK_PROPOSALS .lock() .unwrap() - .replace(all_signers); + .replace(ignoring_signers); let proposals_before = signer_test .running_nodes @@ -677,51 +676,38 @@ fn miner_wait_for_proposal() { submit_tx(&http_origin, &transfer_tx); info!("Submitted transfer tx and waiting for block proposal"); - let start_time = Instant::now(); - while signer_test - .running_nodes - .nakamoto_blocks_proposed - .load(Ordering::SeqCst) - <= proposals_before - { - assert!( - start_time.elapsed() <= short_timeout, - "Timed out waiting for block proposal" - ); - std::thread::sleep(Duration::from_millis(100)); - } + wait_for(30, || { + Ok(signer_test + .running_nodes + .nakamoto_blocks_proposed + .load(Ordering::SeqCst) + > proposals_before) + }) + .expect("Timed out waiting for block proposal"); let block_proposal = test_observer::get_stackerdb_chunks() .into_iter() .flat_map(|chunk| chunk.modified_slots) .filter_map(|chunk| SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()).ok()) - .filter_map(|message| match message { - SignerMessage::BlockProposal(proposal) => Some(proposal), - _ => None, + .filter_map(|message| { + let SignerMessage::BlockProposal(proposal) = message else { + return None; + }; + let has_transfer = proposal.block.txs.iter().any(|tx| { + let TransactionPayload::TokenTransfer(recipient_principal, ..) = tx.clone().payload + else { + return false; + }; + recipient_principal == recipient_addr.into() + }); + if has_transfer { + Some(proposal) + } else { + None + } }) .last() - .expect("Expected a block proposal"); - - // *TEST_MINE_STALL.lock().unwrap() = Some(true); - - let (chainstate, _) = StacksChainState::open( - naka_conf.is_mainnet(), - naka_conf.burnchain.chain_id, - &naka_conf.get_chainstate_path_str(), - None, - ) - .unwrap(); - - let tip = NakamotoChainState::get_canonical_block_header(chainstate.db(), &sortdb) - .unwrap() - .unwrap(); - - // let blocks = test_observer::get_blocks(); - // let last_block = blocks.last().unwrap(); - - let last_stacks_height = tip.stacks_block_height; - // block.header.chain_length = last_stacks_height + 1; - // block.header.parent_block_id = tip.anchored_header.as_stacks_nakamoto().unwrap().block_id(); + .expect("Expected a block proposal to be found"); let block = block_proposal.block; @@ -732,7 +718,7 @@ fn miner_wait_for_proposal() { "------ Proposing Block ------"; "signer_sig_hash" => %block_signer_signature_hash_1, ); - TEST_IGNORE_ALL_BLOCK_PROPOSALS.lock().unwrap().take(); + signer_test.propose_block(block.clone(), short_timeout); info!("------ Mining BTC Block ------"); next_block_and_wait( @@ -740,31 +726,66 @@ fn miner_wait_for_proposal() { &signer_test.running_nodes.blocks_processed, ); - *TEST_MINE_STALL.lock().unwrap() = Some(false); + let reward_cycle = signer_test.get_current_reward_cycle(); - wait_for(30, || { - let tip = NakamotoChainState::get_canonical_block_header(chainstate.db(), &sortdb) - .unwrap() - .unwrap(); - Ok(tip.stacks_block_height > last_stacks_height) - }) - .expect("Timed out waiting for new block to be mined"); + let slot_per_signer = signer_test.get_slot_per_signer(reward_cycle); - let blocks = test_observer::get_blocks(); - let last_block = blocks.last().unwrap(); + TEST_IGNORE_ALL_BLOCK_PROPOSALS.lock().unwrap().take(); - let block_hash = &last_block - .as_object() - .unwrap() - .get("block_hash") - .unwrap() - .as_str() - .unwrap()[2..]; + // Now, generate a block response for each signer + signer_test.spawned_signers.iter().for_each(|signer| { + let sk = signer.config.stacks_private_key; + let pk = Secp256k1PublicKey::from_private(&sk); + let signature = sk.sign(block_signer_signature_hash_1.bits()).unwrap(); + let message = SignerMessage::BlockResponse(BlockResponse::accepted( + block_signer_signature_hash_1, + signature, + )); + let Some(slot_id) = slot_per_signer.get(&pk) else { + return; + }; + let mut stackerdb = StackerDB::::new( + &signer_test.signer_configs[0].node_host, + sk, + naka_conf.is_mainnet(), + reward_cycle, + *slot_id, + ); + info!("----- Sending Block response to slot id {} -----", slot_id); + stackerdb + .send_message_with_retry(message) + .expect("Failed to send message"); + }); - assert_eq!( - block_hash, - to_hex(&block_signer_signature_hash_1.0).as_str() - ); + // First lets wait for our proposed block to be confirmed + wait_for(30, || { + let blocks = test_observer::get_blocks(); + let found_proposed_block = blocks + .iter() + .rev() // Go backwards just because it's towards the end + .find(|block| { + let Some(block_hash) = block.as_object().unwrap().get("block_hash") else { + return false; + }; + block_hash.as_str().unwrap()[2..] == block_signer_signature_hash_1.to_string() + }); + Ok(found_proposed_block.is_some()) + }) + .expect("Timed out waiting for proposed block to be mined"); + + // Now wait for the new miner to build on top of it + wait_for(30, || { + let blocks = test_observer::get_blocks(); + let found_new_block = blocks.iter().rev().find(|block| { + let Some(parent_block_hash) = block.as_object().unwrap().get("parent_block_hash") + else { + return false; + }; + parent_block_hash.as_str().unwrap()[2..] == block_signer_signature_hash_1.to_string() + }); + Ok(found_new_block.is_some()) + }) + .expect("Timed out waiting for new block to be built on top of proposed block"); signer_test.shutdown(); } From e8a567305b868f9ff556970c0bbb74ed94ce25d4 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Mon, 21 Oct 2024 08:02:25 -0700 Subject: [PATCH 7/9] fix: wait for new block proposal to be found --- stacks-signer/src/v0/signer.rs | 4 -- testnet/stacks-node/src/tests/signer/v0.rs | 59 ++++++++++++---------- 2 files changed, 32 insertions(+), 31 deletions(-) diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index f309825a4d..5d068fad0f 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -63,10 +63,6 @@ pub static TEST_PAUSE_BLOCK_BROADCAST: std::sync::Mutex> = std::syn /// Skip broadcasting the block to the network pub static TEST_SKIP_BLOCK_BROADCAST: std::sync::Mutex> = std::sync::Mutex::new(None); -#[cfg(any(test, feature = "testing"))] -/// Skip checking a block proposal's sortition -pub static TEST_SKIP_SORTITION_CHECK: std::sync::Mutex> = std::sync::Mutex::new(None); - /// The stacks signer registered for the reward cycle #[derive(Debug)] pub struct Signer { diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 864637b9c2..9f1e10dc69 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -620,7 +620,7 @@ fn miner_wait_for_proposal() { vec![(sender_addr.clone(), (send_amt + send_fee) * 5)], |_signer_conf| {}, |neon_conf| { - neon_conf.miner.wait_for_proposals_secs = 20; + neon_conf.miner.wait_for_proposals_secs = 30; }, None, None, @@ -641,8 +641,6 @@ fn miner_wait_for_proposal() { .map(StacksPublicKey::from_private) .collect(); - let short_timeout = Duration::from_secs(30); - // Make all but 1 signers ignore. We need 1 signer to view it so that they save // it in their SignerDB. If a signer has never seen a block, they won't // handle other signer's block responses, which is what causes the signer to @@ -685,31 +683,39 @@ fn miner_wait_for_proposal() { }) .expect("Timed out waiting for block proposal"); - let block_proposal = test_observer::get_stackerdb_chunks() - .into_iter() - .flat_map(|chunk| chunk.modified_slots) - .filter_map(|chunk| SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()).ok()) - .filter_map(|message| { - let SignerMessage::BlockProposal(proposal) = message else { - return None; - }; - let has_transfer = proposal.block.txs.iter().any(|tx| { - let TransactionPayload::TokenTransfer(recipient_principal, ..) = tx.clone().payload - else { - return false; + let mut block_proposal: Option = None; + + wait_for(30, || { + block_proposal = test_observer::get_stackerdb_chunks() + .into_iter() + .flat_map(|chunk| chunk.modified_slots) + .filter_map(|chunk| { + SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()).ok() + }) + .filter_map(|message| { + let SignerMessage::BlockProposal(proposal) = message else { + return None; }; - recipient_principal == recipient_addr.into() - }); - if has_transfer { - Some(proposal) - } else { - None - } - }) - .last() - .expect("Expected a block proposal to be found"); + let has_transfer = proposal.block.txs.iter().any(|tx| { + let TransactionPayload::TokenTransfer(recipient_principal, ..) = + tx.clone().payload + else { + return false; + }; + recipient_principal == recipient_addr.into() + }); + if has_transfer { + Some(proposal) + } else { + None + } + }) + .last(); + Ok(block_proposal.is_some()) + }) + .expect("Timed out waiting for block proposal"); - let block = block_proposal.block; + let block = block_proposal.unwrap().block; // First propose a block to the signers that does not have the correct consensus hash or BitVec. This should be rejected BEFORE // the block is submitted to the node for validation. @@ -719,7 +725,6 @@ fn miner_wait_for_proposal() { "signer_sig_hash" => %block_signer_signature_hash_1, ); - signer_test.propose_block(block.clone(), short_timeout); info!("------ Mining BTC Block ------"); next_block_and_wait( &mut signer_test.running_nodes.btc_regtest_controller, From 09f35464dfa5e03dfc3c0aca0515149c82b2608d Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Tue, 22 Oct 2024 10:49:19 -0700 Subject: [PATCH 8/9] feat: ensure new miner doesn't propose a block yet --- testnet/stacks-node/src/tests/signer/v0.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 9f1e10dc69..92463e36e3 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -683,6 +683,11 @@ fn miner_wait_for_proposal() { }) .expect("Timed out waiting for block proposal"); + let proposals_before = signer_test + .running_nodes + .nakamoto_blocks_proposed + .load(Ordering::SeqCst); + let mut block_proposal: Option = None; wait_for(30, || { @@ -737,6 +742,16 @@ fn miner_wait_for_proposal() { TEST_IGNORE_ALL_BLOCK_PROPOSALS.lock().unwrap().take(); + // Ensure that the new miner hasn't proposed any blocks yet + wait_for(10, || { + Ok(signer_test + .running_nodes + .nakamoto_blocks_proposed + .load(Ordering::SeqCst) + > proposals_before) + }) + .expect_err("New miner proposed a block before we expected"); + // Now, generate a block response for each signer signer_test.spawned_signers.iter().for_each(|signer| { let sk = signer.config.stacks_private_key; From 9d8cbd4c97c01c6cbf6a557374a8862a0b3f102c Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Mon, 18 Nov 2024 10:16:53 -0800 Subject: [PATCH 9/9] fix: libsigner clippy fixes --- libsigner/src/v0/messages.rs | 8 ++++---- testnet/stacks-node/src/stacks_events.rs | 2 +- testnet/stacks-node/src/tests/mod.rs | 16 ++++++++-------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/libsigner/src/v0/messages.rs b/libsigner/src/v0/messages.rs index 4e6884a275..cabaeca1fe 100644 --- a/libsigner/src/v0/messages.rs +++ b/libsigner/src/v0/messages.rs @@ -1265,9 +1265,9 @@ mod test { #[test] fn test_backwards_compatibility() { let block_rejected_hex = "010100000050426c6f636b206973206e6f7420612074656e7572652d737461727420626c6f636b2c20616e642068617320616e20756e7265636f676e697a65642074656e75726520636f6e73656e7375732068617368000691f95f84b7045f7dce7757052caa986ef042cb58f7df5031a3b5b5d0e3dda63e80000000006fb349212e1a1af1a3c712878d5159b5ec14636adb6f70be00a6da4ad4f88a9934d8a9abb229620dd8e0f225d63401e36c64817fb29e6c05591dcbe95c512df3"; - let block_rejected_bytes = hex_bytes(&block_rejected_hex).unwrap(); + let block_rejected_bytes = hex_bytes(block_rejected_hex).unwrap(); let block_accepted_hex = "010011717149677c2ac97d15ae5954f7a716f10100b9cb81a2bf27551b2f2e54ef19001c694f8134c5c90f2f2bcd330e9f423204884f001b5df0050f36a2c4ff79dd93522bb2ae395ea87de4964886447507c18374b7a46ee2e371e9bf332f0706a3e8"; - let block_accepted_bytes = hex_bytes(&block_accepted_hex).unwrap(); + let block_accepted_bytes = hex_bytes(block_accepted_hex).unwrap(); let block_rejected = read_next::(&mut &block_rejected_bytes[..]) .expect("Failed to deserialize BlockRejection"); let block_accepted = read_next::(&mut &block_accepted_bytes[..]) @@ -1301,9 +1301,9 @@ mod test { #[test] fn test_block_response_metadata() { let block_rejected_hex = "010100000050426c6f636b206973206e6f7420612074656e7572652d737461727420626c6f636b2c20616e642068617320616e20756e7265636f676e697a65642074656e75726520636f6e73656e7375732068617368000691f95f84b7045f7dce7757052caa986ef042cb58f7df5031a3b5b5d0e3dda63e80000000006fb349212e1a1af1a3c712878d5159b5ec14636adb6f70be00a6da4ad4f88a9934d8a9abb229620dd8e0f225d63401e36c64817fb29e6c05591dcbe95c512df30000000b48656c6c6f20776f726c64"; - let block_rejected_bytes = hex_bytes(&block_rejected_hex).unwrap(); + let block_rejected_bytes = hex_bytes(block_rejected_hex).unwrap(); let block_accepted_hex = "010011717149677c2ac97d15ae5954f7a716f10100b9cb81a2bf27551b2f2e54ef19001c694f8134c5c90f2f2bcd330e9f423204884f001b5df0050f36a2c4ff79dd93522bb2ae395ea87de4964886447507c18374b7a46ee2e371e9bf332f0706a3e80000000b48656c6c6f20776f726c64"; - let block_accepted_bytes = hex_bytes(&block_accepted_hex).unwrap(); + let block_accepted_bytes = hex_bytes(block_accepted_hex).unwrap(); let block_rejected = read_next::(&mut &block_rejected_bytes[..]) .expect("Failed to deserialize BlockRejection"); let block_accepted = read_next::(&mut &block_accepted_bytes[..]) diff --git a/testnet/stacks-node/src/stacks_events.rs b/testnet/stacks-node/src/stacks_events.rs index f63b17a6ab..b37c4d43d8 100644 --- a/testnet/stacks-node/src/stacks_events.rs +++ b/testnet/stacks-node/src/stacks_events.rs @@ -92,7 +92,7 @@ fn handle_connection(mut stream: TcpStream) { contents ); - stream.write(response.as_bytes()).unwrap(); + stream.write_all(response.as_bytes()).unwrap(); stream.flush().unwrap(); } } diff --git a/testnet/stacks-node/src/tests/mod.rs b/testnet/stacks-node/src/tests/mod.rs index 2c555e7232..c0946a8907 100644 --- a/testnet/stacks-node/src/tests/mod.rs +++ b/testnet/stacks-node/src/tests/mod.rs @@ -1030,18 +1030,18 @@ fn should_succeed_handling_malformed_and_valid_txs() { // Transaction #1 should be the coinbase from the leader let coinbase_tx = &chain_tip.block.txs[0]; assert!(coinbase_tx.chain_id == CHAIN_ID_TESTNET); - assert!(match coinbase_tx.payload { - TransactionPayload::Coinbase(..) => true, - _ => false, - }); + assert!(matches!( + coinbase_tx.payload, + TransactionPayload::Coinbase(..) + )); // Transaction #2 should be the smart contract published let contract_tx = &chain_tip.block.txs[1]; assert!(contract_tx.chain_id == CHAIN_ID_TESTNET); - assert!(match contract_tx.payload { - TransactionPayload::SmartContract(..) => true, - _ => false, - }); + assert!(matches!( + contract_tx.payload, + TransactionPayload::SmartContract(..) + )); } 2 => { // Inspecting the chain at round 2.