Skip to content

Commit

Permalink
Updates after signature change
Browse files Browse the repository at this point in the history
  • Loading branch information
djordon committed Oct 10, 2024
1 parent 8032c07 commit 30a49a0
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 31 deletions.
39 changes: 36 additions & 3 deletions signer/src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use config::File;
use libp2p::Multiaddr;
use serde::Deserialize;
use stacks_common::types::chainstate::StacksAddress;
use std::collections::BTreeSet;
use std::path::Path;
use url::Url;

Expand All @@ -17,6 +18,7 @@ use crate::config::serialization::url_deserializer_single;
use crate::config::serialization::url_deserializer_vec;
use crate::keys::PrivateKey;
use crate::keys::PublicKey;
use crate::stacks::wallet::SignerWallet;

mod error;
mod serialization;
Expand Down Expand Up @@ -226,8 +228,12 @@ pub struct SignerConfig {
/// The postgres database endpoint
#[serde(deserialize_with = "url_deserializer_single")]
pub db_endpoint: Url,
/// The public keys of the signer peers.
pub peer_public_keys: Vec<PublicKey>,
/// The public keys of the signer sit during the bootstrapping phase of
/// the signers.
pub bootstrap_signing_set: Vec<PublicKey>,
/// The number of signatures required for the signers' bootstrapped
/// multi-sig wallet on Stacks.
pub bootstrap_signatures_required: u16,
}

impl Validatable for SignerConfig {
Expand All @@ -246,12 +252,38 @@ impl Validatable for SignerConfig {
SignerConfigError::UnsupportedDatabaseDriver(self.db_endpoint.scheme().to_string());
return Err(ConfigError::Message(err.to_string()));
}

// The requirement here is that the bootstrap wallet in the config
// is a valid wallet, and all of those checks are done by the
// `SignerWallet::load_boostrap_wallet` function. Some of these
// checks include checks for an empty `bootstrap_signing_set`, or a
// `bootstrap_signatures_required` that is to high, there are
// others.
if let Err(err) = SignerWallet::load_boostrap_wallet(self) {
return Err(ConfigError::Message(err.to_string()));
}

// db_endpoint note: we don't validate the host because we will never
// get here; the URL deserializer will fail if the host is empty.
Ok(())
}
}

impl SignerConfig {
/// Return the bootstrapped signing set from the config. This function
/// makes sure that the signing set includes the current signer.
pub fn bootstrap_signing_set(&self) -> BTreeSet<PublicKey> {
// We add in the current signer into the signing set from the
// config just in case it hasn't been included already.
let self_public_key = PublicKey::from_private_key(&self.private_key);
self.bootstrap_signing_set
.iter()
.copied()
.chain([self_public_key])
.collect()
}
}

