-
Notifications
You must be signed in to change notification settings - Fork 302
Bugfix for: hotkey_swap on a single subnet wipes all root dividend accumulations
#2527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devnet-ready
Are you sure you want to change the base?
Changes from 6 commits
44a39d7
057fa2f
bb59c0c
34d3b61
495e90f
ce1f225
e8d7abe
872fa29
502b531
38f37c5
46e57ae
542b571
04627d6
93a61e7
b9722c7
bc77a99
a538eac
ac35533
f28e120
20a3aba
330d668
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| use super::*; | ||
| use frame_support::pallet_prelude::Weight; | ||
| use scale_info::prelude::string::String; | ||
| use substrate_fixed::types::I96F32; | ||
|
|
||
| /// Fixes the consequences of a bug in `perform_hotkey_swap_on_one_subnet` where | ||
| /// `transfer_root_claimable_for_new_hotkey` unconditionally transferred the **entire** | ||
| /// `RootClaimable` BTreeMap (all subnets) from the old hotkey to the new hotkey, even | ||
| /// during a single-subnet swap. | ||
| /// | ||
| /// This left the old hotkey with: | ||
| /// - `RootClaimable[old_hotkey]` = empty (wiped for ALL subnets) | ||
| /// - `RootClaimed[(subnet, old_hotkey, coldkey)]` = old watermarks (for non-swapped subnets) | ||
| /// | ||
| /// Resulting in `owed = claimable_rate * root_stake - root_claimed = 0 - positive = negative → 0`, | ||
| /// effectively freezing root dividends for the old hotkey. | ||
| /// | ||
| /// Remediation: for every (netuid, hotkey, coldkey) where claimed > claimable * root_stake, | ||
| /// reset claimed = claimable * root_stake so owed starts at 0 instead of being permanently | ||
| /// negative. Future epoch increments will then produce positive owed normally. | ||
| pub fn migrate_fix_root_claimed_overclaim<T: Config>() -> Weight { | ||
| let migration_name = b"migrate_fix_root_claimed_overclaim".to_vec(); | ||
| let mut weight = T::DbWeight::get().reads(1); | ||
|
|
||
| if HasMigrationRun::<T>::get(&migration_name) { | ||
| log::info!( | ||
| "Migration '{:?}' has already run. Skipping.", | ||
| String::from_utf8_lossy(&migration_name) | ||
| ); | ||
| return weight; | ||
| } | ||
|
|
||
| log::info!( | ||
| "Running migration '{}'", | ||
| String::from_utf8_lossy(&migration_name) | ||
| ); | ||
|
|
||
| // --- Fix overclaimed RootClaimed watermarks --- | ||
| let mut fixed_count: u64 = 0; | ||
| let mut total_count: u64 = 0; | ||
|
|
||
| for ((netuid, hotkey, coldkey), claimed) in RootClaimed::<T>::iter() { | ||
| total_count = total_count.saturating_add(1); | ||
| weight.saturating_accrue(T::DbWeight::get().reads(1)); | ||
|
|
||
| if claimed == 0u128 { | ||
| continue; | ||
| } | ||
|
|
||
| let root_claimable_map = RootClaimable::<T>::get(&hotkey); | ||
| weight.saturating_accrue(T::DbWeight::get().reads(1)); | ||
|
|
||
| let claimable_rate = root_claimable_map | ||
| .get(&netuid) | ||
| .copied() | ||
| .unwrap_or(I96F32::from_num(0)); | ||
|
|
||
| let root_stake = Pallet::<T>::get_stake_for_hotkey_and_coldkey_on_subnet( | ||
| &hotkey, | ||
| &coldkey, | ||
| NetUid::ROOT, | ||
| ); | ||
| weight.saturating_accrue(T::DbWeight::get().reads(1)); | ||
|
|
||
| let claimable: u128 = claimable_rate | ||
| .saturating_mul(I96F32::from_num(u64::from(root_stake))) | ||
| .saturating_to_num::<u128>(); | ||
|
|
||
| if claimed > claimable { | ||
shamil-gadelshin marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| RootClaimed::<T>::insert((&netuid, &hotkey, &coldkey), claimable); | ||
| weight.saturating_accrue(T::DbWeight::get().writes(1)); | ||
| fixed_count = fixed_count.saturating_add(1); | ||
| } | ||
| } | ||
|
Comment on lines
+48
to
+103
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be a nice macro @sam0x17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. make sure to only clear the RootClaim- able/ed maps if the root stake is still |
||
|
|
||
| // Mark migration as completed | ||
| HasMigrationRun::<T>::insert(&migration_name, true); | ||
| weight.saturating_accrue(T::DbWeight::get().writes(1)); | ||
|
|
||
| log::info!( | ||
| "Migration 'migrate_fix_root_claimed_overclaim' completed. \ | ||
| Checked {} RootClaimed entries, fixed {} overclaimed.", | ||
| total_count, | ||
| fixed_count, | ||
| ); | ||
|
|
||
| weight | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.