Skip to content

Commit

Permalink
chore: convert BlockAccepted to a struct
Browse files Browse the repository at this point in the history
  • Loading branch information
@hstove
hstove committed Oct 18, 2024
1 parent 3d1f829 commit 540fcd4
Show file tree
Hide file tree
Showing 5 changed files with 142 additions and 62 deletions.
107 changes: 91 additions & 16 deletions libsigner/src/v0/messages.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ impl From<&BlockResponse> for BlockResponseTypePrefix {
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub enum BlockResponse {
/// The Nakamoto block was accepted and therefore signed
Accepted((Sha512Trunc256Sum, MessageSignature)),
Accepted(BlockAccepted),
/// The Nakamoto block was rejected and therefore not signed
Rejected(BlockRejection),
}
Expand All @@ -616,7 +616,7 @@ impl std::fmt::Display for BlockResponse {
write!(
f,
"BlockAccepted: signer_sighash = {}, signature = {}",
a.0, a.1
a.signer_signature_hash, a.signature
)
}
BlockResponse::Rejected(r) => {
Expand All @@ -633,7 +633,10 @@ impl std::fmt::Display for BlockResponse {
impl BlockResponse {
/// Create a new accepted BlockResponse for the provided block signer signature hash and signature
pub fn accepted(hash: Sha512Trunc256Sum, sig: MessageSignature) -> Self {
Self::Accepted((hash, sig))
Self::Accepted(BlockAccepted {
signer_signature_hash: hash,
signature: sig,
})
}

/// Create a new rejected BlockResponse for the provided block signer signature hash and rejection code and sign it with the provided private key
Expand All @@ -651,9 +654,8 @@ impl StacksMessageCodec for BlockResponse {
fn consensus_serialize<W: Write>(&self, fd: &mut W) -> Result<(), CodecError> {
write_next(fd, &(BlockResponseTypePrefix::from(self) as u8))?;
match self {
BlockResponse::Accepted((hash, sig)) => {
write_next(fd, hash)?;
write_next(fd, sig)?;
BlockResponse::Accepted(accepted) => {
write_next(fd, accepted)?;
}
BlockResponse::Rejected(rejection) => {
write_next(fd, rejection)?;
Expand All @@ -667,9 +669,8 @@ impl StacksMessageCodec for BlockResponse {
let type_prefix = BlockResponseTypePrefix::try_from(type_prefix_byte)?;
let response = match type_prefix {
BlockResponseTypePrefix::Accepted => {
let hash = read_next::<Sha512Trunc256Sum, _>(fd)?;
let sig = read_next::<MessageSignature, _>(fd)?;
BlockResponse::Accepted((hash, sig))
let accepted = read_next::<BlockAccepted, _>(fd)?;
BlockResponse::Accepted(accepted)
}
BlockResponseTypePrefix::Rejected => {
let rejection = read_next::<BlockRejection, _>(fd)?;
Expand All @@ -680,6 +681,32 @@ impl StacksMessageCodec for BlockResponse {
}
}

/// A rejection response from a signer for a proposed block
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct BlockAccepted {
/// The signer signature hash of the block that was accepted
pub signer_signature_hash: Sha512Trunc256Sum,
/// The signer's signature across the acceptance
pub signature: MessageSignature,
}

impl StacksMessageCodec for BlockAccepted {
fn consensus_serialize<W: Write>(&self, fd: &mut W) -> Result<(), CodecError> {
write_next(fd, &self.signer_signature_hash)?;
write_next(fd, &self.signature)?;
Ok(())
}

fn consensus_deserialize<R: Read>(fd: &mut R) -> Result<Self, CodecError> {
let signer_signature_hash = read_next::<Sha512Trunc256Sum, _>(fd)?;
let signature = read_next::<MessageSignature, _>(fd)?;
Ok(Self {
signer_signature_hash,
signature,
})
}
}

/// A rejection response from a signer for a proposed block
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct BlockRejection {
Expand Down Expand Up @@ -894,7 +921,7 @@ mod test {
use clarity::consts::CHAIN_ID_MAINNET;
use clarity::types::chainstate::{ConsensusHash, StacksBlockId, TrieHash};
use clarity::types::PrivateKey;
use clarity::util::hash::MerkleTree;
use clarity::util::hash::{hex_bytes, MerkleTree};
use clarity::util::secp256k1::MessageSignature;
use rand::rngs::mock;
use rand::{thread_rng, Rng, RngCore};
Expand Down Expand Up @@ -958,8 +985,11 @@ mod test {

#[test]
fn serde_block_response() {
let response =
BlockResponse::Accepted((Sha512Trunc256Sum([0u8; 32]), MessageSignature::empty()));
let accepted = BlockAccepted {
signer_signature_hash: Sha512Trunc256Sum([0u8; 32]),
signature: MessageSignature::empty(),
};
let response = BlockResponse::Accepted(accepted);
let serialized_response = response.serialize_to_vec();
let deserialized_response = read_next::<BlockResponse, _>(&mut &serialized_response[..])
.expect("Failed to deserialize BlockResponse");
Expand All @@ -979,10 +1009,11 @@ mod test {

#[test]
fn serde_signer_message() {
let signer_message = SignerMessage::BlockResponse(BlockResponse::Accepted((
Sha512Trunc256Sum([2u8; 32]),
MessageSignature::empty(),
)));
let accepted = BlockAccepted {
signer_signature_hash: Sha512Trunc256Sum([2u8; 32]),
signature: MessageSignature::empty(),
};
let signer_message = SignerMessage::BlockResponse(BlockResponse::Accepted(accepted));
let serialized_signer_message = signer_message.serialize_to_vec();
let deserialized_signer_message =
read_next::<SignerMessage, _>(&mut &serialized_signer_message[..])
Expand Down Expand Up @@ -1122,4 +1153,48 @@ mod test {
.expect("Failed to deserialize MockSignData");
assert_eq!(mock_block, deserialized_data);
}

#[test]
fn test_backwards_compatibility() {
let block_rejected_hex = "010100000050426c6f636b206973206e6f7420612074656e7572652d737461727420626c6f636b2c20616e642068617320616e20756e7265636f676e697a65642074656e75726520636f6e73656e7375732068617368000691f95f84b7045f7dce7757052caa986ef042cb58f7df5031a3b5b5d0e3dda63e80000000006fb349212e1a1af1a3c712878d5159b5ec14636adb6f70be00a6da4ad4f88a9934d8a9abb229620dd8e0f225d63401e36c64817fb29e6c05591dcbe95c512df3";
let block_rejected_bytes = hex_bytes(&block_rejected_hex).unwrap();
let block_accepted_hex = "010011717149677c2ac97d15ae5954f7a716f10100b9cb81a2bf27551b2f2e54ef19001c694f8134c5c90f2f2bcd330e9f423204884f001b5df0050f36a2c4ff79dd93522bb2ae395ea87de4964886447507c18374b7a46ee2e371e9bf332f0706a3e8";
let block_accepted_bytes = hex_bytes(&block_accepted_hex).unwrap();
let block_rejected = read_next::<SignerMessage, _>(&mut &block_rejected_bytes[..])
.expect("Failed to deserialize BlockRejection");
let block_accepted = read_next::<SignerMessage, _>(&mut &block_accepted_bytes[..])
.expect("Failed to deserialize BlockRejection");

assert_eq!(
block_rejected,
SignerMessage::BlockResponse(BlockResponse::Rejected(BlockRejection {
reason_code: RejectCode::ValidationFailed(ValidateRejectCode::NoSuchTenure),
reason: "Block is not a tenure-start block, and has an unrecognized tenure consensus hash".to_string(),
signer_signature_hash: Sha512Trunc256Sum([145, 249, 95, 132, 183, 4, 95, 125, 206, 119, 87, 5, 44, 170, 152, 110, 240, 66, 203, 88, 247, 223, 80, 49, 163, 181, 181, 208, 227, 221, 166, 62]),
chain_id: CHAIN_ID_TESTNET,
signature: MessageSignature([
0, 111, 179, 73, 33, 46, 26, 26, 241, 163, 199, 18, 135, 141, 81, 89, 181, 236,
20, 99, 106, 219, 111, 112, 190, 0, 166, 218, 74, 212, 248, 138, 153, 52, 216,
169, 171, 178, 41, 98, 13, 216, 224, 242, 37, 214, 52, 1, 227, 108, 100, 129,
127, 178, 158, 108, 5, 89, 29, 203, 233, 92, 81, 45, 243,
]),
}))
);

assert_eq!(
block_accepted,
SignerMessage::BlockResponse(BlockResponse::Accepted(BlockAccepted {
signer_signature_hash: Sha512Trunc256Sum([
17, 113, 113, 73, 103, 124, 42, 201, 125, 21, 174, 89, 84, 247, 167, 22, 241,
1, 0, 185, 203, 129, 162, 191, 39, 85, 27, 47, 46, 84, 239, 25
]),
signature: MessageSignature([
0, 28, 105, 79, 129, 52, 197, 201, 15, 47, 43, 205, 51, 14, 159, 66, 50, 4,
136, 79, 0, 27, 93, 240, 5, 15, 54, 162, 196, 255, 121, 221, 147, 82, 43, 178,
174, 57, 94, 168, 125, 228, 150, 72, 134, 68, 117, 7, 193, 131, 116, 183, 164,
110, 226, 227, 113, 233, 191, 51, 47, 7, 6, 163, 232
]),
}))
);
}
}
32 changes: 15 additions & 17 deletions stacks-signer/src/v0/signer.rs
View file
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,13 @@ impl Signer {
self.signer_db
.insert_block(&block_info)
.unwrap_or_else(|_| panic!("{self}: Failed to insert block in DB"));
let accepted = BlockAccepted {
signer_signature_hash: 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,12 +738,11 @@ impl Signer {
}

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

// Have we already processed this block?
Expand Down
13 changes: 8 additions & 5 deletions testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs
View file
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,11 @@ impl SignCoordinator {
}

match message {
SignerMessageV0::BlockResponse(BlockResponse::Accepted((
response_hash,
signature,
))) => {
SignerMessageV0::BlockResponse(BlockResponse::Accepted(accepted)) => {
let BlockAccepted {
signer_signature_hash: response_hash,
signature,
} = accepted;
let block_sighash = block.header.signer_signature_hash();
if block_sighash != response_hash {
warn!(
Expand Down
11 changes: 6 additions & 5 deletions testnet/stacks-node/src/tests/signer/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ use std::time::{Duration, Instant};

use clarity::boot_util::boot_code_id;
use clarity::vm::types::PrincipalData;
use libsigner::v0::messages::{BlockResponse, SignerMessage};
use libsigner::v0::messages::{BlockAccepted, BlockResponse, SignerMessage};
use libsigner::{SignerEntries, SignerEventTrait};
use stacks::chainstate::coordinator::comm::CoordinatorChannels;
use stacks::chainstate::nakamoto::signer_set::NakamotoSigners;
Expand Down Expand Up @@ -578,10 +578,11 @@ 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,
))) => {
SignerMessage::BlockResponse(BlockResponse::Accepted(accepted)) => {
let BlockAccepted {
signer_signature_hash: hash,
signature,
} = accepted;
if hash == *signer_signature_hash
&& expected_signers.iter().any(|pk| {
pk.verify(hash.bits(), &signature)
Expand Down
41 changes: 22 additions & 19 deletions testnet/stacks-node/src/tests/signer/v0.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3411,7 +3411,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 +3425,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 +4655,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 +4900,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

0 comments on commit 540fcd4

Please sign in to comment.