diff --git a/libsigner/src/v0/messages.rs b/libsigner/src/v0/messages.rs index 618aa20937..4e6884a275 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) @@ -1238,6 +1247,21 @@ mod test { 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" + ); + } + #[test] fn test_backwards_compatibility() { let block_rejected_hex = "010100000050426c6f636b206973206e6f7420612074656e7572652d737461727420626c6f636b2c20616e642068617320616e20756e7265636f676e697a65642074656e75726520636f6e73656e7375732068617368000691f95f84b7045f7dce7757052caa986ef042cb58f7df5031a3b5b5d0e3dda63e80000000006fb349212e1a1af1a3c712878d5159b5ec14636adb6f70be00a6da4ad4f88a9934d8a9abb229620dd8e0f225d63401e36c64817fb29e6c05591dcbe95c512df3"; diff --git a/testnet/stacks-node/src/config.rs b/testnet/stacks-node/src/config.rs index 0beed9471d..d6b53a0686 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)] #[serde(deny_unknown_fields)] @@ -2183,6 +2184,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 { @@ -2213,6 +2218,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, } } } @@ -2575,9 +2581,14 @@ 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 { + #[cfg_attr(test, mutants::skip)] fn into_config_default(self, miner_default_config: MinerConfig) -> Result { let mining_key = self .mining_key @@ -2682,12 +2693,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 a08c0ab353..ed23eec729 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,119 @@ impl BlockMinerThread { Ok(()) } + /// 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, + 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) => { + 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(()); + } + }; + + 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(()); + } + + 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( + ChainstateError::MinerAborted, + )); + } + pub fn run_miner( mut self, prior_miner: Option>>, @@ -1023,6 +1140,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, diff --git a/testnet/stacks-node/src/tests/signer/mod.rs b/testnet/stacks-node/src/tests/signer/mod.rs index 42b894398d..95e9a300ac 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 d0f3dfff83..414884aac9 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::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::types::{PrivateKey, PublicKey}; use stacks::util::hash::{hex_bytes, MerkleHashFunc}; use stacks::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; use stacks::util_lib::boot::boot_code_id; @@ -579,6 +579,237 @@ fn miner_gather_signatures() { } } +/// Test that a miner can wait for a proposal from a previous +/// 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() { + 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 = 30; + }, + 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); + + signer_test.boot_to_epoch_3(); + + signer_test.mine_and_verify_confirmed_naka_block(timeout, num_signers); + + info!("----- Test Start for miner_wait_for_proposal -----"); + + let all_signers: Vec<_> = signer_test + .signer_stacks_private_keys + .iter() + .map(StacksPublicKey::from_private) + .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(ignoring_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"); + wait_for(30, || { + Ok(signer_test + .running_nodes + .nakamoto_blocks_proposed + .load(Ordering::SeqCst) + > proposals_before) + }) + .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, || { + 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; + }; + 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.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. + let block_signer_signature_hash_1 = block.header.signer_signature_hash(); + info!( + "------ Proposing Block ------"; + "signer_sig_hash" => %block_signer_signature_hash_1, + ); + + info!("------ Mining BTC Block ------"); + next_block_and_wait( + &mut signer_test.running_nodes.btc_regtest_controller, + &signer_test.running_nodes.blocks_processed, + ); + + let reward_cycle = signer_test.get_current_reward_cycle(); + + let slot_per_signer = signer_test.get_slot_per_signer(reward_cycle); + + 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; + 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"); + }); + + // 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(); +} + #[test] #[ignore] /// Test that signers can handle a transition between Nakamoto reward cycles