From 150c83372d4f27e842f7f612de005b7c0c884f33 Mon Sep 17 00:00:00 2001 From: Simone Orsi Date: Fri, 2 May 2025 22:42:36 +0200 Subject: [PATCH 1/5] Add disallow_reorg_within_first_proposal_burn_block_timing_secs_but_more_than_one_block_scenario() and new madhouse commands Commands added: SendAndMineTransferTx BuildNextBitcoinBlock WaitForAndVerifyBlockRejection VerifyMiner1BlockCount --- Cargo.lock | 29 +-- testnet/stacks-node/Cargo.toml | 4 +- .../src/tests/signer/commands/block_commit.rs | 45 ++++ .../src/tests/signer/commands/block_wait.rs | 203 +++++++++++++++++- .../src/tests/signer/commands/context.rs | 1 + .../src/tests/signer/commands/mod.rs | 10 +- .../src/tests/signer/commands/transfer.rs | 62 ++++++ testnet/stacks-node/src/tests/signer/v0.rs | 47 +++- 8 files changed, 367 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d66c964c00..dd0ec47467 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1889,10 +1889,10 @@ dependencies = [ [[package]] name = "madhouse" -version = "0.1.0" -source = "git+https://github.com/stacks-network/madhouse-rs.git?rev=fc651ddcbaf85e888b06d4a87aa788c4b7ba9309#fc651ddcbaf85e888b06d4a87aa788c4b7ba9309" +version = "0.2.0" +source = "git+https://github.com/stacks-network/madhouse-rs.git?tag=0.2.0#a0983d0f7d43c4b2a765df7f46b7ee28f160924d" dependencies = [ - "proptest 1.6.0 (git+https://github.com/proptest-rs/proptest.git?rev=c9bdf18c232665b2b740c667c81866b598d06dc7)", + "proptest", ] [[package]] @@ -2378,25 +2378,6 @@ dependencies = [ "unarray", ] -[[package]] -name = "proptest" -version = "1.6.0" -source = "git+https://github.com/proptest-rs/proptest.git?rev=c9bdf18c232665b2b740c667c81866b598d06dc7#c9bdf18c232665b2b740c667c81866b598d06dc7" -dependencies = [ - "bit-set", - "bit-vec", - "bitflags 2.4.2", - "lazy_static", - "num-traits", - "rand 0.8.5", - "rand_chacha 0.3.1", - "rand_xorshift", - "regex-syntax 0.8.2", - "rusty-fork", - "tempfile", - "unarray", -] - [[package]] name = "protobuf" version = "2.28.0" @@ -3203,7 +3184,7 @@ dependencies = [ "lazy_static", "libsecp256k1", "nix", - "proptest 1.6.0 (registry+https://github.com/rust-lang/crates.io-index)", + "proptest", "rand 0.8.5", "rand_core 0.6.4", "ripemd", @@ -3242,7 +3223,7 @@ dependencies = [ "mutants", "pico-args", "pinny", - "proptest 1.6.0 (git+https://github.com/proptest-rs/proptest.git?rev=c9bdf18c232665b2b740c667c81866b598d06dc7)", + "proptest", "rand 0.8.5", "regex", "reqwest", diff --git a/testnet/stacks-node/Cargo.toml b/testnet/stacks-node/Cargo.toml index fef83a7c87..6945c719b9 100644 --- a/testnet/stacks-node/Cargo.toml +++ b/testnet/stacks-node/Cargo.toml @@ -52,8 +52,8 @@ tempfile = "3.3" mockito = "1.5" serial_test = "3.2.0" pinny = { git = "https://github.com/BitcoinL2-Labs/pinny-rs.git", rev = "54ba9d533a7b84525a5e65a3eae1a3ae76b9ea49" } #v0.0.2 -madhouse = { git = "https://github.com/stacks-network/madhouse-rs.git", rev = "fc651ddcbaf85e888b06d4a87aa788c4b7ba9309" } -proptest = { git = "https://github.com/proptest-rs/proptest.git", rev = "c9bdf18c232665b2b740c667c81866b598d06dc7" } +madhouse = { git = "https://github.com/stacks-network/madhouse-rs.git", tag = "0.2.0" } +proptest = "1.6.*" [[bin]] name = "stacks-node" diff --git a/testnet/stacks-node/src/tests/signer/commands/block_commit.rs b/testnet/stacks-node/src/tests/signer/commands/block_commit.rs index d2f91ced22..e791bdfab5 100644 --- a/testnet/stacks-node/src/tests/signer/commands/block_commit.rs +++ b/testnet/stacks-node/src/tests/signer/commands/block_commit.rs @@ -35,6 +35,7 @@ impl Command for SubmitBlockCommitMiner2 { let sortdb = burnchain.open_sortition_db(true).unwrap(); self.miners.lock().unwrap().submit_commit_miner_2(&sortdb); + //TODO: ??? state.operations_counter += 1; } fn label(&self) -> String { @@ -79,6 +80,7 @@ impl Command for SubmitBlockCommitMiner1 { let sortdb = burnchain.open_sortition_db(true).unwrap(); self.miners.lock().unwrap().submit_commit_miner_1(&sortdb); + //TODO: ??? state.operations_counter += 1; } fn label(&self) -> String { @@ -93,3 +95,46 @@ impl Command for SubmitBlockCommitMiner1 { ))) } } + +pub struct BuildNextBitcoinBlock { + miners: Arc>, + num_blocks: u64, +} + +impl BuildNextBitcoinBlock { + pub fn new(miners: Arc>, num_blocks: u64) -> Self { + Self { miners, num_blocks } + } +} + +impl Command for BuildNextBitcoinBlock { + fn check(&self, _state: &SignerTestState) -> bool { + info!( + "Checking: Build next {} Bitcoin block(s). Result: {:?}", + self.num_blocks, true + ); + true + } + + fn apply(&self, _state: &mut SignerTestState) { + info!("Applying: Build next {} Bitcoin block(s)", self.num_blocks); + + let mut miners = self.miners.lock().unwrap(); + miners + .btc_regtest_controller_mut() + .build_next_block(self.num_blocks); + //TODO: ??? state.operations_counter += 1; + } + + fn label(&self) -> String { + "BUILD_NEXT_BITCOIN_BLOCK".to_string() + } + + fn build( + ctx: Arc, + ) -> impl Strategy> { + (1u64..=5u64).prop_map(move |num_blocks| { + CommandWrapper::new(BuildNextBitcoinBlock::new(ctx.miners.clone(), num_blocks)) + }) + } +} diff --git a/testnet/stacks-node/src/tests/signer/commands/block_wait.rs b/testnet/stacks-node/src/tests/signer/commands/block_wait.rs index 747c686a12..88d3165bbc 100644 --- a/testnet/stacks-node/src/tests/signer/commands/block_wait.rs +++ b/testnet/stacks-node/src/tests/signer/commands/block_wait.rs @@ -1,11 +1,16 @@ use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex}; +use libsigner::v0::messages::RejectReason; use madhouse::{Command, CommandWrapper}; use proptest::prelude::{Just, Strategy}; use super::context::{SignerTestContext, SignerTestState}; -use crate::tests::signer::v0::{wait_for_block_pushed_by_miner_key, MultipleMinerTest}; +use crate::stacks_common::types::PublicKey; +use crate::tests::signer::v0::{ + get_nakamoto_headers, wait_for_block_global_rejection_with_reject_reason, + wait_for_block_proposal, wait_for_block_pushed_by_miner_key, MultipleMinerTest, +}; pub struct WaitForTenureChangeBlockFromMiner1 { miners: Arc>, @@ -123,3 +128,199 @@ impl Command for WaitForTenureChangeBlockFro )) } } + +/// --------------------------------------------------------- +/// --------------------------------------------------------- +/// --------------------------------------------------------- +/// --------------------------------------------------------- +/// --------------------------------------------------------- + +pub struct WaitForAndVerifyBlockRejection { + miners: Arc>, + reason: RejectReason, + num_signers: usize, +} + +impl WaitForAndVerifyBlockRejection { + pub fn new( + miners: Arc>, + reason: RejectReason, + num_signers: usize, + ) -> Self { + Self { + miners, + reason, + num_signers, + } + } +} + +impl Command for WaitForAndVerifyBlockRejection { + fn check(&self, _state: &SignerTestState) -> bool { + info!( + "Checking: Waiting for block proposal from miner 1 and verifying rejection with reason {:?}", + self.reason + ); + true + } + + fn apply(&self, _state: &mut SignerTestState) { + info!("Applying: Waiting for block proposal from miner 1 and verifying rejection with reason {:?}", self.reason); + + // Get the current block height and miner1's public key + let (block_height, miner_pk_1) = { + let miners = self.miners.lock().unwrap(); + let (conf_1, _) = miners.get_node_configs(); + let chain_info = crate::tests::neon_integrations::get_chain_info(&conf_1); + let current_height = chain_info.stacks_tip_height; + //TODO: let block_n_height = current_height - state.operations_counter as u64; + let block_n_height = current_height - 3; + let (miner_pk_1, _) = miners.get_miner_public_keys(); + (block_n_height, miner_pk_1) + }; + + info!("Waiting for block proposal at height {}", block_height + 1); + + // Wait for a block proposal from miner 1 at height N+1 + let proposed_block = wait_for_block_proposal(30, block_height + 1, &miner_pk_1) + .expect("Timed out waiting for block proposal"); + + let block_hash = proposed_block.header.signer_signature_hash(); + + info!( + "Received block proposal at height {} with hash {:?}", + block_height + 1, + block_hash + ); + + // Check the block has been rejected with the expected reason + wait_for_block_global_rejection_with_reject_reason( + 30, + block_hash, + self.num_signers, + self.reason.clone(), + ) + .expect("Timed out waiting for block rejection"); + + info!( + "Block was rejected with the expected reason: {:?}", + self.reason + ); + } + + fn label(&self) -> String { + format!( + "WAIT_FOR_AND_VERIFY_BLOCK_REJECTION_WITH_REASON_{:?}", + self.reason + ) + } + + fn build( + ctx: Arc, + ) -> impl Strategy> { + (1usize..=5usize).prop_map(move |num_signers: usize| { + CommandWrapper::new(WaitForAndVerifyBlockRejection::new( + ctx.miners.clone(), + RejectReason::ReorgNotAllowed, + num_signers, + )) + }) + } +} + +/// --------------------------------------------------------- +/// --------------------------------------------------------- +/// --------------------------------------------------------- +/// --------------------------------------------------------- +/// --------------------------------------------------------- + +pub struct VerifyMiner1BlockCount { + miners: Arc>, + expected_count: usize, +} + +impl VerifyMiner1BlockCount { + pub fn new(miners: Arc>, expected_count: usize) -> Self { + Self { + miners, + expected_count, + } + } +} + +impl Command for VerifyMiner1BlockCount { + fn check(&self, state: &SignerTestState) -> bool { + info!( + "Checking: Verifying miner 1 block count. Will run if miner 1 commit ops are paused: {:?}", + state.is_primary_miner_skip_commit_op + ); + + // Only run this verification when miner 1's commit operations are paused + state.is_primary_miner_skip_commit_op + } + + fn apply(&self, _state: &mut SignerTestState) { + info!( + "Applying: Verifying miner 1 block count is {}", + self.expected_count + ); + + // Extract everything we need from the locked mutex within this scope + let (stacks_height_before, conf_1, miner_pk_1) = { + let miners = self.miners.lock().unwrap(); + let current_height = miners.get_peer_stacks_tip_height(); + //TODO: let stacks_height_before = current_height - state.operations_counter as u64; + let stacks_height_before = current_height - 3; + + // Get the configs and miner public key + let (conf_1, _) = miners.get_node_configs(); + let (miner_pk_1, _) = miners.get_miner_public_keys(); + + // Return the values we need outside the lock + (stacks_height_before, conf_1, miner_pk_1) + }; + + // Check only expected_count blocks from miner1 have been added after the epoch3 boot + let miner1_blocks_after_boot_to_epoch3 = get_nakamoto_headers(&conf_1) + .into_iter() + .filter(|block| { + // skip first nakamoto block + if block.stacks_block_height == stacks_height_before { + return false; + } + let nakamoto_block_header = block.anchored_header.as_stacks_nakamoto().unwrap(); + miner_pk_1 + .verify( + nakamoto_block_header.miner_signature_hash().as_bytes(), + &nakamoto_block_header.miner_signature, + ) + .unwrap() + }) + .count(); + + assert_eq!( + miner1_blocks_after_boot_to_epoch3, self.expected_count, + "Expected {} blocks from miner 1, but found {}", + self.expected_count, miner1_blocks_after_boot_to_epoch3 + ); + + info!( + "Verified miner 1 has exactly {} blocks after epoch 3 boot", + self.expected_count + ); + } + + fn label(&self) -> String { + format!("VERIFY_MINER_1_BLOCK_COUNT_{}", self.expected_count) + } + + fn build( + ctx: Arc, + ) -> impl Strategy> { + //TODO: randomize expected_count? + Just(CommandWrapper::new(VerifyMiner1BlockCount::new( + ctx.miners.clone(), + 1, + ))) + } +} diff --git a/testnet/stacks-node/src/tests/signer/commands/context.rs b/testnet/stacks-node/src/tests/signer/commands/context.rs index 3feb11a29d..7171ebd40a 100644 --- a/testnet/stacks-node/src/tests/signer/commands/context.rs +++ b/testnet/stacks-node/src/tests/signer/commands/context.rs @@ -53,6 +53,7 @@ pub struct SignerTestState { pub is_secondary_miner_skip_commit_op: bool, pub mining_stalled: bool, pub transfer_txs_submitted: Vec<(StacksHeightBefore, TxId)>, + pub operations_counter: usize, } impl State for SignerTestState {} diff --git a/testnet/stacks-node/src/tests/signer/commands/mod.rs b/testnet/stacks-node/src/tests/signer/commands/mod.rs index 4da0c1ec9a..2dc185d194 100644 --- a/testnet/stacks-node/src/tests/signer/commands/mod.rs +++ b/testnet/stacks-node/src/tests/signer/commands/mod.rs @@ -10,9 +10,12 @@ mod sortition; mod stacks_mining; mod transfer; -pub use bitcoin_mining::MineBitcoinBlock; -pub use block_commit::SubmitBlockCommitMiner2; -pub use block_wait::{WaitForTenureChangeBlockFromMiner1, WaitForTenureChangeBlockFromMiner2}; +pub use bitcoin_mining::{MineBitcoinBlock, MineBitcoinBlockTenureChangeMiner1}; +pub use block_commit::{BuildNextBitcoinBlock, SubmitBlockCommitMiner1, SubmitBlockCommitMiner2}; +pub use block_wait::{ + VerifyMiner1BlockCount, WaitForAndVerifyBlockRejection, WaitForTenureChangeBlockFromMiner1, + WaitForTenureChangeBlockFromMiner2, +}; pub use boot::BootToEpoch3; pub use commit_ops::{SkipCommitOpMiner1, SkipCommitOpMiner2}; pub use context::SignerTestContext; @@ -21,3 +24,4 @@ pub use sortition::{ VerifyLastSortitionWinnerReorged, VerifyMiner1WonSortition, VerifyMiner2WonSortition, }; pub use stacks_mining::{PauseStacksMining, ResumeStacksMining}; +pub use transfer::SendAndMineTransferTx; diff --git a/testnet/stacks-node/src/tests/signer/commands/transfer.rs b/testnet/stacks-node/src/tests/signer/commands/transfer.rs index 749bbe2dfc..5a0524de73 100644 --- a/testnet/stacks-node/src/tests/signer/commands/transfer.rs +++ b/testnet/stacks-node/src/tests/signer/commands/transfer.rs @@ -45,3 +45,65 @@ impl Command for SendTransferTx { Just(CommandWrapper::new(SendTransferTx::new(ctx.miners.clone()))) } } + +pub struct SendAndMineTransferTx { + miners: Arc>, + timeout_secs: u64, +} + +impl SendAndMineTransferTx { + pub fn new(miners: Arc>, timeout_secs: u64) -> Self { + Self { + miners, + timeout_secs, + } + } +} + +impl Command for SendAndMineTransferTx { + fn check(&self, _state: &SignerTestState) -> bool { + info!( + "Checking: Send and mine transfer tx with timeout {} seconds", + self.timeout_secs + ); + true + } + + fn apply(&self, _state: &mut SignerTestState) { + info!( + "Applying: Send and mine transfer tx with timeout {} seconds", + self.timeout_secs + ); + + // Get the configs and check the stacks height before + let (conf_1, _) = self.miners.lock().unwrap().get_node_configs(); + let stacks_height_before = get_chain_info(&conf_1).stacks_tip_height; + + // Execute the send and mine operation + let mut miners = self.miners.lock().unwrap(); + miners + .send_and_mine_transfer_tx(self.timeout_secs) + .expect("Failed to send and mine transfer tx"); + + // Check that the stacks height has increased + let stacks_height_after = get_chain_info(&conf_1).stacks_tip_height; + assert_eq!( + stacks_height_after, + stacks_height_before + 1, + "Stacks height should have increased by 1 after mining a transfer tx" + ); + } + + fn label(&self) -> String { + "SEND_AND_MINE_TRANSFER_TX".to_string() + } + + fn build( + ctx: Arc, + ) -> impl Strategy> { + // Generate a timeout between 20 and 40 seconds + (20u64..40u64).prop_map(move |timeout_secs| { + CommandWrapper::new(SendAndMineTransferTx::new(ctx.miners.clone(), timeout_secs)) + }) + } +} diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 2a5fd3bfd5..8519442414 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -29,7 +29,7 @@ use libsigner::v0::messages::{ use libsigner::{ BlockProposal, BlockProposalData, SignerSession, StackerDBSession, VERSION_STRING, }; -use madhouse::{execute_commands, prop_allof, scenario, Command}; +use madhouse::{execute_commands, prop_allof, scenario, Command, CommandWrapper}; use pinny::tag; use proptest::prelude::Strategy; use rand::{thread_rng, Rng}; @@ -1159,7 +1159,7 @@ fn wait_for_tenure_change_tx( /// Waits for a block proposal to be observed in the test_observer stackerdb chunks at the expected height /// and signed by the expected miner -fn wait_for_block_proposal( +pub fn wait_for_block_proposal( timeout_secs: u64, expected_height: u64, expected_miner: &StacksPublicKey, @@ -1258,7 +1258,7 @@ fn wait_for_block_global_rejection( /// 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 and the specified reject_reason -fn wait_for_block_global_rejection_with_reject_reason( +pub fn wait_for_block_global_rejection_with_reject_reason( timeout_secs: u64, block_signer_signature_hash: Sha512Trunc256Sum, num_signers: usize, @@ -3149,7 +3149,7 @@ fn multiple_miners() { /// 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 { +pub fn get_nakamoto_headers(config: &Config) -> Vec { let nakamoto_block_ids: HashSet<_> = test_observer::get_blocks() .into_iter() .filter_map(|block_json| { @@ -10972,6 +10972,45 @@ fn disallow_reorg_within_first_proposal_burn_block_timing_secs_but_more_than_one miners.shutdown(); } +#[test] +#[ignore] +fn disallow_reorg_within_first_proposal_burn_block_timing_secs_but_more_than_one_block_scenario() { + if env::var("BITCOIND_TEST") != Ok("1".into()) { + return; + } + + let num_signers = 5; + let num_txs = 3; + + let test_context = Arc::new(SignerTestContext::new(num_signers, num_txs)); + + scenario![ + test_context, + SkipCommitOpMiner2, + BootToEpoch3, + SkipCommitOpMiner1, + MineBitcoinBlockTenureChangeMiner1, + VerifyMiner1WonSortition, + SubmitBlockCommitMiner2, + PauseStacksMining, + MineBitcoinBlock, + SubmitBlockCommitMiner1, + ResumeStacksMining, + WaitForTenureChangeBlockFromMiner2, + VerifyMiner2WonSortition, + SendAndMineTransferTx, + SendAndMineTransferTx, + (BuildNextBitcoinBlock::new(test_context.miners.clone(), 1)), + (WaitForAndVerifyBlockRejection::new( + test_context.miners.clone(), + RejectReason::ReorgNotAllowed, + num_signers + )), + (VerifyMiner1BlockCount::new(test_context.miners.clone(), 1)), + ShutdownMiners, + ] +} + #[test] #[ignore] /// This test verifies that a miner will produce a TenureExtend transaction From 5790de27888177fa152af2c75fdb7abc1be4645d Mon Sep 17 00:00:00 2001 From: Simone Orsi Date: Fri, 2 May 2025 22:51:56 +0200 Subject: [PATCH 2/5] Update block_wait.rs --- testnet/stacks-node/src/tests/signer/commands/block_wait.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/testnet/stacks-node/src/tests/signer/commands/block_wait.rs b/testnet/stacks-node/src/tests/signer/commands/block_wait.rs index 88d3165bbc..7a44f551e6 100644 --- a/testnet/stacks-node/src/tests/signer/commands/block_wait.rs +++ b/testnet/stacks-node/src/tests/signer/commands/block_wait.rs @@ -233,6 +233,7 @@ impl Command for WaitForAndVerifyBlockReject /// --------------------------------------------------------- /// --------------------------------------------------------- /// --------------------------------------------------------- +/// TODO: Maybe this should be in another commands/ file pub struct VerifyMiner1BlockCount { miners: Arc>, From 5b4acdb25b5b69327cb79ffa23fb44f3e1e5b315 Mon Sep 17 00:00:00 2001 From: Simone Orsi Date: Mon, 5 May 2025 17:06:37 +0200 Subject: [PATCH 3/5] Add mined blocks count to context --- .../tests/signer/commands/bitcoin_mining.rs | 52 +++++++++++++++++- .../src/tests/signer/commands/block_commit.rs | 47 +--------------- .../src/tests/signer/commands/block_wait.rs | 54 ++++++------------- .../src/tests/signer/commands/context.rs | 2 +- .../src/tests/signer/commands/mod.rs | 4 +- .../src/tests/signer/commands/transfer.rs | 8 ++- testnet/stacks-node/src/tests/signer/v0.rs | 5 +- 7 files changed, 76 insertions(+), 96 deletions(-) diff --git a/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs b/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs index e2b1fbcbaa..d9947fa4a4 100644 --- a/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs +++ b/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs @@ -48,7 +48,7 @@ impl Command for MineBitcoinBlockTenureChang && burn_height > miner_2_submitted_commit_last_burn_height } - fn apply(&self, _state: &mut SignerTestState) { + fn apply(&self, state: &mut SignerTestState) { info!("Applying: Miner 1 mining Bitcoin block and tenure change tx"); let (stacks_height_before, conf_1, miner_pk_1) = { @@ -75,6 +75,9 @@ impl Command for MineBitcoinBlockTenureChang let miner_1_block = wait_for_block_pushed_by_miner_key(30, stacks_height_before + 1, &miner_pk_1) .expect("Failed to get block"); + + state.blocks_mined += 1; + info!("Increased blocks mined count to {}", state.blocks_mined); let mined_block_height = miner_1_block.header.chain_length; info!( @@ -139,7 +142,7 @@ impl Command for MineBitcoinBlockTenureChang && burn_height > miner_1_submitted_commit_last_burn_height } - fn apply(&self, _state: &mut SignerTestState) { + fn apply(&self, state: &mut SignerTestState) { info!("Applying: Miner 2 mining Bitcoin block and tenure change tx"); let stacks_height_before = self.miners.lock().unwrap().get_peer_stacks_tip_height(); @@ -164,6 +167,9 @@ impl Command for MineBitcoinBlockTenureChang wait_for_block_pushed_by_miner_key(30, stacks_height_before + 1, &miner_pk_2) .expect("Failed to get block N"); + state.blocks_mined += 1; + info!("Increased blocks mined count to {}", state.blocks_mined); + let mined_block_height = secondary_miner_block.header.chain_length; let info_after = get_chain_info(&conf_2); @@ -242,3 +248,45 @@ impl Command for MineBitcoinBlock { }) } } + +pub struct BuildNextBitcoinBlocks { + miners: Arc>, + num_blocks: u64, +} + +impl BuildNextBitcoinBlocks { + pub fn new(miners: Arc>, num_blocks: u64) -> Self { + Self { miners, num_blocks } + } +} + +impl Command for BuildNextBitcoinBlocks { + fn check(&self, _state: &SignerTestState) -> bool { + info!( + "Checking: Build next {} Bitcoin block(s). Result: {:?}", + self.num_blocks, true + ); + true + } + + fn apply(&self, _state: &mut SignerTestState) { + info!("Applying: Build next {} Bitcoin block(s)", self.num_blocks); + + let mut miners = self.miners.lock().unwrap(); + miners + .btc_regtest_controller_mut() + .build_next_block(self.num_blocks); + } + + fn label(&self) -> String { + "BUILD_NEXT_BITCOIN_BLOCK".to_string() + } + + fn build( + ctx: Arc, + ) -> impl Strategy> { + (1u64..=5u64).prop_map(move |num_blocks| { + CommandWrapper::new(BuildNextBitcoinBlocks::new(ctx.miners.clone(), num_blocks)) + }) + } +} diff --git a/testnet/stacks-node/src/tests/signer/commands/block_commit.rs b/testnet/stacks-node/src/tests/signer/commands/block_commit.rs index e791bdfab5..bce64bcede 100644 --- a/testnet/stacks-node/src/tests/signer/commands/block_commit.rs +++ b/testnet/stacks-node/src/tests/signer/commands/block_commit.rs @@ -35,7 +35,6 @@ impl Command for SubmitBlockCommitMiner2 { let sortdb = burnchain.open_sortition_db(true).unwrap(); self.miners.lock().unwrap().submit_commit_miner_2(&sortdb); - //TODO: ??? state.operations_counter += 1; } fn label(&self) -> String { @@ -80,7 +79,6 @@ impl Command for SubmitBlockCommitMiner1 { let sortdb = burnchain.open_sortition_db(true).unwrap(); self.miners.lock().unwrap().submit_commit_miner_1(&sortdb); - //TODO: ??? state.operations_counter += 1; } fn label(&self) -> String { @@ -94,47 +92,4 @@ impl Command for SubmitBlockCommitMiner1 { ctx.miners.clone(), ))) } -} - -pub struct BuildNextBitcoinBlock { - miners: Arc>, - num_blocks: u64, -} - -impl BuildNextBitcoinBlock { - pub fn new(miners: Arc>, num_blocks: u64) -> Self { - Self { miners, num_blocks } - } -} - -impl Command for BuildNextBitcoinBlock { - fn check(&self, _state: &SignerTestState) -> bool { - info!( - "Checking: Build next {} Bitcoin block(s). Result: {:?}", - self.num_blocks, true - ); - true - } - - fn apply(&self, _state: &mut SignerTestState) { - info!("Applying: Build next {} Bitcoin block(s)", self.num_blocks); - - let mut miners = self.miners.lock().unwrap(); - miners - .btc_regtest_controller_mut() - .build_next_block(self.num_blocks); - //TODO: ??? state.operations_counter += 1; - } - - fn label(&self) -> String { - "BUILD_NEXT_BITCOIN_BLOCK".to_string() - } - - fn build( - ctx: Arc, - ) -> impl Strategy> { - (1u64..=5u64).prop_map(move |num_blocks| { - CommandWrapper::new(BuildNextBitcoinBlock::new(ctx.miners.clone(), num_blocks)) - }) - } -} +} \ No newline at end of file diff --git a/testnet/stacks-node/src/tests/signer/commands/block_wait.rs b/testnet/stacks-node/src/tests/signer/commands/block_wait.rs index 7a44f551e6..b9a71ae95c 100644 --- a/testnet/stacks-node/src/tests/signer/commands/block_wait.rs +++ b/testnet/stacks-node/src/tests/signer/commands/block_wait.rs @@ -55,6 +55,7 @@ impl Command for WaitForTenureChangeBlockFro let _miner_1_block = wait_for_block_pushed_by_miner_key(30, expected_height, &miner_pk_1) .expect(&format!("Failed to get block {}", expected_height)); + } fn label(&self) -> String { @@ -114,6 +115,7 @@ impl Command for WaitForTenureChangeBlockFro let _miner_2_block_n_1 = wait_for_block_pushed_by_miner_key(30, expected_stacks_height, &miner_pk_2) .expect(&format!("Failed to get block {:?}", expected_stacks_height)); + } fn label(&self) -> String { @@ -129,12 +131,6 @@ impl Command for WaitForTenureChangeBlockFro } } -/// --------------------------------------------------------- -/// --------------------------------------------------------- -/// --------------------------------------------------------- -/// --------------------------------------------------------- -/// --------------------------------------------------------- - pub struct WaitForAndVerifyBlockRejection { miners: Arc>, reason: RejectReason, @@ -164,24 +160,21 @@ impl Command for WaitForAndVerifyBlockReject true } - fn apply(&self, _state: &mut SignerTestState) { + fn apply(&self, state: &mut SignerTestState) { info!("Applying: Waiting for block proposal from miner 1 and verifying rejection with reason {:?}", self.reason); - // Get the current block height and miner1's public key let (block_height, miner_pk_1) = { let miners = self.miners.lock().unwrap(); let (conf_1, _) = miners.get_node_configs(); let chain_info = crate::tests::neon_integrations::get_chain_info(&conf_1); let current_height = chain_info.stacks_tip_height; - //TODO: let block_n_height = current_height - state.operations_counter as u64; - let block_n_height = current_height - 3; + let block_n_height = current_height - state.blocks_mined as u64; let (miner_pk_1, _) = miners.get_miner_public_keys(); (block_n_height, miner_pk_1) }; info!("Waiting for block proposal at height {}", block_height + 1); - // Wait for a block proposal from miner 1 at height N+1 let proposed_block = wait_for_block_proposal(30, block_height + 1, &miner_pk_1) .expect("Timed out waiting for block proposal"); @@ -193,7 +186,6 @@ impl Command for WaitForAndVerifyBlockReject block_hash ); - // Check the block has been rejected with the expected reason wait_for_block_global_rejection_with_reject_reason( 30, block_hash, @@ -228,23 +220,16 @@ impl Command for WaitForAndVerifyBlockReject } } -/// --------------------------------------------------------- -/// --------------------------------------------------------- -/// --------------------------------------------------------- -/// --------------------------------------------------------- -/// --------------------------------------------------------- -/// TODO: Maybe this should be in another commands/ file - pub struct VerifyMiner1BlockCount { miners: Arc>, - expected_count: usize, + expected_block_count: usize, } impl VerifyMiner1BlockCount { - pub fn new(miners: Arc>, expected_count: usize) -> Self { + pub fn new(miners: Arc>, expected_block_count: usize) -> Self { Self { miners, - expected_count, + expected_block_count, } } } @@ -255,37 +240,31 @@ impl Command for VerifyMiner1BlockCount { "Checking: Verifying miner 1 block count. Will run if miner 1 commit ops are paused: {:?}", state.is_primary_miner_skip_commit_op ); - - // Only run this verification when miner 1's commit operations are paused state.is_primary_miner_skip_commit_op } - fn apply(&self, _state: &mut SignerTestState) { + fn apply(&self, state: &mut SignerTestState) { info!( "Applying: Verifying miner 1 block count is {}", - self.expected_count + self.expected_block_count ); - // Extract everything we need from the locked mutex within this scope let (stacks_height_before, conf_1, miner_pk_1) = { let miners = self.miners.lock().unwrap(); let current_height = miners.get_peer_stacks_tip_height(); - //TODO: let stacks_height_before = current_height - state.operations_counter as u64; - let stacks_height_before = current_height - 3; + let stacks_height_before = current_height - state.blocks_mined as u64; - // Get the configs and miner public key let (conf_1, _) = miners.get_node_configs(); let (miner_pk_1, _) = miners.get_miner_public_keys(); - // Return the values we need outside the lock (stacks_height_before, conf_1, miner_pk_1) }; - // Check only expected_count blocks from miner1 have been added after the epoch3 boot + // Check only expected_block_count blocks from miner1 have been added after the epoch3 boot let miner1_blocks_after_boot_to_epoch3 = get_nakamoto_headers(&conf_1) .into_iter() .filter(|block| { - // skip first nakamoto block + // Skip first nakamoto block if block.stacks_block_height == stacks_height_before { return false; } @@ -300,25 +279,24 @@ impl Command for VerifyMiner1BlockCount { .count(); assert_eq!( - miner1_blocks_after_boot_to_epoch3, self.expected_count, + miner1_blocks_after_boot_to_epoch3, self.expected_block_count, "Expected {} blocks from miner 1, but found {}", - self.expected_count, miner1_blocks_after_boot_to_epoch3 + self.expected_block_count, miner1_blocks_after_boot_to_epoch3 ); info!( "Verified miner 1 has exactly {} blocks after epoch 3 boot", - self.expected_count + self.expected_block_count ); } fn label(&self) -> String { - format!("VERIFY_MINER_1_BLOCK_COUNT_{}", self.expected_count) + format!("VERIFY_MINER_1_BLOCK_COUNT_{}", self.expected_block_count) } fn build( ctx: Arc, ) -> impl Strategy> { - //TODO: randomize expected_count? Just(CommandWrapper::new(VerifyMiner1BlockCount::new( ctx.miners.clone(), 1, diff --git a/testnet/stacks-node/src/tests/signer/commands/context.rs b/testnet/stacks-node/src/tests/signer/commands/context.rs index 7171ebd40a..9a44ac53d5 100644 --- a/testnet/stacks-node/src/tests/signer/commands/context.rs +++ b/testnet/stacks-node/src/tests/signer/commands/context.rs @@ -53,7 +53,7 @@ pub struct SignerTestState { pub is_secondary_miner_skip_commit_op: bool, pub mining_stalled: bool, pub transfer_txs_submitted: Vec<(StacksHeightBefore, TxId)>, - pub operations_counter: usize, + pub blocks_mined: usize, } impl State for SignerTestState {} diff --git a/testnet/stacks-node/src/tests/signer/commands/mod.rs b/testnet/stacks-node/src/tests/signer/commands/mod.rs index 2dc185d194..eafc18f9e8 100644 --- a/testnet/stacks-node/src/tests/signer/commands/mod.rs +++ b/testnet/stacks-node/src/tests/signer/commands/mod.rs @@ -10,8 +10,8 @@ mod sortition; mod stacks_mining; mod transfer; -pub use bitcoin_mining::{MineBitcoinBlock, MineBitcoinBlockTenureChangeMiner1}; -pub use block_commit::{BuildNextBitcoinBlock, SubmitBlockCommitMiner1, SubmitBlockCommitMiner2}; +pub use bitcoin_mining::{MineBitcoinBlock, MineBitcoinBlockTenureChangeMiner1, BuildNextBitcoinBlocks}; +pub use block_commit::{SubmitBlockCommitMiner1, SubmitBlockCommitMiner2}; pub use block_wait::{ VerifyMiner1BlockCount, WaitForAndVerifyBlockRejection, WaitForTenureChangeBlockFromMiner1, WaitForTenureChangeBlockFromMiner2, diff --git a/testnet/stacks-node/src/tests/signer/commands/transfer.rs b/testnet/stacks-node/src/tests/signer/commands/transfer.rs index 5a0524de73..e80db5f39c 100644 --- a/testnet/stacks-node/src/tests/signer/commands/transfer.rs +++ b/testnet/stacks-node/src/tests/signer/commands/transfer.rs @@ -69,23 +69,22 @@ impl Command for SendAndMineTransferTx { true } - fn apply(&self, _state: &mut SignerTestState) { + fn apply(&self, state: &mut SignerTestState) { info!( "Applying: Send and mine transfer tx with timeout {} seconds", self.timeout_secs ); - // Get the configs and check the stacks height before let (conf_1, _) = self.miners.lock().unwrap().get_node_configs(); let stacks_height_before = get_chain_info(&conf_1).stacks_tip_height; - // Execute the send and mine operation let mut miners = self.miners.lock().unwrap(); miners .send_and_mine_transfer_tx(self.timeout_secs) .expect("Failed to send and mine transfer tx"); + state.blocks_mined += 1; + info!("Increased blocks mined count to {}", state.blocks_mined); - // Check that the stacks height has increased let stacks_height_after = get_chain_info(&conf_1).stacks_tip_height; assert_eq!( stacks_height_after, @@ -101,7 +100,6 @@ impl Command for SendAndMineTransferTx { fn build( ctx: Arc, ) -> impl Strategy> { - // Generate a timeout between 20 and 40 seconds (20u64..40u64).prop_map(move |timeout_secs| { CommandWrapper::new(SendAndMineTransferTx::new(ctx.miners.clone(), timeout_secs)) }) diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 8519442414..1807cfa950 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -10981,6 +10981,7 @@ fn disallow_reorg_within_first_proposal_burn_block_timing_secs_but_more_than_one let num_signers = 5; let num_txs = 3; + let num_blocks = 1; let test_context = Arc::new(SignerTestContext::new(num_signers, num_txs)); @@ -11000,13 +11001,13 @@ fn disallow_reorg_within_first_proposal_burn_block_timing_secs_but_more_than_one VerifyMiner2WonSortition, SendAndMineTransferTx, SendAndMineTransferTx, - (BuildNextBitcoinBlock::new(test_context.miners.clone(), 1)), + (BuildNextBitcoinBlocks::new(test_context.miners.clone(), num_blocks)), (WaitForAndVerifyBlockRejection::new( test_context.miners.clone(), RejectReason::ReorgNotAllowed, num_signers )), - (VerifyMiner1BlockCount::new(test_context.miners.clone(), 1)), + (VerifyMiner1BlockCount::new(test_context.miners.clone(), num_blocks as usize)), ShutdownMiners, ] } From 84b6aff6f598419a5855cc11861021d4d355dd53 Mon Sep 17 00:00:00 2001 From: Simone Orsi Date: Mon, 5 May 2025 17:14:16 +0200 Subject: [PATCH 4/5] chore: keep linters happy --- .../stacks-node/src/tests/signer/commands/bitcoin_mining.rs | 2 +- testnet/stacks-node/src/tests/signer/commands/block_commit.rs | 2 +- testnet/stacks-node/src/tests/signer/commands/block_wait.rs | 2 -- testnet/stacks-node/src/tests/signer/commands/mod.rs | 4 +++- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs b/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs index d9947fa4a4..6bce067a3d 100644 --- a/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs +++ b/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs @@ -75,7 +75,7 @@ impl Command for MineBitcoinBlockTenureChang let miner_1_block = wait_for_block_pushed_by_miner_key(30, stacks_height_before + 1, &miner_pk_1) .expect("Failed to get block"); - + state.blocks_mined += 1; info!("Increased blocks mined count to {}", state.blocks_mined); diff --git a/testnet/stacks-node/src/tests/signer/commands/block_commit.rs b/testnet/stacks-node/src/tests/signer/commands/block_commit.rs index bce64bcede..d2f91ced22 100644 --- a/testnet/stacks-node/src/tests/signer/commands/block_commit.rs +++ b/testnet/stacks-node/src/tests/signer/commands/block_commit.rs @@ -92,4 +92,4 @@ impl Command for SubmitBlockCommitMiner1 { ctx.miners.clone(), ))) } -} \ No newline at end of file +} diff --git a/testnet/stacks-node/src/tests/signer/commands/block_wait.rs b/testnet/stacks-node/src/tests/signer/commands/block_wait.rs index b9a71ae95c..7572d90582 100644 --- a/testnet/stacks-node/src/tests/signer/commands/block_wait.rs +++ b/testnet/stacks-node/src/tests/signer/commands/block_wait.rs @@ -55,7 +55,6 @@ impl Command for WaitForTenureChangeBlockFro let _miner_1_block = wait_for_block_pushed_by_miner_key(30, expected_height, &miner_pk_1) .expect(&format!("Failed to get block {}", expected_height)); - } fn label(&self) -> String { @@ -115,7 +114,6 @@ impl Command for WaitForTenureChangeBlockFro let _miner_2_block_n_1 = wait_for_block_pushed_by_miner_key(30, expected_stacks_height, &miner_pk_2) .expect(&format!("Failed to get block {:?}", expected_stacks_height)); - } fn label(&self) -> String { diff --git a/testnet/stacks-node/src/tests/signer/commands/mod.rs b/testnet/stacks-node/src/tests/signer/commands/mod.rs index eafc18f9e8..cced710038 100644 --- a/testnet/stacks-node/src/tests/signer/commands/mod.rs +++ b/testnet/stacks-node/src/tests/signer/commands/mod.rs @@ -10,7 +10,9 @@ mod sortition; mod stacks_mining; mod transfer; -pub use bitcoin_mining::{MineBitcoinBlock, MineBitcoinBlockTenureChangeMiner1, BuildNextBitcoinBlocks}; +pub use bitcoin_mining::{ + BuildNextBitcoinBlocks, MineBitcoinBlock, MineBitcoinBlockTenureChangeMiner1, +}; pub use block_commit::{SubmitBlockCommitMiner1, SubmitBlockCommitMiner2}; pub use block_wait::{ VerifyMiner1BlockCount, WaitForAndVerifyBlockRejection, WaitForTenureChangeBlockFromMiner1, From 40bfadedea537cf8803918947e18e94f93ddb4ed Mon Sep 17 00:00:00 2001 From: Simone Orsi Date: Mon, 5 May 2025 20:31:56 +0200 Subject: [PATCH 5/5] Create dynamic MinerCommitOp command --- .../tests/signer/commands/bitcoin_mining.rs | 2 +- .../src/tests/signer/commands/commit_ops.rs | 78 +++++++++++++++++++ .../src/tests/signer/commands/context.rs | 15 ++++ .../src/tests/signer/commands/mod.rs | 2 +- testnet/stacks-node/src/tests/signer/v0.rs | 4 +- 5 files changed, 97 insertions(+), 4 deletions(-) diff --git a/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs b/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs index 6bce067a3d..e95830d794 100644 --- a/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs +++ b/testnet/stacks-node/src/tests/signer/commands/bitcoin_mining.rs @@ -279,7 +279,7 @@ impl Command for BuildNextBitcoinBlocks { } fn label(&self) -> String { - "BUILD_NEXT_BITCOIN_BLOCK".to_string() + "BUILD_NEXT_BITCOIN_BLOCKS".to_string() } fn build( diff --git a/testnet/stacks-node/src/tests/signer/commands/commit_ops.rs b/testnet/stacks-node/src/tests/signer/commands/commit_ops.rs index 6786fa15c7..064badeff0 100644 --- a/testnet/stacks-node/src/tests/signer/commands/commit_ops.rs +++ b/testnet/stacks-node/src/tests/signer/commands/commit_ops.rs @@ -89,3 +89,81 @@ impl Command for SkipCommitOpMiner2 { ))) } } + +pub struct MinerCommitOp { + ctx: Arc, + miner_index: usize, + skip: bool, +} + +impl MinerCommitOp { + pub fn new(ctx: Arc, miner_index: usize, skip: bool) -> Self { + if miner_index != 1 && miner_index != 2 { + panic!( + "Invalid miner index: {}. Only miners 1 and 2 are supported.", + miner_index + ); + } + Self { + ctx, + miner_index, + skip, + } + } +} + +impl Command for MinerCommitOp { + fn check(&self, state: &SignerTestState) -> bool { + let current_state = match self.miner_index { + 1 => state.is_primary_miner_skip_commit_op, + 2 => state.is_secondary_miner_skip_commit_op, + _ => unreachable!(), + }; + + let should_apply = current_state != self.skip; + + info!( + "Checking: {} commit operations for miner {}. Result: {:?}", + if self.skip { "Skipping" } else { "Enabling" }, + self.miner_index, + should_apply + ); + + should_apply + } + + fn apply(&self, state: &mut SignerTestState) { + info!( + "Applying: {} commit operations for miner {}", + if self.skip { "Skipping" } else { "Enabling" }, + self.miner_index + ); + + self.ctx + .get_miner_skip_commit_flag(self.miner_index) + .set(self.skip); + + match self.miner_index { + 1 => state.is_primary_miner_skip_commit_op = self.skip, + 2 => state.is_secondary_miner_skip_commit_op = self.skip, + _ => unreachable!(), + } + } + + fn label(&self) -> String { + format!( + "{}_COMMIT_OP_MINER_{}", + if self.skip { "SKIP" } else { "ENABLE" }, + self.miner_index + ) + } + + fn build( + ctx: Arc, + ) -> impl Strategy> { + use proptest::prelude::*; + (prop_oneof![Just(1), Just(2)], any::()).prop_map(move |(miner_index, skip)| { + CommandWrapper::new(MinerCommitOp::new(ctx.clone(), miner_index, skip)) + }) + } +} diff --git a/testnet/stacks-node/src/tests/signer/commands/context.rs b/testnet/stacks-node/src/tests/signer/commands/context.rs index 9a44ac53d5..8bfb4e0674 100644 --- a/testnet/stacks-node/src/tests/signer/commands/context.rs +++ b/testnet/stacks-node/src/tests/signer/commands/context.rs @@ -41,6 +41,21 @@ impl SignerTestContext { miners: Arc::new(Mutex::new(miners)), } } + + pub fn get_miner_skip_commit_flag( + &self, + miner_index: usize, + ) -> stacks::util::tests::TestFlag { + let miners = self.miners.lock().unwrap(); + match miner_index { + 1 => miners.get_primary_skip_commit_flag(), + 2 => miners.get_secondary_skip_commit_flag(), + _ => panic!( + "Invalid miner index: {}. Only miners 1 and 2 are supported.", + miner_index + ), + } + } } type StacksHeightBefore = u64; diff --git a/testnet/stacks-node/src/tests/signer/commands/mod.rs b/testnet/stacks-node/src/tests/signer/commands/mod.rs index cced710038..dbf7d8b75e 100644 --- a/testnet/stacks-node/src/tests/signer/commands/mod.rs +++ b/testnet/stacks-node/src/tests/signer/commands/mod.rs @@ -19,7 +19,7 @@ pub use block_wait::{ WaitForTenureChangeBlockFromMiner2, }; pub use boot::BootToEpoch3; -pub use commit_ops::{SkipCommitOpMiner1, SkipCommitOpMiner2}; +pub use commit_ops::{MinerCommitOp, SkipCommitOpMiner1, SkipCommitOpMiner2}; pub use context::SignerTestContext; pub use shutdown::ShutdownMiners; pub use sortition::{ diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 1807cfa950..d33a110acd 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -10987,9 +10987,9 @@ fn disallow_reorg_within_first_proposal_burn_block_timing_secs_but_more_than_one scenario![ test_context, - SkipCommitOpMiner2, + (MinerCommitOp::new(test_context.clone(), 2, true)), // SkipCommitOpMiner2 BootToEpoch3, - SkipCommitOpMiner1, + (MinerCommitOp::new(test_context.clone(), 1, true)), // SkipCommitOpMiner1 MineBitcoinBlockTenureChangeMiner1, VerifyMiner1WonSortition, SubmitBlockCommitMiner2,