/// Configuration for the Stacks event observer server (hosted within the signer).
#[derive(Debug, Clone, Deserialize)]
pub struct EventObserverConfig {
Expand Down Expand Up @@ -422,7 +454,8 @@ mod tests {
settings.signer.event_observer.bind,
"0.0.0.0:8801".parse::<SocketAddr>().unwrap()
);
assert!(!settings.signer.peer_public_keys.is_empty());
assert!(!settings.signer.bootstrap_signing_set.is_empty());
assert_eq!(settings.signer.bootstrap_signatures_required, 2);
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion signer/src/stacks/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1089,7 +1089,7 @@ mod tests {
SecretKey::new(&mut rng),
];
let public_keys = secret_keys.map(|sk| sk.public_key(SECP256K1).into());
let wallet = SignerWallet::new(&public_keys, 2, NetworkKind::Testnet, 0).unwrap();
let wallet = SignerWallet::new(public_keys, 2, NetworkKind::Testnet, 0).unwrap();
let deployer = StacksAddress::burn_address(false);

let call = RotateKeysV1::new(&wallet, deployer);
Expand Down
64 changes: 43 additions & 21 deletions signer/src/stacks/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use secp256k1::ecdsa::RecoverableSignature;
use secp256k1::Message;

use crate::config::NetworkKind;
use crate::config::SignerConfig;
use crate::context::Context;
use crate::error::Error;
use crate::keys::PublicKey;
Expand Down Expand Up @@ -98,13 +99,18 @@ impl SignerWallet {
/// is highly unlikely by chance, but a Byzantine actor could trigger
/// it purposefully if we don't require a signer to prove that they
/// control the public key that they submit.
pub fn new(
public_keys: &[PublicKey],
pub fn new<I>(
public_keys: I,
signatures_required: u16,
network_kind: NetworkKind,
nonce: u64,
) -> Result<Self, Error> {
) -> Result<Self, Error>
where
I: IntoIterator<Item = PublicKey>,
{
// Check most error conditions
let public_keys: BTreeSet<PublicKey> = public_keys.into_iter().collect();

let num_keys = public_keys.len();
let invalid_threshold = num_keys < signatures_required as usize;
let invalid_num_keys = num_keys == 0 || num_keys > MAX_KEYS as usize;
Expand All @@ -116,7 +122,6 @@ impl SignerWallet {
));
}

let public_keys: BTreeSet<PublicKey> = public_keys.iter().copied().collect();
// Used for creating the combined stacks address
let pubkeys: Vec<Secp256k1PublicKey> =
public_keys.iter().map(Secp256k1PublicKey::from).collect();
Expand Down Expand Up @@ -146,7 +151,9 @@ impl SignerWallet {
})
}

