Merge pull request #5760 from stacks-network/feat/mine-txs-with-tenure-extend

feat: allow other transactions with tenure extends

Author: obycode

.github/workflows/bitcoin-tests.yml

@@ -124,6 +124,7 @@ jobs:
           - tests::signer::v0::signing_in_0th_tenure_of_reward_cycle
           - tests::signer::v0::continue_after_tenure_extend
           - tests::signer::v0::tenure_extend_after_idle_signers
+          - tests::signer::v0::tenure_extend_with_other_transactions
           - tests::signer::v0::tenure_extend_after_idle_miner
           - tests::signer::v0::tenure_extend_after_failed_miner
           - tests::signer::v0::tenure_extend_succeeds_after_rejected_attempt

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to the versioning scheme outlined in the [README.md](README.md).
 
+## [Unreleased]
+
+### Changed
+
+- Miner will include other transactions in blocks with tenure extend transactions (#5760)
+
 ## [3.1.0.0.4]
 
 ### Added

stackslib/src/chainstate/stacks/miner.rs

@@ -2258,7 +2258,13 @@ impl StacksBlockBuilder {
         // nakamoto miner tenure start heuristic:
         //  mine an empty block so you can start your tenure quickly!
         if let Some(tx) = initial_txs.first() {
-            if matches!(&tx.payload, TransactionPayload::TenureChange(_)) {
+            if matches!(
+                &tx.payload,
+                TransactionPayload::TenureChange(TenureChangePayload {
+                    cause: TenureChangeCause::BlockFound,
+                    ..
+                })
+            ) {
                 info!("Nakamoto miner heuristic: during tenure change blocks, produce a fast short block to begin tenure");
                 return Ok((false, tx_events));
             }

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

@@ -247,9 +247,15 @@ pub fn check_nakamoto_empty_block_heuristics() {
             continue;
         }
         let txs = test_observer::parse_transactions(block);
-        let has_tenure_change = txs
-            .iter()
-            .any(|tx| matches!(tx.payload, TransactionPayload::TenureChange(_)));
+        let has_tenure_change = txs.iter().any(|tx| {
+            matches!(
+                tx.payload,
+                TransactionPayload::TenureChange(TenureChangePayload {
+                    cause: TenureChangeCause::BlockFound,
+                    ..
+                })
+            )
+        });
         if has_tenure_change {
             let only_coinbase_and_tenure_change = txs.iter().all(|tx| {
                 matches!(
@@ -7200,7 +7206,9 @@ fn continue_tenure_extend() {
     let mut tenure_block_founds = vec![];
     let mut transfer_tx_included = false;
     let mut last_block_had_extend = false;
-    for block in test_observer::get_blocks() {
+    for pair in test_observer::get_blocks().windows(2) {
+        let prev_block = &pair[0];
+        let block = &pair[1];
         let mut has_extend = false;
         for tx in block["transactions"].as_array().unwrap() {
             let raw_tx = tx["raw_tx"].as_str().unwrap();
@@ -7221,8 +7229,10 @@ fn continue_tenure_extend() {
                         tenure_extends.push(parsed);
                     }
                     TenureChangeCause::BlockFound => {
-                        if last_block_had_extend {
-                            panic!("Expected a Nakamoto block to happen after tenure extend block");
+                        if last_block_had_extend
+                            && prev_block["transactions"].as_array().unwrap().len() <= 1
+                        {
+                            panic!("Expected other transactions to happen after tenure extend");
                         }
                         tenure_block_founds.push(parsed);
                     }

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

@@ -2616,6 +2616,51 @@ fn tenure_extend_after_idle_signers() {
         return;
     }
 
+    tracing_subscriber::registry()
+        .with(fmt::layer())
+        .with(EnvFilter::from_default_env())
+        .init();
+
+    info!("------------------------- Test Setup -------------------------");
+    let num_signers = 5;
+    let idle_timeout = Duration::from_secs(30);
+    let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
+        num_signers,
+        vec![],
+        |config| {
+            config.tenure_idle_timeout = idle_timeout;
+        },
+        |_| {},
+        None,
+        None,
+    );
+
+    signer_test.boot_to_epoch_3();
+
+    info!("---- Nakamoto booted, starting test ----");
+    signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
+
+    info!("---- Waiting for a tenure extend ----");
+
+    // Now, wait for a block with a tenure extend
+    wait_for(idle_timeout.as_secs() + 10, || {
+        Ok(last_block_contains_tenure_change_tx(
+            TenureChangeCause::Extended,
+        ))
+    })
+    .expect("Timed out waiting for a block with a tenure extend");
+
+    signer_test.shutdown();
+}
+
+#[test]
+#[ignore]
+/// This test verifies that a miner will include other transactions with a TenureExtend transaction.
+fn tenure_extend_with_other_transactions() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
     tracing_subscriber::registry()
         .with(fmt::layer())
         .with(EnvFilter::from_default_env())
@@ -2627,7 +2672,7 @@ fn tenure_extend_after_idle_signers() {
     let sender_addr = tests::to_addr(&sender_sk);
     let send_amt = 100;
     let send_fee = 180;
-    let _recipient = PrincipalData::from(StacksAddress::burn_address(false));
+    let recipient = PrincipalData::from(StacksAddress::burn_address(false));
     let idle_timeout = Duration::from_secs(30);
     let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
         num_signers,
@@ -2639,20 +2684,72 @@ fn tenure_extend_after_idle_signers() {
         None,
         None,
     );
-    let _http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
+    let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
 
     signer_test.boot_to_epoch_3();
 
     info!("---- Nakamoto booted, starting test ----");
     signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
 
-    info!("---- Waiting for a tenure extend ----");
+    info!("Pause miner so it doesn't propose a block before the tenure extend");
+    TEST_MINE_STALL.set(true);
+
+    // Submit a transaction to be included with the tenure extend
+    let transfer_tx = make_stacks_transfer(
+        &sender_sk,
+        0,
+        send_fee,
+        signer_test.running_nodes.conf.burnchain.chain_id,
+        &recipient,
+        send_amt,
+    );
+    let _tx = submit_tx(&http_origin, &transfer_tx);
+
+    info!("---- Wait for tenure extend timeout ----");
+
+    sleep_ms(idle_timeout.as_millis() as u64 + 1000);
+
+    info!("---- Resume miner to propose a block with the tenure extend ----");
+    TEST_MINE_STALL.set(false);
 
     // Now, wait for a block with a tenure extend
     wait_for(idle_timeout.as_secs() + 10, || {
-        Ok(last_block_contains_tenure_change_tx(
-            TenureChangeCause::Extended,
-        ))
+        let blocks = test_observer::get_blocks();
+        let last_block = &blocks.last().unwrap();
+        let transactions = last_block["transactions"].as_array().unwrap();
+        let (first_tx, other_txs) = transactions.split_first().unwrap();
+        let raw_tx = first_tx["raw_tx"].as_str().unwrap();
+        let tx_bytes = hex_bytes(&raw_tx[2..]).unwrap();
+        let parsed = StacksTransaction::consensus_deserialize(&mut &tx_bytes[..]).unwrap();
+        let found_tenure_extend = match &parsed.payload {
+            TransactionPayload::TenureChange(payload)
+                if payload.cause == TenureChangeCause::Extended =>
+            {
+                info!("Found tenure extend transaction: {parsed:?}");
+                true
+            }
+            _ => false,
+        };
+        if found_tenure_extend {
+            let found_transfer = other_txs.iter().any(|tx| {
+                let raw_tx = tx["raw_tx"].as_str().unwrap();
+                let tx_bytes = hex_bytes(&raw_tx[2..]).unwrap();
+                let parsed = StacksTransaction::consensus_deserialize(&mut &tx_bytes[..]).unwrap();
+                match &parsed.payload {
+                    TransactionPayload::TokenTransfer(..) => true,
+                    _ => false,
+                }
+            });
+            if found_transfer {
+                info!("Found transfer transaction");
+                Ok(true)
+            } else {
+                Err("No transfer transaction found together with the tenure extend".to_string())
+            }
+        } else {
+            info!("No tenure change transaction found");
+            Ok(false)
+        }
     })
     .expect("Timed out waiting for a block with a tenure extend");
 
@@ -6438,19 +6535,22 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
 /// Mine 2 empty burn blocks (simulate fast blocks scenario)
 /// Miner 2 proposes block N+1 with a TenureChangePayload
 /// Signers accept and the stacks tip advances to N+1
-/// Miner 2 proposes block N+2 with a TokenTransfer
+/// Miner 2 proposes block N+2 with a TenureExtend
 /// Signers accept and the stacks tip advances to N+2
+/// Miner 2 proposes block N+3 with a TokenTransfer
+/// Signers accept and the stacks tip advances to N+3
 /// Mine an empty burn block
-/// Miner 2 proposes block N+3 with a TenureExtend
-/// Signers accept and the chain advances to N+3
-/// Miner 1 wins the next tenure and proposes a block N+4 with a TenureChangePayload
+/// Miner 2 proposes block N+4 with a TenureExtend
 /// Signers accept and the chain advances to N+4
+/// Miner 1 wins the next tenure and proposes a block N+5 with a TenureChangePayload
+/// Signers accept and the chain advances to N+5
 /// Asserts:
 /// - Block N+1 contains the TenureChangePayload
-/// - Block N+2 contains the TokenTransfer
-/// - Block N+3 contains the TenureExtend
-/// - Block N+4 contains the TenureChangePayload
-/// - The stacks tip advances to N+4
+/// - Block N+2 contains the TenureExtend
+/// - Block N+3 contains the TokenTransfer
+/// - Block N+4 contains the TenureExtend
+/// - Block N+5 contains the TenureChangePayload
+/// - The stacks tip advances to N+5
 #[test]
 #[ignore]
 fn continue_after_fast_block_no_sortition() {
@@ -6811,7 +6911,7 @@ fn continue_after_fast_block_no_sortition() {
     // Allow signers to respond to proposals again
     TEST_REJECT_ALL_BLOCK_PROPOSAL.set(Vec::new());
 
-    info!("------------------------- Wait for Miner B's Block N -------------------------";
+    info!("------------------------- Wait for Miner B's Block N+1 -------------------------";
         "blocks_processed_before_2" => %blocks_processed_before_2,
         "stacks_height_before" => %stacks_height_before,
         "nmb_old_blocks" => %nmb_old_blocks);
@@ -6826,7 +6926,7 @@ fn continue_after_fast_block_no_sortition() {
 
         let blocks_mined1_val = blocks_mined1.load(Ordering::SeqCst);
         let blocks_mined2_val = blocks_mined2.load(Ordering::SeqCst);
-        info!("Waiting for Miner B's Block N";
+        info!("Waiting for Miner B's Block N+1";
             "blocks_mined1_val" => %blocks_mined1_val,
             "blocks_mined2_val" => %blocks_mined2_val,
             "stacks_height" => %stacks_height,
@@ -6841,11 +6941,11 @@ fn continue_after_fast_block_no_sortition() {
     .expect("Timed out waiting for block to be mined and processed");
 
     info!(
-        "------------------------- Verify Tenure Change Tx in Miner B's Block N -------------------------"
+        "------------------------- Verify Tenure Change Tx in Miner B's Block N+1 -------------------------"
     );
     verify_last_block_contains_tenure_change_tx(TenureChangeCause::BlockFound);
 
-    info!("------------------------- Wait for Miner B's Block N+1 -------------------------");
+    info!("------------------------- Wait for Miner B's Block N+2 -------------------------");
 
     let nmb_old_blocks = test_observer::get_blocks().len();
     let blocks_processed_before_2 = blocks_mined2.load(Ordering::SeqCst);
@@ -6855,18 +6955,7 @@ fn continue_after_fast_block_no_sortition() {
         .expect("Failed to get peer info")
         .stacks_tip_height;
 
-    // submit a tx so that the miner will mine an extra block
-    let transfer_tx = make_stacks_transfer(
-        &sender_sk,
-        sender_nonce,
-        send_fee,
-        signer_test.running_nodes.conf.burnchain.chain_id,
-        &recipient,
-        send_amt,
-    );
-    submit_tx(&http_origin, &transfer_tx);
-
-    // wait for the tenure-extend block to be processed
+    // wait for the transfer block to be processed
     wait_for(30, || {
         let stacks_height = signer_test
             .stacks_client
@@ -6881,8 +6970,12 @@ fn continue_after_fast_block_no_sortition() {
     })
     .expect("Timed out waiting for block to be mined and processed");
 
+    info!("------------------------- Verify Miner B's Block N+2 -------------------------");
+
     verify_last_block_contains_tenure_change_tx(TenureChangeCause::Extended);
 
+    info!("------------------------- Wait for Miner B's Block N+3 -------------------------");
+
     let nmb_old_blocks = test_observer::get_blocks().len();
     let blocks_processed_before_2 = blocks_mined2.load(Ordering::SeqCst);
     let stacks_height_before = signer_test
@@ -6891,22 +6984,24 @@ fn continue_after_fast_block_no_sortition() {
         .expect("Failed to get peer info")
         .stacks_tip_height;
 
-    // wait for the new block with the STX transfer to be processed
+    // submit a tx so that the miner will mine an extra block
+    let transfer_tx = make_stacks_transfer(
+        &sender_sk,
+        sender_nonce,
+        send_fee,
+        signer_test.running_nodes.conf.burnchain.chain_id,
+        &recipient,
+        send_amt,
+    );
+    submit_tx(&http_origin, &transfer_tx);
+
+    // wait for the transfer block to be processed
     wait_for(30, || {
         let stacks_height = signer_test
             .stacks_client
             .get_peer_info()
             .expect("Failed to get peer info")
             .stacks_tip_height;
-
-        let blocks_mined1_val = blocks_mined1.load(Ordering::SeqCst);
-        let blocks_mined2_val = blocks_mined2.load(Ordering::SeqCst);
-        info!("Waiting for Miner B's Block N";
-            "blocks_mined1_val" => %blocks_mined1_val,
-            "blocks_mined2_val" => %blocks_mined2_val,
-            "stacks_height" => %stacks_height,
-            "observed_blocks" => %test_observer::get_blocks().len());
-
         Ok(
             blocks_mined2.load(Ordering::SeqCst) > blocks_processed_before_2
                 && stacks_height > stacks_height_before
@@ -6915,7 +7010,7 @@ fn continue_after_fast_block_no_sortition() {
     })
     .expect("Timed out waiting for block to be mined and processed");
 
-    info!("------------------------- Verify Miner B's Block N+1 -------------------------");
+    info!("------------------------- Verify Miner B's Block N+3 -------------------------");
 
     verify_last_block_contains_transfer_tx();
 
@@ -6932,7 +7027,7 @@ fn continue_after_fast_block_no_sortition() {
     .unwrap();
     btc_blocks_mined += 1;
 
-    info!("------------------------- Verify Miner B's Issues a Tenure Change Extend in Block N+2 -------------------------");
+    info!("------------------------- Verify Miner B's Issues a Tenure Change Extend in Block N+4 -------------------------");
     verify_last_block_contains_tenure_change_tx(TenureChangeCause::Extended);
 
     info!("------------------------- Unpause Miner A's Block Commits -------------------------");
@@ -6967,7 +7062,7 @@ fn continue_after_fast_block_no_sortition() {
     assert!(tip.sortition);
     assert_eq!(tip.miner_pk_hash.unwrap(), mining_pkh_1);
 
-    info!("------------------------- Verify Miner A's Issued a Tenure Change in Block N+4 -------------------------");
+    info!("------------------------- Verify Miner A's Issued a Tenure Change in Block N+5 -------------------------");
     verify_last_block_contains_tenure_change_tx(TenureChangeCause::BlockFound);
 
     info!(