From 650c86cf8925a69f37432ea45b9f97522c7fc13d Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Mon, 20 May 2024 16:59:36 -0700 Subject: [PATCH 1/9] feat: gather v0 block signatures from stackerdb --- .github/workflows/bitcoin-tests.yml | 1 + .../stacks-node/src/nakamoto_node/miner.rs | 4 +- .../src/nakamoto_node/sign_coordinator.rs | 168 ++++++++++++++++-- testnet/stacks-node/src/tests/signer/mod.rs | 71 ++++++-- testnet/stacks-node/src/tests/signer/v0.rs | 86 ++++++++- testnet/stacks-node/src/tests/signer/v1.rs | 7 +- 6 files changed, 294 insertions(+), 43 deletions(-) diff --git a/.github/workflows/bitcoin-tests.yml b/.github/workflows/bitcoin-tests.yml index 87fe5a8f09..74b4074b8e 100644 --- a/.github/workflows/bitcoin-tests.yml +++ b/.github/workflows/bitcoin-tests.yml @@ -83,6 +83,7 @@ jobs: - tests::nakamoto_integrations::follower_bootup - tests::nakamoto_integrations::forked_tenure_is_ignored - tests::signer::v0::block_proposal_rejection + - tests::signer::v0::miner_gather_signatures - tests::signer::v1::dkg - tests::signer::v1::sign_request_rejected - tests::signer::v1::filter_bad_transactions diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index 3ed642c9cd..29631cdec0 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -310,7 +310,7 @@ impl BlockMinerThread { &reward_set, reward_cycle, miner_privkey_as_scalar, - aggregate_public_key, + Some(aggregate_public_key), &stackerdbs, &self.config, ) @@ -395,7 +395,7 @@ impl BlockMinerThread { &reward_set, reward_cycle, miner_privkey_as_scalar, - Point::new(), + None, &stackerdbs, &self.config, ) diff --git a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs index c0f42e7820..3cf1c6d144 100644 --- a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs +++ b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs @@ -13,17 +13,18 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . +use std::collections::BTreeMap; use std::sync::mpsc::Receiver; use std::time::{Duration, Instant}; use hashbrown::{HashMap, HashSet}; -use libsigner::v0::messages::SignerMessage as SignerMessageV0; +use libsigner::v0::messages::{BlockResponse, SignerMessage as SignerMessageV0}; use libsigner::v1::messages::{MessageSlotID, SignerMessage as SignerMessageV1}; use libsigner::{BlockProposal, SignerEntries, SignerEvent, SignerSession, StackerDBSession}; use stacks::burnchains::Burnchain; use stacks::chainstate::burn::db::sortdb::SortitionDB; use stacks::chainstate::burn::BlockSnapshot; -use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoChainState}; +use stacks::chainstate::nakamoto::{NakamotoBlock, NakamotoBlockHeader, NakamotoChainState}; use stacks::chainstate::stacks::boot::{NakamotoSignerEntry, RewardSet, MINERS_NAME, SIGNERS_NAME}; use stacks::chainstate::stacks::events::StackerDBChunksEvent; use stacks::chainstate::stacks::{Error as ChainstateError, ThresholdSignature}; @@ -65,6 +66,8 @@ pub struct SignCoordinator { is_mainnet: bool, miners_session: StackerDBSession, signing_round_timeout: Duration, + signer_entries: HashMap, + weight_threshold: u32, pub next_signer_bitvec: BitVec<4000>, } @@ -122,6 +125,7 @@ impl NakamotoSigningParams { } } +#[allow(dead_code)] fn get_signer_commitments( is_mainnet: bool, reward_set: &[NakamotoSignerEntry], @@ -196,9 +200,10 @@ impl SignCoordinator { reward_set: &RewardSet, reward_cycle: u64, message_key: Scalar, - aggregate_public_key: Point, + aggregate_public_key: Option, stackerdb_conn: &StackerDBs, config: &Config, + // v1: bool, ) -> Result { let is_mainnet = config.is_mainnet(); let Some(ref reward_set_signers) = reward_set.signers else { @@ -250,6 +255,32 @@ impl SignCoordinator { ..Default::default() }; + let total_weight = + reward_set_signers + .iter() + .cloned() + .map(|s| s.weight) + .fold(0, |w, acc| { + acc.checked_add(w) + .expect("FATAL: Total signer weight > u32::MAX") + }); + + let threshold = NakamotoBlockHeader::compute_voting_weight_threshold(total_weight)?; + + let signer_public_keys = reward_set_signers + .iter() + .cloned() + .enumerate() + .map(|(idx, signer)| { + let Ok(slot_id) = u32::try_from(idx) else { + return Err(ChainstateError::InvalidStacksBlock( + "Signer index exceeds u32".into(), + )); + }; + Ok((slot_id, signer)) + }) + .collect::, ChainstateError>>()?; + let mut coordinator: FireCoordinator = FireCoordinator::new(coord_config); #[cfg(test)] { @@ -272,25 +303,31 @@ impl SignCoordinator { miners_session, signing_round_timeout: config.miner.wait_on_signers.clone(), next_signer_bitvec, + signer_entries: signer_public_keys, + weight_threshold: threshold, }; - sign_coordinator - .coordinator - .set_aggregate_public_key(Some(aggregate_public_key)); + if let Some(aggregate_public_key) = aggregate_public_key { + 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(), - stackerdb_conn, - reward_cycle, - &aggregate_public_key, - )?; - if let Err(e) = coordinator - .set_key_and_party_polynomials(aggregate_public_key.clone(), party_polynomials) - { - warn!("Failed to set a valid set of party polynomials"; "error" => %e); - }; + if let Some(aggregate_public_key) = aggregate_public_key { + let party_polynomials = get_signer_commitments( + is_mainnet, + reward_set_signers.as_slice(), + stackerdb_conn, + reward_cycle, + &aggregate_public_key, + )?; + if let Err(e) = coordinator + .set_key_and_party_polynomials(aggregate_public_key.clone(), party_polynomials) + { + warn!("Failed to set a valid set of party polynomials"; "error" => %e); + }; + } let (receiver, replaced_other) = STACKER_DB_CHANNEL.register_miner_coordinator(); if replaced_other { @@ -306,6 +343,8 @@ impl SignCoordinator { miners_session, signing_round_timeout: config.miner.wait_on_signers.clone(), next_signer_bitvec, + signer_entries: signer_public_keys, + weight_threshold: threshold, }) } @@ -606,6 +645,9 @@ impl SignCoordinator { }; let block_proposal_message = SignerMessageV0::BlockProposal(block_proposal); + debug!("Sending block proposal message to signers"; + "signer_signature_hash" => ?&block.header.signer_signature_hash().0, + ); Self::send_signers_message::( &self.message_key, sortdb, @@ -636,6 +678,13 @@ impl SignCoordinator { )); }; + let mut total_weight_signed: u32 = 0; + let mut gathered_signatures = BTreeMap::new(); + + info!("SignCoordinator: beginning to watch for block signatures."; + "threshold" => self.weight_threshold, + ); + let start_ts = Instant::now(); while start_ts.elapsed() <= self.signing_round_timeout { let event = match receiver.recv_timeout(EVENT_RECEIVER_POLL) { @@ -657,7 +706,88 @@ impl SignCoordinator { continue; } - // TODO: get messages from signers (#4775) + let modified_slots = &event.modified_slots.clone(); + + // Update `next_signers_bitvec` with the slots that were modified in the event + modified_slots.iter().for_each(|chunk| { + if let Ok(slot_id) = chunk.slot_id.try_into() { + match &self.next_signer_bitvec.set(slot_id, true) { + Err(e) => { + warn!("Failed to set bitvec for next signer: {e:?}"); + } + _ => (), + }; + } else { + error!("FATAL: slot_id greater than u16, which should never happen."); + } + }); + + let Ok(signer_event) = SignerEvent::::try_from(event).map_err(|e| { + warn!("Failure parsing StackerDB event into signer event. Ignoring message."; "err" => ?e); + }) else { + continue; + }; + let SignerEvent::SignerMessages(signer_set, messages) = signer_event else { + debug!("Received signer event other than a signer message. Ignoring."); + continue; + }; + if signer_set != u32::try_from(reward_cycle_id % 2).unwrap() { + debug!("Received signer event for other reward cycle. Ignoring."); + continue; + }; + let slot_ids = modified_slots + .iter() + .map(|chunk| chunk.slot_id) + .collect::>(); + + debug!("SignCoordinator: Received messages from signers"; + "count" => messages.len(), + "slot_ids" => ?slot_ids, + "threshold" => self.weight_threshold + ); + + for (message, slot_id) in messages.into_iter().zip(slot_ids) { + match message { + SignerMessageV0::BlockResponse(BlockResponse::Accepted(( + response_hash, + signature, + ))) => { + let block_sighash = block.header.signer_signature_hash(); + if block_sighash != response_hash { + warn!( + "Processed signature but didn't validate over the expected block. Returning error."; + "signature" => %signature, + "block_signer_signature_hash" => %block_sighash, + "slot_id" => slot_id, + ); + continue; + } + debug!("SignCoordinator: Received valid signature from signer"; "slot_id" => slot_id, "signature" => %signature); + let Some(signer_entry) = &self.signer_entries.get(&slot_id) else { + return Err(NakamotoNodeError::SignerSignatureError( + "Signer entry not found".into(), + )); + }; + total_weight_signed = total_weight_signed + .checked_add(signer_entry.weight) + .expect("FATAL: total weight signed exceeds u32::MAX"); + debug!("SignCoordinator: Total weight signed: {total_weight_signed}"); + gathered_signatures.insert(slot_id, signature); + } + SignerMessageV0::BlockResponse(BlockResponse::Rejected(_)) => { + debug!("Received rejected block response. Ignoring."); + } + SignerMessageV0::BlockProposal(_) => { + debug!("Received block proposal message. Ignoring."); + } + } + } + + // After gathering all signatures, return them if we've hit the threshold + if total_weight_signed >= self.weight_threshold { + info!("SignCoordinator: Received enough signatures. Continuing."); + return Ok(gathered_signatures.values().cloned().collect()); + } } Err(NakamotoNodeError::SignerSignatureError( diff --git a/testnet/stacks-node/src/tests/signer/mod.rs b/testnet/stacks-node/src/tests/signer/mod.rs index d40073bcbc..31d2dabc11 100644 --- a/testnet/stacks-node/src/tests/signer/mod.rs +++ b/testnet/stacks-node/src/tests/signer/mod.rs @@ -38,10 +38,11 @@ use clarity::boot_util::boot_code_id; use libsigner::{SignerEntries, SignerEventTrait}; use stacks::chainstate::coordinator::comm::CoordinatorChannels; use stacks::chainstate::nakamoto::signer_set::NakamotoSigners; -use stacks::chainstate::stacks::boot::SIGNERS_NAME; +use stacks::chainstate::stacks::boot::{NakamotoSignerEntry, SIGNERS_NAME}; use stacks::chainstate::stacks::{StacksPrivateKey, ThresholdSignature}; use stacks::core::StacksEpoch; use stacks::net::api::postblock_proposal::BlockValidateResponse; +use stacks::util::secp256k1::MessageSignature; use stacks_common::codec::StacksMessageCodec; use stacks_common::consts::SIGNER_SLOTS_PER_USER; use stacks_common::types::StacksEpochId; @@ -243,7 +244,7 @@ impl + Send + 'static, T: SignerEventTrait + 'static> SignerTest MinedNakamotoBlockEvent { let new_block = self.mine_nakamoto_block(timeout); let signer_sighash = new_block.signer_signature_hash.clone(); - let signature = self.wait_for_confirmed_block(&signer_sighash, timeout); + let signature = self.wait_for_confirmed_block_v1(&signer_sighash, timeout); assert!(signature.0.verify(&agg_key, signer_sighash.as_bytes())); new_block } @@ -275,15 +276,51 @@ impl + Send + 'static, T: SignerEventTrait + 'static> SignerTest ThresholdSignature { + let block_obj = self.wait_for_confirmed_block_with_hash(block_signer_sighash, timeout); + let signer_signature_hex = block_obj.get("signer_signature").unwrap().as_str().unwrap(); + let signer_signature_bytes = hex_bytes(&signer_signature_hex[2..]).unwrap(); + let signer_signature = + ThresholdSignature::consensus_deserialize(&mut signer_signature_bytes.as_slice()) + .unwrap(); + signer_signature + } + + /// Wait for a confirmed block and return a list of individual + /// signer signatures + fn wait_for_confirmed_block_v0( + &mut self, + block_signer_sighash: &Sha512Trunc256Sum, + timeout: Duration, + ) -> Vec { + let block_obj = self.wait_for_confirmed_block_with_hash(block_signer_sighash, timeout); + block_obj + .get("signer_signature") + .unwrap() + .as_array() + .expect("Expected signer_signature to be an array") + .iter() + .cloned() + .map(serde_json::from_value::) + .collect::, _>>() + .expect("Unable to deserialize array of MessageSignature") + } + + /// Wait for a confirmed block and return a list of individual + /// signer signatures + fn wait_for_confirmed_block_with_hash( + &mut self, + block_signer_sighash: &Sha512Trunc256Sum, + timeout: Duration, + ) -> serde_json::Map { let t_start = Instant::now(); while t_start.elapsed() <= timeout { let blocks = test_observer::get_blocks(); - if let Some(signature) = blocks.iter().find_map(|block_json| { + if let Some(block) = blocks.iter().find_map(|block_json| { let block_obj = block_json.as_object().unwrap(); let sighash = block_obj // use the try operator because non-nakamoto blocks @@ -294,16 +331,9 @@ impl + Send + 'static, T: SignerEventTrait + 'static> SignerTest + Send + 'static, T: SignerEventTrait + 'static> SignerTest PublicKeys { - let entries = self - .stacks_client - .get_reward_set_signers(reward_cycle) - .unwrap() - .unwrap(); + let entries = self.get_reward_set_signers(reward_cycle); let entries = SignerEntries::parse(false, &entries).unwrap(); entries.public_keys } + /// Get the signers for the given reward cycle + pub fn get_reward_set_signers(&self, reward_cycle: u64) -> Vec { + self.stacks_client + .get_reward_set_signers(reward_cycle) + .unwrap() + .unwrap() + } + #[allow(dead_code)] fn get_signer_metrics(&self) -> String { #[cfg(feature = "monitoring_prom")] diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 7d7bebcec5..55115c5f18 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -14,6 +14,7 @@ // along with this program. If not, see . use std::env; +use std::sync::atomic::Ordering; use std::time::Duration; use libsigner::v0::messages::{ @@ -25,6 +26,8 @@ use stacks::chainstate::stacks::boot::MINERS_NAME; use stacks::codec::StacksMessageCodec; use stacks::libstackerdb::StackerDBChunkData; use stacks::types::chainstate::StacksPrivateKey; +use stacks::types::PublicKey; +use stacks::util::secp256k1::Secp256k1PublicKey; use stacks::util_lib::boot::boot_code_id; use stacks_signer::client::{SignerSlotID, StackerDB}; use stacks_signer::runloop::State; @@ -33,7 +36,7 @@ use tracing_subscriber::prelude::*; use tracing_subscriber::{fmt, EnvFilter}; use super::SignerTest; -use crate::tests::nakamoto_integrations::boot_to_epoch_3_reward_set; +use crate::tests::nakamoto_integrations::{boot_to_epoch_3_reward_set, next_block_and}; use crate::tests::neon_integrations::next_block_and_wait; use crate::BurnchainController; @@ -96,6 +99,27 @@ impl SignerTest { debug!("Singers initialized"); self.run_until_epoch_3_boundary(); + + let (vrfs_submitted, commits_submitted) = ( + self.running_nodes.vrfs_submitted.clone(), + self.running_nodes.commits_submitted.clone(), + ); + info!("Submitting 1 BTC block for miner VRF key registration"); + // first block wakes up the run loop, wait until a key registration has been submitted. + next_block_and(&mut self.running_nodes.btc_regtest_controller, 60, || { + let vrf_count = vrfs_submitted.load(Ordering::SeqCst); + Ok(vrf_count >= 1) + }) + .unwrap(); + + info!("Successfully triggered first block to wake up the miner runloop."); + // second block should confirm the VRF register, wait until a block commit is submitted + next_block_and(&mut self.running_nodes.btc_regtest_controller, 60, || { + let commits_count = commits_submitted.load(Ordering::SeqCst); + Ok(commits_count >= 1) + }) + .unwrap(); + info!("Ready to mine Nakamoto blocks!"); } } @@ -212,3 +236,63 @@ fn block_proposal_rejection() { } signer_test.shutdown(); } + +// Basic test to ensure that miners are able to gather block responses +// from signers and create blocks. +#[test] +#[ignore] +fn miner_gather_signatures() { + if env::var("BITCOIND_TEST") != Ok("1".into()) { + return; + } + + tracing_subscriber::registry() + .with(fmt::layer()) + .with(EnvFilter::from_default_env()) + .init(); + + info!("------------------------- Test Setup -------------------------"); + let num_signers = 5; + let mut signer_test: SignerTest = SignerTest::new(num_signers); + signer_test.boot_to_epoch_3(); + let timeout = Duration::from_secs(30); + + info!("------------------------- Try mining one block -------------------------"); + signer_test.mine_nakamoto_block(timeout); + + // Verify that the signers accepted the proposed block, sending back a validate ok response + let proposed_signer_signature_hash = signer_test.wait_for_validate_ok_response(timeout); + let message = proposed_signer_signature_hash.0; + + info!("------------------------- Test Block Signed -------------------------"); + // Verify that the signers signed the proposed block + let signature = + signer_test.wait_for_confirmed_block_v0(&proposed_signer_signature_hash, timeout); + + info!("Got {} signatures", signature.len()); + + assert_eq!(signature.len(), num_signers); + + let reward_cycle = signer_test.get_current_reward_cycle(); + let signers = signer_test.get_reward_set_signers(reward_cycle); + + // Verify that the signers signed the proposed block + + let all_signed = signers.iter().zip(signature).all(|(signer, signature)| { + let stacks_public_key = Secp256k1PublicKey::from_slice(signer.signing_key.as_slice()) + .expect("Failed to convert signing key to StacksPublicKey"); + + // let valid = stacks_public_key.verify(message, signature); + let valid = stacks_public_key + .verify(&message, &signature) + .expect("Failed to verify signature"); + if !valid { + error!( + "Failed to verify signature for signer: {:?}", + stacks_public_key + ); + } + valid + }); + assert!(all_signed); +} diff --git a/testnet/stacks-node/src/tests/signer/v1.rs b/testnet/stacks-node/src/tests/signer/v1.rs index 66855514e7..30f499caae 100644 --- a/testnet/stacks-node/src/tests/signer/v1.rs +++ b/testnet/stacks-node/src/tests/signer/v1.rs @@ -879,7 +879,8 @@ fn block_proposal() { info!("------------------------- Test Block Signed -------------------------"); // Verify that the signers signed the proposed block - let signature = signer_test.wait_for_confirmed_block(&proposed_signer_signature_hash, timeout); + let signature = + signer_test.wait_for_confirmed_block_v1(&proposed_signer_signature_hash, timeout); assert!(signature .0 .verify(&key, proposed_signer_signature_hash.as_bytes())); @@ -1098,7 +1099,7 @@ fn sign_after_signer_reboot() { signer_test.mine_nakamoto_block(timeout); let proposed_signer_signature_hash = signer_test.wait_for_validate_ok_response(short_timeout); let signature = - signer_test.wait_for_confirmed_block(&proposed_signer_signature_hash, short_timeout); + signer_test.wait_for_confirmed_block_v1(&proposed_signer_signature_hash, short_timeout); assert!( signature.verify(&key, proposed_signer_signature_hash.0.as_slice()), @@ -1119,7 +1120,7 @@ fn sign_after_signer_reboot() { let last_block = signer_test.mine_nakamoto_block(timeout); let proposed_signer_signature_hash = signer_test.wait_for_validate_ok_response(short_timeout); let frost_signature = - signer_test.wait_for_confirmed_block(&proposed_signer_signature_hash, short_timeout); + signer_test.wait_for_confirmed_block_v1(&proposed_signer_signature_hash, short_timeout); // Check that the latest block's bitvec is all 1's assert_eq!( From f72ecc8c30ffafb457d41fb1ffac0399253ae4a7 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Tue, 21 May 2024 11:06:58 -0700 Subject: [PATCH 2/9] feat: update metrics in v0 signer, add tests --- stacks-signer/src/monitoring/server.rs | 7 ++++++- stacks-signer/src/v0/signer.rs | 13 ++++++++++--- testnet/stacks-node/Cargo.toml | 2 +- testnet/stacks-node/src/tests/signer/v0.rs | 17 +++++++++++++++++ 4 files changed, 34 insertions(+), 5 deletions(-) diff --git a/stacks-signer/src/monitoring/server.rs b/stacks-signer/src/monitoring/server.rs index 9cecd41ed7..ffde008c9f 100644 --- a/stacks-signer/src/monitoring/server.rs +++ b/stacks-signer/src/monitoring/server.rs @@ -95,7 +95,12 @@ impl MonitoringServer { public_key, format!("http://{}", config.node_host), ); - server.update_metrics()?; + if let Err(e) = server.update_metrics() { + warn!( + "Monitoring: Error updating metrics when starting server: {:?}", + e + ); + }; server.main_loop() } diff --git a/stacks-signer/src/v0/signer.rs b/stacks-signer/src/v0/signer.rs index 682c1433c1..e5471053ae 100644 --- a/stacks-signer/src/v0/signer.rs +++ b/stacks-signer/src/v0/signer.rs @@ -239,6 +239,7 @@ impl Signer { "block_id" => %block_proposal.block.block_id(), ); let block_info = BlockInfo::from(block_proposal.clone()); + crate::monitoring::increment_block_proposals_received(); stacks_client .submit_block_for_validation(block_info.block.clone()) .unwrap_or_else(|e| { @@ -311,11 +312,17 @@ impl Signer { }; // Submit a proposal response to the .signers contract for miners debug!("{self}: Broadcasting a block response to stacks node: {response:?}"); - if let Err(e) = self + match self .stackerdb - .send_message_with_retry::(response.into()) + .send_message_with_retry::(response.clone().into()) { - warn!("{self}: Failed to send block rejection to stacker-db: {e:?}",); + Ok(_) => { + let accepted = matches!(response, BlockResponse::Accepted(..)); + crate::monitoring::increment_block_responses_sent(accepted); + } + Err(e) => { + warn!("{self}: Failed to send block rejection to stacker-db: {e:?}",); + } } self.signer_db .insert_block(&block_info) diff --git a/testnet/stacks-node/Cargo.toml b/testnet/stacks-node/Cargo.toml index bceb484cd7..42f4f858b7 100644 --- a/testnet/stacks-node/Cargo.toml +++ b/testnet/stacks-node/Cargo.toml @@ -62,7 +62,7 @@ name = "stacks-events" path = "src/stacks_events.rs" [features] -monitoring_prom = ["stacks/monitoring_prom", "libsigner/monitoring_prom"] +monitoring_prom = ["stacks/monitoring_prom", "libsigner/monitoring_prom", "stacks-signer/monitoring_prom"] slog_json = ["stacks/slog_json", "stacks-common/slog_json", "clarity/slog_json"] prod-genesis-chainstate = [] default = [] diff --git a/testnet/stacks-node/src/tests/signer/v0.rs b/testnet/stacks-node/src/tests/signer/v0.rs index 55115c5f18..85b971a426 100644 --- a/testnet/stacks-node/src/tests/signer/v0.rs +++ b/testnet/stacks-node/src/tests/signer/v0.rs @@ -295,4 +295,21 @@ fn miner_gather_signatures() { valid }); assert!(all_signed); + + // Test prometheus metrics response + #[cfg(feature = "monitoring_prom")] + { + let metrics_response = signer_test.get_signer_metrics(); + + // Because 5 signers are running in the same process, the prometheus metrics + // are incremented once for every signer. This is why we expect the metric to be + // `5`, even though there is only one block proposed. + let expected_result = format!("stacks_signer_block_proposals_received {}", num_signers); + assert!(metrics_response.contains(&expected_result)); + let expected_result = format!( + "stacks_signer_block_responses_sent{{response_type=\"accepted\"}} {}", + num_signers + ); + assert!(metrics_response.contains(&expected_result)); + } } From aa9baf977c5edfa6b28e47d7f9896e1f3a84c2ef Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Tue, 21 May 2024 11:57:18 -0700 Subject: [PATCH 3/9] fix: move reward_set loading to top-level BlockMinerThread --- .../stacks-node/src/nakamoto_node/miner.rs | 57 +++++++++---------- 1 file changed, 27 insertions(+), 30 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index 29631cdec0..09a70b1178 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -155,6 +155,27 @@ impl BlockMinerThread { let mut stackerdbs = StackerDBs::connect(&self.config.get_stacker_db_file_path(), true) .expect("FATAL: failed to connect to stacker DB"); + 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(), + &self.burn_block.consensus_hash, + ) + .expect("FATAL: could not retrieve chain tip") + .expect("FATAL: could not retrieve chain tip"); + + let reward_set = sort_db + .get_preprocessed_reward_set_of(&tip.sortition_id) + .expect("FATAL: Error fetching reward set") + .expect("FATAL: No reward set found for miner") + .known_selected_anchor_block_owned() + .expect("FATAL: No reward set found for miner"); + let mut attempts = 0; // now, actually run this tenure loop { @@ -182,11 +203,12 @@ impl BlockMinerThread { }; if let Some(mut new_block) = new_block { - let (reward_set, signer_signature) = match self.gather_signatures( + let signer_signature = match self.gather_signatures( &mut new_block, self.burn_block.block_height, &mut stackerdbs, &mut attempts, + &reward_set, ) { Ok(x) => x, Err(e) => { @@ -198,7 +220,7 @@ impl BlockMinerThread { }; new_block.header.signer_signature = signer_signature; - if let Err(e) = self.broadcast(new_block.clone(), None, reward_set) { + if let Err(e) = self.broadcast(new_block.clone(), None, reward_set.clone()) { warn!("Error accepting own block: {e:?}. Will try mining again."); continue; } else { @@ -221,12 +243,6 @@ impl BlockMinerThread { self.mined_blocks.push(new_block); } - 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 wait_start = Instant::now(); while wait_start.elapsed() < self.config.miner.wait_on_interim_blocks { thread::sleep(Duration::from_millis(ABORT_TRY_AGAIN_MS)); @@ -342,7 +358,8 @@ impl BlockMinerThread { burn_block_height: u64, stackerdbs: &mut StackerDBs, attempts: &mut u64, - ) -> Result<(RewardSet, Vec), NakamotoNodeError> { + reward_set: &RewardSet, + ) -> Result, NakamotoNodeError> { let Some(miner_privkey) = self.config.miner.mining_key else { return Err(NakamotoNodeError::MinerConfigurationFailed( "No mining key configured, cannot mine", @@ -370,26 +387,6 @@ impl BlockMinerThread { ) .expect("FATAL: building on a burn block that is before the first burn block"); - let reward_info = match sort_db.get_preprocessed_reward_set_of(&tip.sortition_id) { - Ok(Some(x)) => x, - Ok(None) => { - return Err(NakamotoNodeError::SigningCoordinatorFailure( - "No reward set found. Cannot initialize miner coordinator.".into(), - )); - } - Err(e) => { - return Err(NakamotoNodeError::SigningCoordinatorFailure(format!( - "Failure while fetching reward set. Cannot initialize miner coordinator. {e:?}" - ))); - } - }; - - let Some(reward_set) = reward_info.known_selected_anchor_block_owned() else { - return Err(NakamotoNodeError::SigningCoordinatorFailure( - "Current reward cycle did not select a reward set. Cannot mine!".into(), - )); - }; - let miner_privkey_as_scalar = Scalar::from(miner_privkey.as_slice().clone()); let mut coordinator = SignCoordinator::new( &reward_set, @@ -417,7 +414,7 @@ impl BlockMinerThread { &self.globals.counters, )?; - return Ok((reward_set, signature)); + return Ok(signature); } fn get_stackerdb_contract_and_slots( From 0658bb907345ed7d0baf2a091972a0d4e6ebe9c4 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Thu, 23 May 2024 13:09:15 -0700 Subject: [PATCH 4/9] crc: return err instead of fatal when updating next_bitvec --- .../src/nakamoto_node/sign_coordinator.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs index 3cf1c6d144..081852d783 100644 --- a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs +++ b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs @@ -709,18 +709,18 @@ impl SignCoordinator { let modified_slots = &event.modified_slots.clone(); // Update `next_signers_bitvec` with the slots that were modified in the event - modified_slots.iter().for_each(|chunk| { - if let Ok(slot_id) = chunk.slot_id.try_into() { - match &self.next_signer_bitvec.set(slot_id, true) { - Err(e) => { - warn!("Failed to set bitvec for next signer: {e:?}"); - } - _ => (), - }; - } else { - error!("FATAL: slot_id greater than u16, which should never happen."); - } - }); + for chunk in modified_slots.iter() { + let Ok(slot_id) = chunk.slot_id.try_into() else { + return Err(NakamotoNodeError::SigningCoordinatorFailure( + "Unable to modify next_signer_bitvec: slot_id exceeds u16".into(), + )); + }; + if let Err(e) = &self.next_signer_bitvec.set(slot_id, true) { + return Err(NakamotoNodeError::SigningCoordinatorFailure(format!( + "Failed to set bitvec for next signer: {e:?}" + ))); + }; + } let Ok(signer_event) = SignerEvent::::try_from(event).map_err(|e| { warn!("Failure parsing StackerDB event into signer event. Ignoring message."; "err" => ?e); From 4cb4d15a1ddd71afcc56a9a56e62a82e79d94f36 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Thu, 23 May 2024 14:42:29 -0700 Subject: [PATCH 5/9] feat: load reward set in gather_signatures instead of `run_miner` to prevent panics --- .../stacks-node/src/nakamoto_node/miner.rs | 86 ++++++++++++------- .../src/nakamoto_node/sign_coordinator.rs | 14 --- 2 files changed, 55 insertions(+), 45 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index 09a70b1178..618eed7d6c 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -155,27 +155,6 @@ impl BlockMinerThread { let mut stackerdbs = StackerDBs::connect(&self.config.get_stacker_db_file_path(), true) .expect("FATAL: failed to connect to stacker DB"); - 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(), - &self.burn_block.consensus_hash, - ) - .expect("FATAL: could not retrieve chain tip") - .expect("FATAL: could not retrieve chain tip"); - - let reward_set = sort_db - .get_preprocessed_reward_set_of(&tip.sortition_id) - .expect("FATAL: Error fetching reward set") - .expect("FATAL: No reward set found for miner") - .known_selected_anchor_block_owned() - .expect("FATAL: No reward set found for miner"); - let mut attempts = 0; // now, actually run this tenure loop { @@ -203,12 +182,11 @@ impl BlockMinerThread { }; if let Some(mut new_block) = new_block { - let signer_signature = match self.gather_signatures( + let (reward_set, signer_signature) = match self.gather_signatures( &mut new_block, self.burn_block.block_height, &mut stackerdbs, &mut attempts, - &reward_set, ) { Ok(x) => x, Err(e) => { @@ -243,6 +221,15 @@ impl BlockMinerThread { self.mined_blocks.push(new_block); } + let Ok(sort_db) = SortitionDB::open( + &self.config.get_burn_db_file_path(), + true, + self.burnchain.pox_constants.clone(), + ) else { + error!("Failed to open sortition DB. Will try mining again."); + continue; + }; + let wait_start = Instant::now(); while wait_start.elapsed() < self.config.miner.wait_on_interim_blocks { thread::sleep(Duration::from_millis(ABORT_TRY_AGAIN_MS)); @@ -351,15 +338,14 @@ impl BlockMinerThread { Ok((aggregate_public_key, signature)) } - /// Gather signatures from the signers for the block + /// Gather a list of signatures from the signers for the block fn gather_signatures( &mut self, new_block: &mut NakamotoBlock, burn_block_height: u64, stackerdbs: &mut StackerDBs, attempts: &mut u64, - reward_set: &RewardSet, - ) -> Result, NakamotoNodeError> { + ) -> Result<(RewardSet, Vec), NakamotoNodeError> { let Some(miner_privkey) = self.config.miner.mining_key else { return Err(NakamotoNodeError::MinerConfigurationFailed( "No mining key configured, cannot mine", @@ -370,13 +356,47 @@ impl BlockMinerThread { true, self.burnchain.pox_constants.clone(), ) - .expect("FATAL: could not open sortition DB"); + .map_err(|e| { + NakamotoNodeError::SigningCoordinatorFailure(format!( + "Failed to open sortition DB. Cannot mine! {e:?}" + )) + })?; + 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"); + .map_err(|e| { + NakamotoNodeError::SigningCoordinatorFailure(format!( + "Failed to retrieve chain tip: {:?}", + e + )) + }) + .and_then(|result| { + result.ok_or_else(|| { + NakamotoNodeError::SigningCoordinatorFailure("Failed to retrieve chain tip".into()) + }) + })?; + + let reward_info = match sort_db.get_preprocessed_reward_set_of(&tip.sortition_id) { + Ok(Some(x)) => x, + Ok(None) => { + return Err(NakamotoNodeError::SigningCoordinatorFailure( + "No reward set found. Cannot initialize miner coordinator.".into(), + )); + } + Err(e) => { + return Err(NakamotoNodeError::SigningCoordinatorFailure(format!( + "Failure while fetching reward set. Cannot initialize miner coordinator. {e:?}" + ))); + } + }; + + let Some(reward_set) = reward_info.known_selected_anchor_block_owned() else { + return Err(NakamotoNodeError::SigningCoordinatorFailure( + "Current reward cycle did not select a reward set. Cannot mine!".into(), + )); + }; let reward_cycle = self .burnchain @@ -385,7 +405,11 @@ impl BlockMinerThread { self.burnchain.first_block_height, self.burn_block.block_height, ) - .expect("FATAL: building on a burn block that is before the first burn block"); + .ok_or_else(|| { + NakamotoNodeError::SigningCoordinatorFailure( + "Building on a burn block that is before the first burn block".into(), + ) + })?; let miner_privkey_as_scalar = Scalar::from(miner_privkey.as_slice().clone()); let mut coordinator = SignCoordinator::new( @@ -414,7 +438,7 @@ impl BlockMinerThread { &self.globals.counters, )?; - return Ok(signature); + return Ok((reward_set, signature)); } fn get_stackerdb_contract_and_slots( diff --git a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs index 081852d783..b0b4463d1d 100644 --- a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs +++ b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs @@ -708,20 +708,6 @@ impl SignCoordinator { let modified_slots = &event.modified_slots.clone(); - // Update `next_signers_bitvec` with the slots that were modified in the event - for chunk in modified_slots.iter() { - let Ok(slot_id) = chunk.slot_id.try_into() else { - return Err(NakamotoNodeError::SigningCoordinatorFailure( - "Unable to modify next_signer_bitvec: slot_id exceeds u16".into(), - )); - }; - if let Err(e) = &self.next_signer_bitvec.set(slot_id, true) { - return Err(NakamotoNodeError::SigningCoordinatorFailure(format!( - "Failed to set bitvec for next signer: {e:?}" - ))); - }; - } - let Ok(signer_event) = SignerEvent::::try_from(event).map_err(|e| { warn!("Failure parsing StackerDB event into signer event. Ignoring message."; "err" => ?e); }) else { From 2275aa877d7524b83929d63ea3f894d37a1e4adb Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Thu, 23 May 2024 14:52:32 -0700 Subject: [PATCH 6/9] feat: remove aggregate key related stuff from sign coordinator --- .../stacks-node/src/nakamoto_node/miner.rs | 68 +++++-------------- .../src/nakamoto_node/sign_coordinator.rs | 26 +------ 2 files changed, 18 insertions(+), 76 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index f92f3fc60a..8f23bfc82d 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -280,19 +280,6 @@ impl BlockMinerThread { }) })?; - let reward_cycle = self - .burnchain - .pox_constants - .block_height_to_reward_cycle( - self.burnchain.first_block_height, - self.burn_block.block_height, - ) - .ok_or_else(|| { - NakamotoNodeError::SigningCoordinatorFailure( - "Building on a burn block that is before the first burn block".into(), - ) - })?; - let reward_info = match sort_db.get_preprocessed_reward_set_of(&tip.sortition_id) { Ok(Some(x)) => x, Ok(None) => { @@ -328,19 +315,14 @@ impl BlockMinerThread { }; let miner_privkey_as_scalar = Scalar::from(miner_privkey.as_slice().clone()); - let mut coordinator = SignCoordinator::new( - &reward_set, - reward_cycle, - miner_privkey_as_scalar, - Some(aggregate_public_key), - &stackerdbs, - &self.config, - ) - .map_err(|e| { - NakamotoNodeError::SigningCoordinatorFailure(format!( - "Failed to initialize the signing coordinator. Cannot mine! {e:?}" - )) - })?; + let mut coordinator = + SignCoordinator::new(&reward_set, miner_privkey_as_scalar, &self.config).map_err( + |e| { + NakamotoNodeError::SigningCoordinatorFailure(format!( + "Failed to initialize the signing coordinator. Cannot mine! {e:?}" + )) + }, + )?; *attempts += 1; let signature = coordinator.begin_sign_v1( @@ -417,33 +399,15 @@ impl BlockMinerThread { )); }; - let reward_cycle = self - .burnchain - .pox_constants - .block_height_to_reward_cycle( - self.burnchain.first_block_height, - self.burn_block.block_height, - ) - .ok_or_else(|| { - NakamotoNodeError::SigningCoordinatorFailure( - "Building on a burn block that is before the first burn block".into(), - ) - })?; - let miner_privkey_as_scalar = Scalar::from(miner_privkey.as_slice().clone()); - let mut coordinator = SignCoordinator::new( - &reward_set, - reward_cycle, - miner_privkey_as_scalar, - None, - &stackerdbs, - &self.config, - ) - .map_err(|e| { - NakamotoNodeError::SigningCoordinatorFailure(format!( - "Failed to initialize the signing coordinator. Cannot mine! {e:?}" - )) - })?; + let mut coordinator = + SignCoordinator::new(&reward_set, miner_privkey_as_scalar, &self.config).map_err( + |e| { + NakamotoNodeError::SigningCoordinatorFailure(format!( + "Failed to initialize the signing coordinator. Cannot mine! {e:?}" + )) + }, + )?; *attempts += 1; let signature = coordinator.begin_sign_v0( diff --git a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs index b0b4463d1d..0db0ee9e04 100644 --- a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs +++ b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs @@ -198,10 +198,7 @@ impl SignCoordinator { /// * `aggregate_public_key` - the active aggregate key for this cycle pub fn new( reward_set: &RewardSet, - reward_cycle: u64, message_key: Scalar, - aggregate_public_key: Option, - stackerdb_conn: &StackerDBs, config: &Config, // v1: bool, ) -> Result { @@ -281,7 +278,7 @@ impl SignCoordinator { }) .collect::, ChainstateError>>()?; - let mut coordinator: FireCoordinator = FireCoordinator::new(coord_config); + let coordinator: FireCoordinator = FireCoordinator::new(coord_config); #[cfg(test)] { // In test mode, short-circuit spinning up the SignCoordinator if the TEST_SIGNING @@ -294,7 +291,7 @@ impl SignCoordinator { if replaced_other { warn!("Replaced the miner/coordinator receiver of a prior thread. Prior thread may have crashed."); } - let mut sign_coordinator = Self { + let sign_coordinator = Self { coordinator, message_key, receiver: Some(receiver), @@ -306,28 +303,9 @@ impl SignCoordinator { signer_entries: signer_public_keys, weight_threshold: threshold, }; - if let Some(aggregate_public_key) = aggregate_public_key { - sign_coordinator - .coordinator - .set_aggregate_public_key(Some(aggregate_public_key)); - } return Ok(sign_coordinator); } } - if let Some(aggregate_public_key) = aggregate_public_key { - let party_polynomials = get_signer_commitments( - is_mainnet, - reward_set_signers.as_slice(), - stackerdb_conn, - reward_cycle, - &aggregate_public_key, - )?; - if let Err(e) = coordinator - .set_key_and_party_polynomials(aggregate_public_key.clone(), party_polynomials) - { - warn!("Failed to set a valid set of party polynomials"; "error" => %e); - }; - } let (receiver, replaced_other) = STACKER_DB_CHANNEL.register_miner_coordinator(); if replaced_other { From e9685305e43e6b6d41d1e32e8d4f79e3613f3f97 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Thu, 30 May 2024 11:25:52 -0700 Subject: [PATCH 7/9] feat: additional checks for invalid signatures, duplicates, etc --- .../src/nakamoto_node/sign_coordinator.rs | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs index 0db0ee9e04..149eb84cbf 100644 --- a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs +++ b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs @@ -30,6 +30,8 @@ use stacks::chainstate::stacks::events::StackerDBChunksEvent; use stacks::chainstate::stacks::{Error as ChainstateError, ThresholdSignature}; use stacks::libstackerdb::StackerDBChunkData; use stacks::net::stackerdb::StackerDBs; +use stacks::types::PublicKey; +use stacks::util::hash::MerkleHashFunc; use stacks::util::secp256k1::MessageSignature; use stacks::util_lib::boot::boot_code_id; use stacks_common::bitvec::BitVec; @@ -732,9 +734,32 @@ impl SignCoordinator { "Signer entry not found".into(), )); }; - total_weight_signed = total_weight_signed - .checked_add(signer_entry.weight) - .expect("FATAL: total weight signed exceeds u32::MAX"); + let Ok(signer_pubkey) = + StacksPublicKey::from_slice(&signer_entry.signing_key) + else { + return Err(NakamotoNodeError::SignerSignatureError( + "Failed to parse signer public key".into(), + )); + }; + let Ok(valid_sig) = signer_pubkey.verify(block_sighash.bits(), &signature) + else { + warn!("Got invalid signature from a signer. Ignoring."); + continue; + }; + if !valid_sig { + warn!( + "Processed signature but didn't validate over the expected block. Ignoring"; + "signature" => %signature, + "block_signer_signature_hash" => %block_sighash, + "slot_id" => slot_id, + ); + continue; + } + if !gathered_signatures.contains_key(&slot_id) { + total_weight_signed = total_weight_signed + .checked_add(signer_entry.weight) + .expect("FATAL: total weight signed exceeds u32::MAX"); + } debug!("SignCoordinator: Total weight signed: {total_weight_signed}"); gathered_signatures.insert(slot_id, signature); } From f963b354ed002e7c8ce8f34ef0dd6e74a333e6e1 Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Thu, 30 May 2024 16:36:13 -0700 Subject: [PATCH 8/9] crc: helper method for RewardSet total weight --- stackslib/src/chainstate/nakamoto/mod.rs | 7 +++--- stackslib/src/chainstate/stacks/boot/mod.rs | 17 ++++++++++++++ .../src/nakamoto_node/sign_coordinator.rs | 22 ++++++++++--------- 3 files changed, 32 insertions(+), 14 deletions(-) diff --git a/stackslib/src/chainstate/nakamoto/mod.rs b/stackslib/src/chainstate/nakamoto/mod.rs index 33f6ff2109..d92b373bdd 100644 --- a/stackslib/src/chainstate/nakamoto/mod.rs +++ b/stackslib/src/chainstate/nakamoto/mod.rs @@ -516,10 +516,9 @@ impl NakamotoBlockHeader { // `last_index` is used to prevent out-of-order signatures let mut last_index = None; - let total_weight = signers.iter().map(|s| s.weight).fold(0, |w, acc| { - acc.checked_add(w) - .expect("FATAL: Total signer weight > u32::MAX") - }); + let total_weight = reward_set + .total_signing_weight() + .map_err(|_| ChainstateError::NoRegisteredSigners(0))?; // HashMap of let signers_by_pk: HashMap<_, _> = signers diff --git a/stackslib/src/chainstate/stacks/boot/mod.rs b/stackslib/src/chainstate/stacks/boot/mod.rs index 01ca39be4a..e42f1a0dfa 100644 --- a/stackslib/src/chainstate/stacks/boot/mod.rs +++ b/stackslib/src/chainstate/stacks/boot/mod.rs @@ -277,6 +277,23 @@ impl RewardSet { pub fn metadata_deserialize(from: &str) -> Result { serde_json::from_str(from).map_err(|e| e.to_string()) } + + /// Return the total `weight` of all signers in the reward set. + /// If there are no reward set signers, a ChainstateError is returned. + pub fn total_signing_weight(&self) -> Result { + let Some(ref reward_set_signers) = self.signers else { + return Err(format!( + "Unable to calculate total weight - No signers in reward set" + )); + }; + Ok(reward_set_signers + .iter() + .map(|s| s.weight) + .fold(0, |s, acc| { + acc.checked_add(s) + .expect("FATAL: Total signer weight > u32::MAX") + })) + } } impl RewardSetData { diff --git a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs index 149eb84cbf..078a73590a 100644 --- a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs +++ b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs @@ -254,15 +254,10 @@ impl SignCoordinator { ..Default::default() }; - let total_weight = - reward_set_signers - .iter() - .cloned() - .map(|s| s.weight) - .fold(0, |w, acc| { - acc.checked_add(w) - .expect("FATAL: Total signer weight > u32::MAX") - }); + let total_weight = reward_set.total_signing_weight().map_err(|e| { + warn!("Failed to calculate total weight for the reward set: {e:?}"); + ChainstateError::NoRegisteredSigners(0) + })?; let threshold = NakamotoBlockHeader::compute_voting_weight_threshold(total_weight)?; @@ -760,7 +755,14 @@ impl SignCoordinator { .checked_add(signer_entry.weight) .expect("FATAL: total weight signed exceeds u32::MAX"); } - debug!("SignCoordinator: Total weight signed: {total_weight_signed}"); + debug!("Signature Added to block"; + "block_signer_sighash" => %block_sighash, + "signer_pubkey" => signer_pubkey.to_hex(), + "signer_slot_id" => slot_id, + "signature" => %signature, + // "signer_weight" => signer_entry.weight // commented due to max size of `debug!` + "total_weight_signed" => total_weight_signed, + ); gathered_signatures.insert(slot_id, signature); } SignerMessageV0::BlockResponse(BlockResponse::Rejected(_)) => { From 4c35571c4cabac9889bf7c9b358d31e0d484380e Mon Sep 17 00:00:00 2001 From: Hank Stoever Date: Thu, 30 May 2024 16:44:07 -0700 Subject: [PATCH 9/9] fix: add missing comma to debug metadata --- testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs index 078a73590a..a0be82f06e 100644 --- a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs +++ b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs @@ -760,7 +760,7 @@ impl SignCoordinator { "signer_pubkey" => signer_pubkey.to_hex(), "signer_slot_id" => slot_id, "signature" => %signature, - // "signer_weight" => signer_entry.weight // commented due to max size of `debug!` + "signer_weight" => signer_entry.weight, "total_weight_signed" => total_weight_signed, ); gathered_signatures.insert(slot_id, signature);