Skip to content

Feat/signer two phase commit impl #6319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 26 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8c8e44f
Add SignerMessage::BlockPreCommit struct
jferrant Jul 21, 2025
175a85d
Add block_pre_commit table and update schema version to 17
jferrant Jul 21, 2025
9712344
Add pre commit metrics to prometheus
jferrant Jul 21, 2025
42b10da
Handle pre-commit messages
jferrant Jul 21, 2025
8f821f2
Cleanup: do not prematurely wrap a BlockRejection into a BlockRespons…
jferrant Jul 21, 2025
3e4a0ef
Add a test and fix failing tests
jferrant Jul 21, 2025
28cc681
Add an index for get_block_pre_committers
jferrant Jul 22, 2025
c0e4b16
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Jul 24, 2025
a31dc85
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Jul 28, 2025
8a26a8f
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Jul 30, 2025
f03cb2a
Fix test issues
jferrant Jul 30, 2025
7433f81
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Jul 30, 2025
da28f99
CRC: address nits
jferrant Jul 31, 2025
d8e3cda
Update changelog
jferrant Jul 31, 2025
ef21447
Fix multiversioned_signer_protocol_version_calculation test
jferrant Jul 31, 2025
e304a48
Fix flakiness by ensuring block commits confirm the expected stacks b…
jferrant Aug 1, 2025
a338608
Make sure commit height for both burn and stacks are strictly greater…
jferrant Aug 1, 2025
40ba301
Fix multiversioned_signer_protocol_version_calculation test
jferrant Aug 2, 2025
a6ce2fb
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Aug 4, 2025
b14efd5
Fix rollover_signer_protocol_version
jferrant Aug 4, 2025
77cc93e
Sep block processing and commit processing in mine_nakamoto_block and…
jferrant Aug 4, 2025
a823146
Cleanup tests that should be using mine_nakamoto_block or mine_bitcoi…
jferrant Aug 4, 2025
0375c72
Fix burn_block_height_behavior test
jferrant Aug 5, 2025
d503267
Remove leftover debug logs from testing
jferrant Aug 5, 2025
c730eeb
Merge branch 'develop' of https://github.com/stacks-network/stacks-co…
jferrant Aug 7, 2025
2d778cb
Fix injected_signatures_are_ignored_across_boundaries
jferrant Aug 8, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 40 additions & 3 deletions libsigner/src/v0/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ MessageSlotID {
/// Block Response message from signers
BlockResponse = 1,
/// Signer State Machine Update
StateMachineUpdate = 2
StateMachineUpdate = 2,
/// Block Pre-commit message from signers before they commit to a block response
BlockPreCommit = 3
});

define_u8_enum!(
Expand Down Expand Up @@ -114,7 +116,9 @@ SignerMessageTypePrefix {
/// Mock block message from Epoch 2.5 miners
MockBlock = 5,
/// State machine update
StateMachineUpdate = 6
StateMachineUpdate = 6,
/// Block Pre-commit message
BlockPreCommit = 7
});

#[cfg_attr(test, mutants::skip)]
Expand All @@ -137,7 +141,7 @@ impl MessageSlotID {
#[cfg_attr(test, mutants::skip)]
impl Display for MessageSlotID {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{:?}({})", self, self.to_u8())
write!(f, "{self:?}({})", self.to_u8())
}
}

Expand All @@ -161,6 +165,7 @@ impl From<&SignerMessage> for SignerMessageTypePrefix {
SignerMessage::MockSignature(_) => SignerMessageTypePrefix::MockSignature,
SignerMessage::MockBlock(_) => SignerMessageTypePrefix::MockBlock,
SignerMessage::StateMachineUpdate(_) => SignerMessageTypePrefix::StateMachineUpdate,
SignerMessage::BlockPreCommit(_) => SignerMessageTypePrefix::BlockPreCommit,
}
}
}
Expand All @@ -182,6 +187,8 @@ pub enum SignerMessage {
MockBlock(MockBlock),
/// A state machine update
StateMachineUpdate(StateMachineUpdate),
/// The pre-commit message from signers for other signers to observe
BlockPreCommit(Sha512Trunc256Sum),
}

