From b5cd90a9b0defd5ad2a608f0dd779d9263108b1c Mon Sep 17 00:00:00 2001 From: JesseAbram <33698952+JesseAbram@users.noreply.github.com> Date: Wed, 28 Aug 2024 11:31:03 -0400 Subject: [PATCH] Rotate parent network key takes 2 steps (#1018) * Rotate parent netowrk key takes 2 steps * add rotate keyshare trigger to chain * make sure is signer * add validate rotate keyshare * test for stale check * docs * test fix * Apply suggestions from code review Co-authored-by: peg * change when old network key is deleted * Apply suggestions from code review Co-authored-by: Hernando Castano --------- Co-authored-by: peg Co-authored-by: Hernando Castano --- crates/client/entropy_metadata.scale | Bin 207747 -> 207926 bytes crates/shared/src/constants.rs | 3 + crates/threshold-signature-server/src/lib.rs | 3 +- .../src/validator/api.rs | 97 +++++++++++++++--- .../src/validator/tests.rs | 84 ++++++++++++++- node/cli/src/service.rs | 5 + pallets/propagation/src/lib.rs | 48 +++++++++ pallets/propagation/src/tests.rs | 11 ++ pallets/staking/src/lib.rs | 8 ++ 9 files changed, 244 insertions(+), 15 deletions(-) diff --git a/crates/client/entropy_metadata.scale b/crates/client/entropy_metadata.scale index dfc2ecfa21135ae47c209279c98a5d52de690450..2c9faa9d0e84c708ce08592872a63be6aa7fd84a 100644 GIT binary patch delta 174 zcmZp^&a>?X&xRE<8AUd)ocWwlD#kmtGAO?!u_U$FH?_DpF+DXPvA8%jg@s{qW4 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..fa7a8f880 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}; @@ -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); @@ -176,13 +173,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 +187,58 @@ pub async fn new_reshare( Ok(StatusCode::OK) } +/// HTTP POST endpoint called by the off-chain worker (propagation pallet) after a network key reshare. +/// +/// This rotates network key, deleting the previous network parent key. +#[tracing::instrument(skip_all)] +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?; + validate_rotate_network_key(&api, &rpc).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 = 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); + } + + 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( @@ -236,6 +285,30 @@ pub async fn validate_new_reshare( Ok(()) } +/// 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, +) -> 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 187c46206..07854ac49 100644 --- a/crates/threshold-signature-server/src/validator/tests.rs +++ b/crates/threshold-signature-server/src/validator/tests.rs @@ -101,15 +101,95 @@ async fn test_reshare() { let (key_share_after, aux_info_after): KeyShareWithAuxInfo = deserialize(&key_share_and_aux_data_after).unwrap(); + // Check key share has not yet changed + assert_eq!(serialize(&key_share_before).unwrap(), serialize(&key_share_after).unwrap()); + // Check aux info has not yet changed + 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).unwrap()); + 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).unwrap()); + assert_ne!( + 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() + ); } + + 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(); + + assert_eq!(response_stale.text().await.unwrap(), "Data is stale"); + // 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/node/cli/src/service.rs b/node/cli/src/service.rs index 873e6b6d0..ac481472a 100644 --- a/node/cli/src/service.rs +++ b/node/cli/src/service.rs @@ -367,6 +367,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 06b299b25..630aa4396 100644 --- a/pallets/propagation/src/lib.rs +++ b/pallets/propagation/src/lib.rs @@ -71,6 +71,7 @@ pub mod pallet { let _ = Self::post_reshare(block_number); let _ = Self::post_proactive_refresh(block_number); let _ = Self::post_attestation_request(block_number); + let _ = Self::post_rotate_keyshare(block_number); } fn on_initialize(_block_number: BlockNumberFor) -> Weight { @@ -96,6 +97,10 @@ pub mod pallet { /// Attestations request message passed AttestationRequestMessagePassed(OcwMessageAttestationRequest), + + /// Key Rotate Message passed to validators + /// parameters. [BlockNumberFor] + KeyRotatesMessagePassed(BlockNumberFor), } #[pallet::call] @@ -259,6 +264,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 != block_number { + 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 e139c3cd4..05172236c 100644 --- a/pallets/propagation/src/tests.rs +++ b/pallets/propagation/src/tests.rs @@ -64,6 +64,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(|| { @@ -91,6 +99,9 @@ fn knows_how_to_mock_several_http_calls() { }); // now triggers Propagation::post_reshare(7).unwrap(); + + 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 76af74d3b..fd4c79f50 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<_, BlockNumberFor, ValueQuery>; + /// A type used to simplify the genesis configuration definition. pub type ThresholdServersConfig = ( ::ValidatorId, @@ -505,6 +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( + >::block_number() + sp_runtime::traits::One::one(), + ); Self::deposit_event(Event::SignersRotation(signers_info.next_signers)); Ok(Pays::No.into()) } else {