Fix the ability to revert hotkey swap#2487
Fix the ability to revert hotkey swap#2487evgeny-s wants to merge 25 commits intoopentensor:devnet-readyfrom
Conversation
…st: `test_swap_hotkey_registered_on_other_subnet`
# Conflicts: # pallets/subtensor/src/tests/swap_hotkey_with_subnet.rs # runtime/src/lib.rs
|
please cargo fmt the code and bump runtime version. |
# Conflicts: # runtime/src/lib.rs
evgeny-s
changed the title
ales-otf
added
the
skip-cargo-audit
| hotkey: T::AccountId, | ||
| new_hotkey: T::AccountId, | ||
| netuid: Option<NetUid>, | ||
| maybe_keep_stake: Option<bool>, |
There was a problem hiding this comment.
We need to notify @basfroman @ibraheem-abe about this because some changes may be required from their end.
There was a problem hiding this comment.
Let's create a new extrinsic for this specific reason with the keep_stake parameter. It doesn't make sense to bother the clients. WDYT?
There was a problem hiding this comment.
We can also mark the current extrinsic as deprecated, so they will have plenty of time to migrate.
| hotkey: T::AccountId, | ||
| new_hotkey: T::AccountId, | ||
| netuid: Option<NetUid>, | ||
| maybe_keep_stake: Option<bool>, |
There was a problem hiding this comment.
Are None and Some(false) supposed to have the same behavior? If yes then maybe just having a bool is sufficient?
There was a problem hiding this comment.
I don't like the fact that we add a required parameter after the optional. In this case, clients need to explicitly specify netuid: None and maybe_keep_stake: false, even if they don't want it, but they would just ignore it, since both are optional. I would keep it optional.
| weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); | ||
| } | ||
|
|
||
| // 3. Swap all subnet specific info. |
There was a problem hiding this comment.
When keep_stake is true, we only keep the top hotkey stake, right? Because we still run this in both cases:
// 4. Swap ChildKeys.
// 5. Swap ParentKeys.
// 6. Swap PendingChildKeys.
Self::parent_child_swap_hotkey(old_hotkey, new_hotkey, netuid, weight)?;
| /// is transferred to the new hotkey. | ||
| #[pallet::call_index(70)] | ||
| #[pallet::weight((Weight::from_parts(275_300_000, 0) | ||
| .saturating_add(T::DbWeight::get().reads(52_u64)) |
There was a problem hiding this comment.
Maybe we should put this in Pays::Yes
There was a problem hiding this comment.
I see we have a rate limit for this extrinsic. So, we should be fine.
4 participants
Description
This PR fixes the inability to revert a hotkey swap by disabling
is_hotkey_registered_on_any_networkfor cases where netuid is provided.More changes:
HotkeySwapOnSubnetIntervalfrom 5 days to 1 day.Parent keys,Child keys,Pending child keys,Auto stake destination,Subnet ownership,Dividend records,Voting Power,Root Reclaimable.test_swap_hotkey_registered_on_other_subnet, since it tests the condition that is no longer valid.Related Issue(s)
Type of Change
Breaking Change
If this PR introduces a breaking change, please provide a detailed description of the impact and the migration path for existing applications.
Checklist
./scripts/fix_rust.shto ensure my code is formatted and linted correctlyScreenshots (if applicable)
Please include any relevant screenshots or GIFs that demonstrate the changes made.
Additional Notes
Please provide any additional information or context that may be helpful for reviewers.