Skip to content
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

Add version to signer messages #5336

Merged
merged 4 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
10 changes: 10 additions & 0 deletions libsigner/src/libsigner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,10 @@ use std::cmp::Eq;
use std::fmt::Debug;
use std::hash::Hash;

use blockstack_lib::version_string;
use clarity::codec::StacksMessageCodec;
use clarity::vm::types::QualifiedContractIdentifier;
use lazy_static::lazy_static;

pub use crate::error::{EventError, RPCError};
pub use crate::events::{
Expand All @@ -74,3 +76,11 @@ pub trait SignerMessage<T: MessageSlotID>: StacksMessageCodec {
/// The contract identifier for the message slot in stacker db
fn msg_id(&self) -> Option<T>;
}

lazy_static! {
/// The version string for the signer
pub static ref VERSION_STRING: String = {
let pkg_version = option_env!("STACKS_NODE_VERSION").unwrap_or(env!("CARGO_PKG_VERSION"));
version_string("stacks-signer", pkg_version)
};
}
260 changes: 240 additions & 20 deletions libsigner/src/v0/messages.rs

Large diffs are not rendered by default.

3 changes: 1 addition & 2 deletions stacks-signer/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use clarity::util::hash::Sha256Sum;
use clarity::util::secp256k1::MessageSignature;
use clarity::vm::types::{QualifiedContractIdentifier, TupleData};
use clarity::vm::Value;
use libsigner::VERSION_STRING;
use serde::{Deserialize, Serialize};
use stacks_common::address::{
b58, AddressHashMode, C32_ADDRESS_VERSION_MAINNET_MULTISIG,
Expand All @@ -38,8 +39,6 @@ use stacks_common::address::{
use stacks_common::define_u8_enum;
use stacks_common::types::chainstate::StacksPrivateKey;

use crate::VERSION_STRING;

extern crate alloc;

#[derive(Parser, Debug)]
Expand Down
5 changes: 4 additions & 1 deletion stacks-signer/src/client/stackerdb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,9 @@ mod tests {
use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader};
use clarity::util::hash::{MerkleTree, Sha512Trunc256Sum};
use clarity::util::secp256k1::MessageSignature;
use libsigner::v0::messages::{BlockRejection, BlockResponse, RejectCode, SignerMessage};
use libsigner::v0::messages::{
BlockRejection, BlockResponse, RejectCode, SignerMessage, SignerMessageMetadata,
};
use rand::{thread_rng, RngCore};

use super::*;
Expand Down Expand Up @@ -283,6 +285,7 @@ mod tests {
signer_signature_hash: block.header.signer_signature_hash(),
chain_id: thread_rng().next_u32(),
signature: MessageSignature::empty(),
metadata: SignerMessageMetadata::empty(),
};
let signer_message = SignerMessage::BlockResponse(BlockResponse::Rejected(block_reject));
let ack = StackerDBChunkAckData {
Expand Down
12 changes: 1 addition & 11 deletions stacks-signer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,9 @@ mod tests;
use std::fmt::{Debug, Display};
use std::sync::mpsc::{channel, Receiver, Sender};

use blockstack_lib::version_string;
use chainstate::SortitionsView;
use config::GlobalConfig;
use lazy_static::lazy_static;
use libsigner::{SignerEvent, SignerEventReceiver, SignerEventTrait};
use libsigner::{SignerEvent, SignerEventReceiver, SignerEventTrait, VERSION_STRING};
use runloop::SignerResult;
use slog::{slog_info, slog_warn};
use stacks_common::{info, warn};
Expand All @@ -61,14 +59,6 @@ use crate::client::StacksClient;
use crate::config::SignerConfig;
use crate::runloop::RunLoop;

lazy_static! {
/// The version string for the signer
pub static ref VERSION_STRING: String = {
let pkg_version = option_env!("STACKS_NODE_VERSION").unwrap_or(env!("CARGO_PKG_VERSION"));
version_string("stacks-signer", pkg_version)
};
}

/// A trait which provides a common `Signer` interface for `v0` and `v1`
pub trait Signer<T: SignerEventTrait>: Debug + Display {
/// Create a new `Signer` instance
Expand Down
3 changes: 1 addition & 2 deletions stacks-signer/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use blockstack_lib::util_lib::signed_structured_data::pox4::make_pox_4_signer_ke
use clap::Parser;
use clarity::types::chainstate::StacksPublicKey;
use clarity::util::sleep_ms;
use libsigner::SignerSession;
use libsigner::{SignerSession, VERSION_STRING};
use libstackerdb::StackerDBChunkData;
use slog::{slog_debug, slog_error};
use stacks_common::util::hash::to_hex;
Expand All @@ -47,7 +47,6 @@ use stacks_signer::config::GlobalConfig;
use stacks_signer::monitor_signers::SignerMonitor;
use stacks_signer::utils::stackerdb_session;
use stacks_signer::v0::SpawnedSigner;
use stacks_signer::VERSION_STRING;
use tracing_subscriber::prelude::*;
use tracing_subscriber::{fmt, EnvFilter};

Expand Down
2 changes: 1 addition & 1 deletion stacks-signer/src/monitoring/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::time::Instant;

use clarity::util::hash::to_hex;
use clarity::util::secp256k1::Secp256k1PublicKey;
use libsigner::VERSION_STRING;
use slog::{slog_debug, slog_error, slog_info, slog_warn};
use stacks_common::{debug, error, info, warn};
use tiny_http::{Response as HttpResponse, Server as HttpServer};
Expand All @@ -28,7 +29,6 @@ use crate::client::{ClientError, StacksClient};
use crate::config::{GlobalConfig, Network};
use crate::monitoring::prometheus::gather_metrics_string;
use crate::monitoring::{update_signer_nonce, update_stacks_tip_height};
use crate::VERSION_STRING;

#[derive(thiserror::Error, Debug)]
/// Monitoring server errors
Expand Down
35 changes: 17 additions & 18 deletions stacks-signer/src/v0/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@ use clarity::types::{PrivateKey, StacksEpochId};
use clarity::util::hash::MerkleHashFunc;
use clarity::util::secp256k1::Secp256k1PublicKey;
use libsigner::v0::messages::{
BlockRejection, BlockResponse, MessageSlotID, MockProposal, MockSignature, RejectCode,
SignerMessage,
BlockAccepted, BlockRejection, BlockResponse, MessageSlotID, MockProposal, MockSignature,
RejectCode, SignerMessage,
};
use libsigner::{BlockProposal, SignerEvent};
use slog::{slog_debug, slog_error, slog_info, slog_warn};
use stacks_common::types::chainstate::StacksAddress;
use stacks_common::util::get_epoch_time_secs;
use stacks_common::util::hash::Sha512Trunc256Sum;
use stacks_common::util::secp256k1::MessageSignature;
use stacks_common::{debug, error, info, warn};

Expand Down Expand Up @@ -494,8 +493,8 @@ impl Signer {
block_response: &BlockResponse,
) {
match block_response {
BlockResponse::Accepted((block_hash, signature)) => {
self.handle_block_signature(stacks_client, block_hash, signature);
BlockResponse::Accepted(accepted) => {
self.handle_block_signature(stacks_client, accepted);
}
BlockResponse::Rejected(block_rejection) => {
self.handle_block_rejection(block_rejection);
Expand Down Expand Up @@ -547,13 +546,10 @@ impl Signer {
self.signer_db
.insert_block(&block_info)
.unwrap_or_else(|_| panic!("{self}: Failed to insert block in DB"));
let accepted = BlockAccepted::new(block_info.signer_signature_hash(), signature);
// have to save the signature _after_ the block info
self.handle_block_signature(
stacks_client,
&block_info.signer_signature_hash(),
&signature,
);
Some(BlockResponse::accepted(signer_signature_hash, signature))
self.handle_block_signature(stacks_client, &accepted);
Some(BlockResponse::Accepted(accepted))
}

/// Handle the block validate reject response. Returns our block response if we have one
Expand Down Expand Up @@ -739,13 +735,16 @@ impl Signer {
}

/// Handle an observed signature from another signer
fn handle_block_signature(
&mut self,
stacks_client: &StacksClient,
block_hash: &Sha512Trunc256Sum,
signature: &MessageSignature,
) {
debug!("{self}: Received a block-accept signature: ({block_hash}, {signature})");
fn handle_block_signature(&mut self, stacks_client: &StacksClient, accepted: &BlockAccepted) {
let BlockAccepted {
signer_signature_hash: block_hash,
signature,
metadata,
} = accepted;
debug!(
"{self}: Received a block-accept signature: ({block_hash}, {signature}, {})",
metadata.server_version
);

// Have we already processed this block?
match self
Expand Down
20 changes: 13 additions & 7 deletions testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ use std::sync::Arc;
use std::time::Duration;

use hashbrown::{HashMap, HashSet};
use libsigner::v0::messages::{BlockResponse, MinerSlotID, SignerMessage as SignerMessageV0};
use libsigner::v0::messages::{
BlockAccepted, BlockResponse, MinerSlotID, SignerMessage as SignerMessageV0,
};
use libsigner::{BlockProposal, SignerEntries, SignerEvent, SignerSession, StackerDBSession};
use stacks::burnchains::Burnchain;
use stacks::chainstate::burn::db::sortdb::SortitionDB;
Expand Down Expand Up @@ -450,10 +452,12 @@ impl SignCoordinator {
}

match message {
SignerMessageV0::BlockResponse(BlockResponse::Accepted((
response_hash,
signature,
))) => {
SignerMessageV0::BlockResponse(BlockResponse::Accepted(accepted)) => {
let BlockAccepted {
signer_signature_hash: response_hash,
signature,
metadata,
} = accepted;
let block_sighash = block.header.signer_signature_hash();
if block_sighash != response_hash {
warn!(
Expand All @@ -463,7 +467,8 @@ impl SignCoordinator {
"response_hash" => %response_hash,
"slot_id" => slot_id,
"reward_cycle_id" => reward_cycle_id,
"response_hash" => %response_hash
"response_hash" => %response_hash,
"server_version" => %metadata.server_version
);
continue;
}
Expand Down Expand Up @@ -511,7 +516,8 @@ impl SignCoordinator {
"signer_weight" => signer_entry.weight,
"total_weight_signed" => total_weight_signed,
"stacks_block_hash" => %block.header.block_hash(),
"stacks_block_id" => %block.header.block_id()
"stacks_block_id" => %block.header.block_id(),
"server_version" => metadata.server_version,
);
gathered_signatures.insert(slot_id, signature);
responded_signers.insert(signer_pubkey);
Expand Down
16 changes: 8 additions & 8 deletions testnet/stacks-node/src/tests/signer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -578,17 +578,17 @@ impl<S: Signer<T> + Send + 'static, T: SignerEventTrait + 'static> SignerTest<Sp
let message = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice())
.expect("Failed to deserialize SignerMessage");
match message {
SignerMessage::BlockResponse(BlockResponse::Accepted((
hash,
signature,
))) => {
if hash == *signer_signature_hash
SignerMessage::BlockResponse(BlockResponse::Accepted(accepted)) => {
if accepted.signer_signature_hash == *signer_signature_hash
&& expected_signers.iter().any(|pk| {
pk.verify(hash.bits(), &signature)
.expect("Failed to verify signature")
pk.verify(
accepted.signer_signature_hash.bits(),
&accepted.signature,
)
.expect("Failed to verify signature")
})
{
Some(signature)
Some(accepted.signature)
} else {
None
}
Expand Down
45 changes: 25 additions & 20 deletions testnet/stacks-node/src/tests/signer/v0.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use clarity::vm::StacksEpoch;
use libsigner::v0::messages::{
BlockRejection, BlockResponse, MessageSlotID, MinerSlotID, RejectCode, SignerMessage,
};
use libsigner::{BlockProposal, SignerSession, StackerDBSession};
use libsigner::{BlockProposal, SignerSession, StackerDBSession, VERSION_STRING};
use stacks::address::AddressHashMode;
use stacks::burnchains::Txid;
use stacks::chainstate::burn::db::sortdb::SortitionDB;
Expand Down Expand Up @@ -2567,10 +2567,12 @@ fn empty_sortition() {
};
if let SignerMessage::BlockResponse(BlockResponse::Rejected(BlockRejection {
reason_code,
metadata,
..
})) = latest_msg
{
assert!(matches!(reason_code, RejectCode::SortitionViewMismatch));
assert_eq!(metadata.server_version, VERSION_STRING.to_string());
found_rejections.push(*slot_id);
} else {
info!("Latest message from slot #{slot_id} isn't a block rejection, will wait to see if the signer updates to a rejection");
Expand Down Expand Up @@ -3411,7 +3413,7 @@ fn duplicate_signers() {
})
.filter_map(|message| match message {
SignerMessage::BlockResponse(BlockResponse::Accepted(m)) => {
info!("Message(accepted): {message:?}");
info!("Message(accepted): {:?}", &m);
Some(m)
}
_ => {
Expand All @@ -3425,20 +3427,23 @@ fn duplicate_signers() {
info!("------------------------- Assert there are {unique_signers} unique signatures and recovered pubkeys -------------------------");

// Pick a message hash
let (selected_sighash, _) = signer_accepted_responses
let accepted = signer_accepted_responses
.iter()
.min_by_key(|(sighash, _)| *sighash)
.copied()
.min_by_key(|accepted| accepted.signer_signature_hash)
.expect("No `BlockResponse::Accepted` messages recieved");
let selected_sighash = accepted.signer_signature_hash;

// Filter only resonses for selected block and collect unique pubkeys and signatures
let (pubkeys, signatures): (HashSet<_>, HashSet<_>) = signer_accepted_responses
.into_iter()
.filter(|(hash, _)| *hash == selected_sighash)
.map(|(msg, sig)| {
let pubkey = Secp256k1PublicKey::recover_to_pubkey(msg.bits(), &sig)
.expect("Failed to recover pubkey");
(pubkey, sig)
.filter(|accepted| accepted.signer_signature_hash == selected_sighash)
.map(|accepted| {
let pubkey = Secp256k1PublicKey::recover_to_pubkey(
accepted.signer_signature_hash.bits(),
&accepted.signature,
)
.expect("Failed to recover pubkey");
(pubkey, accepted.signature)
})
.unzip();

Expand Down Expand Up @@ -4652,10 +4657,11 @@ fn reorg_locally_accepted_blocks_across_tenures_succeeds() {
let message = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice())
.expect("Failed to deserialize SignerMessage");
match message {
SignerMessage::BlockResponse(BlockResponse::Accepted((hash, signature))) => {
ignoring_signers
.iter()
.find(|key| key.verify(hash.bits(), &signature).is_ok())
SignerMessage::BlockResponse(BlockResponse::Accepted(accepted)) => {
ignoring_signers.iter().find(|key| {
key.verify(accepted.signer_signature_hash.bits(), &accepted.signature)
.is_ok()
})
}
_ => None,
}
Expand Down Expand Up @@ -4896,12 +4902,11 @@ fn miner_recovers_when_broadcast_block_delay_across_tenures_occurs() {
let message = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice())
.expect("Failed to deserialize SignerMessage");
match message {
SignerMessage::BlockResponse(BlockResponse::Accepted((
hash,
signature,
))) => {
if block.header.signer_signature_hash() == hash {
Some(signature)
SignerMessage::BlockResponse(BlockResponse::Accepted(accepted)) => {
if block.header.signer_signature_hash()
== accepted.signer_signature_hash
{
Some(accepted.signature)
} else {
None
}
Expand Down