Merge pull request #6110 from fdefelici/feat/tx-replay-ignore-forks-across-reward-cycle

implement tx replay rejection on fork across reward cycle

Author: jferrant

stacks-signer/src/v0/signer_state.rs

@@ -849,6 +849,21 @@ impl LocalStateMachine {
         }
         let fork_info =
             client.get_tenure_forking_info(&first_forked_tenure, &last_forked_tenure)?;
+
+        // Check if fork occurred within current reward cycle. Reject tx replay otherwise.
+        let reward_cycle_info = client.get_current_reward_cycle_info()?;
+        let current_reward_cycle = reward_cycle_info.reward_cycle;
+        let is_fork_in_current_reward_cycle = fork_info.iter().all(|fork_info| {
+            let block_height = fork_info.burn_block_height;
+            let block_rc = reward_cycle_info.get_reward_cycle(block_height);
+            block_rc == current_reward_cycle
+        });
+        if !is_fork_in_current_reward_cycle {
+            info!("Detected bitcoin fork occurred in previous reward cycle. Tx replay won't be executed");
+            return Ok(None);
+        }
+
+        // Collect transactions to be replayed across the forked blocks
         let mut forked_blocks = fork_info
             .iter()
             .flat_map(|fork_info| fork_info.nakamoto_blocks.iter().flatten())

stackslib/src/burnchains/burnchain.rs

@@ -578,6 +578,15 @@ impl Burnchain {
             .nakamoto_first_block_of_cycle(self.first_block_height, reward_cycle)
     }
 
+    #[cfg(any(test, feature = "testing"))]
+    /// the last burn block that must be *signed* by the signer set of `reward_cycle`.
+    /// this is the modulo -1 block
+    pub fn nakamoto_last_block_of_cycle(&self, reward_cycle: u64) -> u64 {
+        self.nakamoto_first_block_of_cycle(reward_cycle)
+            + self.pox_constants.reward_cycle_length as u64
+            - 1
+    }
+
     /// What is the reward cycle for this block height?
     /// This considers the modulo 0 block to be in reward cycle `n`, even though
     ///  rewards for cycle `n` do not begin until modulo 1.
