From d65182512cc28584d83a8a0fcd60dcaca1e80ba0 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Wed, 31 Jul 2024 14:13:18 -0500 Subject: [PATCH 1/4] test: move BTC commit stall to counters() --- .../stacks-node/src/nakamoto_node/relayer.rs | 15 ++++++----- testnet/stacks-node/src/run_loop/neon.rs | 14 +++++++++++ .../src/tests/nakamoto_integrations.rs | 11 ++++---- testnet/stacks-node/src/tests/signer/mod.rs | 5 +++- testnet/stacks-node/src/tests/signer/v0.rs | 25 ++++++++++++++++--- 5 files changed, 54 insertions(+), 16 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/relayer.rs b/testnet/stacks-node/src/nakamoto_node/relayer.rs index 12f7dbc9e9..1234ad20cc 100644 --- a/testnet/stacks-node/src/nakamoto_node/relayer.rs +++ b/testnet/stacks-node/src/nakamoto_node/relayer.rs @@ -66,11 +66,6 @@ use crate::run_loop::nakamoto::{Globals, RunLoop}; use crate::run_loop::RegisteredKey; use crate::BitcoinRegtestController; -#[cfg(test)] -lazy_static::lazy_static! { - pub static ref TEST_SKIP_COMMIT_OP: std::sync::Mutex> = std::sync::Mutex::new(None); -} - /// Command types for the Nakamoto relayer thread, issued to it by other threads pub enum RelayerDirective { /// Handle some new data that arrived on the network (such as blocks, transactions, and @@ -937,7 +932,15 @@ impl RelayerThread { let mut last_committed = self.make_block_commit(&tip_block_ch, &tip_block_bh)?; #[cfg(test)] { - if TEST_SKIP_COMMIT_OP.lock().unwrap().unwrap_or(false) { + if self + .globals + .counters + .naka_skip_commit_op + .0 + .lock() + .unwrap() + .unwrap_or(false) + { warn!("Relayer: not submitting block-commit to bitcoin network due to test directive."); return Ok(()); } diff --git a/testnet/stacks-node/src/run_loop/neon.rs b/testnet/stacks-node/src/run_loop/neon.rs index d4aea34f0e..663c14e27b 100644 --- a/testnet/stacks-node/src/run_loop/neon.rs +++ b/testnet/stacks-node/src/run_loop/neon.rs @@ -82,6 +82,17 @@ impl std::ops::Deref for RunLoopCounter { } } +#[cfg(test)] +#[derive(Clone)] +pub struct TestFlag(pub Arc>>); + +#[cfg(test)] +impl Default for TestFlag { + fn default() -> Self { + Self(Arc::new(std::sync::Mutex::new(None))) + } +} + #[derive(Clone, Default)] pub struct Counters { pub blocks_processed: RunLoopCounter, @@ -95,6 +106,9 @@ pub struct Counters { pub naka_mined_blocks: RunLoopCounter, pub naka_proposed_blocks: RunLoopCounter, pub naka_mined_tenures: RunLoopCounter, + + #[cfg(test)] + pub naka_skip_commit_op: TestFlag, } impl Counters { diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index c2fd6245f2..f6056f1b6f 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -95,7 +95,6 @@ use wsts::net::Message; use super::bitcoin_regtest::BitcoinCoreController; use crate::config::{EventKeyType, EventObserverConfig, InitialBalance}; use crate::nakamoto_node::miner::TEST_BROADCAST_STALL; -use crate::nakamoto_node::relayer::TEST_SKIP_COMMIT_OP; use crate::neon::{Counters, RunLoopCounter}; use crate::operations::BurnchainOpSigner; use crate::run_loop::boot_nakamoto; @@ -3723,6 +3722,7 @@ fn forked_tenure_is_ignored() { naka_submitted_commits: commits_submitted, naka_proposed_blocks: proposals_submitted, naka_mined_blocks: mined_blocks, + naka_skip_commit_op: test_skip_commit_op, .. } = run_loop.counters(); @@ -3791,7 +3791,7 @@ fn forked_tenure_is_ignored() { info!("Commit op is submitted; unpause tenure B's block"); // Unpause the broadcast of Tenure B's block, do not submit commits. - TEST_SKIP_COMMIT_OP.lock().unwrap().replace(true); + test_skip_commit_op.0.lock().unwrap().replace(true); TEST_BROADCAST_STALL.lock().unwrap().replace(false); // Wait for a stacks block to be broadcasted @@ -3816,7 +3816,7 @@ fn forked_tenure_is_ignored() { let commits_before = commits_submitted.load(Ordering::SeqCst); let blocks_before = mined_blocks.load(Ordering::SeqCst); next_block_and(&mut btc_regtest_controller, 60, || { - TEST_SKIP_COMMIT_OP.lock().unwrap().replace(false); + test_skip_commit_op.0.lock().unwrap().replace(false); let commits_count = commits_submitted.load(Ordering::SeqCst); let blocks_count = mined_blocks.load(Ordering::SeqCst); Ok(commits_count > commits_before && blocks_count > blocks_before) @@ -5478,6 +5478,7 @@ fn continue_tenure_extend() { blocks_processed, naka_submitted_commits: commits_submitted, naka_proposed_blocks: proposals_submitted, + naka_skip_commit_op: test_skip_commit_op, .. } = run_loop.counters(); @@ -5549,7 +5550,7 @@ fn continue_tenure_extend() { ); info!("Pausing commit ops to trigger a tenure extend."); - TEST_SKIP_COMMIT_OP.lock().unwrap().replace(true); + test_skip_commit_op.0.lock().unwrap().replace(true); next_block_and(&mut btc_regtest_controller, 60, || Ok(true)).unwrap(); @@ -5604,7 +5605,7 @@ fn continue_tenure_extend() { ); info!("Resuming commit ops to mine regular tenures."); - TEST_SKIP_COMMIT_OP.lock().unwrap().replace(false); + test_skip_commit_op.0.lock().unwrap().replace(false); // Mine 15 more regular nakamoto tenures for _i in 0..15 { diff --git a/testnet/stacks-node/src/tests/signer/mod.rs b/testnet/stacks-node/src/tests/signer/mod.rs index 08c4004ed0..78ed2e7c7a 100644 --- a/testnet/stacks-node/src/tests/signer/mod.rs +++ b/testnet/stacks-node/src/tests/signer/mod.rs @@ -57,7 +57,7 @@ use wsts::state_machine::PublicKeys; use crate::config::{Config as NeonConfig, EventKeyType, EventObserverConfig, InitialBalance}; use crate::event_dispatcher::MinedNakamotoBlockEvent; -use crate::neon::Counters; +use crate::neon::{Counters, TestFlag}; use crate::run_loop::boot_nakamoto; use crate::tests::bitcoin_regtest::BitcoinCoreController; use crate::tests::nakamoto_integrations::{ @@ -81,6 +81,7 @@ pub struct RunningNodes { pub blocks_processed: Arc, pub nakamoto_blocks_proposed: Arc, pub nakamoto_blocks_mined: Arc, + pub nakamoto_test_skip_commit_op: TestFlag, pub coord_channel: Arc>, pub conf: NeonConfig, } @@ -679,6 +680,7 @@ fn setup_stx_btc_node ()>( naka_submitted_commits: commits_submitted, naka_proposed_blocks: naka_blocks_proposed, naka_mined_blocks: naka_blocks_mined, + naka_skip_commit_op: nakamoto_test_skip_commit_op, .. } = run_loop.counters(); @@ -711,6 +713,7 @@ fn setup_stx_btc_node ()>( blocks_processed: blocks_processed.0, nakamoto_blocks_proposed: naka_blocks_proposed.0, nakamoto_blocks_mined: naka_blocks_mined.0, + nakamoto_test_skip_commit_op, coord_channel, conf: naka_conf, } diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 98e2d64b55..13bcd575f5 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -52,7 +52,6 @@ use tracing_subscriber::{fmt, EnvFilter}; use super::SignerTest; use crate::event_dispatcher::MinedNakamotoBlockEvent; use crate::nakamoto_node::miner::TEST_BROADCAST_STALL; -use crate::nakamoto_node::relayer::TEST_SKIP_COMMIT_OP; use crate::run_loop::boot_nakamoto; use crate::tests::nakamoto_integrations::{ boot_to_epoch_25, boot_to_epoch_3_reward_set, next_block_and, POX_4_DEFAULT_STACKER_STX_AMT, @@ -859,7 +858,13 @@ fn forked_tenure_testing( info!("Commit op is submitted; unpause tenure B's block"); // Unpause the broadcast of Tenure B's block, do not submit commits. - TEST_SKIP_COMMIT_OP.lock().unwrap().replace(true); + signer_test + .running_nodes + .nakamoto_test_skip_commit_op + .0 + .lock() + .unwrap() + .replace(true); TEST_BROADCAST_STALL.lock().unwrap().replace(false); // Wait for a stacks block to be broadcasted @@ -892,7 +897,13 @@ fn forked_tenure_testing( &mut signer_test.running_nodes.btc_regtest_controller, 60, || { - TEST_SKIP_COMMIT_OP.lock().unwrap().replace(false); + signer_test + .running_nodes + .nakamoto_test_skip_commit_op + .0 + .lock() + .unwrap() + .replace(false); let commits_count = commits_submitted.load(Ordering::SeqCst); let blocks_count = if expect_tenure_c { mined_blocks.load(Ordering::SeqCst) @@ -1624,7 +1635,13 @@ fn empty_sortition() { TEST_BROADCAST_STALL.lock().unwrap().replace(true); info!("Pausing commit op to prevent tenure C from starting..."); - TEST_SKIP_COMMIT_OP.lock().unwrap().replace(true); + signer_test + .running_nodes + .nakamoto_test_skip_commit_op + .0 + .lock() + .unwrap() + .replace(true); let blocks_after = signer_test .running_nodes From ac99ec9d58c8bfec594a729ed6723af70adf4192 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Wed, 31 Jul 2024 19:51:38 -0500 Subject: [PATCH 2/4] test: add a test with 2 miners, one of whom tries to fork the other and is prevented by the signer set --- .../src/tests/nakamoto_integrations.rs | 15 + testnet/stacks-node/src/tests/signer/mod.rs | 3 + testnet/stacks-node/src/tests/signer/v0.rs | 326 +++++++++++++++++- 3 files changed, 340 insertions(+), 4 deletions(-) diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index f6056f1b6f..31dfa3e414 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -625,6 +625,21 @@ where Ok(()) } +pub fn wait_for(timeout_secs: u64, mut check: F) -> Result<(), String> +where + F: FnMut() -> Result, +{ + let start = Instant::now(); + while !check()? { + if start.elapsed() > Duration::from_secs(timeout_secs) { + error!("Timed out waiting for check to process"); + return Err("Timed out".into()); + } + thread::sleep(Duration::from_millis(100)); + } + Ok(()) +} + /// Mine a bitcoin block, and wait until: /// (1) a new block has been processed by the coordinator pub fn next_block_and_process_new_stacks_block( diff --git a/testnet/stacks-node/src/tests/signer/mod.rs b/testnet/stacks-node/src/tests/signer/mod.rs index 78ed2e7c7a..7fe508407b 100644 --- a/testnet/stacks-node/src/tests/signer/mod.rs +++ b/testnet/stacks-node/src/tests/signer/mod.rs @@ -92,6 +92,8 @@ pub struct SignerTest { pub running_nodes: RunningNodes, // The spawned signers and their threads pub spawned_signers: Vec, + // The spawned signers and their threads + pub signer_configs: Vec, // the private keys of the signers pub signer_stacks_private_keys: Vec, // link to the stacks node @@ -209,6 +211,7 @@ impl + Send + 'static, T: SignerEventTrait + 'static> SignerTest. +use std::collections::HashMap; use std::ops::Add; use std::str::FromStr; use std::sync::atomic::Ordering; @@ -26,7 +27,10 @@ use libsigner::v0::messages::{ }; use libsigner::{BlockProposal, SignerSession, StackerDBSession}; use stacks::address::AddressHashMode; -use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader, NakamotoChainState}; +use stacks::chainstate::burn::db::sortdb::SortitionDB; +use stacks::chainstate::nakamoto::{ + NakamotoBlock, NakamotoBlockHeader, NakamotoBlockVote, NakamotoChainState, +}; use stacks::chainstate::stacks::address::PoxAddress; use stacks::chainstate::stacks::boot::MINERS_NAME; use stacks::chainstate::stacks::db::{StacksChainState, StacksHeaderInfo}; @@ -34,9 +38,11 @@ use stacks::codec::StacksMessageCodec; use stacks::core::{StacksEpochId, CHAIN_ID_TESTNET}; use stacks::libstackerdb::StackerDBChunkData; use stacks::net::api::postblock_proposal::TEST_VALIDATE_STALL; -use stacks::types::chainstate::{StacksAddress, StacksPrivateKey, StacksPublicKey}; +use stacks::types::chainstate::{StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey}; use stacks::types::PublicKey; -use stacks::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; +use stacks::util::get_epoch_time_secs; +use stacks::util::hash::Sha512Trunc256Sum; +use stacks::util::secp256k1::{MessageSignature, Secp256k1PrivateKey, Secp256k1PublicKey}; use stacks::util_lib::boot::boot_code_id; use stacks::util_lib::signed_structured_data::pox4::{ make_pox_4_signer_key_signature, Pox4SignatureTopic, @@ -45,6 +51,7 @@ use stacks_common::bitvec::BitVec; use stacks_signer::chainstate::{ProposalEvalConfig, SortitionsView}; use stacks_signer::client::{SignerSlotID, StackerDB}; use stacks_signer::runloop::State; +use stacks_signer::signerdb::{BlockInfo, SignerDb}; use stacks_signer::v0::SpawnedSigner; use tracing_subscriber::prelude::*; use tracing_subscriber::{fmt, EnvFilter}; @@ -52,9 +59,11 @@ use tracing_subscriber::{fmt, EnvFilter}; use super::SignerTest; use crate::event_dispatcher::MinedNakamotoBlockEvent; use crate::nakamoto_node::miner::TEST_BROADCAST_STALL; +use crate::neon::Counters; use crate::run_loop::boot_nakamoto; use crate::tests::nakamoto_integrations::{ - boot_to_epoch_25, boot_to_epoch_3_reward_set, next_block_and, POX_4_DEFAULT_STACKER_STX_AMT, + boot_to_epoch_25, boot_to_epoch_3_reward_set, next_block_and, wait_for, + POX_4_DEFAULT_STACKER_STX_AMT, }; use crate::tests::neon_integrations::{ get_account, get_chain_info, next_block_and_wait, run_until_burnchain_height, submit_tx, @@ -1264,6 +1273,315 @@ fn multiple_miners() { signer_test.shutdown(); } +#[test] +#[ignore] +fn miner_forking() { + if env::var("BITCOIND_TEST") != Ok("1".into()) { + return; + } + + let num_signers = 5; + let sender_sk = Secp256k1PrivateKey::new(); + let sender_addr = tests::to_addr(&sender_sk); + let send_amt = 100; + let send_fee = 180; + + let btc_miner_1_seed = vec![1, 1, 1, 1]; + let btc_miner_2_seed = vec![2, 2, 2, 2]; + let btc_miner_1_pk = Keychain::default(btc_miner_1_seed.clone()).get_pub_key(); + let btc_miner_2_pk = Keychain::default(btc_miner_2_seed.clone()).get_pub_key(); + + let node_1_rpc = 51024; + let node_1_p2p = 51023; + let node_2_rpc = 51026; + let node_2_p2p = 51025; + + let node_1_rpc_bind = format!("127.0.0.1:{}", node_1_rpc); + let node_2_rpc_bind = format!("127.0.0.1:{}", node_2_rpc); + let mut node_2_listeners = Vec::new(); + + // partition the signer set so that ~half are listening and using node 1 for RPC and events, + // and the rest are using node 2 + + let mut signer_test: SignerTest = SignerTest::new_with_config_modifications( + num_signers, + vec![(sender_addr.clone(), send_amt + send_fee)], + Some(Duration::from_secs(15)), + |signer_config| { + let node_host = if signer_config.endpoint.port() % 2 == 0 { + &node_1_rpc_bind + } else { + &node_2_rpc_bind + }; + signer_config.node_host = node_host.to_string(); + // we're deliberately stalling proposals: don't punish this in this test! + signer_config.block_proposal_timeout = Duration::from_secs(240); + // make sure that we don't allow forking due to burn block timing + signer_config.first_proposal_burn_block_timing = Duration::from_secs(1); + }, + |config| { + let localhost = "127.0.0.1"; + config.node.rpc_bind = format!("{}:{}", localhost, node_1_rpc); + config.node.p2p_bind = format!("{}:{}", localhost, node_1_p2p); + config.node.data_url = format!("http://{}:{}", localhost, node_1_rpc); + config.node.p2p_address = format!("{}:{}", localhost, node_1_p2p); + + config.node.seed = btc_miner_1_seed.clone(); + config.node.local_peer_seed = btc_miner_1_seed.clone(); + config.burnchain.local_mining_public_key = Some(btc_miner_1_pk.to_hex()); + config.miner.mining_key = Some(Secp256k1PrivateKey::from_seed(&[1])); + + config.events_observers.retain(|listener| { + let Ok(addr) = std::net::SocketAddr::from_str(&listener.endpoint) else { + warn!( + "Cannot parse {} to a socket, assuming it isn't a signer-listener binding", + listener.endpoint + ); + return true; + }; + if addr.port() % 2 == 0 || addr.port() == test_observer::EVENT_OBSERVER_PORT { + return true; + } + node_2_listeners.push(listener.clone()); + false + }) + }, + &[btc_miner_1_pk.clone(), btc_miner_2_pk.clone()], + ); + let conf = signer_test.running_nodes.conf.clone(); + let mut conf_node_2 = conf.clone(); + let localhost = "127.0.0.1"; + conf_node_2.node.rpc_bind = format!("{}:{}", localhost, node_2_rpc); + conf_node_2.node.p2p_bind = format!("{}:{}", localhost, node_2_p2p); + conf_node_2.node.data_url = format!("http://{}:{}", localhost, node_2_rpc); + conf_node_2.node.p2p_address = format!("{}:{}", localhost, node_2_p2p); + conf_node_2.node.seed = btc_miner_2_seed.clone(); + conf_node_2.burnchain.local_mining_public_key = Some(btc_miner_2_pk.to_hex()); + conf_node_2.node.local_peer_seed = btc_miner_2_seed.clone(); + conf_node_2.node.miner = true; + conf_node_2.events_observers.clear(); + conf_node_2.events_observers.extend(node_2_listeners); + conf_node_2.miner.mining_key = Some(Secp256k1PrivateKey::from_seed(&[2])); + assert!(!conf_node_2.events_observers.is_empty()); + + let node_1_sk = Secp256k1PrivateKey::from_seed(&conf.node.local_peer_seed); + let node_1_pk = StacksPublicKey::from_private(&node_1_sk); + + conf_node_2.node.working_dir = format!("{}-{}", conf_node_2.node.working_dir, "1"); + + conf_node_2.node.set_bootstrap_nodes( + format!("{}@{}", &node_1_pk.to_hex(), conf.node.p2p_bind), + conf.burnchain.chain_id, + conf.burnchain.peer_version, + ); + + let mut run_loop_2 = boot_nakamoto::BootRunLoop::new(conf_node_2.clone()).unwrap(); + let Counters { + naka_skip_commit_op, + naka_submitted_commits: second_miner_commits_submitted, + .. + } = run_loop_2.counters(); + let _run_loop_2_thread = thread::Builder::new() + .name("run_loop_2".into()) + .spawn(move || run_loop_2.start(None, 0)) + .unwrap(); + + signer_test.boot_to_epoch_3(); + let pre_nakamoto_peer_1_height = get_chain_info(&conf).stacks_tip_height; + + naka_skip_commit_op.0.lock().unwrap().replace(false); + info!("------------------------- Reached Epoch 3.0 -------------------------"); + + let mut sortitions_seen = Vec::new(); + let run_sortition = || { + info!("Pausing stacks block proposal to force an empty tenure commit from RL2"); + TEST_BROADCAST_STALL.lock().unwrap().replace(true); + + let rl2_commits_before = second_miner_commits_submitted.load(Ordering::SeqCst); + + signer_test + .running_nodes + .btc_regtest_controller + .build_next_block(1); + naka_skip_commit_op.0.lock().unwrap().replace(false); + + // wait until a commit is submitted by run_loop_2 + wait_for(60, || { + let commits_count = second_miner_commits_submitted.load(Ordering::SeqCst); + Ok(commits_count > rl2_commits_before) + }) + .unwrap(); + + // fetch the current sortition info + let sortdb = conf.get_burnchain().open_sortition_db(true).unwrap(); + let sort_tip = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()).unwrap(); + + // block commits from RL2 -- this will block until the start of the next iteration + // in this loop. + naka_skip_commit_op.0.lock().unwrap().replace(true); + // ensure RL1 performs an RBF after unblock block broadcast + let rl1_commits_before = signer_test + .running_nodes + .commits_submitted + .load(Ordering::SeqCst); + + // unblock block mining + let blocks_len = test_observer::get_blocks().len(); + TEST_BROADCAST_STALL.lock().unwrap().replace(false); + + // wait for a block to be processed (or timeout!) + if let Err(_) = wait_for(60, || Ok(test_observer::get_blocks().len() > blocks_len)) { + info!("Timeout waiting for a block process: assuming this is because RL2 attempted to fork-- will check at end of test"); + return (sort_tip, false); + } + + info!("Nakamoto block processed, waiting for commit from RL1"); + + // wait for a commit from RL1 + wait_for(60, || { + let commits_count = signer_test + .running_nodes + .commits_submitted + .load(Ordering::SeqCst); + Ok(commits_count > rl1_commits_before) + }) + .unwrap(); + + // sleep for 1 second to prevent the block timing from allowing a fork by the signer set + thread::sleep(Duration::from_secs(1)); + (sort_tip, true) + }; + + let mut won_by_miner_2_but_no_tenure = false; + let mut won_by_miner_1_after_tenureless_miner_2 = false; + let miner_1_pk = StacksPublicKey::from_private(conf.miner.mining_key.as_ref().unwrap()); + // miner 2 is expected to be valid iff: + // (a) its the first nakamoto tenure + // (b) the prior sortition didn't have a tenure (because by this time RL2 will have up-to-date block processing) + let mut expects_miner_2_to_be_valid = true; + + // due to the random nature of mining sortitions, the way this test is structured + // is that keeps track of two scenarios that we want to cover, and once enough sortitions + // have been produced to cover those scenarios, it stops and checks the results at the end. + while !(won_by_miner_2_but_no_tenure && won_by_miner_1_after_tenureless_miner_2) { + if sortitions_seen.len() >= 20 { + panic!("Produced 20 sortitions, but didn't cover the test scenarios, aborting"); + } + let (sortition_data, had_tenure) = run_sortition(); + sortitions_seen.push((sortition_data.clone(), had_tenure)); + + let nakamoto_block_ids: Vec<_> = test_observer::get_blocks() + .into_iter() + .filter_map(|block_json| { + if block_json + .as_object() + .unwrap() + .get("miner_signature") + .is_none() + { + return None; + } + let block_id = StacksBlockId::from_hex( + &block_json + .as_object() + .unwrap() + .get("index_block_hash") + .unwrap() + .as_str() + .unwrap()[2..], + ) + .unwrap(); + Some(block_id) + }) + .collect(); + + let (chainstate, _) = StacksChainState::open( + conf.is_mainnet(), + conf.burnchain.chain_id, + &conf.get_chainstate_path_str(), + None, + ) + .unwrap(); + + let nakamoto_headers: HashMap<_, _> = nakamoto_block_ids + .into_iter() + .map(|block_id| { + let header_info = NakamotoChainState::get_block_header(chainstate.db(), &block_id) + .unwrap() + .unwrap(); + (header_info.consensus_hash.clone(), header_info) + }) + .collect(); + + if had_tenure { + let header_info = nakamoto_headers + .get(&sortition_data.consensus_hash) + .unwrap(); + let header = header_info + .anchored_header + .as_stacks_nakamoto() + .unwrap() + .clone(); + let mined_by_miner_1 = miner_1_pk + .verify( + header.miner_signature_hash().as_bytes(), + &header.miner_signature, + ) + .unwrap(); + + info!("Block check"; + "height" => header.chain_length, + "consensus_hash" => %header.consensus_hash, + "block_hash" => %header.block_hash(), + "stacks_block_id" => %header.block_id(), + "mined_by_miner_1?" => mined_by_miner_1, + "expects_miner_2_to_be_valid?" => expects_miner_2_to_be_valid); + if !mined_by_miner_1 { + assert!(expects_miner_2_to_be_valid, "If a block was produced by miner 2, we should have expected miner 2 to be valid"); + } else if won_by_miner_2_but_no_tenure { + // the tenure was won by miner 1, they produced a block, and this follows a tenure that miner 2 won but couldn't + // mine during because they tried to fork. + won_by_miner_1_after_tenureless_miner_2 = true; + } + + // even if it was mined by miner 2, their next block commit should be invalid! + expects_miner_2_to_be_valid = false; + } else { + info!("Sortition without tenure"; "expects_miner_2_to_be_valid?" => expects_miner_2_to_be_valid); + assert!(nakamoto_headers + .get(&sortition_data.consensus_hash) + .is_none()); + assert!(!expects_miner_2_to_be_valid, "If no blocks were produced in the tenure, it should be because miner 2 committed to a fork"); + won_by_miner_2_but_no_tenure = true; + expects_miner_2_to_be_valid = true; + } + } + + let peer_1_height = get_chain_info(&conf).stacks_tip_height; + let peer_2_height = get_chain_info(&conf_node_2).stacks_tip_height; + info!("Peer height information"; "peer_1" => peer_1_height, "peer_2" => peer_2_height, "pre_naka_height" => pre_nakamoto_peer_1_height); + assert_eq!(peer_1_height, peer_2_height); + + let nakamoto_block_ids: Vec<_> = test_observer::get_blocks() + .into_iter() + .filter_map(|block_json| { + block_json + .as_object() + .unwrap() + .get("miner_signature") + .map(|x| x.as_str().unwrap().to_string()) + }) + .collect(); + + assert_eq!( + peer_1_height - pre_nakamoto_peer_1_height, + u64::try_from(nakamoto_block_ids.len()).unwrap(), + "There should be no forks in this test" + ); + + signer_test.shutdown(); +} + #[test] #[ignore] /// This test checks the behavior at the end of a tenure. Specifically: From a567db23c84adce33205b92e3e3c65dd87f7fa05 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 1 Aug 2024 10:26:21 -0500 Subject: [PATCH 3/4] tests: improve forking tests --- .../src/tests/nakamoto_integrations.rs | 33 ++-- testnet/stacks-node/src/tests/signer/v0.rs | 178 +++++++++++------- 2 files changed, 133 insertions(+), 78 deletions(-) diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index 31dfa3e414..542ff7511c 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -1641,6 +1641,10 @@ fn multiple_miners() { let (mut naka_conf, _miner_account) = naka_neon_integration_conf(None); naka_conf.node.local_peer_seed = vec![1, 1, 1, 1]; + naka_conf.miner.mining_key = Some(Secp256k1PrivateKey::from_seed(&[1])); + + let node_2_rpc = 51026; + let node_2_p2p = 51025; let http_origin = format!("http://{}", &naka_conf.node.rpc_bind); naka_conf.miner.wait_on_interim_blocks = Duration::from_secs(1); let sender_sk = Secp256k1PrivateKey::new(); @@ -1665,7 +1669,11 @@ fn multiple_miners() { let stacker_sk = setup_stacker(&mut naka_conf); let mut conf_node_2 = naka_conf.clone(); - set_random_binds(&mut conf_node_2); + let localhost = "127.0.0.1"; + conf_node_2.node.rpc_bind = format!("{}:{}", localhost, node_2_rpc); + conf_node_2.node.p2p_bind = format!("{}:{}", localhost, node_2_p2p); + conf_node_2.node.data_url = format!("http://{}:{}", localhost, node_2_rpc); + conf_node_2.node.p2p_address = format!("{}:{}", localhost, node_2_p2p); conf_node_2.node.seed = vec![2, 2, 2, 2]; conf_node_2.burnchain.local_mining_public_key = Some( Keychain::default(conf_node_2.node.seed.clone()) @@ -1674,6 +1682,8 @@ fn multiple_miners() { ); conf_node_2.node.local_peer_seed = vec![2, 2, 2, 2]; conf_node_2.node.miner = true; + conf_node_2.miner.mining_key = Some(Secp256k1PrivateKey::from_seed(&[2])); + conf_node_2.events_observers.clear(); let node_1_sk = Secp256k1PrivateKey::from_seed(&naka_conf.node.local_peer_seed); let node_1_pk = StacksPublicKey::from_private(&node_1_sk); @@ -1813,16 +1823,14 @@ fn multiple_miners() { make_stacks_transfer(&sender_sk, sender_nonce, send_fee, &recipient, send_amt); submit_tx(&http_origin, &transfer_tx); - loop { + wait_for(20, || { let blocks_processed = coord_channel .lock() .expect("Mutex poisoned") .get_stacks_blocks_processed(); - if blocks_processed > blocks_processed_before { - break; - } - thread::sleep(Duration::from_millis(100)); - } + Ok(blocks_processed > blocks_processed_before) + }) + .unwrap(); let info = get_chain_info_result(&naka_conf).unwrap(); assert_ne!(info.stacks_tip, last_tip); @@ -1832,13 +1840,10 @@ fn multiple_miners() { last_tip_height = info.stacks_tip_height; } - let start_time = Instant::now(); - while commits_submitted.load(Ordering::SeqCst) <= commits_before { - if start_time.elapsed() >= Duration::from_secs(20) { - panic!("Timed out waiting for block-commit"); - } - thread::sleep(Duration::from_millis(100)); - } + wait_for(20, || { + Ok(commits_submitted.load(Ordering::SeqCst) > commits_before) + }) + .unwrap(); } // load the chain tip, and assert that it is a nakamoto block and at least 30 blocks have advanced in epoch 3 diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 27b1df7450..c9b5cf4854 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -13,7 +13,7 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::ops::Add; use std::str::FromStr; use std::sync::atomic::Ordering; @@ -28,9 +28,7 @@ use libsigner::v0::messages::{ use libsigner::{BlockProposal, SignerSession, StackerDBSession}; use stacks::address::AddressHashMode; use stacks::chainstate::burn::db::sortdb::SortitionDB; -use stacks::chainstate::nakamoto::{ - NakamotoBlock, NakamotoBlockHeader, NakamotoBlockVote, NakamotoChainState, -}; +use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader, NakamotoChainState}; use stacks::chainstate::stacks::address::PoxAddress; use stacks::chainstate::stacks::boot::MINERS_NAME; use stacks::chainstate::stacks::db::{StacksChainState, StacksHeaderInfo}; @@ -40,9 +38,7 @@ use stacks::libstackerdb::StackerDBChunkData; use stacks::net::api::postblock_proposal::TEST_VALIDATE_STALL; use stacks::types::chainstate::{StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey}; use stacks::types::PublicKey; -use stacks::util::get_epoch_time_secs; -use stacks::util::hash::Sha512Trunc256Sum; -use stacks::util::secp256k1::{MessageSignature, Secp256k1PrivateKey, Secp256k1PublicKey}; +use stacks::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; use stacks::util_lib::boot::boot_code_id; use stacks::util_lib::signed_structured_data::pox4::{ make_pox_4_signer_key_signature, Pox4SignatureTopic, @@ -51,7 +47,6 @@ use stacks_common::bitvec::BitVec; use stacks_signer::chainstate::{ProposalEvalConfig, SortitionsView}; use stacks_signer::client::{SignerSlotID, StackerDB}; use stacks_signer::runloop::State; -use stacks_signer::signerdb::{BlockInfo, SignerDb}; use stacks_signer::v0::SpawnedSigner; use tracing_subscriber::prelude::*; use tracing_subscriber::{fmt, EnvFilter}; @@ -70,7 +65,7 @@ use crate::tests::neon_integrations::{ test_observer, }; use crate::tests::{self, make_stacks_transfer}; -use crate::{nakamoto_node, BurnchainController, Keychain}; +use crate::{nakamoto_node, BurnchainController, Config, Keychain}; impl SignerTest { /// Run the test until the first epoch 2.5 reward cycle. @@ -1197,6 +1192,7 @@ fn multiple_miners() { config.node.seed = btc_miner_1_seed.clone(); config.node.local_peer_seed = btc_miner_1_seed.clone(); config.burnchain.local_mining_public_key = Some(btc_miner_1_pk.to_hex()); + config.miner.mining_key = Some(Secp256k1PrivateKey::from_seed(&[1])); config.events_observers.retain(|listener| { let Ok(addr) = std::net::SocketAddr::from_str(&listener.endpoint) else { @@ -1225,6 +1221,7 @@ fn multiple_miners() { conf_node_2.node.seed = btc_miner_2_seed.clone(); conf_node_2.burnchain.local_mining_public_key = Some(btc_miner_2_pk.to_hex()); conf_node_2.node.local_peer_seed = btc_miner_2_seed.clone(); + conf_node_2.miner.mining_key = Some(Secp256k1PrivateKey::from_seed(&[2])); conf_node_2.node.miner = true; conf_node_2.events_observers.clear(); conf_node_2.events_observers.extend(node_2_listeners); @@ -1252,9 +1249,59 @@ fn multiple_miners() { info!("------------------------- Reached Epoch 3.0 -------------------------"); - let nakamoto_tenures = 20; - for _i in 0..nakamoto_tenures { - let _mined_block = signer_test.mine_block_wait_on_processing(Duration::from_secs(30)); + let max_nakamoto_tenures = 20; + + // due to the random nature of mining sortitions, the way this test is structured + // is that we keep track of how many tenures each miner produced, and once enough sortitions + // have been produced such that each miner has produced 3 tenures, we stop and check the + // results at the end + + let miner_1_pk = StacksPublicKey::from_private(conf.miner.mining_key.as_ref().unwrap()); + let miner_2_pk = StacksPublicKey::from_private(conf_node_2.miner.mining_key.as_ref().unwrap()); + let mut btc_blocks_mined = 0; + let mut miner_1_tenures = 0; + let mut miner_2_tenures = 0; + while !(miner_1_tenures >= 3 && miner_2_tenures >= 3) { + if btc_blocks_mined > max_nakamoto_tenures { + panic!("Produced {btc_blocks_mined} sortitions, but didn't cover the test scenarios, aborting"); + } + signer_test.mine_block_wait_on_processing(Duration::from_secs(30)); + btc_blocks_mined += 1; + let blocks = get_nakamoto_headers(&conf); + // for this test, there should be one block per tenure + let consensus_hash_set: HashSet<_> = blocks + .iter() + .map(|header| header.consensus_hash.clone()) + .collect(); + assert_eq!( + consensus_hash_set.len(), + blocks.len(), + "In this test, there should only be one block per tenure" + ); + miner_1_tenures = blocks + .iter() + .filter(|header| { + let header = header.anchored_header.as_stacks_nakamoto().unwrap(); + miner_1_pk + .verify( + header.miner_signature_hash().as_bytes(), + &header.miner_signature, + ) + .unwrap() + }) + .count(); + miner_2_tenures = blocks + .iter() + .filter(|header| { + let header = header.anchored_header.as_stacks_nakamoto().unwrap(); + miner_2_pk + .verify( + header.miner_signature_hash().as_bytes(), + &header.miner_signature, + ) + .unwrap() + }) + .count(); } info!( @@ -1268,11 +1315,61 @@ fn multiple_miners() { let peer_2_height = get_chain_info(&conf_node_2).stacks_tip_height; info!("Peer height information"; "peer_1" => peer_1_height, "peer_2" => peer_2_height, "pre_naka_height" => pre_nakamoto_peer_1_height); assert_eq!(peer_1_height, peer_2_height); - assert_eq!(peer_1_height, pre_nakamoto_peer_1_height + nakamoto_tenures); + assert_eq!(peer_1_height, pre_nakamoto_peer_1_height + btc_blocks_mined); + assert_eq!( + btc_blocks_mined, + u64::try_from(miner_1_tenures + miner_2_tenures).unwrap() + ); signer_test.shutdown(); } +/// Read processed nakamoto block IDs from the test observer, and use `config` to open +/// a chainstate DB and returns their corresponding StacksHeaderInfos +fn get_nakamoto_headers(config: &Config) -> Vec { + let nakamoto_block_ids: Vec<_> = test_observer::get_blocks() + .into_iter() + .filter_map(|block_json| { + if block_json + .as_object() + .unwrap() + .get("miner_signature") + .is_none() + { + return None; + } + let block_id = StacksBlockId::from_hex( + &block_json + .as_object() + .unwrap() + .get("index_block_hash") + .unwrap() + .as_str() + .unwrap()[2..], + ) + .unwrap(); + Some(block_id) + }) + .collect(); + + let (chainstate, _) = StacksChainState::open( + config.is_mainnet(), + config.burnchain.chain_id, + &config.get_chainstate_path_str(), + None, + ) + .unwrap(); + + nakamoto_block_ids + .into_iter() + .map(|block_id| { + NakamotoChainState::get_block_header(chainstate.db(), &block_id) + .unwrap() + .unwrap() + }) + .collect() +} + #[test] #[ignore] fn miner_forking() { @@ -1470,47 +1567,9 @@ fn miner_forking() { let (sortition_data, had_tenure) = run_sortition(); sortitions_seen.push((sortition_data.clone(), had_tenure)); - let nakamoto_block_ids: Vec<_> = test_observer::get_blocks() - .into_iter() - .filter_map(|block_json| { - if block_json - .as_object() - .unwrap() - .get("miner_signature") - .is_none() - { - return None; - } - let block_id = StacksBlockId::from_hex( - &block_json - .as_object() - .unwrap() - .get("index_block_hash") - .unwrap() - .as_str() - .unwrap()[2..], - ) - .unwrap(); - Some(block_id) - }) - .collect(); - - let (chainstate, _) = StacksChainState::open( - conf.is_mainnet(), - conf.burnchain.chain_id, - &conf.get_chainstate_path_str(), - None, - ) - .unwrap(); - - let nakamoto_headers: HashMap<_, _> = nakamoto_block_ids + let nakamoto_headers: HashMap<_, _> = get_nakamoto_headers(&conf) .into_iter() - .map(|block_id| { - let header_info = NakamotoChainState::get_block_header(chainstate.db(), &block_id) - .unwrap() - .unwrap(); - (header_info.consensus_hash.clone(), header_info) - }) + .map(|header| (header.consensus_hash.clone(), header)) .collect(); if had_tenure { @@ -1562,20 +1621,11 @@ fn miner_forking() { info!("Peer height information"; "peer_1" => peer_1_height, "peer_2" => peer_2_height, "pre_naka_height" => pre_nakamoto_peer_1_height); assert_eq!(peer_1_height, peer_2_height); - let nakamoto_block_ids: Vec<_> = test_observer::get_blocks() - .into_iter() - .filter_map(|block_json| { - block_json - .as_object() - .unwrap() - .get("miner_signature") - .map(|x| x.as_str().unwrap().to_string()) - }) - .collect(); + let nakamoto_blocks_count = get_nakamoto_headers(&conf).len(); assert_eq!( peer_1_height - pre_nakamoto_peer_1_height, - u64::try_from(nakamoto_block_ids.len()).unwrap(), + u64::try_from(nakamoto_blocks_count).unwrap(), "There should be no forks in this test" ); From 13e73fb93d3970f7c3635c2092925aa2b857f150 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Thu, 1 Aug 2024 11:02:14 -0500 Subject: [PATCH 4/4] ci: add new test to CI, comment for test --- .github/workflows/bitcoin-tests.yml | 1 + testnet/stacks-node/src/tests/signer/v0.rs | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index 5c9de28361..15c2e125b0 100644 --- a/.github/workflows/bitcoin-tests.yml +++ b/.github/workflows/bitcoin-tests.yml @@ -93,6 +93,7 @@ jobs: - tests::signer::v0::bitcoind_forking_test - tests::signer::v0::multiple_miners - tests::signer::v0::mock_sign_epoch_25 + - tests::signer::v0::miner_forking - tests::nakamoto_integrations::stack_stx_burn_op_integration_test - tests::nakamoto_integrations::check_block_heights - tests::nakamoto_integrations::clarity_burn_state diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index c9b5cf4854..fe53ca20cf 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -1372,6 +1372,16 @@ fn get_nakamoto_headers(config: &Config) -> Vec { #[test] #[ignore] +// Test two nakamoto miners, with the signer set split between them. +// One of the miners (run-loop-2) is prevented from submitting "good" block commits +// using the "commit stall" test flag in combination with "block broadcast stalls". +// (Because RL2 isn't able to RBF their initial commits after the tip is broadcasted). +// This test works by tracking two different scenarios: +// 1. RL2 must win a sortition that this block commit behavior would lead to a fork in. +// 2. After such a sortition, RL1 must win another block. +// The test asserts that every nakamoto sortition either has a successful tenure, or if +// RL2 wins and they would be expected to fork, no blocks are produced. The test asserts +// that every block produced increments the chain length. fn miner_forking() { if env::var("BITCOIND_TEST") != Ok("1".into()) { return;