Use correct key rotation endpoint in OCW (#1104)

* Use correct key rotation endpoint in OCW * Tidy up a few things 🧹 * Change endpoint name to just `/rotate_network_key` The `/validator` prefix is a bit unneccessary. * RustFmt * Add `CHANGELOG` entry
entropyxyz · Oct 7, 2024 · 8f61fb0 · 8f61fb0
1 parent 3fa1793
commit 8f61fb0
Show file tree

Hide file tree

Showing 9 changed files with 35 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,6 +9,12 @@ At the moment this project **does not** adhere to
 
 ## [Unreleased](https://github.com/entropyxyz/entropy-core/compare/release/v0.3.0-rc.1...master)
 
+### Breaking Changes
+- In [#1104](https://github.com/entropyxyz/entropy-core/pull/1104) the `/validator/rotate_network_key` endpoint was renamed to `rotate_network_key`
+
+### Changed
+- Use correct key rotation endpoint in OCW ([#1104](https://github.com/entropyxyz/entropy-core/pull/1104))
+
 ## [0.3.0-rc.1](https://github.com/entropyxyz/entropy-core/compare/release/v0.2.0...release/v0.3.0-rc.1) - 2024-10-04
 
 ### Breaking Changes

diff --git a/crates/threshold-signature-server/src/lib.rs b/crates/threshold-signature-server/src/lib.rs
@@ -212,7 +212,7 @@ pub fn app(app_state: AppState) -> Router {
         .route("/user/relay_tx", post(relay_tx))
         .route("/signer/proactive_refresh", post(proactive_refresh))
         .route("/validator/reshare", post(new_reshare))
-        .route("/validator/rotate_network_key", post(rotate_network_key))
+        .route("/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/user/tests.rs b/crates/threshold-signature-server/src/user/tests.rs
@@ -29,7 +29,7 @@ use entropy_protocol::{
 };
 use entropy_shared::{
     HashingAlgorithm, DAVE_VERIFYING_KEY, DEFAULT_VERIFYING_KEY_NOT_REGISTERED, DEVICE_KEY_HASH,
-    EVE_VERIFYING_KEY, NETWORK_PARENT_KEY,
+    NETWORK_PARENT_KEY,
 };
 use entropy_testing_utils::{
     chain_api::{

diff --git a/crates/threshold-signature-server/src/validator/tests.rs b/crates/threshold-signature-server/src/validator/tests.rs
@@ -165,7 +165,7 @@ async fn test_reshare() {
     for signer in new_signers {
         let _ = client
             .post(format!(
-                "http://{}/validator/rotate_network_key",
+                "http://{}/rotate_network_key",
                 std::str::from_utf8(&signer.endpoint).unwrap()
             ))
             .send()
@@ -195,7 +195,7 @@ async fn test_reshare() {
         // calling twice doesn't do anything
         let response = client
             .post(format!(
-                "http://{}/validator/rotate_network_key",
+                "http://{}/rotate_network_key",
                 std::str::from_utf8(&signer.endpoint).unwrap()
             ))
             .send()
@@ -225,7 +225,7 @@ async fn test_reshare() {
     run_to_block(&rpc, current_block_number + 3).await;
 
     let response_stale =
-        client.post("http://127.0.0.1:3001/validator/rotate_network_key").send().await.unwrap();
+        client.post("http://127.0.0.1:3001/rotate_network_key").send().await.unwrap();
 
     assert_eq!(response_stale.text().await.unwrap(), "Data is stale");
 
@@ -301,7 +301,7 @@ async fn test_reshare_none_called() {
 
     for i in 0..validator_ports.len() {
         let response = client
-            .post(format!("http://127.0.0.1:{}/validator/rotate_network_key", validator_ports[i]))
+            .post(format!("http://127.0.0.1:{}/rotate_network_key", validator_ports[i]))
             .send()
             .await
             .unwrap();

diff --git a/crates/threshold-signature-server/tests/register_sign_reshare_sign.rs b/crates/threshold-signature-server/tests/register_sign_reshare_sign.rs
@@ -202,7 +202,7 @@ async fn do_reshare(api: &OnlineClient<EntropyConfig>, rpc: &LegacyRpcMethods<En
     for signer in new_signers {
         let _ = client
             .post(format!(
-                "http://{}/validator/rotate_network_key",
+                "http://{}/rotate_network_key",
                 std::str::from_utf8(&signer.endpoint).unwrap()
             ))
             .send()

diff --git a/node/cli/src/service.rs b/node/cli/src/service.rs
@@ -370,7 +370,7 @@ pub fn new_full_base(
             offchain_db.local_storage_set(
                 sp_core::offchain::StorageKind::PERSISTENT,
                 b"rotate_keyshares",
-                &format!("{}/validator/rotate_keyshares", endpoint).into_bytes(),
+                &format!("{}/rotate_network_key", endpoint).into_bytes(),
             );
             offchain_db.local_storage_set(
                 sp_core::offchain::StorageKind::PERSISTENT,

diff --git a/pallets/propagation/src/lib.rs b/pallets/propagation/src/lib.rs
@@ -70,7 +70,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);
+            let _ = Self::post_rotate_network_key(block_number);
         }
 
         fn on_initialize(_block_number: BlockNumberFor<T>) -> Weight {
@@ -255,20 +255,20 @@ pub mod pallet {
         }
 
         /// Submits a request to rotate parent network key the threshold servers.
-        pub fn post_rotate_keyshare(block_number: BlockNumberFor<T>) -> Result<(), http::Error> {
+        pub fn post_rotate_network_key(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");
+            let from_local = sp_io::offchain::local_storage_get(kind, b"rotate_network_key")
+                .unwrap_or_else(|| b"http://localhost:3001/rotate_network_key".to_vec());
+            let url =
+                str::from_utf8(&from_local).unwrap_or("http://localhost:3001/rotate_network_key");
 
-            log::warn!("propagation::post rotate keyshare");
+            log::warn!("propagation::post rotate network key");
 
             let converted_block_number: u32 =
                 BlockNumberFor::<T>::try_into(block_number).unwrap_or_default();

diff --git a/pallets/propagation/src/tests.rs b/pallets/propagation/src/tests.rs
@@ -64,7 +64,7 @@ fn knows_how_to_mock_several_http_calls() {
         });
         state.expect_request(testing::PendingRequest {
             method: "POST".into(),
-            uri: "http://localhost:3001/validator/rotate_keyshares".into(),
+            uri: "http://localhost:3001/rotate_network_key".into(),
             sent: true,
             response: Some([].to_vec()),
             body: [10, 0, 0, 0].to_vec(),
@@ -100,7 +100,7 @@ fn knows_how_to_mock_several_http_calls() {
         Propagation::post_reshare(7).unwrap();
 
         pallet_staking_extension::RotateKeyshares::<Test>::put(10);
-        Propagation::post_rotate_keyshare(10).unwrap();
+        Propagation::post_rotate_network_key(10).unwrap();
     })
 }
 

diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs
@@ -100,9 +100,6 @@ pub mod pallet {
         type WeightInfo: WeightInfo;
     }
 
-    /// A unique identifier of a subgroup or partition of validators that have the same set of
-    /// threshold shares.
-    pub type SubgroupId = u8;
     /// Endpoint where a threshold server can be reached at
     pub type TssServerURL = Vec<u8>;
 
@@ -599,7 +596,7 @@ pub mod pallet {
         #[pallet::weight(({
             <T as Config>::WeightInfo::confirm_key_reshare_confirmed(MAX_SIGNERS as u32)
             .max(<T as Config>::WeightInfo::confirm_key_reshare_completed())
-    }, DispatchClass::Operational))]
+        }, DispatchClass::Operational))]
         pub fn confirm_key_reshare(origin: OriginFor<T>) -> DispatchResultWithPostInfo {
             let ts_server_account = ensure_signed(origin)?;
             let validator_stash =
@@ -617,23 +614,29 @@ pub mod pallet {
                 Error::<T>::AlreadyConfirmed
             );
 
+            let current_signer_length = signers_info.next_signers.len();
+            let is_last_confirmation =
+                signers_info.confirmations.len() == (current_signer_length - 1);
+
             // TODO (#927): Add another check, such as a signature or a verifying key comparison, to
             // ensure that rotation was indeed successful.
-            let current_signer_length = signers_info.next_signers.len();
-            if signers_info.confirmations.len() == (current_signer_length - 1) {
+            if is_last_confirmation {
                 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 {
                 signers_info.confirmations.push(validator_stash.clone());
                 NextSigners::<T>::put(signers_info);
+
                 Self::deposit_event(Event::SignerConfirmed(validator_stash));
-                Ok(Pays::No.into())
             }
-            // TODO: weight is pays no but want a more accurate weight for max signers vs current signers see https://github.com/entropyxyz/entropy-core/issues/985
+
+            // TODO: Weight is `Pays::No` but want a more accurate weight for max signers vs current
+            // signers see https://github.com/entropyxyz/entropy-core/issues/985
+            Ok(Pays::No.into())
         }
     }