Merge branch 'fix/tenure-extend-burnchain-tip-check' of https://github.com/stacks-network/stacks-core into fix/tenure-extend-burnchain-tip-check

Author: jferrant

.github/workflows/bitcoin-tests.yml

@@ -118,6 +118,7 @@ jobs:
           - tests::signer::v0::mine_2_nakamoto_reward_cycles
           - tests::signer::v0::signer_set_rollover
           - tests::signer::v0::signing_in_0th_tenure_of_reward_cycle
+          - tests::signer::v0::continue_after_tenure_extend
           - 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

@@ -383,7 +383,7 @@ impl BlockMinerThread {
                                 "block_height" => new_block.header.chain_length,
                                 "consensus_hash" => %new_block.header.consensus_hash,
                             );
-                            return Err(e);
+                            continue;
                         }
                         _ => {
                             error!("Error while gathering signatures: {e:?}. Will try mining again.";
@@ -1247,7 +1247,9 @@ impl ParentStacksBlockInfo {
         let burn_chain_tip = SortitionDB::get_canonical_burn_chain_tip(burn_db.conn())
             .expect("FATAL: failed to query sortition DB for canonical burn chain tip");
 
-        if burn_chain_tip.consensus_hash != check_burn_block.consensus_hash {
+        if burn_chain_tip.consensus_hash != check_burn_block.consensus_hash
+            && burn_chain_tip.sortition_id != check_burn_block.sortition_id
+        {
             info!(
                 "New canonical burn chain tip detected. Will not try to mine.";
                 "new_consensus_hash" => %burn_chain_tip.consensus_hash,

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

@@ -251,11 +251,13 @@ impl SignCoordinator {
     }
 
     /// Check if the tenure needs to change
-    fn check_burn_tip_changed(sortdb: &SortitionDB, consensus_hash: &ConsensusHash) -> bool {
+    fn check_burn_tip_changed(sortdb: &SortitionDB, burn_block: &BlockSnapshot) -> 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 != *consensus_hash {
+        if cur_burn_chain_tip.consensus_hash != burn_block.consensus_hash
+            && cur_burn_chain_tip.sortition_id != burn_block.sortition_id
+        {
             info!("SignCoordinator: Cancel signature aggregation; burnchain tip has changed");
             true
         } else {
@@ -365,7 +367,7 @@ impl SignCoordinator {
                 return Ok(stored_block.header.signer_signature);
             }
 
-            if Self::check_burn_tip_changed(&sortdb, &burn_tip.consensus_hash) {
+            if Self::check_burn_tip_changed(&sortdb, &burn_tip) {
                 debug!("SignCoordinator: Exiting due to new burnchain tip");
                 return Err(NakamotoNodeError::BurnchainTipChanged);
             }

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

@@ -6920,10 +6920,10 @@ fn continue_tenure_extend() {
     // setup sender + recipient for a test stx transfer
     let sender_addr = tests::to_addr(&sender_sk);
     let send_amt = 1000;
-    let send_fee = 100;
+    let send_fee = 200;
     naka_conf.add_initial_balance(
         PrincipalData::from(sender_addr.clone()).to_string(),
-        send_amt * 2 + send_fee,
+        (send_amt + send_fee) * 20,
     );
     let sender_signer_sk = Secp256k1PrivateKey::new();
     let sender_signer_addr = tests::to_addr(&sender_signer_sk);
@@ -6933,6 +6933,7 @@ fn continue_tenure_extend() {
     );
     let recipient = PrincipalData::from(StacksAddress::burn_address(false));
     let stacker_sk = setup_stacker(&mut naka_conf);
+    let mut transfer_nonce = 0;
 
     test_observer::spawn();
     test_observer::register_any(&mut naka_conf);
@@ -7053,7 +7054,7 @@ fn continue_tenure_extend() {
     // Submit a TX
     let transfer_tx = make_stacks_transfer(
         &sender_sk,
-        0,
+        transfer_nonce,
         send_fee,
         naka_conf.burnchain.chain_id,
         &recipient,
@@ -7115,6 +7116,26 @@ fn continue_tenure_extend() {
     })
     .unwrap();
 
+    // Mine 3 nakamoto tenures
+    for i in 0..3 {
+        info!("Triggering Nakamoto blocks after extend ({})", i + 1);
+        transfer_nonce += 1;
+        let transfer_tx = make_stacks_transfer(
+            &sender_sk,
+            transfer_nonce,
+            send_fee,
+            naka_conf.burnchain.chain_id,
+            &recipient,
+            send_amt,
+        );
+        submit_tx(&http_origin, &transfer_tx);
+        wait_for(10, || {
+            let sender_nonce = get_account(&http_origin, &to_addr(&sender_sk)).nonce;
+            Ok(sender_nonce >= transfer_nonce)
+        })
+        .expect("Timed out waiting for transfer TX to confirm");
+    }
+
     info!("Resuming commit ops to mine regular tenures.");
     test_skip_commit_op.0.lock().unwrap().replace(false);
 
@@ -7152,7 +7173,9 @@ fn continue_tenure_extend() {
     let mut tenure_extends = vec![];
     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() {
+        let mut has_extend = false;
         for tx in block["transactions"].as_array().unwrap() {
             let raw_tx = tx["raw_tx"].as_str().unwrap();
             if raw_tx == &transfer_tx_hex {
@@ -7166,12 +7189,21 @@ fn continue_tenure_extend() {
             let parsed = StacksTransaction::consensus_deserialize(&mut &tx_bytes[..]).unwrap();
             match &parsed.payload {
                 TransactionPayload::TenureChange(payload) => match payload.cause {
-                    TenureChangeCause::Extended => tenure_extends.push(parsed),
-                    TenureChangeCause::BlockFound => tenure_block_founds.push(parsed),
+                    TenureChangeCause::Extended => {
+                        has_extend = true;
+                        tenure_extends.push(parsed);
+                    }
+                    TenureChangeCause::BlockFound => {
+                        if last_block_had_extend {
+                            panic!("Expected a Nakamoto block to happen after tenure extend block");
+                        }
+                        tenure_block_founds.push(parsed);
+                    }
                 },
                 _ => {}
             };
         }
+        last_block_had_extend = has_extend;
     }
     assert!(
         !tenure_extends.is_empty(),

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

@@ -34,6 +34,7 @@ use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader, NakamotoC
 use stacks::chainstate::stacks::address::PoxAddress;
 use stacks::chainstate::stacks::boot::MINERS_NAME;
 use stacks::chainstate::stacks::db::{StacksBlockHeaderTypes, StacksChainState, StacksHeaderInfo};
+use stacks::chainstate::stacks::{StacksTransaction, TenureChangeCause, TransactionPayload};
 use stacks::codec::StacksMessageCodec;
 use stacks::core::{StacksEpochId, CHAIN_ID_TESTNET};
 use stacks::libstackerdb::StackerDBChunkData;
@@ -42,7 +43,7 @@ use stacks::net::api::postblock_proposal::{ValidateRejectCode, TEST_VALIDATE_STA
 use stacks::net::relay::fault_injection::set_ignore_block;
 use stacks::types::chainstate::{StacksAddress, StacksBlockId, StacksPrivateKey, StacksPublicKey};
 use stacks::types::PublicKey;
-use stacks::util::hash::MerkleHashFunc;
+use stacks::util::hash::{hex_bytes, MerkleHashFunc};
 use stacks::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey};
 use stacks::util_lib::boot::boot_code_id;
 use stacks::util_lib::signed_structured_data::pox4::{
@@ -5076,6 +5077,129 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
     assert_ne!(block_n_2, block_n);
 }
 
+#[test]
+#[ignore]
+/// Test that we can mine a tenure extend and then continue mining afterwards.
+fn continue_after_tenure_extend() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
+    tracing_subscriber::registry()
+        .with(fmt::layer())
+        .with(EnvFilter::from_default_env())
+        .init();
+
+    info!("------------------------- Test Setup -------------------------");
+    let num_signers = 5;
+    let sender_sk = Secp256k1PrivateKey::new();
+    let sender_addr = tests::to_addr(&sender_sk);
+    let recipient = PrincipalData::from(StacksAddress::burn_address(false));
+    let send_amt = 100;
+    let send_fee = 180;
+    let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new(
+        num_signers,
+        vec![(sender_addr.clone(), (send_amt + send_fee) * 5)],
+    );
+    let timeout = Duration::from_secs(200);
+    let coord_channel = signer_test.running_nodes.coord_channel.clone();
+    let http_origin = format!("http://{}", &signer_test.running_nodes.conf.node.rpc_bind);
+
+    signer_test.boot_to_epoch_3();
+
+    info!("------------------------- Mine Normal Tenure -------------------------");
+    signer_test.mine_and_verify_confirmed_naka_block(timeout, num_signers);
+
+    info!("------------------------- Extend Tenure -------------------------");
+    signer_test
+        .running_nodes
+        .nakamoto_test_skip_commit_op
+        .0
+        .lock()
+        .unwrap()
+        .replace(true);
+
+    // It's possible that we have a pending block commit already.
+    // Mine two BTC blocks to "flush" this commit.
+
+    for i in 0..2 {
+        info!(
+            "------------- After pausing commits, triggering 2 BTC blocks: ({} of 2) -----------",
+            i + 1
+        );
+
+        let blocks_processed_before = coord_channel
+            .lock()
+            .expect("Mutex poisoned")
+            .get_stacks_blocks_processed();
+        signer_test
+            .running_nodes
+            .btc_regtest_controller
+            .build_next_block(1);
+
+        wait_for(60, || {
+            let blocks_processed_after = coord_channel
+                .lock()
+                .expect("Mutex poisoned")
+                .get_stacks_blocks_processed();
+            Ok(blocks_processed_after > blocks_processed_before)
+        })
+        .expect("Timed out waiting for tenure extend block");
+    }
+
+    // The last block should have a single instruction in it, the tenure extend
+    let blocks = test_observer::get_blocks();
+    let last_block = blocks.last().unwrap();
+    let transactions = last_block["transactions"].as_array().unwrap();
+    let tx = transactions.first().expect("No transactions in block");
+    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::TenureChange(payload)
+            if payload.cause == TenureChangeCause::Extended => {}
+        _ => panic!("Expected tenure extend transaction, got {:?}", parsed),
+    };
+
+    // Verify that the miner can continue mining in the tenure with the tenure extend
+    info!("------------------------- Mine After Tenure Extend -------------------------");
+    let mut sender_nonce = 0;
+    let mut blocks_processed_before = coord_channel
+        .lock()
+        .expect("Mutex poisoned")
+        .get_stacks_blocks_processed();
+    for _ in 0..5 {
+        // 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,
+        );
+        sender_nonce += 1;
+        submit_tx(&http_origin, &transfer_tx);
+
+        info!("Submitted transfer tx and waiting for block proposal");
+        wait_for(30, || {
+            let blocks_processed_after = coord_channel
+                .lock()
+                .expect("Mutex poisoned")
+                .get_stacks_blocks_processed();
+            Ok(blocks_processed_after > blocks_processed_before)
+        })
+        .expect("Timed out waiting for block proposal");
+        blocks_processed_before = coord_channel
+            .lock()
+            .expect("Mutex poisoned")
+            .get_stacks_blocks_processed();
+        info!("Block {blocks_processed_before} processed, continuing");
+    }
+
+    signer_test.shutdown();
+}
+
 #[test]
 #[ignore]
 /// Test that signers can successfully sign a block proposal in the 0th tenure of a reward cycle