Skip to content

Commit

Permalink
Merge pull request #4451 from stacks-network/bugfix/misuse-signer-id-…
Browse files Browse the repository at this point in the history 
…signer-slot-id

Fix signer id use in stackerdb
  • Loading branch information
@jferrant
jferrant committed Mar 1, 2024
2 parents 911c3ff + 73fd5e0 commit 4888ba2
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 220 deletions.
18 changes: 8 additions & 10 deletions stacks-signer/src/client/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ pub enum ClientError {
/// No reward set exists for the given reward cycle
#[error("No reward set exists for reward cycle {0}")]
NoRewardSet(u64),
/// Reward set contained corrupted data
#[error("{0}")]
CorruptedRewardSet(String),
/// Stacks node does not support a feature we need
#[error("Stacks node does not support a required feature: {0}")]
UnsupportedStacksFeature(String),
Expand Down Expand Up @@ -156,7 +153,8 @@ pub(crate) mod tests {
use wsts::state_machine::PublicKeys;

use super::*;
use crate::config::{GlobalConfig, RegisteredSignersInfo, SignerConfig};
use crate::config::{GlobalConfig, ParsedSignerEntries, SignerConfig};
use crate::signer::SignerSlotID;

pub struct MockServerClient {
pub server: TcpListener,
Expand Down Expand Up @@ -425,7 +423,7 @@ pub(crate) mod tests {
let mut start_key_id = 1u32;
let mut end_key_id = start_key_id;
let mut signer_public_keys = HashMap::new();
let mut signer_slot_ids = HashMap::new();
let mut signer_slot_ids = vec![];
let ecdsa_private_key = config.ecdsa_private_key;
let ecdsa_public_key =
ecdsa::PublicKey::new(&ecdsa_private_key).expect("Failed to create ecdsa public key");
Expand Down Expand Up @@ -459,7 +457,7 @@ pub(crate) mod tests {
&StacksPublicKey::from_slice(ecdsa_public_key.to_bytes().as_slice())
.expect("Failed to create stacks public key"),
);
signer_slot_ids.insert(address, signer_id); // Note in a real world situation, these would not always match
signer_slot_ids.push(SignerSlotID(signer_id));
signer_ids.insert(address, signer_id);

continue;
Expand All @@ -486,23 +484,23 @@ pub(crate) mod tests {
&StacksPublicKey::from_slice(public_key.to_bytes().as_slice())
.expect("Failed to create stacks public key"),
);
signer_slot_ids.insert(address, signer_id); // Note in a real world situation, these would not always match
signer_slot_ids.push(SignerSlotID(signer_id));
signer_ids.insert(address, signer_id);
start_key_id = end_key_id;
}
SignerConfig {
reward_cycle,
signer_id: 0,
signer_slot_id: 0,
signer_slot_id: SignerSlotID(rand::thread_rng().gen_range(0..num_signers)), // Give a random signer slot id between 0 and num_signers
key_ids: signer_key_ids.get(&0).cloned().unwrap_or_default(),
registered_signers: RegisteredSignersInfo {
signer_slot_ids,
signer_entries: ParsedSignerEntries {
public_keys,
coordinator_key_ids,
signer_key_ids,
signer_ids,
signer_public_keys,
},
signer_slot_ids,
ecdsa_private_key: config.ecdsa_private_key,
stacks_private_key: config.stacks_private_key,
node_host: config.node_host,
Expand Down
27 changes: 13 additions & 14 deletions stacks-signer/src/client/stackerdb.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use stacks_common::{debug, warn};
use super::ClientError;
use crate::client::retry_with_exponential_backoff;
use crate::config::SignerConfig;
use crate::signer::SignerSlotID;

/// The StackerDB client for communicating with the .signers contract
pub struct StackerDB {
Expand All @@ -42,9 +43,9 @@ pub struct StackerDB {
/// The private key used in all stacks node communications
stacks_private_key: StacksPrivateKey,
/// A map of a message ID to last chunk version for each session
slot_versions: HashMap<u32, HashMap<u32, u32>>,
slot_versions: HashMap<u32, HashMap<SignerSlotID, u32>>,
/// The signer slot ID -- the index into the signer list for this signer daemon's signing key.
signer_slot_id: u32,
signer_slot_id: SignerSlotID,
/// The reward cycle of the connecting signer
reward_cycle: u64,
/// The stacker-db transaction msg session for the NEXT reward cycle
Expand All @@ -69,7 +70,7 @@ impl StackerDB {
stacks_private_key: StacksPrivateKey,
is_mainnet: bool,
reward_cycle: u64,
signer_slot_id: u32,
signer_slot_id: SignerSlotID,
) -> Self {
let mut signers_message_stackerdb_sessions = HashMap::new();
let stackerdb_issuer = boot_code_addr(is_mainnet);
Expand Down Expand Up @@ -134,7 +135,7 @@ impl StackerDB {
1
};

let mut chunk = StackerDBChunkData::new(slot_id, slot_version, message_bytes.clone());
let mut chunk = StackerDBChunkData::new(slot_id.0, slot_version, message_bytes.clone());
chunk.sign(&self.stacks_private_key)?;

let Some(session) = self.signers_message_stackerdb_sessions.get_mut(&msg_id) else {
Expand Down Expand Up @@ -184,11 +185,11 @@ impl StackerDB {
/// Get the transactions from stackerdb for the signers
fn get_transactions(
transactions_session: &mut StackerDBSession,
signer_ids: &[u32],
signer_ids: &[SignerSlotID],
) -> Result<Vec<StacksTransaction>, ClientError> {
let send_request = || {
transactions_session
.get_latest_chunks(signer_ids)
.get_latest_chunks(&signer_ids.iter().map(|id| id.0).collect::<Vec<_>>())
.map_err(backoff::Error::transient)
};
let chunk_ack = retry_with_exponential_backoff(send_request)?;
Expand Down Expand Up @@ -225,25 +226,23 @@ impl StackerDB {
Ok(transactions)
}

/// Get the latest signer transactions from signer ids for the current reward cycle
/// Get this signer's latest transactions from stackerdb
pub fn get_current_transactions_with_retry(
&mut self,
signer_id: u32,
) -> Result<Vec<StacksTransaction>, ClientError> {
debug!("Signer #{signer_id}: Getting latest transactions from stacker db",);
let Some(transactions_session) = self
.signers_message_stackerdb_sessions
.get_mut(&TRANSACTIONS_MSG_ID)
else {
return Err(ClientError::NotConnected);
};
Self::get_transactions(transactions_session, &[signer_id])
Self::get_transactions(transactions_session, &[self.signer_slot_id])
}

/// Get the latest signer transactions from signer ids for the next reward cycle
pub fn get_next_transactions_with_retry(
&mut self,
signer_ids: &[u32],
signer_ids: &[SignerSlotID],
) -> Result<Vec<StacksTransaction>, ClientError> {
debug!("Getting latest chunks from stackerdb for the following signers: {signer_ids:?}",);
Self::get_transactions(&mut self.next_transaction_session, signer_ids)
Expand All @@ -255,7 +254,7 @@ impl StackerDB {
}

/// Retrieve the signer slot ID
pub fn get_signer_slot_id(&mut self) -> u32 {
pub fn get_signer_slot_id(&mut self) -> SignerSlotID {
self.signer_slot_id
}
}
Expand Down Expand Up @@ -302,8 +301,8 @@ mod tests {
let signer_message = SignerMessage::Transactions(vec![tx.clone()]);
let message = signer_message.serialize_to_vec();

let signer_ids = vec![0, 1];
let h = spawn(move || stackerdb.get_next_transactions_with_retry(&signer_ids));
let signer_slot_ids = vec![SignerSlotID(0), SignerSlotID(1)];
let h = spawn(move || stackerdb.get_next_transactions_with_retry(&signer_slot_ids));
let mut response_bytes = b"HTTP/1.1 200 OK\n\n".to_vec();
response_bytes.extend(message);
let mock_server = mock_server_from_config(&config);
Expand Down
125 changes: 15 additions & 110 deletions stacks-signer/src/client/stacks_client.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use std::net::SocketAddr;
use blockstack_lib::burnchains::Txid;
use blockstack_lib::chainstate::nakamoto::NakamotoBlock;
use blockstack_lib::chainstate::stacks::boot::{
RewardSet, SIGNERS_NAME, SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME,
NakamotoSignerEntry, SIGNERS_VOTING_FUNCTION_NAME, SIGNERS_VOTING_NAME,
};
use blockstack_lib::chainstate::stacks::{
StacksTransaction, StacksTransactionSigner, TransactionAnchorMode, TransactionAuth,
Expand All @@ -34,20 +34,17 @@ use blockstack_lib::net::api::postblock_proposal::NakamotoBlockProposal;
use blockstack_lib::util_lib::boot::{boot_code_addr, boot_code_id};
use clarity::vm::types::{PrincipalData, QualifiedContractIdentifier};
use clarity::vm::{ClarityName, ContractName, Value as ClarityValue};
use hashbrown::{HashMap, HashSet};
use serde_json::json;
use slog::{slog_debug, slog_warn};
use slog::slog_debug;
use stacks_common::codec::StacksMessageCodec;
use stacks_common::consts::{CHAIN_ID_MAINNET, CHAIN_ID_TESTNET};
use stacks_common::debug;
use stacks_common::types::chainstate::{StacksAddress, StacksPrivateKey, StacksPublicKey};
use stacks_common::types::StacksEpochId;
use stacks_common::{debug, warn};
use wsts::curve::ecdsa;
use wsts::curve::point::{Compressed, Point};
use wsts::state_machine::PublicKeys;

use crate::client::{retry_with_exponential_backoff, ClientError};
use crate::config::{GlobalConfig, RegisteredSignersInfo};
use crate::config::GlobalConfig;

/// The Stacks signer client used to communicate with the stacks node
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -296,8 +293,11 @@ impl StacksClient {
Ok(round)
}

/// Get the reward set from the stacks node for the given reward cycle
pub fn get_reward_set(&self, reward_cycle: u64) -> Result<RewardSet, ClientError> {
/// Get the reward set signers from the stacks node for the given reward cycle
pub fn get_reward_set_signers(
&self,
reward_cycle: u64,
) -> Result<Option<Vec<NakamotoSignerEntry>>, ClientError> {
debug!("Getting reward set for reward cycle {reward_cycle}...");
let send_request = || {
self.stacks_node_client
Expand All @@ -310,104 +310,7 @@ impl StacksClient {
return Err(ClientError::RequestFailure(response.status()));
}
let stackers_response = response.json::<GetStackersResponse>()?;
Ok(stackers_response.stacker_set)
}

/// Get the registered signers for a specific reward cycle
/// Returns None if no signers are registered or its not Nakamoto cycle
pub fn get_registered_signers_info(
&self,
reward_cycle: u64,
) -> Result<Option<RegisteredSignersInfo>, ClientError> {
debug!("Getting registered signers for reward cycle {reward_cycle}...");
let reward_set = self.get_reward_set(reward_cycle)?;
let Some(reward_set_signers) = reward_set.signers else {
warn!("No reward set signers found for reward cycle {reward_cycle}.");
return Ok(None);
};
if reward_set_signers.is_empty() {
warn!("No registered signers found for reward cycle {reward_cycle}.");
return Ok(None);
}
// signer uses a Vec<u32> for its key_ids, but coordinator uses a HashSet for each signer since it needs to do lots of lookups
let mut weight_end = 1;
let mut coordinator_key_ids = HashMap::with_capacity(4000);
let mut signer_key_ids = HashMap::with_capacity(reward_set_signers.len());
let mut signer_ids = HashMap::with_capacity(reward_set_signers.len());
let mut public_keys = PublicKeys {
signers: HashMap::with_capacity(reward_set_signers.len()),
key_ids: HashMap::with_capacity(4000),
};
let mut signer_public_keys = HashMap::with_capacity(reward_set_signers.len());
for (i, entry) in reward_set_signers.iter().enumerate() {
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()).map_err(|e| {
ClientError::CorruptedRewardSet(format!(
"Reward cycle {reward_cycle} failed to convert signing key to ecdsa::PublicKey: {e}"
))
})?;
let signer_public_key = Point::try_from(&Compressed::from(ecdsa_public_key.to_bytes()))
.map_err(|e| {
ClientError::CorruptedRewardSet(format!(
"Reward cycle {reward_cycle} failed to convert signing key to Point: {e}"
))
})?;
let stacks_public_key = StacksPublicKey::from_slice(entry.signing_key.as_slice()).map_err(|e| {
ClientError::CorruptedRewardSet(format!(
"Reward cycle {reward_cycle} failed to convert signing key to StacksPublicKey: {e}"
))
})?;

let stacks_address = StacksAddress::p2pkh(self.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);
}
}

let signer_set =
u32::try_from(reward_cycle % 2).expect("FATAL: reward_cycle % 2 exceeds u32::MAX");
let signer_stackerdb_contract_id = boot_code_id(SIGNERS_NAME, self.mainnet);
// Get the signer writers from the stacker-db to find the signer slot id
let signer_slots_weights = self
.get_stackerdb_signer_slots(&signer_stackerdb_contract_id, signer_set)
.unwrap();
let mut signer_slot_ids = HashMap::with_capacity(signer_slots_weights.len());
for (index, (address, _)) in signer_slots_weights.into_iter().enumerate() {
signer_slot_ids.insert(
address,
u32::try_from(index).expect("FATAL: number of signers exceeds u32::MAX"),
);
}

for address in signer_ids.keys() {
if !signer_slot_ids.contains_key(address) {
debug!("Signer {address} does not have a slot id in the stackerdb");
return Ok(None);
}
}

Ok(Some(RegisteredSignersInfo {
public_keys,
signer_key_ids,
signer_ids,
signer_slot_ids,
signer_public_keys,
coordinator_key_ids,
}))
Ok(stackers_response.stacker_set.signers)
}

/// Retreive the current pox data from the stacks node
Expand Down Expand Up @@ -687,7 +590,9 @@ mod tests {

use blockstack_lib::chainstate::nakamoto::NakamotoBlockHeader;
use blockstack_lib::chainstate::stacks::address::PoxAddress;
use blockstack_lib::chainstate::stacks::boot::{NakamotoSignerEntry, PoxStartCycleInfo};
use blockstack_lib::chainstate::stacks::boot::{
NakamotoSignerEntry, PoxStartCycleInfo, RewardSet,
};
use blockstack_lib::chainstate::stacks::ThresholdSignature;
use rand::thread_rng;
use rand_core::RngCore;
Expand Down Expand Up @@ -1232,9 +1137,9 @@ mod tests {
let stackers_response_json = serde_json::to_string(&stackers_response)
.expect("Failed to serialize get stacker response");
let response = format!("HTTP/1.1 200 OK\n\n{stackers_response_json}");
let h = spawn(move || mock.client.get_reward_set(0));
let h = spawn(move || mock.client.get_reward_set_signers(0));
write_response(mock.server, response.as_bytes());
assert_eq!(h.join().unwrap().unwrap(), stacker_set);
assert_eq!(h.join().unwrap().unwrap(), stacker_set.signers);
}

#[test]
Expand Down
Loading

0 comments on commit 4888ba2

Please sign in to comment.