Finalize lazy alpha v1->v2 share-pool migration

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::<T>())
                 // Populate reverse lookup index for EVM address associations.
-                .saturating_add(migrations::migrate_associated_evm_address_index::migrate_associated_evm_address_index::<T>());
+                .saturating_add(migrations::migrate_associated_evm_address_index::migrate_associated_evm_address_index::<T>())
+                // 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::<T>());
             weight
         }
 

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<T: Config>() -> Weight {
+    let migration_name = b"migrate_alpha_v1_to_v2".to_vec();
+    let mut weight = T::DbWeight::get().reads(1);
+
+    if HasMigrationRun::<T>::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::<T>::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::<T>::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::<T>::try_get((&hotkey, &coldkey, netuid)).is_err() {
+            storage_reads = storage_reads.saturating_add(1);
+            AlphaV2::<T>::insert((&hotkey, &coldkey, netuid), migrated);
+            storage_writes = storage_writes.saturating_add(1);
+        }
+
+        Alpha::<T>::remove((&hotkey, &coldkey, netuid));
+        storage_writes = storage_writes.saturating_add(1);
+    }
+
+    let shares_entries: Vec<(T::AccountId, NetUid, U64F64)> =
+        TotalHotkeyShares::<T>::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::<T>::remove(&hotkey, netuid);
+            storage_writes = storage_writes.saturating_add(1);
+            continue;
+        }
+
+        if TotalHotkeySharesV2::<T>::try_get(&hotkey, netuid).is_err() {
+            storage_reads = storage_reads.saturating_add(1);
+            TotalHotkeySharesV2::<T>::insert(&hotkey, netuid, migrated);
+            storage_writes = storage_writes.saturating_add(1);
+        }
+
+        TotalHotkeyShares::<T>::remove(&hotkey, netuid);
+        storage_writes = storage_writes.saturating_add(1);
+    }
+
+    weight = weight.saturating_add(T::DbWeight::get().reads_writes(storage_reads, storage_writes));
+
+    HasMigrationRun::<T>::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
+}

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;

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::<Test>::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::<Test>::insert((hot_a, cold_a, netuid), legacy_alpha_value);
+        Alpha::<Test>::insert((hot_b, cold_b, netuid), U64F64::from(7_u64));
+        Alpha::<Test>::insert((hot_b, cold_a, netuid), U64F64::from(0_u64));
+
+        // Legacy TotalHotkeyShares (v1).
+        let legacy_shares_value = U64F64::from(9_876_u64);
+        TotalHotkeyShares::<Test>::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::<Test>::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::<Test>();
+
+        assert!(
+            HasMigrationRun::<Test>::get(MIGRATION_NAME.to_vec()),
+            "migration should be marked as completed"
+        );
+
+        // Legacy maps are fully drained.
+        assert_eq!(
+            Alpha::<Test>::iter().count(),
+            0,
+            "legacy Alpha map must be empty"
+        );
+        assert_eq!(
+            TotalHotkeyShares::<Test>::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::<Test>::get((hot_a, cold_a, netuid)),
+            SafeFloat::from(legacy_alpha_value)
+        );
+        assert_eq!(
+            TotalHotkeySharesV2::<Test>::get(hot_a, netuid),
+            SafeFloat::from(legacy_shares_value)
+        );
+
+        // The zero legacy entry was dropped, not carried into v2.
+        assert!(
+            AlphaV2::<Test>::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::<Test>::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::<Test>::iter().count();
+        let _ = migrations::migrate_alpha_v1_to_v2::migrate_alpha_v1_to_v2::<Test>();
+        assert_eq!(
+            AlphaV2::<Test>::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::<Test>::remove(MIGRATION_NAME.to_vec());
+
+        // No legacy data at all (fresh state / fully lazy-migrated steady state).
+        assert_eq!(Alpha::<Test>::iter().count(), 0);
+        assert_eq!(TotalHotkeyShares::<Test>::iter().count(), 0);
+
+        let _ = migrations::migrate_alpha_v1_to_v2::migrate_alpha_v1_to_v2::<Test>();
+
+        assert!(HasMigrationRun::<Test>::get(MIGRATION_NAME.to_vec()));
+        assert_eq!(
+            AlphaV2::<Test>::iter().count(),
+            0,
+            "must not synthesize any v2 entries"
+        );
+        assert_eq!(TotalHotkeySharesV2::<Test>::iter().count(), 0);
+    });
+}

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,