Skip to content

Commit 3aa237a

Browse files
Fix race condition: wait for the tip to advance before continuing after wait_for_block_pushed
Signed-off-by: Jacinta Ferrant <236437600+jacinta-stacks@users.noreply.github.com>
1 parent 6149710 commit 3aa237a

File tree

6 files changed

+196
-285
lines changed

6 files changed

+196
-285
lines changed

stacks-node/src/tests/signer/v0/capitulate_parent_tenure_view.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,13 @@ fn deadlock_50_50_split_capitulates_to_node_tip() {
133133
.expect("Timed out waiting for N to be mined and processed");
134134

135135
// Ensure that the block was accepted globally so the stacks tip has advanced to N
136-
let block_n =
137-
wait_for_block_pushed_by_miner_key(30, info_before.stacks_tip_height + 1, &miner_pk)
138-
.expect("Timed out waiting for block N to be mined");
136+
let _block_n =
137+
wait_for_block_pushed_and_tip(30, info_before.stacks_tip_height + 1, &miner_pk, || {
138+
signer_test.get_peer_info().stacks_tip
139+
})
140+
.expect("Timed out waiting for block N to be mined");
139141

140142
let info_after = signer_test.get_peer_info();
141-
assert_eq!(info_after.stacks_tip, block_n.header.block_hash());
142143
assert_eq!(
143144
info_after.stacks_tip_height,
144145
info_before.stacks_tip_height + 1
@@ -400,12 +401,13 @@ fn minority_signers_capitulate_to_supermajority_consensus() {
400401
.expect("Timed out waiting for N to be mined and processed");
401402

402403
// Ensure that the block was accepted globally so the stacks tip has advanced to N
403-
let block_n =
404-
wait_for_block_pushed_by_miner_key(30, info_before.stacks_tip_height + 1, &miner_pk)
405-
.expect("Timed out waiting for block N to be mined");
404+
let _block_n =
405+
wait_for_block_pushed_and_tip(30, info_before.stacks_tip_height + 1, &miner_pk, || {
406+
signer_test.get_peer_info().stacks_tip
407+
})
408+
.expect("Timed out waiting for block N to be mined");
406409

407410
let info_after = signer_test.get_peer_info();
408-
assert_eq!(info_after.stacks_tip, block_n.header.block_hash());
409411
assert_eq!(
410412
info_after.stacks_tip_height,
411413
info_before.stacks_tip_height + 1

stacks-node/src/tests/signer/v0/late_block_proposal.rs

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ use super::SignerTest;
3131
use crate::tests::nakamoto_integrations::wait_for;
3232
use crate::tests::neon_integrations::{get_chain_info, submit_tx, test_observer};
3333
use crate::tests::signer::v0::{
34-
get_stackerdb_signer_messages, wait_for_block_proposal, wait_for_block_pushed_by_miner_key,
34+
get_stackerdb_signer_messages, wait_for_block_proposal, wait_for_block_pushed_and_tip,
3535
};
3636

3737
#[tag(bitcoind)]
@@ -108,15 +108,10 @@ fn signer_rejects_proposal_after_block_pushed() {
108108
wait_for_block_proposal(30, info_before.stacks_tip_height + 1, &miner_pk)
109109
.expect("Timed out waiting for block N+1 to be proposed");
110110
let signer_signature_hash = block_n_proposal.block.header.signer_signature_hash();
111-
let _ = wait_for_block_pushed_by_miner_key(30, info_before.stacks_tip_height + 1, &miner_pk)
112-
.expect("Failed to get BlockPushed for block N");
113-
info!("------------------------- Advance Chain to Include Block N -------------------------");
114-
// Shouldn't have to wait long for the chain to advance
115-
wait_for(10, || {
116-
let info_after = get_chain_info(&signer_test.running_nodes.conf);
117-
Ok(info_after.stacks_tip_height >= info_before.stacks_tip_height + 1)
111+
let _ = wait_for_block_pushed_and_tip(30, info_before.stacks_tip_height + 1, &miner_pk, || {
112+
get_chain_info(&signer_test.running_nodes.conf).stacks_tip
118113
})
119-
.expect("Chain did not advance to block N+1");
114+
.expect("Failed to get BlockPushed for block N");
120115

121116
info!("------------------------- Verify Signer 1 did NOT respond to the Block Proposal -------------------------");
122117
let messages = get_stackerdb_signer_messages();

stacks-node/src/tests/signer/v0/mod.rs

Lines changed: 29 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1357,9 +1357,30 @@ pub fn wait_for_block_pushed_by_miner_key(
13571357
}
13581358
Ok(false)
13591359
})?;
1360+
13601361
block.ok_or_else(|| "Failed to find block pushed".to_string())
13611362
}
13621363

1364+
/// Waits for a block to be pushed by the specified miner, then waits for
1365+
/// the node's tip to advance to that block. This prevents race conditions
1366+
/// where subsequent calls read a stale `stacks_tip_height` because the
1367+
/// coordinator hasn't yet processed the pushed block.
1368+
///
1369+
/// `get_tip` should return the current stacks tip hash (e.g. from
1370+
/// `get_peer_info().stacks_tip` or `get_chain_info().stacks_tip`).
1371+
pub fn wait_for_block_pushed_and_tip(
1372+
timeout_secs: u64,
1373+
expected_height: u64,
1374+
expected_miner: &StacksPublicKey,
1375+
get_tip: impl Fn() -> BlockHeaderHash,
1376+
) -> Result<NakamotoBlock, String> {
1377+
let block = wait_for_block_pushed_by_miner_key(timeout_secs, expected_height, expected_miner)?;
1378+
let block_hash = block.header.block_hash();
1379+
wait_for(timeout_secs, || Ok(get_tip() == block_hash))
1380+
.map_err(|e| format!("Tip did not advance to pushed block: {e}"))?;
1381+
Ok(block)
1382+
}
1383+
13631384
/// Waits for all of the provided signers to send a pre-commit for a block
13641385
/// with the provided signer signature hash
13651386
pub fn wait_for_block_pre_commits_from_signers(
@@ -4058,14 +4079,10 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
40584079
info!("Submitted tx {tx} in to mine block N");
40594080
sender_nonce += 1;
40604081
let block_n =
4061-
wait_for_block_pushed_by_miner_key(30, info_before.stacks_tip_height + 1, &miner_pk)
4062-
.expect("Timed out waiting for block N to be mined");
4063-
4064-
wait_for(30, || {
4065-
let info = signer_test.get_peer_info();
4066-
Ok(info.stacks_tip == block_n.header.block_hash())
4067-
})
4068-
.expect("Tip did not advance to block N");
4082+
wait_for_block_pushed_and_tip(30, info_before.stacks_tip_height + 1, &miner_pk, || {
4083+
signer_test.get_peer_info().stacks_tip
4084+
})
4085+
.expect("Timed out waiting for block N to be mined");
40694086

40704087
info!("------------------------- Attempt to Mine Nakamoto Block N+1 -------------------------");
40714088
// Propose a valid block, but force the miner to ignore the returned signatures and delay the block being
@@ -4212,14 +4229,10 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
42124229
info_before.stacks_tip_height + 2
42134230
);
42144231
let block_n_2 =
4215-
wait_for_block_pushed_by_miner_key(30, info_before.stacks_tip_height + 2, &miner_pk)
4216-
.expect("Timed out waiting for block N+2 to be mined");
4217-
4218-
wait_for(30, || {
4219-
let info = signer_test.get_peer_info();
4220-
Ok(info.stacks_tip == block_n_2.header.block_hash())
4221-
})
4222-
.expect("Tip did not advance to block N+2");
4232+
wait_for_block_pushed_and_tip(30, info_before.stacks_tip_height + 2, &miner_pk, || {
4233+
signer_test.get_peer_info().stacks_tip
4234+
})
4235+
.expect("Timed out waiting for block N+2 to be mined");
42234236
assert_eq!(
42244237
block_n_2.header.parent_block_id,
42254238
block_n_1.header.block_id()

0 commit comments

Comments
 (0)