fix: explicit burnchain checks in miner thread

kantai · kantai · commit 0abb85a754c5 · 2025-01-16T14:32:36.000-06:00
diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs
@@ -81,6 +81,9 @@ pub enum MinerDirective {
         /// 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
@@ -170,7 +173,7 @@ pub struct BlockMinerThread {
     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,
@@ -185,6 +188,12 @@ pub struct BlockMinerThread {
     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 {
@@ -195,6 +204,7 @@ impl BlockMinerThread {
         burn_election_block: BlockSnapshot,
         burn_block: BlockSnapshot,
         parent_tenure_id: StacksBlockId,
+        burn_tip_at_start: &ConsensusHash,
         reason: MinerReason,
     ) -> BlockMinerThread {
         BlockMinerThread {
@@ -212,6 +222,7 @@ impl BlockMinerThread {
             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(),
         }
     }
@@ -357,10 +368,11 @@ impl BlockMinerThread {
             self.event_dispatcher.stackerdb_channel.clone(),
             self.globals.should_keep_running.clone(),
             &reward_set,
-            &burn_tip,
+            &self.burn_election_block,
             &self.burnchain,
             miner_privkey,
             &self.config,
+            &self.burn_tip_at_start,
         )
         .map_err(|e| {
             NakamotoNodeError::SigningCoordinatorFailure(format!(
@@ -433,7 +445,7 @@ impl BlockMinerThread {
                 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))
                 {
@@ -571,10 +583,7 @@ impl BlockMinerThread {
         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);
             }
         }
@@ -602,13 +611,12 @@ impl BlockMinerThread {
             })?;
         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,
         )
     }
 
@@ -1116,7 +1124,7 @@ impl BlockMinerThread {
         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
@@ -1220,7 +1228,7 @@ impl BlockMinerThread {
         // 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)
     }
 
@@ -1359,26 +1367,14 @@ impl BlockMinerThread {
     /// 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 {
diff --git a/testnet/stacks-node/src/nakamoto_node/relayer.rs b/testnet/stacks-node/src/nakamoto_node/relayer.rs
@@ -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,
                 });
             }
@@ -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,
                 });
             }
@@ -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!(
@@ -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();
@@ -1021,6 +1018,7 @@ impl RelayerThread {
             burn_election_block,
             burn_tip,
             parent_tenure_id,
+            burn_tip_at_start,
             reason,
         );
         Ok(miner_thread_state)
@@ -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)
@@ -1052,6 +1051,7 @@ impl RelayerThread {
             burn_tip.clone(),
             parent_tenure_start,
             reason,
+            burn_tip_at_start,
         )?;
 
         debug!("Relayer: starting new tenure thread");
@@ -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 {
@@ -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.";
diff --git a/testnet/stacks-node/src/nakamoto_node/signer_coordinator.rs b/testnet/stacks-node/src/nakamoto_node/signer_coordinator.rs
@@ -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 {
@@ -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));
@@ -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();
@@ -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:?}");
@@ -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,
         };
 
@@ -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();
 
@@ -267,7 +277,6 @@ impl SignerCoordinator {
             &block.block_id(),
             chain_state,
             sortdb,
-            burn_tip,
             counters,
         )
     }
@@ -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 {
@@ -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);
                     }
@@ -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 {
diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs
