diff --git a/pallets/subtensor/src/macros/hooks.rs b/pallets/subtensor/src/macros/hooks.rs index ed0b46314b..1ef19c65c6 100644 --- a/pallets/subtensor/src/macros/hooks.rs +++ b/pallets/subtensor/src/macros/hooks.rs @@ -179,7 +179,11 @@ mod hooks { // Fix lock state left behind by subnet-scoped hotkey swaps. .saturating_add(migrations::migrate_fix_subnet_hotkey_lock_swaps::migrate_fix_subnet_hotkey_lock_swaps::()) // Populate reverse lookup index for EVM address associations. - .saturating_add(migrations::migrate_associated_evm_address_index::migrate_associated_evm_address_index::()); + .saturating_add(migrations::migrate_associated_evm_address_index::migrate_associated_evm_address_index::()) + // Finalize the lazy Alpha -> AlphaV2 / TotalHotkeyShares -> TotalHotkeySharesV2 + // migration (issue #2636): copy remaining legacy entries into the v2 maps and + // clear the legacy maps so the v1-first read fallback can be removed later. + .saturating_add(migrations::migrate_alpha_v1_to_v2::migrate_alpha_v1_to_v2::()); weight } diff --git a/pallets/subtensor/src/migrations/migrate_alpha_v1_to_v2.rs b/pallets/subtensor/src/migrations/migrate_alpha_v1_to_v2.rs new file mode 100644 index 0000000000..6c1ef14de1 --- /dev/null +++ b/pallets/subtensor/src/migrations/migrate_alpha_v1_to_v2.rs @@ -0,0 +1,120 @@ +use super::*; +use frame_support::{traits::Get, weights::Weight}; +use scale_info::prelude::string::String; +use share_pool::SafeFloat; +use substrate_fixed::types::U64F64; + +/// Finalize the lazy `Alpha` -> `AlphaV2` and `TotalHotkeyShares` -> +/// `TotalHotkeySharesV2` migration. +/// +/// # Background (issue #2636) +/// +/// The alpha share-pool was moved from the legacy `Alpha` / `TotalHotkeyShares` +/// maps (`U64F64`) to `AlphaV2` / `TotalHotkeySharesV2` (`SafeFloat`). To avoid a +/// one-shot rewrite of every staker at the v2 cutover, the move is performed +/// lazily by `HotkeyAlphaSharePoolDataOperations::set_share` / +/// `set_denominator`, which delete the legacy entry and write the v2 entry on +/// every write. Reads still consult the legacy map first and fall back to v2. +/// +/// Lazy migration only ever touches keys that are *written*. Keys that are only +/// ever read remain in the legacy maps indefinitely, so the legacy maps can never +/// be retired without an explicit finalization pass. This migration *is* that +/// pass: it copies every remaining legacy entry into the corresponding v2 map and +/// removes the legacy entry. Once it has run, the legacy maps are empty and the +/// v1-first read fallback in `HotkeyAlphaSharePoolDataOperations` becomes a dead +/// branch that a follow-up can delete, closing the "Deprecate v1 alpha share pool +/// maps after they have lazy-migrated" sub-task of #2636. +/// +/// # Safety +/// +/// The legacy and v2 maps are mutually exclusive per key: the lazy writer always +/// deletes the legacy entry before writing v2, and the legacy maps have no +/// remaining write paths (`insert` / `mutate` / `put`) outside this migration, so +/// a legacy entry is guaranteed to have no v2 counterpart and copying legacy -> +/// v2 can never overwrite a newer v2 value. As defense in depth we still skip the +/// copy when a v2 value is already present, preserving the "v2 wins on duplicate" +/// semantics of `Pallet::alpha_iter`. +/// +/// The persisted v2 value is produced with the exact same `SafeFloat::from(U64F64)` +/// conversion the read path (`get_share` / `get_denominator`) already applies on +/// every read, so this migration changes no observable value - it only relocates +/// it from the legacy key to the v2 key. +pub fn migrate_alpha_v1_to_v2() -> Weight { + let migration_name = b"migrate_alpha_v1_to_v2".to_vec(); + let mut weight = T::DbWeight::get().reads(1); + + if HasMigrationRun::::get(&migration_name) { + log::info!( + target: "runtime", + "Migration '{}' already run - skipping.", + String::from_utf8_lossy(&migration_name) + ); + return weight; + } + + log::info!(target: "runtime", "Running migration 'migrate_alpha_v1_to_v2'"); + + let mut storage_reads: u64 = 0; + let mut storage_writes: u64 = 0; + + // Collect entries up front: we mutate the very map we iterate over, so a live + // iterator cursor would be invalidated by the removals below. + let alpha_entries: Vec<((T::AccountId, T::AccountId, NetUid), U64F64)> = + Alpha::::iter().collect(); + for ((hotkey, coldkey, netuid), legacy_value) in alpha_entries { + storage_reads = storage_reads.saturating_add(1); + + // Same conversion the read path uses; SafeFloat exposes is_zero (U64F64 does not). + let migrated = SafeFloat::from(legacy_value); + if migrated.is_zero() { + // Defensive: the lazy writer removes zero entries, but clear any that + // somehow remain instead of carrying them into v2. + Alpha::::remove((&hotkey, &coldkey, netuid)); + storage_writes = storage_writes.saturating_add(1); + continue; + } + + // Defense in depth: never overwrite an existing v2 entry. Legacy and v2 + // are mutually exclusive per key, so this branch is not expected to fire, + // but if it ever did the v2 value (the newer one) must win. + if AlphaV2::::try_get((&hotkey, &coldkey, netuid)).is_err() { + storage_reads = storage_reads.saturating_add(1); + AlphaV2::::insert((&hotkey, &coldkey, netuid), migrated); + storage_writes = storage_writes.saturating_add(1); + } + + Alpha::::remove((&hotkey, &coldkey, netuid)); + storage_writes = storage_writes.saturating_add(1); + } + + let shares_entries: Vec<(T::AccountId, NetUid, U64F64)> = + TotalHotkeyShares::::iter().collect(); + for (hotkey, netuid, legacy_value) in shares_entries { + storage_reads = storage_reads.saturating_add(1); + + let migrated = SafeFloat::from(legacy_value); + if migrated.is_zero() { + TotalHotkeyShares::::remove(&hotkey, netuid); + storage_writes = storage_writes.saturating_add(1); + continue; + } + + if TotalHotkeySharesV2::::try_get(&hotkey, netuid).is_err() { + storage_reads = storage_reads.saturating_add(1); + TotalHotkeySharesV2::::insert(&hotkey, netuid, migrated); + storage_writes = storage_writes.saturating_add(1); + } + + TotalHotkeyShares::::remove(&hotkey, netuid); + storage_writes = storage_writes.saturating_add(1); + } + + weight = weight.saturating_add(T::DbWeight::get().reads_writes(storage_reads, storage_writes)); + + HasMigrationRun::::insert(&migration_name, true); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + + log::info!(target: "runtime", "Migration 'migrate_alpha_v1_to_v2' completed."); + + weight +} diff --git a/pallets/subtensor/src/migrations/mod.rs b/pallets/subtensor/src/migrations/mod.rs index 98699ccbce..a3716f59a9 100644 --- a/pallets/subtensor/src/migrations/mod.rs +++ b/pallets/subtensor/src/migrations/mod.rs @@ -4,6 +4,7 @@ use frame_support::pallet_prelude::Weight; use sp_io::KillStorageResult; use sp_io::hashing::twox_128; use sp_io::storage::clear_prefix; +pub mod migrate_alpha_v1_to_v2; pub mod migrate_associated_evm_address_index; pub mod migrate_auto_stake_destination; pub mod migrate_cleanup_swap_v3; diff --git a/pallets/subtensor/src/tests/migration.rs b/pallets/subtensor/src/tests/migration.rs index 133934c93f..262d684ed5 100644 --- a/pallets/subtensor/src/tests/migration.rs +++ b/pallets/subtensor/src/tests/migration.rs @@ -28,6 +28,7 @@ use frame_system::Config; use pallet_drand::types::RoundNumber; use pallet_scheduler::ScheduledOf; use scale_info::prelude::collections::VecDeque; +use share_pool::SafeFloat; use sp_core::{H160, H256, U256, crypto::Ss58Codec}; use sp_io::hashing::twox_128; use sp_runtime::{ @@ -5067,3 +5068,124 @@ fn test_migrate_dynamic_tempo_idempotent() { ); }); } + +#[test] +fn test_migrate_alpha_v1_to_v2_finalizes_legacy_maps_and_preserves_values() { + new_test_ext(1).execute_with(|| { + const MIGRATION_NAME: &[u8] = b"migrate_alpha_v1_to_v2"; + HasMigrationRun::::remove(MIGRATION_NAME.to_vec()); + + let hot_a = U256::from(1); + let cold_a = U256::from(2); + let hot_b = U256::from(3); + let cold_b = U256::from(4); + let netuid = NetUid::from(7); + + // Legacy Alpha (v1): two non-zero entries plus a zero entry. + let legacy_alpha_value = U64F64::from(123_456_u64); + Alpha::::insert((hot_a, cold_a, netuid), legacy_alpha_value); + Alpha::::insert((hot_b, cold_b, netuid), U64F64::from(7_u64)); + Alpha::::insert((hot_b, cold_a, netuid), U64F64::from(0_u64)); + + // Legacy TotalHotkeyShares (v1). + let legacy_shares_value = U64F64::from(9_876_u64); + TotalHotkeyShares::::insert(hot_a, netuid, legacy_shares_value); + + // Defense-in-depth probe: a colliding v2 entry already exists. The lazy + // invariant says this never happens on-chain, but if it did the migration + // must keep the (newer) v2 value. + let pre_existing_v2 = SafeFloat::from(999_u64); + AlphaV2::::insert((hot_b, cold_b, netuid), pre_existing_v2.clone()); + + // Capture the merged read-path view before migration (merges v1 + v2, v2 wins). + let before: BTreeMap<_, SafeFloat> = SubtensorModule::alpha_iter().collect(); + + // Run the migration. + let _ = migrations::migrate_alpha_v1_to_v2::migrate_alpha_v1_to_v2::(); + + assert!( + HasMigrationRun::::get(MIGRATION_NAME.to_vec()), + "migration should be marked as completed" + ); + + // Legacy maps are fully drained. + assert_eq!( + Alpha::::iter().count(), + 0, + "legacy Alpha map must be empty" + ); + assert_eq!( + TotalHotkeyShares::::iter().count(), + 0, + "legacy TotalHotkeyShares map must be empty" + ); + + // Non-zero legacy values are relocated byte-for-byte (same conversion the + // read path already uses). + assert_eq!( + AlphaV2::::get((hot_a, cold_a, netuid)), + SafeFloat::from(legacy_alpha_value) + ); + assert_eq!( + TotalHotkeySharesV2::::get(hot_a, netuid), + SafeFloat::from(legacy_shares_value) + ); + + // The zero legacy entry was dropped, not carried into v2. + assert!( + AlphaV2::::get((hot_b, cold_a, netuid)).is_zero(), + "zero legacy entry must not be carried into v2" + ); + + // Defense in depth: the pre-existing v2 value wins over the colliding + // legacy value (7), matching the v2-wins semantics of alpha_iter. + assert_eq!( + AlphaV2::::get((hot_b, cold_b, netuid)), + pre_existing_v2 + ); + + // The merged read path is unchanged by the migration (behavior preservation). + let after: BTreeMap<_, SafeFloat> = SubtensorModule::alpha_iter().collect(); + assert_eq!( + before.get(&(hot_a, cold_a, netuid)), + after.get(&(hot_a, cold_a, netuid)), + "read-path value for the migrated key must be unchanged" + ); + assert_eq!( + before.get(&(hot_b, cold_b, netuid)), + after.get(&(hot_b, cold_b, netuid)), + "colliding key read-path value must be unchanged" + ); + + // Idempotency: a second run is a no-op. + let v2_count = AlphaV2::::iter().count(); + let _ = migrations::migrate_alpha_v1_to_v2::migrate_alpha_v1_to_v2::(); + assert_eq!( + AlphaV2::::iter().count(), + v2_count, + "second run must not mutate" + ); + }); +} + +#[test] +fn test_migrate_alpha_v1_to_v2_is_a_noop_on_empty_legacy_maps() { + new_test_ext(1).execute_with(|| { + const MIGRATION_NAME: &[u8] = b"migrate_alpha_v1_to_v2"; + HasMigrationRun::::remove(MIGRATION_NAME.to_vec()); + + // No legacy data at all (fresh state / fully lazy-migrated steady state). + assert_eq!(Alpha::::iter().count(), 0); + assert_eq!(TotalHotkeyShares::::iter().count(), 0); + + let _ = migrations::migrate_alpha_v1_to_v2::migrate_alpha_v1_to_v2::(); + + assert!(HasMigrationRun::::get(MIGRATION_NAME.to_vec())); + assert_eq!( + AlphaV2::::iter().count(), + 0, + "must not synthesize any v2 entries" + ); + assert_eq!(TotalHotkeySharesV2::::iter().count(), 0); + }); +} diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 48106b0fdf..017a71a855 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -235,7 +235,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { // `spec_version`, and `authoring_version` are the same between Wasm and native. // This value is set to 100 to notify Polkadot-JS App (https://polkadot.js.org/apps) to use // the compatible custom types. - spec_version: 428, + spec_version: 429, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 1,