From e577139b64423aa045ccfafd57743f5f90bfe8ab Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Fri, 8 Mar 2024 16:29:23 -0500 Subject: [PATCH 01/26] Have miners abort signing a block if the burn tip changes Signed-off-by: Jacinta Ferrant --- testnet/stacks-node/src/nakamoto_node/miner.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index 88d13d22e..4a4411479 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -376,15 +376,13 @@ impl BlockMinerThread { fn wait_for_signer_signature( &self, + sortdb: &SortitionDB, stackerdbs: &StackerDBs, aggregate_public_key: &Point, signer_signature_hash: &Sha512Trunc256Sum, signer_weights: HashMap, + reward_cycle: u64, ) -> Result { - let reward_cycle = self - .burnchain - .block_height_to_reward_cycle(self.burn_block.block_height) - .expect("FATAL: no reward cycle for burn block"); let (signers_contract_id, slot_ids_addresses) = self.get_stackerdb_contract_and_slots(stackerdbs, BLOCK_MSG_ID, reward_cycle)?; let slot_ids = slot_ids_addresses.keys().cloned().collect::>(); @@ -396,6 +394,10 @@ impl BlockMinerThread { let now = Instant::now(); debug!("Miner: waiting for block response from reward cycle {reward_cycle } signers..."); while now.elapsed() < self.config.miner.wait_on_signers { + if self.check_burn_tip_changed(&sortdb).is_err() { + info!("Miner: burnchain tip changed while waiting for signer signature."); + return Err(NakamotoNodeError::BurnchainTipChanged); + } // Get the block responses from the signers for the block we just proposed debug!("Miner: retreiving latest signer messsages"; "signers_contract_id" => %signers_contract_id, @@ -531,10 +533,12 @@ impl BlockMinerThread { )?; let signature = self .wait_for_signer_signature( + &sort_db, &stackerdbs, &aggregate_public_key, &block.header.signer_signature_hash(), signer_weights, + reward_cycle, ) .map_err(|e| { ChainstateError::InvalidStacksBlock(format!("Invalid Nakamoto block: {e:?}")) From db8f0cce88235005001066b5df1014bd278769f8 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Fri, 8 Mar 2024 19:28:12 -0500 Subject: [PATCH 02/26] chore: log coordinator state --- stacks-signer/src/signer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index b03a2da36..053be6755 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -367,7 +367,7 @@ impl Signer { } State::OperationInProgress => { // We cannot execute the next command until the current one is finished... - debug!("{self}: Waiting for coordinator {coordinator_id:?} operation to finish...",); + debug!("{self}: Waiting for coordinator {coordinator_id:?} operation to finish. Coordinator state = {:?}", self.coordinator.state); } } } From d56895a600fe8972ffc8a41befad95dea0fc9d6d Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 8 Mar 2024 18:50:57 -0600 Subject: [PATCH 03/26] feat: use block proposal struct for miner -> signers comms. check claimed reward-cycle --- libsigner/src/events.rs | 37 +++++++++++-- libsigner/src/libsigner.rs | 3 +- stacks-signer/src/signer.rs | 25 +++++++-- stackslib/src/chainstate/nakamoto/miner.rs | 4 +- .../stacks-node/src/nakamoto_node/miner.rs | 54 ++++++++++++------- 5 files changed, 93 insertions(+), 30 deletions(-) diff --git a/libsigner/src/events.rs b/libsigner/src/events.rs index 0d73b9579..ce26447e5 100644 --- a/libsigner/src/events.rs +++ b/libsigner/src/events.rs @@ -50,11 +50,22 @@ use wsts::state_machine::signer; use crate::http::{decode_http_body, decode_http_request}; use crate::{EventError, SignerMessage}; +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] +/// BlockProposal sent to signers +pub struct BlockProposalSigners { + /// The block itself + pub block: NakamotoBlock, + /// The burn height the block is mined during + pub burn_height: u64, + /// The reward cycle the block is mined during + pub reward_cycle: u64, +} + /// Event enum for newly-arrived signer subscribed events #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub enum SignerEvent { /// The miner proposed blocks for signers to observe and sign - ProposedBlocks(Vec), + ProposedBlocks(Vec), /// 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), @@ -64,6 +75,26 @@ pub enum SignerEvent { StatusCheck, } +impl StacksMessageCodec for BlockProposalSigners { + fn consensus_serialize(&self, fd: &mut W) -> Result<(), CodecError> { + self.block.consensus_serialize(fd)?; + self.burn_height.consensus_serialize(fd)?; + self.reward_cycle.consensus_serialize(fd)?; + Ok(()) + } + + fn consensus_deserialize(fd: &mut R) -> Result { + let block = NakamotoBlock::consensus_deserialize(fd)?; + let burn_height = u64::consensus_deserialize(fd)?; + let reward_cycle = u64::consensus_deserialize(fd)?; + Ok(BlockProposalSigners { + block, + burn_height, + reward_cycle, + }) + } +} + /// Trait to implement a stop-signaler for the event receiver thread. /// The caller calls `send()` and the event receiver loop (which lives in a separate thread) will /// terminate. @@ -337,10 +368,10 @@ fn process_stackerdb_event( .map_err(|e| EventError::Deserialize(format!("Could not decode body to JSON: {:?}", &e)))?; let signer_event = if event.contract_id == boot_code_id(MINERS_NAME, is_mainnet) { - let blocks: Vec = event + let blocks: Vec = event .modified_slots .iter() - .filter_map(|chunk| read_next::(&mut &chunk.data[..]).ok()) + .filter_map(|chunk| read_next::(&mut &chunk.data[..]).ok()) .collect(); SignerEvent::ProposedBlocks(blocks) } else if event.contract_id.name.to_string().starts_with(SIGNERS_NAME) diff --git a/libsigner/src/libsigner.rs b/libsigner/src/libsigner.rs index e48f4014e..1ae699d6e 100644 --- a/libsigner/src/libsigner.rs +++ b/libsigner/src/libsigner.rs @@ -45,7 +45,8 @@ mod session; pub use crate::error::{EventError, RPCError}; pub use crate::events::{ - EventReceiver, EventStopSignaler, SignerEvent, SignerEventReceiver, SignerStopSignaler, + BlockProposalSigners, EventReceiver, EventStopSignaler, SignerEvent, SignerEventReceiver, + SignerStopSignaler, }; pub use crate::messages::{ BlockRejection, BlockResponse, RejectCode, SignerMessage, BLOCK_MSG_ID, TRANSACTIONS_MSG_ID, diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 053be6755..4159eedf3 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -24,7 +24,9 @@ use blockstack_lib::chainstate::stacks::boot::SIGNERS_VOTING_FUNCTION_NAME; use blockstack_lib::chainstate::stacks::StacksTransaction; use blockstack_lib::net::api::postblock_proposal::BlockValidateResponse; use hashbrown::HashSet; -use libsigner::{BlockRejection, BlockResponse, RejectCode, SignerEvent, SignerMessage}; +use libsigner::{ + BlockProposalSigners, BlockRejection, BlockResponse, RejectCode, SignerEvent, SignerMessage, +}; use serde_derive::{Deserialize, Serialize}; use slog::{slog_debug, slog_error, slog_info, slog_warn}; use stacks_common::codec::{read_next, StacksMessageCodec}; @@ -497,17 +499,30 @@ impl Signer { } /// Handle proposed blocks submitted by the miners to stackerdb - fn handle_proposed_blocks(&mut self, stacks_client: &StacksClient, blocks: &[NakamotoBlock]) { - for block in blocks { + fn handle_proposed_blocks( + &mut self, + stacks_client: &StacksClient, + proposals: &[BlockProposalSigners], + ) { + for proposal in proposals { + if proposal.reward_cycle != self.reward_cycle { + debug!( + "Signer #{}: Received proposal for block outside of my reward cycle, ignoring.", + self.signer_id; + "proposal_reward_cycle" => proposal.reward_cycle, + "proposal_burn_height" => proposal.burn_height, + ); + continue; + } // Store the block in our cache self.signer_db - .insert_block(&BlockInfo::new(block.clone())) + .insert_block(&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(block.clone()) + .submit_block_for_validation(proposal.block.clone()) .unwrap_or_else(|e| { warn!("{self}: Failed to submit block for validation: {e:?}"); }); diff --git a/stackslib/src/chainstate/nakamoto/miner.rs b/stackslib/src/chainstate/nakamoto/miner.rs index 5edeac4c6..961fd32db 100644 --- a/stackslib/src/chainstate/nakamoto/miner.rs +++ b/stackslib/src/chainstate/nakamoto/miner.rs @@ -524,11 +524,11 @@ impl NakamotoBlockBuilder { /// 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( + pub fn make_stackerdb_block_proposal( sortdb: &SortitionDB, tip: &BlockSnapshot, stackerdbs: &StackerDBs, - block: &NakamotoBlock, + block: &T, miner_privkey: &StacksPrivateKey, miners_contract_id: &QualifiedContractIdentifier, ) -> Result, Error> { diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index 4a4411479..c7e772b20 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -23,8 +23,8 @@ use clarity::vm::clarity::ClarityConnection; use clarity::vm::types::{PrincipalData, QualifiedContractIdentifier}; use hashbrown::HashSet; use libsigner::{ - BlockResponse, RejectCode, SignerMessage, SignerSession, StackerDBSession, BLOCK_MSG_ID, - TRANSACTIONS_MSG_ID, + BlockProposalSigners, BlockResponse, RejectCode, SignerMessage, SignerSession, + StackerDBSession, BLOCK_MSG_ID, TRANSACTIONS_MSG_ID, }; use stacks::burnchains::{Burnchain, BurnchainParameters}; use stacks::chainstate::burn::db::sortdb::SortitionDB; @@ -199,37 +199,53 @@ impl BlockMinerThread { .expect("FATAL: could not open sortition DB"); let tip = SortitionDB::get_canonical_burn_chain_tip(sort_db.conn()) .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"); if let Some(new_block) = new_block { - match NakamotoBlockBuilder::make_stackerdb_block_proposal( + 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, - &new_block, + &proposal_msg, &miner_privkey, &miners_contract_id, ) { - Ok(Some(chunk)) => { - // Propose the block to the observing signers through the .miners stackerdb instance - let miner_contract_id = boot_code_id(MINERS_NAME, self.config.is_mainnet()); - let mut miners_stackerdb = - StackerDBSession::new(&self.config.node.rpc_bind, miner_contract_id); - match miners_stackerdb.put_chunk(&chunk) { - Ok(ack) => { - info!("Proposed block to stackerdb: {ack:?}"); - } - Err(e) => { - warn!("Failed to propose block to stackerdb {e:?}"); - return; - } - } - } + Ok(Some(chunk)) => chunk, Ok(None) => { warn!("Failed to propose block to stackerdb: no slot available"); + continue; } Err(e) => { warn!("Failed to propose block to stackerdb: {e:?}"); + continue; + } + }; + + // Propose the block to the observing signers through the .miners stackerdb instance + let miner_contract_id = boot_code_id(MINERS_NAME, self.config.is_mainnet()); + let mut miners_stackerdb = + StackerDBSession::new(&self.config.node.rpc_bind, miner_contract_id); + match miners_stackerdb.put_chunk(&proposal) { + Ok(ack) => { + info!("Proposed block to stackerdb: {ack:?}"); + } + Err(e) => { + warn!("Failed to propose block to stackerdb {e:?}"); + return; } } + self.globals.counters.bump_naka_proposed_blocks(); if let Err(e) = From b9c9ece00f64d6a4d03f2d305f4511bfac571f78 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 8 Mar 2024 21:01:20 -0600 Subject: [PATCH 04/26] feat: add a parity check and filter events sent to the signer instances --- stacks-signer/src/runloop.rs | 16 ++++++++++++++++ testnet/stacks-node/src/tests/signer.rs | 4 +++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index d131d5884..4a9a8d528 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -357,6 +357,22 @@ impl SignerRunLoop, RunLoopCommand> for RunLoop { error!("Failed to refresh signers: {e}. Signer may have an outdated view of the network. Attempting to process event anyway."); } for signer in self.stacks_signers.values_mut() { + let event_parity = match event { + Some(SignerEvent::BlockValidationResponse(_)) => Some(current_reward_cycle % 2), + // Block proposal events do have reward cycles, but each proposal has its own cycle, + // and the vec could be heterogenous, so, don't differentiate. + Some(SignerEvent::ProposedBlocks(_)) => None, + Some(SignerEvent::SignerMessages(msg_parity, ..)) => { + Some(u64::from(msg_parity) % 2) + } + Some(SignerEvent::StatusCheck) => None, + None => None, + }; + let other_signer_parity = (signer.reward_cycle + 1) % 2; + if event_parity == Some(other_signer_parity) { + continue; + } + if let Err(e) = signer.process_event( &self.stacks_client, event.as_ref(), diff --git a/testnet/stacks-node/src/tests/signer.rs b/testnet/stacks-node/src/tests/signer.rs index 54e851be9..ebdef33d4 100644 --- a/testnet/stacks-node/src/tests/signer.rs +++ b/testnet/stacks-node/src/tests/signer.rs @@ -447,7 +447,9 @@ impl SignerTest { panic!("Received SignError {}", sign_error); } OperationResult::Dkg(point) => { - panic!("Received aggregate_group_key {point}"); + // should not panic, because DKG may have just run for the + // next reward cycle. + info!("Received aggregate_group_key {point}"); } } } From b61b2be0d4d4633cba9a01640b0d82dc8571955d Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 8 Mar 2024 21:58:00 -0600 Subject: [PATCH 05/26] logs: signer block responses to info. display formatting for block response --- libsigner/src/messages.rs | 21 +++++++++++++++++++++ stacks-signer/src/signer.rs | 11 +++++++---- stackslib/src/chainstate/stacks/mod.rs | 6 ++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/libsigner/src/messages.rs b/libsigner/src/messages.rs index 6135312a8..debb43218 100644 --- a/libsigner/src/messages.rs +++ b/libsigner/src/messages.rs @@ -823,6 +823,27 @@ pub enum BlockResponse { Rejected(BlockRejection), } +impl std::fmt::Display for BlockResponse { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + BlockResponse::Accepted(a) => { + write!( + f, + "BlockAccepted: signer_sighash = {}, signature = {}", + a.0, a.1 + ) + } + BlockResponse::Rejected(r) => { + write!( + f, + "BlockRejected: signer_sighash = {}, code = {}, reason = {}", + r.reason_code, r.reason, r.signer_signature_hash + ) + } + } + } +} + impl BlockResponse { /// Create a new accepted BlockResponse for the provided block signer signature hash and signature pub fn accepted(hash: Sha512Trunc256Sum, sig: Signature) -> Self { diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 4159eedf3..e7d78637d 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -989,15 +989,18 @@ impl Signer { let block_submission = if block_vote.rejected { // We signed a rejection message. Return a rejection message - BlockResponse::rejected(block_vote.signer_signature_hash, signature.clone()).into() + BlockResponse::rejected(block_vote.signer_signature_hash, signature.clone()) } else { // we agreed to sign the block hash. Return an approval message - BlockResponse::accepted(block_vote.signer_signature_hash, signature.clone()).into() + BlockResponse::accepted(block_vote.signer_signature_hash, signature.clone()) }; // Submit signature result to miners to observe - debug!("{self}: submit block response {block_submission:?}"); - if let Err(e) = self.stackerdb.send_message_with_retry(block_submission) { + info!("{self}: Submit block response: {block_submission}"); + if let Err(e) = self + .stackerdb + .send_message_with_retry(block_submission.into()) + { warn!("{self}: Failed to send block submission to stacker-db: {e:?}"); } } diff --git a/stackslib/src/chainstate/stacks/mod.rs b/stackslib/src/chainstate/stacks/mod.rs index 7247a28f7..f9ad4fff3 100644 --- a/stackslib/src/chainstate/stacks/mod.rs +++ b/stackslib/src/chainstate/stacks/mod.rs @@ -693,6 +693,12 @@ impl FromSql for ThresholdSignature { } } +impl fmt::Display for ThresholdSignature { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + to_hex(&self.serialize_to_vec()).fmt(f) + } +} + impl ToSql for ThresholdSignature { fn to_sql(&self) -> rusqlite::Result { let bytes = self.serialize_to_vec(); From 92325502d21a477e9f8db4057c283566b59a784c Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 8 Mar 2024 22:05:55 -0600 Subject: [PATCH 06/26] logs: better miner assembly and broadcasting logging --- testnet/stacks-node/src/nakamoto_node/miner.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index c7e772b20..dc75ef56c 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -253,6 +253,14 @@ impl BlockMinerThread { { warn!("Error broadcasting block: {e:?}"); } else { + info!( + "Miner: Block signed by signer set and broadcasted"; + "signer_sighash" => %new_block.header.signer_signature_hash(), + "block_hash" => %new_block.header.block_hash(), + "stacks_block_id" => %new_block.header.block_id(), + "block_height" => new_block.header.chain_length, + "consensus_hash" => %new_block.header.consensus_hash, + ); self.globals.coord().announce_new_stacks_block(); } @@ -861,15 +869,11 @@ impl BlockMinerThread { block.header.miner_signature = miner_signature; info!( - "Miner: Succeeded assembling {} block #{}: {}, with {} txs", - if parent_block_info.parent_block_total_burn == 0 { - "Genesis" - } else { - "Stacks" - }, + "Miner: Assembled block #{} for signer set proposal: {}, with {} txs", block.header.chain_length, block.header.block_hash(), - block.txs.len(), + block.txs.len(); + "signer_sighash" => %block.header.signer_signature_hash(), ); // last chance -- confirm that the stacks tip is unchanged (since it could have taken long From 8cdbf08aa5099a2e5b4e256c94b999082e148463 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 8 Mar 2024 23:27:02 -0600 Subject: [PATCH 07/26] logs: add coordinator id to signer binary logging --- stacks-signer/src/signer.rs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index e7d78637d..aa4c205a2 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -172,8 +172,10 @@ impl std::fmt::Display for Signer { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!( f, - "Reward Cycle #{} Signer #{}", - self.reward_cycle, self.signer_id, + "Cycle #{} Signer #{}(C:{})", + self.reward_cycle, + self.signer_id, + self.coordinator_selector.get_coordinator().0, ) } } @@ -454,8 +456,9 @@ impl Signer { { // We are the coordinator. Trigger a signing round for this block debug!( - "{self}: triggering a signing round over the block {}", - block_info.block.header.block_hash() + "{self}: attempt to trigger a signing round for block"; + "signer_sighash" => %block_info.block.header.signer_signature_hash(), + "block_hash" => %block_info.block.header.block_hash(), ); self.commands.push_back(Command::Sign { block: block_info.block.clone(), From a736360ee42a54019f400d950a578697e89552c5 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Sat, 9 Mar 2024 09:30:51 -0600 Subject: [PATCH 08/26] logs: improved rejection logging, debug logs in the db --- stacks-signer/src/signer.rs | 13 ++++++++++--- stacks-signer/src/signerdb.rs | 12 ++++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index aa4c205a2..5bff83629 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -592,7 +592,10 @@ impl Signer { { Some(Some(vote)) => { // Overwrite with our agreed upon value in case another message won majority or the coordinator is trying to cheat... - debug!("{self}: set vote for {} to {vote:?}", block_vote.rejected); + debug!( + "{self}: Set vote (rejected = {}) to {vote:?}", block_vote.rejected; + "requested_sighash" => %block_vote.signer_signature_hash, + ); request.message = vote.serialize_to_vec(); true } @@ -600,7 +603,10 @@ impl Signer { // We never agreed to sign this block. Reject it. // This can happen if the coordinator received enough votes to sign yes // or no on a block before we received validation from the stacks node. - debug!("{self}: Received a signature share request for a block we never agreed to sign. Ignore it."); + debug!( + "{self}: Received a signature share request for a block we never agreed to sign. Ignore it."; + "requested_sighash" => %block_vote.signer_signature_hash, + ); false } None => { @@ -608,7 +614,8 @@ impl Signer { // blocks we have seen a Nonce Request for (and subsequent validation) // We are missing the context here necessary to make a decision. Reject the block debug!( - "{self}: Received a signature share request from an unknown block. Reject it." + "{self}: Received a signature share request from an unknown block. Reject it."; + "requested_sighash" => %block_vote.signer_signature_hash, ); false } diff --git a/stacks-signer/src/signerdb.rs b/stacks-signer/src/signerdb.rs index 9b211a603..fdd2ec6fe 100644 --- a/stacks-signer/src/signerdb.rs +++ b/stacks-signer/src/signerdb.rs @@ -18,6 +18,8 @@ use std::path::Path; use blockstack_lib::util_lib::db::{query_row, sqlite_open, table_exists, Error as DBError}; use rusqlite::{Connection, Error as SqliteError, OpenFlags, NO_PARAMS}; +use slog::slog_debug; +use stacks_common::debug; use stacks_common::util::hash::Sha512Trunc256Sum; use crate::signer::BlockInfo; @@ -89,6 +91,16 @@ impl SignerDb { let block_json = serde_json::to_string(&block_info).expect("Unable to serialize block info"); let hash = &block_info.signer_signature_hash(); + debug!( + "Inserting block_info: sighash = {hash}, vote = {:?}", + block_info.vote.as_ref().map(|v| { + if v.rejected { + "REJECT" + } else { + "ACCEPT" + } + }) + ); self.db .execute( "INSERT OR REPLACE INTO blocks (signer_signature_hash, block_info) VALUES (?1, ?2)", From 31d52272b76d311c2ddacb635d72d8358e7b0caf Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Fri, 8 Mar 2024 17:02:05 -0500 Subject: [PATCH 09/26] chore: cleanup excessive logging --- testnet/stacks-node/src/nakamoto_node/miner.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index dc75ef56c..e77964d3d 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -135,15 +135,11 @@ impl BlockMinerThread { /// Stop a miner tenure by blocking the miner and then joining the tenure thread pub fn stop_miner(globals: &Globals, prior_miner: JoinHandle<()>) { let id = prior_miner.thread().id(); - debug!("Blocking miner thread ID {:?}", id); globals.block_miner(); - debug!("Joining miner thread ID {:?}", id); prior_miner .join() .expect("FATAL: IO failure joining prior mining thread"); - debug!("Joined miner thread ID {:?}", id); globals.unblock_miner(); - debug!("Unblocked miner."); } pub fn run_miner(mut self, prior_miner: Option>) { @@ -155,7 +151,6 @@ impl BlockMinerThread { "parent_tenure_id" => %self.parent_tenure_id, "thread_id" => ?thread::current().id(), ); - debug!("Parent tenure ID: {:?}", self.parent_tenure_id); if let Some(prior_miner) = prior_miner { Self::stop_miner(&self.globals, prior_miner); } From dd95ad00e8fe4a62b9ebc4409e4cce8ddc659e53 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Fri, 8 Mar 2024 17:06:25 -0500 Subject: [PATCH 10/26] chore: remove unused variable --- testnet/stacks-node/src/nakamoto_node/miner.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index e77964d3d..752492286 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -134,7 +134,6 @@ impl BlockMinerThread { /// Stop a miner tenure by blocking the miner and then joining the tenure thread pub fn stop_miner(globals: &Globals, prior_miner: JoinHandle<()>) { - let id = prior_miner.thread().id(); globals.block_miner(); prior_miner .join() From 9997c1b032d33c89bd09d57514db0c273b4314b2 Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Sat, 9 Mar 2024 09:11:04 -0500 Subject: [PATCH 11/26] Cleanup outdated signers Signed-off-by: Jacinta Ferrant --- stacks-signer/src/runloop.rs | 11 +++++++++++ stacks-signer/src/signer.rs | 5 +++++ 2 files changed, 16 insertions(+) diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 4a9a8d528..a5ff642b6 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -356,7 +356,13 @@ impl SignerRunLoop, RunLoopCommand> for RunLoop { } error!("Failed to refresh signers: {e}. Signer may have an outdated view of the network. Attempting to process event anyway."); } + let mut outdated_signers = Vec::with_capacity(self.stacks_signers.len()); for signer in self.stacks_signers.values_mut() { + if signer.reward_cycle < current_reward_cycle { + debug!("{signer}: Signer's tenure has completed. Ignoring event: {event:?}"); + outdated_signers.push(signer.reward_cycle % 2); + continue; + } let event_parity = match event { Some(SignerEvent::BlockValidationResponse(_)) => Some(current_reward_cycle % 2), // Block proposal events do have reward cycles, but each proposal has its own cycle, @@ -402,6 +408,11 @@ impl SignerRunLoop, RunLoopCommand> for RunLoop { // After processing event, run the next command for each signer signer.process_next_command(&self.stacks_client); } + for i in outdated_signers.into_iter() { + if let Some(signer) = self.stacks_signers.remove(&i) { + info!("{signer}: Tenure has completed. Removing signer from runloop.",); + } + } None } } diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 5bff83629..190e4d053 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -128,6 +128,8 @@ pub enum State { Idle, /// The signer is executing a DKG or Sign round OperationInProgress, + /// The signer's reward cycle has finished + TenureCompleted, } /// The stacks signer registered for the reward cycle @@ -373,6 +375,9 @@ impl Signer { // We cannot execute the next command until the current one is finished... debug!("{self}: Waiting for coordinator {coordinator_id:?} operation to finish. Coordinator state = {:?}", self.coordinator.state); } + State::TenureCompleted => { + debug!("{self}: Tenure completed. Will not process any more commands.",); + } } } From 59f2377d690e44e35c66a19ea4999bdd6f0f84bf Mon Sep 17 00:00:00 2001 From: Jacinta Ferrant Date: Sat, 9 Mar 2024 09:22:29 -0500 Subject: [PATCH 12/26] Cleanup outdated signer within refresh signer Signed-off-by: Jacinta Ferrant --- stacks-signer/src/runloop.rs | 23 ++++++++++++++--------- stacks-signer/src/signer.rs | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index a5ff642b6..942e2dd38 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -279,7 +279,14 @@ impl RunLoop { self.refresh_signer_config(next_reward_cycle); // TODO: do not use an empty consensus hash let pox_consensus_hash = ConsensusHash::empty(); + let mut to_delete = Vec::new(); for signer in self.stacks_signers.values_mut() { + if signer.reward_cycle < current_reward_cycle { + debug!("{signer}: Signer's tenure has completed."); + // We don't really need this state, but it's useful for debugging + signer.state = SignerState::TenureCompleted; + to_delete.push(signer.reward_cycle % 2); + } let old_coordinator_id = signer.coordinator_selector.get_coordinator().0; let updated_coordinator_id = signer .coordinator_selector @@ -302,6 +309,11 @@ impl RunLoop { })?; } } + for i in to_delete.into_iter() { + if let Some(signer) = self.stacks_signers.remove(&i) { + info!("{signer}: Tenure has completed. Removing signer from runloop.",); + } + } if self.stacks_signers.is_empty() { info!("Signer is not registered for the current reward cycle ({current_reward_cycle}) or next reward cycle ({next_reward_cycle}). Waiting for confirmed registration..."); self.state = State::Uninitialized; @@ -356,11 +368,9 @@ impl SignerRunLoop, RunLoopCommand> for RunLoop { } error!("Failed to refresh signers: {e}. Signer may have an outdated view of the network. Attempting to process event anyway."); } - let mut outdated_signers = Vec::with_capacity(self.stacks_signers.len()); for signer in self.stacks_signers.values_mut() { - if signer.reward_cycle < current_reward_cycle { - debug!("{signer}: Signer's tenure has completed. Ignoring event: {event:?}"); - outdated_signers.push(signer.reward_cycle % 2); + if signer.state == SignerState::TenureCompleted { + warn!("{signer}: Signer's tenure has completed. This signer should have been cleaned up during refresh."); continue; } let event_parity = match event { @@ -408,11 +418,6 @@ impl SignerRunLoop, RunLoopCommand> for RunLoop { // After processing event, run the next command for each signer signer.process_next_command(&self.stacks_client); } - for i in outdated_signers.into_iter() { - if let Some(signer) = self.stacks_signers.remove(&i) { - info!("{signer}: Tenure has completed. Removing signer from runloop.",); - } - } None } } diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 190e4d053..4d5c47044 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -376,7 +376,7 @@ impl Signer { debug!("{self}: Waiting for coordinator {coordinator_id:?} operation to finish. Coordinator state = {:?}", self.coordinator.state); } State::TenureCompleted => { - debug!("{self}: Tenure completed. Will not process any more commands.",); + warn!("{self}: Tenure completed. This signer should have been cleaned up during refresh.",); } } } From d1d1365d55d28db66fa170e1416aea46c2060e81 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Sat, 9 Mar 2024 09:50:11 -0800 Subject: [PATCH 13/26] fix: try not deleting accepted blocks from signerDB --- stacks-signer/src/signer.rs | 12 +++++++----- stacks-signer/src/signerdb.rs | 5 ++++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 5bff83629..5dfbdc213 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -73,7 +73,7 @@ pub struct BlockInfo { /// The associated packet nonce request if we have one nonce_request: Option, /// Whether this block is already being signed over - signed_over: bool, + pub signed_over: bool, } impl BlockInfo { @@ -992,10 +992,12 @@ impl Signer { return; }; - // TODO: proper garbage collection...This is currently our only cleanup of blocks - self.signer_db - .remove_block(&block_vote.signer_signature_hash) - .expect(&format!("{self}: Failed to remove block from to signer DB")); + // WIP: try not deleting a block from signerDB until we have a better garbage collection strategy. + // This causes issues when we have to reprocess a block and we have already deleted it from the signerDB + // // TODO: proper garbage collection...This is currently our only cleanup of blocks + // self.signer_db + // .remove_block(&block_vote.signer_signature_hash) + // .expect(&format!("{self}: Failed to remove block from to signer DB")); let block_submission = if block_vote.rejected { // We signed a rejection message. Return a rejection message diff --git a/stacks-signer/src/signerdb.rs b/stacks-signer/src/signerdb.rs index fdd2ec6fe..bd24804ec 100644 --- a/stacks-signer/src/signerdb.rs +++ b/stacks-signer/src/signerdb.rs @@ -91,8 +91,10 @@ impl SignerDb { 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: sighash = {hash}, vote = {:?}", + "Inserting block_info: sighash = {hash}, block_id = {block_id}, signed = {signed_over} vote = {:?}", block_info.vote.as_ref().map(|v| { if v.rejected { "REJECT" @@ -117,6 +119,7 @@ impl SignerDb { /// Remove a block pub fn remove_block(&mut self, hash: &Sha512Trunc256Sum) -> Result<(), DBError> { + debug!("Deleting block_info: sighash = {hash}"); self.db.execute( "DELETE FROM blocks WHERE signer_signature_hash = ?", &[format!("{}", hash)], From 1d0929977a0999ce04732359aaae74488ba07c66 Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Sat, 9 Mar 2024 15:20:16 -0600 Subject: [PATCH 14/26] feat: signer does not treat repeated proposals as new --- stacks-signer/src/signer.rs | 56 +++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 15 deletions(-) diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 5dfbdc213..1e35f167d 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -21,7 +21,7 @@ use std::time::Instant; use blockstack_lib::chainstate::nakamoto::signer_set::NakamotoSigners; use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockVote}; use blockstack_lib::chainstate::stacks::boot::SIGNERS_VOTING_FUNCTION_NAME; -use blockstack_lib::chainstate::stacks::StacksTransaction; +use blockstack_lib::chainstate::stacks::{StacksTransaction, ThresholdSignature}; use blockstack_lib::net::api::postblock_proposal::BlockValidateResponse; use hashbrown::HashSet; use libsigner::{ @@ -510,25 +510,51 @@ impl Signer { for proposal in proposals { if proposal.reward_cycle != self.reward_cycle { debug!( - "Signer #{}: Received proposal for block outside of my reward cycle, ignoring.", - self.signer_id; + "{self}: Received proposal for block outside of my reward cycle, ignoring."; "proposal_reward_cycle" => proposal.reward_cycle, "proposal_burn_height" => proposal.burn_height, ); continue; } - // Store the block in our cache - self.signer_db - .insert_block(&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:?}"); - }); + let sig_hash = proposal.block.header.signer_signature_hash(); + match self.signer_db.block_lookup(&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(&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; + } + } } } From d24e1a06221c004c48f176b05fdae2ff8e2fe076 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Sat, 9 Mar 2024 16:50:52 -0500 Subject: [PATCH 15/26] chore: use index to make the code more readable --- stacks-signer/src/runloop.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 942e2dd38..56e95a631 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -280,12 +280,12 @@ impl RunLoop { // TODO: do not use an empty consensus hash let pox_consensus_hash = ConsensusHash::empty(); let mut to_delete = Vec::new(); - for signer in self.stacks_signers.values_mut() { + for (idx, signer) in &mut self.stacks_signers { if signer.reward_cycle < current_reward_cycle { debug!("{signer}: Signer's tenure has completed."); // We don't really need this state, but it's useful for debugging signer.state = SignerState::TenureCompleted; - to_delete.push(signer.reward_cycle % 2); + to_delete.push(*idx); } let old_coordinator_id = signer.coordinator_selector.get_coordinator().0; let updated_coordinator_id = signer From f077e08d11829594e91d0f9663e2a0ec76a4d1ad Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Sun, 10 Mar 2024 15:35:50 -0400 Subject: [PATCH 16/26] fix: use http/1.1, not http/1.0 --- libsigner/src/events.rs | 2 +- libsigner/src/http.rs | 4 ++-- libsigner/src/tests/mod.rs | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libsigner/src/events.rs b/libsigner/src/events.rs index ce26447e5..009a741bf 100644 --- a/libsigner/src/events.rs +++ b/libsigner/src/events.rs @@ -226,7 +226,7 @@ impl EventStopSignaler for SignerStopSignaler { // We need to send actual data to trigger the event receiver let body = "Yo. Shut this shit down!".to_string(); let req = format!( - "POST /shutdown HTTP/1.0\r\nContent-Length: {}\r\n\r\n{}", + "POST /shutdown HTTP/1.1\r\nContent-Length: {}\r\n\r\n{}", &body.len(), body ); diff --git a/libsigner/src/http.rs b/libsigner/src/http.rs index 95f2e2b3c..fe841415a 100644 --- a/libsigner/src/http.rs +++ b/libsigner/src/http.rs @@ -238,12 +238,12 @@ pub fn run_http_request( let req_txt = if let Some(content_type) = content_type { format!( - "{} {} HTTP/1.0\r\nHost: {}\r\nConnection: close\r\nContent-Type: {}\r\n{}User-Agent: libsigner/0.1\r\nAccept: */*\r\n\r\n", + "{} {} HTTP/1.1\r\nHost: {}\r\nConnection: close\r\nContent-Type: {}\r\n{}User-Agent: libsigner/0.1\r\nAccept: */*\r\n\r\n", verb, path, host, content_type, content_length_hdr ) } else { format!( - "{} {} HTTP/1.0\r\nHost: {}\r\nConnection: close\r\n{}User-Agent: libsigner/0.1\r\nAccept: */*\r\n\r\n", + "{} {} HTTP/1.1\r\nHost: {}\r\nConnection: close\r\n{}User-Agent: libsigner/0.1\r\nAccept: */*\r\n\r\n", verb, path, host, content_length_hdr ) }; diff --git a/libsigner/src/tests/mod.rs b/libsigner/src/tests/mod.rs index 9f320b42f..b63203455 100644 --- a/libsigner/src/tests/mod.rs +++ b/libsigner/src/tests/mod.rs @@ -135,7 +135,7 @@ fn test_simple_signer() { let ev = &thread_chunks[num_sent]; let body = serde_json::to_string(ev).unwrap(); - let req = format!("POST /stackerdb_chunks HTTP/1.0\r\nConnection: close\r\nContent-Length: {}\r\n\r\n{}", &body.len(), body); + let req = format!("POST /stackerdb_chunks HTTP/1.1\r\nConnection: close\r\nContent-Length: {}\r\n\r\n{}", &body.len(), body); debug!("Send:\n{}", &req); sock.write_all(req.as_bytes()).unwrap(); @@ -188,13 +188,13 @@ fn test_status_endpoint() { return; } }; - let req = "GET /status HTTP/1.0\r\nConnection: close\r\n\r\n"; + let req = "GET /status HTTP/1.1\r\nConnection: close\r\n\r\n"; sock.write_all(req.as_bytes()).unwrap(); let mut buf = [0; 128]; let _ = sock.read(&mut buf).unwrap(); let res_str = std::str::from_utf8(&buf).unwrap(); - let expected_status_res = "HTTP/1.0 200 OK\r\n"; + let expected_status_res = "HTTP/1.1 200 OK\r\n"; assert_eq!(expected_status_res, &res_str[..expected_status_res.len()]); sock.flush().unwrap(); }); From a1c226c308aae10ee133de1329aff3cad1c482fa Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Mon, 11 Mar 2024 08:22:55 -0700 Subject: [PATCH 17/26] feat: select coordinator just from pubkey alone --- stacks-signer/src/coordinator.rs | 18 +++++++++--------- stacks-signer/src/signer.rs | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/stacks-signer/src/coordinator.rs b/stacks-signer/src/coordinator.rs index 234d1ade8..fc582ec77 100644 --- a/stacks-signer/src/coordinator.rs +++ b/stacks-signer/src/coordinator.rs @@ -20,7 +20,6 @@ use blockstack_lib::chainstate::burn::ConsensusHashExtensions; use slog::slog_debug; use stacks_common::debug; use stacks_common::types::chainstate::ConsensusHash; -use stacks_common::util::hash::Sha256Sum; use wsts::curve::ecdsa; use wsts::state_machine::PublicKeys; @@ -136,7 +135,7 @@ impl CoordinatorSelector { ) } - /// Calculate the ordered list of coordinator ids by comparing the provided public keys against the pox consensus hash + /// Calculate the ordered list of coordinator ids by comparing the provided public keys pub fn calculate_coordinator_ids( public_keys: &PublicKeys, pox_consensus_hash: &ConsensusHash, @@ -147,13 +146,14 @@ impl CoordinatorSelector { .signers .iter() .map(|(&id, pk)| { - let pk_bytes = pk.to_bytes(); - let mut buffer = - Vec::with_capacity(pk_bytes.len() + pox_consensus_hash.as_bytes().len()); - buffer.extend_from_slice(&pk_bytes[..]); - buffer.extend_from_slice(pox_consensus_hash.as_bytes()); - let digest = Sha256Sum::from_data(&buffer).as_bytes().to_vec(); - (id, digest) + (id, pk.to_bytes()) + // WIP: removing this to try and improve stability / debugability of coordinator selection + // let mut buffer = + // Vec::with_capacity(pk_bytes.len() + pox_consensus_hash.as_bytes().len()); + // buffer.extend_from_slice(&pk_bytes[..]); + // buffer.extend_from_slice(pox_consensus_hash.as_bytes()); + // let digest = Sha256Sum::from_data(&buffer).as_bytes().to_vec(); + // (id, digest) }) .collect::>(); diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 26701c254..632305300 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -21,7 +21,7 @@ use std::time::Instant; use blockstack_lib::chainstate::nakamoto::signer_set::NakamotoSigners; use blockstack_lib::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockVote}; use blockstack_lib::chainstate::stacks::boot::SIGNERS_VOTING_FUNCTION_NAME; -use blockstack_lib::chainstate::stacks::{StacksTransaction, ThresholdSignature}; +use blockstack_lib::chainstate::stacks::StacksTransaction; use blockstack_lib::net::api::postblock_proposal::BlockValidateResponse; use hashbrown::HashSet; use libsigner::{ From 830b10dc57681e9fa6ff8edb19a17a6d21ccb380 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Sun, 10 Mar 2024 19:59:19 -0400 Subject: [PATCH 18/26] chore: cleanup unused field --- .../stacks-node/src/nakamoto_node/miner.rs | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index 752492286..088299083 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -39,7 +39,6 @@ use stacks::chainstate::stacks::{ TenureChangeCause, TenureChangePayload, ThresholdSignature, TransactionAnchorMode, TransactionPayload, TransactionVersion, }; -use stacks::core::FIRST_BURNCHAIN_CONSENSUS_HASH; use stacks::net::stackerdb::StackerDBs; use stacks_common::codec::{read_next, StacksMessageCodec}; use stacks_common::types::chainstate::{StacksAddress, StacksBlockId}; @@ -83,8 +82,6 @@ struct ParentTenureInfo { struct ParentStacksBlockInfo { /// Header metadata for the Stacks block we're going to build on top of stacks_parent_header: StacksHeaderInfo, - /// the total amount burned in the sortition that selected the Stacks block parent - parent_block_total_burn: u64, /// nonce to use for this new block's coinbase transaction coinbase_nonce: u64, parent_tenure: Option, @@ -692,7 +689,6 @@ impl BlockMinerThread { parent_tenure_blocks: 0, }), stacks_parent_header: chain_tip.metadata, - parent_block_total_burn: 0, coinbase_nonce: 0, }); }; @@ -915,26 +911,6 @@ impl ParentStacksBlockInfo { .expect("Failed to look up block's parent snapshot") .expect("Failed to look up block's parent snapshot"); - let parent_sortition_id = &parent_snapshot.sortition_id; - - let parent_block_total_burn = - if &stacks_tip_header.consensus_hash == &FIRST_BURNCHAIN_CONSENSUS_HASH { - 0 - } else { - let parent_burn_block = - SortitionDB::get_block_snapshot(burn_db.conn(), parent_sortition_id) - .expect("SortitionDB failure.") - .ok_or_else(|| { - error!( - "Failed to find block snapshot for the parent sortition"; - "parent_sortition_id" => %parent_sortition_id - ); - NakamotoNodeError::SnapshotNotFoundForChainTip - })?; - - parent_burn_block.total_burn - }; - // don't mine off of an old burnchain block let burn_chain_tip = SortitionDB::get_canonical_burn_chain_tip(burn_db.conn()) .expect("FATAL: failed to query sortition DB for canonical burn chain tip"); @@ -1029,7 +1005,6 @@ impl ParentStacksBlockInfo { Ok(ParentStacksBlockInfo { stacks_parent_header: stacks_tip_header, - parent_block_total_burn, coinbase_nonce, parent_tenure: parent_tenure_info, }) From fd1750108c00005fa1238fc4894f1e716bd45174 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Mon, 11 Mar 2024 11:19:10 -0400 Subject: [PATCH 19/26] feat: skip rest of loop when signer's tenure has completed --- stacks-signer/src/runloop.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 56e95a631..359b5a18f 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -286,6 +286,7 @@ impl RunLoop { // We don't really need this state, but it's useful for debugging signer.state = SignerState::TenureCompleted; to_delete.push(*idx); + continue; } let old_coordinator_id = signer.coordinator_selector.get_coordinator().0; let updated_coordinator_id = signer From 6daf9730c2d23f94cfac85d5ba4df5d57ccb52ae Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Mon, 11 Mar 2024 11:37:37 -0400 Subject: [PATCH 20/26] logs: quiet warning when signer is not registered for future cycle --- stacks-signer/src/runloop.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 359b5a18f..0d76a36ee 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -229,7 +229,7 @@ impl RunLoop { } /// Refresh signer configuration for a specific reward cycle - fn refresh_signer_config(&mut self, reward_cycle: u64) { + fn refresh_signer_config(&mut self, reward_cycle: u64, current: bool) { let reward_index = reward_cycle % 2; let mut needs_refresh = false; if let Some(signer) = self.stacks_signers.get_mut(&reward_index) { @@ -266,7 +266,11 @@ impl RunLoop { .insert(reward_index, Signer::from(new_signer_config)); debug!("Reward cycle #{reward_cycle} Signer #{signer_id} initialized."); } else { - warn!("Signer is not registered for reward cycle {reward_cycle}. Waiting for confirmed registration..."); + if current { + warn!("Signer is not registered for the current reward cycle ({reward_cycle}). Waiting for confirmed registration..."); + } else { + debug!("Signer is not registered for reward cycle {reward_cycle}. Waiting for confirmed registration..."); + } } } } @@ -275,8 +279,8 @@ impl RunLoop { /// Note: this will trigger DKG if required fn refresh_signers(&mut self, current_reward_cycle: u64) -> Result<(), ClientError> { let next_reward_cycle = current_reward_cycle.saturating_add(1); - self.refresh_signer_config(current_reward_cycle); - self.refresh_signer_config(next_reward_cycle); + self.refresh_signer_config(current_reward_cycle, true); + self.refresh_signer_config(next_reward_cycle, false); // TODO: do not use an empty consensus hash let pox_consensus_hash = ConsensusHash::empty(); let mut to_delete = Vec::new(); From 17656a3863ef531bda6faa66fad9ba0ae582346e Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Mon, 11 Mar 2024 13:15:58 -0400 Subject: [PATCH 21/26] chore: ensure all http/1.1 requests are properly structured --- libsigner/src/events.rs | 10 +++++++--- libsigner/src/tests/mod.rs | 12 ++++++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/libsigner/src/events.rs b/libsigner/src/events.rs index 009a741bf..1c29ec941 100644 --- a/libsigner/src/events.rs +++ b/libsigner/src/events.rs @@ -226,11 +226,15 @@ impl EventStopSignaler for SignerStopSignaler { // We need to send actual data to trigger the event receiver let body = "Yo. Shut this shit down!".to_string(); let req = format!( - "POST /shutdown HTTP/1.1\r\nContent-Length: {}\r\n\r\n{}", - &body.len(), + "POST /shutdown HTTP/1.1\r\nHost: {}\r\nConnection: close\r\nContent-Length: {}\r\nContent-Type: text/plain\r\n\r\n{}", + self.local_addr, + body.len(), body ); - stream.write_all(req.as_bytes()).unwrap(); + match stream.write_all(req.as_bytes()) { + Err(e) => error!("Failed to send shutdown request: {}", e), + _ => (), + }; } } } diff --git a/libsigner/src/tests/mod.rs b/libsigner/src/tests/mod.rs index b63203455..1d3e1f3cc 100644 --- a/libsigner/src/tests/mod.rs +++ b/libsigner/src/tests/mod.rs @@ -135,7 +135,12 @@ fn test_simple_signer() { let ev = &thread_chunks[num_sent]; let body = serde_json::to_string(ev).unwrap(); - let req = format!("POST /stackerdb_chunks HTTP/1.1\r\nConnection: close\r\nContent-Length: {}\r\n\r\n{}", &body.len(), body); + let req = format!( + "POST /stackerdb_chunks HTTP/1.1\r\nHost: {}\r\nConnection: close\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}", + endpoint, + &body.len(), + body + ); debug!("Send:\n{}", &req); sock.write_all(req.as_bytes()).unwrap(); @@ -188,7 +193,10 @@ fn test_status_endpoint() { return; } }; - let req = "GET /status HTTP/1.1\r\nConnection: close\r\n\r\n"; + let req = format!( + "GET /status HTTP/1.1\r\nHost: {}\r\nConnection: close\r\n\r\n", + endpoint + ); sock.write_all(req.as_bytes()).unwrap(); let mut buf = [0; 128]; From 7891f7e3f005d3111acb6f3c6723e6a6c4c979d8 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Mon, 11 Mar 2024 10:18:57 -0700 Subject: [PATCH 22/26] feat: add const to determine whether to rotate coordinator --- stacks-signer/src/coordinator.rs | 35 +++++++++++++++++++------------- 1 file changed, 21 insertions(+), 14 deletions(-) diff --git a/stacks-signer/src/coordinator.rs b/stacks-signer/src/coordinator.rs index fc582ec77..3f82d1e49 100644 --- a/stacks-signer/src/coordinator.rs +++ b/stacks-signer/src/coordinator.rs @@ -20,6 +20,7 @@ use blockstack_lib::chainstate::burn::ConsensusHashExtensions; use slog::slog_debug; use stacks_common::debug; use stacks_common::types::chainstate::ConsensusHash; +use stacks_common::util::hash::Sha256Sum; use wsts::curve::ecdsa; use wsts::state_machine::PublicKeys; @@ -68,6 +69,9 @@ impl From for CoordinatorSelector { } } +/// Whether or not to rotate to new coordinators in `update_coordinator` +const ROTATE_COORDINATORS: bool = false; + impl CoordinatorSelector { /// Update the coordinator id fn update_coordinator(&mut self, new_coordinator_ids: Vec) { @@ -80,7 +84,7 @@ impl CoordinatorSelector { .coordinator_ids .first() .expect("FATAL: No registered signers"); - if new_coordinator_id == self.coordinator_id { + if ROTATE_COORDINATORS && new_coordinator_id == self.coordinator_id { // If the newly selected coordinator is the same as the current and we have more than one available, advance immediately to the next if self.coordinator_ids.len() > 1 { new_index = new_index.saturating_add(1); @@ -88,12 +92,16 @@ impl CoordinatorSelector { } new_index } else { - let mut new_index = self.coordinator_index.saturating_add(1); - if new_index == self.coordinator_ids.len() { - // We have exhausted all potential coordinators. Go back to the start - new_index = 0; + if ROTATE_COORDINATORS { + let mut new_index = self.coordinator_index.saturating_add(1); + if new_index == self.coordinator_ids.len() { + // We have exhausted all potential coordinators. Go back to the start + new_index = 0; + } + new_index + } else { + self.coordinator_index } - new_index }; self.coordinator_id = *self .coordinator_ids @@ -146,14 +154,13 @@ impl CoordinatorSelector { .signers .iter() .map(|(&id, pk)| { - (id, pk.to_bytes()) - // WIP: removing this to try and improve stability / debugability of coordinator selection - // let mut buffer = - // Vec::with_capacity(pk_bytes.len() + pox_consensus_hash.as_bytes().len()); - // buffer.extend_from_slice(&pk_bytes[..]); - // buffer.extend_from_slice(pox_consensus_hash.as_bytes()); - // let digest = Sha256Sum::from_data(&buffer).as_bytes().to_vec(); - // (id, digest) + let pk_bytes = pk.to_bytes(); + let mut buffer = + Vec::with_capacity(pk_bytes.len() + pox_consensus_hash.as_bytes().len()); + buffer.extend_from_slice(&pk_bytes[..]); + buffer.extend_from_slice(pox_consensus_hash.as_bytes()); + let digest = Sha256Sum::from_data(&buffer).as_bytes().to_vec(); + (id, digest) }) .collect::>(); From 3202e2748963aa0c0da330727f1fe28f5620a51b Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Mon, 11 Mar 2024 16:05:51 -0400 Subject: [PATCH 23/26] fix: use two different blocks for signing test --- testnet/stacks-node/src/tests/signer.rs | 60 +++++++++++++++++++------ 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/testnet/stacks-node/src/tests/signer.rs b/testnet/stacks-node/src/tests/signer.rs index ebdef33d4..fb867db0a 100644 --- a/testnet/stacks-node/src/tests/signer.rs +++ b/testnet/stacks-node/src/tests/signer.rs @@ -965,8 +965,8 @@ fn stackerdb_sign() { info!("------------------------- Test Setup -------------------------"); - info!("Creating an invalid block to sign..."); - let header = NakamotoBlockHeader { + info!("Creating invalid blocks to sign..."); + let header1 = NakamotoBlockHeader { version: 1, chain_length: 2, burn_spent: 3, @@ -978,12 +978,12 @@ fn stackerdb_sign() { signer_signature: ThresholdSignature::empty(), signer_bitvec: BitVec::zeros(1).unwrap(), }; - let mut block = NakamotoBlock { - header, + let mut block1 = NakamotoBlock { + header: header1, txs: vec![], }; - let tx_merkle_root = { - let txid_vecs = block + let tx_merkle_root1 = { + let txid_vecs = block1 .txs .iter() .map(|tx| tx.txid().as_bytes().to_vec()) @@ -991,14 +991,46 @@ fn stackerdb_sign() { MerkleTree::::new(&txid_vecs).root() }; - block.header.tx_merkle_root = tx_merkle_root; + block1.header.tx_merkle_root = tx_merkle_root1; + + let header2 = NakamotoBlockHeader { + version: 1, + chain_length: 3, + burn_spent: 4, + consensus_hash: ConsensusHash([0x05; 20]), + parent_block_id: StacksBlockId([0x06; 32]), + tx_merkle_root: Sha512Trunc256Sum([0x07; 32]), + state_index_root: TrieHash([0x08; 32]), + miner_signature: MessageSignature::empty(), + signer_signature: ThresholdSignature::empty(), + signer_bitvec: BitVec::zeros(1).unwrap(), + }; + let mut block2 = NakamotoBlock { + header: header2, + txs: vec![], + }; + let tx_merkle_root2 = { + let txid_vecs = block2 + .txs + .iter() + .map(|tx| tx.txid().as_bytes().to_vec()) + .collect(); + + MerkleTree::::new(&txid_vecs).root() + }; + block2.header.tx_merkle_root = tx_merkle_root2; // The block is invalid so the signers should return a signature across a rejection - let block_vote = NakamotoBlockVote { - signer_signature_hash: block.header.signer_signature_hash(), + let block1_vote = NakamotoBlockVote { + signer_signature_hash: block1.header.signer_signature_hash(), rejected: true, }; - let msg = block_vote.serialize_to_vec(); + let msg1 = block1_vote.serialize_to_vec(); + let block2_vote = NakamotoBlockVote { + signer_signature_hash: block2.header.signer_signature_hash(), + rejected: true, + }; + let msg2 = block2_vote.serialize_to_vec(); let timeout = Duration::from_secs(200); let mut signer_test = SignerTest::new(10); @@ -1012,7 +1044,7 @@ fn stackerdb_sign() { let sign_command = RunLoopCommand { reward_cycle, command: SignerCommand::Sign { - block: block.clone(), + block: block1, is_taproot: false, merkle_root: None, }, @@ -1020,7 +1052,7 @@ fn stackerdb_sign() { let sign_taproot_command = RunLoopCommand { reward_cycle, command: SignerCommand::Sign { - block: block.clone(), + block: block2, is_taproot: true, merkle_root: None, }, @@ -1037,12 +1069,12 @@ fn stackerdb_sign() { let schnorr_proofs = signer_test.wait_for_taproot_signatures(timeout); for frost_signature in frost_signatures { - assert!(frost_signature.verify(&key, &msg)); + assert!(frost_signature.verify(&key, &msg1)); } for schnorr_proof in schnorr_proofs { let tweaked_key = tweaked_public_key(&key, None); assert!( - schnorr_proof.verify(&tweaked_key.x(), &msg), + schnorr_proof.verify(&tweaked_key.x(), &msg2), "Schnorr proof verification failed" ); } From 481b01c5363ee7319a7f676c308a41d8ae34e3d1 Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Tue, 12 Mar 2024 10:28:10 -0400 Subject: [PATCH 24/26] fix: add reward cycle to signerdb The blocks should always be associated with a specific reward cycle so that the signers do not take actions on blocks from the previous cycle. --- stacks-signer/src/signer.rs | 32 +++++++++++-------- stacks-signer/src/signerdb.rs | 58 ++++++++++++++++++++++++----------- 2 files changed, 59 insertions(+), 31 deletions(-) diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 632305300..3de6cc13a 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -317,7 +317,7 @@ impl Signer { let signer_signature_hash = block.header.signer_signature_hash(); let mut block_info = self .signer_db - .block_lookup(&signer_signature_hash) + .block_lookup(self.reward_cycle, &signer_signature_hash) .unwrap_or_else(|_| Some(BlockInfo::new(block.clone()))) .unwrap_or_else(|| BlockInfo::new(block.clone())); if block_info.signed_over { @@ -339,7 +339,7 @@ impl Signer { debug!("{self}: ACK: {ack:?}",); block_info.signed_over = true; self.signer_db - .insert_block(&block_info) + .insert_block(self.reward_cycle, &block_info) .unwrap_or_else(|e| { error!("{self}: Failed to insert block in DB: {e:?}"); }); @@ -392,7 +392,10 @@ impl Signer { BlockValidateResponse::Ok(block_validate_ok) => { let signer_signature_hash = block_validate_ok.signer_signature_hash; // For mutability reasons, we need to take the block_info out of the map and add it back after processing - let mut block_info = match self.signer_db.block_lookup(&signer_signature_hash) { + let mut block_info = match self + .signer_db + .block_lookup(self.reward_cycle, &signer_signature_hash) + { Ok(Some(block_info)) => block_info, Ok(None) => { // We have not seen this block before. Why are we getting a response for it? @@ -407,7 +410,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(&block_info) + .insert_block(self.reward_cycle, &block_info) .expect(&format!("{self}: Failed to insert block in DB")); info!( "{self}: Treating block validation for block {} as valid: {:?}", @@ -418,7 +421,10 @@ impl Signer { } BlockValidateResponse::Reject(block_validate_reject) => { let signer_signature_hash = block_validate_reject.signer_signature_hash; - let mut block_info = match self.signer_db.block_lookup(&signer_signature_hash) { + let mut block_info = match self + .signer_db + .block_lookup(self.reward_cycle, &signer_signature_hash) + { Ok(Some(block_info)) => block_info, Ok(None) => { // We have not seen this block before. Why are we getting a response for it? @@ -481,7 +487,7 @@ impl Signer { } } self.signer_db - .insert_block(&block_info) + .insert_block(self.reward_cycle, &block_info) .expect(&format!("{self}: Failed to insert block in DB")); } @@ -522,7 +528,7 @@ impl Signer { continue; } let sig_hash = proposal.block.header.signer_signature_hash(); - match self.signer_db.block_lookup(&sig_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."; @@ -542,7 +548,7 @@ impl Signer { Ok(None) => { // Store the block in our cache self.signer_db - .insert_block(&BlockInfo::new(proposal.block.clone())) + .insert_block(self.reward_cycle, &BlockInfo::new(proposal.block.clone())) .unwrap_or_else(|e| { error!("{self}: Failed to insert block in DB: {e:?}"); }); @@ -617,7 +623,7 @@ impl Signer { match self .signer_db - .block_lookup(&block_vote.signer_signature_hash) + .block_lookup(self.reward_cycle, &block_vote.signer_signature_hash) .expect(&format!("{self}: Failed to connect to DB")) .map(|b| b.vote) { @@ -671,7 +677,7 @@ impl Signer { let signer_signature_hash = block.header.signer_signature_hash(); let mut block_info = match self .signer_db - .block_lookup(&signer_signature_hash) + .block_lookup(self.reward_cycle, &signer_signature_hash) .expect("Failed to connect to signer DB") { Some(block_info) => block_info, @@ -679,7 +685,7 @@ impl Signer { 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..."); let block_info = BlockInfo::new_with_request(block.clone(), nonce_request.clone()); self.signer_db - .insert_block(&block_info) + .insert_block(self.reward_cycle, &block_info) .expect(&format!("{self}: Failed to insert block in DB")); stacks_client .submit_block_for_validation(block) @@ -699,7 +705,7 @@ impl Signer { self.determine_vote(&mut block_info, nonce_request); self.signer_db - .insert_block(&block_info) + .insert_block(self.reward_cycle, &block_info) .expect(&format!("{self}: Failed to insert block in DB")); true } @@ -1066,7 +1072,7 @@ impl Signer { }; let Some(block_info) = self .signer_db - .block_lookup(&block_vote.signer_signature_hash) + .block_lookup(self.reward_cycle, &block_vote.signer_signature_hash) .expect(&format!("{self}: Failed to connect to signer DB")) else { debug!( diff --git a/stacks-signer/src/signerdb.rs b/stacks-signer/src/signerdb.rs index bd24804ec..052025b91 100644 --- a/stacks-signer/src/signerdb.rs +++ b/stacks-signer/src/signerdb.rs @@ -34,8 +34,10 @@ pub struct SignerDb { const CREATE_BLOCKS_TABLE: &'static str = " CREATE TABLE IF NOT EXISTS blocks ( - signer_signature_hash TEXT PRIMARY KEY, - block_info TEXT NOT NULL + reward_cycle INTEGER NOT NULL, + signer_signature_hash TEXT NOT NULL, + block_info TEXT NOT NULL, + PRIMARY KEY (reward_cycle, signer_signature_hash) )"; impl SignerDb { @@ -70,11 +72,15 @@ impl SignerDb { /// Fetch a block from the database using the block's /// `signer_signature_hash` - pub fn block_lookup(&self, hash: &Sha512Trunc256Sum) -> Result, DBError> { + pub fn block_lookup( + &self, + reward_cycle: u64, + hash: &Sha512Trunc256Sum, + ) -> Result, DBError> { let result: Option = query_row( &self.db, - "SELECT block_info FROM blocks WHERE signer_signature_hash = ?", - &[format!("{}", hash)], + "SELECT block_info FROM blocks WHERE reward_cycle = ? AND signer_signature_hash = ?", + &[&reward_cycle.to_string(), &format!("{}", hash)], )?; if let Some(block_info) = result { let block_info: BlockInfo = @@ -87,14 +93,18 @@ impl SignerDb { /// Insert a block into the database. /// `hash` is the `signer_signature_hash` of the block. - pub fn insert_block(&mut self, block_info: &BlockInfo) -> Result<(), DBError> { + pub fn insert_block( + &mut self, + reward_cycle: u64, + 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: sighash = {hash}, block_id = {block_id}, signed = {signed_over} vote = {:?}", + "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" @@ -105,8 +115,8 @@ impl SignerDb { ); self.db .execute( - "INSERT OR REPLACE INTO blocks (signer_signature_hash, block_info) VALUES (?1, ?2)", - &[format!("{}", hash), block_json], + "INSERT OR REPLACE INTO blocks (reward_cycle, signer_signature_hash, block_info) VALUES (?1, ?2, ?3)", + &[reward_cycle.to_string(), format!("{}", hash), block_json], ) .map_err(|e| { return DBError::Other(format!( @@ -118,11 +128,15 @@ impl SignerDb { } /// Remove a block - pub fn remove_block(&mut self, hash: &Sha512Trunc256Sum) -> Result<(), DBError> { + pub fn remove_block( + &mut self, + reward_cycle: u64, + hash: &Sha512Trunc256Sum, + ) -> Result<(), DBError> { debug!("Deleting block_info: sighash = {hash}"); self.db.execute( - "DELETE FROM blocks WHERE signer_signature_hash = ?", - &[format!("{}", hash)], + "DELETE FROM blocks WHERE reward_cycle = ? AND signer_signature_hash = ?", + &[reward_cycle.to_string(), format!("{}", hash)], )?; Ok(()) @@ -193,16 +207,23 @@ 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(&block_info) + db.insert_block(reward_cycle, &block_info) .expect("Unable to insert block into db"); let block_info = db - .block_lookup(&block.header.signer_signature_hash()) + .block_lookup(reward_cycle, &block.header.signer_signature_hash()) .unwrap() .expect("Unable to get block from db"); assert_eq!(BlockInfo::new(block.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()) + .unwrap(); + assert!(block_info.is_none()); } #[test] @@ -220,12 +241,13 @@ 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(&block_info) + db.insert_block(reward_cycle, &block_info) .expect("Unable to insert block into db"); let block_info = db - .block_lookup(&block.header.signer_signature_hash()) + .block_lookup(reward_cycle, &block.header.signer_signature_hash()) .unwrap() .expect("Unable to get block from db"); @@ -246,11 +268,11 @@ mod tests { rejected: false, }; block_info.vote = Some(vote.clone()); - db.insert_block(&block_info) + db.insert_block(reward_cycle, &block_info) .expect("Unable to insert block into db"); let block_info = db - .block_lookup(&block.header.signer_signature_hash()) + .block_lookup(reward_cycle, &block.header.signer_signature_hash()) .unwrap() .expect("Unable to get block from db"); From 49eb61a705e7f64be204ecdf15fcca3bd623dabc Mon Sep 17 00:00:00 2001 From: Brice Dobry Date: Wed, 13 Mar 2024 16:03:41 -0400 Subject: [PATCH 25/26] chore: delete commented code We decided that since the signerdb is in the filesystem, we do not need to garbage collect it. --- stacks-signer/src/signer.rs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 3de6cc13a..65c32dc1c 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -1029,13 +1029,6 @@ impl Signer { return; }; - // WIP: try not deleting a block from signerDB until we have a better garbage collection strategy. - // This causes issues when we have to reprocess a block and we have already deleted it from the signerDB - // // TODO: proper garbage collection...This is currently our only cleanup of blocks - // self.signer_db - // .remove_block(&block_vote.signer_signature_hash) - // .expect(&format!("{self}: Failed to remove block from to signer DB")); - let block_submission = if block_vote.rejected { // We signed a rejection message. Return a rejection message BlockResponse::rejected(block_vote.signer_signature_hash, signature.clone()) From 6e4d9cce7305d2c47504ec00cead5ba1a159a3ed Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Fri, 15 Mar 2024 11:43:15 -0500 Subject: [PATCH 26/26] chore: add TODO for tracking burnchain/stacks views --- stacks-signer/src/runloop.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 0d76a36ee..607bb8489 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -266,6 +266,7 @@ impl RunLoop { .insert(reward_index, Signer::from(new_signer_config)); debug!("Reward cycle #{reward_cycle} Signer #{signer_id} initialized."); } else { + // TODO: Update `current` here once the signer binary is tracking its own latest burnchain/stacks views. if current { warn!("Signer is not registered for the current reward cycle ({reward_cycle}). Waiting for confirmed registration..."); } else {