CRC: add config.supported_signer_protocol_version and allow it to be overwritten in a test

Signed-off-by: Jacinta Ferrant <[email protected]>

Author: jferrant

stacks-signer/CHANGELOG.md

@@ -7,10 +7,6 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 
 ## [Unreleased]
 
-### Changed
-
-- Upgraded `SUPPORTED_SIGNER_PROTOCOL_VERSION` to 2
-
 ### Added
 
 - Introduced `capitulate_miner_view_timeout_secs`: the duration (in seconds) for the signer to wait between updating the local state machine viewpoint and capitulating to other signers' miner views.

stacks-signer/src/client/mod.rs

@@ -156,6 +156,8 @@ pub(crate) mod tests {
 
     use super::*;
     use crate::config::{GlobalConfig, SignerConfig, SignerConfigMode};
+    #[cfg(any(test, feature = "testing"))]
+    use crate::v0::signer_state::SUPPORTED_SIGNER_PROTOCOL_VERSION;
 
     pub struct MockServerClient {
         pub server: TcpListener,
@@ -433,6 +435,8 @@ pub(crate) mod tests {
             proposal_wait_for_parent_time: config.proposal_wait_for_parent_time,
             validate_with_replay_tx: config.validate_with_replay_tx,
             capitulate_miner_view_timeout: config.capitulate_miner_view_timeout,
+            #[cfg(any(test, feature = "testing"))]
+            supported_signer_protocol_version: SUPPORTED_SIGNER_PROTOCOL_VERSION,
         }
     }
 

stacks-signer/src/config.rs

@@ -33,6 +33,8 @@ use stacks_common::types::chainstate::{StacksAddress, StacksPrivateKey, StacksPu
 use stacks_common::util::hash::Hash160;
 
 use crate::client::SignerSlotID;
+#[cfg(any(test, feature = "testing"))]
+use crate::v0::signer_state::SUPPORTED_SIGNER_PROTOCOL_VERSION;
 
 const EVENT_TIMEOUT_MS: u64 = 5000;
 const BLOCK_PROPOSAL_TIMEOUT_MS: u64 = 120_000;
@@ -189,6 +191,9 @@ pub struct SignerConfig {
     pub validate_with_replay_tx: bool,
     /// Time to wait between updating our local state machine view point and capitulating to other signers miner view
     pub capitulate_miner_view_timeout: Duration,
+    #[cfg(any(test, feature = "testing"))]
+    /// Only used for testing purposes to enable overriding the signer version
+    pub supported_signer_protocol_version: u64,
 }
 
 /// The parsed configuration for the signer
@@ -244,6 +249,9 @@ pub struct GlobalConfig {
     pub validate_with_replay_tx: bool,
     /// Time to wait between updating our local state machine view point and capitulating to other signers miner view
     pub capitulate_miner_view_timeout: Duration,
+    #[cfg(any(test, feature = "testing"))]
+    /// Only used for testing to enable specific signer protocol versions
+    pub supported_signer_protocol_version: u64,
 }
 
 /// Internal struct for loading up the config file
@@ -297,6 +305,9 @@ struct RawConfigFile {
     pub validate_with_replay_tx: Option<bool>,
     /// Time to wait (in secs) between updating our local state machine view point and capitulating to other signers miner view
     pub capitulate_miner_view_timeout_secs: Option<u64>,
+    #[cfg(any(test, feature = "testing"))]
+    /// Only used for testing to enable specific signer protocol versions
+    pub supported_signer_protocol_version: Option<u64>,
 }
 
 impl RawConfigFile {
@@ -428,6 +439,11 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
                 .unwrap_or(DEFAULT_CAPITULATE_MINER_VIEW_SECS),
         );
 
+        #[cfg(any(test, feature = "testing"))]
+        let supported_signer_protocol_version = raw_data
+            .supported_signer_protocol_version
+            .unwrap_or(SUPPORTED_SIGNER_PROTOCOL_VERSION);
+
         Ok(Self {
             node_host: raw_data.node_host,
             endpoint,
@@ -451,6 +467,8 @@ impl TryFrom<RawConfigFile> for GlobalConfig {
             proposal_wait_for_parent_time,
             validate_with_replay_tx,
             capitulate_miner_view_timeout,
+            #[cfg(any(test, feature = "testing"))]
+            supported_signer_protocol_version,
         })
     }
 }

stacks-signer/src/runloop.rs

@@ -330,6 +330,8 @@ impl<Signer: SignerTrait<T>, T: StacksMessageCodec + Clone + Send + Debug> RunLo
             proposal_wait_for_parent_time: self.config.proposal_wait_for_parent_time,
             validate_with_replay_tx: self.config.validate_with_replay_tx,
             capitulate_miner_view_timeout: self.config.capitulate_miner_view_timeout,
+            #[cfg(any(test, feature = "testing"))]
+            supported_signer_protocol_version: self.config.supported_signer_protocol_version,
         }))
     }
 

