Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: explicit burnchain checks in miner thread #5717

Merged
merged 8 commits into from
Jan 17, 2025
48 changes: 22 additions & 26 deletions testnet/stacks-node/src/nakamoto_node/miner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,9 @@
/// This is the block ID of the first block in the parent tenure
parent_tenure_start: StacksBlockId,
/// This is the snapshot that this miner won, and will produce a tenure for
election_block: BlockSnapshot,
/// This is the snapshot that caused the relayer to initiate this event (may be different
/// than the election block in the case where the miner is trying to mine a late block).
burnchain_tip: BlockSnapshot,
/// This is `true` if the snapshot above is known not to be the the latest burnchain tip,
/// but an ancestor of it (for example, the burnchain tip could be an empty flash block, but the
Expand Down Expand Up @@ -170,7 +173,7 @@
burn_election_block: BlockSnapshot,
/// Current burnchain tip as of the last TenureChange
/// * if the last tenure-change was a BlockFound, then this is the same as the
/// `burn_election_block`.
/// `burn_election_block` (and it is also the `burn_view`)
/// * otherwise, if the last tenure-change is an Extend, then this is the sortition of the burn
/// view consensus hash in the TenureChange
burn_block: BlockSnapshot,
Expand All @@ -185,6 +188,12 @@
signer_set_cache: Option<RewardSet>,
/// The time at which tenure change/extend was attempted
tenure_change_time: Instant,
/// The current tip when this miner thread was started.
/// This *should not* be passed into any block building code, as it
/// is not necessarily the burn view for the block being constructed.
/// Rather, this burn block is used to determine whether or not a new
/// burn block has arrived since this thread started.
burn_tip_at_start: ConsensusHash,
}

impl BlockMinerThread {
Expand All @@ -195,6 +204,7 @@
burn_election_block: BlockSnapshot,
burn_block: BlockSnapshot,
parent_tenure_id: StacksBlockId,
burn_tip_at_start: &ConsensusHash,
reason: MinerReason,
) -> BlockMinerThread {
BlockMinerThread {
Expand All @@ -212,6 +222,7 @@
reason,
p2p_handle: rt.get_p2p_handle(),
signer_set_cache: None,
burn_tip_at_start: burn_tip_at_start.clone(),
tenure_change_time: Instant::now(),
}
}
Expand Down Expand Up @@ -349,7 +360,7 @@
self.burnchain.pox_constants.clone(),
)
.expect("FATAL: could not open sortition DB");
let burn_tip = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn())

Check failure on line 363 in testnet/stacks-node/src/nakamoto_node/miner.rs

View workflow job for this annotation

GitHub Actions / Stacks Core Build Tests / Check the constants from stacks-inspect

unused variable: `burn_tip`
.expect("FATAL: failed to query sortition DB for canonical burn chain tip");