impl SignerMessage {
Expand All @@ -197,6 +204,7 @@ impl SignerMessage {
| Self::MockBlock(_) => None,
Self::BlockResponse(_) | Self::MockSignature(_) => Some(MessageSlotID::BlockResponse), // Mock signature uses the same slot as block response since its exclusively for epoch 2.5 testing
Self::StateMachineUpdate(_) => Some(MessageSlotID::StateMachineUpdate),
Self::BlockPreCommit(_) => Some(MessageSlotID::BlockPreCommit),
}
}
}
Expand All @@ -216,6 +224,9 @@ impl StacksMessageCodec for SignerMessage {
SignerMessage::StateMachineUpdate(state_machine_update) => {
state_machine_update.consensus_serialize(fd)
}
SignerMessage::BlockPreCommit(block_pre_commit) => {
block_pre_commit.consensus_serialize(fd)
}
}?;
Ok(())
}
Expand Down Expand Up @@ -253,6 +264,10 @@ impl StacksMessageCodec for SignerMessage {
let state_machine_update = StacksMessageCodec::consensus_deserialize(fd)?;
SignerMessage::StateMachineUpdate(state_machine_update)
}
SignerMessageTypePrefix::BlockPreCommit => {
let signer_signature_hash = StacksMessageCodec::consensus_deserialize(fd)?;
SignerMessage::BlockPreCommit(signer_signature_hash)
}
};
Ok(message)
}
Expand Down Expand Up @@ -1145,6 +1160,18 @@ pub enum BlockResponse {
Rejected(BlockRejection),
}

impl From<BlockRejection> for BlockResponse {
fn from(rejection: BlockRejection) -> Self {
BlockResponse::Rejected(rejection)
}
}

impl From<BlockAccepted> for BlockResponse {
fn from(accepted: BlockAccepted) -> Self {
BlockResponse::Accepted(accepted)
}
}

#[cfg_attr(test, mutants::skip)]
impl std::fmt::Display for BlockResponse {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
Expand Down Expand Up @@ -2604,4 +2631,14 @@ mod test {

assert_eq!(signer_message, signer_message_deserialized);
}

#[test]
fn serde_block_signer_message_pre_commit() {
let pre_commit = SignerMessage::BlockPreCommit(Sha512Trunc256Sum([0u8; 32]));
let serialized_pre_commit = pre_commit.serialize_to_vec();
let deserialized_pre_commit =
read_next::<SignerMessage, _>(&mut &serialized_pre_commit[..])
.expect("Failed to deserialize pre-commit");
assert_eq!(pre_commit, deserialized_pre_commit);
}
}
3 changes: 3 additions & 0 deletions stacks-node/src/nakamoto_node/stackerdb_listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,9 @@ impl StackerDBListener {
SignerMessageV0::StateMachineUpdate(update) => {
self.update_global_state_evaluator(&signer_pubkey, update);
}
SignerMessageV0::BlockPreCommit(_) => {
debug!("Received block pre-commit message. Ignoring.");
}
};
}
}
Expand Down
109 changes: 90 additions & 19 deletions stacks-node/src/tests/signer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ mod commands;
pub mod multiversion;
pub mod v0;

use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
use std::fs::File;
use std::path::PathBuf;
use std::sync::atomic::{AtomicBool, Ordering};
Expand Down Expand Up @@ -70,6 +70,7 @@ use super::nakamoto_integrations::{
use super::neon_integrations::{
copy_dir_all, get_account, get_sortition_info_ch, submit_tx_fallible, Account,
};
use crate::nakamoto_node::miner::TEST_MINE_SKIP;
use crate::neon::Counters;
use crate::run_loop::boot_nakamoto;
use crate::tests::bitcoin_regtest::BitcoinCoreController;
Expand All @@ -80,6 +81,7 @@ use crate::tests::neon_integrations::{
get_chain_info, next_block_and_wait, run_until_burnchain_height, test_observer,
wait_for_runloop,
};
use crate::tests::signer::v0::wait_for_state_machine_update_by_miner_tenure_id;
use crate::tests::to_addr;
use crate::BitcoinRegtestController;

Expand Down Expand Up @@ -535,11 +537,16 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
}

pub fn mine_bitcoin_block(&self) {
let mined_btc_block_time = Instant::now();
let info = self.get_peer_info();
next_block_and(&self.running_nodes.btc_regtest_controller, 60, || {
Ok(get_chain_info(&self.running_nodes.conf).burn_block_height > info.burn_block_height)
})
.unwrap();
info!(
"Bitcoin block mine time elapsed: {:?}",
mined_btc_block_time.elapsed()
);
}

/// Fetch the local signer state machine for all the signers,
Expand Down Expand Up @@ -1098,26 +1105,63 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
output
}