/// Load the multi-sig wallet from storage.
/// Load the multi-sig wallet from the last rotate-keys-trasnaction
/// stored in the database. If it's not there, fall back to the
/// bootstrap multi-sig wallet in the signer's config.
///
/// The wallet that is loaded is the one that cooresponds to the signer
/// set defined in the last confirmed key rotation contract call.
Expand All @@ -156,15 +163,26 @@ impl SignerWallet {
{
// Get the key rotation transaction from the database. This maps to
// what the stacks network thinks the signers' address is.
let last_key_rotation = ctx
.get_storage()
.get_last_key_rotation(chain_tip)
.await?
.ok_or(Error::MissingKeyRotation)?;
let last_key_rotation = ctx.get_storage().get_last_key_rotation(chain_tip).await?;

let config = &ctx.config().signer;
let network_kind = config.network;

match last_key_rotation {
Some(keys) => {
let public_keys = keys.signer_set;
let signatures_required = keys.signatures_required;
SignerWallet::new(public_keys, signatures_required, network_kind, 0)
}
None => Self::load_boostrap_wallet(config),
}
}

let public_keys = last_key_rotation.signer_set.as_slice();
let signatures_required = last_key_rotation.signatures_required;
let network_kind = ctx.config().signer.network;
/// Load the bootstrap wallet implicitly defined in the signer config.
pub fn load_boostrap_wallet(config: &SignerConfig) -> Result<SignerWallet, Error> {
let network_kind = config.network;
let public_keys = config.bootstrap_signing_set();
let signatures_required = config.bootstrap_signatures_required;

SignerWallet::new(public_keys, signatures_required, network_kind, 0)
}
Expand Down Expand Up @@ -443,7 +461,7 @@ mod tests {
.collect();

let public_keys: Vec<_> = key_pairs.iter().map(|kp| kp.public_key().into()).collect();
let wallet = SignerWallet::new(&public_keys, signatures_required, network, 1).unwrap();
let wallet = SignerWallet::new(public_keys, signatures_required, network, 1).unwrap();

let mut tx_signer = MultisigTx::new_contract_call(TestContractCall, &wallet, TX_FEE);
let tx = tx_signer.tx();
Expand Down Expand Up @@ -490,7 +508,7 @@ mod tests {
.collect();

let public_keys: Vec<_> = key_pairs.iter().map(|kp| kp.public_key().into()).collect();
let wallet = SignerWallet::new(&public_keys, signatures_required, network, 1).unwrap();
let wallet = SignerWallet::new(public_keys, signatures_required, network, 1).unwrap();

let mut tx_signer = MultisigTx::new_contract_call(TestContractCall, &wallet, TX_FEE);

Expand Down Expand Up @@ -547,15 +565,14 @@ mod tests {
.collect();

let pks1 = public_keys.clone();
let wallet1 = SignerWallet::new(&pks1, 5, network, 0).unwrap();

// Although it's unlikely, it's possible for the shuffle to not
// shuffle anything, so we need to keep trying.
while pks1 == public_keys {
public_keys.shuffle(&mut OsRng);
}
let wallet1 = SignerWallet::new(pks1, 5, network, 0).unwrap();

let wallet2 = SignerWallet::new(&public_keys, 5, network, 0).unwrap();
let wallet2 = SignerWallet::new(public_keys, 5, network, 0).unwrap();

assert_eq!(wallet1.address(), wallet2.address())
}
Expand Down Expand Up @@ -600,7 +617,8 @@ mod tests {
.collect();
let signatures_required = 5;
let network = NetworkKind::Regtest;
let wallet1 = SignerWallet::new(&signer_keys, signatures_required, network, 0).unwrap();
let public_keys = signer_keys.clone();
let wallet1 = SignerWallet::new(public_keys, signatures_required, network, 0).unwrap();

// Let's store the key information about this wallet into the database
let rotate_keys = RotateKeysTransaction {
Expand All @@ -618,8 +636,12 @@ mod tests {
.unwrap();

// We haven't stored any RotateKeysTransactions into the database
// yet, so loading the wallet should fail.
assert!(SignerWallet::load(&ctx, &bitcoin_chain_tip).await.is_err());
// yet, so it will try to load the wallet from the context.
let ans = SignerWallet::load(&ctx, &bitcoin_chain_tip).await.unwrap();
let config = &ctx.config().signer;
let bootstrap_aggregate_key =
PublicKey::combine_keys(&config.bootstrap_signing_set()).unwrap();
assert_eq!(ans.aggregate_key, bootstrap_aggregate_key);

let tx = model::StacksTransaction {
txid: rotate_keys.txid,
Expand Down
1 change: 1 addition & 0 deletions signer/src/testing/transaction_coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ where
private_key,
context_window,
threshold,
bitcoin_network: bitcoin::Network::Regtest,
signing_round_max_duration: Duration::from_secs(10),
dkg_max_duration: Duration::from_secs(10),
},
Expand Down
15 changes: 9 additions & 6 deletions signer/src/transaction_coordinator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,8 +588,8 @@ where
return Ok(DkgState::DkgComplete);
}

let signer_keys = &self.context.config().signer.peer_public_keys;
let signer_set_aggregate_key = PublicKey::combine_keys(signer_keys)?;
let signer_keys = self.context.config().signer.bootstrap_signing_set();
let signer_set_aggregate_key = PublicKey::combine_keys(&signer_keys)?;

let shares = db
.get_encrypted_dkg_shares_by_signing_set(&signer_set_aggregate_key)
Expand Down Expand Up @@ -739,13 +739,13 @@ where
Ok((aggregate_key, signer_set))
}
None => {
let signer_set = &self.context.config().signer.peer_public_keys;
let signer_set_aggregate_key = PublicKey::combine_keys(signer_set)?;
let signer_set = self.context.config().signer.bootstrap_signing_set();
let signer_set_aggregate_key = PublicKey::combine_keys(&signer_set)?;
let shares = db
.get_encrypted_dkg_shares_by_signing_set(&signer_set_aggregate_key)
.await?
.ok_or(Error::MissingDkgShares)?;
Ok((shares.aggregate_key, signer_set.iter().copied().collect()))
Ok((shares.aggregate_key, signer_set))
}
}
}
Expand All @@ -755,7 +755,10 @@ where
}

/// This function provides a deterministic 32-byte identifier for the
/// signer. This should probably deterministically too.
/// signer.
///
/// TODO: Maybe this should change deterministically with the bitcoin
/// chain tip.
fn coordinator_id(&self) -> [u8; 32] {
sha2::Sha256::new_with_prefix("SIGNER_COORDINATOR_ID")
.chain_update(self.pub_key().serialize())
Expand Down

0 comments on commit 30a49a0

Please sign in to comment.