diff --git a/evm-tests/test/neuron.precompile.reveal-weights.test.ts b/evm-tests/test/neuron.precompile.reveal-weights.test.ts index 8045ac18f1..4ac63468db 100644 --- a/evm-tests/test/neuron.precompile.reveal-weights.test.ts +++ b/evm-tests/test/neuron.precompile.reveal-weights.test.ts @@ -11,9 +11,9 @@ import { generateRandomEthersWallet } from "../src/utils" import { convertH160ToPublicKey } from "../src/address-utils" import { blake2AsU8a } from "@polkadot/util-crypto" import { - forceSetBalanceToEthAddress, forceSetBalanceToSs58Address, addNewSubnetwork, setCommitRevealWeightsEnabled, setWeightsSetRateLimit, burnedRegister, + forceSetBalanceToEthAddress, forceSetBalanceToSs58Address, addNewSubnetwork, setWeightsSetRateLimit, burnedRegister, setTempo, setCommitRevealWeightsInterval, - startCall + startCall, } from "../src/subtensor" // hardcode some values for reveal hash @@ -52,6 +52,7 @@ describe("Test neuron precompile reveal weights", () => { const coldkey = getRandomSubstrateKeypair(); let api: TypedApi + let commitEpoch: number; // sudo account alice as signer let alice: PolkadotSigner; @@ -65,13 +66,11 @@ describe("Test neuron precompile reveal weights", () => { await forceSetBalanceToSs58Address(api, convertPublicKeyToSs58(coldkey.publicKey)) await forceSetBalanceToEthAddress(api, wallet.address) let netuid = await addNewSubnetwork(api, hotkey, coldkey) + // await disableCommitRevealWeights(api, netuid) await startCall(api, netuid, coldkey) console.log("test the case on subnet ", netuid) - // enable commit reveal feature - await setCommitRevealWeightsEnabled(api, netuid, true) - // set it as 0, we can set the weight anytime await setWeightsSetRateLimit(api, netuid, BigInt(0)) const ss58Address = convertH160ToSS58(wallet.address) @@ -90,8 +89,15 @@ describe("Test neuron precompile reveal weights", () => { const subnetId = totalNetworks - 1 const commitHash = getCommitHash(subnetId, wallet.address) const contract = new ethers.Contract(INEURON_ADDRESS, INeuronABI, wallet); - const tx = await contract.commitWeights(subnetId, commitHash) - await tx.wait() + try { + const tx = await contract.commitWeights(subnetId, commitHash) + await tx.wait() + } catch (e) { + console.log("commitWeights failed", e) + } + + const commitBlock = await api.query.System.Number.getValue() + commitEpoch = Math.trunc(commitBlock / (100 + 1)) const ss58Address = convertH160ToSS58(wallet.address) @@ -108,9 +114,19 @@ describe("Test neuron precompile reveal weights", () => { const netuid = totalNetworks - 1 const contract = new ethers.Contract(INEURON_ADDRESS, INeuronABI, wallet); // set tempo or epoch large, then enough time to reveal weight - await setTempo(api, netuid, 60000) - // set interval epoch as 0, we can reveal at the same epoch - await setCommitRevealWeightsInterval(api, netuid, BigInt(0)) + await setTempo(api, netuid, 100) + // set interval epoch as 1, it is the minimum value now + await setCommitRevealWeightsInterval(api, netuid, BigInt(1)) + + while (true) { + const currentBlock = await api.query.System.Number.getValue() + const currentEpoch = Math.trunc(currentBlock / (100 + 1)) + // wait for one second for fast blocks + if (currentEpoch > commitEpoch) { + break + } + await new Promise(resolve => setTimeout(resolve, 1000)) + } const tx = await contract.revealWeights( netuid, @@ -120,6 +136,7 @@ describe("Test neuron precompile reveal weights", () => { version_key ); await tx.wait() + const ss58Address = convertH160ToSS58(wallet.address) // check the weight commit is removed after reveal successfully diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 4334a1f8be..03a20a5f21 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -107,8 +107,6 @@ pub mod pallet { BondsMovingAverageMaxReached, /// Only root can set negative sigmoid steepness values NegativeSigmoidSteepness, - /// Reveal Peroid is not within the valid range. - RevealPeriodOutOfBounds, } /// Enum for specifying the type of precompile operation. #[derive( @@ -1311,14 +1309,10 @@ pub mod pallet { Error::::SubnetDoesNotExist ); - const MAX_COMMIT_REVEAL_PEROIDS: u64 = 100; - ensure!( - interval <= MAX_COMMIT_REVEAL_PEROIDS, - Error::::RevealPeriodOutOfBounds - ); - - pallet_subtensor::Pallet::::set_reveal_period(netuid, interval); log::debug!("SetWeightCommitInterval( netuid: {netuid:?}, interval: {interval:?} ) "); + + pallet_subtensor::Pallet::::set_reveal_period(netuid, interval)?; + Ok(()) } diff --git a/pallets/admin-utils/src/tests/mod.rs b/pallets/admin-utils/src/tests/mod.rs index 754befc805..5290d3ddfc 100644 --- a/pallets/admin-utils/src/tests/mod.rs +++ b/pallets/admin-utils/src/tests/mod.rs @@ -1120,7 +1120,7 @@ fn test_sudo_set_commit_reveal_weights_enabled() { let netuid = NetUid::from(1); add_network(netuid, 10); - let to_be_set: bool = true; + let to_be_set: bool = false; let init_value: bool = SubtensorModule::get_commit_reveal_weights_enabled(netuid); assert_ok!(AdminUtils::sudo_set_commit_reveal_weights_enabled( @@ -1459,7 +1459,7 @@ fn sudo_set_commit_reveal_weights_interval() { netuid, too_high ), - Error::::RevealPeriodOutOfBounds + pallet_subtensor::Error::::RevealPeriodTooLarge ); let to_be_set = 55; diff --git a/pallets/subtensor/src/benchmarks.rs b/pallets/subtensor/src/benchmarks.rs index 6a2a35e3c0..5cdb47685d 100644 --- a/pallets/subtensor/src/benchmarks.rs +++ b/pallets/subtensor/src/benchmarks.rs @@ -60,6 +60,7 @@ mod pallet_benchmarks { Subtensor::::set_network_registration_allowed(netuid, true); Subtensor::::set_max_registrations_per_block(netuid, 4096); Subtensor::::set_target_registrations_per_interval(netuid, 4096); + Subtensor::::set_commit_reveal_weights_enabled(netuid, false); let mut seed: u32 = 1; let mut dests = Vec::new(); diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 173d9bd0f4..ef41b07c78 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -106,6 +106,11 @@ pub mod pallet { /// Minimum balance required to perform a coldkey swap pub const MIN_BALANCE_TO_PERFORM_COLDKEY_SWAP: TaoCurrency = TaoCurrency::new(100_000_000); // 0.1 TAO in RAO + /// Minimum commit reveal periods + pub const MIN_COMMIT_REVEAL_PEROIDS: u64 = 1; + /// Maximum commit reveal periods + pub const MAX_COMMIT_REVEAL_PEROIDS: u64 = 100; + #[pallet::pallet] #[pallet::without_storage_info] #[pallet::storage_version(STORAGE_VERSION)] @@ -761,7 +766,7 @@ pub mod pallet { #[pallet::type_value] /// Default value for weight commit/reveal enabled. pub fn DefaultCommitRevealWeightsEnabled() -> bool { - false + true } #[pallet::type_value] /// Default value for weight commit/reveal version. diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index e3cc7eb74b..e6d9c231d1 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -244,6 +244,10 @@ mod errors { SymbolAlreadyInUse, /// Incorrect commit-reveal version. IncorrectCommitRevealVersion, + /// Reveal period is too large. + RevealPeriodTooLarge, + /// Reveal period is too small. + RevealPeriodTooSmall, /// Generic error for out-of-range parameter value InvalidValue, } diff --git a/pallets/subtensor/src/macros/hooks.rs b/pallets/subtensor/src/macros/hooks.rs index bec3155f2a..4bf7a6b115 100644 --- a/pallets/subtensor/src/macros/hooks.rs +++ b/pallets/subtensor/src/macros/hooks.rs @@ -130,6 +130,8 @@ mod hooks { .saturating_add(migrations::migrate_subnet_symbols::migrate_subnet_symbols::()) // Migrate CRV3 add commit_block .saturating_add(migrations::migrate_crv3_commits_add_block::migrate_crv3_commits_add_block::()) + // Migrate Commit-Reveal Settings + .saturating_add(migrations::migrate_commit_reveal_settings::migrate_commit_reveal_settings::()) //Migrate CRV3 to TimelockedCommits .saturating_add(migrations::migrate_crv3_v2_to_timelocked::migrate_crv3_v2_to_timelocked::()) // Migrate to fix root counters diff --git a/pallets/subtensor/src/migrations/migrate_commit_reveal_settings.rs b/pallets/subtensor/src/migrations/migrate_commit_reveal_settings.rs new file mode 100644 index 0000000000..54df469600 --- /dev/null +++ b/pallets/subtensor/src/migrations/migrate_commit_reveal_settings.rs @@ -0,0 +1,64 @@ +use alloc::string::String; + +use crate::MIN_COMMIT_REVEAL_PEROIDS; +use frame_support::IterableStorageMap; +use frame_support::{traits::Get, weights::Weight}; +use subtensor_runtime_common::NetUid; + +use super::*; + +pub fn migrate_commit_reveal_settings() -> Weight { + let migration_name = b"migrate_commit_reveal_settings".to_vec(); + + // Initialize the weight with one read operation. + let mut weight = T::DbWeight::get().reads(1); + + // Check if the migration has already run + if HasMigrationRun::::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) + ); + + let netuids: Vec = as IterableStorageMap>::iter() + .map(|(netuid, _)| netuid) + .collect(); + weight = weight.saturating_add( + T::DbWeight::get() + .reads(netuids.len() as u64) + .saturating_mul(2), + ); + + for netuid in netuids.iter() { + if netuid.is_root() { + continue; + } + if !CommitRevealWeightsEnabled::::get(*netuid) { + CommitRevealWeightsEnabled::::insert(*netuid, true); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + } + + if RevealPeriodEpochs::::get(*netuid) == 0 { + RevealPeriodEpochs::::insert(*netuid, MIN_COMMIT_REVEAL_PEROIDS); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + } + } + + // Mark the migration as completed + HasMigrationRun::::insert(&migration_name, true); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + + log::info!( + "Migration '{:?}' completed.", + String::from_utf8_lossy(&migration_name) + ); + + // Return the migration weight. + weight +} diff --git a/pallets/subtensor/src/migrations/mod.rs b/pallets/subtensor/src/migrations/mod.rs index c2109eb9e6..65dda18125 100644 --- a/pallets/subtensor/src/migrations/mod.rs +++ b/pallets/subtensor/src/migrations/mod.rs @@ -6,6 +6,7 @@ use sp_io::hashing::twox_128; use sp_io::storage::clear_prefix; pub mod migrate_chain_identity; pub mod migrate_coldkey_swap_scheduled; +pub mod migrate_commit_reveal_settings; pub mod migrate_commit_reveal_v2; pub mod migrate_create_root_network; pub mod migrate_crv3_commits_add_block; diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 7d49e0d40a..737329ccb7 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -1,6 +1,8 @@ use super::*; use crate::epoch::math::*; +use crate::{Error, MAX_COMMIT_REVEAL_PEROIDS, MIN_COMMIT_REVEAL_PEROIDS}; use codec::Compact; +use frame_support::dispatch::DispatchResult; use safe_math::*; use sp_core::{ConstU32, H256}; use sp_runtime::{ @@ -9,7 +11,6 @@ use sp_runtime::{ }; use sp_std::{collections::vec_deque::VecDeque, vec}; use subtensor_runtime_common::NetUid; - impl Pallet { /// ---- The implementation for committing weight hashes. /// @@ -1062,9 +1063,21 @@ impl Pallet { (first_reveal_block, last_reveal_block) } - pub fn set_reveal_period(netuid: NetUid, reveal_period: u64) { + pub fn set_reveal_period(netuid: NetUid, reveal_period: u64) -> DispatchResult { + ensure!( + reveal_period <= MAX_COMMIT_REVEAL_PEROIDS, + Error::::RevealPeriodTooLarge + ); + + ensure!( + reveal_period >= MIN_COMMIT_REVEAL_PEROIDS, + Error::::RevealPeriodTooSmall + ); + RevealPeriodEpochs::::insert(netuid, reveal_period); + Self::deposit_event(Event::CommitRevealPeriodsSet(netuid, reveal_period)); + Ok(()) } pub fn get_reveal_period(netuid: NetUid) -> u64 { RevealPeriodEpochs::::get(netuid) diff --git a/pallets/subtensor/src/tests/children.rs b/pallets/subtensor/src/tests/children.rs index 41d25c8aea..2ffc21ea1d 100644 --- a/pallets/subtensor/src/tests/children.rs +++ b/pallets/subtensor/src/tests/children.rs @@ -2637,7 +2637,8 @@ fn test_childkey_set_weights_single_parent() { new_test_ext(1).execute_with(|| { let subnet_owner_coldkey = U256::from(1001); let subnet_owner_hotkey = U256::from(1002); - let netuid = add_dynamic_network(&subnet_owner_hotkey, &subnet_owner_coldkey); + let netuid = + add_dynamic_network_disable_commit_reveal(&subnet_owner_hotkey, &subnet_owner_coldkey); Tempo::::insert(netuid, 1); // Define hotkeys @@ -2746,7 +2747,8 @@ fn test_set_weights_no_parent() { new_test_ext(1).execute_with(|| { let subnet_owner_coldkey = U256::from(1001); let subnet_owner_hotkey = U256::from(1002); - let netuid = add_dynamic_network(&subnet_owner_hotkey, &subnet_owner_coldkey); + let netuid = + add_dynamic_network_disable_commit_reveal(&subnet_owner_hotkey, &subnet_owner_coldkey); let hotkey: U256 = U256::from(2); let spare_hk: U256 = U256::from(3); @@ -3568,7 +3570,7 @@ fn test_dividend_distribution_with_children() { fn test_dynamic_parent_child_relationships() { new_test_ext(1).execute_with(|| { let netuid = NetUid::from(1); - add_network(netuid, 1, 0); + add_network_disable_commit_reveal(netuid, 1, 0); // Define hotkeys and coldkeys let parent = U256::from(1); diff --git a/pallets/subtensor/src/tests/coinbase.rs b/pallets/subtensor/src/tests/coinbase.rs index a660f1b815..927f98e2fa 100644 --- a/pallets/subtensor/src/tests/coinbase.rs +++ b/pallets/subtensor/src/tests/coinbase.rs @@ -2444,7 +2444,7 @@ fn test_drain_pending_emission_no_miners_all_drained() { #[test] fn test_drain_pending_emission_zero_emission() { new_test_ext(1).execute_with(|| { - let netuid = add_dynamic_network(&U256::from(1), &U256::from(2)); + let netuid = add_dynamic_network_disable_commit_reveal(&U256::from(1), &U256::from(2)); let hotkey = U256::from(3); let coldkey = U256::from(4); let miner_hk = U256::from(5); @@ -2522,6 +2522,7 @@ fn test_run_coinbase_not_started() { let sn_owner_ck = U256::from(8); add_network_without_emission_block(netuid, tempo, 0); + SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); assert_eq!(FirstEmissionBlockNumber::::get(netuid), None); SubnetOwner::::insert(netuid, sn_owner_ck); @@ -2610,6 +2611,7 @@ fn test_run_coinbase_not_started_start_after() { let sn_owner_ck = U256::from(8); add_network_without_emission_block(netuid, tempo, 0); + SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); assert_eq!(FirstEmissionBlockNumber::::get(netuid), None); SubnetOwner::::insert(netuid, sn_owner_ck); diff --git a/pallets/subtensor/src/tests/epoch.rs b/pallets/subtensor/src/tests/epoch.rs index bdf675648b..25b4c48781 100644 --- a/pallets/subtensor/src/tests/epoch.rs +++ b/pallets/subtensor/src/tests/epoch.rs @@ -159,7 +159,7 @@ fn init_run_epochs( bonds_penalty: u16, ) { // === Create the network - add_network(netuid, u16::MAX - 1, 0); // set higher tempo to avoid built-in epoch, then manual epoch instead + add_network_disable_commit_reveal(netuid, u16::MAX - 1, 0); // set higher tempo to avoid built-in epoch, then manual epoch instead // === Set bonds penalty SubtensorModule::set_bonds_penalty(netuid, bonds_penalty); @@ -560,7 +560,7 @@ fn test_1_graph() { let hotkey = U256::from(0); let uid: u16 = 0; let stake_amount: u64 = 1_000_000_000; - add_network(netuid, u16::MAX - 1, 0); // set higher tempo to avoid built-in epoch, then manual epoch instead + add_network_disable_commit_reveal(netuid, u16::MAX - 1, 0); // set higher tempo to avoid built-in epoch, then manual epoch instead SubtensorModule::set_max_allowed_uids(netuid, 1); SubtensorModule::add_balance_to_coldkey_account( &coldkey, @@ -630,7 +630,7 @@ fn test_10_graph() { // each with 1 stake and self weights. let n: usize = 10; let netuid = NetUid::from(1); - add_network(netuid, u16::MAX - 1, 0); // set higher tempo to avoid built-in epoch, then manual epoch instead + add_network_disable_commit_reveal(netuid, u16::MAX - 1, 0); // set higher tempo to avoid built-in epoch, then manual epoch instead SubtensorModule::set_max_allowed_uids(netuid, n as u16); for i in 0..10 { add_node(netuid, U256::from(i), U256::from(i), i as u16, 1) @@ -1004,7 +1004,7 @@ fn test_bonds() { let max_stake: u64 = 4; let stakes: Vec = vec![1, 2, 3, 4, 0, 0, 0, 0]; let block_number = System::block_number(); - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); SubtensorModule::set_max_allowed_uids( netuid, n ); assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), n); SubtensorModule::set_max_registrations_per_block( netuid, n ); @@ -1351,7 +1351,7 @@ fn test_active_stake() { let tempo: u16 = 1; let block_number: u64 = System::block_number(); let stake: u64 = 1; - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); SubtensorModule::set_max_allowed_uids(netuid, n); assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), n); SubtensorModule::set_max_registrations_per_block(netuid, n); @@ -1567,7 +1567,7 @@ fn test_outdated_weights() { let tempo: u16 = 0; let mut block_number: u64 = System::block_number(); let stake: u64 = 1; - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); SubtensorModule::set_max_allowed_uids(netuid, n); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_max_registrations_per_block(netuid, n); @@ -1757,7 +1757,7 @@ fn test_zero_weights() { let tempo: u16 = u16::MAX - 1; // high tempo to skip automatic epochs in on_initialize, use manual epochs instead let mut block_number: u64 = 0; let stake: u64 = 1; - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); SubtensorModule::set_max_allowed_uids(netuid, n); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_max_registrations_per_block(netuid, n); @@ -1960,7 +1960,7 @@ fn test_deregistered_miner_bonds() { let high_tempo: u16 = u16::MAX - 1; // high tempo to skip automatic epochs in on_initialize, use manual epochs instead let stake: u64 = 1; - add_network(netuid, high_tempo, 0); + add_network_disable_commit_reveal(netuid, high_tempo, 0); SubtensorModule::set_max_allowed_uids(netuid, n); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_max_registrations_per_block(netuid, n); @@ -2669,7 +2669,7 @@ pub fn assert_approx_eq(left: I32F32, right: I32F32, epsilon: I32F32) { fn setup_yuma_3_scenario(netuid: NetUid, n: u16, sparse: bool, max_stake: u64, stakes: Vec) { let block_number = System::block_number(); let tempo: u16 = 1; // high tempo to skip automatic epochs in on_initialize, use manual epochs instead - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); SubtensorModule::set_max_allowed_uids(netuid, n); assert_eq!(SubtensorModule::get_max_allowed_uids(netuid), n); @@ -3567,7 +3567,7 @@ fn test_epoch_masks_incoming_to_sniped_uid_prevents_inheritance() { let reveal: u64 = 2; add_network(netuid, tempo, 0); - SubtensorModule::set_reveal_period(netuid, reveal); + assert_ok!(SubtensorModule::set_reveal_period(netuid, reveal)); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); SubtensorModule::set_max_allowed_uids(netuid, 3); SubtensorModule::set_target_registrations_per_interval(netuid, u16::MAX); @@ -3709,7 +3709,7 @@ fn test_epoch_does_not_mask_outside_window_but_masks_inside() { let reveal: u16 = 2; add_network(netuid, tempo, 0); - SubtensorModule::set_reveal_period(netuid, reveal as u64); + assert_ok!(SubtensorModule::set_reveal_period(netuid, reveal as u64)); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); SubtensorModule::set_target_registrations_per_interval(netuid, u16::MAX); diff --git a/pallets/subtensor/src/tests/migration.rs b/pallets/subtensor/src/tests/migration.rs index fbe8825a5f..e93aab7669 100644 --- a/pallets/subtensor/src/tests/migration.rs +++ b/pallets/subtensor/src/tests/migration.rs @@ -1172,6 +1172,129 @@ fn test_migrate_disable_commit_reveal() { }); } +#[test] +fn test_migrate_commit_reveal_settings() { + new_test_ext(1).execute_with(|| { + const MIGRATION_NAME: &str = "migrate_commit_reveal_settings"; + + // Set up some networks first + let netuid1: u16 = 1; + let netuid2: u16 = 2; + // Add networks to simulate existing networks + add_network(netuid1.into(), 1, 0); + add_network(netuid2.into(), 1, 0); + + // Ensure the storage items use default values initially (but aren't explicitly set) + // Since these are ValueQuery storage items, they return defaults even when not set + assert_eq!(RevealPeriodEpochs::::get(NetUid::from(netuid1)), 1u64); + assert_eq!(RevealPeriodEpochs::::get(NetUid::from(netuid2)), 1u64); + assert!(CommitRevealWeightsEnabled::::get(NetUid::from(netuid1))); + assert!(CommitRevealWeightsEnabled::::get(NetUid::from(netuid2))); + + // Check migration hasn't run + assert!(!HasMigrationRun::::get(MIGRATION_NAME.as_bytes().to_vec())); + + // Run migration + let weight = crate::migrations::migrate_commit_reveal_settings::migrate_commit_reveal_settings::(); + + // Check migration has been marked as run + assert!(HasMigrationRun::::get(MIGRATION_NAME.as_bytes().to_vec())); + + // Verify RevealPeriodEpochs was set correctly + assert_eq!(RevealPeriodEpochs::::get(NetUid::from(netuid1)), 1u64); + assert_eq!(RevealPeriodEpochs::::get(NetUid::from(netuid2)), 1u64); + + // Verify CommitRevealWeightsEnabled was set correctly + assert!(CommitRevealWeightsEnabled::::get(NetUid::from(netuid1))); + assert!(CommitRevealWeightsEnabled::::get(NetUid::from(netuid2))); + }); +} + +#[test] +fn test_migrate_commit_reveal_settings_already_run() { + new_test_ext(1).execute_with(|| { + const MIGRATION_NAME: &str = "migrate_commit_reveal_settings"; + // Mark migration as already run + HasMigrationRun::::insert(MIGRATION_NAME.as_bytes().to_vec(), true); + + // Run migration + let weight = crate::migrations::migrate_commit_reveal_settings::migrate_commit_reveal_settings::(); + + // Should only have read weight for checking migration status + let expected_weight = ::DbWeight::get().reads(1); + assert_eq!(weight, expected_weight); + }); +} + +#[test] +fn test_migrate_commit_reveal_settings_no_networks() { + new_test_ext(1).execute_with(|| { + const MIGRATION_NAME: &str = "migrate_commit_reveal_settings"; + + // Check migration hasn't run + assert!(!HasMigrationRun::::get(MIGRATION_NAME.as_bytes().to_vec())); + + // Run migration + let weight = crate::migrations::migrate_commit_reveal_settings::migrate_commit_reveal_settings::(); + + // Check migration has been marked as run + assert!(HasMigrationRun::::get(MIGRATION_NAME.as_bytes().to_vec())); + + // Check that weight calculation is correct (no networks, so no additional reads/writes) + // 1 read for migration check + 0 reads for networks + 0 writes for storage + 1 write for migration flag + let expected_weight = ::DbWeight::get().reads(1) + ::DbWeight::get().writes(1); + assert_eq!(weight, expected_weight); + }); +} + +#[test] +fn test_migrate_commit_reveal_settings_multiple_networks() { + new_test_ext(1).execute_with(|| { + const MIGRATION_NAME: &str = "migrate_commit_reveal_settings"; + + // Set up multiple networks + let netuids = vec![1u16, 2u16, 3u16, 10u16, 42u16]; + for netuid in &netuids { + add_network((*netuid).into(), 1, 0); + } + + // Run migration + let weight = crate::migrations::migrate_commit_reveal_settings::migrate_commit_reveal_settings::(); + + // Verify all networks have correct settings + for netuid in &netuids { + assert_eq!(RevealPeriodEpochs::::get(NetUid::from(*netuid)), 1u64); + assert!(CommitRevealWeightsEnabled::::get(NetUid::from(*netuid))); + } + + // Check migration has been marked as run + assert!(HasMigrationRun::::get(MIGRATION_NAME.as_bytes().to_vec())); + }); +} + +#[test] +fn test_migrate_commit_reveal_settings_values_access() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + add_network(netuid.into(), 1, 0); + + // Run migration + crate::migrations::migrate_commit_reveal_settings::migrate_commit_reveal_settings::(); + + // Test that we can access the values using the pallet functions + assert_eq!( + SubtensorModule::get_reveal_period(NetUid::from(netuid)), + 1u64 + ); + + // Test direct storage access + assert_eq!(RevealPeriodEpochs::::get(NetUid::from(netuid)), 1u64); + assert!(CommitRevealWeightsEnabled::::get(NetUid::from( + netuid + ))); + }); +} + #[test] fn test_migrate_crv3_v2_to_timelocked() { new_test_ext(1).execute_with(|| { diff --git a/pallets/subtensor/src/tests/mock.rs b/pallets/subtensor/src/tests/mock.rs index fd7dcbab45..8aa6fe6cdd 100644 --- a/pallets/subtensor/src/tests/mock.rs +++ b/pallets/subtensor/src/tests/mock.rs @@ -870,6 +870,19 @@ pub fn add_dynamic_network_without_emission_block(hotkey: &U256, coldkey: &U256) netuid } +#[allow(dead_code)] +pub fn add_dynamic_network_disable_commit_reveal(hotkey: &U256, coldkey: &U256) -> NetUid { + let netuid = add_dynamic_network(hotkey, coldkey); + SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); + netuid +} + +#[allow(dead_code)] +pub fn add_network_disable_commit_reveal(netuid: NetUid, tempo: u16, _modality: u16) { + add_network(netuid, tempo, _modality); + SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); +} + // Helper function to set up a neuron with stake #[allow(dead_code)] pub fn setup_neuron_with_stake(netuid: NetUid, hotkey: U256, coldkey: U256, stake: TaoCurrency) { diff --git a/pallets/subtensor/src/tests/weights.rs b/pallets/subtensor/src/tests/weights.rs index 4bce2ec3af..4784c6b00e 100644 --- a/pallets/subtensor/src/tests/weights.rs +++ b/pallets/subtensor/src/tests/weights.rs @@ -814,6 +814,7 @@ fn test_set_weights_is_root_error() { let weights = vec![1]; let version_key: u64 = 0; let hotkey = U256::from(1); + SubtensorModule::set_commit_reveal_weights_enabled(NetUid::ROOT, false); assert_err!( SubtensorModule::set_weights( @@ -836,7 +837,7 @@ fn test_weights_err_no_validator_permit() { let hotkey_account_id = U256::from(55); let netuid = NetUid::from(1); let tempo: u16 = 13; - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); SubtensorModule::set_min_allowed_weights(netuid, 0); SubtensorModule::set_max_allowed_uids(netuid, 3); SubtensorModule::set_max_weight_limit(netuid, u16::MAX); @@ -884,7 +885,7 @@ fn test_set_stake_threshold_failed() { let hotkey = U256::from(0); let coldkey = U256::from(0); - add_network(netuid, 1, 0); + add_network_disable_commit_reveal(netuid, 1, 0); register_ok_neuron(netuid, hotkey, coldkey, 2143124); SubtensorModule::set_stake_threshold(20_000_000_000_000); SubtensorModule::add_balance_to_coldkey_account(&hotkey, u64::MAX); @@ -946,8 +947,8 @@ fn test_weights_version_key() { let netuid0 = NetUid::from(1); let netuid1 = NetUid::from(2); - add_network(netuid0, 1, 0); - add_network(netuid1, 1, 0); + add_network_disable_commit_reveal(netuid0, 1, 0); + add_network_disable_commit_reveal(netuid1, 1, 0); register_ok_neuron(netuid0, hotkey, coldkey, 2143124); register_ok_neuron(netuid1, hotkey, coldkey, 3124124); @@ -1022,7 +1023,7 @@ fn test_weights_err_setting_weights_too_fast() { let hotkey_account_id = U256::from(55); let netuid = NetUid::from(1); let tempo: u16 = 13; - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); SubtensorModule::set_min_allowed_weights(netuid, 0); SubtensorModule::set_max_allowed_uids(netuid, 3); SubtensorModule::set_max_weight_limit(netuid, u16::MAX); @@ -1175,7 +1176,7 @@ fn test_weights_err_max_weight_limit() { // Add network. let netuid = NetUid::from(1); let tempo: u16 = 100; - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); // Set params. SubtensorModule::set_max_allowed_uids(netuid, 5); @@ -1264,6 +1265,7 @@ fn test_no_signature() { new_test_ext(0).execute_with(|| { let uids: Vec = vec![]; let values: Vec = vec![]; + SubtensorModule::set_commit_reveal_weights_enabled(1.into(), false); let result = SubtensorModule::set_weights(RuntimeOrigin::none(), 1.into(), uids, values, 0); assert_eq!(result, Err(DispatchError::BadOrigin)); }); @@ -1348,7 +1350,7 @@ fn test_set_weight_not_enough_values() { let tempo: u16 = 13; let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let account_id = U256::from(1); - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); register_ok_neuron(netuid, account_id, U256::from(2), 100000); let neuron_uid: u16 = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &U256::from(1)) @@ -1411,7 +1413,7 @@ fn test_set_weight_too_many_uids() { new_test_ext(0).execute_with(|| { let netuid = NetUid::from(1); let tempo: u16 = 13; - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); register_ok_neuron(1.into(), U256::from(1), U256::from(2), 100_000); let neuron_uid: u16 = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &U256::from(1)) @@ -3196,10 +3198,10 @@ fn test_reveal_at_exact_epoch() { 1.into(), ); - let reveal_periods: Vec = vec![0, 1, 2, 7, 40, 86, 100]; + let reveal_periods: Vec = vec![1, 2, 7, 40, 86, 100]; for &reveal_period in &reveal_periods { - SubtensorModule::set_reveal_period(netuid, reveal_period); + assert_ok!(SubtensorModule::set_reveal_period(netuid, reveal_period)); let salt: Vec = vec![42; 8]; let commit_hash: H256 = BlakeTwo256::hash_of(&( @@ -3335,7 +3337,7 @@ fn test_tempo_and_reveal_period_change_during_commit_reveal_process() { let initial_tempo: u16 = 100; let initial_reveal_period: u64 = 1; add_network(netuid, initial_tempo, 0); - SubtensorModule::set_reveal_period(netuid, initial_reveal_period); + assert_ok!(SubtensorModule::set_reveal_period(netuid, initial_reveal_period)); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); SubtensorModule::set_weights_set_rate_limit(netuid, 0); @@ -3379,7 +3381,7 @@ fn test_tempo_and_reveal_period_change_during_commit_reveal_process() { let new_tempo: u16 = 50; let new_reveal_period: u64 = 2; SubtensorModule::set_tempo(netuid, new_tempo); - SubtensorModule::set_reveal_period(netuid, new_reveal_period); + assert_ok!(SubtensorModule::set_reveal_period(netuid, new_reveal_period)); log::info!( "Changed tempo to {new_tempo} and reveal period to {new_reveal_period}" ); @@ -3433,10 +3435,11 @@ fn test_tempo_and_reveal_period_change_during_commit_reveal_process() { let new_tempo_after_reveal: u16 = 200; let new_reveal_period_after_reveal: u64 = 1; SubtensorModule::set_tempo(netuid, new_tempo_after_reveal); - SubtensorModule::set_reveal_period(netuid, new_reveal_period_after_reveal); - log::info!( - "Changed tempo to {new_tempo_after_reveal} and reveal period to {new_reveal_period_after_reveal} after reveal" - ); + assert_ok!(SubtensorModule::set_reveal_period( + netuid, + new_reveal_period_after_reveal + )); + log::info!("Changed tempo to {new_tempo_after_reveal} and reveal period to {new_reveal_period_after_reveal} after reveal"); // Step 5: Commit again let new_salt: Vec = vec![43; 8]; @@ -3623,7 +3626,7 @@ fn test_reveal_at_exact_block() { let tempo: u16 = 360; System::set_block_number(0); - add_network(netuid, tempo, 0); + add_network_disable_commit_reveal(netuid, tempo, 0); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); SubtensorModule::set_weights_set_rate_limit(netuid, 0); @@ -3633,24 +3636,10 @@ fn test_reveal_at_exact_block() { SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); - let reveal_periods: Vec = vec![ - 0, - 1, - 2, - 5, - 19, - 21, - 30, - 77, - 104, - 833, - 1999, - 36398, - u32::MAX as u64, - ]; + let reveal_periods: Vec = vec![1, 2, 5, 19, 21, 30, 77]; for &reveal_period in &reveal_periods { - SubtensorModule::set_reveal_period(netuid, reveal_period); + assert_ok!(SubtensorModule::set_reveal_period(netuid, reveal_period)); // Step 1: Commit weights let salt: Vec = vec![42 + (reveal_period % 100) as u16; 8]; @@ -4403,7 +4392,7 @@ fn test_highly_concurrent_commits_and_reveals_with_multiple_hotkeys() { add_network(netuid, initial_tempo, 0); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); SubtensorModule::set_weights_set_rate_limit(netuid, 0); - SubtensorModule::set_reveal_period(netuid, initial_reveal_period); + assert_ok!(SubtensorModule::set_reveal_period(netuid, initial_reveal_period)); SubtensorModule::set_max_registrations_per_block(netuid, u16::MAX); SubtensorModule::set_target_registrations_per_interval(netuid, u16::MAX); @@ -4514,7 +4503,7 @@ fn test_highly_concurrent_commits_and_reveals_with_multiple_hotkeys() { // ==== Modify Network Parameters During Commits ==== SubtensorModule::set_tempo(netuid, 150); - SubtensorModule::set_reveal_period(netuid, 7); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 7)); log::info!("Changed tempo to 150 and reveal_period to 7 during commits."); step_epochs(3, netuid); @@ -4560,7 +4549,7 @@ fn test_highly_concurrent_commits_and_reveals_with_multiple_hotkeys() { // ==== Change Network Parameters Again ==== SubtensorModule::set_tempo(netuid, 200); - SubtensorModule::set_reveal_period(netuid, 10); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 10)); log::info!("Changed tempo to 200 and reveal_period to 10 after initial reveals."); step_epochs(10, netuid); @@ -5013,7 +5002,7 @@ fn test_reveal_crv3_commits_success() { SubtensorModule::set_stake_threshold(0); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 3); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 3)); let neuron_uid1 = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey1) .expect("Failed to get neuron UID for hotkey1"); @@ -5165,7 +5154,7 @@ fn test_reveal_crv3_commits_cannot_reveal_after_reveal_epoch() { register_ok_neuron(netuid, hotkey2, U256::from(4), 100_000); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 3); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 3)); let neuron_uid1 = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey1) .expect("Failed to get neuron UID for hotkey1"); @@ -5621,7 +5610,7 @@ fn test_reveal_crv3_commits_multiple_commits_some_fail_some_succeed() { register_ok_neuron(netuid, hotkey1, U256::from(3), 100_000); register_ok_neuron(netuid, hotkey2, U256::from(4), 100_000); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 1); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 1)); SubtensorModule::set_weights_set_rate_limit(netuid, 0); // Prepare a valid payload for hotkey1 @@ -5744,7 +5733,7 @@ fn test_reveal_crv3_commits_do_set_weights_failure() { add_network(netuid, 5, 0); register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 3); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 3)); SubtensorModule::set_weights_set_rate_limit(netuid, 0); // Prepare payload with mismatched uids and values lengths @@ -5830,7 +5819,7 @@ fn test_reveal_crv3_commits_payload_decoding_failure() { add_network(netuid, 5, 0); register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 3); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 3)); SubtensorModule::set_weights_set_rate_limit(netuid, 0); let invalid_payload = vec![0u8; 10]; // Not a valid encoding of WeightsTlockPayload @@ -5908,7 +5897,7 @@ fn test_reveal_crv3_commits_signature_deserialization_failure() { add_network(netuid, 5, 0); register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 3); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 3)); SubtensorModule::set_weights_set_rate_limit(netuid, 0); let version_key = SubtensorModule::get_weights_version_key(netuid); @@ -6054,7 +6043,7 @@ fn test_reveal_crv3_commits_with_incorrect_identity_message() { add_network(netuid, 5, 0); register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 1); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 1)); SubtensorModule::set_weights_set_rate_limit(netuid, 0); // Prepare a valid payload but use incorrect identity message during encryption @@ -6142,7 +6131,7 @@ fn test_multiple_commits_by_same_hotkey_within_limit() { add_network(netuid, 5, 0); register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 1); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 1)); SubtensorModule::set_weights_set_rate_limit(netuid, 0); for i in 0..10 { @@ -6180,7 +6169,7 @@ fn test_reveal_crv3_commits_removes_past_epoch_commits() { add_network(netuid, /*tempo*/ 5, 0); register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 1); // reveal_period = 1 epoch + assert_ok!(SubtensorModule::set_reveal_period(netuid, 1)); // reveal_period = 1 epoch SubtensorModule::set_weights_set_rate_limit(netuid, 0); // --------------------------------------------------------------------- @@ -6237,7 +6226,7 @@ fn test_reveal_crv3_commits_multiple_valid_commits_all_processed() { // ───── network parameters ─────────────────────────────────────────── add_network(netuid, 5, 0); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 1); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 1)); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_stake_threshold(0); SubtensorModule::set_max_registrations_per_block(netuid, 100); @@ -6353,7 +6342,7 @@ fn test_reveal_crv3_commits_max_neurons() { // ───── network parameters ─────────────────────────────────────────── add_network(netuid, 5, 0); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 1); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 1)); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_stake_threshold(0); SubtensorModule::set_max_registrations_per_block(netuid, 10_000); @@ -6582,7 +6571,7 @@ fn test_reveal_crv3_commits_hotkey_check() { SubtensorModule::set_stake_threshold(0); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 3); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 3)); let neuron_uid1 = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey1) .expect("Failed to get neuron UID for hotkey1"); @@ -6699,7 +6688,7 @@ fn test_reveal_crv3_commits_hotkey_check() { SubtensorModule::set_stake_threshold(0); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 3); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 3)); let neuron_uid1 = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey1) .expect("Failed to get neuron UID for hotkey1"); @@ -6848,7 +6837,7 @@ fn test_reveal_crv3_commits_retry_on_missing_pulse() { add_network(netuid, 5, 0); register_ok_neuron(netuid, hotkey, U256::from(3), 100_000); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 3); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 3)); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_stake_threshold(0); @@ -6963,7 +6952,7 @@ fn test_reveal_crv3_commits_legacy_payload_success() { SubtensorModule::set_stake_threshold(0); SubtensorModule::set_weights_set_rate_limit(netuid, 0); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - SubtensorModule::set_reveal_period(netuid, 3); + assert_ok!(SubtensorModule::set_reveal_period(netuid, 3)); let uid1 = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey1).unwrap(); let uid2 = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey2).unwrap(); diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 6fb38b491a..60687d3a30 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -220,7 +220,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: 305, + spec_version: 306, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 1,