Merge pull request #1964 from opentensor/commit-reveal-mandatory

enable the commit reveal for all subnets

Author: sam0x17

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<typeof devnet>
+    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

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::<T>::SubnetDoesNotExist
             );
 
-            const MAX_COMMIT_REVEAL_PEROIDS: u64 = 100;
-            ensure!(
-                interval <= MAX_COMMIT_REVEAL_PEROIDS,
-                Error::<T>::RevealPeriodOutOfBounds
-            );
-
-            pallet_subtensor::Pallet::<T>::set_reveal_period(netuid, interval);
             log::debug!("SetWeightCommitInterval( netuid: {netuid:?}, interval: {interval:?} ) ");
+
+            pallet_subtensor::Pallet::<T>::set_reveal_period(netuid, interval)?;
+
             Ok(())
         }
 

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::<Test>::RevealPeriodOutOfBounds
+            pallet_subtensor::Error::<Test>::RevealPeriodTooLarge
         );
 
         let to_be_set = 55;

pallets/subtensor/src/benchmarks.rs

@@ -60,6 +60,7 @@ mod pallet_benchmarks {
         Subtensor::<T>::set_network_registration_allowed(netuid, true);
         Subtensor::<T>::set_max_registrations_per_block(netuid, 4096);
         Subtensor::<T>::set_target_registrations_per_interval(netuid, 4096);
+        Subtensor::<T>::set_commit_reveal_weights_enabled(netuid, false);
 
         let mut seed: u32 = 1;
         let mut dests = Vec::new();

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<T: Config>() -> bool {
-        false
+        true
     }
     #[pallet::type_value]
     /// Default value for weight commit/reveal version.

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,
     }

pallets/subtensor/src/macros/hooks.rs

@@ -130,6 +130,8 @@ mod hooks {
                 .saturating_add(migrations::migrate_subnet_symbols::migrate_subnet_symbols::<T>())
                 // Migrate CRV3 add commit_block
                 .saturating_add(migrations::migrate_crv3_commits_add_block::migrate_crv3_commits_add_block::<T>())
+                // Migrate Commit-Reveal Settings
+                .saturating_add(migrations::migrate_commit_reveal_settings::migrate_commit_reveal_settings::<T>())
                 //Migrate CRV3 to TimelockedCommits
                 .saturating_add(migrations::migrate_crv3_v2_to_timelocked::migrate_crv3_v2_to_timelocked::<T>())
                 // Migrate to fix root counters

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<T: Config>() -> 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::<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)
+    );
+
+    let netuids: Vec<NetUid> = <NetworksAdded<T> as IterableStorageMap<NetUid, bool>>::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::<T>::get(*netuid) {
+            CommitRevealWeightsEnabled::<T>::insert(*netuid, true);
+            weight = weight.saturating_add(T::DbWeight::get().writes(1));
+        }
+
+        if RevealPeriodEpochs::<T>::get(*netuid) == 0 {
+            RevealPeriodEpochs::<T>::insert(*netuid, MIN_COMMIT_REVEAL_PEROIDS);
+            weight = weight.saturating_add(T::DbWeight::get().writes(1));
+        }
+    }
+
+    // Mark the migration as completed
+    HasMigrationRun::<T>::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
+}

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;

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<T: Config> Pallet<T> {
     /// ---- The implementation for committing weight hashes.
     ///
@@ -1062,9 +1063,21 @@ impl<T: Config> Pallet<T> {
         (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::<T>::RevealPeriodTooLarge
+        );
+
+        ensure!(
+            reveal_period >= MIN_COMMIT_REVEAL_PEROIDS,
+            Error::<T>::RevealPeriodTooSmall
+        );
+
         RevealPeriodEpochs::<T>::insert(netuid, reveal_period);
+
         Self::deposit_event(Event::CommitRevealPeriodsSet(netuid, reveal_period));
+        Ok(())
     }
     pub fn get_reveal_period(netuid: NetUid) -> u64 {
         RevealPeriodEpochs::<T>::get(netuid)