use hash instead of coldkey during announcement

Author: l0r1s

pallets/subtensor/src/macros/dispatches.rs

@@ -9,7 +9,7 @@ mod dispatches {
     use frame_support::traits::schedule::v3::Anon as ScheduleAnon;
     use frame_system::pallet_prelude::BlockNumberFor;
     use sp_core::ecdsa::Signature;
-    use sp_runtime::Percent;
+    use sp_runtime::{Percent, traits::Hash};
 
     use crate::MAX_CRV3_COMMIT_SIZE_BYTES;
     use crate::MAX_NUM_ROOT_CLAIMS;
@@ -2335,12 +2335,13 @@ mod dispatches {
             Ok(())
         }
 
-        /// Announces a coldkey swap. This is required before the coldkey swap can be performed after the delay period.
+        /// Announces a coldkey swap using coldkey hash.
+        /// This is required before the coldkey swap can be performed after the delay period.
         #[pallet::call_index(125)]
         #[pallet::weight(Weight::zero())]
         pub fn announce_coldkey_swap(
             origin: OriginFor<T>,
-            new_coldkey: T::AccountId,
+            new_coldkey_hash: T::Hash,
         ) -> DispatchResult {
             let who = ensure_signed(origin)?;
             let now = <frame_system::Pallet<T>>::block_number();
@@ -2354,25 +2355,34 @@ mod dispatches {
                 );
             }
 
-            ColdkeySwapAnnouncements::<T>::insert(who.clone(), (now, new_coldkey.clone()));
+            ColdkeySwapAnnouncements::<T>::insert(who.clone(), (now, new_coldkey_hash.clone()));
 
             Self::deposit_event(Event::ColdkeySwapAnnounced {
                 who: who.clone(),
-                new_coldkey: new_coldkey.clone(),
+                new_coldkey_hash: new_coldkey_hash.clone(),
                 block_number: now,
             });
             Ok(())
         }
 
         /// Performs a coldkey swap iff an announcement has been made.
+        /// The provided new coldkey must match the announced coldkey hash.
         #[pallet::call_index(126)]
         #[pallet::weight(Weight::zero())]
