feat: new test scenario, fix ordering of txs

Author: hstove

stacks-signer/src/v0/signer_state.rs

@@ -1002,15 +1002,19 @@ impl LocalStateMachine {
             }
             let fork_info =
                 client.get_tenure_forking_info(&first_forked_tenure, &last_forked_tenure)?;
-            let forked_txs = fork_info
+            let mut forked_blocks = fork_info
                 .iter()
                 .flat_map(|fork_info| {
                     fork_info
                         .nakamoto_blocks
                         .iter()
                         .flat_map(|blocks| blocks.iter())
-                        .flat_map(|block| block.txs.iter())
                 })
+                .collect::<Vec<_>>();
+            forked_blocks.sort_by_key(|block| block.header.chain_length);
+            let forked_txs = forked_blocks
+                .iter()
+                .flat_map(|block| block.txs.iter())
                 .filter(|tx| match tx.payload {
                     // Don't include Coinbase, TenureChange, or PoisonMicroblock transactions
                     TransactionPayload::TenureChange(..)

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

@@ -23,6 +23,7 @@ use std::time::{Duration, Instant};
 
 use clarity::boot_util::boot_code_id;
 use clarity::vm::types::PrincipalData;
+use clarity::vm::Value;
 use libsigner::v0::messages::{
     BlockAccepted, BlockResponse, MessageSlotID, PeerInfo, SignerMessage,
 };
@@ -451,43 +452,36 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
         submit_tx_fallible(&http_origin, &transfer_tx).map(|resp| (resp, sender_nonce))
     }
 
-    /// Submit a burn block dependent contract for publishing
-    ///  and wait until it is included in a block
-    pub fn submit_burn_block_contract_and_wait(
+    /// Submit a contract deploy and return (txid, sender_nonce)
+    pub fn submit_contract_deploy(
         &mut self,
         sender_sk: &StacksPrivateKey,
-    ) -> Result<String, String> {
+        contract_code: &str,
+        contract_name: &str,
+    ) -> Result<(String, u64), String> {
         let http_origin = format!("http://{}", &self.running_nodes.conf.node.rpc_bind);
         let sender_addr = to_addr(&sender_sk);
         let sender_nonce = get_account(&http_origin, &sender_addr).nonce;
-        let burn_height_contract = "
-         (define-data-var local-burn-block-ht uint u0)
-         (define-public (run-update)
-           (ok (var-set local-burn-block-ht burn-block-height)))
-        ";
+
         let contract_tx = make_contract_publish(
             &sender_sk,
-            0,
+            sender_nonce,
             1000,
             self.running_nodes.conf.burnchain.chain_id,
-            "burn-height-local",
-            burn_height_contract,
+            contract_name,
+            contract_code,
         );
-        let txid = submit_tx_fallible(&http_origin, &contract_tx)?;
-
-        wait_for(120, || {
-            let next_nonce = get_account(&http_origin, &sender_addr).nonce;
-            Ok(next_nonce > sender_nonce)
-        })
-        .map(|()| txid)
+        submit_tx_fallible(&http_origin, &contract_tx).map(|resp| (resp, sender_nonce))
     }
 
-    /// Submit a burn block dependent contract-call
-    ///  and wait until it is included in a block
-    pub fn submit_burn_block_call_and_wait(
+    /// Submit a contract call and return (txid, sender_nonce)
+    pub fn submit_contract_call(
         &mut self,
         sender_sk: &StacksPrivateKey,
-    ) -> Result<String, String> {
+        contract_name: &str,
+        contract_func: &str,
+        contract_args: &[Value],
+    ) -> Result<(String, u64), String> {
         let http_origin = format!("http://{}", &self.running_nodes.conf.node.rpc_bind);
         let sender_addr = to_addr(&sender_sk);
         let sender_nonce = get_account(&http_origin, &sender_addr).nonce;
@@ -497,17 +491,54 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
             1000,
             self.running_nodes.conf.burnchain.chain_id,
             &sender_addr,
-            "burn-height-local",
-            "run-update",
-            &[],
+            contract_name,
+            contract_func,
+            contract_args,
         );
-        let txid = submit_tx_fallible(&http_origin, &contract_call_tx)?;
+        submit_tx_fallible(&http_origin, &contract_call_tx).map(|resp| (resp, sender_nonce))
+    }
 
+    pub fn wait_for_nonce_increase(
+        &mut self,
+        sender_addr: &StacksAddress,
+        sender_nonce: u64,
+    ) -> Result<(), String> {
+        let http_origin = format!("http://{}", &self.running_nodes.conf.node.rpc_bind);
         wait_for(120, || {
             let next_nonce = get_account(&http_origin, &sender_addr).nonce;
             Ok(next_nonce > sender_nonce)
         })
-        .map(|()| txid)
+    }
+
+    /// Submit a burn block dependent contract for publishing
+    ///  and wait until it is included in a block
+    pub fn submit_burn_block_contract_and_wait(
+        &mut self,
+        sender_sk: &StacksPrivateKey,
+    ) -> Result<String, String> {
+        let burn_height_contract = "
+         (define-data-var local-burn-block-ht uint u0)
+         (define-public (run-update)
+           (ok (var-set local-burn-block-ht burn-block-height)))
+        ";
+        let (txid, sender_nonce) =
+            self.submit_contract_deploy(sender_sk, burn_height_contract, "burn-height-local")?;
+
+        self.wait_for_nonce_increase(&to_addr(&sender_sk), sender_nonce)?;
+        Ok(txid)
+    }
+
+    /// Submit a burn block dependent contract-call
+    ///  and wait until it is included in a block
+    pub fn submit_burn_block_call_and_wait(
+        &mut self,
+        sender_sk: &StacksPrivateKey,
+    ) -> Result<String, String> {
+        let (txid, sender_nonce) =
+            self.submit_contract_call(sender_sk, "burn-height-local", "run-update", &[])?;
+
+        self.wait_for_nonce_increase(&to_addr(&sender_sk), sender_nonce)?;
+        Ok(txid)
     }
 
     /// Get the local state machines and most recent peer info from the stacks-node,

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

@@ -3049,6 +3049,9 @@ fn bitcoind_forking_test() {
 /// - Verify that the signer moves into tx replay state
 /// - Verify that the signer correctly includes the stx transfer
 ///   in the tx replay set
+///
+/// Then, a second fork scenario is tested, which
+/// includes multiple txs across multiple tenures.
 fn tx_replay_forking_test() {
     if env::var("BITCOIND_TEST") != Ok("1".into()) {
         return;
@@ -3061,7 +3064,7 @@ fn tx_replay_forking_test() {
     let send_fee = 180;
     let mut signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
         num_signers,
-        vec![(sender_addr, send_amt + send_fee)],
+        vec![(sender_addr, (send_amt + send_fee) * 10)],
         |_| {},
         |node_config| {
             node_config.miner.block_commit_delay = Duration::from_secs(1);
@@ -3079,9 +3082,10 @@ fn tx_replay_forking_test() {
     for i in 0..pre_fork_tenures {
         info!("Mining pre-fork tenure {} of {pre_fork_tenures}", i + 1);
         signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
-        signer_test.check_signer_states_normal();
     }
 
+    signer_test.check_signer_states_normal();
+
     let burn_blocks = test_observer::get_burn_blocks();
     let forked_blocks = burn_blocks.iter().rev().take(2).collect::<Vec<_>>();
     let last_forked_tenure: ConsensusHash = hex_bytes(
@@ -3220,6 +3224,8 @@ fn tx_replay_forking_test() {
 
     TEST_MINE_STALL.set(false);
 
+    info!("---- Mining post-fork block to clear tx replay set ----");
+
     // Now, make a new stacks block, which should clear the tx replay set
     signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
     let (signer_states, _) = signer_test.get_burn_updated_states();
@@ -3230,6 +3236,146 @@ fn tx_replay_forking_test() {
         );
     }
 
+    // Now, we'll trigger another fork, with more txs, across tenures
+
+    // The forked blocks are:
+    // Tenure 1:
+    // - Block with stx transfer
+    // Tenure 2:
+    // - Block with contract deploy
+    // - Block with contract call
+
+    signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
+
+    let pre_fork_2_tip = get_chain_info(&signer_test.running_nodes.conf);
+
+    let contract_code = "
+    (define-public (call-fn)
+      (ok true)
+    )
+    ";
+    let contract_name = "test-contract";
+
+    let (transfer_txid, transfer_nonce) = signer_test
+        .submit_transfer_tx(&sender_sk, send_fee, send_amt)
+        .expect("Failed to submit transfer tx");
+    signer_test
+        .wait_for_nonce_increase(&sender_addr, transfer_nonce)
+        .expect("Failed to wait for nonce increase");
+    signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
+
+    let (contract_deploy_txid, deploy_nonce) = signer_test
+        .submit_contract_deploy(&sender_sk, contract_code, contract_name)
+        .expect("Failed to submit contract deploy");
+    signer_test
+        .wait_for_nonce_increase(&sender_addr, deploy_nonce)
+        .expect("Failed to wait for nonce increase");
+
+    let (contract_call_txid, contract_call_nonce) = signer_test
+        .submit_contract_call(&sender_sk, contract_name, "call-fn", &[])
+        .expect("Failed to submit contract call");
+    signer_test
+        .wait_for_nonce_increase(&sender_addr, contract_call_nonce)
+        .expect("Failed to wait for nonce increase");
+    signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
+
+    TEST_MINE_STALL.set(true);
+
+    let burn_header_hash_to_fork = signer_test
+        .running_nodes
+        .btc_regtest_controller
+        .get_block_hash(pre_fork_2_tip.burn_block_height);
+    signer_test
+        .running_nodes
+        .btc_regtest_controller
+        .invalidate_block(&burn_header_hash_to_fork);
+    signer_test
+        .running_nodes
+        .btc_regtest_controller
+        .build_next_block(3);
+
+    let burn_blocks = test_observer::get_burn_blocks();
+    let forked_blocks = burn_blocks.iter().rev().take(2).collect::<Vec<_>>();
+    let last_forked_tenure: ConsensusHash = hex_bytes(
+        &forked_blocks[0]
+            .get("consensus_hash")
+            .unwrap()
+            .as_str()
+            .unwrap()[2..],
+    )
+    .unwrap()
+    .as_slice()
+    .into();
+    let first_forked_tenure: ConsensusHash = hex_bytes(
+        &forked_blocks[1]
+            .get("consensus_hash")
+            .unwrap()
+            .as_str()
+            .unwrap()[2..],
+    )
+    .unwrap()
+    .as_slice()
+    .into();
+
+    let fork_info = signer_test
+        .stacks_client
+        .get_tenure_forking_info(&first_forked_tenure, &last_forked_tenure)
+        .unwrap();
+
+    info!("---- Fork info: {fork_info:?} ----");
+
+    for fork in fork_info {
+        info!("---- Fork: {} ----", fork.consensus_hash);
+        fork.nakamoto_blocks.inspect(|blocks| {
+            for block in blocks {
+                info!("---- Block: {} ----", block.header.chain_length);
+            }
+        });
+    }
+
+    for i in 0..3 {
+        let current_burn_height = get_chain_info(&signer_test.running_nodes.conf).burn_block_height;
+        info!(
+            "Mining block #{i} to be considered a frequent miner";
+            "current_burn_height" => current_burn_height,
+        );
+        let commits_count = submitted_commits.load(Ordering::SeqCst);
+        next_block_and_controller(
+            &mut signer_test.running_nodes.btc_regtest_controller,
+            60,
+            |_btc_controller| {
+                let commits_submitted = submitted_commits.load(Ordering::SeqCst);
+                Ok(commits_submitted > commits_count)
+            },
+        )
+        .unwrap();
+    }
+
+    let expected_tx_replay_txids = vec![transfer_txid, contract_deploy_txid, contract_call_txid];
+
+    let (signer_states, _) = signer_test.get_burn_updated_states();
+    for state in signer_states {
+        match state {
+            LocalStateMachine::Initialized(signer_state_machine) => {
+                let Some(tx_replay_set) = signer_state_machine.tx_replay_set else {
+                    panic!(
+                        "Signer state machine is in tx replay state, but tx replay set is not set"
+                    );
+                };
+                info!("---- Tx replay set: {:?} ----", tx_replay_set);
+                assert_eq!(tx_replay_set.len(), expected_tx_replay_txids.len());
+                let state_replay_txids = tx_replay_set
+                    .iter()
+                    .map(|tx| tx.txid().to_hex())
+                    .collect::<Vec<_>>();
+                assert_eq!(state_replay_txids, expected_tx_replay_txids);
+            }
+            _ => {
+                panic!("Signer state is not in the initialized state");
+            }
+        }
+    }
+
     signer_test.shutdown();
 }