Merge pull request #6341 from jferrant/fix/use-local-protocol-version

Use the local supported version by default if no consensus is found

Author: jferrant

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

@@ -1424,6 +1424,35 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
         }
     }
 
+    /// Kills the signer runloop at index `signer_idx`
+    ///  and returns GlobalConfig of the killed signer
+    ///
+    /// # Panics
+    /// Panics if `signer_idx` is out of bounds
+    fn stop_signer(&mut self, signer_idx: usize) -> stacks_signer::config::GlobalConfig {
+        let running_signer = self.spawned_signers.remove(signer_idx);
+        let _signer_key = self.signer_stacks_private_keys.remove(signer_idx);
+        let signer_config = self.signer_configs.remove(signer_idx);
+        running_signer.stop();
+        signer_config
+    }
+
+    /// (Re)starts a new signer runloop with the given private key and adds it to the list
+    /// of running signers, updating the list of signer_stacks_private_keys and signer_configs
+    fn restart_signer(
+        &mut self,
+        signer_idx: usize,
+        signer_config: stacks_signer::config::GlobalConfig,
+    ) {
+        info!("Restarting signer");
+        self.signer_stacks_private_keys
+            .insert(signer_idx, signer_config.stacks_private_key.clone());
+        self.signer_configs
+            .insert(signer_idx, signer_config.clone());
+        self.spawned_signers
+            .insert(signer_idx, Z::new(signer_config));
+    }
+
     /// Get the latest block response from the given slot
     pub fn get_latest_block_response(&self, slot_id: u32) -> BlockResponse {
         let mut stackerdb = StackerDB::new_normal(

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

@@ -18195,3 +18195,160 @@ fn multiversioned_signer_protocol_version_calculation() {
     .unwrap();
     signer_test.shutdown();
 }
+
+/// Ensure that signers can immediately start participating in signing when starting up
+/// after crashing mid reward cycle.
+///
+/// Test scenario:
+/// - Miner A wins Tenure A
+/// - Miner A proposes block N (Tenure Change)
+/// - All signers sign block N.
+/// - Miner B wins Tenure B.
+/// - Shutdown one signer.
+/// - Shutdown signer is restarted.
+/// - Miner B proposes block N+1 (TenureChange).
+/// - All signers sign the block without issue
+/// -> Verifies that updates are loaded from signerdb on init
+/// - Same signer is shutdown.
+/// - Shutdown signers db is cleared.
+/// - Signer is restarted.
+/// - Miner B proposes block N+2 (Transfer).
+/// - All signers including the restarted signer sign block N+2
+/// -> Verifies that updates are loaded from stackerdb on init
+#[test]
+#[ignore]
+fn signer_loads_stackerdb_updates_on_startup() {
+    if env::var("BITCOIND_TEST") != Ok("1".into()) {
+        return;
+    }
+
+    let num_signers = 5;
+    let mut miners = MultipleMinerTest::new(num_signers, 1);
+
+    let skip_commit_op_rl1 = miners
+        .signer_test
+        .running_nodes
+        .counters
+        .naka_skip_commit_op
+        .clone();
+    let skip_commit_op_rl2 = miners.rl2_counters.naka_skip_commit_op.clone();
+
+    let (conf_1, _conf_2) = miners.get_node_configs();
+    let (miner_pk_1, miner_pk_2) = miners.get_miner_public_keys();
+    let (miner_pkh_1, miner_pkh_2) = miners.get_miner_public_key_hashes();
+
+    let all_signers = miners.signer_test.signer_test_pks();
+
+    // Pause Miner 2's commits to ensure Miner 1 wins the first sortition.
+    skip_commit_op_rl2.set(true);
+    miners.boot_to_epoch_3();
+
+    let sortdb = conf_1.get_burnchain().open_sortition_db(true).unwrap();
+
+    info!("Pausing miner 1's block commit submissions");
+    skip_commit_op_rl1.set(true);
+
+    info!("------------------------- Miner A Wins Tenure A -------------------------");
+    let info_before = get_chain_info(&conf_1);
+    // Let's not mine anything until we see consensus on new tenure start.
+    TEST_MINE_SKIP.set(true);
+    next_block_and(&miners.btc_regtest_controller_mut(), 60, || {
+        let info = get_chain_info(&conf_1);
+        Ok(info.burn_block_height > info_before.burn_block_height)
+    })
+    .unwrap();
+    let chain_after = get_chain_info(&conf_1);
+    wait_for_state_machine_update_by_miner_tenure_id(
+        30,
+        &chain_after.pox_consensus,
+        &miners.signer_test.signer_addresses_versions(),
+    )
+    .expect("Timed out waiting for the signers to update their state");
+    verify_sortition_winner(&sortdb, &miner_pkh_1);
+
+    info!(
+        "------------------------- Miner A Mines Block N (Tenure Change) -------------------------"
+    );
+    TEST_MINE_SKIP.set(false);
+    let block_n =
+        wait_for_block_pushed_by_miner_key(30, chain_after.stacks_tip_height + 1, &miner_pk_1)
+            .expect("Failed to mine block N");
+    wait_for_block_acceptance_from_signers(
+        30,
+        &block_n.header.signer_signature_hash(),
+        &all_signers,
+    )
+    .expect("Not all signers accepted the block");
+
+    info!("------------------------- Miner B Wins Tenure B -------------------------");
+    miners.submit_commit_miner_2(&sortdb);
+    let chain_before = get_chain_info(&conf_1);
+    // Let's not mine anything until we see consensus on new tenure start.
+    TEST_MINE_SKIP.set(true);
+    next_block_and(&miners.btc_regtest_controller_mut(), 60, || {
+        let info = get_chain_info(&conf_1);
+        Ok(info.burn_block_height > chain_before.burn_block_height)
+    })
+    .unwrap();
+    let chain_after = get_chain_info(&conf_1);
+    wait_for_state_machine_update_by_miner_tenure_id(
+        30,
+        &chain_after.pox_consensus,
+        &miners.signer_test.signer_addresses_versions(),
+    )
+    .expect("Signers failed to update their state");
+    verify_sortition_winner(&sortdb, &miner_pkh_2);
+
+    let stop_idx = 0;
+    info!("------------------------- Shutdown Signer at idx {stop_idx} -------------------------");
+    let stopped_signer_config = miners.signer_test.stop_signer(stop_idx);
+    info!("------------------------- Restart Signer at idx {stop_idx} -------------------------");
+    miners
+        .signer_test
+        .restart_signer(stop_idx, stopped_signer_config);
+
+    info!("------------------------- Miner B Mines Block N+1 (Tenure Change) -------------------------");
+    TEST_MINE_SKIP.set(false);
+    let block_n_1 =
+        wait_for_block_pushed_by_miner_key(30, chain_after.stacks_tip_height + 1, &miner_pk_2)
+            .expect("Failed to mine block N+1");
+    wait_for_block_acceptance_from_signers(
+        30,
+        &block_n_1.header.signer_signature_hash(),
+        &all_signers,
+    )
+    .expect("Not all signers accepted the block");
+
+    info!("------------------------- Shutdown Signer at idx {stop_idx} -------------------------");
+    let stopped_signer_config = miners.signer_test.stop_signer(stop_idx);
+    {
+        let mut signer_db = SignerDb::new(stopped_signer_config.db_path.clone()).unwrap();
+        signer_db
+            .clear_state_machine_updates()
+            .expect("Failed to clear state machine updates");
+    }
+    info!("------------------------- Restart Signer at idx {stop_idx} -------------------------");
+    miners
+        .signer_test
+        .restart_signer(stop_idx, stopped_signer_config);
+
+    // Wait until signer boots up BEFORE proposing the next block
+    miners.signer_test.wait_for_registered();
+    info!("------------------------- Miner B Mines Block N+2 (Transfer) -------------------------");
+    let (accepting, ignoring) = all_signers.split_at(4);
+    // Make some of the signers ignore so that we CANNOT advance without approval from the restarted signer (its at index 0)
+    TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(ignoring.into());
+    miners.send_transfer_tx();
+    let block_n_2 =
+        wait_for_block_pushed_by_miner_key(30, chain_after.stacks_tip_height + 2, &miner_pk_2)
+            .expect("Failed to mine block N+2");
+    wait_for_block_acceptance_from_signers(
+        30,
+        &block_n_2.header.signer_signature_hash(),
+        &accepting,
+    )
+    .expect("Not all signers accepted the block");
+
+    info!("------------------------- Shutdown -------------------------");
+    miners.shutdown();
+}

stacks-signer/src/signerdb.rs

@@ -1705,6 +1705,15 @@ impl SignerDb {
         Ok(())
     }
 
+    #[cfg(any(test, feature = "testing"))]
+    /// Clear out signer state machine updates for testing purposes ONLY.
+    pub fn clear_state_machine_updates(&mut self) -> Result<(), DBError> {
+        debug!("Clearing all updates.");
+        self.db
+            .execute("DELETE FROM signer_state_machine_updates", params![])?;
+        Ok(())
+    }
+
     /// Get the most recent signer states from the signer state machine for the given reward cycle
     pub fn get_signer_state_machine_updates(
         &mut self,

stacks-signer/src/v0/signer.rs

@@ -715,17 +715,26 @@ impl Signer {
         block: &NakamotoBlock,
     ) -> Option<BlockResponse> {
         // First update our global state evaluator with our local state if we have one
-        let version = self.get_signer_protocol_version();
+        let local_version = self.get_signer_protocol_version();
         if let Ok(update) = self
             .local_state_machine
-            .try_into_update_message_with_version(version)
+            .try_into_update_message_with_version(local_version)
         {
             self.global_state_evaluator
                 .insert_update(self.stacks_address, update);
         };
         let Some(latest_version) = self
             .global_state_evaluator
             .determine_latest_supported_signer_protocol_version()
+            .or_else(|| {
+                // Don't default if we are in a global consensus activation state as its pointless
+                if SortitionStateVersion::from_protocol_version(local_version).uses_global_state() {
+                    None
+                } else {
+                    warn!("{self}: No consensus on signer protocol version. Defaulting to local state version: {local_version}.");
+                    Some(local_version)
+                }
+            })
         else {
             warn!(
                 "{self}: No consensus on signer protocol version. Unable to validate block. Rejecting.";