From 56af0687d3616c1e8d860b6b6ab95002bdcafed1 Mon Sep 17 00:00:00 2001 From: Loom Agent Date: Sun, 12 Jul 2026 21:08:22 +0000 Subject: [PATCH] feat(subtensor): finalize lazy alpha v1->v2 share-pool migration Add the `migrate_alpha_v1_to_v2` runtime migration that completes the lazy `Alpha` -> `AlphaV2` and `TotalHotkeyShares` -> `TotalHotkeySharesV2` share-pool migration, advancing issue #2636 ("Deprecate v1 alpha share pool maps after they have lazy-migrated"). Lazy migration only relocates keys that are written, so read-only keys remain in the legacy maps indefinitely and 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 using the same `SafeFloat::from(U64F64)` conversion the read path already applies, then removes the legacy entry. Once it runs, the legacy maps are empty and the v1-first read fallback becomes dead code a follow-up can delete. The legacy and v2 maps are mutually exclusive per key (the lazy writer deletes the legacy entry on every v2 write, and no write paths remain for the legacy maps), so the copy cannot overwrite a newer v2 value; a defense-in-depth check skips the copy when v2 already holds the key. The migration is idempotent via `HasMigrationRun`. Wired into the subtensor pallet `on_runtime_upgrade` hook; spec_version bumped 428 -> 429. --- pallets/subtensor/src/macros/hooks.rs | 6 +- .../src/migrations/migrate_alpha_v1_to_v2.rs | 120 +++++++++++++++++ pallets/subtensor/src/migrations/mod.rs | 1 + pallets/subtensor/src/tests/migration.rs | 122 ++++++++++++++++++ runtime/src/lib.rs | 2 +- 5 files changed, 249 insertions(+), 2 deletions(-) create mode 100644 pallets/subtensor/src/migrations/migrate_alpha_v1_to_v2.rs 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,