// Start the signer coordinator
Expand All @@ -357,10 +368,11 @@
self.event_dispatcher.stackerdb_channel.clone(),
self.globals.should_keep_running.clone(),
&reward_set,
&burn_tip,
obycode marked this conversation as resolved.
Show resolved Hide resolved
&self.burn_election_block,
&self.burnchain,
miner_privkey,
&self.config,
&self.burn_tip_at_start,
)
.map_err(|e| {
NakamotoNodeError::SigningCoordinatorFailure(format!(
Expand Down Expand Up @@ -433,7 +445,7 @@
let mut burn_db =
SortitionDB::open(&burn_db_path, true, self.burnchain.pox_constants.clone())
.expect("FATAL: could not open sortition DB");
let burn_tip_changed = self.check_burn_tip_changed(&burn_db, &mut chain_state);
let burn_tip_changed = self.check_burn_tip_changed(&burn_db);
match burn_tip_changed
.and_then(|_| self.load_block_parent_info(&mut burn_db, &mut chain_state))
{
Expand Down Expand Up @@ -571,10 +583,7 @@
let wait_start = Instant::now();
while wait_start.elapsed() < self.config.miner.wait_on_interim_blocks {
thread::sleep(Duration::from_millis(ABORT_TRY_AGAIN_MS));
if self
.check_burn_tip_changed(&sort_db, &mut chain_state)
.is_err()
{
if self.check_burn_tip_changed(&sort_db).is_err() {
return Err(NakamotoNodeError::BurnchainTipChanged);
}
}
Expand Down Expand Up @@ -602,13 +611,12 @@
})?;
coordinator.propose_block(
new_block,
&self.burn_block,
&self.burnchain,
sortdb,
&mut chain_state,
stackerdbs,
&self.globals.counters,
&self.burn_election_block.consensus_hash,
&self.burn_election_block,
)
}

Expand Down Expand Up @@ -1116,7 +1124,7 @@
let mut chain_state = neon_node::open_chainstate_with_faults(&self.config)
.expect("FATAL: could not open chainstate DB");

self.check_burn_tip_changed(&burn_db, &mut chain_state)?;
self.check_burn_tip_changed(&burn_db)?;
neon_node::fault_injection_long_tenure();

let mut mem_pool = self
Expand Down Expand Up @@ -1220,7 +1228,7 @@
// last chance -- confirm that the stacks tip is unchanged (since it could have taken long
// enough to build this block that another block could have arrived), and confirm that all
// Stacks blocks with heights higher than the canonical tip are processed.
self.check_burn_tip_changed(&burn_db, &mut chain_state)?;
self.check_burn_tip_changed(&burn_db)?;
Ok(block)
}

Expand Down Expand Up @@ -1359,26 +1367,14 @@
/// Check if the tenure needs to change -- if so, return a BurnchainTipChanged error
/// The tenure should change if there is a new burnchain tip with a valid sortition,
/// or if the stacks chain state's burn view has advanced beyond our burn view.
fn check_burn_tip_changed(
&self,
sortdb: &SortitionDB,
_chain_state: &mut StacksChainState,
) -> Result<(), NakamotoNodeError> {
if let MinerReason::BlockFound { late } = &self.reason {
if *late && self.last_block_mined.is_none() {
// this is a late BlockFound tenure change that ought to be appended to the Stacks
// chain tip, and we haven't submitted it yet.
return Ok(());
}
}

fn check_burn_tip_changed(&self, sortdb: &SortitionDB) -> Result<(), NakamotoNodeError> {
let cur_burn_chain_tip = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn())
.expect("FATAL: failed to query sortition DB for canonical burn chain tip");

if cur_burn_chain_tip.consensus_hash != self.burn_block.consensus_hash {
if cur_burn_chain_tip.consensus_hash != self.burn_tip_at_start {
info!("Miner: Cancel block assembly; burnchain tip has changed";
"new_tip" => %cur_burn_chain_tip.consensus_hash,
"local_tip" => %self.burn_block.consensus_hash);
"local_tip" => %self.burn_tip_at_start);
self.globals.counters.bump_missed_tenures();
Err(NakamotoNodeError::BurnchainTipChanged)
} else {
Expand Down
29 changes: 16 additions & 13 deletions testnet/stacks-node/src/nakamoto_node/relayer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,8 @@ impl RelayerThread {
"winning_sortition" => %sn.consensus_hash);
return Some(MinerDirective::BeginTenure {
parent_tenure_start: committed_index_hash,
burnchain_tip: sn,
burnchain_tip: sn.clone(),
election_block: sn,
late: false,
});
}
Expand Down Expand Up @@ -589,7 +590,8 @@ impl RelayerThread {
parent_tenure_start: StacksBlockId(
last_winning_snapshot.winning_stacks_block_hash.clone().0,
),
burnchain_tip: last_winning_snapshot,
burnchain_tip: sn,
election_block: last_winning_snapshot,
late: true,
});
}
Expand Down Expand Up @@ -975,6 +977,7 @@ impl RelayerThread {
burn_tip: BlockSnapshot,
parent_tenure_id: StacksBlockId,
reason: MinerReason,
burn_tip_at_start: &ConsensusHash,
) -> Result<BlockMinerThread, NakamotoNodeError> {
if fault_injection_skip_mining(&self.config.node.rpc_bind, burn_tip.block_height) {
debug!(
Expand All @@ -991,14 +994,8 @@ impl RelayerThread {

let burn_chain_tip = burn_chain_sn.burn_header_hash;

let allow_late = if let MinerReason::BlockFound { late } = &reason {
*late
} else {
false
};

if burn_chain_tip != burn_header_hash && !allow_late {
debug!(
if &burn_chain_sn.consensus_hash != burn_tip_at_start {
info!(
"Relayer: Drop stale RunTenure for {burn_header_hash}: current sortition is for {burn_chain_tip}"
);
self.globals.counters.bump_missed_tenures();
Expand All @@ -1021,6 +1018,7 @@ impl RelayerThread {
burn_election_block,
burn_tip,
parent_tenure_id,
burn_tip_at_start,
reason,
);
Ok(miner_thread_state)
Expand All @@ -1032,6 +1030,7 @@ impl RelayerThread {
block_election_snapshot: BlockSnapshot,
burn_tip: BlockSnapshot,
reason: MinerReason,
burn_tip_at_start: &ConsensusHash,
) -> Result<(), NakamotoNodeError> {
// when starting a new tenure, block the mining thread if its currently running.
// the new mining thread will join it (so that the new mining thread stalls, not the relayer)
Expand All @@ -1052,6 +1051,7 @@ impl RelayerThread {
burn_tip.clone(),
parent_tenure_start,
reason,
burn_tip_at_start,
)?;

debug!("Relayer: starting new tenure thread");
Expand Down Expand Up @@ -1372,14 +1372,15 @@ impl RelayerThread {
StacksBlockId::new(&canonical_stacks_tip_ch, &canonical_stacks_tip_bh);

let reason = MinerReason::Extended {
burn_view_consensus_hash: new_burn_view,
burn_view_consensus_hash: new_burn_view.clone(),
};

if let Err(e) = self.start_new_tenure(
canonical_stacks_tip.clone(),
canonical_stacks_tip_election_snapshot.clone(),
burn_tip.clone(),
reason.clone(),
&new_burn_view,
) {
error!("Relayer: Failed to start new tenure: {e:?}");
} else {
Expand Down Expand Up @@ -1415,12 +1416,14 @@ impl RelayerThread {
MinerDirective::BeginTenure {
parent_tenure_start,
burnchain_tip,
election_block,
late,
} => match self.start_new_tenure(
parent_tenure_start,
burnchain_tip.clone(),
burnchain_tip.clone(),
election_block.clone(),
election_block.clone(),
MinerReason::BlockFound { late },
&burnchain_tip.consensus_hash,
) {
Ok(()) => {
debug!("Relayer: successfully started new tenure.";
Expand Down
40 changes: 22 additions & 18 deletions testnet/stacks-node/src/nakamoto_node/signer_coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ pub struct SignerCoordinator {
keep_running: Arc<AtomicBool>,
/// Handle for the signer DB listener thread
listener_thread: Option<JoinHandle<()>>,
/// The current tip when this miner thread was started.
/// This *should not* be passed into any block building code, as it
/// is not necessarily the burn view for the block being constructed.
/// Rather, this burn block is used to determine whether or not a new
/// burn block has arrived since this thread started.
burn_tip_at_start: ConsensusHash,
}

impl SignerCoordinator {
Expand All @@ -70,10 +76,11 @@ impl SignerCoordinator {
stackerdb_channel: Arc<Mutex<StackerDBChannel>>,
node_keep_running: Arc<AtomicBool>,
reward_set: &RewardSet,
burn_tip: &BlockSnapshot,
election_block: &BlockSnapshot,
burnchain: &Burnchain,
message_key: StacksPrivateKey,
config: &Config,
burn_tip_at_start: &ConsensusHash,
) -> Result<Self, ChainstateError> {
info!("SignerCoordinator: starting up");
let keep_running = Arc::new(AtomicBool::new(true));
Expand All @@ -84,7 +91,7 @@ impl SignerCoordinator {
node_keep_running.clone(),
keep_running.clone(),
reward_set,
burn_tip,
election_block,
burnchain,
)?;
let is_mainnet = config.is_mainnet();
Expand All @@ -104,11 +111,15 @@ impl SignerCoordinator {
stackerdb_comms: listener.get_comms(),
keep_running,
listener_thread: None,
burn_tip_at_start: burn_tip_at_start.clone(),
};

// Spawn the signer DB listener thread
let listener_thread = std::thread::Builder::new()
.name(format!("stackerdb_listener_{}", burn_tip.block_height))
.name(format!(
"stackerdb_listener_{}",
election_block.block_height
))
.spawn(move || {
if let Err(e) = listener.run() {
error!("StackerDBListener: exited with error: {e:?}");
Expand Down Expand Up @@ -208,24 +219,23 @@ impl SignerCoordinator {
pub fn propose_block(
&mut self,
block: &NakamotoBlock,
burn_tip: &BlockSnapshot,
burnchain: &Burnchain,
sortdb: &SortitionDB,
chain_state: &mut StacksChainState,
stackerdbs: &StackerDBs,
counters: &Counters,
election_sortition: &ConsensusHash,
election_sortition: &BlockSnapshot,
) -> Result<Vec<MessageSignature>, NakamotoNodeError> {
// Add this block to the block status map.
self.stackerdb_comms.insert_block(&block.header);

let reward_cycle_id = burnchain
.block_height_to_reward_cycle(burn_tip.block_height)
.block_height_to_reward_cycle(election_sortition.block_height)
.expect("FATAL: tried to initialize coordinator before first burn block height");

let block_proposal = BlockProposal {
block: block.clone(),
burn_height: burn_tip.block_height,
burn_height: election_sortition.block_height,
reward_cycle: reward_cycle_id,
};

Expand All @@ -236,13 +246,13 @@ impl SignerCoordinator {
Self::send_miners_message::<SignerMessageV0>(
&self.message_key,
sortdb,
burn_tip,
election_sortition,
stackerdbs,
block_proposal_message,
MinerSlotID::BlockProposal,
self.is_mainnet,
&mut self.miners_session,
election_sortition,
&election_sortition.consensus_hash,
)?;
counters.bump_naka_proposed_blocks();

Expand All @@ -267,7 +277,6 @@ impl SignerCoordinator {
&block.block_id(),
chain_state,
sortdb,
burn_tip,
counters,
)
}
Expand All @@ -283,7 +292,6 @@ impl SignerCoordinator {
block_id: &StacksBlockId,
chain_state: &mut StacksChainState,
sortdb: &SortitionDB,
burn_tip: &BlockSnapshot,
counters: &Counters,
) -> Result<Vec<MessageSignature>, NakamotoNodeError> {
loop {
Expand Down Expand Up @@ -324,7 +332,7 @@ impl SignerCoordinator {
return Ok(stored_block.header.signer_signature);
}

if Self::check_burn_tip_changed(sortdb, chain_state, burn_tip) {
if self.check_burn_tip_changed(sortdb) {
debug!("SignCoordinator: Exiting due to new burnchain tip");
return Err(NakamotoNodeError::BurnchainTipChanged);
}
Expand Down Expand Up @@ -366,15 +374,11 @@ impl SignerCoordinator {
}

/// Check if the tenure needs to change
fn check_burn_tip_changed(
sortdb: &SortitionDB,
_chain_state: &mut StacksChainState,
burn_block: &BlockSnapshot,
) -> bool {
fn check_burn_tip_changed(&self, sortdb: &SortitionDB) -> bool {
let cur_burn_chain_tip = SortitionDB::get_canonical_burn_chain_tip(sortdb.conn())
.expect("FATAL: failed to query sortition DB for canonical burn chain tip");

if cur_burn_chain_tip.consensus_hash != burn_block.consensus_hash {
if cur_burn_chain_tip.consensus_hash != self.burn_tip_at_start {
info!("SignCoordinator: Cancel signature aggregation; burnchain tip has changed");
true
} else {
Expand Down
Loading
Loading