Merge branch 'fix/5285' into feat/shadow-block-tooling

Author: jcnelson

.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

@@ -898,23 +898,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| {
@@ -923,29 +914,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.");
@@ -1018,13 +1039,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/epoch_25.rs

@@ -23,7 +23,7 @@ use stacks_common::types::chainstate::StacksPrivateKey;
 
 use crate::config::InitialBalance;
 use crate::tests::bitcoin_regtest::BitcoinCoreController;
-use crate::tests::nakamoto_integrations::wait_for;
+use crate::tests::nakamoto_integrations::{next_block_and, wait_for};
 use crate::tests::neon_integrations::{
     get_account, get_chain_info, neon_integration_test_conf, next_block_and_wait, submit_tx,
     test_observer, wait_for_runloop,
@@ -162,6 +162,9 @@ fn microblocks_disabled() {
     // push us to block 205
     next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
 
+    // Ensure we start off with 0 microblocks
+    assert!(test_observer::get_microblocks().is_empty());
+
     let tx = make_stacks_transfer_mblock_only(
         &spender_1_sk,
         0,
@@ -172,7 +175,11 @@ fn microblocks_disabled() {
     );
     submit_tx(&http_origin, &tx);
 
-    // wait until just before epoch 2.5
+    // Wait for a microblock to be assembled
+    wait_for(60, || Ok(test_observer::get_microblocks().len() == 1))
+        .expect("Failed to wait for microblocks to be assembled");
+
+    // mine Bitcoin blocks up until just before epoch 2.5
     wait_for(120, || {
         let tip_info = get_chain_info(&conf);
         if tip_info.burn_block_height >= epoch_2_5 - 2 {
@@ -183,6 +190,14 @@ fn microblocks_disabled() {
     })
     .expect("Failed to wait until just before epoch 2.5");
 
+    // Verify that the microblock was processed
+    let account = get_account(&http_origin, &spender_1_addr);
+    assert_eq!(
+        u64::try_from(account.balance).unwrap(),
+        spender_1_bal - 1_000
+    );
+    assert_eq!(account.nonce, 1);
+
     let old_tip_info = get_chain_info(&conf);
     next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
     next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
@@ -194,13 +209,8 @@ fn microblocks_disabled() {
     .expect("Failed to process block");
 
     info!("Test passed processing 2.5");
-    let account = get_account(&http_origin, &spender_1_addr);
-    assert_eq!(
-        u64::try_from(account.balance).unwrap(),
-        spender_1_bal - 1_000
-    );
-    assert_eq!(account.nonce, 1);
 
+    // Submit another microblock only transaction
     let tx = make_stacks_transfer_mblock_only(
         &spender_1_sk,
         1,
@@ -211,19 +221,12 @@ fn microblocks_disabled() {
     );
     submit_tx(&http_origin, &tx);
 
-    let mut last_block_height = get_chain_info(&conf).burn_block_height;
-    for _i in 0..5 {
-        next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
-        wait_for(30, || {
-            let tip_info = get_chain_info(&conf);
-            if tip_info.burn_block_height > last_block_height {
-                last_block_height = tip_info.burn_block_height;
-                return Ok(true);
-            }
-            Ok(false)
-        })
-        .expect("Failed to mine");
-    }
+    // Wait for a microblock to be assembled, but expect none to be assembled
+    wait_for(30, || Ok(test_observer::get_microblocks().len() > 1))
+        .expect_err("Microblocks should not have been assembled");
+
+    // Mine a block to see if the microblock gets processed
+    next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
 
     // second transaction should not have been processed!
     let account = get_account(&http_origin, &spender_1_addr);
@@ -233,31 +236,18 @@ fn microblocks_disabled() {
     );
     assert_eq!(account.nonce, 1);
 
-    let microblocks_assembled = test_observer::get_microblocks().len();
-    info!("Microblocks assembled: {microblocks_assembled}",);
-    assert!(
-        microblocks_assembled > 0,
-        "There should be at least 1 microblock assembled"
-    );
-
     let miner_nonce_before_microblock_assembly = get_account(&http_origin, &miner_account).nonce;
 
     // Now, lets tell the miner to try to mine microblocks, but don't try to confirm them!
+    info!("Setting STACKS_TEST_FORCE_MICROBLOCKS_POST_25");
     env::set_var("STACKS_TEST_FORCE_MICROBLOCKS_POST_25", "1");
 
-    let mut last_block_height = get_chain_info(&conf).burn_block_height;
-    for _i in 0..2 {
-        next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
-        wait_for(30, || {
-            let tip_info = get_chain_info(&conf);
-            if tip_info.burn_block_height > last_block_height {
-                last_block_height = tip_info.burn_block_height;
-                return Ok(true);
-            }
-            Ok(false)
-        })
-        .expect("Failed to mine");
-    }
+    // Wait for a second microblock to be assembled
+    wait_for(60, || Ok(test_observer::get_microblocks().len() == 2))
+        .expect("Failed to wait for microblocks to be assembled");
+
+    // Mine a block to see if the microblock gets processed
+    next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
 
     let miner_nonce_after_microblock_assembly = get_account(&http_origin, &miner_account).nonce;
 
@@ -270,44 +260,35 @@ fn microblocks_disabled() {
     );
     assert_eq!(account.nonce, 1);
 
-    // but we should have assembled and announced at least 1 more block to the observer
-    assert!(test_observer::get_microblocks().len() > microblocks_assembled);
     info!(
         "Microblocks assembled: {}",
         test_observer::get_microblocks().len()
     );
 
     // and our miner should have gotten some blocks accepted
-    assert!(
-        miner_nonce_after_microblock_assembly > miner_nonce_before_microblock_assembly,
+    assert_eq!(
+        miner_nonce_after_microblock_assembly, miner_nonce_before_microblock_assembly + 1,
         "Mined before started microblock assembly: {miner_nonce_before_microblock_assembly}, Mined after started microblock assembly: {miner_nonce_after_microblock_assembly}"
     );
 
     // Now, tell the miner to try to confirm microblocks as well.
     //  This should test that the block gets rejected by append block
+    info!("Setting STACKS_TEST_CONFIRM_MICROBLOCKS_POST_25");
     env::set_var("STACKS_TEST_CONFIRM_MICROBLOCKS_POST_25", "1");
 
-    let mut last_block_height = get_chain_info(&conf).burn_block_height;
-    for _i in 0..2 {
-        next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
-        wait_for(30, || {
-            let tip_info = get_chain_info(&conf);
-            if tip_info.burn_block_height > last_block_height {
-                last_block_height = tip_info.burn_block_height;
-                return Ok(true);
-            }
-            Ok(false)
-        })
-        .expect("Failed to mine");
-    }
+    // Wait for a third microblock to be assembled
+    wait_for(60, || Ok(test_observer::get_microblocks().len() == 3))
+        .expect("Failed to wait for microblocks to be assembled");
+
+    // Mine a block to see if the microblock gets processed
+    next_block_and_wait(&mut btc_regtest_controller, &blocks_processed);
 
     let miner_nonce_after_microblock_confirmation = get_account(&http_origin, &miner_account).nonce;
 
-    // and our miner should have gotten at most one more block accepted
-    //  (because they may have had 1 block confirmation in the bitcoin mempool which didn't confirm a microblock
-    //   before we flipped the flag)
-    assert!(
-        miner_nonce_after_microblock_confirmation <= miner_nonce_after_microblock_assembly + 1,
+    // our miner should not have gotten any more blocks accepted
+    assert_eq!(
+        miner_nonce_after_microblock_confirmation,
+        miner_nonce_after_microblock_assembly + 1,
         "Mined after started microblock confimration: {miner_nonce_after_microblock_confirmation}",
     );