@@ -1765,3 +1774,173 @@ impl Burnchain {
         Ok(block_header)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use regex::Regex;
+
+    use super::*;
+    use crate::burnchains::*;
+
+    #[test]
+    fn test_creation_by_new_for_bitcoin_mainnet() {
+        let burn_chain = Burnchain::new("workdir/path", "bitcoin", "mainnet");
+        assert!(burn_chain.is_ok());
+
+        let burn_chain = burn_chain.unwrap();
+        let first_block_hash =
+            BurnchainHeaderHash::from_hex(BITCOIN_MAINNET_FIRST_BLOCK_HASH).unwrap();
+        assert_eq!(PEER_VERSION_MAINNET, burn_chain.peer_version);
+        assert_eq!(BITCOIN_NETWORK_ID_MAINNET, burn_chain.network_id);
+        assert_eq!(BITCOIN_MAINNET_NAME, burn_chain.network_name);
+        assert_eq!("workdir/path", burn_chain.working_dir);
+        assert_eq!(24, burn_chain.consensus_hash_lifetime);
+        assert_eq!(7, burn_chain.stable_confirmations);
+        assert_eq!(
+            BITCOIN_MAINNET_FIRST_BLOCK_HEIGHT,
+            burn_chain.first_block_height
+        );
+        assert_eq!(first_block_hash, burn_chain.first_block_hash);
+        assert_eq!(
+            BITCOIN_MAINNET_FIRST_BLOCK_TIMESTAMP,
+            burn_chain.first_block_timestamp
+        );
+        assert_eq!(PoxConstants::mainnet_default(), burn_chain.pox_constants);
+        assert_eq!(
+            BITCOIN_MAINNET_INITIAL_REWARD_START_BLOCK,
+            burn_chain.initial_reward_start_block
+        );
+    }
+
+    #[test]
+    fn test_creation_by_new_for_bitcoin_testnet() {
+        let burn_chain = Burnchain::new("workdir/path", "bitcoin", "testnet");
+        assert!(burn_chain.is_ok());
+
+        let burn_chain = burn_chain.unwrap();
+        let first_block_hash =
+            BurnchainHeaderHash::from_hex(BITCOIN_TESTNET_FIRST_BLOCK_HASH).unwrap();
+        assert_eq!(PEER_VERSION_TESTNET, burn_chain.peer_version);
+        assert_eq!(BITCOIN_NETWORK_ID_TESTNET, burn_chain.network_id);
+        assert_eq!(BITCOIN_TESTNET_NAME, burn_chain.network_name);
+        assert_eq!("workdir/path", burn_chain.working_dir);
+        assert_eq!(24, burn_chain.consensus_hash_lifetime);
+        assert_eq!(7, burn_chain.stable_confirmations);
+        assert_eq!(
+            BITCOIN_TESTNET_FIRST_BLOCK_HEIGHT,
+            burn_chain.first_block_height
+        );
+        assert_eq!(first_block_hash, burn_chain.first_block_hash);
+        assert_eq!(
+            BITCOIN_TESTNET_FIRST_BLOCK_TIMESTAMP,
+            burn_chain.first_block_timestamp
+        );
+        assert_eq!(PoxConstants::testnet_default(), burn_chain.pox_constants);
+        assert_eq!(1_990_000, burn_chain.initial_reward_start_block);
+    }
+
+    #[test]
+    fn test_creation_by_new_for_bitcoin_regtest() {
+        let burn_chain = Burnchain::new("workdir/path", "bitcoin", "regtest");
+        assert!(burn_chain.is_ok());
+
+        let burn_chain = burn_chain.unwrap();
+        let first_block_hash =
+            BurnchainHeaderHash::from_hex(BITCOIN_REGTEST_FIRST_BLOCK_HASH).unwrap();
+        assert_eq!(PEER_VERSION_TESTNET, burn_chain.peer_version);
+        assert_eq!(BITCOIN_NETWORK_ID_REGTEST, burn_chain.network_id);
+        assert_eq!(BITCOIN_REGTEST_NAME, burn_chain.network_name);
+        assert_eq!("workdir/path", burn_chain.working_dir);
+        assert_eq!(24, burn_chain.consensus_hash_lifetime);
+        assert_eq!(1, burn_chain.stable_confirmations);
+        assert_eq!(
+            BITCOIN_REGTEST_FIRST_BLOCK_HEIGHT,
+            burn_chain.first_block_height
+        );
+        assert_eq!(first_block_hash, burn_chain.first_block_hash);
+        assert_eq!(
+            BITCOIN_REGTEST_FIRST_BLOCK_TIMESTAMP,
+            burn_chain.first_block_timestamp
+        );
+        assert_eq!(PoxConstants::regtest_default(), burn_chain.pox_constants);
+        assert_eq!(
+            BITCOIN_REGTEST_FIRST_BLOCK_HEIGHT,
+            burn_chain.initial_reward_start_block
+        );
+    }
+
+    #[test]
+    fn test_creation_by_new_failure() {
+        //case: wrong chain name
+        let burn_chain = Burnchain::new("workdir/path", "wrong_chain_name", "regtest");
+        assert!(burn_chain.is_err());
+        assert!(matches!(
+            burn_chain.unwrap_err(),
+            burnchain_error::UnsupportedBurnchain
+        ));
+
+        //case: wrong network name
+        let burn_chain = Burnchain::new("workdir/path", "bitcoin", "wrong_net_name");
+        assert!(burn_chain.is_err());
+        assert!(matches!(
+            burn_chain.unwrap_err(),
+            burnchain_error::UnsupportedBurnchain
+        ));
+
+        //case: wrong chain name + wrong network name
+        let burn_chain = Burnchain::new("workdir/path", "wrong_chain_name", "wrong_net_name");
+        assert!(burn_chain.is_err());
+        assert!(matches!(
+            burn_chain.unwrap_err(),
+            burnchain_error::UnsupportedBurnchain
+        ));
+    }
+
+    #[test]
+    fn test_creation_by_default_unittest() {
+        let first_block_height = 0;
+        let first_block_hash = BurnchainHeaderHash([0u8; 32]);
+        let burn_chain = Burnchain::default_unittest(first_block_height, &first_block_hash);
+
+        let workdir_re = Regex::new(r"^/tmp/stacks-node-tests/unit-tests-[0-9a-f]{32}$").unwrap();
+
+        assert_eq!(PEER_VERSION_MAINNET, burn_chain.peer_version);
+        assert_eq!(BITCOIN_NETWORK_ID_MAINNET, burn_chain.network_id);
+        assert_eq!(BITCOIN_MAINNET_NAME, burn_chain.network_name);
+        assert!(workdir_re.is_match(&burn_chain.working_dir));
+        assert_eq!(24, burn_chain.consensus_hash_lifetime);
+        assert_eq!(7, burn_chain.stable_confirmations);
+        assert_eq!(first_block_height, burn_chain.first_block_height);
+        assert_eq!(first_block_hash, burn_chain.first_block_hash);
+        assert_eq!(
+            BITCOIN_MAINNET_FIRST_BLOCK_TIMESTAMP,
+            burn_chain.first_block_timestamp
+        );
+        assert_eq!(PoxConstants::mainnet_default(), burn_chain.pox_constants);
+        assert_eq!(first_block_height, burn_chain.initial_reward_start_block);
+    }
+
+    #[test]
+    fn test_nakamoto_reward_cycle_boundaries() {
+        let first_block_height = 0;
+        let first_block_hash = BurnchainHeaderHash([0u8; 32]);
+        let burn_chain = Burnchain::default_unittest(first_block_height, &first_block_hash);
+
+        //making obvious the reward cycle length used
+        assert_eq!(2100, burn_chain.pox_constants.reward_cycle_length);
+
+        //Reward Cycle: 0
+        let rc = 0;
+        let rc_first_block = burn_chain.nakamoto_first_block_of_cycle(rc);
+        let rc_last_block = burn_chain.nakamoto_last_block_of_cycle(rc);
+        assert_eq!(0, rc_first_block);
+        assert_eq!(2099, rc_last_block);
+
+        //Reward Cycle: 1
+        let rc = 1;
+        let rc_first_block = burn_chain.nakamoto_first_block_of_cycle(rc);
+        let rc_last_block = burn_chain.nakamoto_last_block_of_cycle(rc);
+        assert_eq!(2100, rc_first_block);
+        assert_eq!(4199, rc_last_block);
+    }
+}

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

