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 <peg@magmacollective.org>

* change when old network key is deleted

* Apply suggestions from code review

Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

---------

Co-authored-by: peg <peg@magmacollective.org>
Co-authored-by: Hernando Castano <HCastano@users.noreply.github.com>

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_";
 
+// 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
 pub const TOTAL_SIGNERS: u8 = 3;
 

crates/threshold-signature-server/src/lib.rs

@@ -149,7 +149,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)]
@@ -171,6 +171,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))

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,20 +173,72 @@ 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.
     confirm_key_reshare(&api, &rpc, &signer).await?;
     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<AppState>,
+) -> Result<StatusCode, ValidatorErr> {
+    // 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<EntropyConfig>,
+    rpc: &LegacyRpcMethods<EntropyConfig>,
+) -> 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<EntropyConfig>,

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() {

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",

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<T>) -> Weight {
@@ -96,6 +97,10 @@ pub mod pallet {
 
         /// Attestations request message passed
         AttestationRequestMessagePassed(OcwMessageAttestationRequest),
+
+        /// Key Rotate Message passed to validators
+        /// parameters. [BlockNumberFor<T>]
+        KeyRotatesMessagePassed(BlockNumberFor<T>),
     }
 
     #[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<T>) -> Result<(), http::Error> {
+            let rotate_keyshares = pallet_staking_extension::Pallet::<T>::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::<T>::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::<Vec<u8>>();
+
+            Self::deposit_event(Event::KeyRotatesMessagePassed(block_number));
+
+            Ok(())
+        }
+
         /// Submits a request for a TDX attestation.
         pub fn post_attestation_request(
             block_number: BlockNumberFor<T>,

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::<Test>::put(10);
+        Propagation::post_rotate_keyshare(10).unwrap();
     })
 }
 

pallets/staking/src/lib.rs

@@ -230,6 +230,11 @@ pub mod pallet {
     #[pallet::getter(fn jump_start_progress)]
     pub type JumpStartProgress<T: Config> = StorageValue<_, JumpStartDetails<T>, ValueQuery>;
 
+    /// Tell Signers to rotate keyshare
+    #[pallet::storage]
+    #[pallet::getter(fn rotate_keyshares)]
+    pub type RotateKeyshares<T: Config> = StorageValue<_, BlockNumberFor<T>, ValueQuery>;
+
     /// A type used to simplify the genesis configuration definition.
     pub type ThresholdServersConfig<T> = (
         <T as pallet_session::Config>::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::<T>::put(signers_info.next_signers.clone());
+                RotateKeyshares::<T>::put(
+                    <frame_system::Pallet<T>>::block_number() + sp_runtime::traits::One::one(),
+                );
                 Self::deposit_event(Event::SignersRotation(signers_info.next_signers));
                 Ok(Pays::No.into())
             } else {