diff --git a/libsigner/src/v0/messages.rs b/libsigner/src/v0/messages.rs index 8854522bdf..8b783dfe11 100644 --- a/libsigner/src/v0/messages.rs +++ b/libsigner/src/v0/messages.rs @@ -71,7 +71,9 @@ MessageSlotID { /// Block Response message from signers BlockResponse = 1, /// Signer State Machine Update - StateMachineUpdate = 2 + StateMachineUpdate = 2, + /// Block Pre-commit message from signers before they commit to a block response + BlockPreCommit = 3 }); define_u8_enum!( @@ -114,7 +116,9 @@ SignerMessageTypePrefix { /// Mock block message from Epoch 2.5 miners MockBlock = 5, /// State machine update - StateMachineUpdate = 6 + StateMachineUpdate = 6, + /// Block Pre-commit message + BlockPreCommit = 7 }); #[cfg_attr(test, mutants::skip)] @@ -137,7 +141,7 @@ impl MessageSlotID { #[cfg_attr(test, mutants::skip)] impl Display for MessageSlotID { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{:?}({})", self, self.to_u8()) + write!(f, "{self:?}({})", self.to_u8()) } } @@ -161,6 +165,7 @@ impl From<&SignerMessage> for SignerMessageTypePrefix { SignerMessage::MockSignature(_) => SignerMessageTypePrefix::MockSignature, SignerMessage::MockBlock(_) => SignerMessageTypePrefix::MockBlock, SignerMessage::StateMachineUpdate(_) => SignerMessageTypePrefix::StateMachineUpdate, + SignerMessage::BlockPreCommit(_) => SignerMessageTypePrefix::BlockPreCommit, } } } @@ -182,6 +187,8 @@ pub enum SignerMessage { MockBlock(MockBlock), /// A state machine update StateMachineUpdate(StateMachineUpdate), + /// The pre-commit message from signers for other signers to observe + BlockPreCommit(Sha512Trunc256Sum), } impl SignerMessage { @@ -197,6 +204,7 @@ impl SignerMessage { | Self::MockBlock(_) => None, Self::BlockResponse(_) | Self::MockSignature(_) => Some(MessageSlotID::BlockResponse), // Mock signature uses the same slot as block response since its exclusively for epoch 2.5 testing Self::StateMachineUpdate(_) => Some(MessageSlotID::StateMachineUpdate), + Self::BlockPreCommit(_) => Some(MessageSlotID::BlockPreCommit), } } } @@ -216,6 +224,9 @@ impl StacksMessageCodec for SignerMessage { SignerMessage::StateMachineUpdate(state_machine_update) => { state_machine_update.consensus_serialize(fd) } + SignerMessage::BlockPreCommit(block_pre_commit) => { + block_pre_commit.consensus_serialize(fd) + } }?; Ok(()) } @@ -253,6 +264,10 @@ impl StacksMessageCodec for SignerMessage { let state_machine_update = StacksMessageCodec::consensus_deserialize(fd)?; SignerMessage::StateMachineUpdate(state_machine_update) } + SignerMessageTypePrefix::BlockPreCommit => { + let signer_signature_hash = StacksMessageCodec::consensus_deserialize(fd)?; + SignerMessage::BlockPreCommit(signer_signature_hash) + } }; Ok(message) } @@ -1145,6 +1160,18 @@ pub enum BlockResponse { Rejected(BlockRejection), } +impl From for BlockResponse { + fn from(rejection: BlockRejection) -> Self { + BlockResponse::Rejected(rejection) + } +} + +impl From for BlockResponse { + fn from(accepted: BlockAccepted) -> Self { + BlockResponse::Accepted(accepted) + } +} + #[cfg_attr(test, mutants::skip)] impl std::fmt::Display for BlockResponse { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { @@ -2604,4 +2631,14 @@ mod test { assert_eq!(signer_message, signer_message_deserialized); } + + #[test] + fn serde_block_signer_message_pre_commit() { + let pre_commit = SignerMessage::BlockPreCommit(Sha512Trunc256Sum([0u8; 32])); + let serialized_pre_commit = pre_commit.serialize_to_vec(); + let deserialized_pre_commit = + read_next::(&mut &serialized_pre_commit[..]) + .expect("Failed to deserialize pre-commit"); + assert_eq!(pre_commit, deserialized_pre_commit); + } } diff --git a/stacks-node/src/nakamoto_node/stackerdb_listener.rs b/stacks-node/src/nakamoto_node/stackerdb_listener.rs index cfe23474db..59869143e2 100644 --- a/stacks-node/src/nakamoto_node/stackerdb_listener.rs +++ b/stacks-node/src/nakamoto_node/stackerdb_listener.rs @@ -500,6 +500,9 @@ impl StackerDBListener { SignerMessageV0::StateMachineUpdate(update) => { self.update_global_state_evaluator(&signer_pubkey, update); } + SignerMessageV0::BlockPreCommit(_) => { + debug!("Received block pre-commit message. Ignoring."); + } }; } } diff --git a/stacks-node/src/tests/signer/mod.rs b/stacks-node/src/tests/signer/mod.rs index 0db7e20d61..fea036b169 100644 --- a/stacks-node/src/tests/signer/mod.rs +++ b/stacks-node/src/tests/signer/mod.rs @@ -17,7 +17,7 @@ mod commands; pub mod multiversion; pub mod v0; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::fs::File; use std::path::PathBuf; use std::sync::atomic::{AtomicBool, Ordering}; @@ -70,6 +70,7 @@ use super::nakamoto_integrations::{ use super::neon_integrations::{ copy_dir_all, get_account, get_sortition_info_ch, submit_tx_fallible, Account, }; +use crate::nakamoto_node::miner::TEST_MINE_SKIP; use crate::neon::Counters; use crate::run_loop::boot_nakamoto; use crate::tests::bitcoin_regtest::BitcoinCoreController; @@ -80,6 +81,7 @@ use crate::tests::neon_integrations::{ get_chain_info, next_block_and_wait, run_until_burnchain_height, test_observer, wait_for_runloop, }; +use crate::tests::signer::v0::wait_for_state_machine_update_by_miner_tenure_id; use crate::tests::to_addr; use crate::BitcoinRegtestController; @@ -536,11 +538,16 @@ impl SignerTest { } pub fn mine_bitcoin_block(&self) { + let mined_btc_block_time = Instant::now(); let info = self.get_peer_info(); next_block_and(&self.running_nodes.btc_regtest_controller, 60, || { Ok(get_chain_info(&self.running_nodes.conf).burn_block_height > info.burn_block_height) }) .unwrap(); + info!( + "Bitcoin block mine time elapsed: {:?}", + mined_btc_block_time.elapsed() + ); } /// Fetch the local signer state machine for all the signers, @@ -1099,26 +1106,63 @@ impl SignerTest { output } - /// Mine a BTC block and wait for a new Stacks block to be mined + /// Mine a BTC block and wait for a new Stacks block to be mined, but do not wait for a commit /// Note: do not use nakamoto blocks mined heuristic if running a test with multiple miners - fn mine_nakamoto_block(&self, timeout: Duration, use_nakamoto_blocks_mined: bool) { - let mined_block_time = Instant::now(); - let mined_before = self.running_nodes.counters.naka_mined_blocks.get(); - let info_before = self.get_peer_info(); - - next_block_and( - &self.running_nodes.btc_regtest_controller, + fn mine_nakamoto_block_without_commit( + &self, + timeout: Duration, + use_nakamoto_blocks_mined: bool, + ) { + let info_before = get_chain_info(&self.running_nodes.conf); + info!("Pausing stacks block mining"); + TEST_MINE_SKIP.set(true); + let mined_blocks = self.running_nodes.counters.naka_mined_blocks.clone(); + let mined_before = mined_blocks.get(); + self.mine_bitcoin_block(); + wait_for_state_machine_update_by_miner_tenure_id( timeout.as_secs(), - || { - let info_after = self.get_peer_info(); - let blocks_mined = self.running_nodes.counters.naka_mined_blocks.get(); - Ok(info_after.stacks_tip_height > info_before.stacks_tip_height - && (!use_nakamoto_blocks_mined || blocks_mined > mined_before)) - }, + &get_chain_info(&self.running_nodes.conf).pox_consensus, + &self.signer_addresses_versions_majority(), ) - .unwrap(); - let mined_block_elapsed_time = mined_block_time.elapsed(); - info!("Nakamoto block mine time elapsed: {mined_block_elapsed_time:?}"); + .expect("Failed to update signer state machine"); + + info!("Unpausing stacks block mining"); + let mined_block_time = Instant::now(); + TEST_MINE_SKIP.set(false); + // Do these wait for's in two steps not only for increased timeout but for easier debugging. + // Ensure that the tenure change transaction is mined + wait_for(timeout.as_secs(), || { + Ok(get_chain_info(&self.running_nodes.conf).stacks_tip_height + > info_before.stacks_tip_height + && (!use_nakamoto_blocks_mined || mined_blocks.get() > mined_before)) + }) + .expect("Failed to mine Tenure Change block"); + info!( + "Nakamoto block mine time elapsed: {:?}", + mined_block_time.elapsed() + ); + } + + /// Mine a BTC block and wait for a new Stacks block to be mined and commit to be submitted + /// Note: do not use nakamoto blocks mined heuristic if running a test with multiple miners + fn mine_nakamoto_block(&self, timeout: Duration, use_nakamoto_blocks_mined: bool) { + let Counters { + naka_submitted_commits: commits_submitted, + naka_submitted_commit_last_burn_height: commits_last_burn_height, + naka_submitted_commit_last_stacks_tip: commits_last_stacks_tip, + .. + } = self.running_nodes.counters.clone(); + let commits_before = commits_submitted.get(); + let commit_burn_height_before = commits_last_burn_height.get(); + self.mine_nakamoto_block_without_commit(timeout, use_nakamoto_blocks_mined); + // Ensure the subsequent block commit confirms the previous Tenure Change block + let stacks_tip_height = get_chain_info(&self.running_nodes.conf).stacks_tip_height; + wait_for(timeout.as_secs(), || { + Ok(commits_submitted.get() > commits_before + && commits_last_burn_height.get() > commit_burn_height_before + && commits_last_stacks_tip.get() >= stacks_tip_height) + }) + .expect("Failed to update Block Commit"); } fn mine_block_wait_on_processing( @@ -1344,7 +1388,7 @@ impl SignerTest { .collect() } - /// Get the signer addresses and corresponding versions + /// Get the signer addresses and corresponding versions configured versions pub fn signer_addresses_versions(&self) -> Vec<(StacksAddress, u64)> { self.signer_stacks_private_keys .iter() @@ -1358,6 +1402,33 @@ impl SignerTest { .collect() } + /// Get the signer addresses and corresponding majority versions + pub fn signer_addresses_versions_majority(&self) -> Vec<(StacksAddress, u64)> { + let mut signer_address_versions = self.signer_addresses_versions(); + let majority = (signer_address_versions.len() * 7 / 10) as u64; + let mut protocol_versions = HashMap::new(); + for (_, version) in &self.signer_addresses_versions() { + let entry = protocol_versions.entry(*version).or_insert_with(|| 0); + *entry += 1; + } + + // find the highest version number supported by a threshold number of signers + let mut protocol_versions: Vec<_> = protocol_versions.into_iter().collect(); + protocol_versions.sort_by_key(|(version, _)| *version); + let mut total_weight_support = 0; + for (version, weight_support) in protocol_versions.into_iter().rev() { + total_weight_support += weight_support; + if total_weight_support > majority { + // We need to actually overwrite the versions passed in since the signers will go with the majority value if they can + signer_address_versions + .iter_mut() + .for_each(|(_, v)| *v = version); + break; + } + } + signer_address_versions + } + /// 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/stacks-node/src/tests/signer/v0.rs b/stacks-node/src/tests/signer/v0.rs index fac59e3a76..d800002504 100644 --- a/stacks-node/src/tests/signer/v0.rs +++ b/stacks-node/src/tests/signer/v0.rs @@ -66,12 +66,13 @@ use stacks::net::api::postblock_proposal::{ BlockValidateResponse, ValidateRejectCode, TEST_VALIDATE_DELAY_DURATION_SECS, TEST_VALIDATE_STALL, }; +use stacks::net::api::poststackerdbchunk::StackerDBErrorCodes; use stacks::net::relay::fault_injection::{clear_ignore_block, set_ignore_block}; use stacks::types::chainstate::{ BlockHeaderHash, BurnchainHeaderHash, StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey, }; -use stacks::types::PublicKey; +use stacks::types::{PrivateKey, PublicKey}; use stacks::util::get_epoch_time_secs; use stacks::util::hash::{hex_bytes, Hash160, MerkleHashFunc, Sha512Trunc256Sum}; use stacks::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; @@ -94,7 +95,8 @@ use stacks_signer::v0::signer_state::{ use stacks_signer::v0::tests::{ TEST_IGNORE_ALL_BLOCK_PROPOSALS, TEST_PAUSE_BLOCK_BROADCAST, TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION, TEST_REJECT_ALL_BLOCK_PROPOSAL, - TEST_SKIP_BLOCK_BROADCAST, TEST_SKIP_SIGNER_CLEANUP, TEST_STALL_BLOCK_VALIDATION_SUBMISSION, + TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST, TEST_SKIP_BLOCK_BROADCAST, + TEST_SKIP_SIGNER_CLEANUP, TEST_STALL_BLOCK_VALIDATION_SUBMISSION, }; use stacks_signer::v0::SpawnedSigner; use tracing_subscriber::prelude::*; @@ -174,27 +176,7 @@ impl SignerTest { // Note, we don't use `nakamoto_blocks_mined` counter, because there // could be other miners mining blocks. info!("Waiting for first Epoch 3.0 tenure to start"); - let info = get_chain_info(&self.running_nodes.conf); - next_block_and(&self.running_nodes.btc_regtest_controller, 30, || { - Ok(get_chain_info(&self.running_nodes.conf).burn_block_height > info.burn_block_height) - }) - .unwrap(); - let info = get_chain_info(&self.running_nodes.conf); - let res = wait_for_state_machine_update_by_miner_tenure_id( - 30, - &info.pox_consensus, - &self.signer_addresses_versions(), - ); - if res.is_err() { - warn!("Signer updates failed to update but attempting to continue test anyway"); - } - TEST_MINE_SKIP.set(false); - let height_before = info.stacks_tip_height; - info!("Waiting for first Nakamoto block: {}", height_before + 1); - wait_for(30, || { - Ok(get_chain_info(&self.running_nodes.conf).stacks_tip_height > height_before) - }) - .expect("Timed out waiting for first Nakamoto block after 3.0 boundary"); + self.mine_nakamoto_block(Duration::from_secs(60), false); info!("Ready to mine Nakamoto blocks!"); } } @@ -808,6 +790,7 @@ impl MultipleMinerTest { self.btc_regtest_controller_mut() .build_next_block(nmb_blocks); + wait_for(timeout_secs, || { let burn_block = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()) .unwrap() @@ -1330,6 +1313,37 @@ pub fn wait_for_block_pushed_by_miner_key( block.ok_or_else(|| "Failed to find block pushed".to_string()) } +/// Waits for all of the provided signers to send a pre-commit for a block +/// with the provided signer signature hash +pub fn wait_for_block_pre_commits_from_signers( + timeout_secs: u64, + signer_signature_hash: &Sha512Trunc256Sum, + expected_signers: &[StacksPublicKey], +) -> Result<(), String> { + wait_for(timeout_secs, || { + let chunks = test_observer::get_stackerdb_chunks() + .into_iter() + .flat_map(|chunk| chunk.modified_slots) + .filter_map(|chunk| { + let pk = chunk.recover_pk().expect("Failed to recover pk"); + if !expected_signers.contains(&pk) { + return None; + } + let message = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()) + .expect("Failed to deserialize SignerMessage"); + + if let SignerMessage::BlockPreCommit(hash) = message { + if hash == *signer_signature_hash { + return Some(pk); + } + } + None + }) + .collect::>(); + Ok(chunks.len() == expected_signers.len()) + }) +} + /// Waits for >30% of num_signers block rejection to be observed in the test_observer stackerdb chunks for a block /// with the provided signer signature hash fn wait_for_block_global_rejection( @@ -1831,61 +1845,59 @@ fn miner_gather_signatures() { info!("------------------------- Test Setup -------------------------"); let num_signers = 5; let signer_test: SignerTest = SignerTest::new(num_signers, vec![]); - let miner_sk = signer_test - .running_nodes - .conf - .miner - .mining_key - .clone() - .unwrap(); - let miner_pk = StacksPublicKey::from_private(&miner_sk); - let miner_pkh = Hash160::from_node_public_key(&miner_pk); signer_test.boot_to_epoch_3(); info!("------------------------- Test Mine and Verify Confirmed Nakamoto Block -------------------------"); TEST_MINE_SKIP.set(true); - let info_before = get_chain_info(&signer_test.running_nodes.conf); - next_block_and( - &signer_test.running_nodes.btc_regtest_controller, - 30, - || { - let info = get_chain_info(&signer_test.running_nodes.conf); - Ok(info.burn_block_height > info_before.burn_block_height) - }, - ) - .expect("Failed to process bitcoin block"); - let info_after = get_chain_info(&signer_test.running_nodes.conf); - wait_for_state_machine_update( - 30, - &info_after.pox_consensus, - info_after.burn_block_height, - Some((miner_pkh, info_before.stacks_tip_height)), - &signer_test.signer_addresses_versions(), - ) - .expect("Failed to update state machine"); + signer_test.mine_bitcoin_block(); TEST_MINE_SKIP.set(false); signer_test.check_signer_states_normal(); // Test prometheus metrics response #[cfg(feature = "monitoring_prom")] { + let min_num_expected = (num_signers * 2) as u64; wait_for(30, || { + use regex::Regex; + let metrics_response = signer_test.get_signer_metrics(); + let re_precommits = + Regex::new(r#"stacks_signer_block_pre_commits_sent (\d+)"#).unwrap(); + let re_proposals = + Regex::new(r#"stacks_signer_block_proposals_received (\d+)"#).unwrap(); + let re_responses = Regex::new( + r#"stacks_signer_block_responses_sent\{response_type="accepted"\} (\d+)"#, + ) + .unwrap(); - // Because 5 signers are running in the same process, the prometheus metrics - // are incremented once for every signer.When booting to Epoch 3.0, the old - // miner will attempt to propose a block before its burnchain tip has updated - // causing an additional block proposal that gets rejected due to consensus hash - // mismatch, hence why we expect 15 rather than just 10 proposals. - 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)) + let precommits = re_precommits + .captures(&metrics_response) + .and_then(|caps| caps.get(1)) + .map(|m| m.as_str().parse::().ok()) + .flatten(); + + let proposals = re_proposals + .captures(&metrics_response) + .and_then(|caps| caps.get(1)) + .map(|m| m.as_str().parse::().ok()) + .flatten(); + + let responses = re_responses + .captures(&metrics_response) + .and_then(|caps| caps.get(1)) + .map(|m| m.as_str().parse::().ok()) + .flatten(); + + if let (Some(proposals), Some(responses), Some(precommits)) = + (proposals, responses, precommits) + { + Ok(proposals >= min_num_expected + && responses >= min_num_expected + && precommits >= min_num_expected) + } else { + Ok(false) + } }) .expect("Failed to advance prometheus metrics"); } @@ -4625,7 +4637,7 @@ fn tx_replay_with_fork_after_empty_tenures_before_starting_replaying_txs() { let sender1_nonce_post_fork = get_account(&http_origin, &sender1_addr).nonce; assert_eq!(0, sender1_nonce_post_fork); - info!("------------------- Produce Empty Tenuree -------------------------"); + info!("------------------- Produce Empty Tenure -------------------------"); fault_injection_unstall_miner(); let tip = get_chain_info(&conf); _ = wait_for_tenure_change_tx(30, TenureChangeCause::BlockFound, tip.stacks_tip_height + 1); @@ -7066,17 +7078,7 @@ fn empty_tenure_delayed() { } = signer_test.running_nodes.counters.clone(); info!("------------------------- Test Mine Regular Tenure A -------------------------"); - let info_before = signer_test.get_peer_info(); - // Mine a regular tenure, but wait for commits to be submitted - next_block_and( - &signer_test.running_nodes.btc_regtest_controller, - 60, - || { - let info = signer_test.get_peer_info(); - Ok(info.stacks_tip_height > info_before.stacks_tip_height) - }, - ) - .unwrap(); + signer_test.mine_nakamoto_block(Duration::from_secs(30), true); signer_test.check_signer_states_normal(); info!("------------------------- Test Mine Empty Tenure B -------------------------"); @@ -7217,17 +7219,14 @@ fn empty_sortition_before_approval() { signer_test.boot_to_epoch_3(); - let skip_commit_op = signer_test - .running_nodes - .counters - .naka_skip_commit_op - .clone(); - let proposed_blocks = signer_test - .running_nodes - .counters - .naka_proposed_blocks - .clone(); + let Counters { + naka_submitted_commits: commits_submitted, + naka_proposed_blocks: proposed_blocks, + naka_skip_commit_op: skip_commit_op, + .. + } = signer_test.running_nodes.counters.clone(); + let commits_before = commits_submitted.load(Ordering::SeqCst); next_block_and_process_new_stacks_block( &signer_test.running_nodes.btc_regtest_controller, 60, @@ -7235,8 +7234,12 @@ fn empty_sortition_before_approval() { ) .unwrap(); + wait_for(30, || { + Ok(commits_submitted.load(Ordering::SeqCst) > commits_before) + }) + .expect("Timed out waiting for commit to be submitted for Tenure A"); + let info = get_chain_info(&signer_test.running_nodes.conf); - let burn_height_before = info.burn_block_height; let stacks_height_before = info.stacks_tip_height; info!("Forcing miner to ignore signatures for next block"); @@ -7259,15 +7262,7 @@ fn empty_sortition_before_approval() { info!("------------------------- Test Mine Empty Tenure B -------------------------"); // Trigger an empty tenure - next_block_and( - &signer_test.running_nodes.btc_regtest_controller, - 60, - || { - let burn_height = get_chain_info(&signer_test.running_nodes.conf).burn_block_height; - Ok(burn_height == burn_height_before + 2) - }, - ) - .expect("Failed to mine empty tenure"); + signer_test.mine_bitcoin_block(); signer_test.check_signer_states_normal_missed_sortition(); info!("Unpause block commits"); @@ -9291,7 +9286,7 @@ fn locally_rejected_blocks_overriden_by_global_acceptance() { /// The stacks node is then advanced to Epoch 3.0 boundary to allow block signing. /// /// Test Execution: -/// The node mines 1 stacks block N (all signers sign it). The subsequent block N+1 is proposed, but <30% accept it. The remaining signers +/// The node mines 1 stacks block N (all signers sign it). The subsequent block N+1 is proposed, but <30% pre-commit to it. The remaining signers /// do not make a decision on the block. A new tenure begins and the miner proposes a new block N+1' which all signers accept. /// /// Test Assertion: @@ -9382,7 +9377,7 @@ fn reorg_locally_accepted_blocks_across_tenures_succeeds() { .cloned() .skip(num_signers * 7 / 10) .collect(); - TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(ignoring_signers.clone()); + TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST.set(ignoring_signers.clone()); // Clear the stackerdb chunks test_observer::clear(); @@ -9402,12 +9397,12 @@ fn reorg_locally_accepted_blocks_across_tenures_succeeds() { wait_for_block_proposal(30, info_before.stacks_tip_height + 1, &miner_pk) .expect("Timed out waiting for block N+1 to be proposed"); // Make sure that the non ignoring signers do actually accept it though - wait_for_block_acceptance_from_signers( + wait_for_block_pre_commits_from_signers( 30, &block_n_1_proposal.header.signer_signature_hash(), &non_ignoring_signers, ) - .expect("Timed out waiting for block acceptances of N+1"); + .expect("Timed out waiting for block pre-commits of N+1"); let info_after = signer_test.get_peer_info(); assert_eq!(info_after, info_before); assert_ne!( @@ -9450,7 +9445,7 @@ fn reorg_locally_accepted_blocks_across_tenures_succeeds() { ); let info_before = signer_test.get_peer_info(); test_observer::clear(); - TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(Vec::new()); + TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST.set(Vec::new()); TEST_MINE_SKIP.set(false); let block_n_1_prime = @@ -9600,7 +9595,7 @@ fn reorg_locally_accepted_blocks_across_tenures_fails() { .cloned() .skip(num_signers * 7 / 10) .collect(); - TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(ignoring_signers.clone()); + TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST.set(ignoring_signers.clone()); // Clear the stackerdb chunks test_observer::clear(); @@ -10551,19 +10546,10 @@ fn block_commit_delay() { TEST_REJECT_ALL_BLOCK_PROPOSAL.set(all_signers); info!("------------------------- Test Mine Burn Block -------------------------"); - let burn_height_before = get_chain_info(&signer_test.running_nodes.conf).burn_block_height; let commits_before = commits_submitted.load(Ordering::SeqCst); // Mine a burn block and wait for it to be processed. - next_block_and( - &signer_test.running_nodes.btc_regtest_controller, - 60, - || { - let burn_height = get_chain_info(&signer_test.running_nodes.conf).burn_block_height; - Ok(burn_height > burn_height_before) - }, - ) - .unwrap(); + signer_test.mine_bitcoin_block(); // Sleep an extra minute to ensure no block commits are sent sleep_ms(60_000); @@ -11138,17 +11124,7 @@ fn new_tenure_while_validating_previous_scenario() { info!("----- Mining a new BTC block -----"); TEST_MINE_SKIP.set(true); - next_block_and( - &signer_test.running_nodes.btc_regtest_controller, - 30, - || { - Ok( - get_chain_info(&signer_test.running_nodes.conf).burn_block_height - > info_before.burn_block_height, - ) - }, - ) - .unwrap(); + signer_test.mine_bitcoin_block(); let info = signer_test.get_peer_info(); wait_for_state_machine_update_by_miner_tenure_id( @@ -11892,17 +11868,7 @@ fn global_acceptance_depends_on_block_announcement() { TEST_REJECT_ALL_BLOCK_PROPOSAL.set(Vec::new()); TEST_IGNORE_SIGNERS.set(false); test_observer::clear(); - next_block_and( - &signer_test.running_nodes.btc_regtest_controller, - 60, - || { - Ok( - get_chain_info(&signer_test.running_nodes.conf).burn_block_height - > info_before.burn_block_height, - ) - }, - ) - .unwrap(); + signer_test.mine_bitcoin_block(); let info = get_chain_info(&signer_test.running_nodes.conf); info!( @@ -12824,7 +12790,7 @@ fn injected_signatures_are_ignored_across_boundaries() { .collect(); assert_eq!(ignoring_signers.len(), 3); assert_eq!(non_ignoring_signers.len(), 2); - TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(ignoring_signers.clone()); + TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST.set(ignoring_signers.clone()); let info_before = signer_test.get_peer_info(); // submit a tx so that the miner will ATTEMPT to mine a stacks block N @@ -12841,7 +12807,7 @@ fn injected_signatures_are_ignored_across_boundaries() { info!("Submitted tx {tx} in attempt to mine block N"); let mut new_signature_hash = None; wait_for(30, || { - let accepted_signers = test_observer::get_stackerdb_chunks() + let accepted_signers: HashSet<_> = test_observer::get_stackerdb_chunks() .into_iter() .flat_map(|chunk| chunk.modified_slots) .filter_map(|chunk| { @@ -12851,12 +12817,13 @@ fn injected_signatures_are_ignored_across_boundaries() { new_signature_hash = Some(accepted.signer_signature_hash); return non_ignoring_signers.iter().find(|key| { key.verify(accepted.signer_signature_hash.bits(), &accepted.signature) - .is_ok() + .unwrap() }); } None - }); - Ok(accepted_signers.count() + ignoring_signers.len() == new_num_signers) + }) + .collect(); + Ok(accepted_signers.len() + ignoring_signers.len() == new_num_signers) }) .expect("FAIL: Timed out waiting for block proposal acceptance"); let new_signature_hash = new_signature_hash.expect("Failed to get new signature hash"); @@ -13218,16 +13185,8 @@ fn reorg_attempts_activity_timeout_exceeded() { info!("------------------------- Start Tenure B -------------------------"); test_observer::clear(); - let chain_before = get_chain_info(&signer_test.running_nodes.conf); - next_block_and( - &signer_test.running_nodes.btc_regtest_controller, - 60, - || { - let chain_info = get_chain_info(&signer_test.running_nodes.conf); - Ok(chain_info.burn_block_height > chain_before.burn_block_height) - }, - ) - .unwrap(); + let chain_before = chain_after; + signer_test.mine_bitcoin_block(); let chain_after = get_chain_info(&signer_test.running_nodes.conf); wait_for_state_machine_update_by_miner_tenure_id( 30, @@ -14034,14 +13993,8 @@ fn disallow_reorg_within_first_proposal_burn_block_timing_secs_but_more_than_one get_chain_info(&conf_1).stacks_tip_height, block_n_height + 3 ); - let burn_height_before = get_chain_info(&conf_1).burn_block_height; info!("------------------------- Miner 1 Wins the Next Tenure, Mines N+1', got rejected -------------------------"); - next_block_and(miners.btc_regtest_controller_mut(), 30, || { - let info = get_chain_info(&conf_1); - Ok(info.burn_block_height == burn_height_before + 1) - }) - .expect("Failed to advance chain tip"); - + miners.signer_test.mine_bitcoin_block(); // assure we have a successful sortition that miner 1 won verify_sortition_winner(&sortdb, &miner_pkh_1); // wait for a block N+1' proposal from miner1 @@ -15560,6 +15513,7 @@ fn mark_miner_as_invalid_if_reorg_is_rejected_v1() { info!("------------------------- Miner 1 Mines a Nakamoto Block N -------------------------"); let info_before = get_chain_info(&conf_1); + // Because rl1 is not submitting commits, we cannot use mine_nakamoto_block (commit will never advance) next_block_and(&miners.btc_regtest_controller_mut(), 30, || { let chain_info = get_chain_info(&conf_1); Ok(chain_info.stacks_tip_height > info_before.stacks_tip_height) @@ -15589,12 +15543,7 @@ fn mark_miner_as_invalid_if_reorg_is_rejected_v1() { fault_injection_stall_miner(); info!("------------------------- Mine 2 wins the Next Tenure -------------------------"); - let info_before = info_after; - next_block_and(&miners.btc_regtest_controller_mut(), 30, || { - let chain_info = get_chain_info(&conf_1); - Ok(chain_info.burn_block_height > info_before.burn_block_height) - }) - .expect("Failed to build BTC block"); + miners.signer_test.mine_bitcoin_block(); verify_sortition_winner(&sortdb, &miner_pkh_2); miners.signer_test.check_signer_states_normal(); @@ -15616,12 +15565,7 @@ fn mark_miner_as_invalid_if_reorg_is_rejected_v1() { info!("------------------------- Miner 1 Wins the Next Tenure, Mines N+1' -------------------------"); test_observer::clear(); - let info_before = info_after; - next_block_and(&miners.btc_regtest_controller_mut(), 30, || { - let chain_info = get_chain_info(&conf_1); - Ok(chain_info.burn_block_height > info_before.burn_block_height) - }) - .expect("Failed to build BTC block"); + miners.signer_test.mine_bitcoin_block(); let block_n_1_prime = wait_for_block_proposal(30, block_n_height + 1, &miner_pk_1) .expect("Failed to get block proposal N+1'"); @@ -15631,10 +15575,8 @@ fn mark_miner_as_invalid_if_reorg_is_rejected_v1() { .signer_test .check_signer_states_reorg(&approving_signers, &rejecting_signers); - info!("------------------------- Wait for 3 acceptances and 2 rejections -------------------------"); let signer_signature_hash = block_n_1_prime.header.signer_signature_hash(); - wait_for_block_acceptance_from_signers(30, &signer_signature_hash, &approving_signers) - .expect("Timed out waiting for block acceptance from approving signers"); + info!("------------------------- Wait for 3 acceptances and 2 rejections of {signer_signature_hash} -------------------------"); let rejections = wait_for_block_rejections_from_signers(30, &signer_signature_hash, &rejecting_signers) .expect("Timed out waiting for block rejection from rejecting signers"); @@ -15645,6 +15587,8 @@ fn mark_miner_as_invalid_if_reorg_is_rejected_v1() { "Reject reason is not ReorgNotAllowed" ); } + wait_for_block_pre_commits_from_signers(30, &signer_signature_hash, &approving_signers) + .expect("Timed out waiting for block pre-commits from approving signers"); info!("------------------------- Miner 1 Proposes N+1' Again -------------------------"); test_observer::clear(); @@ -17245,16 +17189,7 @@ fn burn_block_height_behavior() { skip_commit_op.set(true); // Mine a regular tenure - let info_before = signer_test.get_peer_info(); - next_block_and( - &signer_test.running_nodes.btc_regtest_controller, - 60, - || { - let chain_info = get_chain_info(&signer_test.running_nodes.conf); - Ok(chain_info.stacks_tip_height > info_before.stacks_tip_height) - }, - ) - .expect("Timed out waiting for block"); + signer_test.mine_nakamoto_block_without_commit(Duration::from_secs(30), true); let info = get_chain_info(&signer_test.running_nodes.conf); let stacks_height_before = info.stacks_tip_height; @@ -17698,9 +17633,11 @@ fn rollover_signer_protocol_version() { .collect(); TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION.set(pinned_signers_versions); - info!("------------------------- Confirm Signers Sign The Block After Complete Downgraded Version Number -------------------------"); - signer_test.mine_and_verify_confirmed_naka_block(Duration::from_secs(30), num_signers, true); - + // Not strictly necessary, but makes it easier to logic out if miner doesn't send a proposal until signers are on same page... + TEST_MINE_SKIP.set(true); + info!("------------------------- Confirm Signers Sent Downgraded State Machine Updates -------------------------"); + // Cannot use any built in functions that call mine_nakamoto_block since it expects signer updates matching the majority version and we are manually messing with these versions + signer_test.mine_bitcoin_block(); let tip = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()).unwrap(); let burn_consensus_hash = tip.consensus_hash; let burn_height = tip.block_height; @@ -17719,6 +17656,24 @@ fn rollover_signer_protocol_version() { ) .expect("Timed out waiting for signers to send their state update for block N+2"); + let info = signer_test.get_peer_info(); + info!("------------------------- Confirm Signers Sign The Block After Complete Downgraded Version Number -------------------------"); + TEST_MINE_SKIP.set(false); + let expected_miner = StacksPublicKey::from_private( + &signer_test + .running_nodes + .conf + .miner + .mining_key + .clone() + .unwrap(), + ); + let block = wait_for_block_pushed_by_miner_key(60, info.stacks_tip_height + 1, &expected_miner) + .expect("Failed to mine block after downgraded version number."); + // Expect ALL signers even after downgrade to approve the proposed blocks + wait_for_block_acceptance_from_signers(30, &block.header.signer_signature_hash(), &all_signers) + .expect("Failed to confirm all signers accepted last block"); + info!("------------------------- Reset All Signers to {SUPPORTED_SIGNER_PROTOCOL_VERSION} -------------------------"); TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION.set(HashMap::new()); test_observer::clear(); @@ -18275,6 +18230,7 @@ fn multiversioned_signer_protocol_version_calculation() { .unwrap(); info!("------------------------- Resuming Mining of Tenure Start Block for Tenure A -------------------------"); + test_observer::clear(); TEST_MINE_SKIP.set(false); wait_for(30, || { Ok(signer_test.get_peer_info().stacks_tip_height > peer_info_before.stacks_tip_height) @@ -18359,14 +18315,9 @@ fn signer_loads_stackerdb_updates_on_startup() { skip_commit_op_rl1.set(true); info!("------------------------- Miner A Wins Tenure A -------------------------"); - let info_before = get_chain_info(&conf_1); // Let's not mine anything until we see consensus on new tenure start. TEST_MINE_SKIP.set(true); - next_block_and(&miners.btc_regtest_controller_mut(), 60, || { - let info = get_chain_info(&conf_1); - Ok(info.burn_block_height > info_before.burn_block_height) - }) - .unwrap(); + miners.signer_test.mine_bitcoin_block(); let chain_after = get_chain_info(&conf_1); wait_for_state_machine_update_by_miner_tenure_id( 30, @@ -18392,14 +18343,9 @@ fn signer_loads_stackerdb_updates_on_startup() { info!("------------------------- Miner B Wins Tenure B -------------------------"); miners.submit_commit_miner_2(&sortdb); - let chain_before = get_chain_info(&conf_1); // Let's not mine anything until we see consensus on new tenure start. TEST_MINE_SKIP.set(true); - next_block_and(&miners.btc_regtest_controller_mut(), 60, || { - let info = get_chain_info(&conf_1); - Ok(info.burn_block_height > chain_before.burn_block_height) - }) - .unwrap(); + miners.signer_test.mine_bitcoin_block(); let chain_after = get_chain_info(&conf_1); wait_for_state_machine_update_by_miner_tenure_id( 30, @@ -18462,3 +18408,256 @@ fn signer_loads_stackerdb_updates_on_startup() { info!("------------------------- Shutdown -------------------------"); miners.shutdown(); } + +// Basic test to ensure that signers will not issue a signature over a block proposal unless +// a threshold number of signers have pre-committed to sign. +#[test] +#[ignore] +fn signers_do_not_commit_unless_threshold_precommitted() { + if env::var("BITCOIND_TEST") != Ok("1".into()) { + return; + } + + info!("------------------------- Test Setup -------------------------"); + let num_signers = 20; + + let mut signer_test: SignerTest = SignerTest::new(num_signers, vec![]); + let miner_sk = signer_test + .running_nodes + .conf + .miner + .mining_key + .clone() + .unwrap(); + let miner_pk = StacksPublicKey::from_private(&miner_sk); + let all_signers = signer_test.signer_test_pks(); + + signer_test.boot_to_epoch_3(); + + // Make sure that more than 30% of signers are set to ignore any incoming proposals so that consensus is not reached + // on pre-commit round. + let (ignore_slice, pre_commit_slice) = all_signers.split_at(all_signers.len() / 2); + let ignore_signers: Vec<_> = ignore_slice.to_vec(); + let pre_commit_signers: Vec<_> = pre_commit_slice.to_vec(); + TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(ignore_signers); + test_observer::clear(); + let blocks_before = test_observer::get_mined_nakamoto_blocks().len(); + let height_before = signer_test.get_peer_info().stacks_tip_height; + next_block_and( + &mut signer_test.running_nodes.btc_regtest_controller, + 30, + || Ok(test_observer::get_mined_nakamoto_blocks().len() > blocks_before), + ) + .unwrap(); + + let proposal = wait_for_block_proposal(30, height_before + 1, &miner_pk) + .expect("Timed out waiting for block proposal"); + let hash = proposal.header.signer_signature_hash(); + wait_for_block_pre_commits_from_signers(30, &hash, &pre_commit_signers) + .expect("Timed out waiting for pre-commits"); + assert!( + wait_for(30, || { + for chunk in test_observer::get_stackerdb_chunks() + .into_iter() + .flat_map(|chunk| chunk.modified_slots) + { + let message = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()) + .expect("Failed to deserialize SignerMessage"); + if let SignerMessage::BlockResponse(BlockResponse::Accepted(accepted)) = message { + if accepted.signer_signature_hash == hash { + return Ok(true); + } + } + } + Ok(false) + }) + .is_err(), + "Should not have found a single block accept for the block hash {hash}" + ); + + info!("------------------------- Shutdown -------------------------"); + signer_test.shutdown(); +} + +// Test to ensure a signer operating a two phase commit signer will treat +// signatures from other signers as pre-commits if it has yet to see their pre-commits +// for that block. This enables upgraded pre-commit signers to operate as they should +// with unupgraded signers or if the pre-commit message was somehow dropped. +#[test] +#[ignore] +fn signers_treat_signatures_as_precommits() { + if env::var("BITCOIND_TEST") != Ok("1".into()) { + return; + } + + info!("------------------------- Test Setup -------------------------"); + let num_signers = 3; + + let signer_test: SignerTest = SignerTest::new(num_signers, vec![]); + let miner_sk = signer_test + .running_nodes + .conf + .miner + .mining_key + .clone() + .unwrap(); + let miner_pk = StacksPublicKey::from_private(&miner_sk); + let all_signers = signer_test.signer_test_pks(); + + signer_test.boot_to_epoch_3(); + + let operating_signer = all_signers[0].clone(); + let disabled_signers = all_signers[1..].to_vec(); + + // Disable a majority of signers so that we can inject our own custom signatures to simulate an un-upgraded signer. + + info!( + "------------------------- Disabling {} Signers -------------------------", + disabled_signers.len() + ); + + TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(disabled_signers.clone()); + let peer_info = signer_test.get_peer_info(); + + info!( + "------------------------- Trigger Tenure Change Block Proposal -------------------------" + ); + signer_test.mine_bitcoin_block(); + + let block_proposal = wait_for_block_proposal(30, peer_info.stacks_tip_height + 1, &miner_pk) + .expect("Failed to propose a new tenure block"); + + info!( + "------------------------- Verify Only Operating Signer Issues Pre-Commit -------------------------" + ); + + let signer_signature_hash = block_proposal.header.signer_signature_hash(); + wait_for_block_pre_commits_from_signers( + 30, + &signer_signature_hash, + &[operating_signer.clone()], + ) + .expect("Operating signer did not send a pre-commit"); + assert!( + wait_for_block_pre_commits_from_signers(10, &signer_signature_hash, &disabled_signers) + .is_err(), + "Disabled signers should not have issued any pre-commits" + ); + + test_observer::clear(); + + let reward_cycle = signer_test.get_current_reward_cycle(); + // Do not send a signature for the operating signer. Just for the disabled. The operating signer should then issue as signature only after the other 2 signers send their signature + // Only the operating signer should send a block pre commit. + for (i, signer_private_key) in signer_test + .signer_stacks_private_keys + .iter() + .enumerate() + .skip(1) + { + let signature = signer_private_key + .sign(signer_signature_hash.bits()) + .expect("Failed to sign block"); + let accepted = BlockResponse::accepted( + block_proposal.header.signer_signature_hash(), + signature, + get_epoch_time_secs().wrapping_add(u64::MAX), + ); + + let signers_contract_id = + MessageSlotID::BlockResponse.stacker_db_contract(false, reward_cycle); + let mut session = StackerDBSession::new( + &signer_test.running_nodes.conf.node.rpc_bind, + signers_contract_id, + ); + let message = SignerMessage::BlockResponse(accepted); + + // Manually submit signature + let mut accepted = false; + let mut version = 0; + let start = Instant::now(); + info!( + "------------------------- Manually Submitting Signer {i} Block Approval ------------------------", + ); + // Don't know which slot corresponds to which signer, so just try all of them :) + let mut slot_id = 0; + while !accepted { + let mut chunk = StackerDBChunkData::new(slot_id, version, message.serialize_to_vec()); + chunk + .sign(&signer_private_key) + .expect("Failed to sign message chunk"); + debug!("Produced a signature: {:?}", chunk.sig); + let result = session.put_chunk(&chunk).expect("Failed to put chunk"); + accepted = result.accepted; + if !accepted && result.code.unwrap() == StackerDBErrorCodes::BadSigner as u32 { + slot_id += 1; + assert!( + slot_id < num_signers as u32, + "Failed to find a matching slot id" + ); + continue; + } + version += 1; + debug!("Test Put Chunk ACK: {result:?}"); + assert!( + start.elapsed() < Duration::from_secs(30), + "Timed out waiting for signer signature to be accepted" + ); + } + if i == 1 { + // Signer will not have seen enough signatures (fake pre-commits) to reach threshold + info!("------------------------- Verifying Operating Signer Does NOT Issue a Signature ------------------------"); + } else { + // Signer will have seen enough signatures (fake pre-commits) to reach threshold + info!("------------------------- Verifying Operating Signer Issues a Signature ------------------------"); + } + let result = wait_for(20, || { + for chunk in test_observer::get_stackerdb_chunks() + .into_iter() + .flat_map(|chunk| chunk.modified_slots) + { + let message = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()) + .expect("Failed to deserialize SignerMessage"); + let SignerMessage::BlockResponse(BlockResponse::Accepted(accepted)) = message + else { + continue; + }; + assert_eq!( + accepted.signer_signature_hash, signer_signature_hash, + "Got an acceptance message for an unknown proposal" + ); + let signed_by_operating_signer = operating_signer + .verify(signer_signature_hash.bits(), &accepted.signature) + .unwrap(); + if i == 1 { + assert!(!signed_by_operating_signer, "The operating signer should only issue a signature once it sees BOTH signatures from the other signers"); + } else if signed_by_operating_signer { + return Ok(true); + } + } + Ok(false) + }); + // If this is the first iteration of the loop (which starts from 1 since we skipped), the operating signer should do nothing (has yet to reach the threshold) + if i == 1 { + assert!( + result.is_err(), + "We saw a signature from the operating signer before our other two signers issued their signatures!" + ); + } else { + assert!( + result.is_ok(), + "We never saw our operating signer issue a signature!" + ); + } + } + + info!("------------------------- Ensure Chain Advances -------------------------"); + + wait_for(30, || { + Ok(signer_test.get_peer_info().stacks_tip_height > peer_info.stacks_tip_height) + }) + .expect("We failed to mine the tenure change block"); + + info!("------------------------- Shutdown -------------------------"); + signer_test.shutdown(); +} diff --git a/stacks-signer/CHANGELOG.md b/stacks-signer/CHANGELOG.md index 4a3f341afa..b8e2f158c8 100644 --- a/stacks-signer/CHANGELOG.md +++ b/stacks-signer/CHANGELOG.md @@ -5,6 +5,16 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to the versioning scheme outlined in the [README.md](README.md). +## [Unreleased] + +### Added + +- Added two-phase commit to signer block responses ensuring signers only issue a signature in a BlockResponse when a majority threshold number have pre-committed to sign a proposed Naka block + +### Changed + +- Database schema updated to version 17 + ## [3.2.0.0.1.0] ### Changed diff --git a/stacks-signer/src/monitoring/mod.rs b/stacks-signer/src/monitoring/mod.rs index dabf529b6b..d6da832c44 100644 --- a/stacks-signer/src/monitoring/mod.rs +++ b/stacks-signer/src/monitoring/mod.rs @@ -91,6 +91,11 @@ pub mod actions { BLOCK_PROPOSALS_RECEIVED.inc(); } + /// Increment the block pre-commit sent counter + pub fn increment_block_pre_commits_sent() { + BLOCK_PRE_COMMITS_SENT.inc(); + } + /// Update the stx balance of the signer pub fn update_signer_stx_balance(balance: i64) { SIGNER_STX_BALANCE.set(balance); @@ -206,6 +211,9 @@ pub mod actions { /// Increment the number of block proposals received pub fn increment_block_proposals_received() {} + /// Increment the block pre-commits sent counter + pub fn increment_block_pre_commits_sent() {} + /// Update the stx balance of the signer pub fn update_signer_stx_balance(_balance: i64) {} diff --git a/stacks-signer/src/monitoring/prometheus.rs b/stacks-signer/src/monitoring/prometheus.rs index 2114b7f4cd..84b28b7310 100644 --- a/stacks-signer/src/monitoring/prometheus.rs +++ b/stacks-signer/src/monitoring/prometheus.rs @@ -48,6 +48,11 @@ lazy_static! { "The number of block proposals received by the signer" )) .unwrap(); + pub static ref BLOCK_PRE_COMMITS_SENT: IntCounter = register_int_counter!(opts!( + "stacks_signer_block_pre_commits_sent", + "The number of block pre-commits sent by the signer" + )) + .unwrap(); pub static ref CURRENT_REWARD_CYCLE: IntGauge = register_int_gauge!(opts!( "stacks_signer_current_reward_cycle", "The current reward cycle" diff --git a/stacks-signer/src/signerdb.rs b/stacks-signer/src/signerdb.rs index 905538e470..89a073ee2c 100644 --- a/stacks-signer/src/signerdb.rs +++ b/stacks-signer/src/signerdb.rs @@ -673,6 +673,23 @@ CREATE TABLE IF NOT EXISTS block_rejection_signer_addrs ( PRIMARY KEY (signer_signature_hash, signer_addr) ) STRICT;"#; +static CREATE_BLOCK_PRE_COMMITS_TABLE: &str = r#" +CREATE TABLE IF NOT EXISTS block_pre_commits ( + -- The block sighash commits to all of the stacks and burnchain state as of its parent, + -- as well as the tenure itself so there's no need to include the reward cycle. Just + -- the sighash is sufficient to uniquely identify the block across all burnchain, PoX, + -- and stacks forks. + signer_signature_hash TEXT NOT NULL, + -- signer address committing to sign the block + signer_addr TEXT NOT NULL, + PRIMARY KEY (signer_signature_hash, signer_addr) +) STRICT;"#; + +/// Used by get_block_pre_committers +static CREATE_BLOCK_PRE_COMMITS_BY_SIGHASH_INDEX: &str = r#" +CREATE INDEX idx_block_pre_commits_by_sighash ON block_pre_commits(signer_signature_hash); +"#; + static SCHEMA_1: &[&str] = &[ DROP_SCHEMA_0, CREATE_DB_CONFIG, @@ -786,6 +803,12 @@ static SCHEMA_16: &[&str] = &[ "INSERT INTO db_config (version) VALUES (16);", ]; +static SCHEMA_17: &[&str] = &[ + CREATE_BLOCK_PRE_COMMITS_TABLE, + CREATE_BLOCK_PRE_COMMITS_BY_SIGHASH_INDEX, + "INSERT INTO db_config (version) VALUES (17);", +]; + struct Migration { version: u32, statements: &'static [&'static str], @@ -856,11 +879,15 @@ static MIGRATIONS: &[Migration] = &[ version: 16, statements: SCHEMA_16, }, + Migration { + version: 17, + statements: SCHEMA_17, + }, ]; impl SignerDb { /// The current schema version used in this build of the signer binary. - pub const SCHEMA_VERSION: u32 = 16; + pub const SCHEMA_VERSION: u32 = 17; /// Create a new `SignerState` instance. /// This will create a new SQLite database at the given path @@ -1830,6 +1857,62 @@ impl SignerDb { Ok(None) } + + /// Record an observed block pre-commit + pub fn add_block_pre_commit( + &self, + block_sighash: &Sha512Trunc256Sum, + address: &StacksAddress, + ) -> Result<(), DBError> { + let qry = "INSERT OR REPLACE INTO block_pre_commits (signer_signature_hash, signer_addr) VALUES (?1, ?2);"; + let args = params![block_sighash, address.to_string()]; + + debug!("Inserting block pre-commit."; + "signer_signature_hash" => %block_sighash, + "signer_addr" => %address); + + self.db.execute(qry, args)?; + Ok(()) + } + + /// Check if the given address has already committed to sign the block identified by block_sighash + pub fn has_committed( + &self, + block_sighash: &Sha512Trunc256Sum, + address: &StacksAddress, + ) -> Result { + let qry_check = " + SELECT 1 FROM block_pre_commits + WHERE signer_signature_hash = ?1 AND signer_addr = ?2 + LIMIT 1;"; + + let exists: Option = self + .db + .query_row( + qry_check, + params![block_sighash, address.to_string()], + |row| row.get(0), + ) + .optional()?; + + Ok(exists.is_some()) + } + + /// Get all pre-committers for a block + pub fn get_block_pre_committers( + &self, + block_sighash: &Sha512Trunc256Sum, + ) -> Result, DBError> { + let qry = "SELECT signer_addr FROM block_pre_commits WHERE signer_signature_hash = ?1"; + let args = params![block_sighash]; + let addrs_txt: Vec = query_rows(&self.db, qry, args)?; + + let res: Result, _> = addrs_txt + .into_iter() + .map(|addr| StacksAddress::from_string(&addr).ok_or(DBError::Corruption)) + .collect(); + res + } } fn try_deserialize(s: Option) -> Result, DBError> @@ -3390,4 +3473,56 @@ pub mod tests { assert!(result_3.is_none()); } + + #[test] + fn insert_and_get_state_block_pre_commits() { + let db_path = tmp_db_path(); + let db = SignerDb::new(db_path).expect("Failed to create signer db"); + let block_sighash1 = Sha512Trunc256Sum([1u8; 32]); + let address1 = StacksAddress::p2pkh( + false, + &StacksPublicKey::from_private(&StacksPrivateKey::random()), + ); + let block_sighash2 = Sha512Trunc256Sum([2u8; 32]); + let address2 = StacksAddress::p2pkh( + false, + &StacksPublicKey::from_private(&StacksPrivateKey::random()), + ); + let address3 = StacksAddress::p2pkh( + false, + &StacksPublicKey::from_private(&StacksPrivateKey::random()), + ); + assert!(db + .get_block_pre_committers(&block_sighash1) + .unwrap() + .is_empty()); + + db.add_block_pre_commit(&block_sighash1, &address1).unwrap(); + assert_eq!( + db.get_block_pre_committers(&block_sighash1).unwrap(), + vec![address1.clone()] + ); + + db.add_block_pre_commit(&block_sighash1, &address2).unwrap(); + let commits = db.get_block_pre_committers(&block_sighash1).unwrap(); + assert_eq!(commits.len(), 2); + assert!(commits.contains(&address2)); + assert!(commits.contains(&address1)); + + db.add_block_pre_commit(&block_sighash2, &address3).unwrap(); + let commits = db.get_block_pre_committers(&block_sighash1).unwrap(); + assert_eq!(commits.len(), 2); + assert!(commits.contains(&address2)); + assert!(commits.contains(&address1)); + let commits = db.get_block_pre_committers(&block_sighash2).unwrap(); + assert_eq!(commits.len(), 1); + assert!(commits.contains(&address3)); + + assert!(db.has_committed(&block_sighash1, &address1).unwrap()); + assert!(db.has_committed(&block_sighash1, &address2).unwrap()); + assert!(!db.has_committed(&block_sighash1, &address3).unwrap()); + assert!(!db.has_committed(&block_sighash2, &address1).unwrap()); + assert!(!db.has_committed(&block_sighash2, &address2).unwrap()); + assert!(db.has_committed(&block_sighash2, &address3).unwrap()); + } } diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index cff3dcacd2..90ca62494c 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -452,21 +452,22 @@ impl Signer { let valid = block_info.valid?; let response = if valid { debug!("{self}: Accepting block {}", block_info.block.block_id()); - self.create_block_acceptance(&block_info.block) + self.create_block_acceptance(&block_info.block).into() } else { debug!("{self}: Rejecting block {}", block_info.block.block_id()); self.create_block_rejection(RejectReason::RejectedInPriorRound, &block_info.block) + .into() }; Some(response) } - /// Create a block acceptance response for a block - pub fn create_block_acceptance(&self, block: &NakamotoBlock) -> BlockResponse { + /// Create a block acceptance for a block + pub fn create_block_acceptance(&self, block: &NakamotoBlock) -> BlockAccepted { let signature = self .private_key .sign(block.header.signer_signature_hash().bits()) .expect("Failed to sign block"); - BlockResponse::accepted( + BlockAccepted::new( block.header.signer_signature_hash(), signature, self.signer_db.calculate_tenure_extend_timestamp( @@ -518,6 +519,15 @@ impl Signer { ), SignerMessage::StateMachineUpdate(update) => self .handle_state_machine_update(signer_public_key, update, received_time), + SignerMessage::BlockPreCommit(signer_signature_hash) => { + let stacker_address = + StacksAddress::p2pkh(self.mainnet, signer_public_key); + self.handle_block_pre_commit( + stacks_client, + &stacker_address, + signer_signature_hash, + ) + } _ => {} } } @@ -667,8 +677,8 @@ impl Signer { &self, reject_reason: RejectReason, block: &NakamotoBlock, - ) -> BlockResponse { - BlockResponse::rejected( + ) -> BlockRejection { + BlockRejection::new( block.header.signer_signature_hash(), reject_reason, &self.private_key, @@ -707,13 +717,13 @@ impl Signer { } /// Check if block should be rejected based on the appropriate state (either local or global) - /// Will return a BlockResponse::Rejection if the block is invalid, none otherwise. + /// Will return a BlockRejection if the block is invalid, none otherwise. fn check_block_against_state( &mut self, stacks_client: &StacksClient, sortition_state: &mut Option, block: &NakamotoBlock, - ) -> Option { + ) -> Option { // First update our global state evaluator with our local state if we have one let local_version = self.get_signer_protocol_version(); if let Ok(update) = self @@ -752,14 +762,14 @@ impl Signer { } /// Check if block should be rejected based on the local view of the sortition state - /// Will return a BlockResponse::Rejection if the block is invalid, none otherwise. + /// Will return a BlockRejection if the block is invalid, none otherwise. /// This is the pre-global signer state activation path. fn check_block_against_local_state( &mut self, stacks_client: &StacksClient, sortition_state: &mut Option, block: &NakamotoBlock, - ) -> Option { + ) -> Option { let signer_signature_hash = block.header.signer_signature_hash(); let block_id = block.block_id(); // Get sortition view if we don't have it @@ -813,13 +823,13 @@ impl Signer { } /// Check if block should be rejected based on global signer state - /// Will return a BlockResponse::Rejection if the block is invalid, none otherwise. + /// Will return a BlockRejection if the block is invalid, none otherwise. /// This is the Post-global signer state activation path fn check_block_against_global_state( &mut self, stacks_client: &StacksClient, block: &NakamotoBlock, - ) -> Option { + ) -> Option { let signer_signature_hash = block.header.signer_signature_hash(); let block_id = block.block_id(); let Some(global_state) = self.global_state_evaluator.determine_global_state() else { @@ -874,13 +884,9 @@ impl Signer { /// The actual `send_block_response` implementation. Declared so that we do /// not need to duplicate in testing. - fn impl_send_block_response( - &mut self, - block: Option<&NakamotoBlock>, - block_response: BlockResponse, - ) { + fn impl_send_block_response(&mut self, block: &NakamotoBlock, block_response: BlockResponse) { info!( - "{self}: Broadcasting a block response to stacks node: {block_response:?}"; + "{self}: Broadcasting block response to stacks node: {block_response:?}"; ); let accepted = matches!(block_response, BlockResponse::Accepted(..)); match self @@ -895,9 +901,7 @@ impl Signer { ); } crate::monitoring::actions::increment_block_responses_sent(accepted); - if let Some(block) = block { - crate::monitoring::actions::record_block_response_latency(block); - } + crate::monitoring::actions::record_block_response_latency(block); } Err(e) => { warn!("{self}: Failed to send block response to stacker-db: {e:?}",); @@ -906,11 +910,10 @@ impl Signer { } #[cfg(any(test, feature = "testing"))] - fn send_block_response( - &mut self, - block: Option<&NakamotoBlock>, - block_response: BlockResponse, - ) { + fn send_block_response(&mut self, block: &NakamotoBlock, block_response: BlockResponse) { + if self.test_skip_block_response_broadcast(&block_response) { + return; + } const NUM_REPEATS: usize = 1; let mut count = 0; let public_keys = TEST_REPEAT_PROPOSAL_RESPONSE.get(); @@ -928,14 +931,34 @@ impl Signer { } #[cfg(not(any(test, feature = "testing")))] - fn send_block_response( - &mut self, - block: Option<&NakamotoBlock>, - block_response: BlockResponse, - ) { + fn send_block_response(&mut self, block: &NakamotoBlock, block_response: BlockResponse) { self.impl_send_block_response(block, block_response) } + /// Send a pre block commit message to signers to indicate that we will be signing the proposed block + fn send_block_pre_commit(&mut self, signer_signature_hash: Sha512Trunc256Sum) { + info!( + "{self}: Broadcasting block pre-commit to stacks node for {signer_signature_hash}"; + ); + match self + .stackerdb + .send_message_with_retry(SignerMessage::BlockPreCommit(signer_signature_hash)) + { + Ok(ack) => { + if !ack.accepted { + warn!( + "{self}: Block pre-commit not accepted by stacker-db: {:?}", + ack.reason + ); + } + crate::monitoring::actions::increment_block_pre_commits_sent(); + } + Err(e) => { + warn!("{self}: Failed to send block pre-commit to stacker-db: {e:?}",); + } + } + } + /// Handle signer state update message fn handle_state_machine_update( &mut self, @@ -943,7 +966,9 @@ impl Signer { update: &StateMachineUpdate, received_time: &SystemTime, ) { - info!("{self}: Received a new state machine update from signer {signer_public_key:?}: {update:?}"); + info!( + "{self}: Received state machine update from signer {signer_public_key:?}: {update:?}" + ); let address = StacksAddress::p2pkh(self.mainnet, signer_public_key); // Store the state machine update so we can reload it if we crash if let Err(e) = self.signer_db.insert_state_machine_update( @@ -958,6 +983,94 @@ impl Signer { .insert_update(address, update.clone()); } + /// Handle pre-commit message from another signer + fn handle_block_pre_commit( + &mut self, + stacks_client: &StacksClient, + stacker_address: &StacksAddress, + block_hash: &Sha512Trunc256Sum, + ) { + debug!( + "{self}: Received pre-commit from signer ({stacker_address:?}) for block ({block_hash})", + ); + let Some(mut block_info) = self.block_lookup_by_reward_cycle(block_hash) else { + debug!("{self}: Received pre-commit for a block we have not seen before. Ignoring..."); + return; + }; + if block_info.has_reached_consensus() { + debug!( + "{self}: Received pre-commit for a block that is already marked as {}. Ignoring...", + block_info.state + ); + return; + }; + // Make sure the sender is part of our signing set + let is_valid_sender = self.signer_addresses.iter().any(|addr| { + // it only matters that the address hash bytes match + stacker_address.bytes() == addr.bytes() + }); + + if !is_valid_sender { + debug!("{self}: Received pre-commit message from an unknown sender {stacker_address:?}. Will not store."); + return; + } + + if self.signer_db.has_committed(block_hash, stacker_address).inspect_err(|e| warn!("Failed to check if pre-commit message already considered for {stacker_address:?} for {block_hash}: {e}")).unwrap_or(false) { + debug!("{self}: Already considered pre-commit message from {stacker_address:?} for {block_hash}. Ignoring..."); + return; + } + // commit message is from a valid sender! store it + self.signer_db + .add_block_pre_commit(block_hash, stacker_address) + .unwrap_or_else(|_| panic!("{self}: Failed to save block pre-commit")); + + // do we have enough pre-commits to reach consensus? + // i.e. is the threshold reached? + let committers = self + .signer_db + .get_block_pre_committers(block_hash) + .unwrap_or_else(|_| panic!("{self}: Failed to load block commits")); + + let commit_weight = self.compute_signature_signing_weight(committers.iter()); + let total_weight = self.compute_signature_total_weight(); + + let min_weight = NakamotoBlockHeader::compute_voting_weight_threshold(total_weight) + .unwrap_or_else(|_| { + panic!("{self}: Failed to compute threshold weight for {total_weight}") + }); + + if min_weight > commit_weight { + debug!( + "{self}: Not enough pre-committed to block {block_hash} (have {commit_weight}, need at least {min_weight}/{total_weight})" + ); + return; + } + + // have enough commits, so maybe we should actually broadcast our signature... + if block_info.valid == Some(false) { + // We already marked this block as invalid. We should not do anything further as we do not change our votes on rejected blocks. + debug!( + "{self}: Enough pre-committed to block {block_hash}, but we do not view the block as valid. Doing nothing." + ); + return; + } + // It is only considered globally accepted IFF we receive a new block event confirming it OR see the chain tip of the node advance to it. + if let Err(e) = block_info.mark_locally_accepted(false) { + if !block_info.has_reached_consensus() { + warn!("{self}: Failed to mark block as locally accepted: {e:?}",); + } + block_info.signed_self.get_or_insert(get_epoch_time_secs()); + } + + self.signer_db + .insert_block(&block_info) + .unwrap_or_else(|e| self.handle_insert_block_error(e)); + let accepted = self.create_block_acceptance(&block_info.block); + // have to save the signature _after_ the block info + self.handle_block_signature(stacks_client, &accepted); + self.send_block_response(&block_info.block, accepted.into()); + } + /// Handle block proposal messages submitted to signers stackerdb fn handle_block_proposal( &mut self, @@ -1046,16 +1159,16 @@ impl Signer { } // Check if proposal can be rejected now if not valid against sortition view - let block_response = + let block_rejection = self.check_block_against_state(stacks_client, sortition_state, &block_proposal.block); #[cfg(any(test, feature = "testing"))] - let block_response = - self.test_reject_block_proposal(block_proposal, &mut block_info, block_response); + let block_rejection = + self.test_reject_block_proposal(block_proposal, &mut block_info, block_rejection); - if let Some(block_response) = block_response { + if let Some(block_rejection) = block_rejection { // We know proposal is invalid. Send rejection message, do not do further validation and do not store it. - self.send_block_response(Some(&block_info.block), block_response); + self.send_block_response(&block_info.block, block_rejection.into()); } else { // Just in case check if the last block validation submission timed out. self.check_submitted_block_proposal(); @@ -1109,7 +1222,7 @@ impl Signer { return; }; - self.impl_send_block_response(Some(&block_info.block), block_response); + self.send_block_response(&block_info.block, block_response); } /// Handle block response messages from a signer @@ -1137,7 +1250,7 @@ impl Signer { &mut self, stacks_client: &StacksClient, proposed_block: &NakamotoBlock, - ) -> Option { + ) -> Option { let signer_signature_hash = proposed_block.header.signer_signature_hash(); // If this is a tenure change block, ensure that it confirms the correct number of blocks from the parent tenure. if let Some(tenure_change) = proposed_block.get_tenure_change_tx_payload() { @@ -1212,13 +1325,13 @@ impl Signer { } } - /// Handle the block validate ok response. Returns our block response if we have one + /// Handle the block validate ok response fn handle_block_validate_ok( &mut self, stacks_client: &StacksClient, block_validate_ok: &BlockValidateOk, sortition_state: &mut Option, - ) -> Option { + ) { crate::monitoring::actions::increment_block_validation_responses(true); let signer_signature_hash = block_validate_ok.signer_signature_hash; if self @@ -1247,17 +1360,16 @@ impl Signer { let Some(mut block_info) = self.block_lookup_by_reward_cycle(&signer_signature_hash) else { // We have not seen this block before. Why are we getting a response for it? debug!("{self}: Received a block validate response for a block we have not seen before. Ignoring..."); - return None; + return; }; if block_info.is_locally_finalized() { debug!("{self}: Received block validation for a block that is already marked as {}. Ignoring...", block_info.state); - return None; + return; } - if let Some(block_response) = + if let Some(block_rejection) = self.check_block_against_signer_db_state(stacks_client, &block_info.block) { - let block_rejection = block_response.as_block_rejection()?; // The signer db state has changed. We no longer view this block as valid. Override the validation response. if let Err(e) = block_info.mark_locally_rejected() { if !block_info.has_reached_consensus() { @@ -1267,13 +1379,13 @@ impl Signer { self.signer_db .insert_block(&block_info) .unwrap_or_else(|e| self.handle_insert_block_error(e)); - self.handle_block_rejection(block_rejection, sortition_state); - Some(block_response) + self.handle_block_rejection(&block_rejection, sortition_state); + self.send_block_response(&block_info.block, block_rejection.into()); } else { if let Err(e) = block_info.mark_locally_accepted(false) { if !block_info.has_reached_consensus() { warn!("{self}: Failed to mark block as locally accepted: {e:?}",); - return None; + return; } block_info.signed_self.get_or_insert(get_epoch_time_secs()); } @@ -1287,19 +1399,19 @@ impl Signer { self.signer_db .insert_block(&block_info) .unwrap_or_else(|e| self.handle_insert_block_error(e)); - let block_response = self.create_block_acceptance(&block_info.block); + self.send_block_pre_commit(signer_signature_hash); // have to save the signature _after_ the block info - self.handle_block_signature(stacks_client, block_response.as_block_accepted()?); - Some(block_response) + let address = self.stacks_address.clone(); + self.handle_block_pre_commit(stacks_client, &address, &signer_signature_hash); } } - /// Handle the block validate reject response. Returns our block response if we have one + /// Handle the block validate reject response fn handle_block_validate_reject( &mut self, block_validate_reject: &BlockValidateReject, sortition_state: &mut Option, - ) -> Option { + ) { crate::monitoring::actions::increment_block_validation_responses(false); let signer_signature_hash = block_validate_reject.signer_signature_hash; if self @@ -1312,16 +1424,16 @@ impl Signer { let Some(mut block_info) = self.block_lookup_by_reward_cycle(&signer_signature_hash) else { // We have not seen this block before. Why are we getting a response for it? debug!("{self}: Received a block validate response for a block we have not seen before. Ignoring..."); - return None; + return; }; if block_info.is_locally_finalized() { debug!("{self}: Received block validation for a block that is already marked as {}. Ignoring...", block_info.state); - return None; + return; } if let Err(e) = block_info.mark_locally_rejected() { if !block_info.has_reached_consensus() { warn!("{self}: Failed to mark block as locally rejected: {e:?}",); - return None; + return; } } let block_rejection = BlockRejection::from_validate_rejection( @@ -1342,7 +1454,7 @@ impl Signer { .insert_block(&block_info) .unwrap_or_else(|e| self.handle_insert_block_error(e)); self.handle_block_rejection(&block_rejection, sortition_state); - Some(BlockResponse::Rejected(block_rejection)) + self.send_block_response(&block_info.block, block_rejection.into()); } /// Handle the block validate response returned from our prior calls to submit a block for validation @@ -1353,15 +1465,15 @@ impl Signer { sortition_state: &mut Option, ) { info!("{self}: Received a block validate response: {block_validate_response:?}"); - let block_response = match block_validate_response { + match block_validate_response { BlockValidateResponse::Ok(block_validate_ok) => { crate::monitoring::actions::record_block_validation_latency( block_validate_ok.validation_time_ms, ); - self.handle_block_validate_ok(stacks_client, block_validate_ok, sortition_state) + self.handle_block_validate_ok(stacks_client, block_validate_ok, sortition_state); } BlockValidateResponse::Reject(block_validate_reject) => { - self.handle_block_validate_reject(block_validate_reject, sortition_state) + self.handle_block_validate_reject(block_validate_reject, sortition_state); } }; // Remove this block validation from the pending table @@ -1370,15 +1482,6 @@ impl Signer { .remove_pending_block_validation(&signer_sig_hash) .unwrap_or_else(|e| warn!("{self}: Failed to remove pending block validation: {e:?}")); - if let Some(response) = block_response { - let block = self - .signer_db - .block_lookup(&signer_sig_hash) - .unwrap_or_default() - .map(|info| info.block); - self.impl_send_block_response(block.as_ref(), response); - }; - // Check if there is a pending block validation that we need to submit to the node self.check_pending_block_validations(stacks_client); } @@ -1463,13 +1566,13 @@ impl Signer { ), &block_info.block, ); - block_info.reject_reason = Some(rejection.get_response_data().reject_reason.clone()); + block_info.reject_reason = Some(rejection.response_data.reject_reason.clone()); if let Err(e) = block_info.mark_locally_rejected() { if !block_info.has_reached_consensus() { warn!("{self}: Failed to mark block as locally rejected: {e:?}"); } }; - self.impl_send_block_response(Some(&block_info.block), rejection); + self.send_block_response(&block_info.block, rejection.into()); self.signer_db .insert_block(&block_info) @@ -1706,6 +1809,11 @@ impl Signer { return; } + // If this isn't our own signature, try treating it as a pre-commit in case the caller is running an outdated version + if signer_address != self.stacks_address { + self.handle_block_pre_commit(stacks_client, &signer_address, block_hash); + } + // do we have enough signatures to broadcast? // i.e. is the threshold reached? let signatures = self diff --git a/stacks-signer/src/v0/tests.rs b/stacks-signer/src/v0/tests.rs index 3a6767e76d..8c87cd9a4e 100644 --- a/stacks-signer/src/v0/tests.rs +++ b/stacks-signer/src/v0/tests.rs @@ -17,7 +17,7 @@ use std::collections::HashMap; use std::sync::LazyLock; use blockstack_lib::chainstate::nakamoto::NakamotoBlock; -use libsigner::v0::messages::{BlockResponse, RejectReason}; +use libsigner::v0::messages::{BlockRejection, BlockResponse, RejectReason}; use libsigner::BlockProposal; use stacks_common::types::chainstate::StacksPublicKey; use stacks_common::util::get_epoch_time_secs; @@ -53,6 +53,10 @@ pub static TEST_STALL_BLOCK_VALIDATION_SUBMISSION: LazyLock> = /// A global variable that can be used to prevent signer cleanup pub static TEST_SKIP_SIGNER_CLEANUP: LazyLock> = LazyLock::new(TestFlag::default); +/// A global variable that can be used to skip signature broadcast if the signer's public key is in the provided list +pub static TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST: LazyLock>> = + LazyLock::new(TestFlag::default); + impl Signer { /// Skip the block broadcast if the TEST_SKIP_BLOCK_BROADCAST flag is set pub fn test_skip_block_broadcast(&self, block: &NakamotoBlock) -> bool { @@ -81,8 +85,8 @@ impl Signer { &mut self, block_proposal: &BlockProposal, block_info: &mut BlockInfo, - block_response: Option, - ) -> Option { + block_rejection: Option, + ) -> Option { let public_keys = TEST_REJECT_ALL_BLOCK_PROPOSAL.get(); if public_keys.contains( &stacks_common::types::chainstate::StacksPublicKey::from_private(&self.private_key), @@ -108,7 +112,7 @@ impl Signer { .unwrap_or_else(|e| self.handle_insert_block_error(e)); Some(self.create_block_rejection(RejectReason::TestingDirective, &block_proposal.block)) } else { - block_response + block_rejection } } @@ -169,4 +173,23 @@ impl Signer { } self.supported_signer_protocol_version } + + /// Skip the block broadcast if the TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST flag is set for the signer + pub fn test_skip_block_response_broadcast(&self, block_response: &BlockResponse) -> bool { + if block_response.as_block_accepted().is_none() { + return false; + } + let hash = block_response.get_signer_signature_hash(); + let public_keys = TEST_SIGNERS_SKIP_BLOCK_RESPONSE_BROADCAST.get(); + if public_keys.contains( + &stacks_common::types::chainstate::StacksPublicKey::from_private(&self.private_key), + ) { + warn!( + "{self}: Skipping signature broadcast due to testing directive"; + "signer_signature_hash" => %hash + ); + return true; + } + false + } }