CRC: Add SortitionStateVersion, replace v3-1-00-13 with v3-1-0-0-13, fix typos, remove duplicate code in make_miner_state

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

Author: jferrant

.vscode/settings.json

@@ -5,5 +5,5 @@
   },
   "rust-analyzer.rustfmt.extraArgs": ["+nightly"],
   "files.eol": "\n",
-  "rust-analyzer.cargo.features": ["build-signer-v3-1-00-13"],
+  "rust-analyzer.cargo.features": ["build-signer-v3-1-0-0-13"],
 }

libsigner/src/v0/messages.rs

@@ -1817,7 +1817,7 @@ impl std::fmt::Display for RejectReason {
             RejectReason::IrrecoverablePubkeyHash => {
                 write!(
                     f,
-                    "The block has an irreocverable associated miner public key hash."
+                    "The block has an irrecoverable associated miner public key hash."
                 )
             }
             RejectReason::NoSignerConsensus => {

stacks-signer/CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to the versioning scheme outlined in the [README.md](RE
 ### 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.
+- Added codepath to enable signers to evaluate block proposals and miner activity against global signer state for improved consistency and correctness. Currently feature gated behind the `SUPPORTED_SIGNER_PROTOCOL_VERSION`
 
 ## [3.1.0.0.12.0]
 

stacks-signer/src/chainstate/mod.rs

@@ -420,6 +420,33 @@ impl SortitionData {
     }
 }
 
+/// The version of the sortition state
+#[derive(Debug, Clone)]
+pub enum SortitionStateVersion {
+    /// Version 1: Local Signer State evaluation only
+    V1,
+    /// Version 2: Global Siner State evaluation
+    V2,
+}
+
+impl SortitionStateVersion {
+    /// Convert the protocol version to a sortition state version
+    pub fn from_protocol_version(version: u64) -> Self {
+        if version < GLOBAL_SIGNER_STATE_ACTIVATION_VERSION {
+            Self::V1
+        } else {
+            Self::V2
+        }
+    }
+    /// Uses global state version
+    pub fn uses_global_state(&self) -> bool {
+        match self {
+            Self::V1 => false,
+            Self::V2 => true,
+        }
+    }
+}
+
 /// The wrapped SortitionState to enable multiple implementations
 pub enum SortitionState {
     /// THe V1 implementation of SortitionState
@@ -430,14 +457,21 @@ pub enum SortitionState {
 
 impl SortitionState {
     /// Create a new SortitionState from the provided active protocol version and data
-    pub fn new(active_version: u64, data: SortitionData) -> Self {
-        if active_version < GLOBAL_SIGNER_STATE_ACTIVATION_VERSION {
-            Self::V1(SortitionStateV1 {
+    pub fn new(version: SortitionStateVersion, data: SortitionData) -> Self {
+        match version {
+            SortitionStateVersion::V1 => Self::V1(SortitionStateV1 {
                 data,
                 miner_status: SortitionMinerStatus::Valid,
-            })
-        } else {
-            Self::V2(SortitionStateV2 { data })
+            }),
+            SortitionStateVersion::V2 => Self::V2(SortitionStateV2 { data }),
+        }
+    }
+
+    /// Get the SorttionState version
+    pub fn version(&self) -> SortitionStateVersion {
+        match self {
+            Self::V1(_) => SortitionStateVersion::V1,
+            Self::V2(_) => SortitionStateVersion::V2,
         }
     }
 
@@ -460,7 +494,9 @@ impl SortitionState {
         if !chose_good_parent {
             return Ok(false);
         }
-        self.is_timed_out(
+        Self::is_timed_out(
+            &self.version(),
+            &data.consensus_hash,
             signer_db,
             client.get_signer_address(),
             proposal_config,
@@ -477,23 +513,23 @@ impl SortitionState {
         }
     }
 
-    /// Check if the sortition identified by the ConsensusHash within SortitionState
-    /// is timed out based on the blocks within the signer db and the block proposal timeout
+    /// Check if the tenure identified by the ConsensusHash is timed out
     pub fn is_timed_out(
-        &self,
+        version: &SortitionStateVersion,
+        consensus_hash: &ConsensusHash,
         signer_db: &SignerDb,
         local_address: &StacksAddress,
         proposal_config: &ProposalEvalConfig,
         eval: &GlobalStateEvaluator,
     ) -> Result<bool, SignerChainstateError> {
-        match self {
-            Self::V1(state) => SortitionStateV1::is_timed_out(
-                &state.data.consensus_hash,
+        match version {
+            SortitionStateVersion::V1 => SortitionStateV1::is_timed_out(
+                consensus_hash,
                 signer_db,
                 proposal_config.block_proposal_timeout,
             ),
-            Self::V2(state) => SortitionStateV2::is_timed_out(
-                &state.data.consensus_hash,
+            SortitionStateVersion::V2 => SortitionStateV2::is_timed_out(
+                consensus_hash,
                 signer_db,
                 eval,
                 local_address,

stacks-signer/src/v0/signer.rs

@@ -49,14 +49,12 @@ use super::signer_state::LocalStateMachine;
 use super::signer_state::SUPPORTED_SIGNER_PROTOCOL_VERSION;
 use crate::chainstate::v1::{SortitionMinerStatus, SortitionsView};
 use crate::chainstate::v2::GlobalStateView;
-use crate::chainstate::{ProposalEvalConfig, SortitionData};
+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};
-use crate::v0::signer_state::{
-    NewBurnBlock, ReplayScopeOpt, GLOBAL_SIGNER_STATE_ACTIVATION_VERSION,
-};
+use crate::v0::signer_state::{NewBurnBlock, ReplayScopeOpt};
 use crate::Signer as SignerTrait;
 
 /// A global variable that can be used to make signers repeat their proposal
@@ -666,16 +664,18 @@ impl Signer {
             );
             return Some(self.create_block_rejection(RejectReason::NoSignerConsensus, block));
         };
-        if latest_version < GLOBAL_SIGNER_STATE_ACTIVATION_VERSION {
-            self.check_block_against_sortition_state(stacks_client, sortition_state, block)
-        } else {
+        let state_version = SortitionStateVersion::from_protocol_version(latest_version);
+        if state_version.uses_global_state() {
             self.check_block_against_global_state(stacks_client, block)
+        } else {
+            self.check_block_against_local_state(stacks_client, sortition_state, block)
         }
     }
 
-    /// Check if block should be rejected based on sortition state
+    /// Check if block should be rejected based on the local view of the sortition state
     /// Will return a BlockResponse::Rejection if the block is invalid, none otherwise.
-    fn check_block_against_sortition_state(
+    /// This is the pre-global signer state activation path.
+    fn check_block_against_local_state(
         &mut self,
         stacks_client: &StacksClient,
         sortition_state: &mut Option<SortitionsView>,
@@ -735,6 +735,7 @@ impl Signer {
 
     /// Check if block should be rejected based on global signer state
     /// Will return a BlockResponse::Rejection if the block is invalid, none otherwise.
+    /// This is the Post-global signer state activation path
     fn check_block_against_global_state(
         &mut self,
         stacks_client: &StacksClient,

stacks-signer/src/v0/signer_state.rs

@@ -38,18 +38,17 @@ use stacks_common::util::hash::Sha512Trunc256Sum;
 use stacks_common::util::secp256k1::Secp256k1PublicKey;
 use stacks_common::{debug, info, warn};
 
-use crate::chainstate::v1::{
-    SortitionMinerStatus, SortitionState as SortitionStateV1, SortitionsView,
+use crate::chainstate::v1::{SortitionMinerStatus, SortitionsView};
+use crate::chainstate::{
+    ProposalEvalConfig, SignerChainstateError, SortitionData, SortitionState, SortitionStateVersion,
 };
-use crate::chainstate::v2::SortitionState as SortitionStateV2;
-use crate::chainstate::{ProposalEvalConfig, SignerChainstateError, SortitionData, SortitionState};
 use crate::client::{ClientError, CurrentAndLastSortition, StackerDB, StacksClient};
 use crate::signerdb::{BlockValidatedByReplaySet, SignerDb};
 
 /// This is the latest supported protocol version for this signer binary
 pub static SUPPORTED_SIGNER_PROTOCOL_VERSION: u64 = 2;
 /// The version at which global signer state activates
-pub static GLOBAL_SIGNER_STATE_ACTIVATION_VERSION: u64 = 2;
+pub static GLOBAL_SIGNER_STATE_ACTIVATION_VERSION: u64 = u64::MAX;
 
 /// Vec of pubkeys that should ignore checking for a bitcoin fork
 #[cfg(any(test, feature = "testing"))]
@@ -302,19 +301,17 @@ impl LocalStateMachine {
             return Ok(());
         };
 
-        let is_timed_out = if state_machine.active_signer_protocol_version
-            < GLOBAL_SIGNER_STATE_ACTIVATION_VERSION
-        {
-            SortitionStateV1::is_timed_out(tenure_id, db, proposal_config.block_proposal_timeout)
-        } else {
-            SortitionStateV2::is_timed_out(
-                tenure_id,
-                db,
-                eval,
-                client.get_signer_address(),
-                proposal_config.block_proposal_timeout,
-            )
-        }?;
+        let version = SortitionStateVersion::from_protocol_version(
+            state_machine.active_signer_protocol_version,
+        );
+        let is_timed_out = SortitionState::is_timed_out(
+            &version,
+            tenure_id,
+            db,
+            client.get_signer_address(),
+            proposal_config,
+            eval,
+        )?;
 
         if !is_timed_out {
             return Ok(());
@@ -325,7 +322,12 @@ impl LocalStateMachine {
             client.get_current_and_last_sortition()?;
         let Some(last_sortition) = last_sortition
             .and_then(|val| SortitionData::try_from(val).ok())
-            .map(|data| SortitionState::new(state_machine.active_signer_protocol_version, data))
+            .map(|data| {
+                SortitionState::new(
+                    version,
+                    data,
+                )
+            })
         else {
             warn!("Signer State: Current miner timed out due to inactivity, but could not find a valid prior miner. Allowing current miner to continue");
             return Ok(());
@@ -422,45 +424,13 @@ impl LocalStateMachine {
         let next_current_miner_pkh = sortition_to_set.miner_pkh;
         let next_parent_tenure_id = sortition_to_set.parent_tenure_id;
 
-        let stacks_node_last_block = client
-            .get_tenure_tip(&next_parent_tenure_id)
-            .inspect_err(|e| {
-                warn!(
-                    "Signer State: Failed to fetch last block in parent tenure from stacks-node";
-                    "parent_tenure_id" => %sortition_to_set.parent_tenure_id,
-                    "err" => ?e,
-                )
-            })
-            .ok()
-            .map(|header| {
-                (
-                    header.height(),
-                    StacksBlockId::new(&next_parent_tenure_id, &header.block_hash()),
-                )
-            });
-        let signerdb_last_block = SortitionData::get_tenure_last_block_info(
-            &next_parent_tenure_id,
-            db,
-            tenure_last_block_proposal_timeout,
-        )?
-        .map(|info| (info.block.header.chain_length, info.block.block_id()));
-
         let (parent_tenure_last_block_height, parent_tenure_last_block) =
-            match (stacks_node_last_block, signerdb_last_block) {
-                (Some(stacks_node_info), Some(signerdb_info)) => {
-                    std::cmp::max_by_key(stacks_node_info, signerdb_info, |(chain_length, _)| {
-                        *chain_length
-                    })
-                }
-                (None, Some(signerdb_info)) => signerdb_info,
-                (Some(stacks_node_info), None) => stacks_node_info,
-                (None, None) => {
-                    return Err(SignerChainstateError::NoParentTenureInfo(
-                        next_parent_tenure_id,
-                    ))
-                }
-            };
-
+            Self::get_parent_tenure_last_block(
+                client,
+                db,
+                tenure_last_block_proposal_timeout,
+                &next_parent_tenure_id,
+            )?;
         let miner_state = MinerState::ActiveMiner {
             current_miner_pkh: next_current_miner_pkh,
             tenure_id: sortition_to_set.consensus_hash,
@@ -683,10 +653,10 @@ impl LocalStateMachine {
             last_sortition,
         } = client.get_current_and_last_sortition()?;
 
-        let cur_sortition = SortitionState::new(
+        let version = SortitionStateVersion::from_protocol_version(
             prior_state_machine.active_signer_protocol_version,
-            current_sortition.try_into()?,
         );
+        let cur_sortition = SortitionState::new(version.clone(), current_sortition.try_into()?);
         let is_current_valid = cur_sortition.is_tenure_valid(db, client, proposal_config, eval)?;
 
         let miner_state = if is_current_valid {
@@ -706,10 +676,7 @@ impl LocalStateMachine {
                 })?
                 .try_into()?;
 
-            let last_sortition = SortitionState::new(
-                prior_state_machine.active_signer_protocol_version,
-                last_sortition_data,
-            );
+            let last_sortition = SortitionState::new(version, last_sortition_data);
             let is_last_valid =
                 last_sortition.is_tenure_valid(db, client, proposal_config, eval)?;
 

testnet/stacks-node/Cargo.toml

@@ -32,10 +32,10 @@ async-std = { version = "1.6", optional = true, features = ["attributes"] }
 http-types = { version = "2.12", optional = true }
 thiserror = { workspace = true }
 
-# This dependency is used for the multiversion integration tests which live behind the build-v3-1-00-13 feature flag
+# This dependency is used for the multiversion integration tests which live behind the build-v3-1-0-0-13 feature flag
 # TODO: update the rev commit to the tagged master commit once v3_1_00_13 is released.
-signer_v3_1_00_13 = { package = "stacks-signer", git = "https://github.com/stacks-network/stacks-core.git", rev="0fa1ff3ba65d4ee5c54d398f7086bcaf9e6ae532", optional = true, features = ["testing", "default"]}
-libsigner_v3_1_00_13 = { package = "libsigner", git = "https://github.com/stacks-network/stacks-core.git", rev="0fa1ff3ba65d4ee5c54d398f7086bcaf9e6ae532", optional = true}
+signer_v3_1_0_0_13 = { package = "stacks-signer", git = "https://github.com/stacks-network/stacks-core.git", rev="0fa1ff3ba65d4ee5c54d398f7086bcaf9e6ae532", optional = true, features = ["testing", "default"]}
+libsigner_v3_1_0_0_13 = { package = "libsigner", git = "https://github.com/stacks-network/stacks-core.git", rev="0fa1ff3ba65d4ee5c54d398f7086bcaf9e6ae532", optional = true}
 stacks_v3_1_00_13 = { package = "stackslib", git = "https://github.com/stacks-network/stacks-core.git", rev="0fa1ff3ba65d4ee5c54d398f7086bcaf9e6ae532", optional = true, features = ["testing", "default"]}
 stacks_common_v3_1_00_13 = { package = "stacks-common", git = "https://github.com/stacks-network/stacks-core.git", rev="0fa1ff3ba65d4ee5c54d398f7086bcaf9e6ae532", optional = true, features = ["testing", "default"]}
 [target.'cfg(not(any(target_os = "macos", target_os="windows", target_arch = "arm")))'.dependencies]
@@ -76,7 +76,7 @@ slog_json = ["stacks/slog_json", "stacks-common/slog_json", "clarity/slog_json"]
 prod-genesis-chainstate = []
 default = []
 testing = ["stacks-common/testing", "stacks/testing", "clarity/testing", "stacks-signer/testing"]
-build-signer-v3-1-00-13 = ["signer_v3_1_00_13", "libsigner_v3_1_00_13", "stacks_v3_1_00_13", "stacks_common_v3_1_00_13"]
+build-signer-v3-1-0-0-13 = ["signer_v3_1_0_0_13", "libsigner_v3_1_0_0_13", "stacks_v3_1_00_13", "stacks_common_v3_1_00_13"]
 
 [package.metadata.pinny]
 allowed = ["bitcoind", "flaky", "slow"]

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

@@ -13,7 +13,7 @@
 // You should have received a copy of the GNU General Public License
 // along with this program.  If not, see <http://www.gnu.org/licenses/>.
 mod commands;
-#[cfg(feature = "build-signer-v3-1-00-13")]
+#[cfg(feature = "build-signer-v3-1-0-0-13")]
 pub mod multiversion;
 pub mod v0;