Merge pull request #5411 from stacks-network/fix/fast-block-test

Miner can continue mining after an empty tenure followed by empty sortition

Author: jferrant

.github/workflows/bitcoin-tests.yml

@@ -124,6 +124,7 @@ jobs:
           - tests::signer::v0::continue_after_tenure_extend
           - tests::signer::v0::multiple_miners_with_custom_chain_id
           - tests::signer::v0::block_commit_delay
+          - tests::signer::v0::continue_after_fast_block_no_sortition
           - tests::nakamoto_integrations::burn_ops_integration_test
           - tests::nakamoto_integrations::check_block_heights
           - tests::nakamoto_integrations::clarity_burn_state

testnet/stacks-node/src/nakamoto_node/miner.rs

@@ -109,6 +109,8 @@ pub enum MinerReason {
         /// sortition.
         burn_view_consensus_hash: ConsensusHash,
     },
+    /// The miner thread was spawned to initialize a prior empty tenure
+    EmptyTenure,
 }
 
 impl std::fmt::Display for MinerReason {
@@ -121,6 +123,7 @@ impl std::fmt::Display for MinerReason {
                 f,
                 "Extended: burn_view_consensus_hash = {burn_view_consensus_hash:?}",
             ),
+            MinerReason::EmptyTenure => write!(f, "EmptyTenure"),
         }
     }
 }
