From 237b7fd68e34b23013ac98fab9d6802a60738869 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Fri, 5 Dec 2025 09:06:55 -0800 Subject: [PATCH 1/4] Upgrade SUPPORTED_SIGNER_PROTOCOL_VERSION and set GLOBAL_SIGNER_STATE_ACTIVATION_VERSION Signed-off-by: Jacinta Ferrant --- stacks-signer/CHANGELOG.md | 6 ++++++ stacks-signer/src/v0/signer_state.rs | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/stacks-signer/CHANGELOG.md b/stacks-signer/CHANGELOG.md index fc7ce56508e..12e54217fed 100644 --- a/stacks-signer/CHANGELOG.md +++ b/stacks-signer/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to the versioning scheme outlined in the [README.md](README.md). +## [Unreleased] + +### Changed + +- Upgraded `SUPPORTED_SIGNER_PROTOCOL_VERSION` to 2. +- Set `GLOBAL_SIGNER_STATE_ACTIVATION_VERSION` to 2, activating global signer state agreement block processing. ## [3.2.0.0.2.0] diff --git a/stacks-signer/src/v0/signer_state.rs b/stacks-signer/src/v0/signer_state.rs index bc2190d5c89..e426b397d71 100644 --- a/stacks-signer/src/v0/signer_state.rs +++ b/stacks-signer/src/v0/signer_state.rs @@ -48,9 +48,9 @@ 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 = 1; +pub static SUPPORTED_SIGNER_PROTOCOL_VERSION: u64 = 2; /// The version at which global signer state activates -pub static GLOBAL_SIGNER_STATE_ACTIVATION_VERSION: u64 = u64::MAX; +pub static GLOBAL_SIGNER_STATE_ACTIVATION_VERSION: u64 = 2; /// Vec of pubkeys that should ignore checking for a bitcoin fork #[cfg(any(test, feature = "testing"))] From 73530e4e20a940eace19c52cc7d0561baca5cd89 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Thu, 11 Dec 2025 14:39:45 -0800 Subject: [PATCH 2/4] Send signer updates even if signer doesn't support the current active version and update tests Signed-off-by: Jacinta Ferrant --- libsigner/src/v0/messages.rs | 49 ++++- stacks-node/src/tests/signer/mod.rs | 13 +- stacks-node/src/tests/signer/v0.rs | 280 ++++++++++++++++++++++++++- stacks-signer/src/v0/signer_state.rs | 39 ++-- 4 files changed, 341 insertions(+), 40 deletions(-) diff --git a/libsigner/src/v0/messages.rs b/libsigner/src/v0/messages.rs index 36a5dd05a6f..b22877e22e0 100644 --- a/libsigner/src/v0/messages.rs +++ b/libsigner/src/v0/messages.rs @@ -52,7 +52,7 @@ use stacks_common::types::chainstate::StacksBlockId; use stacks_common::util::hash::{Hash160, Sha512Trunc256Sum}; use crate::stacks_common::types::PublicKey; -use crate::v0::signer_state::ReplayTransactionSet; +use crate::v0::signer_state::{ReplayTransactionSet, SignerStateMachine}; use crate::{ BlockProposal, MessageSlotID as MessageSlotIDTrait, SignerMessage as SignerMessageTrait, VERSION_STRING, @@ -623,8 +623,8 @@ impl StateMachineUpdate { local_supported_signer_protocol_version: u64, content: StateMachineUpdateContent, ) -> Result { - if !content.is_protocol_version_compatible(active_signer_protocol_version) { - return Err(CodecError::DeserializeError(format!("StateMachineUpdateContent is incompatible with protocol version: {active_signer_protocol_version}"))); + if !content.is_protocol_version_supported(active_signer_protocol_version) { + return Err(CodecError::DeserializeError(format!("StateMachineUpdateContent is incompatible with the active protocol version: {active_signer_protocol_version}"))); } Ok(Self { active_signer_protocol_version, @@ -692,12 +692,45 @@ impl StacksMessageCodec for StateMachineUpdateMinerState { } impl StateMachineUpdateContent { - // Is the protocol version specified one that uses self's content? - fn is_protocol_version_compatible(&self, version: u64) -> bool { + /// Attempt to create a new state machine update content with the specified version + pub fn new( + version: u64, + current_miner: StateMachineUpdateMinerState, + state_machine: &SignerStateMachine, + ) -> Result { + let content = match version { + 0 => StateMachineUpdateContent::V0 { + burn_block: state_machine.burn_block.clone(), + burn_block_height: state_machine.burn_block_height, + current_miner, + }, + 1 => StateMachineUpdateContent::V1 { + burn_block: state_machine.burn_block.clone(), + burn_block_height: state_machine.burn_block_height, + current_miner, + replay_transactions: state_machine.tx_replay_set.clone().unwrap_or_default(), + }, + 2 => StateMachineUpdateContent::V2 { + burn_block: state_machine.burn_block.clone(), + burn_block_height: state_machine.burn_block_height, + current_miner, + replay_transactions: state_machine.tx_replay_set.clone().unwrap_or_default(), + }, + other => { + return Err(CodecError::DeserializeError(format!( + "Signer protocol version is unknown: {other}" + ))) + } + }; + Ok(content) + } + + // Is the active protocol version supported by the one used by self's content? + fn is_protocol_version_supported(&self, active_version: u64) -> bool { match self { - Self::V0 { .. } => version == 0, - Self::V1 { .. } => version == 1, - Self::V2 { .. } => version == 2, + Self::V0 { .. } => true, + Self::V1 { .. } => active_version >= 1, + Self::V2 { .. } => active_version >= 2, } } diff --git a/stacks-node/src/tests/signer/mod.rs b/stacks-node/src/tests/signer/mod.rs index d8db0e960d0..266941ea1a3 100644 --- a/stacks-node/src/tests/signer/mod.rs +++ b/stacks-node/src/tests/signer/mod.rs @@ -63,6 +63,7 @@ use stacks_signer::config::{build_signer_config_tomls, GlobalConfig as SignerCon use stacks_signer::runloop::{SignerResult, State, StateInfo}; use stacks_signer::signerdb::SignerDb; use stacks_signer::v0::signer_state::LocalStateMachine; +use stacks_signer::v0::tests::TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION; use stacks_signer::{Signer, SpawnedSigner}; use super::nakamoto_integrations::{ @@ -1414,10 +1415,14 @@ impl SignerTest { .iter() .zip(self.signer_configs.clone()) .map(|(privk, config)| { - ( - StacksAddress::p2pkh(false, &StacksPublicKey::from_private(privk)), - config.supported_signer_protocol_version, - ) + let public_key = StacksPublicKey::from_private(privk); + let pinned_versions = TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION.get(); + let version = if let Some(pinned_version) = pinned_versions.get(&public_key) { + *pinned_version + } else { + config.supported_signer_protocol_version + }; + (StacksAddress::p2pkh(false, &public_key), version) }) .collect() } diff --git a/stacks-node/src/tests/signer/v0.rs b/stacks-node/src/tests/signer/v0.rs index 06556bc5863..5476fdd6be6 100644 --- a/stacks-node/src/tests/signer/v0.rs +++ b/stacks-node/src/tests/signer/v0.rs @@ -1841,7 +1841,14 @@ fn block_proposal_rejection() { if signer_signature_hash == block_signer_signature_hash_1 { found_signer_signature_hash_1 = true; assert_eq!(reason_code, RejectCode::SortitionViewMismatch,); - assert_eq!(response_data.reject_reason, RejectReason::InvalidBitvec); + assert!( + matches!( + response_data.reject_reason, + RejectReason::ConsensusHashMismatch { .. } + ), + "Unexpected reject reason: {}", + response_data.reject_reason + ); } else if signer_signature_hash == block_signer_signature_hash_2 { found_signer_signature_hash_2 = true; assert!(matches!( @@ -7588,7 +7595,8 @@ fn empty_tenure_delayed() { .collect(); assert_eq!(signer_slot_ids.len(), num_signers); - // The miner's proposed block should get rejected by all the signers + // The miner's proposed block should get rejected by all the signers as the consensus hash does not match + // the active miner's tenure id let mut found_rejections = Vec::new(); wait_for(short_timeout.as_secs(), || { for slot_id in signer_slot_ids.iter() { @@ -7614,7 +7622,7 @@ fn empty_tenure_delayed() { })) = latest_msg { assert_eq!(reason_code, RejectCode::SortitionViewMismatch); - assert_eq!(response_data.reject_reason, RejectReason::InvalidMiner); + assert!(matches!(response_data.reject_reason, RejectReason::ConsensusHashMismatch { .. }), "Unexpected reject reason: {}", response_data.reject_reason); assert_eq!(metadata.server_version, VERSION_STRING.to_string()); found_rejections.push(*slot_id); } else { @@ -15692,7 +15700,7 @@ fn non_blocking_minority_configured_to_favour_incoming_miner() { /// - The stacks tip advances to N+3 #[test] #[ignore] -fn non_blocking_minority_configured_to_favour_prev_miner() { +fn non_blocking_minority_configured_to_favour_prev_miner_v1() { if env::var("BITCOIND_TEST") != Ok("1".into()) { return; } @@ -15726,7 +15734,10 @@ fn non_blocking_minority_configured_to_favour_prev_miner() { config.miner.block_commit_delay = Duration::from_secs(0); }, ); - + let all_signers = miners.signer_test.signer_test_pks(); + // Pin all the signers to version 1; + let pinned_signers = all_signers.iter().map(|key| (key.clone(), 1)).collect(); + TEST_PIN_SUPPORTED_SIGNER_PROTOCOL_VERSION.set(pinned_signers); let (conf_1, _) = 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(); @@ -15888,6 +15899,247 @@ fn non_blocking_minority_configured_to_favour_prev_miner() { miners.shutdown(); } +/// Test a scenario where a non-blocking majority of signers are configured to favour the previous miner +/// extending their tenure when the incoming miner is slow to propose a block. The incoming miner should succeed +/// and its subsequent blocks should be be approved. +/// Two miners boot to Nakamoto. +/// Miner 1 wins the first tenure A. +/// Miner 1 proposes a block N with a TenureChangeCause::BlockFound +/// Signers accept and the stacks tip advances to N +/// Miner 2 wins the second tenure B. +/// A minority of signers mark miner 2 as invalid. +/// Miner 1 proposes block N+1' with a TenureChangeCause::Extended +/// ALL signers reject block N+1' +/// Miner 2 proposes block N+1 with a TenureChangeCause::BlockFound +/// ALL signers accept block N+1. +/// Miner 2 proposes block N+2 with a transfer tx +/// ALL signers should accept block N+2. +/// Miner 1 wins the third tenure C. +/// Miner 1 proposes block N+3 with a TenureChangeCause::BlockFound +/// Signers accept and the stacks tip advances to N+3 +/// +/// Asserts: +/// - Block N contains the TenureChangeCause::BlockFound +/// - Block N+1' contains a TenureChangeCause::Extended and is rejected +/// - Block N+1 contains the TenureChangeCause::BlockFound +/// - Block N+2 is accepted. +/// - Block N+3 contains the TenureChangeCause::BlockFound. +/// - The stacks tip advances to N+3 +#[test] +#[ignore] +fn non_blocking_minority_configured_to_favour_prev_miner() { + if env::var("BITCOIND_TEST") != Ok("1".into()) { + return; + } + + let num_signers = 5; + let non_block_minority = num_signers * 2 / 10; + let num_txs = 1; + + let favour_prev_miner_block_proposal_timeout = Duration::from_secs(20); + let favour_incoming_miner_block_proposal_timeout = Duration::from_secs(500); + // Make sure the miner attempts to extend after the minority mark the incoming as invalid + let tenure_extend_wait_timeout = favour_prev_miner_block_proposal_timeout; + + let mut miners = MultipleMinerTest::new_with_config_modifications( + num_signers, + num_txs, + |signer_config| { + let port = signer_config.endpoint.port(); + // Note signer ports are based on the number of them, the first being 3000, the last being 3000 + num_signers - 1 + if port < 3000 + non_block_minority as u16 { + signer_config.block_proposal_timeout = favour_prev_miner_block_proposal_timeout; + } else { + signer_config.block_proposal_timeout = favour_incoming_miner_block_proposal_timeout; + } + }, + |config| { + config.miner.tenure_extend_wait_timeout = tenure_extend_wait_timeout; + config.miner.block_commit_delay = Duration::from_secs(0); + }, + |config| { + config.miner.block_commit_delay = Duration::from_secs(0); + }, + ); + let all_signers = miners.signer_test.signer_test_pks(); + let non_blocking_minority_signers = &all_signers[..non_block_minority]; + let non_blocking_signer_versions: Vec<_> = miners + .signer_test + .signer_addresses_versions() + .into_iter() + .filter(|(address, _)| { + non_blocking_minority_signers + .iter() + .find(|pubkey| &StacksAddress::p2pkh(false, pubkey) == address) + .is_some() + }) + .collect(); + let (conf_1, _) = 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 rl1_skip_commit_op = miners + .signer_test + .running_nodes + .counters + .naka_skip_commit_op + .clone(); + let rl2_skip_commit_op = miners.rl2_counters.naka_skip_commit_op.clone(); + + info!("------------------------- Pause Miner 2's Block Commits -------------------------"); + + // Make sure Miner 2 cannot win a sortition at first. + rl2_skip_commit_op.set(true); + + miners.boot_to_epoch_3(); + + let burnchain = conf_1.get_burnchain(); + let sortdb = burnchain.open_sortition_db(true).unwrap(); + + let get_burn_height = || { + SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()) + .unwrap() + .block_height + }; + let starting_peer_height = get_chain_info(&conf_1).stacks_tip_height; + let starting_burn_height = get_burn_height(); + let mut btc_blocks_mined = 0; + + info!("------------------------- Pause Miner 1's Block Commit -------------------------"); + // Make sure miner 1 doesn't submit any further block commits for the next tenure BEFORE mining the bitcoin block + rl1_skip_commit_op.set(true); + + info!("------------------------- Miner 1 Mines a Normal Tenure A -------------------------"); + miners + .mine_bitcoin_block_and_tenure_change_tx(&sortdb, TenureChangeCause::BlockFound, 30) + .expect("Failed to mine BTC block and Tenure Change Tx Block"); + btc_blocks_mined += 1; + + // assure we have a successful sortition that miner 1 won + verify_sortition_winner(&sortdb, &miner_pkh_1); + + info!("------------------------- Submit Miner 2 Block Commit -------------------------"); + miners.submit_commit_miner_2(&sortdb); + // Pause the block proposal broadcast so that miner 2 will be unable to broadcast its + // tenure change proposal BEFORE miner 1 attempts to extend. + TEST_BROADCAST_PROPOSAL_STALL.set(vec![miner_pk_2.clone()]); + + let stacks_height_before = miners.get_peer_stacks_tip_height(); + info!("------------------------- Miner 2 Wins Tenure B -------------------------"; + "stacks_height_before" => %stacks_height_before); + test_observer::clear(); + miners + .mine_bitcoin_blocks_and_confirm(&sortdb, 1, 30) + .expect("Failed to start Tenure B"); + btc_blocks_mined += 1; + + assert_eq!(stacks_height_before, miners.get_peer_stacks_tip_height()); + + // assure we have a successful sortition that miner 2 won + verify_sortition_winner(&sortdb, &miner_pkh_2); + info!( + "------------------------- Wait for Miner 1 to think Miner 2 is Invalid -------------------------" + ); + // Make sure that miner 1 thinks miner 2 is invalid. + std::thread::sleep(tenure_extend_wait_timeout.add(Duration::from_secs(1))); + let get_burn_consensus_hash = || { + SortitionDB::get_canonical_burn_chain_tip(sortdb.conn()) + .unwrap() + .consensus_hash + }; + // Lets make sure our non blocking minority tries to mark the miner invalid + wait_for_state_machine_update( + 30, + &get_burn_consensus_hash(), + miners.get_peer_info().burn_block_height, + Some((miner_pkh_1.clone(), stacks_height_before - 1)), + &non_blocking_signer_versions, + ) + .expect("Timed out waiting for minority signers to send a state update"); + + info!("------------------------- Wait for Miner 1's Block N+1' to be Proposed ------------------------"; + "stacks_height_before" => %stacks_height_before); + + let miner_1_block_n_1_prime = + wait_for_block_proposal(30, stacks_height_before + 1, &miner_pk_1) + .expect("Miner 1 failed to propose block N+1'"); + assert!(miner_1_block_n_1_prime + .try_get_tenure_change_payload() + .unwrap() + .cause + .is_eq(&TenureChangeCause::Extended)); + + info!("------------------------- Verify that Miner 1's Block N+1' was Rejected by ALL signers ------------------------"); + wait_for_block_rejections_from_signers( + 30, + &miner_1_block_n_1_prime.header.signer_signature_hash(), + &all_signers, + ) + .expect("Failed to reach rejection consensus for Miner 1's Block N+1'"); + + assert_eq!(stacks_height_before, miners.get_peer_stacks_tip_height()); + + info!("------------------------- Wait for Miner 2's Block N+1 BlockFound to be Proposed and Approved------------------------"; + "stacks_height_before" => %stacks_height_before + ); + + TEST_BROADCAST_PROPOSAL_STALL.set(vec![]); + + let miner_2_block_n_1 = + wait_for_block_pushed_by_miner_key(30, stacks_height_before + 1, &miner_pk_2) + .expect("Miner 2's block N+1 was not mined"); + let peer_info = miners.get_peer_info(); + assert_eq!(peer_info.stacks_tip, miner_2_block_n_1.header.block_hash()); + assert_eq!(peer_info.stacks_tip_height, stacks_height_before + 1); + + info!("------------------------- Verify ALL the Signer's Accepted Miner 2's Block N+1 -------------------------"); + wait_for_block_acceptance_from_signers( + 30, + &miner_2_block_n_1.header.signer_signature_hash(), + &all_signers, + ) + .expect("Failed to get expected acceptances for Miner 2's block N+1."); + info!( + "------------------------- Verify BlockFound in Miner 2's Block N+1 -------------------------" + ); + verify_last_block_contains_tenure_change_tx(TenureChangeCause::BlockFound); + + info!("------------------------- Miner 2 Mines Block N+2 with Transfer Tx -------------------------"); + let stacks_height_before = miners.get_peer_stacks_tip_height(); + miners + .send_and_mine_transfer_tx(30) + .expect("Failed to Mine Block N+2"); + + let miner_2_block_n_2 = + wait_for_block_pushed_by_miner_key(30, stacks_height_before + 1, &miner_pk_2) + .expect("Miner 2's block N+1 was not mined"); + let peer_info = miners.get_peer_info(); + assert_eq!(peer_info.stacks_tip, miner_2_block_n_2.header.block_hash()); + assert_eq!(peer_info.stacks_tip_height, stacks_height_before + 1); + + info!("------------------------- Unpause Miner 1's Block Commits -------------------------"); + miners.submit_commit_miner_1(&sortdb); + + info!("------------------------- Miner 1 Mines a Normal Tenure C -------------------------"); + miners + .mine_bitcoin_block_and_tenure_change_tx(&sortdb, TenureChangeCause::BlockFound, 30) + .expect("Failed to start Tenure C and mine block N+3"); + btc_blocks_mined += 1; + + // assure we have a successful sortition that miner 1 won + verify_sortition_winner(&sortdb, &miner_pkh_1); + + info!( + "------------------------- Confirm Burn and Stacks Block Heights -------------------------" + ); + assert_eq!(get_burn_height(), starting_burn_height + btc_blocks_mined); + assert_eq!( + miners.get_peer_stacks_tip_height(), + starting_peer_height + 4 + ); + miners.shutdown(); +} + /// Test a scenario where: /// Two miners boot to Nakamoto. /// Sortition occurs. Miner 1 wins. @@ -19000,19 +19252,37 @@ fn signers_do_not_commit_unless_threshold_precommitted() { let ignore_signers: Vec<_> = ignore_slice.to_vec(); let pre_commit_signers: Vec<_> = pre_commit_slice.to_vec(); TEST_IGNORE_ALL_BLOCK_PROPOSALS.set(ignore_signers); + // Pause the miner proposing blocks because we need to make sure that the global signer state updates before we evaluate the proposal to ensure + // it is isn't immediately rejected under global signer state conditions + TEST_BROADCAST_PROPOSAL_STALL.set(vec![miner_pk.clone()]); test_observer::clear(); let blocks_before = test_observer::get_mined_nakamoto_blocks().len(); let height_before = signer_test.get_peer_info().stacks_tip_height; + info!("------------------------- Start Tenure A -------------------------"); next_block_and( &mut signer_test.running_nodes.btc_regtest_controller, 30, || Ok(test_observer::get_mined_nakamoto_blocks().len() > blocks_before), ) .unwrap(); + let peer_after = signer_test.get_peer_info(); + wait_for_state_machine_update( + 30, + &peer_after.pox_consensus, + peer_after.burn_block_height, + None, + &signer_test.signer_addresses_versions(), + ) + .expect("Failed to update to tenure A"); + TEST_BROADCAST_PROPOSAL_STALL.set(vec![]); + + info!("------------------------- Wait for block proposal N -------------------------"); let proposal = wait_for_block_proposal(30, height_before + 1, &miner_pk) .expect("Timed out waiting for block proposal"); let hash = proposal.header.signer_signature_hash(); + + info!("------------------------- Wait for pre-commits for N ({hash})-------------------------"); wait_for_block_pre_commits_from_signers(30, &hash, &pre_commit_signers) .expect("Timed out waiting for pre-commits"); assert!( diff --git a/stacks-signer/src/v0/signer_state.rs b/stacks-signer/src/v0/signer_state.rs index e426b397d71..b66327a0e8b 100644 --- a/stacks-signer/src/v0/signer_state.rs +++ b/stacks-signer/src/v0/signer_state.rs @@ -187,30 +187,23 @@ impl LocalStateMachine { MinerState::NoValidMiner => StateMachineUpdateMinerState::NoValidMiner, }; - let content = match state_machine.active_signer_protocol_version { - 0 => StateMachineUpdateContent::V0 { - burn_block: state_machine.burn_block.clone(), - burn_block_height: state_machine.burn_block_height, - current_miner, - }, - 1 => StateMachineUpdateContent::V1 { - burn_block: state_machine.burn_block.clone(), - burn_block_height: state_machine.burn_block_height, - current_miner, - replay_transactions: state_machine.tx_replay_set.clone().unwrap_or_default(), - }, - 2 => StateMachineUpdateContent::V2 { - burn_block: state_machine.burn_block.clone(), - burn_block_height: state_machine.burn_block_height, - current_miner, - replay_transactions: state_machine.tx_replay_set.clone().unwrap_or_default(), - }, - other => { - return Err(CodecError::DeserializeError(format!( - "Active signer protocol version is unknown: {other}" - ))) - } + let active_signer_protocol_version = state_machine.active_signer_protocol_version; + // Make sure we fall back to our local supported version in the case of the other signers being ahead of us + // so we can still participate/inform the global state what our view point is + let content_version = if active_signer_protocol_version + > local_supported_signer_protocol_version + { + warn!( + "Cannot create state machine update for future protocol version {active_signer_protocol_version}; falling back to supported version {local_supported_signer_protocol_version}" + ); + local_supported_signer_protocol_version + } else { + active_signer_protocol_version }; + + let content = + StateMachineUpdateContent::new(content_version, current_miner, &state_machine)?; + StateMachineUpdateMessage::new( state_machine.active_signer_protocol_version, local_supported_signer_protocol_version, From b574621ddc9f85ba1d897e04b3e77936b146275a Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Thu, 11 Dec 2025 14:44:41 -0800 Subject: [PATCH 3/4] Fix comment Signed-off-by: Jacinta Ferrant --- libsigner/src/v0/messages.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsigner/src/v0/messages.rs b/libsigner/src/v0/messages.rs index b22877e22e0..596c48d2dac 100644 --- a/libsigner/src/v0/messages.rs +++ b/libsigner/src/v0/messages.rs @@ -725,7 +725,7 @@ impl StateMachineUpdateContent { Ok(content) } - // Is the active protocol version supported by the one used by self's content? + // Is the current self's content supported by the given active protocol version? fn is_protocol_version_supported(&self, active_version: u64) -> bool { match self { Self::V0 { .. } => true, From 1ddc9854a0f0f1c2990eaf84db1017de07b5a71b Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Thu, 11 Dec 2025 15:36:15 -0800 Subject: [PATCH 4/4] Fix the version compatibility check Signed-off-by: Jacinta Ferrant --- libsigner/src/v0/messages.rs | 58 ++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/libsigner/src/v0/messages.rs b/libsigner/src/v0/messages.rs index 596c48d2dac..da369fa43e1 100644 --- a/libsigner/src/v0/messages.rs +++ b/libsigner/src/v0/messages.rs @@ -623,9 +623,18 @@ impl StateMachineUpdate { local_supported_signer_protocol_version: u64, content: StateMachineUpdateContent, ) -> Result { - if !content.is_protocol_version_supported(active_signer_protocol_version) { - return Err(CodecError::DeserializeError(format!("StateMachineUpdateContent is incompatible with the active protocol version: {active_signer_protocol_version}"))); + let version = content.version(); + // The content version MUST be exactly the expected one: + // If local supports at least the active version, use active version + // Otherwise, use local supported version + let expected_version = + active_signer_protocol_version.min(local_supported_signer_protocol_version); + if version != expected_version { + return Err(CodecError::DeserializeError(format!( + "Content version {version} does not match expected protocol version {expected_version} (active={active_signer_protocol_version}, supported={local_supported_signer_protocol_version})" + ))); } + Ok(Self { active_signer_protocol_version, local_supported_signer_protocol_version, @@ -725,12 +734,11 @@ impl StateMachineUpdateContent { Ok(content) } - // Is the current self's content supported by the given active protocol version? - fn is_protocol_version_supported(&self, active_version: u64) -> bool { + fn version(&self) -> u64 { match self { - Self::V0 { .. } => true, - Self::V1 { .. } => active_version >= 1, - Self::V2 { .. } => active_version >= 2, + Self::V0 { .. } => 0, + Self::V1 { .. } => 1, + Self::V2 { .. } => 2, } } @@ -2457,23 +2465,29 @@ mod test { #[test] fn version_check_state_machine_update() { - let error = StateMachineUpdate::new( - 1, - 3, - StateMachineUpdateContent::V0 { - burn_block: ConsensusHash([0x55; 20]), - burn_block_height: 100, - current_miner: StateMachineUpdateMinerState::ActiveMiner { - current_miner_pkh: Hash160([0xab; 20]), - tenure_id: ConsensusHash([0x44; 20]), - parent_tenure_id: ConsensusHash([0x22; 20]), - parent_tenure_last_block: StacksBlockId([0x33; 32]), - parent_tenure_last_block_height: 1, - }, + let content = StateMachineUpdateContent::V1 { + burn_block: ConsensusHash([0x55; 20]), + burn_block_height: 100, + current_miner: StateMachineUpdateMinerState::ActiveMiner { + current_miner_pkh: Hash160([0xab; 20]), + tenure_id: ConsensusHash([0x44; 20]), + parent_tenure_id: ConsensusHash([0x22; 20]), + parent_tenure_last_block: StacksBlockId([0x33; 32]), + parent_tenure_last_block_height: 1, }, - ) - .unwrap_err(); + replay_transactions: vec![], + }; + // We active version does not support the content + let error = StateMachineUpdate::new(0, 1, content.clone()).unwrap_err(); + assert!(matches!(error, CodecError::DeserializeError(_))); + // The content should be the min of the active/local versions but it is lower + let error = StateMachineUpdate::new(2, 3, content.clone()).unwrap_err(); + assert!(matches!(error, CodecError::DeserializeError(_))); + // The content should be the min of the active/local versions but it is greater + let error = StateMachineUpdate::new(2, 0, content.clone()).unwrap_err(); assert!(matches!(error, CodecError::DeserializeError(_))); + // the content version is equal to the min of the active/local versions + assert!(StateMachineUpdate::new(1, 2, content.clone()).is_ok()) } #[test]