Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Key reshare addition #950

Merged
merged 27 commits into from
Jul 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file modified crates/client/entropy_metadata.scale
Binary file not shown.
2 changes: 1 addition & 1 deletion crates/protocol/src/execute_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl RandomizedPrehashSigner<sr25519::Signature> for PairWrapper {
}
}

async fn execute_protocol_generic<Res: synedrion::ProtocolResult>(
pub async fn execute_protocol_generic<Res: synedrion::ProtocolResult>(
mut chans: Channels,
session: Session<Res, sr25519::Signature, PairWrapper, PartyId>,
session_id_hash: [u8; 32],
Expand Down
4 changes: 2 additions & 2 deletions crates/protocol/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ pub enum SessionId {
/// A distributed key generation protocol session for registering
Dkg { user: AccountId32, block_number: u32 },
/// A proactive refresh session
ProactiveRefresh { verifying_key: Vec<u8>, block_number: u32 },
Reshare { verifying_key: Vec<u8>, block_number: u32 },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems strange to remove the term 'proactive refresh' here but keep it elsewhere.

My proposal would be to call it reshare but include an Option with the new_signer field from OcwMessageReshare. Including that in the session Id ensures everyone is in agreement, and we can do a reshare with or without a change in the signing committee. (Whether we choose to do refreshes without a change is another question but i would like to keep the option open).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but I kinda wanna push on and handle this in the refactor

/// A signing session
Sign(SigningSessionInfo),
}
Expand All @@ -185,7 +185,7 @@ impl Hash for SessionId {
user.0.hash(state);
block_number.hash(state);
},
SessionId::ProactiveRefresh { verifying_key, block_number } => {
SessionId::Reshare { verifying_key, block_number } => {
verifying_key.hash(state);
block_number.hash(state);
},
Expand Down
6 changes: 3 additions & 3 deletions crates/protocol/tests/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ struct ServerState {
#[derive(Clone)]
pub enum ProtocolOutput {
Sign(RecoverableSignature),
ProactiveRefresh(ThresholdKeyShare<KeyParams, PartyId>),
Reshare(ThresholdKeyShare<KeyParams, PartyId>),
Dkg(KeyShareWithAuxInfo),
}

Expand Down Expand Up @@ -130,7 +130,7 @@ pub async fn server(
let (signature, recovery_id) = rsig.to_backend();
Ok(ProtocolOutput::Sign(RecoverableSignature { signature, recovery_id }))
},
SessionId::ProactiveRefresh { .. } => {
SessionId::Reshare { .. } => {
let new_keyshare = execute_proactive_refresh(
session_id,
channels,
Expand All @@ -139,7 +139,7 @@ pub async fn server(
threshold_keyshare.unwrap(),
)
.await?;
Ok(ProtocolOutput::ProactiveRefresh(new_keyshare))
Ok(ProtocolOutput::Reshare(new_keyshare))
},
SessionId::Dkg { .. } => {
let keyshare_and_aux_info =
Expand Down
4 changes: 2 additions & 2 deletions crates/protocol/tests/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ async fn test_refresh_with_parties(num_parties: usize) {
let keyshares = KeyShare::<KeyParams, PartyId>::new_centralized(&mut OsRng, &ids, None);
let verifying_key = keyshares[&PartyId::from(pairs[0].public())].verifying_key();

let session_id = SessionId::ProactiveRefresh {
let session_id = SessionId::Reshare {
verifying_key: verifying_key.to_encoded_point(true).as_bytes().to_vec(),
block_number: 0,
};
Expand All @@ -131,7 +131,7 @@ async fn test_refresh_with_parties(num_parties: usize) {
.collect();
let threshold = parties.len();
let mut outputs = test_protocol_with_parties(parties, session_id, threshold).await;
if let ProtocolOutput::ProactiveRefresh(keyshare) = outputs.pop().unwrap() {
if let ProtocolOutput::Reshare(keyshare) = outputs.pop().unwrap() {
assert!(keyshare.verifying_key() == verifying_key);
} else {
panic!("Unexpected protocol output");
Expand Down
10 changes: 10 additions & 0 deletions crates/shared/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,16 @@ pub struct OcwMessageDkg {
pub validators_info: Vec<ValidatorInfo>,
}

/// Offchain worker message for initiating a refresh
#[cfg(not(feature = "wasm"))]
#[cfg_attr(feature = "std", derive(Serialize, Deserialize))]
#[derive(Clone, Encode, Decode, Debug, Eq, PartialEq, TypeInfo)]
pub struct OcwMessageReshare {
// Stash address of new signer
pub new_signer: Vec<u8>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If i've understood right, this is the stash address of the new member of the committee. I think we should put a doccomment to avoid stash address vs tss account confusion.

pub block_number: BlockNumber,
}

/// Offchain worker message for initiating a proactive refresh
#[cfg(not(feature = "wasm"))]
#[derive(
Expand Down
39 changes: 39 additions & 0 deletions crates/threshold-signature-server/src/helpers/substrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ use crate::{
user::UserErr,
};
pub use entropy_client::substrate::{query_chain, submit_transaction};
use entropy_shared::user::ValidatorInfo;
use subxt::{backend::legacy::LegacyRpcMethods, utils::AccountId32, Config, OnlineClient};

/// Given a threshold server's account ID, return its corresponding stash (validator) address.
Expand Down Expand Up @@ -72,3 +73,41 @@ pub async fn get_registered_details(
.ok_or_else(|| UserErr::ChainFetch("Not Registering error: Register Onchain first"))?;
Ok(result)
}

/// Takes Stash keys and returns validator info from chain
pub async fn get_validators_info(
api: &OnlineClient<EntropyConfig>,
rpc: &LegacyRpcMethods<EntropyConfig>,
validators: Vec<AccountId32>,
) -> Result<Vec<ValidatorInfo>, UserErr> {
let mut handles = Vec::new();
let block_hash = rpc.chain_get_block_hash(None).await?;
for validator in validators {
let handle: tokio::task::JoinHandle<Result<ValidatorInfo, UserErr>> = tokio::task::spawn({
let api = api.clone();
let rpc = rpc.clone();

async move {
let threshold_address_query =
entropy::storage().staking_extension().threshold_servers(validator);
let server_info = query_chain(&api, &rpc, threshold_address_query, block_hash)
.await?
.ok_or_else(|| {
UserErr::OptionUnwrapError("Failed to unwrap validator info".to_string())
})?;

Ok(ValidatorInfo {
x25519_public_key: server_info.x25519_public_key,
ip_address: std::str::from_utf8(&server_info.endpoint)?.to_string(),
tss_account: server_info.tss_account,
})
}
});
handles.push(handle);
}
let mut all_signers: Vec<ValidatorInfo> = vec![];
for handle in handles {
all_signers.push(handle.await.unwrap().unwrap());
}
Ok(all_signers)
}
21 changes: 12 additions & 9 deletions crates/threshold-signature-server/src/helpers/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use crate::{
use axum::{routing::IntoMakeService, Router};
use entropy_kvdb::{encrypted_sled::PasswordMethod, get_db_path, kv_manager::KvManager};
use entropy_protocol::PartyId;
use entropy_shared::{DAVE_VERIFYING_KEY, EVE_VERIFYING_KEY};
use entropy_shared::{DAVE_VERIFYING_KEY, EVE_VERIFYING_KEY, NETWORK_PARENT_KEY};
use std::time::Duration;
use subxt::{
backend::legacy::LegacyRpcMethods, ext::sp_core::sr25519, tx::PairSigner,
Expand Down Expand Up @@ -118,7 +118,7 @@ pub async fn create_clients(
}

/// Spawn 3 TSS nodes with pre-stored keyshares
pub async fn spawn_testing_validators() -> (Vec<String>, Vec<PartyId>) {
pub async fn spawn_testing_validators(add_parent_key: bool) -> (Vec<String>, Vec<PartyId>) {
// spawn threshold servers
let ports = [3001i64, 3002, 3003];

Expand All @@ -143,9 +143,9 @@ pub async fn spawn_testing_validators() -> (Vec<String>, Vec<PartyId>) {

let ids = vec![alice_id, bob_id, charlie_id];

put_keyshares_in_db("alice", alice_kv).await;
put_keyshares_in_db("bob", bob_kv).await;
put_keyshares_in_db("charlie", charlie_kv).await;
put_keyshares_in_db("alice", alice_kv, add_parent_key).await;
put_keyshares_in_db("bob", bob_kv, add_parent_key).await;
put_keyshares_in_db("charlie", charlie_kv, add_parent_key).await;

let listener_alice = tokio::net::TcpListener::bind(format!("0.0.0.0:{}", ports[0]))
.await
Expand Down Expand Up @@ -175,9 +175,12 @@ pub async fn spawn_testing_validators() -> (Vec<String>, Vec<PartyId>) {
}

/// Add the pre-generated test keyshares to a kvdb
async fn put_keyshares_in_db(holder_name: &str, kvdb: KvManager) {
let user_names_and_verifying_keys = [("eve", EVE_VERIFYING_KEY), ("dave", DAVE_VERIFYING_KEY)];

async fn put_keyshares_in_db(holder_name: &str, kvdb: KvManager, add_parent_key: bool) {
let mut user_names_and_verifying_keys =
vec![("eve", hex::encode(EVE_VERIFYING_KEY)), ("dave", hex::encode(DAVE_VERIFYING_KEY))];
if add_parent_key {
user_names_and_verifying_keys.push(("eve", hex::encode(NETWORK_PARENT_KEY)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, we use a second copy of eve's keyshares as the parent key.

I guess at some point we will remove per-user keyshares and this will be the only pre-configured keyshare.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yup pretty much, why at a new key, If no objections Im happy with keeping less code here

}
for (user_name, user_verifying_key) in user_names_and_verifying_keys {
let keyshare_bytes = {
let project_root =
Expand All @@ -188,7 +191,7 @@ async fn put_keyshares_in_db(holder_name: &str, kvdb: KvManager) {
));
std::fs::read(file_path).unwrap()
};
let reservation = kvdb.kv().reserve_key(hex::encode(user_verifying_key)).await.unwrap();
let reservation = kvdb.kv().reserve_key(user_verifying_key).await.unwrap();
kvdb.kv().put(reservation, keyshare_bytes).await.unwrap();
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/threshold-signature-server/src/helpers/validator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ fn get_hkdf_from_mnemonic(mnemonic: &str) -> Result<Hkdf<Sha256>, UserErr> {
}

/// Derive signing keypair
fn get_signer_from_hkdf(
pub fn get_signer_from_hkdf(
hkdf: &Hkdf<Sha256>,
) -> Result<PairSigner<EntropyConfig, sr25519::Pair>, UserErr> {
let mut sr25519_seed = [0u8; 32];
Expand Down
8 changes: 5 additions & 3 deletions crates/threshold-signature-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@
//! in a [crate::validation::SignedMessage].
//!
//! - [`/ws`](crate::signing_client::api::ws_handler()) - Websocket server for signing and DKG protocol
//! messages. This is opened by other threshold servers when the signing procotol is initiated.
//! messages. This is opened by other threshold servers when the signing procotol is initiated.
//!
//! - [`/validator/sync_kvdb`](crate::validator::api::sync_kvdb()) - POST - Called by another
//! threshold server when joining to get the key-shares from a member of their sub-group.
//! threshold server when joining to get the key-shares from a member of their sub-group.
//!
//! Takes a list of users account IDs for which shares are requested, wrapped in a
//! [crate::validation::SignedMessage].
Expand Down Expand Up @@ -119,7 +119,7 @@
//!
//! - Axum server - Includes global state and mutex locked IPs
//! - [kvdb](entropy_kvdb) - Encrypted key-value database for storing key-shares and other data, build using
//! [sled](https://docs.rs/sled)
//! [sled](https://docs.rs/sled)
#![doc(html_logo_url = "https://entropy.xyz/assets/logo_02.png")]
pub use entropy_client::chain_api;
pub(crate) mod health;
Expand Down Expand Up @@ -155,6 +155,7 @@ use crate::{
r#unsafe::api::{delete, put, remove_keys, unsafe_get},
signing_client::{api::*, ListenerState},
user::api::*,
validator::api::new_reshare,
};

#[derive(Clone)]
Expand All @@ -176,6 +177,7 @@ pub fn app(app_state: AppState) -> Router {
.route("/user/new", post(new_user))
.route("/user/sign_tx", post(sign_tx))
.route("/signer/proactive_refresh", post(proactive_refresh))
.route("/validator/reshare", post(new_reshare))
.route("/healthz", get(healthz))
.route("/version", get(get_version))
.route("/hashes", get(hashes))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ pub async fn do_proactive_refresh(
tracing::debug!("Preparing to perform proactive refresh");
tracing::debug!("Signing with {:?}", &signer.signer().public());

let session_id = SessionId::ProactiveRefresh { verifying_key, block_number };
let session_id = SessionId::Reshare { verifying_key, block_number };
let account_id = SubxtAccountId32(signer.signer().public().0);
let mut converted_validator_info = vec![];
let mut tss_accounts = vec![];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ async fn test_proactive_refresh() {
clean_tests();
let _cxt = test_node_process_testing_state(false).await;

let (validator_ips, _ids) = spawn_testing_validators().await;
let (validator_ips, _ids) = spawn_testing_validators(false).await;

let client = reqwest::Client::new();

Expand Down
16 changes: 8 additions & 8 deletions crates/threshold-signature-server/src/user/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ async fn test_sign_tx_no_chain() {
let one = AccountKeyring::Dave;
let two = AccountKeyring::Two;

let (_validator_ips, _validator_ids) = spawn_testing_validators().await;
let (_validator_ips, _validator_ids) = spawn_testing_validators(false).await;
let substrate_context = test_context_stationary().await;
let entropy_api = get_api(&substrate_context.node_proc.ws_url).await.unwrap();
let rpc = get_rpc(&substrate_context.node_proc.ws_url).await.unwrap();
Expand Down Expand Up @@ -369,7 +369,7 @@ async fn test_sign_tx_no_chain_fail() {

let one = AccountKeyring::Dave;

let (_validator_ips, _validator_ids) = spawn_testing_validators().await;
let (_validator_ips, _validator_ids) = spawn_testing_validators(false).await;
let substrate_context = test_context_stationary().await;
let entropy_api = get_api(&substrate_context.node_proc.ws_url).await.unwrap();
let rpc = get_rpc(&substrate_context.node_proc.ws_url).await.unwrap();
Expand Down Expand Up @@ -493,7 +493,7 @@ async fn test_program_with_config() {
let one = AccountKeyring::Dave;
let two = AccountKeyring::Two;

let (_validator_ips, _validator_ids) = spawn_testing_validators().await;
let (_validator_ips, _validator_ids) = spawn_testing_validators(false).await;
let substrate_context = test_context_stationary().await;
let entropy_api = get_api(&substrate_context.node_proc.ws_url).await.unwrap();
let rpc = get_rpc(&substrate_context.node_proc.ws_url).await.unwrap();
Expand Down Expand Up @@ -559,7 +559,7 @@ async fn test_store_share() {
let program_manager = AccountKeyring::Dave;

let cxt = test_context_stationary().await;
let (_validator_ips, _validator_ids) = spawn_testing_validators().await;
let (_validator_ips, _validator_ids) = spawn_testing_validators(false).await;
let api = get_api(&cxt.node_proc.ws_url).await.unwrap();
let rpc = get_rpc(&cxt.node_proc.ws_url).await.unwrap();

Expand Down Expand Up @@ -730,7 +730,7 @@ async fn test_jumpstart_network() {
let alice = AccountKeyring::Alice;

let cxt = test_context_stationary().await;
let (_validator_ips, _validator_ids) = spawn_testing_validators().await;
let (_validator_ips, _validator_ids) = spawn_testing_validators(false).await;
let api = get_api(&cxt.node_proc.ws_url).await.unwrap();
let rpc = get_rpc(&cxt.node_proc.ws_url).await.unwrap();

Expand Down Expand Up @@ -927,7 +927,7 @@ async fn test_fail_infinite_program() {
let one = AccountKeyring::Dave;
let two = AccountKeyring::Two;

let (validator_ips, _validator_ids) = spawn_testing_validators().await;
let (validator_ips, _validator_ids) = spawn_testing_validators(false).await;
let substrate_context = test_context_stationary().await;
let entropy_api = get_api(&substrate_context.node_proc.ws_url).await.unwrap();
let rpc = get_rpc(&substrate_context.node_proc.ws_url).await.unwrap();
Expand Down Expand Up @@ -1026,7 +1026,7 @@ async fn test_device_key_proxy() {

let one = AccountKeyring::Dave;

let (_validator_ips, _validator_ids) = spawn_testing_validators().await;
let (_validator_ips, _validator_ids) = spawn_testing_validators(false).await;
let substrate_context = test_context_stationary().await;
let entropy_api = get_api(&substrate_context.node_proc.ws_url).await.unwrap();
let rpc = get_rpc(&substrate_context.node_proc.ws_url).await.unwrap();
Expand Down Expand Up @@ -1134,7 +1134,7 @@ async fn test_faucet() {
let two = AccountKeyring::Eve;
let alice = AccountKeyring::Alice;

let (validator_ips, _validator_ids) = spawn_testing_validators().await;
let (validator_ips, _validator_ids) = spawn_testing_validators(false).await;
let substrate_context = test_node_process_testing_state(true).await;
let entropy_api = get_api(&substrate_context.ws_url).await.unwrap();
let rpc = get_rpc(&substrate_context.ws_url).await.unwrap();
Expand Down
Loading