From 160f8a857d4ad2a7f9be50f01c139dc7ede6dc95 Mon Sep 17 00:00:00 2001 From: Jesse Abramowitz Date: Wed, 21 Aug 2024 12:18:24 -0400 Subject: [PATCH 01/10] Rotate parent netowrk key takes 2 steps --- crates/shared/src/constants.rs | 3 ++ crates/threshold-signature-server/src/lib.rs | 3 +- .../src/validator/api.rs | 37 ++++++++++++++++--- .../src/validator/tests.rs | 26 ++++++++++++- 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/crates/shared/src/constants.rs b/crates/shared/src/constants.rs index 3193ff998..d6a4ee964 100644 --- a/crates/shared/src/constants.rs +++ b/crates/shared/src/constants.rs @@ -61,6 +61,9 @@ pub const DEVICE_KEY_PROXY: &[u8] = include_bytes!("../device_key_proxy.wasm"); /// Network parent key specific size to fit into [u8; 32] to save extra code pub const NETWORK_PARENT_KEY: &str = "NETWORK_PARENT_KEY_FOR_ENTROPY_"; +// Next Network parent key ready to be swapped in to new one +pub const NEXT_NETWORK_PARENT_KEY: &str = "NEXT_NETWORK_PARENT_KEY"; + /// Total signers on the network with the parent key pub const TOTAL_SIGNERS: u8 = 3; diff --git a/crates/threshold-signature-server/src/lib.rs b/crates/threshold-signature-server/src/lib.rs index ff145a3b3..99e52da85 100644 --- a/crates/threshold-signature-server/src/lib.rs +++ b/crates/threshold-signature-server/src/lib.rs @@ -157,7 +157,7 @@ use crate::{ r#unsafe::api::{delete, put, remove_keys, unsafe_get}, signing_client::{api::*, ListenerState}, user::api::*, - validator::api::new_reshare, + validator::api::{new_reshare, rotate_network_key}, }; #[derive(Clone)] @@ -180,6 +180,7 @@ pub fn app(app_state: AppState) -> Router { .route("/user/sign_tx", post(sign_tx)) .route("/signer/proactive_refresh", post(proactive_refresh)) .route("/validator/reshare", post(new_reshare)) + .route("/validator/rotate_network_key", post(rotate_network_key)) .route("/attest", post(attest)) .route("/healthz", get(healthz)) .route("/version", get(get_version)) diff --git a/crates/threshold-signature-server/src/validator/api.rs b/crates/threshold-signature-server/src/validator/api.rs index 810996711..f3e6d0954 100644 --- a/crates/threshold-signature-server/src/validator/api.rs +++ b/crates/threshold-signature-server/src/validator/api.rs @@ -35,7 +35,7 @@ pub use entropy_protocol::{ execute_protocol::{execute_protocol_generic, execute_reshare, Channels, PairWrapper}, KeyParams, KeyShareWithAuxInfo, Listener, PartyId, SessionId, ValidatorInfo, }; -use entropy_shared::{OcwMessageReshare, NETWORK_PARENT_KEY}; +use entropy_shared::{OcwMessageReshare, NETWORK_PARENT_KEY, NEXT_NETWORK_PARENT_KEY}; use parity_scale_codec::{Decode, Encode}; use sp_core::Pair; use std::{collections::BTreeSet, str::FromStr}; @@ -176,13 +176,13 @@ pub async fn new_reshare( let serialized_key_share = key_serialize(&(new_key_share, aux_info)) .map_err(|_| ProtocolErr::KvSerialize("Kv Serialize Error".to_string()))?; - let network_parent_key = hex::encode(NETWORK_PARENT_KEY); - // TODO: should this be a two step process? see # https://github.com/entropyxyz/entropy-core/issues/968 - if app_state.kv_store.kv().exists(&network_parent_key).await? { - app_state.kv_store.kv().delete(&network_parent_key).await? + let next_network_parent_key = hex::encode(NEXT_NETWORK_PARENT_KEY); + + if app_state.kv_store.kv().exists(&next_network_parent_key).await? { + app_state.kv_store.kv().delete(&next_network_parent_key).await? }; - let reservation = app_state.kv_store.kv().reserve_key(network_parent_key).await?; + let reservation = app_state.kv_store.kv().reserve_key(next_network_parent_key).await?; app_state.kv_store.kv().put(reservation, serialized_key_share.clone()).await?; // TODO: Error handling really complex needs to be thought about. @@ -190,6 +190,31 @@ pub async fn new_reshare( Ok(StatusCode::OK) } +// HTTP POST endpoint called by the off-chain worker (propagation pallet) after network reshare. +/// +/// This roatates network key. +#[tracing::instrument(skip_all)] +pub async fn rotate_network_key( + State(app_state): State, +) -> Result { + // validate from chain + + let network_parent_key_heading = hex::encode(NETWORK_PARENT_KEY); + let next_network_parent_key_heading = hex::encode(NEXT_NETWORK_PARENT_KEY); + + let new_parent_key = app_state.kv_store.kv().get(&next_network_parent_key_heading).await?; + + if app_state.kv_store.kv().exists(&network_parent_key_heading).await? { + app_state.kv_store.kv().delete(&network_parent_key_heading).await?; + }; + + app_state.kv_store.kv().delete(&next_network_parent_key_heading).await?; + + let reservation = app_state.kv_store.kv().reserve_key(network_parent_key_heading).await?; + app_state.kv_store.kv().put(reservation, new_parent_key).await?; + Ok(StatusCode::OK) +} + // Validates new reshare endpoint /// Checks the chain for validity of data and block number of data matches current block pub async fn validate_new_reshare( diff --git a/crates/threshold-signature-server/src/validator/tests.rs b/crates/threshold-signature-server/src/validator/tests.rs index 187c46206..9b6b9909a 100644 --- a/crates/threshold-signature-server/src/validator/tests.rs +++ b/crates/threshold-signature-server/src/validator/tests.rs @@ -102,9 +102,31 @@ async fn test_reshare() { deserialize(&key_share_and_aux_data_after).unwrap(); // Check key share has changed - assert_ne!(serialize(&key_share_before).unwrap(), serialize(&key_share_after).unwrap()); + assert_eq!(serialize(&key_share_before).unwrap(), serialize(&key_share_after).unwrap()); // Check aux info has changed - assert_ne!(serialize(&aux_info_before).unwrap(), serialize(&aux_info_after).unwrap()); + assert_eq!(serialize(&aux_info_before).unwrap(), serialize(&aux_info_after).unwrap()); + + let _ = client + .post(format!("http://127.0.0.1:{}/validator/rotate_network_key", validator_ports[i])) + .send() + .await + .unwrap(); + + let key_share_and_aux_data_after_rotate = + unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), validator_ports[i]).await; + let (key_share_after_rotate, aux_info_after_rotate): KeyShareWithAuxInfo = + deserialize(&key_share_and_aux_data_after_rotate).unwrap(); + + // Check key share has changed + assert_ne!( + serialize(&key_share_before).unwrap(), + serialize(&key_share_after_rotate).unwrap() + ); + // Check aux info has changed + assert_ne!( + serialize(&aux_info_before).unwrap(), + serialize(&aux_info_after_rotate).unwrap() + ); } // TODO #981 - test signing a message with the new keyshare set clean_tests(); From 9e4da4c73a11b38caeb645f6a73f752487122edb Mon Sep 17 00:00:00 2001 From: Jesse Abramowitz Date: Wed, 21 Aug 2024 16:12:35 -0400 Subject: [PATCH 02/10] add rotate keyshare trigger to chain --- node/cli/src/service.rs | 5 ++++ pallets/propagation/src/lib.rs | 47 ++++++++++++++++++++++++++++++++ pallets/propagation/src/tests.rs | 11 ++++++++ pallets/staking/src/lib.rs | 6 ++++ 4 files changed, 69 insertions(+) diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs index 72e7fac5b..0dbb6a88e 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -372,6 +372,11 @@ pub fn new_full_base( b"reshare_validators", &format!("{}/validator/reshare", endpoint).into_bytes(), ); + offchain_db.local_storage_set( + sp_core::offchain::StorageKind::PERSISTENT, + b"rotate_keyshares", + &format!("{}/validator/rotate_keyshares", endpoint).into_bytes(), + ); offchain_db.local_storage_set( sp_core::offchain::StorageKind::PERSISTENT, b"attest", diff --git a/pallets/propagation/src/lib.rs b/pallets/propagation/src/lib.rs index 31424c5bf..06326276e 100644 --- a/pallets/propagation/src/lib.rs +++ b/pallets/propagation/src/lib.rs @@ -98,6 +98,10 @@ pub mod pallet { /// Attestations request message passed AttestationRequestMessagePassed(OcwMessageAttestationRequest), + + /// Key Rotate Message passed to validators + /// parameters. [BlockNumberFor] + KeyRotatesMessagePassed(BlockNumberFor), } #[pallet::call] @@ -319,6 +323,49 @@ pub mod pallet { Ok(()) } + /// Submits a request to rotate parent network key the threshold servers. + pub fn post_rotate_keyshare(block_number: BlockNumberFor) -> Result<(), http::Error> { + let rotate_keyshares = pallet_staking_extension::Pallet::::rotate_keyshares(); + if !rotate_keyshares { + return Ok(()); + } + + let deadline = sp_io::offchain::timestamp().add(Duration::from_millis(2_000)); + let kind = sp_core::offchain::StorageKind::PERSISTENT; + let from_local = sp_io::offchain::local_storage_get(kind, b"rotate_keyshares") + .unwrap_or_else(|| b"http://localhost:3001/validator/rotate_keyshares".to_vec()); + let url = str::from_utf8(&from_local) + .unwrap_or("http://localhost:3001/validator/rotate_keyshares"); + + log::warn!("propagation::post rotate keyshare"); + + let converted_block_number: u32 = + BlockNumberFor::::try_into(block_number).unwrap_or_default(); + + // We construct the request + // important: the header->Content-Type must be added and match that of the receiving + // party!! + let pending = http::Request::post(url, vec![converted_block_number.encode()]) + .deadline(deadline) + .send() + .map_err(|_| http::Error::IoError)?; + + // We await response, same as in fn get() + let response = + pending.try_wait(deadline).map_err(|_| http::Error::DeadlineReached)??; + + // check response code + if response.code != 200 { + log::warn!("Unexpected status code: {}", response.code); + return Err(http::Error::Unknown); + } + let _res_body = response.body().collect::>(); + + Self::deposit_event(Event::KeyRotatesMessagePassed(block_number)); + + Ok(()) + } + /// Submits a request for a TDX attestation. pub fn post_attestation_request( block_number: BlockNumberFor, diff --git a/pallets/propagation/src/tests.rs b/pallets/propagation/src/tests.rs index b3e411e11..857f2b705 100644 --- a/pallets/propagation/src/tests.rs +++ b/pallets/propagation/src/tests.rs @@ -81,6 +81,14 @@ fn knows_how_to_mock_several_http_calls() { body: [32, 1, 0, 0, 0, 0, 0, 0, 0, 6, 0, 0, 0].to_vec(), ..Default::default() }); + state.expect_request(testing::PendingRequest { + method: "POST".into(), + uri: "http://localhost:3001/validator/rotate_keyshares".into(), + sent: true, + response: Some([].to_vec()), + body: [10, 0, 0, 0].to_vec(), + ..Default::default() + }); }); t.execute_with(|| { @@ -137,6 +145,9 @@ fn knows_how_to_mock_several_http_calls() { }); // now triggers Propagation::post_reshare(7).unwrap(); + + pallet_staking_extension::RotateKeyshares::::put(true); + Propagation::post_rotate_keyshare(10).unwrap(); }) } diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs index 38d357f1f..c67308f74 100644 --- a/pallets/staking/src/lib.rs +++ b/pallets/staking/src/lib.rs @@ -230,6 +230,11 @@ pub mod pallet { #[pallet::getter(fn jump_start_progress)] pub type JumpStartProgress = StorageValue<_, JumpStartDetails, ValueQuery>; + /// Tell Signers to rotate keyshare + #[pallet::storage] + #[pallet::getter(fn rotate_keyshares)] + pub type RotateKeyshares = StorageValue<_, bool, ValueQuery>; + /// A type used to simplify the genesis configuration definition. pub type ThresholdServersConfig = ( ::ValidatorId, @@ -505,6 +510,7 @@ pub mod pallet { let current_signer_length = signers_info.next_signers.len(); if signers_info.confirmations.len() == (current_signer_length - 1) { Signers::::put(signers_info.next_signers.clone()); + RotateKeyshares::::put(true); Self::deposit_event(Event::SignersRotation(signers_info.next_signers)); Ok(Pays::No.into()) } else { From 4c64b8bc78fefde6317a87737ceeee023a9c2e67 Mon Sep 17 00:00:00 2001 From: Jesse Abramowitz Date: Wed, 21 Aug 2024 19:02:17 -0400 Subject: [PATCH 03/10] make sure is signer --- .../src/validator/api.rs | 23 +++++++++++++++++++ pallets/propagation/src/lib.rs | 4 +++- 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/crates/threshold-signature-server/src/validator/api.rs b/crates/threshold-signature-server/src/validator/api.rs index f3e6d0954..e0586affb 100644 --- a/crates/threshold-signature-server/src/validator/api.rs +++ b/crates/threshold-signature-server/src/validator/api.rs @@ -198,6 +198,29 @@ pub async fn rotate_network_key( State(app_state): State, ) -> Result { // validate from chain + let api = get_api(&app_state.configuration.endpoint).await?; + let rpc = get_rpc(&app_state.configuration.endpoint).await?; + + let (signer, _) = get_signer_and_x25519_secret(&app_state.kv_store) + .await + .map_err(|e| ValidatorErr::UserError(e.to_string()))?; + + let signers_query = entropy::storage().staking_extension().signers(); + let signers = query_chain(&api, &rpc, signers_query, None) + .await? + .ok_or_else(|| ValidatorErr::ChainFetch("Error getting signers"))?; + + let validators_info = get_validators_info(&api, &rpc, signers) + .await + .map_err(|e| ValidatorErr::UserError(e.to_string()))?; + + let is_proper_signer = validators_info + .iter() + .any(|validator_info| validator_info.tss_account == *signer.account_id()); + + if !is_proper_signer { + return Ok(StatusCode::MISDIRECTED_REQUEST); + } let network_parent_key_heading = hex::encode(NETWORK_PARENT_KEY); let next_network_parent_key_heading = hex::encode(NEXT_NETWORK_PARENT_KEY); diff --git a/pallets/propagation/src/lib.rs b/pallets/propagation/src/lib.rs index 06326276e..21f0306b0 100644 --- a/pallets/propagation/src/lib.rs +++ b/pallets/propagation/src/lib.rs @@ -70,13 +70,15 @@ pub mod pallet { let _ = Self::post_dkg(block_number); let _ = Self::post_reshare(block_number); let _ = Self::post_user_registration(block_number); - let _ = Self::post_proactive_refresh(block_number); let _ = Self::post_attestation_request(block_number); + let _ = Self::post_proactive_refresh(block_number); + let _ = Self::post_rotate_keyshare } fn on_initialize(block_number: BlockNumberFor) -> Weight { pallet_registry::Dkg::::remove(block_number.saturating_sub(2u32.into())); pallet_staking_extension::ProactiveRefresh::::take(); + // delete rotate keyshare trigger ::WeightInfo::on_initialize() } } From 5175183d2ec2eb6c0bf99b65d4d2cfe39f7c9deb Mon Sep 17 00:00:00 2001 From: Jesse Abramowitz Date: Thu, 22 Aug 2024 14:08:17 -0400 Subject: [PATCH 04/10] add validate rotate keyshare --- crates/client/entropy_metadata.scale | Bin 209102 -> 209508 bytes .../src/validator/api.rs | 24 +++++++++ .../src/validator/tests.rs | 50 ++++++++++++++++++ pallets/propagation/src/lib.rs | 5 +- pallets/propagation/src/tests.rs | 2 +- pallets/staking/src/lib.rs | 6 ++- 6 files changed, 81 insertions(+), 6 deletions(-) diff --git a/crates/client/entropy_metadata.scale b/crates/client/entropy_metadata.scale index c0c5982a0fe6dd2d75315561d406a7d993720319..dd28d7ff493e0fabf6c9ae55e13a4bfc3667fc3a 100644 GIT binary patch delta 522 zcmZ`$JxeP=6y1$eSolOl8{xq%`~j9CBKV2np{R+4pv7e8#vQVm8)s(yz(U35HG&cg z!Q}D4N-M!4{sb%kfuNxH6KuV11Z|vdF6W$k?m72QUC+;5*O%vE-_6zeYdEm_2X~gp zOOI|2os;XhAe}=cg8BjV{}P=I20$B8myO@`&SrP-L_$$zKCPC}&X64~j;$S@G+QUl z@Ns2NoGzy15h6+x1%^D75Ngu0CXj_qw7RTO6_*We*D&&^#yu}*2bxR}io0)Lz-@1} zaXJE*ma3Y83btZ2nXbi+l!>Tj^Clu|SuAh%YN?-+UIsaFH!EGt1Z{W*iie;GUB<0S zAH^&sJ#0zGd)iip=c(YCH6F2j<1%uw@s6Y$J!5h?b9A z$(u3-LNtEl6${`pC>tx2_&;qeSqB!AOx6?l7(sr8!s#Um`qs-^czYZUw*LLiJ&2Lf pb|>79i={;u20c9iKd}!*OcE&Qb%U(SdN%79Dxc8S?oqCA@ebbU#5({0 delta 119 zcmV--0EqwO07yx0V{dYD zWn*+nZg6#UL}7Gc00ICwPH$6mVRL9fV`Fc1Zgcfi2~gOw-Jv5Q=|ULEg=8^ diff --git a/crates/threshold-signature-server/src/validator/api.rs b/crates/threshold-signature-server/src/validator/api.rs index e0586affb..8a4d3cee5 100644 --- a/crates/threshold-signature-server/src/validator/api.rs +++ b/crates/threshold-signature-server/src/validator/api.rs @@ -200,6 +200,7 @@ pub async fn rotate_network_key( // validate from chain let api = get_api(&app_state.configuration.endpoint).await?; let rpc = get_rpc(&app_state.configuration.endpoint).await?; + validate_rotate_network_key(&api, &rpc).await?; let (signer, _) = get_signer_and_x25519_secret(&app_state.kv_store) .await @@ -284,6 +285,29 @@ pub async fn validate_new_reshare( Ok(()) } +/// Validates rotate_network_key +/// Checks the chain that the reshare was completed +pub async fn validate_rotate_network_key( + api: &OnlineClient, + rpc: &LegacyRpcMethods, +) -> Result<(), ValidatorErr> { + let latest_block_number = rpc + .chain_get_header(None) + .await? + .ok_or_else(|| ValidatorErr::OptionUnwrapError("Failed to get block number".to_string()))? + .number; + + let rotate_keyshares_info_query = entropy::storage().staking_extension().rotate_keyshares(); + let rotate_keyshare_block = query_chain(api, rpc, rotate_keyshares_info_query, None) + .await? + .ok_or_else(|| ValidatorErr::ChainFetch("Rotate Keyshare not in progress"))?; + + if latest_block_number > rotate_keyshare_block { + return Err(ValidatorErr::StaleData); + } + + Ok(()) +} /// Confirms that a validator has succefully reshared. pub async fn confirm_key_reshare( api: &OnlineClient, diff --git a/crates/threshold-signature-server/src/validator/tests.rs b/crates/threshold-signature-server/src/validator/tests.rs index 9b6b9909a..e98821d6a 100644 --- a/crates/threshold-signature-server/src/validator/tests.rs +++ b/crates/threshold-signature-server/src/validator/tests.rs @@ -127,11 +127,61 @@ async fn test_reshare() { serialize(&aux_info_before).unwrap(), serialize(&aux_info_after_rotate).unwrap() ); + + // calling twice doesn't do anything + let response = client + .post(format!("http://127.0.0.1:{}/validator/rotate_network_key", validator_ports[i])) + .send() + .await + .unwrap(); + + assert_eq!(response.text().await.unwrap(), "Kv error: Recv Error: channel closed"); + let key_share_and_aux_data_after_rotate_twice = + unsafe_get(&client, hex::encode(NETWORK_PARENT_KEY), validator_ports[i]).await; + let (key_share_after_rotate_twice, aux_info_after_rotate_twice): KeyShareWithAuxInfo = + deserialize(&key_share_and_aux_data_after_rotate_twice).unwrap(); + + // Check key share has not changed + assert_eq!( + serialize(&key_share_after_rotate_twice).unwrap(), + serialize(&key_share_after_rotate).unwrap() + ); + // Check aux info has not changed + assert_eq!( + serialize(&aux_info_after_rotate_twice).unwrap(), + serialize(&aux_info_after_rotate).unwrap() + ); } // TODO #981 - test signing a message with the new keyshare set clean_tests(); } +#[tokio::test] +#[serial] +async fn test_reshare_none_called() { + initialize_test_logger().await; + clean_tests(); + + let _cxt = test_node_process_testing_state(true).await; + + let add_parent_key_to_kvdb = true; + let (_validator_ips, _validator_ids) = spawn_testing_validators(add_parent_key_to_kvdb).await; + + let validator_ports = vec![3001, 3002, 3003]; + + let client = reqwest::Client::new(); + + for i in 0..validator_ports.len() { + let response = client + .post(format!("http://127.0.0.1:{}/validator/rotate_network_key", validator_ports[i])) + .send() + .await + .unwrap(); + + assert_eq!(response.text().await.unwrap(), "Chain Fetch: Rotate Keyshare not in progress"); + } +} + #[tokio::test] #[serial] async fn test_reshare_validation_fail() { diff --git a/pallets/propagation/src/lib.rs b/pallets/propagation/src/lib.rs index 21f0306b0..7d110665a 100644 --- a/pallets/propagation/src/lib.rs +++ b/pallets/propagation/src/lib.rs @@ -72,13 +72,12 @@ pub mod pallet { let _ = Self::post_user_registration(block_number); let _ = Self::post_attestation_request(block_number); let _ = Self::post_proactive_refresh(block_number); - let _ = Self::post_rotate_keyshare + let _ = Self::post_rotate_keyshare(block_number); } fn on_initialize(block_number: BlockNumberFor) -> Weight { pallet_registry::Dkg::::remove(block_number.saturating_sub(2u32.into())); pallet_staking_extension::ProactiveRefresh::::take(); - // delete rotate keyshare trigger ::WeightInfo::on_initialize() } } @@ -328,7 +327,7 @@ pub mod pallet { /// Submits a request to rotate parent network key the threshold servers. pub fn post_rotate_keyshare(block_number: BlockNumberFor) -> Result<(), http::Error> { let rotate_keyshares = pallet_staking_extension::Pallet::::rotate_keyshares(); - if !rotate_keyshares { + if rotate_keyshares != block_number { return Ok(()); } diff --git a/pallets/propagation/src/tests.rs b/pallets/propagation/src/tests.rs index 857f2b705..eae4e59a3 100644 --- a/pallets/propagation/src/tests.rs +++ b/pallets/propagation/src/tests.rs @@ -146,7 +146,7 @@ fn knows_how_to_mock_several_http_calls() { // now triggers Propagation::post_reshare(7).unwrap(); - pallet_staking_extension::RotateKeyshares::::put(true); + pallet_staking_extension::RotateKeyshares::::put(10); Propagation::post_rotate_keyshare(10).unwrap(); }) } diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs index c67308f74..b94707d02 100644 --- a/pallets/staking/src/lib.rs +++ b/pallets/staking/src/lib.rs @@ -233,7 +233,7 @@ pub mod pallet { /// Tell Signers to rotate keyshare #[pallet::storage] #[pallet::getter(fn rotate_keyshares)] - pub type RotateKeyshares = StorageValue<_, bool, ValueQuery>; + pub type RotateKeyshares = StorageValue<_, BlockNumberFor, ValueQuery>; /// A type used to simplify the genesis configuration definition. pub type ThresholdServersConfig = ( @@ -510,7 +510,9 @@ pub mod pallet { let current_signer_length = signers_info.next_signers.len(); if signers_info.confirmations.len() == (current_signer_length - 1) { Signers::::put(signers_info.next_signers.clone()); - RotateKeyshares::::put(true); + RotateKeyshares::::put( + >::block_number() + sp_runtime::traits::One::one(), + ); Self::deposit_event(Event::SignersRotation(signers_info.next_signers)); Ok(Pays::No.into()) } else { From eddb5e5e6095588d7bb798576afefaa68d12cd58 Mon Sep 17 00:00:00 2001 From: Jesse Abramowitz Date: Thu, 22 Aug 2024 16:56:17 -0400 Subject: [PATCH 05/10] test for stale check --- crates/threshold-signature-server/src/validator/tests.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/crates/threshold-signature-server/src/validator/tests.rs b/crates/threshold-signature-server/src/validator/tests.rs index e98821d6a..d1b78a6a1 100644 --- a/crates/threshold-signature-server/src/validator/tests.rs +++ b/crates/threshold-signature-server/src/validator/tests.rs @@ -152,6 +152,14 @@ async fn test_reshare() { serialize(&aux_info_after_rotate).unwrap() ); } + + run_to_block(&rpc, block_number + 5).await; + + let response_stale = + client.post("http://127.0.0.1:3001/validator/rotate_network_key").send().await.unwrap(); + + assert_eq!(response_stale.text().await.unwrap(), "Data is stale"); + // TODO #981 - test signing a message with the new keyshare set clean_tests(); } From dd4a0cb2bc1e11448b9098eed8138de9ac9be971 Mon Sep 17 00:00:00 2001 From: Jesse Abramowitz Date: Thu, 22 Aug 2024 16:57:11 -0400 Subject: [PATCH 06/10] docs --- crates/threshold-signature-server/src/validator/api.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/threshold-signature-server/src/validator/api.rs b/crates/threshold-signature-server/src/validator/api.rs index 8a4d3cee5..2def65eaf 100644 --- a/crates/threshold-signature-server/src/validator/api.rs +++ b/crates/threshold-signature-server/src/validator/api.rs @@ -286,7 +286,8 @@ pub async fn validate_new_reshare( } /// Validates rotate_network_key -/// Checks the chain that the reshare was completed +/// Checks the chain that the reshare was completed recently +/// We only care that this happens after a reshare so we just check that message isn't stale pub async fn validate_rotate_network_key( api: &OnlineClient, rpc: &LegacyRpcMethods, From 2f4a73153bca4b20e148f94e9779fb3912083267 Mon Sep 17 00:00:00 2001 From: Jesse Abramowitz Date: Thu, 22 Aug 2024 20:51:45 -0400 Subject: [PATCH 07/10] test fix --- crates/threshold-signature-server/src/validator/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/threshold-signature-server/src/validator/tests.rs b/crates/threshold-signature-server/src/validator/tests.rs index d1b78a6a1..e33ca8bf5 100644 --- a/crates/threshold-signature-server/src/validator/tests.rs +++ b/crates/threshold-signature-server/src/validator/tests.rs @@ -153,7 +153,7 @@ async fn test_reshare() { ); } - run_to_block(&rpc, block_number + 5).await; + run_to_block(&rpc, block_number + 7).await; let response_stale = client.post("http://127.0.0.1:3001/validator/rotate_network_key").send().await.unwrap(); From 085a0fb0cfd6fb85be1f456de013d99420f5ecce Mon Sep 17 00:00:00 2001 From: JesseAbram <33698952+JesseAbram@users.noreply.github.com> Date: Mon, 26 Aug 2024 08:36:21 -0700 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: peg --- crates/shared/src/constants.rs | 2 +- crates/threshold-signature-server/src/validator/tests.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/shared/src/constants.rs b/crates/shared/src/constants.rs index d6a4ee964..d7723fed0 100644 --- a/crates/shared/src/constants.rs +++ b/crates/shared/src/constants.rs @@ -61,7 +61,7 @@ pub const DEVICE_KEY_PROXY: &[u8] = include_bytes!("../device_key_proxy.wasm"); /// Network parent key specific size to fit into [u8; 32] to save extra code pub const NETWORK_PARENT_KEY: &str = "NETWORK_PARENT_KEY_FOR_ENTROPY_"; -// Next Network parent key ready to be swapped in to new one +// Lookup key for the next network parent keyshare ready to be swapped in to new one pub const NEXT_NETWORK_PARENT_KEY: &str = "NEXT_NETWORK_PARENT_KEY"; /// Total signers on the network with the parent key diff --git a/crates/threshold-signature-server/src/validator/tests.rs b/crates/threshold-signature-server/src/validator/tests.rs index e33ca8bf5..07854ac49 100644 --- a/crates/threshold-signature-server/src/validator/tests.rs +++ b/crates/threshold-signature-server/src/validator/tests.rs @@ -101,9 +101,9 @@ async fn test_reshare() { let (key_share_after, aux_info_after): KeyShareWithAuxInfo = deserialize(&key_share_and_aux_data_after).unwrap(); - // Check key share has changed + // Check key share has not yet changed assert_eq!(serialize(&key_share_before).unwrap(), serialize(&key_share_after).unwrap()); - // Check aux info has changed + // Check aux info has not yet changed assert_eq!(serialize(&aux_info_before).unwrap(), serialize(&aux_info_after).unwrap()); let _ = client From 705ffa3a06ef44860b17cacf7dfd5b7c264e39d9 Mon Sep 17 00:00:00 2001 From: Jesse Abramowitz Date: Tue, 27 Aug 2024 19:43:14 -0400 Subject: [PATCH 09/10] change when old network key is deleted --- .../src/validator/api.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/threshold-signature-server/src/validator/api.rs b/crates/threshold-signature-server/src/validator/api.rs index 2def65eaf..caa91c82c 100644 --- a/crates/threshold-signature-server/src/validator/api.rs +++ b/crates/threshold-signature-server/src/validator/api.rs @@ -93,12 +93,9 @@ pub async fn new_reshare( ) .map_err(|e| ValidatorErr::VerifyingKeyError(e.to_string()))?; - let is_proper_signer = is_signer_or_delete_parent_key( - signer.account_id(), - validators_info.clone(), - &app_state.kv_store, - ) - .await?; + let is_proper_signer = validators_info + .iter() + .any(|validator_info| validator_info.tss_account == *signer.account_id()); if !is_proper_signer { return Ok(StatusCode::MISDIRECTED_REQUEST); @@ -215,9 +212,12 @@ pub async fn rotate_network_key( .await .map_err(|e| ValidatorErr::UserError(e.to_string()))?; - let is_proper_signer = validators_info - .iter() - .any(|validator_info| validator_info.tss_account == *signer.account_id()); + let is_proper_signer = is_signer_or_delete_parent_key( + signer.account_id(), + validators_info.clone(), + &app_state.kv_store, + ) + .await?; if !is_proper_signer { return Ok(StatusCode::MISDIRECTED_REQUEST); From 50e02c5f538125c6324a7befb871257eebefc65a Mon Sep 17 00:00:00 2001 From: JesseAbram <33698952+JesseAbram@users.noreply.github.com> Date: Tue, 27 Aug 2024 16:47:39 -0700 Subject: [PATCH 10/10] Apply suggestions from code review Co-authored-by: Hernando Castano --- crates/threshold-signature-server/src/validator/api.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/threshold-signature-server/src/validator/api.rs b/crates/threshold-signature-server/src/validator/api.rs index caa91c82c..fa7a8f880 100644 --- a/crates/threshold-signature-server/src/validator/api.rs +++ b/crates/threshold-signature-server/src/validator/api.rs @@ -187,9 +187,9 @@ pub async fn new_reshare( Ok(StatusCode::OK) } -// HTTP POST endpoint called by the off-chain worker (propagation pallet) after network reshare. +/// HTTP POST endpoint called by the off-chain worker (propagation pallet) after a network key reshare. /// -/// This roatates network key. +/// This rotates network key, deleting the previous network parent key. #[tracing::instrument(skip_all)] pub async fn rotate_network_key( State(app_state): State, @@ -285,8 +285,8 @@ pub async fn validate_new_reshare( Ok(()) } -/// Validates rotate_network_key -/// Checks the chain that the reshare was completed recently +/// Checks the chain that the reshare was completed recently. +/// /// We only care that this happens after a reshare so we just check that message isn't stale pub async fn validate_rotate_network_key( api: &OnlineClient,