/// Mine a BTC block and wait for a new Stacks block to be mined
/// Mine a BTC block and wait for a new Stacks block to be mined, but do not wait for a commit
/// Note: do not use nakamoto blocks mined heuristic if running a test with multiple miners
fn mine_nakamoto_block(&self, timeout: Duration, use_nakamoto_blocks_mined: bool) {
let mined_block_time = Instant::now();
let mined_before = self.running_nodes.counters.naka_mined_blocks.get();
let info_before = self.get_peer_info();

next_block_and(
&self.running_nodes.btc_regtest_controller,
fn mine_nakamoto_block_without_commit(
&self,
timeout: Duration,
use_nakamoto_blocks_mined: bool,
) {
let info_before = get_chain_info(&self.running_nodes.conf);
info!("Pausing stacks block mining");
TEST_MINE_SKIP.set(true);
let mined_blocks = self.running_nodes.counters.naka_mined_blocks.clone();
let mined_before = mined_blocks.get();
self.mine_bitcoin_block();
wait_for_state_machine_update_by_miner_tenure_id(
timeout.as_secs(),
|| {
let info_after = self.get_peer_info();
let blocks_mined = self.running_nodes.counters.naka_mined_blocks.get();
Ok(info_after.stacks_tip_height > info_before.stacks_tip_height
&& (!use_nakamoto_blocks_mined || blocks_mined > mined_before))
},
&get_chain_info(&self.running_nodes.conf).pox_consensus,
&self.signer_addresses_versions_majority(),
)
.unwrap();
let mined_block_elapsed_time = mined_block_time.elapsed();
info!("Nakamoto block mine time elapsed: {mined_block_elapsed_time:?}");
.expect("Failed to update signer state machine");

info!("Unpausing stacks block mining");
let mined_block_time = Instant::now();
TEST_MINE_SKIP.set(false);
// Do these wait for's in two steps not only for increased timeout but for easier debugging.
// Ensure that the tenure change transaction is mined
wait_for(timeout.as_secs(), || {
Ok(get_chain_info(&self.running_nodes.conf).stacks_tip_height
> info_before.stacks_tip_height
&& (!use_nakamoto_blocks_mined || mined_blocks.get() > mined_before))
})
.expect("Failed to mine Tenure Change block");
info!(
"Nakamoto block mine time elapsed: {:?}",
mined_block_time.elapsed()
);
}

/// Mine a BTC block and wait for a new Stacks block to be mined and commit to be submitted
/// Note: do not use nakamoto blocks mined heuristic if running a test with multiple miners
fn mine_nakamoto_block(&self, timeout: Duration, use_nakamoto_blocks_mined: bool) {
let Counters {
naka_submitted_commits: commits_submitted,
naka_submitted_commit_last_burn_height: commits_last_burn_height,
naka_submitted_commit_last_stacks_tip: commits_last_stacks_tip,
..
} = self.running_nodes.counters.clone();
let commits_before = commits_submitted.get();
let commit_burn_height_before = commits_last_burn_height.get();
self.mine_nakamoto_block_without_commit(timeout, use_nakamoto_blocks_mined);
// Ensure the subsequent block commit confirms the previous Tenure Change block
let stacks_tip_height = get_chain_info(&self.running_nodes.conf).stacks_tip_height;
wait_for(timeout.as_secs(), || {
Ok(commits_submitted.get() > commits_before
&& commits_last_burn_height.get() > commit_burn_height_before
&& commits_last_stacks_tip.get() >= stacks_tip_height)
})
.expect("Failed to update Block Commit");
}

fn mine_block_wait_on_processing(
Expand Down Expand Up @@ -1343,7 +1387,7 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
.collect()
}

/// Get the signer addresses and corresponding versions
/// Get the signer addresses and corresponding versions configured versions
pub fn signer_addresses_versions(&self) -> Vec<(StacksAddress, u64)> {
self.signer_stacks_private_keys
.iter()
Expand All @@ -1357,6 +1401,33 @@ impl<Z: SpawnedSignerTrait> SignerTest<Z> {
.collect()
}

/// Get the signer addresses and corresponding majority versions
pub fn signer_addresses_versions_majority(&self) -> Vec<(StacksAddress, u64)> {
let mut signer_address_versions = self.signer_addresses_versions();
let majority = (signer_address_versions.len() * 7 / 10) as u64;
let mut protocol_versions = HashMap::new();
for (_, version) in &self.signer_addresses_versions() {
let entry = protocol_versions.entry(*version).or_insert_with(|| 0);
*entry += 1;
}

// find the highest version number supported by a threshold number of signers
let mut protocol_versions: Vec<_> = protocol_versions.into_iter().collect();
protocol_versions.sort_by_key(|(version, _)| *version);
let mut total_weight_support = 0;
for (version, weight_support) in protocol_versions.into_iter().rev() {
total_weight_support += weight_support;
if total_weight_support > majority {
// We need to actually overwrite the versions passed in since the signers will go with the majority value if they can
signer_address_versions
.iter_mut()
.for_each(|(_, v)| *v = version);
break;
}
}
signer_address_versions
}

/// 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);
Expand Down
Loading
Loading