stacks-signer/src/v0/signer.rs

@@ -45,15 +45,15 @@ use stacks_common::util::secp256k1::MessageSignature;
 use stacks_common::{debug, error, info, warn};
 
 use super::signer_state::LocalStateMachine;
-#[cfg(not(any(test, feature = "testing")))]
-use super::signer_state::SUPPORTED_SIGNER_PROTOCOL_VERSION;
 use crate::chainstate::v1::{SortitionMinerStatus, SortitionsView};
 use crate::chainstate::v2::GlobalStateView;
 use crate::chainstate::{ProposalEvalConfig, SortitionData, SortitionStateVersion};
 use crate::client::{ClientError, SignerSlotID, StackerDB, StacksClient};
 use crate::config::{SignerConfig, SignerConfigMode};
 use crate::runloop::SignerResult;
 use crate::signerdb::{BlockInfo, BlockState, SignerDb};
+#[cfg(not(any(test, feature = "testing")))]
+use crate::v0::signer_state::SUPPORTED_SIGNER_PROTOCOL_VERSION;
 use crate::v0::signer_state::{NewBurnBlock, ReplayScopeOpt};
 use crate::Signer as SignerTrait;
 
@@ -131,6 +131,9 @@ pub struct Signer {
     pub tx_replay_scope: ReplayScopeOpt,
     /// Time to wait between updating our local state machine view point and capitulating to other signers miner view
     pub capitulate_miner_view_timeout: Duration,
+    /// The signer supported protocol version. used only in testing
+    #[cfg(any(test, feature = "testing"))]
+    pub supported_signer_protocol_version: u64,
 }
 
 impl std::fmt::Display for SignerMode {
@@ -223,11 +226,16 @@ impl SignerTrait<SignerMessage> for Signer {
             updates,
             signer_config.signer_entries.signer_addr_to_weight.clone(),
         );
+        #[cfg(any(test, feature = "testing"))]
+        let version = signer_config.supported_signer_protocol_version;
+        #[cfg(not(any(test, feature = "testing")))]
+        let version = SUPPORTED_SIGNER_PROTOCOL_VERSION;
         let signer_state = LocalStateMachine::new(
             &signer_db,
             stacks_client,
             &proposal_config,
             &global_state_evaluator,
+            version,
         )
         .unwrap_or_else(|e| {
             warn!("Failed to initialize local state machine for signer: {e:?}");
@@ -254,6 +262,8 @@ impl SignerTrait<SignerMessage> for Signer {
             validate_with_replay_tx: signer_config.validate_with_replay_tx,
             tx_replay_scope: None,
             capitulate_miner_view_timeout: signer_config.capitulate_miner_view_timeout,
+            #[cfg(any(test, feature = "testing"))]
+            supported_signer_protocol_version: signer_config.supported_signer_protocol_version,
         }
     }
 
@@ -275,19 +285,19 @@ impl SignerTrait<SignerMessage> for Signer {
         self.check_pending_block_validations(stacks_client);
 
         let mut prior_state = self.local_state_machine.clone();
+        let local_signer_protocol_version = self.get_signer_protocol_version();
         if self.reward_cycle <= current_reward_cycle {
             self.local_state_machine.handle_pending_update(&self.signer_db, stacks_client,
                 &self.proposal_config,
-                &mut self.tx_replay_scope, &self.global_state_evaluator)
+                &mut self.tx_replay_scope, &self.global_state_evaluator, local_signer_protocol_version)
                 .unwrap_or_else(|e| error!("{self}: failed to update local state machine for pending update"; "err" => ?e));
         }
         // See if we should capitulate our viewpoint...
-        let version = self.get_signer_protocol_version();
         self.local_state_machine.capitulate_viewpoint(
             stacks_client,
             &mut self.signer_db,
             &mut self.global_state_evaluator,
-            version,
+            local_signer_protocol_version,
             self.reward_cycle,
             sortition_state,
             self.capitulate_miner_view_timeout,
@@ -546,13 +556,14 @@ impl Signer {
                         panic!("{self} Failed to write burn block event to signerdb: {e}");
                     });
 
+                let active_signer_protocol_version = self.get_signer_protocol_version();
                 self.local_state_machine
                     .bitcoin_block_arrival(&self.signer_db, stacks_client, &self.proposal_config, Some(NewBurnBlock {
                         burn_block_height: *burn_height,
                         consensus_hash: *consensus_hash,
                     }),
                     &mut self.tx_replay_scope
-                , &self.global_state_evaluator)
+                , &self.global_state_evaluator, active_signer_protocol_version)
                     .unwrap_or_else(|e| error!("{self}: failed to update local state machine for latest bitcoin block arrival"; "err" => ?e));
                 *sortition_state = None;
             }