@@ -3388,6 +3388,131 @@ fn tx_replay_forking_test() {
     signer_test.shutdown();
 }
 
+#[test]
+#[ignore]
+/// Trigger a Bitcoin fork across reward cycle
+/// and ensure that the signers detect the fork,
+/// but reject to move into a tx replay state
+///
+/// The test flow is:
+///
+/// - Boot to Epoch 3 (that is in the middle of reward cycle N)
+/// - Mine until the last tenure of the reward cycle N
+/// - Include a STX transfer in the last tenure
+/// - Mine 1 Bitcoin block in the next reward cycle N+1
+/// - Trigger a Bitcoin fork from reward cycle N (3 blocks)
+/// - Verify that signers don't move into tx replay state
+/// - In the end, the STX transfer transaction is not replayed
+fn tx_replay_rejected_when_forking_across_reward_cycle() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
+    let num_signers = 5;
+    let sender_sk = Secp256k1PrivateKey::random();
+    let sender_addr = tests::to_addr(&sender_sk);
+    let send_amt = 100;
+    let send_fee = 180;
+    let signer_test: SignerTest<SpawnedSigner> = SignerTest::new_with_config_modifications(
+        num_signers,
+        vec![(sender_addr, (send_amt + send_fee) * 10)],
+        |_| {},
+        |node_config| {
+            node_config.miner.block_commit_delay = Duration::from_secs(1);
+        },
+        None,
+        None,
+    );
+    let conf = signer_test.running_nodes.conf.clone();
+    let http_origin = format!("http://{}", &conf.node.rpc_bind);
+    let btc_controller = &signer_test.running_nodes.btc_regtest_controller;
+    let burn_chain = btc_controller.get_burnchain();
+    let counters = &signer_test.running_nodes.counters;
+
+    signer_test.boot_to_epoch_3();
+    info!("------------------------- Reached Epoch 3.0 -------------------------");
+
+    let burn_block_height = get_chain_info(&conf).burn_block_height;
+    let initial_reward_cycle = signer_test.get_current_reward_cycle();
+    let rc_last_height = burn_chain.nakamoto_last_block_of_cycle(initial_reward_cycle);
+
+    info!("----- Mine to the end of reward cycle {initial_reward_cycle} height {rc_last_height} -----");
+    let pre_fork_tenures = rc_last_height - burn_block_height;
+    for i in 1..=pre_fork_tenures {
+        info!("Mining pre-fork tenure {i} of {pre_fork_tenures}");
+        signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
+    }
+    signer_test.check_signer_states_normal();
+
+    info!("----- Submit Stx transfer in last tenure height {rc_last_height} -----");
+    // Make a transfer tx that will get forked
+    let tip = get_chain_info(&conf);
+    let _ = signer_test
+        .submit_transfer_tx(&sender_sk, send_fee, send_amt)
+        .unwrap();
+    wait_for(30, || {
+        let new_tip = get_chain_info(&conf);
+        Ok(new_tip.stacks_tip_height > tip.stacks_tip_height)
+    })
+    .expect("Timed out waiting for transfer tx to be mined");
+
+    let pre_fork_tx_nonce = get_account(&http_origin, &sender_addr).nonce;
+    assert_eq!(1, pre_fork_tx_nonce);
+
+    info!("----- Mine 1 block in new reward cycle -----");
+    signer_test.mine_nakamoto_block(Duration::from_secs(30), true);
+    signer_test.check_signer_states_normal();
+
+    let next_reward_cycle = initial_reward_cycle + 1;
+    let new_burn_block_height = get_chain_info(&conf).burn_block_height;
+    assert_eq!(next_reward_cycle, signer_test.get_current_reward_cycle());
+    assert_eq!(
+        new_burn_block_height,
+        burn_chain.nakamoto_first_block_of_cycle(next_reward_cycle)
+    );
+
+    info!("----- Trigger Bitcoin fork -----");
+    //Fork on the third-to-last tenure of prev reward cycle
+    let burn_block_hash_to_fork = btc_controller.get_block_hash(new_burn_block_height - 2);
+    btc_controller.invalidate_block(&burn_block_hash_to_fork);
+    btc_controller.build_next_block(3);
+
+    // note, we should still have normal signer states!
+    signer_test.check_signer_states_normal();
+
+    //mine throught the fork (just check commits because of naka block mining stalled)
+    TEST_MINE_STALL.set(true);
+
+    let submitted_commits = counters.naka_submitted_commits.clone();
+    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(btc_controller, 60, || {
+            let commits_submitted = submitted_commits.load(Ordering::SeqCst);
+            Ok(commits_submitted > commits_count)
+        })
+        .unwrap();
+    }
+
+    let post_fork_tx_nonce = get_account(&http_origin, &sender_addr).nonce;
+    assert_eq!(0, post_fork_tx_nonce);
+
+    info!("----- Check Signers Tx Replay state -----");
+    let (signer_states, _) = signer_test.get_burn_updated_states();
+    for state in signer_states {
+        assert!(
+            state.get_tx_replay_set().is_none(),
+            "Signer state is in tx replay state, when it shouldn't be"
+        );
+    }
+
+    signer_test.shutdown();
+}
+
 #[test]
 #[ignore]
 fn multiple_miners() {