From 167ed42cc4249b1ed9408c6ba9303d0d825cbbde Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Thu, 18 Apr 2024 10:48:34 -0700 Subject: [PATCH] Remove redundant block proposal message and fix tests Signed-off-by: Jacinta Ferrant --- libsigner/src/events.rs | 35 +---- stacks-signer/src/main.rs | 20 ++- stacks-signer/src/signer.rs | 146 ++++++------------ stacks-signer/src/signerdb.rs | 96 +++++++----- stackslib/src/chainstate/nakamoto/miner.rs | 37 ----- .../src/chainstate/nakamoto/tests/mod.rs | 36 ++--- stackslib/src/net/stackerdb/mod.rs | 2 +- .../stacks-node/src/nakamoto_node/miner.rs | 108 +------------ .../src/nakamoto_node/sign_coordinator.rs | 58 ++++++- .../src/tests/nakamoto_integrations.rs | 39 +++-- testnet/stacks-node/src/tests/signer.rs | 18 ++- 11 files changed, 247 insertions(+), 348 deletions(-) diff --git a/libsigner/src/events.rs b/libsigner/src/events.rs index 2d156559f..67eb97057 100644 --- a/libsigner/src/events.rs +++ b/libsigner/src/events.rs @@ -68,14 +68,9 @@ pub struct BlockProposalSigners { #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub enum SignerEvent { /// A miner sent a message over .miners - /// The `Vec` will contain any block proposals made by the miner during this StackerDB event. /// The `Vec` will contain any signer WSTS messages made by the miner while acting as a coordinator. - /// The `Option` will contain the message sender's public key if either of the vecs is non-empty. - MinerMessages( - Vec, - Vec, - Option, - ), + /// The `Option` will contain the message sender's public key if the vec is non-empty. + MinerMessages(Vec, Option), /// The signer messages for other signers and miners to observe /// The u32 is the signer set to which the message belongs (either 0 or 1) SignerMessages(u32, Vec), @@ -417,7 +412,6 @@ impl TryFrom for SignerEvent { let signer_event = if event.contract_id.name.as_str() == MINERS_NAME && event.contract_id.is_boot() { - let mut blocks = vec![]; let mut messages = vec![]; let mut miner_pk = None; for chunk in event.modified_slots { @@ -426,28 +420,13 @@ impl TryFrom for SignerEvent { "Failed to recover PK from StackerDB chunk: {e}" )) })?); - if chunk.slot_id % MINER_SLOT_COUNT == 0 { - // block - let Ok(block) = - BlockProposalSigners::consensus_deserialize(&mut chunk.data.as_slice()) - else { - continue; - }; - blocks.push(block); - } else if chunk.slot_id % MINER_SLOT_COUNT == 1 { - // message - let Ok(msg) = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()) - else { - continue; - }; - messages.push(msg); - } else { - return Err(EventError::UnrecognizedEvent( - "Unrecognized slot_id for miners contract".into(), - )); + let Ok(msg) = SignerMessage::consensus_deserialize(&mut chunk.data.as_slice()) + else { + continue; }; + messages.push(msg); } - SignerEvent::MinerMessages(blocks, messages, miner_pk) + SignerEvent::MinerMessages(messages, miner_pk) } else if event.contract_id.name.starts_with(SIGNERS_NAME) && event.contract_id.is_boot() { let Some((signer_set, _)) = get_signers_db_signer_set_message_id(event.contract_id.name.as_str()) diff --git a/stacks-signer/src/main.rs b/stacks-signer/src/main.rs index c7f3fcd68..09337c44a 100644 --- a/stacks-signer/src/main.rs +++ b/stacks-signer/src/main.rs @@ -32,11 +32,13 @@ use std::path::{Path, PathBuf}; use std::sync::mpsc::{channel, Receiver, Sender}; use std::time::Duration; -use blockstack_lib::chainstate::nakamoto::NakamotoBlock; use blockstack_lib::util_lib::signed_structured_data::pox4::make_pox_4_signer_key_signature; use clap::Parser; use clarity::vm::types::QualifiedContractIdentifier; -use libsigner::{RunningSigner, Signer, SignerEventReceiver, SignerSession, StackerDBSession}; +use libsigner::{ + BlockProposalSigners, RunningSigner, Signer, SignerEventReceiver, SignerSession, + StackerDBSession, +}; use libstackerdb::StackerDBChunkData; use slog::{slog_debug, slog_error, slog_info}; use stacks_common::codec::read_next; @@ -208,15 +210,16 @@ fn handle_dkg(args: RunDkgArgs) { fn handle_sign(args: SignArgs) { debug!("Signing message..."); let spawned_signer = spawn_running_signer(&args.config); - let Some(block) = read_next::(&mut &args.data[..]).ok() else { - error!("Unable to parse provided message as a NakamotoBlock."); + let Some(block_proposal) = read_next::(&mut &args.data[..]).ok() + else { + error!("Unable to parse provided message as a BlockProposalSigners."); spawned_signer.running_signer.stop(); return; }; let sign_command = RunLoopCommand { reward_cycle: args.reward_cycle, command: SignerCommand::Sign { - block, + block_proposal, is_taproot: false, merkle_root: None, }, @@ -230,8 +233,9 @@ fn handle_sign(args: SignArgs) { fn handle_dkg_sign(args: SignArgs) { debug!("Running DKG and signing message..."); let spawned_signer = spawn_running_signer(&args.config); - let Some(block) = read_next::(&mut &args.data[..]).ok() else { - error!("Unable to parse provided message as a NakamotoBlock."); + let Some(block_proposal) = read_next::(&mut &args.data[..]).ok() + else { + error!("Unable to parse provided message as a BlockProposalSigners."); spawned_signer.running_signer.stop(); return; }; @@ -242,7 +246,7 @@ fn handle_dkg_sign(args: SignArgs) { let sign_command = RunLoopCommand { reward_cycle: args.reward_cycle, command: SignerCommand::Sign { - block, + block_proposal, is_taproot: false, merkle_root: None, }, diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index e7c7adb4c..c3dabb6ba 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -72,6 +72,10 @@ impl std::fmt::Display for SignerSlotID { pub struct BlockInfo { /// The block we are considering pub block: NakamotoBlock, + /// The burn block height at which the block was proposed + pub burn_block_height: u64, + /// The reward cycle the block belongs to + pub reward_cycle: u64, /// Our vote on the block if we have one yet pub vote: Option, /// Whether the block contents are valid @@ -82,27 +86,29 @@ pub struct BlockInfo { pub signed_over: bool, } -impl BlockInfo { - /// Create a new BlockInfo - pub const fn new(block: NakamotoBlock) -> Self { +impl From for BlockInfo { + fn from(value: BlockProposalSigners) -> Self { Self { - block, + block: value.block, + burn_block_height: value.burn_height, + reward_cycle: value.reward_cycle, vote: None, valid: None, nonce_request: None, signed_over: false, } } - +} +impl BlockInfo { /// Create a new BlockInfo with an associated nonce request packet - pub const fn new_with_request(block: NakamotoBlock, nonce_request: NonceRequest) -> Self { - Self { - block, - vote: None, - valid: None, - nonce_request: Some(nonce_request), - signed_over: true, - } + pub fn new_with_request( + block_proposal: BlockProposalSigners, + nonce_request: NonceRequest, + ) -> Self { + let mut block_info = BlockInfo::from(block_proposal); + block_info.nonce_request = Some(nonce_request); + block_info.signed_over = true; + block_info } /// Return the block's signer signature hash @@ -119,7 +125,7 @@ pub enum Command { /// Sign a message Sign { /// The block to sign over - block: NakamotoBlock, + block_proposal: BlockProposalSigners, /// Whether to make a taproot signature is_taproot: bool, /// Taproot merkle root @@ -399,7 +405,7 @@ impl Signer { } } Command::Sign { - block, + block_proposal, is_taproot, merkle_root, } => { @@ -407,23 +413,23 @@ impl Signer { debug!("{self}: Cannot sign a block without an approved aggregate public key. Ignore it."); return; } - let signer_signature_hash = block.header.signer_signature_hash(); + let signer_signature_hash = block_proposal.block.header.signer_signature_hash(); let mut block_info = self .signer_db .block_lookup(self.reward_cycle, &signer_signature_hash) - .unwrap_or_else(|_| Some(BlockInfo::new(block.clone()))) - .unwrap_or_else(|| BlockInfo::new(block.clone())); + .unwrap_or_else(|_| Some(BlockInfo::from(block_proposal.clone()))) + .unwrap_or_else(|| BlockInfo::from(block_proposal.clone())); if block_info.signed_over { debug!("{self}: Received a sign command for a block we are already signing over. Ignore it."); return; } info!("{self}: Signing block"; - "block_consensus_hash" => %block.header.consensus_hash, - "block_height" => block.header.chain_length, - "pre_sign_block_id" => %block.block_id(), + "block_consensus_hash" => %block_proposal.block.header.consensus_hash, + "block_height" => block_proposal.block.header.chain_length, + "pre_sign_block_id" => %block_proposal.block.block_id(), ); match self.coordinator.start_signing_round( - &block.serialize_to_vec(), + &block_proposal.serialize_to_vec(), *is_taproot, *merkle_root, ) { @@ -432,7 +438,7 @@ impl Signer { debug!("{self}: ACK: {ack:?}",); block_info.signed_over = true; self.signer_db - .insert_block(self.reward_cycle, &block_info) + .insert_block(&block_info) .unwrap_or_else(|e| { error!("{self}: Failed to insert block in DB: {e:?}"); }); @@ -517,7 +523,7 @@ impl Signer { let is_valid = self.verify_block_transactions(stacks_client, &block_info.block); block_info.valid = Some(is_valid); self.signer_db - .insert_block(self.reward_cycle, &block_info) + .insert_block(&block_info) .unwrap_or_else(|_| panic!("{self}: Failed to insert block in DB")); info!( "{self}: Treating block validation for block {} as valid: {:?}", @@ -574,7 +580,7 @@ impl Signer { "signed_over" => block_info.signed_over, ); self.signer_db - .insert_block(self.reward_cycle, &block_info) + .insert_block(&block_info) .unwrap_or_else(|_| panic!("{self}: Failed to insert block in DB")); } @@ -607,63 +613,6 @@ impl Signer { self.handle_packets(stacks_client, res, &packets, current_reward_cycle); } - /// Handle proposed blocks submitted by the miners to stackerdb - fn handle_proposed_blocks( - &mut self, - stacks_client: &StacksClient, - proposals: &[BlockProposalSigners], - ) { - for proposal in proposals { - if proposal.reward_cycle != self.reward_cycle { - debug!( - "{self}: Received proposal for block outside of my reward cycle, ignoring."; - "proposal_reward_cycle" => proposal.reward_cycle, - "proposal_burn_height" => proposal.burn_height, - ); - continue; - } - let sig_hash = proposal.block.header.signer_signature_hash(); - match self.signer_db.block_lookup(self.reward_cycle, &sig_hash) { - Ok(Some(block)) => { - debug!( - "{self}: Received proposal for block already known, ignoring new proposal."; - "signer_sighash" => %sig_hash, - "proposal_burn_height" => proposal.burn_height, - "vote" => ?block.vote.as_ref().map(|v| { - if v.rejected { - "REJECT" - } else { - "ACCEPT" - } - }), - "signed_over" => block.signed_over, - ); - continue; - } - Ok(None) => { - // Store the block in our cache - self.signer_db - .insert_block(self.reward_cycle, &BlockInfo::new(proposal.block.clone())) - .unwrap_or_else(|e| { - error!("{self}: Failed to insert block in DB: {e:?}"); - }); - // Submit the block for validation - stacks_client - .submit_block_for_validation(proposal.block.clone()) - .unwrap_or_else(|e| { - warn!("{self}: Failed to submit block for validation: {e:?}"); - }); - } - Err(e) => { - error!( - "{self}: Failed to lookup block in DB: {e:?}. Dropping proposal request." - ); - continue; - } - } - } - } - /// Helper function for determining if the provided message is a DKG specific message fn is_dkg_message(msg: &Message) -> bool { matches!( @@ -808,26 +757,35 @@ impl Signer { stacks_client: &StacksClient, nonce_request: &mut NonceRequest, ) -> Option { - let Some(block) = - NakamotoBlock::consensus_deserialize(&mut nonce_request.message.as_slice()).ok() + let Some(block_proposal) = + BlockProposalSigners::consensus_deserialize(&mut nonce_request.message.as_slice()).ok() else { - // We currently reject anything that is not a block + // We currently reject anything that is not a valid block proposal warn!("{self}: Received a nonce request for an unknown message stream. Reject it.",); return None; }; - let signer_signature_hash = block.header.signer_signature_hash(); + if block_proposal.reward_cycle != self.reward_cycle { + // We are not signing for this reward cycle. Reject the block + warn!( + "{self}: Received a nonce request for a different reward cycle. Reject it."; + "requested_reward_cycle" => block_proposal.reward_cycle, + ); + return None; + } + // TODO: could add a check to ignore an old burn block height if we know its oudated. Would require us to store the burn block height we last saw on the side. + let signer_signature_hash = block_proposal.block.header.signer_signature_hash(); let Some(mut block_info) = self .signer_db .block_lookup(self.reward_cycle, &signer_signature_hash) .expect("Failed to connect to signer DB") else { debug!( - "{self}: We have received a block sign request for a block we have not seen before. Cache the nonce request and submit the block for validation..."; - "signer_sighash" => %block.header.signer_signature_hash(), + "{self}: received a nonce request for a new block. Submit block for validation. "; + "signer_sighash" => %signer_signature_hash, ); - let block_info = BlockInfo::new_with_request(block.clone(), nonce_request.clone()); + let block_info = BlockInfo::new_with_request(block_proposal, nonce_request.clone()); stacks_client - .submit_block_for_validation(block) + .submit_block_for_validation(block_info.block.clone()) .unwrap_or_else(|e| { warn!("{self}: Failed to submit block for validation: {e:?}",); }); @@ -1000,7 +958,7 @@ impl Signer { return None; }; self.signer_db - .insert_block(self.reward_cycle, &updated_block_info) + .insert_block(&updated_block_info) .unwrap_or_else(|_| panic!("{self}: Failed to insert block in DB")); let process_request = updated_block_info.vote.is_some(); if !process_request { @@ -1533,7 +1491,7 @@ impl Signer { ); self.handle_signer_messages(stacks_client, res, messages, current_reward_cycle); } - Some(SignerEvent::MinerMessages(blocks, messages, miner_key)) => { + Some(SignerEvent::MinerMessages(messages, miner_key)) => { if let Some(miner_key) = miner_key { let miner_key = PublicKey::try_from(miner_key.to_bytes_compressed().as_slice()) .expect("FATAL: could not convert from StacksPublicKey to PublicKey"); @@ -1545,13 +1503,11 @@ impl Signer { return Ok(()); } debug!( - "{self}: Received {} block proposals and {} messages from the miner", - blocks.len(), + "{self}: Received {} messages from the miner", messages.len(); "miner_key" => ?miner_key, ); self.handle_signer_messages(stacks_client, res, messages, current_reward_cycle); - self.handle_proposed_blocks(stacks_client, blocks); } Some(SignerEvent::StatusCheck) => { debug!("{self}: Received a status check event.") diff --git a/stacks-signer/src/signerdb.rs b/stacks-signer/src/signerdb.rs index 4c3329741..20a443dea 100644 --- a/stacks-signer/src/signerdb.rs +++ b/stacks-signer/src/signerdb.rs @@ -39,6 +39,7 @@ CREATE TABLE IF NOT EXISTS blocks ( reward_cycle INTEGER NOT NULL, signer_signature_hash TEXT NOT NULL, block_info TEXT NOT NULL, + burn_block_height INTEGER NOT NULL, PRIMARY KEY (reward_cycle, signer_signature_hash) )"; @@ -125,30 +126,29 @@ impl SignerDb { /// Insert a block into the database. /// `hash` is the `signer_signature_hash` of the block. - pub fn insert_block( - &mut self, - reward_cycle: u64, - block_info: &BlockInfo, - ) -> Result<(), DBError> { + pub fn insert_block(&mut self, block_info: &BlockInfo) -> Result<(), DBError> { let block_json = serde_json::to_string(&block_info).expect("Unable to serialize block info"); let hash = &block_info.signer_signature_hash(); let block_id = &block_info.block.block_id(); let signed_over = &block_info.signed_over; - debug!( - "Inserting block_info: reward_cycle = {reward_cycle}, sighash = {hash}, block_id = {block_id}, signed = {signed_over} vote = {:?}", - block_info.vote.as_ref().map(|v| { - if v.rejected { - "REJECT" - } else { - "ACCEPT" - } - }) + let vote = block_info + .vote + .as_ref() + .map(|v| if v.rejected { "REJECT" } else { "ACCEPT" }); + + debug!("Inserting block_info."; + "reward_cycle" => %block_info.reward_cycle, + "burn_block_height" => %block_info.burn_block_height, + "sighash" => %hash, + "block_id" => %block_id, + "signed" => %signed_over, + "vote" => vote ); self.db .execute( - "INSERT OR REPLACE INTO blocks (reward_cycle, signer_signature_hash, block_info) VALUES (?1, ?2, ?3)", - params![&u64_to_sql(reward_cycle)?, hash.to_string(), &block_json], + "INSERT OR REPLACE INTO blocks (reward_cycle, burn_block_height, signer_signature_hash, block_info) VALUES (?1, ?2, ?3, ?4)", + params![u64_to_sql(block_info.reward_cycle)?, u64_to_sql(block_info.burn_block_height)?, hash.to_string(), &block_json], )?; Ok(()) @@ -184,6 +184,7 @@ mod tests { NakamotoBlock, NakamotoBlockHeader, NakamotoBlockVote, }; use blockstack_lib::chainstate::stacks::ThresholdSignature; + use libsigner::BlockProposalSigners; use stacks_common::bitvec::BitVec; use stacks_common::types::chainstate::{ConsensusHash, StacksBlockId, TrieHash}; use stacks_common::util::secp256k1::MessageSignature; @@ -197,8 +198,8 @@ mod tests { } fn create_block_override( - overrides: impl FnOnce(&mut NakamotoBlock), - ) -> (BlockInfo, NakamotoBlock) { + overrides: impl FnOnce(&mut BlockProposalSigners), + ) -> (BlockInfo, BlockProposalSigners) { let header = NakamotoBlockHeader { version: 1, chain_length: 2, @@ -211,15 +212,20 @@ mod tests { signer_signature: ThresholdSignature::empty(), signer_bitvec: BitVec::zeros(1).unwrap(), }; - let mut block = NakamotoBlock { + let block = NakamotoBlock { header, txs: vec![], }; - overrides(&mut block); - (BlockInfo::new(block.clone()), block) + let mut block_proposal = BlockProposalSigners { + block, + burn_height: 7, + reward_cycle: 42, + }; + overrides(&mut block_proposal); + (BlockInfo::from(block_proposal.clone()), block_proposal) } - fn create_block() -> (BlockInfo, NakamotoBlock) { + fn create_block() -> (BlockInfo, BlockProposalSigners) { create_block_override(|_| {}) } @@ -232,21 +238,26 @@ mod tests { fn test_basic_signer_db_with_path(db_path: impl AsRef) { let mut db = SignerDb::new(db_path).expect("Failed to create signer db"); - let reward_cycle = 1; - let (block_info, block) = create_block(); - db.insert_block(reward_cycle, &block_info) + let (block_info, block_proposal) = create_block(); + let reward_cycle = block_info.reward_cycle; + db.insert_block(&block_info) .expect("Unable to insert block into db"); - let block_info = db - .block_lookup(reward_cycle, &block.header.signer_signature_hash()) + .block_lookup( + reward_cycle, + &block_proposal.block.header.signer_signature_hash(), + ) .unwrap() .expect("Unable to get block from db"); - assert_eq!(BlockInfo::new(block.clone()), block_info); + assert_eq!(BlockInfo::from(block_proposal.clone()), block_info); // Test looking up a block from a different reward cycle let block_info = db - .block_lookup(reward_cycle + 1, &block.header.signer_signature_hash()) + .block_lookup( + reward_cycle + 1, + &block_proposal.block.header.signer_signature_hash(), + ) .unwrap(); assert!(block_info.is_none()); } @@ -266,23 +277,27 @@ mod tests { fn test_update_block() { let db_path = tmp_db_path(); let mut db = SignerDb::new(db_path).expect("Failed to create signer db"); - let reward_cycle = 42; - let (block_info, block) = create_block(); - db.insert_block(reward_cycle, &block_info) + let (block_info, block_proposal) = create_block(); + let reward_cycle = block_info.reward_cycle; + db.insert_block(&block_info) .expect("Unable to insert block into db"); let block_info = db - .block_lookup(reward_cycle, &block.header.signer_signature_hash()) + .block_lookup( + reward_cycle, + &block_proposal.block.header.signer_signature_hash(), + ) .unwrap() .expect("Unable to get block from db"); - assert_eq!(BlockInfo::new(block.clone()), block_info); + assert_eq!(BlockInfo::from(block_proposal.clone()), block_info); let old_block_info = block_info; - let old_block = block; + let old_block_proposal = block_proposal; - let (mut block_info, block) = create_block_override(|b| { - b.header.signer_signature = old_block.header.signer_signature.clone(); + let (mut block_info, block_proposal) = create_block_override(|b| { + b.block.header.signer_signature = + old_block_proposal.block.header.signer_signature.clone(); }); assert_eq!( block_info.signer_signature_hash(), @@ -293,11 +308,14 @@ mod tests { rejected: false, }; block_info.vote = Some(vote.clone()); - db.insert_block(reward_cycle, &block_info) + db.insert_block(&block_info) .expect("Unable to insert block into db"); let block_info = db - .block_lookup(reward_cycle, &block.header.signer_signature_hash()) + .block_lookup( + reward_cycle, + &block_proposal.block.header.signer_signature_hash(), + ) .unwrap() .expect("Unable to get block from db"); diff --git a/stackslib/src/chainstate/nakamoto/miner.rs b/stackslib/src/chainstate/nakamoto/miner.rs index 33ee26536..7488e37fc 100644 --- a/stackslib/src/chainstate/nakamoto/miner.rs +++ b/stackslib/src/chainstate/nakamoto/miner.rs @@ -507,43 +507,6 @@ impl NakamotoBlockBuilder { pub fn get_bytes_so_far(&self) -> u64 { self.bytes_so_far } - - /// Make a StackerDB chunk message containing a proposed block. - /// Sign it with the miner's private key. - /// Automatically determine which StackerDB slot and version number to use. - /// Returns Some(chunk) if the given key corresponds to one of the expected miner slots - /// Returns None if not - /// Returns an error on signing or DB error - pub fn make_stackerdb_block_proposal( - sortdb: &SortitionDB, - tip: &BlockSnapshot, - stackerdbs: &StackerDBs, - block: &T, - miner_privkey: &StacksPrivateKey, - miners_contract_id: &QualifiedContractIdentifier, - ) -> Result, Error> { - let miner_pubkey = StacksPublicKey::from_private(&miner_privkey); - let Some(slot_range) = NakamotoChainState::get_miner_slot(sortdb, tip, &miner_pubkey)? - else { - // No slot exists for this miner - return Ok(None); - }; - // proposal slot is the first slot. - let slot_id = slot_range.start; - // Get the LAST slot version number written to the DB. If not found, use 0. - // Add 1 to get the NEXT version number - // Note: we already check above for the slot's existence - let slot_version = stackerdbs - .get_slot_version(&miners_contract_id, slot_id)? - .unwrap_or(0) - .saturating_add(1); - let block_bytes = block.serialize_to_vec(); - let mut chunk = StackerDBChunkData::new(slot_id, slot_version, block_bytes); - chunk - .sign(miner_privkey) - .map_err(|_| net_error::SigningError("Failed to sign StackerDB chunk".into()))?; - Ok(Some(chunk)) - } } impl BlockBuilder for NakamotoBlockBuilder { diff --git a/stackslib/src/chainstate/nakamoto/tests/mod.rs b/stackslib/src/chainstate/nakamoto/tests/mod.rs index 9844fa7b7..f8d048aaf 100644 --- a/stackslib/src/chainstate/nakamoto/tests/mod.rs +++ b/stackslib/src/chainstate/nakamoto/tests/mod.rs @@ -23,6 +23,7 @@ use clarity::vm::clarity::ClarityConnection; use clarity::vm::costs::ExecutionCost; use clarity::vm::types::StacksAddressExtensions; use clarity::vm::Value; +use libstackerdb::StackerDBChunkData; use rand::{thread_rng, RngCore}; use rusqlite::{Connection, ToSql}; use stacks_common::address::AddressHashMode; @@ -2019,31 +2020,26 @@ fn test_make_miners_stackerdb_config() { txs: vec![], }; let tip = SortitionDB::get_canonical_burn_chain_tip(sort_db.conn()).unwrap(); + let miner_privkey = &miner_keys[i]; + let miner_pubkey = StacksPublicKey::from_private(miner_privkey); + let slot_id = NakamotoChainState::get_miner_slot(&sort_db, &tip, &miner_pubkey) + .expect("Failed to get miner slot"); if sortition { - let chunk = NakamotoBlockBuilder::make_stackerdb_block_proposal( - &sort_db, - &tip, - &stackerdbs, - &block, - &miner_keys[i], - &miners_contract_id, - ) - .unwrap() - .unwrap(); + let slot_id = slot_id.expect("No miner slot exists for this miner").start; + let slot_version = stackerdbs + .get_slot_version(&miners_contract_id, slot_id) + .expect("Failed to get slot version") + .unwrap_or(0) + .saturating_add(1); + let block_bytes = block.serialize_to_vec(); + let mut chunk = StackerDBChunkData::new(slot_id, slot_version, block_bytes); + chunk.sign(&miner_keys[i]).expect("Failed to sign chunk"); assert_eq!(chunk.slot_version, 1); assert_eq!(chunk.data, block.serialize_to_vec()); stackerdb_chunks.push(chunk); } else { - assert!(NakamotoBlockBuilder::make_stackerdb_block_proposal( - &sort_db, - &tip, - &stackerdbs, - &block, - &miner_keys[i], - &miners_contract_id, - ) - .unwrap() - .is_none()); + // We are not a miner anymore and should not have any slot + assert!(slot_id.is_none()); } } // miners are "stable" across snapshots diff --git a/stackslib/src/net/stackerdb/mod.rs b/stackslib/src/net/stackerdb/mod.rs index 7a1b29b2e..24265410e 100644 --- a/stackslib/src/net/stackerdb/mod.rs +++ b/stackslib/src/net/stackerdb/mod.rs @@ -151,7 +151,7 @@ pub const STACKERDB_MAX_PAGE_COUNT: u32 = 2; pub const STACKERDB_SLOTS_FUNCTION: &str = "stackerdb-get-signer-slots"; pub const STACKERDB_CONFIG_FUNCTION: &str = "stackerdb-get-config"; -pub const MINER_SLOT_COUNT: u32 = 2; +pub const MINER_SLOT_COUNT: u32 = 1; /// Final result of synchronizing state with a remote set of DB replicas pub struct StackerDBSyncResult { diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index 3a976aecc..a58c00349 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -18,20 +18,16 @@ use std::thread; use std::thread::JoinHandle; use std::time::{Duration, Instant}; -use clarity::boot_util::boot_code_id; use clarity::vm::clarity::ClarityConnection; use clarity::vm::types::{PrincipalData, QualifiedContractIdentifier}; use hashbrown::HashSet; -use libsigner::{ - BlockProposalSigners, MessageSlotID, SignerMessage, SignerSession, StackerDBSession, -}; +use libsigner::{MessageSlotID, SignerMessage}; use stacks::burnchains::Burnchain; use stacks::chainstate::burn::db::sortdb::SortitionDB; use stacks::chainstate::burn::{BlockSnapshot, ConsensusHash}; use stacks::chainstate::nakamoto::miner::{NakamotoBlockBuilder, NakamotoTenureInfo}; use stacks::chainstate::nakamoto::signer_set::NakamotoSigners; use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoChainState}; -use stacks::chainstate::stacks::boot::MINERS_NAME; use stacks::chainstate::stacks::db::{StacksChainState, StacksHeaderInfo}; use stacks::chainstate::stacks::{ CoinbasePayload, Error as ChainstateError, StacksTransaction, StacksTransactionSigner, @@ -179,13 +175,9 @@ impl BlockMinerThread { }; if let Some(mut new_block) = new_block { - if let Err(e) = self.propose_block(&new_block, &stackerdbs) { - error!("Unrecoverable error while proposing block to signer set: {e:?}. Ending tenure."); - return; - } - let (aggregate_public_key, signers_signature) = match self.coordinate_signature( &new_block, + self.burn_block.block_height, &mut stackerdbs, &mut attempts, ) { @@ -239,6 +231,7 @@ impl BlockMinerThread { fn coordinate_signature( &mut self, new_block: &NakamotoBlock, + burn_block_height: u64, stackerdbs: &mut StackerDBs, attempts: &mut u64, ) -> Result<(Point, ThresholdSignature), NakamotoNodeError> { @@ -302,18 +295,6 @@ impl BlockMinerThread { )); }; - #[cfg(test)] - { - // In test mode, short-circuit spinning up the SignCoordinator if the TEST_SIGNING - // channel has been created. This allows integration tests for the stacks-node - // independent of the stacks-signer. - if let Some(signature) = - crate::tests::nakamoto_integrations::TestSigningChannel::get_signature() - { - return Ok((aggregate_public_key, signature)); - } - } - let miner_privkey_as_scalar = Scalar::from(miner_privkey.as_slice().clone()); let mut coordinator = SignCoordinator::new( &reward_set, @@ -332,97 +313,18 @@ impl BlockMinerThread { *attempts += 1; let signature = coordinator.begin_sign( new_block, + burn_block_height, *attempts, &tip, &self.burnchain, &sort_db, &stackerdbs, + &self.globals.counters, )?; Ok((aggregate_public_key, signature)) } - fn propose_block( - &mut self, - new_block: &NakamotoBlock, - stackerdbs: &StackerDBs, - ) -> Result<(), NakamotoNodeError> { - let rpc_socket = self.config.node.get_rpc_loopback().ok_or_else(|| { - NakamotoNodeError::MinerConfigurationFailed("Could not parse RPC bind") - })?; - let miners_contract_id = boot_code_id(MINERS_NAME, self.config.is_mainnet()); - let mut miners_session = - StackerDBSession::new(&rpc_socket.to_string(), miners_contract_id.clone()); - let Some(miner_privkey) = self.config.miner.mining_key else { - return Err(NakamotoNodeError::MinerConfigurationFailed( - "No mining key configured, cannot mine", - )); - }; - let sort_db = SortitionDB::open( - &self.config.get_burn_db_file_path(), - true, - self.burnchain.pox_constants.clone(), - ) - .expect("FATAL: could not open sortition DB"); - let tip = SortitionDB::get_block_snapshot_consensus( - sort_db.conn(), - &new_block.header.consensus_hash, - ) - .expect("FATAL: could not retrieve chain tip") - .expect("FATAL: could not retrieve chain tip"); - let reward_cycle = self - .burnchain - .pox_constants - .block_height_to_reward_cycle( - self.burnchain.first_block_height, - self.burn_block.block_height, - ) - .expect("FATAL: building on a burn block that is before the first burn block"); - - let proposal_msg = BlockProposalSigners { - block: new_block.clone(), - burn_height: self.burn_block.block_height, - reward_cycle, - }; - let proposal = match NakamotoBlockBuilder::make_stackerdb_block_proposal( - &sort_db, - &tip, - &stackerdbs, - &proposal_msg, - &miner_privkey, - &miners_contract_id, - ) { - Ok(Some(chunk)) => chunk, - Ok(None) => { - warn!("Failed to propose block to stackerdb: no slot available"); - return Ok(()); - } - Err(e) => { - warn!("Failed to propose block to stackerdb: {e:?}"); - return Ok(()); - } - }; - - // Propose the block to the observing signers through the .miners stackerdb instance - match miners_session.put_chunk(&proposal) { - Ok(ack) => { - info!( - "Proposed block to stackerdb"; - "signer_sighash" => %new_block.header.signer_signature_hash(), - "ack_msg" => ?ack, - ); - } - Err(e) => { - return Err(NakamotoNodeError::SigningCoordinatorFailure(format!( - "Failed to propose block to stackerdb {e:?}" - ))); - } - } - - self.globals.counters.bump_naka_proposed_blocks(); - Ok(()) - } - fn get_stackerdb_contract_and_slots( &self, stackerdbs: &StackerDBs, diff --git a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs index 9dc1c1574..e42cd658c 100644 --- a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs +++ b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs @@ -18,7 +18,8 @@ use std::time::{Duration, Instant}; use hashbrown::{HashMap, HashSet}; use libsigner::{ - MessageSlotID, SignerEntries, SignerEvent, SignerMessage, SignerSession, StackerDBSession, + BlockProposalSigners, MessageSlotID, SignerEntries, SignerEvent, SignerMessage, SignerSession, + StackerDBSession, }; use stacks::burnchains::Burnchain; use stacks::chainstate::burn::db::sortdb::SortitionDB; @@ -43,6 +44,7 @@ use wsts::v2::Aggregator; use super::Error as NakamotoNodeError; use crate::event_dispatcher::STACKER_DB_CHANNEL; +use crate::neon::Counters; use crate::Config; /// How long should the coordinator poll on the event receiver before @@ -238,6 +240,33 @@ impl SignCoordinator { }; let mut coordinator: FireCoordinator = FireCoordinator::new(coord_config); + #[cfg(test)] + { + // In test mode, short-circuit spinning up the SignCoordinator if the TEST_SIGNING + // channel has been created. This allows integration tests for the stacks-node + // independent of the stacks-signer. + use crate::tests::nakamoto_integrations::TEST_SIGNING; + if TEST_SIGNING.lock().unwrap().is_some() { + debug!("Short-circuiting spinning up coordinator from signer commitments. Using test signers channel."); + let (receiver, replaced_other) = STACKER_DB_CHANNEL.register_miner_coordinator(); + if replaced_other { + warn!("Replaced the miner/coordinator receiver of a prior thread. Prior thread may have crashed."); + } + let mut sign_coordinator = Self { + coordinator, + message_key, + receiver: Some(receiver), + wsts_public_keys, + is_mainnet, + miners_session, + signing_round_timeout: config.miner.wait_on_signers.clone(), + }; + sign_coordinator + .coordinator + .set_aggregate_public_key(Some(aggregate_public_key)); + return Ok(sign_coordinator); + } + } let party_polynomials = get_signer_commitments( is_mainnet, reward_set_signers.as_slice(), @@ -291,8 +320,8 @@ impl SignCoordinator { else { return Err("No slot for miner".into()); }; - let target_slot = 1; - let slot_id = slot_range.start + target_slot; + // We only have one slot per miner + let slot_id = slot_range.start; if !slot_range.contains(&slot_id) { return Err("Not enough slots for miner messages".into()); } @@ -326,11 +355,13 @@ impl SignCoordinator { pub fn begin_sign( &mut self, block: &NakamotoBlock, + burn_block_height: u64, block_attempt: u64, burn_tip: &BlockSnapshot, burnchain: &Burnchain, sortdb: &SortitionDB, stackerdbs: &StackerDBs, + counters: &Counters, ) -> Result { let sign_id = Self::get_sign_id(burn_tip.block_height, burnchain); let sign_iter_id = block_attempt; @@ -340,7 +371,13 @@ impl SignCoordinator { self.coordinator.current_sign_id = sign_id; self.coordinator.current_sign_iter_id = sign_iter_id; - let block_bytes = block.serialize_to_vec(); + let proposal_msg = BlockProposalSigners { + block: block.clone(), + burn_height: burn_block_height, + reward_cycle: reward_cycle_id, + }; + + let block_bytes = proposal_msg.serialize_to_vec(); let nonce_req_msg = self .coordinator .start_signing_round(&block_bytes, false, None) @@ -359,6 +396,19 @@ impl SignCoordinator { &mut self.miners_session, ) .map_err(NakamotoNodeError::SigningCoordinatorFailure)?; + counters.bump_naka_proposed_blocks(); + #[cfg(test)] + { + // In test mode, short-circuit waiting for the signers if the TEST_SIGNING + // channel has been created. This allows integration tests for the stacks-node + // independent of the stacks-signer. + if let Some(signature) = + crate::tests::nakamoto_integrations::TestSigningChannel::get_signature() + { + debug!("Short-circuiting waiting for signers, using test signature"); + return Ok(signature); + } + } let Some(ref mut receiver) = self.receiver else { return Err(NakamotoNodeError::SigningCoordinatorFailure( diff --git a/testnet/stacks-node/src/tests/nakamoto_integrations.rs b/testnet/stacks-node/src/tests/nakamoto_integrations.rs index aa545514f..893e523e2 100644 --- a/testnet/stacks-node/src/tests/nakamoto_integrations.rs +++ b/testnet/stacks-node/src/tests/nakamoto_integrations.rs @@ -26,7 +26,7 @@ use clarity::vm::costs::ExecutionCost; use clarity::vm::types::{PrincipalData, QualifiedContractIdentifier}; use http_types::headers::AUTHORIZATION; use lazy_static::lazy_static; -use libsigner::{SignerSession, StackerDBSession}; +use libsigner::{BlockProposalSigners, SignerMessage, SignerSession, StackerDBSession}; use rand::RngCore; use stacks::burnchains::{MagicBytes, Txid}; use stacks::chainstate::burn::db::sortdb::SortitionDB; @@ -36,7 +36,7 @@ use stacks::chainstate::burn::operations::{ use stacks::chainstate::coordinator::comm::CoordinatorChannels; use stacks::chainstate::nakamoto::miner::NakamotoBlockBuilder; use stacks::chainstate::nakamoto::test_signers::TestSigners; -use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoChainState}; +use stacks::chainstate::nakamoto::NakamotoChainState; use stacks::chainstate::stacks::address::{PoxAddress, StacksAddressExtensions}; use stacks::chainstate::stacks::boot::{ MINERS_NAME, SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME, @@ -72,6 +72,7 @@ use stacks_common::types::StacksPublicKeyBuffer; use stacks_common::util::hash::{to_hex, Sha512Trunc256Sum}; use stacks_common::util::secp256k1::{Secp256k1PrivateKey, Secp256k1PublicKey}; use stacks_common::util::sleep_ms; +use wsts::net::Message; use super::bitcoin_regtest::BitcoinCoreController; use crate::config::{EventKeyType, EventObserverConfig, InitialBalance}; @@ -309,13 +310,23 @@ pub fn read_and_sign_block_proposal( .block_height_to_reward_cycle(tip.block_height) .unwrap(); - let mut proposed_block: NakamotoBlock = { + let mut proposed_block = { let miner_contract_id = boot_code_id(MINERS_NAME, false); let mut miners_stackerdb = StackerDBSession::new(&conf.node.rpc_bind, miner_contract_id); - miners_stackerdb + let message: SignerMessage = miners_stackerdb .get_latest(miner_slot_id.start) - .map_err(|_| "Failed to get latest chunk from the miner slot ID")? - .ok_or("No chunk found")? + .expect("Failed to get latest chunk from the miner slot ID") + .expect("No chunk found"); + let SignerMessage::Packet(packet) = message else { + panic!("Expected a signer message packet. Got {message:?}"); + }; + let Message::NonceRequest(nonce_request) = packet.msg else { + panic!("Expected a nonce request. Got {:?}", packet.msg); + }; + let block_proposal = + BlockProposalSigners::consensus_deserialize(&mut nonce_request.message.as_slice()) + .expect("Failed to deserialize block proposal"); + block_proposal.block }; let proposed_block_hash = format!("0x{}", proposed_block.header.block_hash()); let signer_sig_hash = proposed_block.header.signer_signature_hash(); @@ -2159,14 +2170,24 @@ fn miner_writes_proposed_block_to_stackerdb() { .expect("Unable to get miner slot") .expect("No miner slot exists"); - let proposed_block: NakamotoBlock = { + let proposed_block = { let miner_contract_id = boot_code_id(MINERS_NAME, false); let mut miners_stackerdb = StackerDBSession::new(&naka_conf.node.rpc_bind, miner_contract_id); - miners_stackerdb + let message: SignerMessage = miners_stackerdb .get_latest(slot_id.start) .expect("Failed to get latest chunk from the miner slot ID") - .expect("No chunk found") + .expect("No chunk found"); + let SignerMessage::Packet(packet) = message else { + panic!("Expected a signer message packet. Got {message:?}"); + }; + let Message::NonceRequest(nonce_request) = packet.msg else { + panic!("Expected a nonce request. Got {:?}", packet.msg); + }; + let block_proposal = + BlockProposalSigners::consensus_deserialize(&mut nonce_request.message.as_slice()) + .expect("Failed to deserialize block proposal"); + block_proposal.block }; let proposed_block_hash = format!("0x{}", proposed_block.header.block_hash()); diff --git a/testnet/stacks-node/src/tests/signer.rs b/testnet/stacks-node/src/tests/signer.rs index 867421b5c..1eba5b666 100644 --- a/testnet/stacks-node/src/tests/signer.rs +++ b/testnet/stacks-node/src/tests/signer.rs @@ -9,8 +9,8 @@ use std::{env, thread}; use clarity::boot_util::boot_code_id; use clarity::vm::Value; use libsigner::{ - BlockResponse, MessageSlotID, RejectCode, RunningSigner, Signer, SignerEventReceiver, - SignerMessage, + BlockProposalSigners, BlockResponse, MessageSlotID, RejectCode, RunningSigner, Signer, + SignerEventReceiver, SignerMessage, }; use rand::thread_rng; use rand_core::RngCore; @@ -1037,13 +1037,23 @@ fn stackerdb_sign() { info!("------------------------- Test Sign -------------------------"); let reward_cycle = signer_test.get_current_reward_cycle(); + let block_proposal_1 = BlockProposalSigners { + block: block1.clone(), + burn_height: 0, + reward_cycle, + }; + let block_proposal_2 = BlockProposalSigners { + block: block2.clone(), + burn_height: 0, + reward_cycle, + }; // Determine the coordinator of the current node height info!("signer_runloop: spawn send commands to do sign"); let sign_now = Instant::now(); let sign_command = RunLoopCommand { reward_cycle, command: SignerCommand::Sign { - block: block1, + block_proposal: block_proposal_1, is_taproot: false, merkle_root: None, }, @@ -1051,7 +1061,7 @@ fn stackerdb_sign() { let sign_taproot_command = RunLoopCommand { reward_cycle, command: SignerCommand::Sign { - block: block2, + block_proposal: block_proposal_2, is_taproot: true, merkle_root: None, },