Skip to content

Commit 846909f

Browse files
committed
fix: resolve two mock miner stalls
The mock miner would stall if: 1. Through bad timing it hits the `self.last_block_mined.is_none() && parent_block_info.parent_tenure.is_none()` case 2. A block with no sortition arrives This commit fixes both and adds integration tests for them.
1 parent 1d0f97c commit 846909f

File tree

4 files changed

+300
-3
lines changed

4 files changed

+300
-3
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
1111

1212
- Setup for epoch 3.4 and Clarity version 5. Epoch 3.4 is currently set to activate at Bitcoin height 3,400,000 (very far in the future) until an activation height is selected. Clarity will activate with epoch 3.4.
1313

14+
### Fixed
15+
16+
- Resolved several cases where a mock-miner would stop mining
17+
1418
## [3.3.0.0.5]
1519

1620
### Added

stacks-node/src/nakamoto_node/miner.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,8 +1527,14 @@ impl BlockMinerThread {
15271527
}
15281528

15291529
if self.last_block_mined.is_none() && parent_block_info.parent_tenure.is_none() {
1530-
warn!("Miner should be starting a new tenure, but failed to load parent tenure info");
1531-
return Err(NakamotoNodeError::ParentNotFound);
1530+
if self.config.node.mock_mining {
1531+
info!("Mock miner will follow canonical tip within an ongoing tenure; no parent tenure info loaded yet");
1532+
} else {
1533+
warn!(
1534+
"Miner should be starting a new tenure, but failed to load parent tenure info"
1535+
);
1536+
return Err(NakamotoNodeError::ParentNotFound);
1537+
}
15321538
};
15331539

15341540
// create our coinbase if this is the first block we've mined this tenure

stacks-node/src/nakamoto_node/relayer.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,9 @@ impl RelayerThread {
816816
false
817817
});
818818