@@ -1809,12 +1820,20 @@ impl Signer {
 
     #[cfg(not(any(test, feature = "testing")))]
     fn get_signer_protocol_version(&self) -> u64 {
-        SUPPORTED_SIGNER_PROTOCOL_VERSION
+        crate::v0::signer_state::SUPPORTED_SIGNER_PROTOCOL_VERSION
     }
 
     #[cfg(any(test, feature = "testing"))]
     fn get_signer_protocol_version(&self) -> u64 {
-        self.test_get_signer_protocol_version()
+        use crate::v0::tests::TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION;
+        let public_keys = TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION.get();
+        if let Some(version) = public_keys.get(
+            &stacks_common::types::chainstate::StacksPublicKey::from_private(&self.private_key),
+        ) {
+            warn!("{self}: signer version is pinned to {version}");
+            return *version;
+        }
+        self.supported_signer_protocol_version
     }
 }
 

stacks-signer/src/v0/signer_state.rs

@@ -46,7 +46,7 @@ use crate::client::{ClientError, CurrentAndLastSortition, StackerDB, StacksClien
 use crate::signerdb::{BlockValidatedByReplaySet, SignerDb};
 
 /// This is the latest supported protocol version for this signer binary
-pub static SUPPORTED_SIGNER_PROTOCOL_VERSION: u64 = 2;
+pub static SUPPORTED_SIGNER_PROTOCOL_VERSION: u64 = 1;
 /// The version at which global signer state activates
 pub static GLOBAL_SIGNER_STATE_ACTIVATION_VERSION: u64 = u64::MAX;
 
@@ -141,9 +141,18 @@ impl LocalStateMachine {
         client: &StacksClient,
         proposal_config: &ProposalEvalConfig,
         eval: &GlobalStateEvaluator,
+        active_signer_protocol_version: u64,
     ) -> Result<Self, SignerChainstateError> {
         let mut instance = Self::Uninitialized;
-        instance.bitcoin_block_arrival(db, client, proposal_config, None, &mut None, eval)?;
+        instance.bitcoin_block_arrival(
+            db,
+            client,
+            proposal_config,
+            None,
+            &mut None,
+            eval,
+            active_signer_protocol_version,
+        )?;
 
         Ok(instance)
     }
@@ -207,12 +216,12 @@ impl LocalStateMachine {
         )
     }
 