-        pub fn swap_coldkey_announced(origin: OriginFor<T>) -> DispatchResult {
+        pub fn swap_coldkey_announced(
+            origin: OriginFor<T>,
+            new_coldkey: T::AccountId,
+        ) -> DispatchResult {
             let who = ensure_signed(origin)?;
 
-            let (when, new_coldkey) = ColdkeySwapAnnouncements::<T>::take(who.clone())
+            let (when, new_coldkey_hash) = ColdkeySwapAnnouncements::<T>::take(who.clone())
                 .ok_or(Error::<T>::ColdKeySwapAnnouncementNotFound)?;
 
+            ensure!(
+                new_coldkey_hash == T::Hashing::hash_of(&new_coldkey),
+                Error::<T>::AnnouncedColdkeyHashDoesNotMatch
+            );
+
             let now = <frame_system::Pallet<T>>::block_number();
             let delay = ColdkeySwapScheduleDuration::<T>::get();
             ensure!(now > when + delay, Error::<T>::ColdKeySwapTooEarly);

pallets/subtensor/src/macros/errors.rs

@@ -154,6 +154,8 @@ mod errors {
         ColdKeySwapTooEarly,
         /// Coldkey swap reannounced too early.
         ColdKeySwapReannouncedTooEarly,
+        /// The announced coldkey hash does not match the new coldkey hash.
+        AnnouncedColdkeyHashDoesNotMatch,
         /// New coldkey is hotkey
         NewColdKeyIsHotkey,
         /// Childkey take is invalid.

pallets/subtensor/src/macros/events.rs

@@ -174,8 +174,8 @@ mod events {
         ColdkeySwapAnnounced {
             /// The account ID of the coldkey that made the announcement.
             who: T::AccountId,
-            /// The account ID of the new coldkey.
-            new_coldkey: T::AccountId,
+            /// The hash of the new coldkey.
+            new_coldkey_hash: T::Hash,
             /// The block number the announcement was made.
             block_number: BlockNumberFor<T>,
         },

pallets/subtensor/src/tests/swap_coldkey.rs

@@ -16,6 +16,7 @@ use frame_support::traits::schedule::v3::Named as ScheduleNamed;
 use frame_support::{assert_err, assert_noop, assert_ok};
 use frame_system::{Config, RawOrigin};
 use sp_core::{Get, H256, U256};
+use sp_runtime::traits::Hash;
 use sp_runtime::traits::{DispatchInfoOf, DispatchTransaction, TransactionExtension};
 use sp_runtime::{DispatchError, traits::TxBaseImplication};
 use substrate_fixed::types::U96F32;
@@ -33,24 +34,25 @@ fn test_announce_coldkey_swap_works() {
     new_test_ext(1).execute_with(|| {
         let who = U256::from(1);
         let new_coldkey = U256::from(2);
+        let new_coldkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey);
 
         assert_eq!(ColdkeySwapAnnouncements::<Test>::iter().count(), 0);
 
         assert_ok!(SubtensorModule::announce_coldkey_swap(
             RuntimeOrigin::signed(who.clone()),
-            new_coldkey,
+            new_coldkey_hash,
         ));
 
         let now = System::block_number();
         assert_eq!(
             ColdkeySwapAnnouncements::<Test>::iter().collect::<Vec<_>>(),
-            vec![(who.clone(), (now, new_coldkey))]
+            vec![(who.clone(), (now, new_coldkey_hash))]
         );
         assert_eq!(
             last_event(),
             RuntimeEvent::SubtensorModule(Event::ColdkeySwapAnnounced {
                 who,
-                new_coldkey,
+                new_coldkey_hash,
                 block_number: now,
             })
         );
@@ -62,33 +64,35 @@ fn test_announce_coldkey_swap_with_existing_announcement_past_delay_works() {
     new_test_ext(1).execute_with(|| {
         let who = U256::from(1);
         let new_coldkey = U256::from(2);
+        let new_coldkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey);
         let new_coldkey_2 = U256::from(3);
+        let new_coldkey_2_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey_2);
 
         assert_eq!(ColdkeySwapAnnouncements::<Test>::iter().count(), 0);
 
         assert_ok!(SubtensorModule::announce_coldkey_swap(
             RuntimeOrigin::signed(who.clone()),
-            new_coldkey,
+            new_coldkey_hash,
         ));
 
         let now = System::block_number();
         assert_eq!(
             ColdkeySwapAnnouncements::<Test>::iter().collect::<Vec<_>>(),
-            vec![(who.clone(), (now, new_coldkey))]
+            vec![(who.clone(), (now, new_coldkey_hash))]
         );
 
         let delay = ColdkeySwapScheduleDuration::<Test>::get() + 1;
         System::run_to_block::<AllPalletsWithSystem>(now + delay);
 
         assert_ok!(SubtensorModule::announce_coldkey_swap(
             RuntimeOrigin::signed(who.clone()),
-            new_coldkey_2,
+            new_coldkey_2_hash,
         ));
 
         let now = System::block_number();
         assert_eq!(
             ColdkeySwapAnnouncements::<Test>::iter().collect::<Vec<_>>(),
-            vec![(who.clone(), (now, new_coldkey_2))]
+            vec![(who.clone(), (now, new_coldkey_2_hash))]
         );
     });
 }
@@ -97,14 +101,15 @@ fn test_announce_coldkey_swap_with_existing_announcement_past_delay_works() {
 fn test_announce_coldkey_swap_with_bad_origin_fails() {
     new_test_ext(1).execute_with(|| {
         let new_coldkey = U256::from(1);
+        let new_coldkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey);
 
         assert_noop!(
-            SubtensorModule::announce_coldkey_swap(RuntimeOrigin::none(), new_coldkey),
+            SubtensorModule::announce_coldkey_swap(RuntimeOrigin::none(), new_coldkey_hash),
             BadOrigin
         );
 
         assert_noop!(
-            SubtensorModule::announce_coldkey_swap(RuntimeOrigin::root(), new_coldkey),
+            SubtensorModule::announce_coldkey_swap(RuntimeOrigin::root(), new_coldkey_hash),
             BadOrigin
         );
     });
@@ -115,19 +120,21 @@ fn test_announce_coldkey_swap_with_existing_announcement_not_past_delay_fails()
     new_test_ext(1).execute_with(|| {
         let who = U256::from(1);
         let new_coldkey = U256::from(2);
+        let new_coldkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey);
         let new_coldkey_2 = U256::from(3);
+        let new_coldkey_2_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey_2);
 
         assert_eq!(ColdkeySwapAnnouncements::<Test>::iter().count(), 0);
 
         assert_ok!(SubtensorModule::announce_coldkey_swap(
             RuntimeOrigin::signed(who.clone()),
-            new_coldkey,
+            new_coldkey_hash,
         ));
 
         let now = System::block_number();
         assert_eq!(
             ColdkeySwapAnnouncements::<Test>::iter().collect::<Vec<_>>(),
-            vec![(who.clone(), (now, new_coldkey))]
+            vec![(who.clone(), (now, new_coldkey_hash))]
         );
 
         let unmet_delay = ColdkeySwapScheduleDuration::<Test>::get();
@@ -136,7 +143,7 @@ fn test_announce_coldkey_swap_with_existing_announcement_not_past_delay_fails()
         assert_noop!(
             SubtensorModule::announce_coldkey_swap(
                 RuntimeOrigin::signed(who.clone()),
-                new_coldkey_2,
+                new_coldkey_2_hash,
             ),
             Error::<Test>::ColdKeySwapReannouncedTooEarly
         );
@@ -148,6 +155,7 @@ fn test_swap_coldkey_announced_works() {
     new_test_ext(1).execute_with(|| {
         let who = U256::from(1);
         let new_coldkey = U256::from(2);
+        let new_coldkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey);
         let hotkey1 = U256::from(1001);
         let hotkey2 = U256::from(1002);
         let hotkey3 = U256::from(1003);
@@ -167,11 +175,11 @@ fn test_swap_coldkey_announced_works() {
         // Announce the coldkey swap
         assert_ok!(SubtensorModule::announce_coldkey_swap(
             RuntimeOrigin::signed(who.clone()),
-            new_coldkey,
+            new_coldkey_hash,
         ));
         assert_eq!(
-            ColdkeySwapAnnouncements::<Test>::get(who),
-            Some((now, new_coldkey))
+            ColdkeySwapAnnouncements::<Test>::iter().collect::<Vec<_>>(),
+            vec![(who.clone(), (now, new_coldkey_hash))]
         );
 
         // Run some blocks for the announcement to be past the delay
@@ -256,7 +264,8 @@ fn test_swap_coldkey_announced_works() {
         let total_stake_before = SubtensorModule::get_total_stake();
 
         assert_ok!(SubtensorModule::swap_coldkey_announced(
-            <Test as frame_system::Config>::RuntimeOrigin::signed(who)
+            <Test as frame_system::Config>::RuntimeOrigin::signed(who),
+            new_coldkey
         ));
 
         // Ensure the announcement has been consumed
@@ -416,12 +425,12 @@ fn test_swap_coldkey_announced_with_bad_origin_fails() {
         let new_coldkey = U256::from(2);
 
         assert_noop!(
-            SubtensorModule::swap_coldkey_announced(RuntimeOrigin::none()),
+            SubtensorModule::swap_coldkey_announced(RuntimeOrigin::none(), new_coldkey),
             BadOrigin
         );
 
         assert_noop!(
-            SubtensorModule::swap_coldkey_announced(RuntimeOrigin::root()),
+            SubtensorModule::swap_coldkey_announced(RuntimeOrigin::root(), new_coldkey),
             BadOrigin
         );
     });
@@ -434,26 +443,48 @@ fn test_swap_coldkey_announced_without_announcement_fails() {
         let new_coldkey = U256::from(2);
 
         assert_noop!(
-            SubtensorModule::swap_coldkey_announced(RuntimeOrigin::signed(who)),
+            SubtensorModule::swap_coldkey_announced(RuntimeOrigin::signed(who), new_coldkey),
             Error::<Test>::ColdKeySwapAnnouncementNotFound
         );
     })
 }
 
+#[test]
+fn test_swap_coldkey_announced_with_mismatched_coldkey_hash_fails() {
+    new_test_ext(1).execute_with(|| {
+        let who = U256::from(1);
+        let new_coldkey = U256::from(2);
+        let new_coldkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey);
+        let other_coldkey = U256::from(3);
+
+        assert_ok!(SubtensorModule::announce_coldkey_swap(
+            RuntimeOrigin::signed(who.clone()),
+            new_coldkey_hash,
+        ));
+
+        assert_noop!(
+            SubtensorModule::swap_coldkey_announced(RuntimeOrigin::signed(who), other_coldkey),
+            Error::<Test>::AnnouncedColdkeyHashDoesNotMatch
+        );
+    })
+}
+
 #[test]
 fn test_swap_coldkey_announced_too_early_fails() {
     new_test_ext(1).execute_with(|| {
         let who = U256::from(1);
         let new_coldkey = U256::from(2);
+        let new_coldkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey);
 
         assert_ok!(SubtensorModule::announce_coldkey_swap(
             RuntimeOrigin::signed(who.clone()),
-            new_coldkey,
+            new_coldkey_hash,
         ));
 
         assert_noop!(
             SubtensorModule::swap_coldkey_announced(
-                <Test as frame_system::Config>::RuntimeOrigin::signed(who)
+                <Test as frame_system::Config>::RuntimeOrigin::signed(who),
+                new_coldkey
             ),
             Error::<Test>::ColdKeySwapTooEarly
         );
@@ -465,11 +496,12 @@ fn test_swap_coldkey_announced_with_already_associated_coldkey_fails() {
     new_test_ext(1).execute_with(|| {
         let who = U256::from(1);
         let new_coldkey = U256::from(2);
+        let new_coldkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey);
         let hotkey = U256::from(3);
 
         assert_ok!(SubtensorModule::announce_coldkey_swap(
             RuntimeOrigin::signed(who.clone()),
-            new_coldkey,
+            new_coldkey_hash,
         ));
 
         let now = System::block_number();
@@ -483,7 +515,8 @@ fn test_swap_coldkey_announced_with_already_associated_coldkey_fails() {
 
         assert_noop!(
             SubtensorModule::swap_coldkey_announced(
-                <Test as frame_system::Config>::RuntimeOrigin::signed(who)
+                <Test as frame_system::Config>::RuntimeOrigin::signed(who),
+                new_coldkey
             ),
             Error::<Test>::ColdKeyAlreadyAssociated
         );
@@ -496,10 +529,11 @@ fn test_swap_coldkey_announced_with_hotkey_fails() {
         let who = U256::from(1);
         let new_coldkey = U256::from(2);
         let hotkey = U256::from(3);
+        let hotkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&hotkey);
 
         assert_ok!(SubtensorModule::announce_coldkey_swap(
             RuntimeOrigin::signed(who.clone()),
-            hotkey.clone(),
+            hotkey_hash,
         ));
 
         let now = System::block_number();
@@ -513,7 +547,8 @@ fn test_swap_coldkey_announced_with_hotkey_fails() {
 
         assert_noop!(
             SubtensorModule::swap_coldkey_announced(
-                <Test as frame_system::Config>::RuntimeOrigin::signed(who)
+                <Test as frame_system::Config>::RuntimeOrigin::signed(who),
+                hotkey
             ),
             Error::<Test>::NewColdKeyIsHotkey
         );
@@ -1257,6 +1292,7 @@ fn test_subtensor_extension_rejects_any_call_that_is_not_swap_coldkey_announced(
         let netuid = NetUid::from(1);
         let who = U256::from(0);
         let new_coldkey = U256::from(1);
+        let new_coldkey_hash = <Test as frame_system::Config>::Hashing::hash_of(&new_coldkey);
         let hotkey = U256::from(2);
         let stake = DefaultMinStake::<Test>::get().to_u64();
         assert_ne!(hotkey, who);
@@ -1274,7 +1310,7 @@ fn test_subtensor_extension_rejects_any_call_that_is_not_swap_coldkey_announced(
         // Schedule the coldkey for a swap
         assert_ok!(SubtensorModule::announce_coldkey_swap(
             <Test as frame_system::Config>::RuntimeOrigin::signed(who),
-            new_coldkey,
+            new_coldkey_hash,
         ));
         assert!(ColdkeySwapAnnouncements::<Test>::contains_key(who));
 
@@ -1360,7 +1396,8 @@ fn test_subtensor_extension_rejects_any_call_that_is_not_swap_coldkey_announced(
         }
 
         // Swap coldkey announced should succeed
-        let call = RuntimeCall::SubtensorModule(SubtensorCall::swap_coldkey_announced {});
+        let call =
+            RuntimeCall::SubtensorModule(SubtensorCall::swap_coldkey_announced { new_coldkey });
         let info = call.get_dispatch_info();
         let ext = SubtensorTransactionExtension::<Test>::new();
         assert_ok!(ext.dispatch_transaction(RuntimeOrigin::signed(who).into(), call, &info, 0, 0));
@@ -1403,8 +1440,4 @@ fn test_schedule_swap_coldkey_deprecated() {
     });
 }
 
-// TEST STAKING HOTKEY ARE ADDITIVE TO THE EXISTING ONES
-
-// TEST HOTKEYS OWNERSHIP IS ADDITIVE TO THE EXISTING ONES
-
 // TEST TRANSFER ROOT CLAIM WITH NEW KEYS + ROOT CASE