819-
if won_last_winning_snapshot && commits_to_tip_tenure {
819+
if (won_last_winning_snapshot && commits_to_tip_tenure)
820+
|| self.config.get_node_config(false).mock_mining
821+
{
820822
debug!(
821823
"Relayer: we won the last winning sortition {}",
822824
&last_winning_snapshot.consensus_hash

stacks-node/src/tests/nakamoto_integrations.rs

Lines changed: 285 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9324,6 +9324,291 @@ fn mock_mining() {
93249324
follower_thread.join().unwrap();
93259325
}
93269326

9327+
fn run_mock_mining_ongoing_tenure_boot_test(check_empty_sortition_recovery: bool) {
9328+
if env::var("BITCOIND_TEST") != Ok("1".into()) {
9329+
return;
9330+
}
9331+
9332+
let (mut naka_conf, _miner_account) = naka_neon_integration_conf(None);
9333+
naka_conf.node.pox_sync_sample_secs = 30;
9334+
naka_conf.miner.tenure_cost_limit_per_block_percentage = None;
9335+
let sender_sk = Secp256k1PrivateKey::random();
9336+
let sender_signer_sk = Secp256k1PrivateKey::random();
9337+
let sender_signer_addr = tests::to_addr(&sender_signer_sk);
9338+
let mut signers = TestSigners::new(vec![sender_signer_sk.clone()]);
9339+
let sender_addr = tests::to_addr(&sender_sk);
9340+
let send_amt = 100;
9341+
let send_fee = 180;
9342+
9343+
let node_1_rpc = gen_random_port();
9344+
let node_1_p2p = gen_random_port();
9345+
let node_2_rpc = gen_random_port();
9346+
let node_2_p2p = gen_random_port();
9347+
9348+
let localhost = "127.0.0.1";
9349+
naka_conf.node.rpc_bind = format!("{localhost}:{node_1_rpc}");
9350+
naka_conf.node.p2p_bind = format!("{localhost}:{node_1_p2p}");
9351+
naka_conf.node.data_url = format!("http://{localhost}:{node_1_rpc}");
9352+
naka_conf.node.p2p_address = format!("{localhost}:{node_1_p2p}");
9353+
let http_origin = format!("http://{}", &naka_conf.node.rpc_bind);
9354+
9355+
naka_conf.add_initial_balance(
9356+
PrincipalData::from(sender_addr.clone()).to_string(),
9357+
(send_amt + send_fee) * 10,
9358+
);
9359+
naka_conf.add_initial_balance(
9360+
PrincipalData::from(sender_signer_addr.clone()).to_string(),
9361+
100000,
9362+
);
9363+
let recipient = PrincipalData::from(StacksAddress::burn_address(false));
9364+
let stacker_sk = setup_stacker(&mut naka_conf);
9365+
9366+
test_observer::spawn();
9367+
test_observer::register_any(&mut naka_conf);
9368+
9369+
let mut btcd_controller = BitcoinCoreController::from_stx_config(&naka_conf);
9370+
btcd_controller
9371+
.start_bitcoind()
9372+
.expect("Failed starting bitcoind");
9373+
let mut btc_regtest_controller = BitcoinRegtestController::new(naka_conf.clone(), None);
9374+
btc_regtest_controller.bootstrap_chain(201);
9375+
9376+
let mut run_loop = boot_nakamoto::BootRunLoop::new(naka_conf.clone()).unwrap();
9377+
let run_loop_stopper = run_loop.get_termination_switch();
9378+
let Counters {
9379+
blocks_processed,
9380+
naka_submitted_commits: commits_submitted,
9381+
..
9382+
} = run_loop.counters();
9383+
let counters = run_loop.counters();
9384+
let coord_channel = run_loop.coordinator_channels();
9385+
9386+
let run_loop_thread = thread::Builder::new()
9387+
.name("run_loop".into())
9388+
.spawn(move || run_loop.start(None, 0))
9389+
.unwrap();
9390+
wait_for_runloop(&blocks_processed);
9391+
boot_to_epoch_3(
9392+
&naka_conf,
9393+
&blocks_processed,
9394+
&[stacker_sk.clone()],
9395+
&[sender_signer_sk],
9396+
&mut Some(&mut signers),
9397+
&mut btc_regtest_controller,
9398+
);
9399+
9400+
info!("Nakamoto miner started...");
9401+
blind_signer(&naka_conf, &signers, &counters);
9402+
9403+
// Wait one block to confirm the VRF register, wait until a block commit is submitted.
9404+
wait_for_first_naka_block_commit(60, &commits_submitted);
9405+
9406+
// Mine the next burn block so the regular miner starts a new tenure before the follower boots.
9407+
next_block_and_process_new_stacks_block(&mut btc_regtest_controller, 60, &coord_channel)
9408+
.expect("Failed to mine initial tenure start block");
9409+
9410+
let mut follower_conf = naka_conf.clone();
9411+
follower_conf.node.mock_mining = true;
9412+
follower_conf.events_observers.clear();
9413+
follower_conf.node.working_dir = format!("{}-follower", &naka_conf.node.working_dir);
9414+
follower_conf.node.seed = vec![0x01; 32];
9415+
follower_conf.node.local_peer_seed = vec![0x02; 32];
9416+
follower_conf.node.rpc_bind = format!("{localhost}:{node_2_rpc}");
9417+
follower_conf.node.p2p_bind = format!("{localhost}:{node_2_p2p}");
9418+
follower_conf.node.data_url = format!("http://{localhost}:{node_2_rpc}");
9419+
follower_conf.node.p2p_address = format!("{localhost}:{node_2_p2p}");
9420+
9421+
let node_info = get_chain_info(&naka_conf);
9422+
follower_conf.node.add_bootstrap_node(
9423+
&format!(
9424+
"{}@{}",
9425+
&node_info.node_public_key.unwrap(),
9426+
naka_conf.node.p2p_bind
9427+
),
9428+
naka_conf.burnchain.chain_id,
9429+
PEER_VERSION_TESTNET,
9430+
);
9431+
9432+
let mut follower_run_loop = boot_nakamoto::BootRunLoop::new(follower_conf.clone()).unwrap();
9433+
let follower_run_loop_stopper = follower_run_loop.get_termination_switch();
9434+
let follower_coord_channel = follower_run_loop.coordinator_channels();
9435+
9436+
let follower_thread = thread::Builder::new()
9437+
.name("follower-thread".into())
9438+
.spawn(move || follower_run_loop.start(None, 0))
9439+
.unwrap();
9440+
9441+
info!("Booted follower-thread, waiting for the follower to sync to the chain tip");
9442+
wait_for(600, || {
9443+
let Some(miner_node_info) = get_chain_info_opt(&naka_conf) else {
9444+
return Ok(false);
9445+
};
9446+
let Some(follower_node_info) = get_chain_info_opt(&follower_conf) else {
9447+
return Ok(false);
9448+
};
9449+
Ok(
9450+
miner_node_info.stacks_tip_height == follower_node_info.stacks_tip_height
9451+
&& miner_node_info.stacks_tip == follower_node_info.stacks_tip,
9452+
)
9453+
})
9454+
.expect("Timed out waiting for follower to catch up to the miner");
9455+
9456+
// Restart follower during an ongoing tenure so it begins with `last_block_mined = None`
9457+
// while the canonical tip is already inside an active tenure.
9458+
follower_coord_channel
9459+
.lock()
9460+
.expect("Mutex poisoned")
9461+
.stop_chains_coordinator();
9462+
follower_run_loop_stopper.store(false, Ordering::SeqCst);
9463+
follower_thread.join().unwrap();
9464+
9465+
next_block_and_process_new_stacks_block(&mut btc_regtest_controller, 60, &coord_channel)
9466+
.expect("Failed to mine a tenure-start block while follower was offline");
9467+
9468+
// Mine an interim block in the same tenure while follower is still offline.
9469+
let interim_height_before = get_chain_info(&naka_conf).stacks_tip_height;
9470+
let sender_nonce = get_account(&http_origin, &sender_addr).nonce;
9471+
let transfer_tx = make_stacks_transfer_serialized(
9472+
&sender_sk,
9473+
sender_nonce,
9474+
send_fee,
9475+
naka_conf.burnchain.chain_id,
9476+
&recipient,
9477+
send_amt,
9478+
);
9479+
submit_tx(&http_origin, &transfer_tx);
9480+
wait_for(60, || {
9481+
Ok(get_chain_info(&naka_conf).stacks_tip_height > interim_height_before)
9482+
})
9483+
.expect("Failed to mine an interim block while follower was offline");
9484+
let target_miner_height = get_chain_info(&naka_conf).stacks_tip_height;
9485+
9486+
let mut follower_run_loop = boot_nakamoto::BootRunLoop::new(follower_conf.clone()).unwrap();
9487+
let follower_run_loop_stopper = follower_run_loop.get_termination_switch();
9488+
let follower_coord_channel = follower_run_loop.coordinator_channels();
9489+
let Counters {
9490+
naka_mined_blocks: follower_mined_blocks,
9491+
..
9492+
} = follower_run_loop.counters();
9493+
9494+
let follower_thread = thread::Builder::new()
9495+
.name("follower-thread-restarted".into())
9496+
.spawn(move || follower_run_loop.start(None, 0))
9497+
.unwrap();
9498+
9499+
wait_for(180, || {
9500+
let Some(follower_node_info) = get_chain_info_opt(&follower_conf) else {
9501+
return Ok(false);
9502+
};
9503+
Ok(follower_node_info.stacks_tip_height >= target_miner_height)
9504+
})
9505+
.expect("Timed out waiting for restarted follower to sync");
9506+
9507+
if check_empty_sortition_recovery {
9508+
// Precondition for this scenario: the restarted follower has mined at least one block in
9509+
// the ongoing tenure, so we can verify it *continues* across an empty sortition.
9510+
let follower_mined_before_precondition = follower_mined_blocks.load(Ordering::SeqCst);
9511+
TEST_P2P_BROADCAST_STALL.set(true);
9512+
let sender_nonce = get_account(&http_origin, &sender_addr).nonce;
9513+
let transfer_tx = make_stacks_transfer_serialized(
9514+
&sender_sk,
9515+
sender_nonce,
9516+
send_fee,
9517+
naka_conf.burnchain.chain_id,
9518+
&recipient,
9519+
send_amt,
9520+
);
9521+
submit_tx(&http_origin, &transfer_tx);
9522+
wait_for(60, || {
9523+
Ok(follower_mined_blocks.load(Ordering::SeqCst) > follower_mined_before_precondition)
9524+
})
9525+
.expect("Precondition failed: mock miner did not mine before empty sortition");
9526+
let follower_mined_before_empty_sortition = follower_mined_blocks.load(Ordering::SeqCst);
9527+
9528+
// Force an empty sortition and ensure the restarted mock miner keeps mining afterwards.
9529+
counters.naka_skip_commit_op.set(true);
9530+
let miner_burn_height_before = get_chain_info(&naka_conf).burn_block_height;
9531+
let follower_burn_height_before = get_chain_info(&follower_conf).burn_block_height;
9532+
9533+
btc_regtest_controller.build_empty_block();
9534+
wait_for(60, || {
9535+
let miner_info = get_chain_info(&naka_conf);
9536+
let follower_info = get_chain_info(&follower_conf);
9537+
Ok(miner_info.burn_block_height > miner_burn_height_before
9538+
&& follower_info.burn_block_height > follower_burn_height_before)
9539+
})
9540+
.expect("Timed out waiting for empty-sortition block to process");
9541+
9542+
let sortition = get_sortition_info(&naka_conf);
9543+
assert!(
9544+
!sortition.was_sortition,
9545+
"Expected empty sortition while commit ops were skipped"
9546+
);
9547+
9548+
wait_for(60, || {
9549+
Ok(
9550+
follower_mined_blocks.load(Ordering::SeqCst)
9551+
> follower_mined_before_empty_sortition,
9552+
)
9553+
})
9554+
.expect("Mock miner did not continue mining after empty sortition");
9555+
TEST_P2P_BROADCAST_STALL.set(false);
9556+
counters.naka_skip_commit_op.set(false);
9557+
} else {
9558+
// Confirm the restarted follower can start mining in the middle of an ongoing tenure.
9559+
let follower_mined_before_mid_tenure = follower_mined_blocks.load(Ordering::SeqCst);
9560+
TEST_P2P_BROADCAST_STALL.set(true);
9561+
let sender_nonce = get_account(&http_origin, &sender_addr).nonce;
9562+
let transfer_tx = make_stacks_transfer_serialized(
9563+
&sender_sk,
9564+
sender_nonce,
9565+
send_fee,
9566+
naka_conf.burnchain.chain_id,
9567+
&recipient,
9568+
send_amt,
9569+
);
9570+
submit_tx(&http_origin, &transfer_tx);
9571+
wait_for(60, || {
9572+
Ok(follower_mined_blocks.load(Ordering::SeqCst) > follower_mined_before_mid_tenure)
9573+
})
9574+
.expect("Mock miner failed to start mining in the middle of an ongoing tenure");
9575+
TEST_P2P_BROADCAST_STALL.set(false);
9576+
}
9577+
9578+
// Best-effort reset for test globals before teardown.
9579+
TEST_P2P_BROADCAST_STALL.set(false);
9580+
counters.naka_skip_commit_op.set(false);
9581+
9582+
coord_channel
9583+
.lock()
9584+
.expect("Mutex poisoned")
9585+
.stop_chains_coordinator();
9586+
run_loop_stopper.store(false, Ordering::SeqCst);
9587+
9588+
follower_coord_channel
9589+
.lock()
9590+
.expect("Mutex poisoned")
9591+
.stop_chains_coordinator();
9592+
follower_run_loop_stopper.store(false, Ordering::SeqCst);
9593+
9594+
run_loop_thread.join().unwrap();
9595+
follower_thread.join().unwrap();
9596+
}
9597+
9598+
/// Confirm that a mock miner can start mining if it boots mid-tenure.
9599+
#[test]
9600+
#[ignore]
9601+
fn mock_mining_start_mid_tenure() {
9602+
run_mock_mining_ongoing_tenure_boot_test(false);
9603+
}
9604+
9605+
/// Confirm that a mock miner continues mining after a burn block with no sortition.
9606+
#[test]
9607+
#[ignore]
9608+
fn mock_mining_continue_after_empty_sortition() {
9609+
run_mock_mining_ongoing_tenure_boot_test(true);
9610+
}
9611+
93279612
#[test]
93289613
#[ignore]
93299614
/// This test checks for the proper handling of the case where UTXOs are not

0 commit comments

Comments
 (0)