-    fn place_holder() -> SignerStateMachine {
+    fn place_holder(version: u64) -> SignerStateMachine {
         SignerStateMachine {
             burn_block: ConsensusHash::empty(),
             burn_block_height: 0,
             current_miner: MinerState::NoValidMiner,
-            active_signer_protocol_version: SUPPORTED_SIGNER_PROTOCOL_VERSION,
+            active_signer_protocol_version: version,
             tx_replay_set: ReplayTransactionSet::none(),
             update_time: UpdateTime::now(),
         }
@@ -266,6 +275,7 @@ impl LocalStateMachine {
         proposal_config: &ProposalEvalConfig,
         tx_replay_scope: &mut ReplayScopeOpt,
         eval: &GlobalStateEvaluator,
+        local_signer_protocol_version: u64,
     ) -> Result<(), SignerChainstateError> {
         let LocalStateMachine::Pending { update, .. } = self else {
             return self.check_miner_inactivity(db, client, proposal_config, eval);
@@ -278,6 +288,7 @@ impl LocalStateMachine {
                 Some(expected_burn_height),
                 tx_replay_scope,
                 eval,
+                local_signer_protocol_version,
             ),
         }
     }
@@ -549,6 +560,7 @@ impl LocalStateMachine {
     }
 
     /// Handle a new bitcoin block arrival
+    #[allow(clippy::too_many_arguments)]
     pub fn bitcoin_block_arrival(
         &mut self,
         db: &SignerDb,
@@ -557,13 +569,14 @@ impl LocalStateMachine {
         mut expected_burn_block: Option<NewBurnBlock>,
         tx_replay_scope: &mut ReplayScopeOpt,
         eval: &GlobalStateEvaluator,
+        local_signer_protocol_version: u64,
     ) -> Result<(), SignerChainstateError> {
         // set self to uninitialized so that if this function errors,
         //  self is left as uninitialized.
         let prior_state = std::mem::replace(self, Self::Uninitialized);
         let prior_state_machine = match prior_state.clone() {
             // if the local state machine was uninitialized, just initialize it
-            LocalStateMachine::Uninitialized => Self::place_holder(),
+            LocalStateMachine::Uninitialized => Self::place_holder(local_signer_protocol_version),
             LocalStateMachine::Initialized(signer_state_machine) => signer_state_machine,
             LocalStateMachine::Pending { update, prior } => {
                 // This works as long as the pending updates are only burn blocks,

stacks-signer/src/v0/tests.rs

@@ -25,7 +25,6 @@ use stacks_common::util::tests::TestFlag;
 use stacks_common::{info, warn};
 
 use super::signer::Signer;
-use super::signer_state::SUPPORTED_SIGNER_PROTOCOL_VERSION;
 use crate::signerdb::BlockInfo;
 
 /// A global variable that can be used to pin a signer's highest supported protocol version if the signer's public key is in the provided list
@@ -158,16 +157,4 @@ impl Signer {
             warn!("{self}: Block validation submission is no longer stalled due to testing directive. Continuing...");
         }
     }
-
-    /// Get the pinned signer version for the signer
-    pub fn test_get_signer_protocol_version(&self) -> u64 {
-        let public_keys = TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION.get();
-        if let Some(version) = public_keys.get(
-            &stacks_common::types::chainstate::StacksPublicKey::from_private(&self.private_key),
-        ) {
-            warn!("{self}: signer version is pinned to {version}");
-            return *version;
-        }
-        SUPPORTED_SIGNER_PROTOCOL_VERSION
-    }
 }

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

@@ -1343,6 +1343,20 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
             .collect()
     }
 
+    /// Get the signer addresses and corresponding versions
+    pub fn signer_addresses_versions(&self) -> Vec<(StacksAddress, u64)> {
+        self.signer_stacks_private_keys
+            .iter()
+            .zip(self.signer_configs.clone())
+            .map(|(privk, config)| {
+                (
+                    StacksAddress::p2pkh(false, &StacksPublicKey::from_private(privk)),
+                    config.supported_signer_protocol_version,
+                )
+            })
+            .collect()
+    }
+
     /// Get the signer public keys for the given reward cycle
     fn get_signer_public_keys(&self, reward_cycle: u64) -> Vec<StacksPublicKey> {
         let entries = self.get_reward_set_signers(reward_cycle);

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

@@ -26,7 +26,7 @@ use stacks::util::secp256k1::Secp256k1PrivateKey;
 use stacks_common::types::chainstate::{ConsensusHash, StacksBlockId};
 use stacks_common_v3_1_00_13::codec::StacksMessageCodec as OldStacksMessageCodec;
 use stacks_signer::runloop::{RewardCycleInfo, State, StateInfo};
-use stacks_signer::v0::signer_state::{LocalStateMachine, SUPPORTED_SIGNER_PROTOCOL_VERSION};
+use stacks_signer::v0::signer_state::LocalStateMachine;
 use stacks_signer::v0::SpawnedSigner;
 use {libsigner_v3_1_0_0_13, signer_v3_1_0_0_13, stacks_common_v3_1_00_13, stacks_v3_1_00_13};
 
@@ -271,10 +271,10 @@ impl SpawnedSignerTrait for MultiverSpawnedSigner {
 #[test]
 #[ignore]
 fn with_new_miner_and_old_signers() {
-    with_new_miners::<MultiverSpawnedSigner>();
+    with_new_miners::<MultiverSpawnedSigner>(2);
 }
 
-fn with_new_miners<S: SpawnedSignerTrait>() {
+fn with_new_miners<S: SpawnedSignerTrait>(supported_signer_protocol_version: u64) {
     let sender_sk = Secp256k1PrivateKey::from_seed(&[0xde, 0xad, 0xbe, 0xef, 0xaa, 0xbb]);
     let sender_addr = tests::to_addr(&sender_sk);
     let send_amt = 1000;
@@ -297,7 +297,9 @@ fn with_new_miners<S: SpawnedSignerTrait>() {
     let signer_test: SignerTest<S> = SignerTest::new_with_config_modifications(
         num_signers,
         initial_balances,
-        |_| {},
+        |signer_config| {
+            signer_config.supported_signer_protocol_version = supported_signer_protocol_version;
+        },
         |config| {
             config.node.rpc_bind = format!("{localhost}:{node_1_rpc}");
             config.node.p2p_bind = format!("{localhost}:{node_1_p2p}");
@@ -375,7 +377,7 @@ fn with_new_miners<S: SpawnedSignerTrait>() {
             else {
                 return false;
             };
-            local_supported_signer_protocol_version == SUPPORTED_SIGNER_PROTOCOL_VERSION
+            local_supported_signer_protocol_version == supported_signer_protocol_version
         })
         .count();