@@ -921,19 +924,19 @@ impl BlockMinerThread {
         let vrf_proof = if self.config.get_node_config(false).mock_mining {
             self.keychain.generate_proof(
                 VRF_MOCK_MINER_KEY,
-                self.burn_block.sortition_hash.as_bytes(),
+                self.burn_election_block.sortition_hash.as_bytes(),
             )
         } else {
             self.keychain.generate_proof(
                 self.registered_key.target_block_height,
-                self.burn_block.sortition_hash.as_bytes(),
+                self.burn_election_block.sortition_hash.as_bytes(),
             )
         };
 
         debug!(
             "Generated VRF Proof: {} over {} ({},{}) with key {}",
             vrf_proof.to_hex(),
-            &self.burn_block.sortition_hash,
+            &self.burn_election_block.sortition_hash,
             &self.burn_block.block_height,
             &self.burn_block.burn_header_hash,
             &self.registered_key.vrf_public_key.to_hex()
@@ -1154,7 +1157,7 @@ impl BlockMinerThread {
         };
 
         let (tenure_change_tx, coinbase_tx) = match &self.reason {
-            MinerReason::BlockFound => {
+            MinerReason::BlockFound | MinerReason::EmptyTenure => {
                 let tenure_change_tx =
                     self.generate_tenure_change_tx(current_miner_nonce, payload)?;
                 let coinbase_tx =

testnet/stacks-node/src/nakamoto_node/relayer.rs

@@ -874,23 +874,14 @@ impl RelayerThread {
             SortitionDB::get_canonical_stacks_chain_tip_hash(self.sortdb.conn()).unwrap();
         let canonical_stacks_tip =
             StacksBlockId::new(&canonical_stacks_tip_ch, &canonical_stacks_tip_bh);
-        let block_election_snapshot =
-            SortitionDB::get_block_snapshot_consensus(self.sortdb.conn(), &canonical_stacks_tip_ch)
-                .map_err(|e| {
-                    error!("Relayer: failed to get block snapshot for canonical tip: {e:?}");
-                    NakamotoNodeError::SnapshotNotFoundForChainTip
-                })?
-                .ok_or_else(|| {
-                    error!("Relayer: failed to get block snapshot for canonical tip");
-                    NakamotoNodeError::SnapshotNotFoundForChainTip
-                })?;
 
         let Some(ref mining_key) = self.config.miner.mining_key else {
             return Ok(());
         };
         let mining_pkh = Hash160::from_node_public_key(&StacksPublicKey::from_private(mining_key));
 
-        let last_winner_snapshot = {
+        // If we won the last sortition, then we should start a new tenure off of it.
+        let last_block_election_snapshot = {
             let ih = self.sortdb.index_handle(&burn_tip.sortition_id);
             ih.get_last_snapshot_with_sortition(burn_tip.block_height)
                 .map_err(|e| {
@@ -899,29 +890,59 @@ impl RelayerThread {
                 })?
         };
 
-        let won_last_sortition = last_winner_snapshot.miner_pk_hash == Some(mining_pkh);
+        let won_last_sortition = last_block_election_snapshot.miner_pk_hash == Some(mining_pkh);
         debug!(
             "Relayer: Current burn block had no sortition. Checking for tenure continuation.";
             "won_last_sortition" => won_last_sortition,
             "current_mining_pkh" => %mining_pkh,
-            "last_winner_snapshot.miner_pk_hash" => ?last_winner_snapshot.miner_pk_hash,
+            "last_block_election_snapshot.consensus_hash" => %last_block_election_snapshot.consensus_hash,
+            "last_block_election_snapshot.miner_pk_hash" => ?last_block_election_snapshot.miner_pk_hash,
             "canonical_stacks_tip_id" => %canonical_stacks_tip,
             "canonical_stacks_tip_ch" => %canonical_stacks_tip_ch,
-            "block_election_ch" => %block_election_snapshot.consensus_hash,
             "burn_view_ch" => %new_burn_view,
         );
 
         if !won_last_sortition {
             return Ok(());
         }
 
+        let canonical_block_snapshot =
+            SortitionDB::get_block_snapshot_consensus(self.sortdb.conn(), &canonical_stacks_tip_ch)
+                .map_err(|e| {
+                    error!("Relayer: failed to get block snapshot for canonical tip: {e:?}");
+                    NakamotoNodeError::SnapshotNotFoundForChainTip
+                })?
+                .ok_or_else(|| {
+                    error!("Relayer: failed to get block snapshot for canonical tip");
+                    NakamotoNodeError::SnapshotNotFoundForChainTip
+                })?;
+
+        let won_canonical_block_snapshot =
+            canonical_block_snapshot.miner_pk_hash == Some(mining_pkh);
+
+        let (parent_tenure_start, block_election_snapshot, reason) =
+            if !won_canonical_block_snapshot {
+                debug!("Relayer: Failed to issue a tenure change payload in our last tenure. Issue a new tenure change payload.");
+                (
+                    StacksBlockId(last_block_election_snapshot.winning_stacks_block_hash.0),
+                    last_block_election_snapshot,
+                    MinerReason::EmptyTenure,
+                )
+            } else {
+                debug!("Relayer: Successfully issued a tenure change payload in its tenure. Issue a continue extend from the chain tip.");
+                (
+                    canonical_stacks_tip, //For tenure extend, we should be extending off the canonical tip
+                    canonical_block_snapshot,
+                    MinerReason::Extended {
+                        burn_view_consensus_hash: new_burn_view,
+                    },
+                )
+            };
         match self.start_new_tenure(
-            canonical_stacks_tip, // For tenure extend, we should be extending off the canonical tip
+            parent_tenure_start,
             block_election_snapshot,
             burn_tip,
-            MinerReason::Extended {
-                burn_view_consensus_hash: new_burn_view,
-            },
+            reason,
         ) {
             Ok(()) => {
                 debug!("Relayer: successfully started new tenure.");
@@ -994,13 +1015,7 @@ impl RelayerThread {
 
     #[cfg(test)]
     fn fault_injection_skip_block_commit(&self) -> bool {
-        self.globals
-            .counters
-            .naka_skip_commit_op
-            .0
-            .lock()
-            .unwrap()
-            .unwrap_or(false)
+        self.globals.counters.naka_skip_commit_op.get()
     }
 
     #[cfg(not(test))]

testnet/stacks-node/src/run_loop/neon.rs

@@ -93,6 +93,19 @@ impl Default for TestFlag {
     }
 }
 
+#[cfg(test)]
+impl TestFlag {
+    /// Set the test flag to the given value
+    pub fn set(&self, value: bool) {
+        *self.0.lock().unwrap() = Some(value);
+    }
+
+    /// Get the test flag value. Defaults to false if the flag is not set.
+    pub fn get(&self) -> bool {
+        self.0.lock().unwrap().unwrap_or(false)
+    }
+}
+
 #[derive(Clone, Default)]
 pub struct Counters {
     pub blocks_processed: RunLoopCounter,
@@ -1022,7 +1035,7 @@ impl RunLoop {
     /// This function will block by looping infinitely.
     /// It will start the burnchain (separate thread), set-up a channel in
     /// charge of coordinating the new blocks coming from the burnchain and
-    /// the nodes, taking turns on tenures.  
+    /// the nodes, taking turns on tenures.
     ///
     /// Returns `Option<NeonGlobals>` so that data can be passed to `NakamotoNode`
     pub fn start(

testnet/stacks-node/src/tests/nakamoto_integrations.rs

@@ -4984,7 +4984,7 @@ fn forked_tenure_is_ignored() {
 
     // Unpause the broadcast of Tenure B's block, do not submit commits, and do not allow blocks to
     // be processed
-    test_skip_commit_op.0.lock().unwrap().replace(true);
+    test_skip_commit_op.set(true);
     TEST_BROADCAST_STALL.lock().unwrap().replace(false);
 
     // Wait for a stacks block to be broadcasted.
@@ -5037,7 +5037,7 @@ fn forked_tenure_is_ignored() {
         .expect("Mutex poisoned")
         .get_stacks_blocks_processed();
     next_block_and(&mut btc_regtest_controller, 60, || {
-        test_skip_commit_op.0.lock().unwrap().replace(false);
+        test_skip_commit_op.set(false);
         TEST_BLOCK_ANNOUNCE_STALL.lock().unwrap().replace(false);
         let commits_count = commits_submitted.load(Ordering::SeqCst);
         let blocks_count = mined_blocks.load(Ordering::SeqCst);
@@ -6972,7 +6972,7 @@ fn continue_tenure_extend() {
         .get_stacks_blocks_processed();
 
     info!("Pausing commit ops to trigger a tenure extend.");
-    test_skip_commit_op.0.lock().unwrap().replace(true);
+    test_skip_commit_op.set(true);
 
     next_block_and(&mut btc_regtest_controller, 60, || Ok(true)).unwrap();
 
@@ -7071,7 +7071,7 @@ fn continue_tenure_extend() {
     }
 
     info!("Resuming commit ops to mine regular tenures.");
-    test_skip_commit_op.0.lock().unwrap().replace(false);
+    test_skip_commit_op.set(false);
 
     // Mine 15 more regular nakamoto tenures
     for _i in 0..15 {