From 9297a7010a831d609d86231ce90d3919dbb558ba Mon Sep 17 00:00:00 2001 From: Aaron Blankstein Date: Mon, 18 Mar 2024 11:58:31 -0500 Subject: [PATCH] refactor: signer set parsing --- libsigner/src/events.rs | 4 +- libsigner/src/libsigner.rs | 2 + libsigner/src/signer_set.rs | 137 ++++++++++++++++++ stacks-signer/src/client/mod.rs | 3 +- stacks-signer/src/config.rs | 20 +-- stacks-signer/src/runloop.rs | 79 ++-------- stacks-signer/src/signer.rs | 20 ++- .../stacks-node/src/nakamoto_node/miner.rs | 1 - .../src/nakamoto_node/sign_coordinator.rs | 76 ++++------ 9 files changed, 200 insertions(+), 142 deletions(-) create mode 100644 libsigner/src/signer_set.rs diff --git a/libsigner/src/events.rs b/libsigner/src/events.rs index 791cd440a..bd6e5b936 100644 --- a/libsigner/src/events.rs +++ b/libsigner/src/events.rs @@ -70,7 +70,7 @@ pub enum SignerEvent { /// The `Vec` will contain any block proposals made by the miner during this StackerDB event. /// The `Vec` will contain any signer WSTS messages made by the miner while acting as a coordinator. /// The `Option` will contain the message sender's public key if either of the vecs is non-empty. - ProposedBlocks( + MinerMessages( Vec, Vec, Option, @@ -440,7 +440,7 @@ impl TryFrom for SignerEvent { )); }; } - SignerEvent::ProposedBlocks(blocks, messages, miner_pk) + SignerEvent::MinerMessages(blocks, messages, miner_pk) } else if event.contract_id.name.starts_with(SIGNERS_NAME) && event.contract_id.issuer.1 == [0u8; 20] { diff --git a/libsigner/src/libsigner.rs b/libsigner/src/libsigner.rs index 33c5918fe..59465ffa2 100644 --- a/libsigner/src/libsigner.rs +++ b/libsigner/src/libsigner.rs @@ -42,6 +42,7 @@ mod http; mod messages; mod runloop; mod session; +mod signer_set; pub use crate::error::{EventError, RPCError}; pub use crate::events::{ @@ -53,3 +54,4 @@ pub use crate::messages::{ }; pub use crate::runloop::{RunningSigner, Signer, SignerRunLoop}; pub use crate::session::{SignerSession, StackerDBSession}; +pub use crate::signer_set::{Error as ParseSignerEntriesError, ParsedSignerEntries}; diff --git a/libsigner/src/signer_set.rs b/libsigner/src/signer_set.rs new file mode 100644 index 000000000..1877b1dee --- /dev/null +++ b/libsigner/src/signer_set.rs @@ -0,0 +1,137 @@ +// Copyright (C) 2024 Stacks Open Internet Foundation +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with this program. If not, see . + +use blockstack_lib::chainstate::stacks::boot::NakamotoSignerEntry; +use hashbrown::{HashMap, HashSet}; +use stacks_common::types::chainstate::{StacksAddress, StacksPublicKey}; +use wsts::curve::ecdsa; +use wsts::curve::point::{Compressed, Point}; +use wsts::state_machine::PublicKeys; + +/// A reward set parsed into the structures required by WSTS party members and coordinators. +#[derive(Debug, Clone)] +pub struct ParsedSignerEntries { + /// The signer addresses mapped to signer id + pub signer_ids: HashMap, + /// The signer ids mapped to public key and key ids mapped to public keys + pub public_keys: PublicKeys, + /// The signer ids mapped to key ids + pub signer_key_ids: HashMap>, + /// The signer ids mapped to wsts public keys + pub signer_public_keys: HashMap, + /// The signer ids mapped to a hash set of key ids + /// The wsts coordinator uses a hash set for each signer since it needs to do lots of lookups + pub coordinator_key_ids: HashMap>, +} + +/// Parsing errors for `ParsedSignerEntries` +#[derive(Debug)] +pub enum Error { + /// A member of the signing set has a signing key buffer + /// which does not represent a ecdsa public key. + BadSignerPublicKey(String), + /// The number of signers was greater than u32::MAX + SignerCountOverflow, +} + +impl ParsedSignerEntries { + /// Try to parse the reward set defined by `NakamotoSignEntry` into the structures required + /// by WSTS party members and coordinators. + pub fn parse(is_mainnet: bool, reward_set: &[NakamotoSignerEntry]) -> Result { + let mut weight_end = 1; + let mut signer_key_ids = HashMap::with_capacity(reward_set.len()); + let mut signer_public_keys = HashMap::with_capacity(reward_set.len()); + let mut coordinator_key_ids = HashMap::with_capacity(4000); + let mut signer_ids = HashMap::with_capacity(reward_set.len()); + let mut wsts_signers = HashMap::new(); + let mut wsts_key_ids = HashMap::new(); + for (i, entry) in reward_set.iter().enumerate() { + let signer_id = u32::try_from(i).map_err(|_| Error::SignerCountOverflow)?; + let ecdsa_pk = + ecdsa::PublicKey::try_from(entry.signing_key.as_slice()).map_err(|e| { + Error::BadSignerPublicKey(format!( + "Failed to convert signing key to ecdsa::PublicKey: {e}" + )) + })?; + let signer_public_key = Point::try_from(&Compressed::from(ecdsa_pk.to_bytes())) + .map_err(|e| { + Error::BadSignerPublicKey(format!( + "Failed to convert signing key to wsts::Point: {e}" + )) + })?; + let stacks_public_key = StacksPublicKey::from_slice(entry.signing_key.as_slice()) + .map_err(|e| { + Error::BadSignerPublicKey(format!( + "Failed to convert signing key to StacksPublicKey: {e}" + )) + })?; + + let stacks_address = StacksAddress::p2pkh(is_mainnet, &stacks_public_key); + signer_ids.insert(stacks_address, signer_id); + + signer_public_keys.insert(signer_id, signer_public_key); + let weight_start = weight_end; + weight_end = weight_start + entry.weight; + let key_ids: HashSet = (weight_start..weight_end).collect(); + for key_id in key_ids.iter() { + wsts_key_ids.insert(*key_id, ecdsa_pk.clone()); + } + signer_key_ids.insert(signer_id, (weight_start..weight_end).collect()); + coordinator_key_ids.insert(signer_id, key_ids); + wsts_signers.insert(signer_id, ecdsa_pk); + } + + Ok(Self { + signer_ids, + public_keys: PublicKeys { + signers: wsts_signers, + key_ids: wsts_key_ids, + }, + signer_key_ids, + signer_public_keys, + coordinator_key_ids, + }) + } + + /// Return the number of Key IDs in the WSTS group signature + pub fn count_keys(&self) -> Result { + self.public_keys + .key_ids + .len() + .try_into() + .map_err(|_| Error::SignerCountOverflow) + } + + /// Return the number of Key IDs in the WSTS group signature + pub fn count_signers(&self) -> Result { + self.public_keys + .signers + .len() + .try_into() + .map_err(|_| Error::SignerCountOverflow) + } + + /// Return the number of Key IDs required to sign a message with the WSTS group signature + pub fn get_signing_threshold(&self) -> Result { + let num_keys = self.count_keys()?; + Ok((num_keys as f64 * 7_f64 / 10_f64).ceil() as u32) + } + + /// Return the number of Key IDs required to sign a message with the WSTS group signature + pub fn get_dkg_threshold(&self) -> Result { + let num_keys = self.count_keys()?; + Ok((num_keys as f64 * 9_f64 / 10_f64).ceil() as u32) + } +} diff --git a/stacks-signer/src/client/mod.rs b/stacks-signer/src/client/mod.rs index 8e4302904..cd63f4608 100644 --- a/stacks-signer/src/client/mod.rs +++ b/stacks-signer/src/client/mod.rs @@ -140,6 +140,7 @@ pub(crate) mod tests { use clarity::vm::types::TupleData; use clarity::vm::Value as ClarityValue; use hashbrown::{HashMap, HashSet}; + use libsigner::ParsedSignerEntries; use rand::distributions::Standard; use rand::{thread_rng, Rng}; use rand_core::{OsRng, RngCore}; @@ -154,7 +155,7 @@ pub(crate) mod tests { use wsts::state_machine::PublicKeys; use super::*; - use crate::config::{GlobalConfig, ParsedSignerEntries, SignerConfig}; + use crate::config::{GlobalConfig, SignerConfig}; use crate::signer::SignerSlotID; pub struct MockServerClient { diff --git a/stacks-signer/src/config.rs b/stacks-signer/src/config.rs index d2b2de905..e3cc41a98 100644 --- a/stacks-signer/src/config.rs +++ b/stacks-signer/src/config.rs @@ -20,7 +20,7 @@ use std::path::PathBuf; use std::time::Duration; use blockstack_lib::chainstate::stacks::TransactionVersion; -use hashbrown::{HashMap, HashSet}; +use libsigner::ParsedSignerEntries; use serde::Deserialize; use stacks_common::address::{ AddressHashMode, C32_ADDRESS_VERSION_MAINNET_SINGLESIG, C32_ADDRESS_VERSION_TESTNET_SINGLESIG, @@ -28,9 +28,7 @@ use stacks_common::address::{ use stacks_common::consts::{CHAIN_ID_MAINNET, CHAIN_ID_TESTNET}; use stacks_common::types::chainstate::{StacksAddress, StacksPrivateKey, StacksPublicKey}; use stacks_common::types::PrivateKey; -use wsts::curve::point::Point; use wsts::curve::scalar::Scalar; -use wsts::state_machine::PublicKeys; use crate::signer::SignerSlotID; @@ -112,22 +110,6 @@ impl Network { } } -/// Parsed Reward Set -#[derive(Debug, Clone)] -pub struct ParsedSignerEntries { - /// The signer addresses mapped to signer id - pub signer_ids: HashMap, - /// The signer ids mapped to public key and key ids mapped to public keys - pub public_keys: PublicKeys, - /// The signer ids mapped to key ids - pub signer_key_ids: HashMap>, - /// The signer ids mapped to wsts public keys - pub signer_public_keys: HashMap, - /// The signer ids mapped to a hash set of key ids - /// The wsts coordinator uses a hash set for each signer since it needs to do lots of lookups - pub coordinator_key_ids: HashMap>, -} - /// The Configuration info needed for an individual signer per reward cycle #[derive(Debug, Clone)] pub struct SignerConfig { diff --git a/stacks-signer/src/runloop.rs b/stacks-signer/src/runloop.rs index 1af875105..3463e0aab 100644 --- a/stacks-signer/src/runloop.rs +++ b/stacks-signer/src/runloop.rs @@ -18,20 +18,18 @@ use std::sync::mpsc::Sender; use std::time::Duration; use blockstack_lib::chainstate::burn::ConsensusHashExtensions; -use blockstack_lib::chainstate::stacks::boot::{NakamotoSignerEntry, SIGNERS_NAME}; +use blockstack_lib::chainstate::stacks::boot::SIGNERS_NAME; use blockstack_lib::util_lib::boot::boot_code_id; -use hashbrown::{HashMap, HashSet}; -use libsigner::{SignerEvent, SignerRunLoop}; +use hashbrown::HashMap; +use libsigner::{ParsedSignerEntries, SignerEvent, SignerRunLoop}; use slog::{slog_debug, slog_error, slog_info, slog_warn}; -use stacks_common::types::chainstate::{ConsensusHash, StacksAddress, StacksPublicKey}; +use stacks_common::types::chainstate::{ConsensusHash, StacksAddress}; use stacks_common::{debug, error, info, warn}; -use wsts::curve::ecdsa; -use wsts::curve::point::{Compressed, Point}; use wsts::state_machine::coordinator::State as CoordinatorState; -use wsts::state_machine::{OperationResult, PublicKeys}; +use wsts::state_machine::OperationResult; use crate::client::{retry_with_exponential_backoff, ClientError, StacksClient}; -use crate::config::{GlobalConfig, ParsedSignerEntries, SignerConfig}; +use crate::config::{GlobalConfig, SignerConfig}; use crate::signer::{Command as SignerCommand, Signer, SignerSlotID, State as SignerState}; /// Which operation to perform @@ -82,57 +80,6 @@ impl From for RunLoop { } impl RunLoop { - /// Parse Nakamoto signer entries into relevant signer information - pub fn parse_nakamoto_signer_entries( - signers: &[NakamotoSignerEntry], - is_mainnet: bool, - ) -> ParsedSignerEntries { - let mut weight_end = 1; - let mut coordinator_key_ids = HashMap::with_capacity(4000); - let mut signer_key_ids = HashMap::with_capacity(signers.len()); - let mut signer_ids = HashMap::with_capacity(signers.len()); - let mut public_keys = PublicKeys { - signers: HashMap::with_capacity(signers.len()), - key_ids: HashMap::with_capacity(4000), - }; - let mut signer_public_keys = HashMap::with_capacity(signers.len()); - for (i, entry) in signers.iter().enumerate() { - // TODO: track these signer ids as non participating if any of the conversions fail - let signer_id = u32::try_from(i).expect("FATAL: number of signers exceeds u32::MAX"); - let ecdsa_public_key = ecdsa::PublicKey::try_from(entry.signing_key.as_slice()) - .expect("FATAL: corrupted signing key"); - let signer_public_key = Point::try_from(&Compressed::from(ecdsa_public_key.to_bytes())) - .expect("FATAL: corrupted signing key"); - let stacks_public_key = StacksPublicKey::from_slice(entry.signing_key.as_slice()) - .expect("FATAL: Corrupted signing key"); - - let stacks_address = StacksAddress::p2pkh(is_mainnet, &stacks_public_key); - signer_ids.insert(stacks_address, signer_id); - signer_public_keys.insert(signer_id, signer_public_key); - let weight_start = weight_end; - weight_end = weight_start + entry.weight; - for key_id in weight_start..weight_end { - public_keys.key_ids.insert(key_id, ecdsa_public_key); - public_keys.signers.insert(signer_id, ecdsa_public_key); - coordinator_key_ids - .entry(signer_id) - .or_insert(HashSet::with_capacity(entry.weight as usize)) - .insert(key_id); - signer_key_ids - .entry(signer_id) - .or_insert(Vec::with_capacity(entry.weight as usize)) - .push(key_id); - } - } - ParsedSignerEntries { - signer_ids, - public_keys, - signer_key_ids, - signer_public_keys, - coordinator_key_ids, - } - } - /// Get the registered signers for a specific reward cycle /// Returns None if no signers are registered or its not Nakamoto cycle pub fn get_parsed_reward_set( @@ -148,10 +95,9 @@ impl RunLoop { warn!("No registered signers found for reward cycle {reward_cycle}."); return Ok(None); } - Ok(Some(Self::parse_nakamoto_signer_entries( - &signers, - self.config.network.is_mainnet(), - ))) + let entries = + ParsedSignerEntries::parse(self.config.network.is_mainnet(), &signers).unwrap(); + Ok(Some(entries)) } /// Get the stackerdb signer slots for a specific reward cycle @@ -388,7 +334,7 @@ impl SignerRunLoop, RunLoopCommand> for RunLoop { 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::MinerMessages(..)) => None, Some(SignerEvent::SignerMessages(msg_parity, ..)) => { Some(u64::from(msg_parity) % 2) } @@ -435,10 +381,9 @@ impl SignerRunLoop, RunLoopCommand> for RunLoop { #[cfg(test)] mod tests { use blockstack_lib::chainstate::stacks::boot::NakamotoSignerEntry; + use libsigner::ParsedSignerEntries; use stacks_common::types::chainstate::{StacksPrivateKey, StacksPublicKey}; - use super::RunLoop; - #[test] fn parse_nakamoto_signer_entries_test() { let nmb_signers = 10; @@ -455,7 +400,7 @@ mod tests { }); } - let parsed_entries = RunLoop::parse_nakamoto_signer_entries(&signer_entries, false); + let parsed_entries = ParsedSignerEntries::parse(false, &signer_entries).unwrap(); assert_eq!(parsed_entries.signer_ids.len(), nmb_signers); let mut signer_ids = parsed_entries.signer_ids.into_values().collect::>(); signer_ids.sort(); diff --git a/stacks-signer/src/signer.rs b/stacks-signer/src/signer.rs index 5a49be31f..1d43e7316 100644 --- a/stacks-signer/src/signer.rs +++ b/stacks-signer/src/signer.rs @@ -211,12 +211,22 @@ impl From for Signer { fn from(signer_config: SignerConfig) -> Self { let stackerdb = StackerDB::from(&signer_config); - let num_signers = u32::try_from(signer_config.signer_entries.public_keys.signers.len()) + let num_signers = signer_config + .signer_entries + .count_signers() .expect("FATAL: Too many registered signers to fit in a u32"); - let num_keys = u32::try_from(signer_config.signer_entries.public_keys.key_ids.len()) + let num_keys = signer_config + .signer_entries + .count_keys() + .expect("FATAL: Too many key ids to fit in a u32"); + let threshold = signer_config + .signer_entries + .get_signing_threshold() + .expect("FATAL: Too many key ids to fit in a u32"); + let dkg_threshold = signer_config + .signer_entries + .get_dkg_threshold() .expect("FATAL: Too many key ids to fit in a u32"); - let threshold = (num_keys as f64 * 7_f64 / 10_f64).ceil() as u32; - let dkg_threshold = (num_keys as f64 * 9_f64 / 10_f64).ceil() as u32; let coordinator_config = CoordinatorConfig { threshold, @@ -1283,7 +1293,7 @@ impl Signer { ); self.handle_signer_messages(stacks_client, res, messages, current_reward_cycle); } - Some(SignerEvent::ProposedBlocks(blocks, messages, miner_key)) => { + Some(SignerEvent::MinerMessages(blocks, messages, miner_key)) => { if let Some(miner_key) = miner_key { let miner_key = PublicKey::try_from(miner_key.to_bytes_compressed().as_slice()) .expect("FATAL: could not convert from StacksPublicKey to PublicKey"); diff --git a/testnet/stacks-node/src/nakamoto_node/miner.rs b/testnet/stacks-node/src/nakamoto_node/miner.rs index 9450e11f0..d28241728 100644 --- a/testnet/stacks-node/src/nakamoto_node/miner.rs +++ b/testnet/stacks-node/src/nakamoto_node/miner.rs @@ -336,7 +336,6 @@ impl BlockMinerThread { reward_cycle, miner_privkey_as_scalar, aggregate_public_key, - self.config.is_mainnet(), &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 e7b460bc3..894a1d933 100644 --- a/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs +++ b/testnet/stacks-node/src/nakamoto_node/sign_coordinator.rs @@ -17,7 +17,9 @@ use std::sync::mpsc::Receiver; use std::time::{Duration, Instant}; use hashbrown::{HashMap, HashSet}; -use libsigner::{MessageSlotID, SignerEvent, SignerMessage, SignerSession, StackerDBSession}; +use libsigner::{ + MessageSlotID, ParsedSignerEntries, SignerEvent, SignerMessage, SignerSession, StackerDBSession, +}; use stacks::burnchains::Burnchain; use stacks::chainstate::burn::db::sortdb::SortitionDB; use stacks::chainstate::burn::BlockSnapshot; @@ -32,7 +34,7 @@ use stacks_common::codec::StacksMessageCodec; use stacks_common::types::chainstate::{StacksPrivateKey, StacksPublicKey}; use wsts::common::PolyCommitment; use wsts::curve::ecdsa; -use wsts::curve::point::{Compressed, Point}; +use wsts::curve::point::Point; use wsts::curve::scalar::Scalar; use wsts::state_machine::coordinator::fire::Coordinator as FireCoordinator; use wsts::state_machine::coordinator::{Config as CoordinatorConfig, Coordinator}; @@ -80,55 +82,35 @@ impl Drop for SignCoordinator { } } -impl From<&[NakamotoSignerEntry]> for NakamotoSigningParams { - fn from(reward_set: &[NakamotoSignerEntry]) -> Self { - let mut weight_end = 1; - let mut signer_key_ids = HashMap::with_capacity(reward_set.len()); - let mut signer_public_keys = HashMap::with_capacity(reward_set.len()); - let mut wsts_signers = HashMap::new(); - let mut wsts_key_ids = HashMap::new(); - for (i, entry) in reward_set.iter().enumerate() { - let signer_id = u32::try_from(i).expect("FATAL: number of signers exceeds u32::MAX"); - let ecdsa_pk = ecdsa::PublicKey::try_from(entry.signing_key.as_slice()) - .map_err(|e| format!("Failed to convert signing key to ecdsa::PublicKey: {e}")) - .unwrap_or_else(|err| { - panic!("FATAL: failed to convert signing key to Point: {err}") - }); - let signer_public_key = Point::try_from(&Compressed::from(ecdsa_pk.to_bytes())) - .map_err(|e| format!("Failed to convert signing key to wsts::Point: {e}")) - .unwrap_or_else(|err| { - panic!("FATAL: failed to convert signing key to Point: {err}") - }); +impl NakamotoSigningParams { + pub fn parse( + is_mainnet: bool, + reward_set: &[NakamotoSignerEntry], + ) -> Result { + let parsed = ParsedSignerEntries::parse(is_mainnet, reward_set).map_err(|e| { + ChainstateError::InvalidStacksBlock(format!( + "Invalid Reward Set: Could not parse into WSTS structs: {e:?}" + )) + })?; - signer_public_keys.insert(signer_id, signer_public_key); - let weight_start = weight_end; - weight_end = weight_start + entry.weight; - let key_ids: HashSet = (weight_start..weight_end).collect(); - for key_id in key_ids.iter() { - wsts_key_ids.insert(*key_id, ecdsa_pk.clone()); - } - signer_key_ids.insert(signer_id, key_ids); - wsts_signers.insert(signer_id, ecdsa_pk); - } - - let num_keys = weight_end - 1; - let threshold = (num_keys * 70) / 100; - let num_signers = reward_set - .len() - .try_into() + let num_keys = parsed + .count_keys() + .expect("FATAL: more than u32::max() signers in the reward set"); + let num_signers = parsed + .count_signers() + .expect("FATAL: more than u32::max() signers in the reward set"); + let threshold = parsed + .get_signing_threshold() .expect("FATAL: more than u32::max() signers in the reward set"); - NakamotoSigningParams { + Ok(NakamotoSigningParams { num_signers, threshold, num_keys, - signer_key_ids, - signer_public_keys, - wsts_public_keys: PublicKeys { - signers: wsts_signers, - key_ids: wsts_key_ids, - }, - } + signer_key_ids: parsed.coordinator_key_ids, + signer_public_keys: parsed.signer_public_keys, + wsts_public_keys: parsed.public_keys, + }) } } @@ -207,10 +189,10 @@ impl SignCoordinator { reward_cycle: u64, message_key: Scalar, aggregate_public_key: Point, - is_mainnet: bool, stackerdb_conn: &StackerDBs, config: &Config, ) -> Result { + let is_mainnet = config.is_mainnet(); let Some(ref reward_set_signers) = reward_set.signers else { error!("Could not initialize WSTS coordinator for reward set without signer"); return Err(ChainstateError::NoRegisteredSigners(0)); @@ -230,7 +212,7 @@ impl SignCoordinator { signer_key_ids, signer_public_keys, wsts_public_keys, - } = NakamotoSigningParams::from(reward_set_signers.as_slice()); + } = NakamotoSigningParams::parse(is_mainnet, reward_set_signers.as_slice())?; debug!( "Initializing miner/coordinator"; "num_signers" => num_signers,