From 8810f4bb2868650f642bf081a7a887c7ed842806 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Fri, 25 Oct 2024 16:13:13 -0700 Subject: [PATCH 01/15] store more blocks & batch reveals out of order --- pallets/subtensor/src/lib.rs | 2 +- pallets/subtensor/src/subnets/weights.rs | 348 +++++++++++++---------- pallets/subtensor/tests/swap_hotkey.rs | 4 +- pallets/subtensor/tests/weights.rs | 2 +- 4 files changed, 204 insertions(+), 152 deletions(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 5081d69d3..9a6d60f8f 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -1258,7 +1258,7 @@ pub mod pallet { u16, Twox64Concat, T::AccountId, - VecDeque<(H256, u64)>, + VecDeque<(H256, u64, u64, u64)>, OptionQuery, >; #[pallet::storage] diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index ba673afa4..8cbf7695b 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -39,14 +39,19 @@ impl Pallet { Error::::CommitRevealDisabled ); - // --- 3. Mutate the WeightCommits to retrieve existing commits for the user. + // --- 3. Calculate the reveal blocks based on tempo and reveal period. + let commit_block: u64 = Self::get_current_block_as_u64(); + let (first_reveal_block, last_reveal_block) = Self::get_reveal_blocks(netuid, commit_block); + + // --- 4. Mutate the WeightCommits to retrieve existing commits for the user. WeightCommits::::try_mutate(netuid, &who, |maybe_commits| -> DispatchResult { - // --- 4. Take the existing commits or create a new VecDeque. - let mut commits: VecDeque<(H256, u64)> = maybe_commits.take().unwrap_or_default(); + // --- 5. Take the existing commits or create a new VecDeque. + let mut commits: VecDeque<(H256, u64, u64, u64)> = + maybe_commits.take().unwrap_or_default(); - // --- 5. Remove any expired commits from the front of the queue. - while let Some((_, commit_block)) = commits.front() { - if Self::is_commit_expired(netuid, *commit_block) { + // --- 6. Remove any expired commits from the front of the queue. + while let Some((_, commit_block_existing, _, _)) = commits.front() { + if Self::is_commit_expired(netuid, *commit_block_existing) { // Remove the expired commit commits.pop_front(); } else { @@ -54,16 +59,21 @@ impl Pallet { } } - // --- 6. Check if the current number of unrevealed commits is within the allowed limit. + // --- 7. Check if the current number of unrevealed commits is within the allowed limit. ensure!(commits.len() < 10, Error::::TooManyUnrevealedCommits); - // --- 7. Append the new commit to the queue. - commits.push_back((commit_hash, Self::get_current_block_as_u64())); + // --- 8. Append the new commit to the queue. + commits.push_back(( + commit_hash, + commit_block, + first_reveal_block, + last_reveal_block, + )); - // --- 8. Store the updated queue back to storage. + // --- 9. Store the updated queue back to storage. *maybe_commits = Some(commits); - // --- 9. Return ok. + // --- 10. Return ok. Ok(()) }) } @@ -131,7 +141,7 @@ impl Pallet { // --- 4. Remove any expired commits from the front of the queue, collecting their hashes. let mut expired_hashes = Vec::new(); - while let Some((hash, commit_block)) = commits.front() { + while let Some((hash, commit_block, _, _)) = commits.front() { if Self::is_commit_expired(netuid, *commit_block) { // Collect the expired commit hash expired_hashes.push(*hash); @@ -163,9 +173,12 @@ impl Pallet { } // --- 7. Search for the provided_hash in the non-expired commits. - if let Some(position) = commits.iter().position(|(hash, _)| *hash == provided_hash) { + if let Some(position) = commits + .iter() + .position(|(hash, _, _, _)| *hash == provided_hash) + { // --- 8. Get the commit block for the commit being revealed. - let (_, commit_block) = commits + let (_, commit_block, _, _) = commits .get(position) .ok_or(Error::::NoWeightsCommitFound)?; @@ -199,152 +212,174 @@ impl Pallet { }) } - /// ---- The implementation for batch revealing committed weights. - /// - /// # Args: - /// * `origin`: (`::RuntimeOrigin`): - /// - The signature of the revealing hotkey. - /// - /// * `netuid` (`u16`): - /// - The u16 network identifier. - /// - /// * `uids_list` (`Vec>`): - /// - A list of uids for each set of weights being revealed. - /// - /// * `values_list` (`Vec>`): - /// - A list of values for each set of weights being revealed. - /// - /// * `salts_list` (`Vec>`): - /// - A list of salts used to generate the commit hashes. - /// - /// * `version_keys` (`Vec`): - /// - A list of network version keys. - /// - /// # Raises: - /// * `CommitRevealDisabled`: - /// - Attempting to reveal weights when the commit-reveal mechanism is disabled. - /// - /// * `NoWeightsCommitFound`: - /// - Attempting to reveal weights without an existing commit. - /// - /// * `ExpiredWeightCommit`: - /// - Attempting to reveal a weight commit that has expired. - /// - /// * `RevealTooEarly`: - /// - Attempting to reveal weights outside the valid reveal period. - /// - /// * `InvalidRevealCommitHashNotMatch`: - /// - The revealed hash does not match any committed hash. - /// - /// * `InvalidInputLengths`: - /// - The input vectors are of mismatched lengths. - pub fn do_batch_reveal_weights( - origin: T::RuntimeOrigin, - netuid: u16, - uids_list: Vec>, - values_list: Vec>, - salts_list: Vec>, - version_keys: Vec, - ) -> DispatchResult { - // --- 1. Check that the input lists are of the same length. - let num_reveals = uids_list.len(); - ensure!( - num_reveals == values_list.len() - && num_reveals == salts_list.len() - && num_reveals == version_keys.len(), - Error::::InputLengthsUnequal - ); - - // --- 2. Check the caller's signature (hotkey). - let who = ensure_signed(origin.clone())?; - - log::debug!( - "do_batch_reveal_weights( hotkey:{:?} netuid:{:?})", - who, - netuid - ); +/// ---- The implementation for batch revealing committed weights. +/// +/// # Args: +/// * `origin`: (`::RuntimeOrigin`): +/// - The signature of the revealing hotkey. +/// +/// * `netuid` (`u16`): +/// - The u16 network identifier. +/// +/// * `uids_list` (`Vec>`): +/// - A list of uids for each set of weights being revealed. +/// +/// * `values_list` (`Vec>`): +/// - A list of values for each set of weights being revealed. +/// +/// * `salts_list` (`Vec>`): +/// - A list of salts used to generate the commit hashes. +/// +/// * `version_keys` (`Vec`): +/// - A list of network version keys. +/// +/// # Raises: +/// * `CommitRevealDisabled`: +/// - Attempting to reveal weights when the commit-reveal mechanism is disabled. +/// +/// * `NoWeightsCommitFound`: +/// - Attempting to reveal weights without an existing commit. +/// +/// * `ExpiredWeightCommit`: +/// - Attempting to reveal a weight commit that has expired. +/// +/// * `RevealTooEarly`: +/// - Attempting to reveal weights outside the valid reveal period. +/// +/// * `InvalidRevealCommitHashNotMatch`: +/// - The revealed hash does not match any committed hash. +/// +/// * `InputLengthsUnequal`: +/// - The input vectors are of mismatched lengths. +pub fn do_batch_reveal_weights( + origin: T::RuntimeOrigin, + netuid: u16, + uids_list: Vec>, + values_list: Vec>, + salts_list: Vec>, + version_keys: Vec, +) -> DispatchResult { + // --- 1. Check that the input lists are of the same length. + let num_reveals = uids_list.len(); + ensure!( + num_reveals == values_list.len() + && num_reveals == salts_list.len() + && num_reveals == version_keys.len(), + Error::::InputLengthsUnequal + ); + + // --- 2. Check the caller's signature (hotkey). + let who = ensure_signed(origin.clone())?; + + log::debug!( + "do_batch_reveal_weights( hotkey:{:?} netuid:{:?})", + who, + netuid + ); + + // --- 3. Ensure commit-reveal is enabled for the network. + ensure!( + Self::get_commit_reveal_weights_enabled(netuid), + Error::::CommitRevealDisabled + ); + + // --- 4. Mutate the WeightCommits to retrieve existing commits for the user. + WeightCommits::::try_mutate_exists(netuid, &who, |maybe_commits| -> DispatchResult { + let commits = maybe_commits + .as_mut() + .ok_or(Error::::NoWeightsCommitFound)?; + + // --- 5. Remove any expired commits from the front of the queue, collecting their hashes. + let mut expired_hashes = Vec::new(); + while let Some((hash, commit_block, _, _)) = commits.front() { + if Self::is_commit_expired(netuid, *commit_block) { + // Collect the expired commit hash + expired_hashes.push(*hash); + commits.pop_front(); + } else { + break; + } + } - // --- 3. Ensure commit-reveal is enabled for the network. - ensure!( - Self::get_commit_reveal_weights_enabled(netuid), - Error::::CommitRevealDisabled - ); + // --- 6. Prepare to collect all provided hashes and their corresponding reveals. + let mut provided_hashes = Vec::new(); + let mut reveals = Vec::new(); - // --- 4. Mutate the WeightCommits to retrieve existing commits for the user. - WeightCommits::::try_mutate_exists(netuid, &who, |maybe_commits| -> DispatchResult { - let commits = maybe_commits - .as_mut() - .ok_or(Error::::NoWeightsCommitFound)?; + for ((uids, values), (salt, version_key)) in uids_list + .into_iter() + .zip(values_list) + .zip(salts_list.into_iter().zip(version_keys)) + { + // --- 6a. Hash the provided data. + let provided_hash: H256 = BlakeTwo256::hash_of(&( + who.clone(), + netuid, + uids.clone(), + values.clone(), + salt.clone(), + version_key, + )); + provided_hashes.push(provided_hash); + reveals.push((uids, values, version_key, provided_hash)); + } - // --- 5. Remove any expired commits from the front of the queue, collecting their hashes. - let mut expired_hashes = Vec::new(); - while let Some((hash, commit_block)) = commits.front() { - if Self::is_commit_expired(netuid, *commit_block) { - // Collect the expired commit hash - expired_hashes.push(*hash); - commits.pop_front(); + // --- 7. Validate all reveals first to ensure atomicity. + // This prevents partial updates if any reveal fails. + for (_uids, _values, _version_key, provided_hash) in &reveals { + // --- 7a. Check if the provided_hash is in the non-expired commits. + if !commits.iter().any(|(hash, _, _, _)| *hash == *provided_hash) { + // --- 7b. If not found, check if it matches any expired commits. + if expired_hashes.contains(provided_hash) { + return Err(Error::::ExpiredWeightCommit.into()); } else { - break; + return Err(Error::::InvalidRevealCommitHashNotMatch.into()); } } - // --- 6. Process each reveal. - for ((uids, values), (salt, version_key)) in uids_list - .into_iter() - .zip(values_list) - .zip(salts_list.into_iter().zip(version_keys)) - { - // --- 6a. Hash the provided data. - let provided_hash: H256 = BlakeTwo256::hash_of(&( - who.clone(), - netuid, - uids.clone(), - values.clone(), - salt.clone(), - version_key, - )); - - // --- 6b. Search for the provided_hash in the non-expired commits. - if let Some(position) = commits.iter().position(|(hash, _)| *hash == provided_hash) - { - // --- 6c. Get the commit block for the commit being revealed. - let (_, commit_block) = commits - .get(position) - .ok_or(Error::::NoWeightsCommitFound)?; - - // --- 6d. Ensure the commit is ready to be revealed in the current block range. - ensure!( - Self::is_reveal_block_range(netuid, *commit_block), - Error::::RevealTooEarly - ); - - // --- 6e. Remove all commits up to and including the one being revealed. - for _ in 0..=position { - commits.pop_front(); - } - - // --- 6f. Proceed to set the revealed weights. - Self::do_set_weights(origin.clone(), netuid, uids, values, version_key)?; + // --- 7c. Find the commit corresponding to the provided_hash. + let commit = commits + .iter() + .find(|(hash, _, _, _)| *hash == *provided_hash) + .ok_or(Error::::NoWeightsCommitFound)?; + + // --- 7d. Check if the commit is within the reveal window. + let current_block: u64 = Self::get_current_block_as_u64(); + let (_, _, first_reveal_block, last_reveal_block) = commit; + ensure!( + current_block >= *first_reveal_block && current_block <= *last_reveal_block, + Error::::RevealTooEarly + ); + } + + // --- 8. All reveals are valid. Proceed to remove and process each reveal. + for (uids, values, version_key, provided_hash) in reveals { + // --- 8a. Find the position of the provided_hash. + if let Some(position) = commits.iter().position(|(hash, _, _, _)| *hash == provided_hash) { + // --- 8b. Remove the commit from the queue. + commits.remove(position); + + // --- 8c. Proceed to set the revealed weights. + Self::do_set_weights(origin.clone(), netuid, uids, values, version_key)?; + } else { + // This case should not occur as we've already validated the existence of the hash. + // However, to ensure safety, we handle it. + if expired_hashes.contains(&provided_hash) { + return Err(Error::::ExpiredWeightCommit.into()); } else { - // The provided_hash does not match any non-expired commits. - // Check if it matches any expired commits - if expired_hashes.contains(&provided_hash) { - return Err(Error::::ExpiredWeightCommit.into()); - } else { - return Err(Error::::InvalidRevealCommitHashNotMatch.into()); - } + return Err(Error::::InvalidRevealCommitHashNotMatch.into()); } } + } - // --- 7. If the queue is now empty, remove the storage entry for the user. - if commits.is_empty() { - *maybe_commits = None; - } + // --- 9. If the queue is now empty, remove the storage entry for the user. + if commits.is_empty() { + *maybe_commits = None; + } - // --- 8. Return ok. - Ok(()) - }) - } + // --- 10. Return ok. + Ok(()) + }) +} /// ---- The implementation for the extrinsic set_weights. /// @@ -703,6 +738,23 @@ impl Pallet { current_epoch > commit_epoch.saturating_add(reveal_period) } + pub fn get_reveal_blocks(netuid: u16, commit_block: u64) -> (u64, u64) { + let reveal_period: u64 = Self::get_reveal_period(netuid); + let tempo: u64 = Self::get_tempo(netuid) as u64; + let tempo_plus_one: u64 = tempo.saturating_add(1); + let netuid_plus_one: u64 = (netuid as u64).saturating_add(1); + + let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); + let reveal_epoch: u64 = commit_epoch.saturating_add(reveal_period); + + let first_reveal_block = reveal_epoch + .saturating_mul(tempo_plus_one) + .saturating_sub(netuid_plus_one); + let last_reveal_block = first_reveal_block.saturating_add(tempo); + + (first_reveal_block, last_reveal_block) + } + pub fn set_reveal_period(netuid: u16, reveal_period: u64) { RevealPeriodEpochs::::insert(netuid, reveal_period); } diff --git a/pallets/subtensor/tests/swap_hotkey.rs b/pallets/subtensor/tests/swap_hotkey.rs index bf5ecb301..ad4d0414c 100644 --- a/pallets/subtensor/tests/swap_hotkey.rs +++ b/pallets/subtensor/tests/swap_hotkey.rs @@ -351,8 +351,8 @@ fn test_swap_weight_commits() { let new_hotkey = U256::from(2); let coldkey = U256::from(3); let netuid = 0u16; - let mut weight_commits: VecDeque<(H256, u64)> = VecDeque::new(); - weight_commits.push_back((H256::from_low_u64_be(100), 200)); + let mut weight_commits: VecDeque<(H256, u64, u64, u64)> = VecDeque::new(); + weight_commits.push_back((H256::from_low_u64_be(100), 200, 1, 1)); let mut weight = Weight::zero(); add_network(netuid, 0, 1); diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index ddc2ccd77..7f6592f23 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -2432,7 +2432,7 @@ fn test_expired_commits_handling_in_commit_and_reveal() { )); // 6. Verify that the number of unrevealed, non-expired commits is now 6 - let commits: VecDeque<(H256, u64)> = + let commits: VecDeque<(H256, u64, u64, u64)> = pallet_subtensor::WeightCommits::::get(netuid, hotkey) .expect("Expected a commit"); assert_eq!(commits.len(), 6); // 5 non-expired commits from epoch 1 + new commit From 6886a6d5d5992d8d03b2319057ad2b3b483d869f Mon Sep 17 00:00:00 2001 From: johnreedv Date: Mon, 28 Oct 2024 14:49:58 -0700 Subject: [PATCH 02/15] update test --- pallets/subtensor/tests/weights.rs | 69 +++++++++++------------------- 1 file changed, 25 insertions(+), 44 deletions(-) diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 7f6592f23..c21c754af 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -3584,7 +3584,7 @@ fn test_batch_reveal_with_out_of_order_commits() { SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); - // 1. Commit multiple times + // 1. Commit multiple times (A, B, C) let mut commit_info = Vec::new(); for i in 0..3 { let salt: Vec = vec![i as u16; 8]; @@ -3606,25 +3606,25 @@ fn test_batch_reveal_with_out_of_order_commits() { step_epochs(1, netuid); - // 2. Prepare batch reveal parameters out of order + // 2. Prepare batch reveal parameters for commits A and C (out of order) let salts_list: Vec> = vec![ - commit_info[2].1.clone(), // Third commit - commit_info[0].1.clone(), // First commit - commit_info[1].1.clone(), // Second commit + commit_info[2].1.clone(), // Third commit (C) + commit_info[0].1.clone(), // First commit (A) ]; let uids_list_out_of_order = vec![ - uids_list[2].clone(), - uids_list[0].clone(), - uids_list[1].clone(), + uids_list[2].clone(), // C + uids_list[0].clone(), // A ]; let weight_values_list_out_of_order = vec![ - weight_values_list[2].clone(), - weight_values_list[0].clone(), - weight_values_list[1].clone(), + weight_values_list[2].clone(), // C + weight_values_list[0].clone(), // A + ]; + let version_keys_out_of_order = vec![ + version_keys[2], // C + version_keys[0], // A ]; - let version_keys_out_of_order = vec![version_keys[2], version_keys[0], version_keys[1]]; - // 3. Attempt batch reveal out of order + // 3. Attempt batch reveal of A and C out of order let result = SubtensorModule::do_batch_reveal_weights( RuntimeOrigin::signed(hotkey), netuid, @@ -3634,44 +3634,25 @@ fn test_batch_reveal_with_out_of_order_commits() { version_keys_out_of_order, ); - // 4. Ensure the batch reveal fails with InvalidRevealCommitHashNotMatch - assert_err!(result, Error::::InvalidRevealCommitHashNotMatch); - - // 5. Reveal the first commit to proceed correctly - let first_salt = commit_info[0].1.clone(); - let first_uids = uids_list[0].clone(); - let first_weights = weight_values_list[0].clone(); - let first_version_key = version_keys[0]; - - assert_ok!(SubtensorModule::do_batch_reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - vec![first_uids], - vec![first_weights], - vec![first_salt], - vec![first_version_key], - )); + // 4. Ensure the batch reveal succeeds + assert_ok!(result); - // 6. Now attempt to reveal the remaining commits in order - let remaining_salts = vec![ - commit_info[1].1.clone(), // Second commit - commit_info[2].1.clone(), // Third commit - ]; - let remaining_uids_list = vec![uids_list[1].clone(), uids_list[2].clone()]; - let remaining_weight_values_list = - vec![weight_values_list[1].clone(), weight_values_list[2].clone()]; - let remaining_version_keys = vec![version_keys[1], version_keys[2]]; + // 5. Prepare and reveal the remaining commit (B) + let remaining_salt = commit_info[1].1.clone(); + let remaining_uids = uids_list[1].clone(); + let remaining_weights = weight_values_list[1].clone(); + let remaining_version_key = version_keys[1]; assert_ok!(SubtensorModule::do_batch_reveal_weights( RuntimeOrigin::signed(hotkey), netuid, - remaining_uids_list, - remaining_weight_values_list, - remaining_salts, - remaining_version_keys, + vec![remaining_uids], + vec![remaining_weights], + vec![remaining_salt], + vec![remaining_version_key], )); - // 7. Ensure all commits are removed + // 6. Ensure all commits are removed let commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey); assert!(commits.is_none()); }); From 59426cd4bb264a137071bd9a477be03e4e884a95 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Mon, 28 Oct 2024 17:06:59 -0700 Subject: [PATCH 03/15] add test_get_reveal_blocks & fmt --- pallets/subtensor/src/subnets/weights.rs | 312 ++++++++++++----------- pallets/subtensor/tests/weights.rs | 124 +++++++++ 2 files changed, 283 insertions(+), 153 deletions(-) diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 8cbf7695b..44c343d69 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -212,174 +212,180 @@ impl Pallet { }) } -/// ---- The implementation for batch revealing committed weights. -/// -/// # Args: -/// * `origin`: (`::RuntimeOrigin`): -/// - The signature of the revealing hotkey. -/// -/// * `netuid` (`u16`): -/// - The u16 network identifier. -/// -/// * `uids_list` (`Vec>`): -/// - A list of uids for each set of weights being revealed. -/// -/// * `values_list` (`Vec>`): -/// - A list of values for each set of weights being revealed. -/// -/// * `salts_list` (`Vec>`): -/// - A list of salts used to generate the commit hashes. -/// -/// * `version_keys` (`Vec`): -/// - A list of network version keys. -/// -/// # Raises: -/// * `CommitRevealDisabled`: -/// - Attempting to reveal weights when the commit-reveal mechanism is disabled. -/// -/// * `NoWeightsCommitFound`: -/// - Attempting to reveal weights without an existing commit. -/// -/// * `ExpiredWeightCommit`: -/// - Attempting to reveal a weight commit that has expired. -/// -/// * `RevealTooEarly`: -/// - Attempting to reveal weights outside the valid reveal period. -/// -/// * `InvalidRevealCommitHashNotMatch`: -/// - The revealed hash does not match any committed hash. -/// -/// * `InputLengthsUnequal`: -/// - The input vectors are of mismatched lengths. -pub fn do_batch_reveal_weights( - origin: T::RuntimeOrigin, - netuid: u16, - uids_list: Vec>, - values_list: Vec>, - salts_list: Vec>, - version_keys: Vec, -) -> DispatchResult { - // --- 1. Check that the input lists are of the same length. - let num_reveals = uids_list.len(); - ensure!( - num_reveals == values_list.len() - && num_reveals == salts_list.len() - && num_reveals == version_keys.len(), - Error::::InputLengthsUnequal - ); - - // --- 2. Check the caller's signature (hotkey). - let who = ensure_signed(origin.clone())?; - - log::debug!( - "do_batch_reveal_weights( hotkey:{:?} netuid:{:?})", - who, - netuid - ); - - // --- 3. Ensure commit-reveal is enabled for the network. - ensure!( - Self::get_commit_reveal_weights_enabled(netuid), - Error::::CommitRevealDisabled - ); - - // --- 4. Mutate the WeightCommits to retrieve existing commits for the user. - WeightCommits::::try_mutate_exists(netuid, &who, |maybe_commits| -> DispatchResult { - let commits = maybe_commits - .as_mut() - .ok_or(Error::::NoWeightsCommitFound)?; - - // --- 5. Remove any expired commits from the front of the queue, collecting their hashes. - let mut expired_hashes = Vec::new(); - while let Some((hash, commit_block, _, _)) = commits.front() { - if Self::is_commit_expired(netuid, *commit_block) { - // Collect the expired commit hash - expired_hashes.push(*hash); - commits.pop_front(); - } else { - break; - } - } + /// ---- The implementation for batch revealing committed weights. + /// + /// # Args: + /// * `origin`: (`::RuntimeOrigin`): + /// - The signature of the revealing hotkey. + /// + /// * `netuid` (`u16`): + /// - The u16 network identifier. + /// + /// * `uids_list` (`Vec>`): + /// - A list of uids for each set of weights being revealed. + /// + /// * `values_list` (`Vec>`): + /// - A list of values for each set of weights being revealed. + /// + /// * `salts_list` (`Vec>`): + /// - A list of salts used to generate the commit hashes. + /// + /// * `version_keys` (`Vec`): + /// - A list of network version keys. + /// + /// # Raises: + /// * `CommitRevealDisabled`: + /// - Attempting to reveal weights when the commit-reveal mechanism is disabled. + /// + /// * `NoWeightsCommitFound`: + /// - Attempting to reveal weights without an existing commit. + /// + /// * `ExpiredWeightCommit`: + /// - Attempting to reveal a weight commit that has expired. + /// + /// * `RevealTooEarly`: + /// - Attempting to reveal weights outside the valid reveal period. + /// + /// * `InvalidRevealCommitHashNotMatch`: + /// - The revealed hash does not match any committed hash. + /// + /// * `InputLengthsUnequal`: + /// - The input vectors are of mismatched lengths. + pub fn do_batch_reveal_weights( + origin: T::RuntimeOrigin, + netuid: u16, + uids_list: Vec>, + values_list: Vec>, + salts_list: Vec>, + version_keys: Vec, + ) -> DispatchResult { + // --- 1. Check that the input lists are of the same length. + let num_reveals = uids_list.len(); + ensure!( + num_reveals == values_list.len() + && num_reveals == salts_list.len() + && num_reveals == version_keys.len(), + Error::::InputLengthsUnequal + ); - // --- 6. Prepare to collect all provided hashes and their corresponding reveals. - let mut provided_hashes = Vec::new(); - let mut reveals = Vec::new(); + // --- 2. Check the caller's signature (hotkey). + let who = ensure_signed(origin.clone())?; - for ((uids, values), (salt, version_key)) in uids_list - .into_iter() - .zip(values_list) - .zip(salts_list.into_iter().zip(version_keys)) - { - // --- 6a. Hash the provided data. - let provided_hash: H256 = BlakeTwo256::hash_of(&( - who.clone(), - netuid, - uids.clone(), - values.clone(), - salt.clone(), - version_key, - )); - provided_hashes.push(provided_hash); - reveals.push((uids, values, version_key, provided_hash)); - } + log::debug!( + "do_batch_reveal_weights( hotkey:{:?} netuid:{:?})", + who, + netuid + ); - // --- 7. Validate all reveals first to ensure atomicity. - // This prevents partial updates if any reveal fails. - for (_uids, _values, _version_key, provided_hash) in &reveals { - // --- 7a. Check if the provided_hash is in the non-expired commits. - if !commits.iter().any(|(hash, _, _, _)| *hash == *provided_hash) { - // --- 7b. If not found, check if it matches any expired commits. - if expired_hashes.contains(provided_hash) { - return Err(Error::::ExpiredWeightCommit.into()); + // --- 3. Ensure commit-reveal is enabled for the network. + ensure!( + Self::get_commit_reveal_weights_enabled(netuid), + Error::::CommitRevealDisabled + ); + + // --- 4. Mutate the WeightCommits to retrieve existing commits for the user. + WeightCommits::::try_mutate_exists(netuid, &who, |maybe_commits| -> DispatchResult { + let commits = maybe_commits + .as_mut() + .ok_or(Error::::NoWeightsCommitFound)?; + + // --- 5. Remove any expired commits from the front of the queue, collecting their hashes. + let mut expired_hashes = Vec::new(); + while let Some((hash, commit_block, _, _)) = commits.front() { + if Self::is_commit_expired(netuid, *commit_block) { + // Collect the expired commit hash + expired_hashes.push(*hash); + commits.pop_front(); } else { - return Err(Error::::InvalidRevealCommitHashNotMatch.into()); + break; } } - // --- 7c. Find the commit corresponding to the provided_hash. - let commit = commits - .iter() - .find(|(hash, _, _, _)| *hash == *provided_hash) - .ok_or(Error::::NoWeightsCommitFound)?; + // --- 6. Prepare to collect all provided hashes and their corresponding reveals. + let mut provided_hashes = Vec::new(); + let mut reveals = Vec::new(); - // --- 7d. Check if the commit is within the reveal window. - let current_block: u64 = Self::get_current_block_as_u64(); - let (_, _, first_reveal_block, last_reveal_block) = commit; - ensure!( - current_block >= *first_reveal_block && current_block <= *last_reveal_block, - Error::::RevealTooEarly - ); - } + for ((uids, values), (salt, version_key)) in uids_list + .into_iter() + .zip(values_list) + .zip(salts_list.into_iter().zip(version_keys)) + { + // --- 6a. Hash the provided data. + let provided_hash: H256 = BlakeTwo256::hash_of(&( + who.clone(), + netuid, + uids.clone(), + values.clone(), + salt.clone(), + version_key, + )); + provided_hashes.push(provided_hash); + reveals.push((uids, values, version_key, provided_hash)); + } - // --- 8. All reveals are valid. Proceed to remove and process each reveal. - for (uids, values, version_key, provided_hash) in reveals { - // --- 8a. Find the position of the provided_hash. - if let Some(position) = commits.iter().position(|(hash, _, _, _)| *hash == provided_hash) { - // --- 8b. Remove the commit from the queue. - commits.remove(position); + // --- 7. Validate all reveals first to ensure atomicity. + // This prevents partial updates if any reveal fails. + for (_uids, _values, _version_key, provided_hash) in &reveals { + // --- 7a. Check if the provided_hash is in the non-expired commits. + if !commits + .iter() + .any(|(hash, _, _, _)| *hash == *provided_hash) + { + // --- 7b. If not found, check if it matches any expired commits. + if expired_hashes.contains(provided_hash) { + return Err(Error::::ExpiredWeightCommit.into()); + } else { + return Err(Error::::InvalidRevealCommitHashNotMatch.into()); + } + } - // --- 8c. Proceed to set the revealed weights. - Self::do_set_weights(origin.clone(), netuid, uids, values, version_key)?; - } else { - // This case should not occur as we've already validated the existence of the hash. - // However, to ensure safety, we handle it. - if expired_hashes.contains(&provided_hash) { - return Err(Error::::ExpiredWeightCommit.into()); + // --- 7c. Find the commit corresponding to the provided_hash. + let commit = commits + .iter() + .find(|(hash, _, _, _)| *hash == *provided_hash) + .ok_or(Error::::NoWeightsCommitFound)?; + + // --- 7d. Check if the commit is within the reveal window. + let current_block: u64 = Self::get_current_block_as_u64(); + let (_, _, first_reveal_block, last_reveal_block) = commit; + ensure!( + current_block >= *first_reveal_block && current_block <= *last_reveal_block, + Error::::RevealTooEarly + ); + } + + // --- 8. All reveals are valid. Proceed to remove and process each reveal. + for (uids, values, version_key, provided_hash) in reveals { + // --- 8a. Find the position of the provided_hash. + if let Some(position) = commits + .iter() + .position(|(hash, _, _, _)| *hash == provided_hash) + { + // --- 8b. Remove the commit from the queue. + commits.remove(position); + + // --- 8c. Proceed to set the revealed weights. + Self::do_set_weights(origin.clone(), netuid, uids, values, version_key)?; } else { - return Err(Error::::InvalidRevealCommitHashNotMatch.into()); + // This case should not occur as we've already validated the existence of the hash. + // However, to ensure safety, we handle it. + if expired_hashes.contains(&provided_hash) { + return Err(Error::::ExpiredWeightCommit.into()); + } else { + return Err(Error::::InvalidRevealCommitHashNotMatch.into()); + } } } - } - // --- 9. If the queue is now empty, remove the storage entry for the user. - if commits.is_empty() { - *maybe_commits = None; - } + // --- 9. If the queue is now empty, remove the storage entry for the user. + if commits.is_empty() { + *maybe_commits = None; + } - // --- 10. Return ok. - Ok(()) - }) -} + // --- 10. Return ok. + Ok(()) + }) + } /// ---- The implementation for the extrinsic set_weights. /// diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index c21c754af..0625e1e20 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -3935,3 +3935,127 @@ fn test_highly_concurrent_commits_and_reveals_with_multiple_hotkeys() { assert_eq!(SubtensorModule::get_tempo(netuid), 200); }) } + +#[test] +fn test_get_reveal_blocks() { + new_test_ext(1).execute_with(|| { + // **1. Define Test Parameters** + let netuid: u16 = 1; + let uids: Vec = vec![0, 1]; + let weight_values: Vec = vec![10, 10]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; + let version_key: u64 = 0; + let hotkey: U256 = U256::from(1); + + // **2. Generate the Commit Hash** + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + + // **3. Initialize the Block Number to 0** + System::set_block_number(0); + + // **4. Define Network Parameters** + let tempo: u16 = 5; + add_network(netuid, tempo, 0); + + // **5. Register Neurons and Configure the Network** + register_ok_neuron(netuid, U256::from(3), U256::from(4), 300_000); + register_ok_neuron(netuid, U256::from(1), U256::from(2), 100_000); + SubtensorModule::set_weights_set_rate_limit(netuid, 5); + SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + + // **6. Commit Weights at Block 0** + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + + // **7. Retrieve the Reveal Blocks Using `get_reveal_blocks`** + let (first_reveal_block, last_reveal_block) = SubtensorModule::get_reveal_blocks(netuid, 0); + + // **8. Assert Correct Calculation of Reveal Blocks** + // With tempo=5, netuid=1, reveal_period=1: + // commit_epoch = (0 + 2) / 6 = 0 + // reveal_epoch = 0 + 1 = 1 + // first_reveal_block = 1 * 6 - 2 = 4 + // last_reveal_block = 4 + 5 = 9 + assert_eq!(first_reveal_block, 4); + assert_eq!(last_reveal_block, 9); + + // **9. Attempt to Reveal Before `first_reveal_block` (Block 3)** + step_block(3); // Advance to block 3 + let result = SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + ); + assert_err!(result, Error::::RevealTooEarly); + + // **10. Advance to `first_reveal_block` (Block 4)** + step_block(1); // Advance to block 4 + let result = SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + ); + assert_ok!(result); + + // **11. Attempt to Reveal Again at Block 4 (Should Fail)** + let result = SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + ); + assert_err!(result, Error::::NoWeightsCommitFound); + + // **12. Advance to After `last_reveal_block` (Block 10)** + step_block(6); // Advance from block 4 to block 10 + + // **13. Attempt to Reveal at Block 10 (Should Fail)** + let result = SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + ); + assert_err!(result, Error::::NoWeightsCommitFound); + + // **14. Attempt to Reveal Outside of Any Reveal Window (No Commit)** + let result = SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + ); + assert_err!(result, Error::::NoWeightsCommitFound); + + // **15. Verify that All Commits Have Been Removed from Storage** + let commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey); + assert!( + commits.is_none(), + "Commits should be cleared after successful reveal" + ); + }) +} From 25a26e88e528d5225c22113f184303a9c86516a0 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Tue, 29 Oct 2024 03:28:07 -0700 Subject: [PATCH 04/15] update doc --- pallets/subtensor/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 9a6d60f8f..328a59f7e 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -1251,7 +1251,7 @@ pub mod pallet { /// ITEM( weights_min_stake ) pub type WeightsMinStake = StorageValue<_, u64, ValueQuery, DefaultWeightsMinStake>; #[pallet::storage] - /// --- MAP (netuid, who) --> VecDeque<(hash, commit_block)> | Stores a queue of commits for an account on a given netuid. + /// --- MAP (netuid, who) --> VecDeque<(hash, commit_block, first_reveal_block, last_reveal_block)> | Stores a queue of commits for an account on a given netuid. pub type WeightCommits = StorageDoubleMap< _, Twox64Concat, From a82a1f89ff81e9a412e4654974c8dd8b9434f738 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Tue, 29 Oct 2024 17:19:14 -0700 Subject: [PATCH 05/15] disable set weights rate limit for commit-reveal --- pallets/subtensor/src/subnets/weights.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 44c343d69..a449b8316 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -505,10 +505,12 @@ impl Pallet { // --- 9. Ensure the uid is not setting weights faster than the weights_set_rate_limit. let neuron_uid = Self::get_uid_for_net_and_hotkey(netuid, &hotkey)?; let current_block: u64 = Self::get_current_block_as_u64(); - ensure!( - Self::check_rate_limit(netuid, neuron_uid, current_block), - Error::::SettingWeightsTooFast - ); + if !Self::get_commit_reveal_weights_enabled(netuid) { + ensure!( + Self::check_rate_limit(netuid, neuron_uid, current_block), + Error::::SettingWeightsTooFast + ); + } // --- 10. Check that the neuron uid is an allowed validator permitted to set non-self weights. ensure!( From f5df0ba71921dc4dad47404480a6178379875906 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 30 Oct 2024 09:27:44 -0700 Subject: [PATCH 06/15] doc comments and lints --- pallets/subtensor/src/lib.rs | 2 +- pallets/subtensor/src/subnets/weights.rs | 17 ++++------------- 2 files changed, 5 insertions(+), 14 deletions(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 328a59f7e..e500b7e84 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -569,7 +569,7 @@ pub mod pallet { 0 } #[pallet::type_value] - /// Default minimum stake for weights. + /// Default Reveal Period Epochs pub fn DefaultRevealPeriodEpochs() -> u64 { 1 } diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index a449b8316..e56c36335 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -163,7 +163,6 @@ impl Pallet { // --- 6. After removing expired commits, check if any commits are left. if commits.is_empty() { - // No non-expired commits // Check if provided_hash matches any expired commits if expired_hashes.contains(&provided_hash) { return Err(Error::::ExpiredWeightCommit.into()); @@ -202,7 +201,6 @@ impl Pallet { Self::do_set_weights(origin, netuid, uids, values, version_key) } else { // --- 13. The provided_hash does not match any non-expired commits. - // Check if provided_hash matches any expired commits if expired_hashes.contains(&provided_hash) { Err(Error::::ExpiredWeightCommit.into()) } else { @@ -324,7 +322,6 @@ impl Pallet { } // --- 7. Validate all reveals first to ensure atomicity. - // This prevents partial updates if any reveal fails. for (_uids, _values, _version_key, provided_hash) in &reveals { // --- 7a. Check if the provided_hash is in the non-expired commits. if !commits @@ -346,10 +343,8 @@ impl Pallet { .ok_or(Error::::NoWeightsCommitFound)?; // --- 7d. Check if the commit is within the reveal window. - let current_block: u64 = Self::get_current_block_as_u64(); - let (_, _, first_reveal_block, last_reveal_block) = commit; ensure!( - current_block >= *first_reveal_block && current_block <= *last_reveal_block, + Self::is_reveal_block_range(netuid, commit.1), Error::::RevealTooEarly ); } @@ -366,14 +361,10 @@ impl Pallet { // --- 8c. Proceed to set the revealed weights. Self::do_set_weights(origin.clone(), netuid, uids, values, version_key)?; + } else if expired_hashes.contains(&provided_hash) { + return Err(Error::::ExpiredWeightCommit.into()); } else { - // This case should not occur as we've already validated the existence of the hash. - // However, to ensure safety, we handle it. - if expired_hashes.contains(&provided_hash) { - return Err(Error::::ExpiredWeightCommit.into()); - } else { - return Err(Error::::InvalidRevealCommitHashNotMatch.into()); - } + return Err(Error::::InvalidRevealCommitHashNotMatch.into()); } } From b07c1ffa923f68d5b1c21a30ea6c15d06a70af4c Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 30 Oct 2024 11:54:12 -0700 Subject: [PATCH 07/15] add events --- pallets/subtensor/src/macros/events.rs | 20 +++++++++++++++++ pallets/subtensor/src/subnets/weights.rs | 28 ++++++++++++++++++++---- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/pallets/subtensor/src/macros/events.rs b/pallets/subtensor/src/macros/events.rs index ac6b69012..f3b03684d 100644 --- a/pallets/subtensor/src/macros/events.rs +++ b/pallets/subtensor/src/macros/events.rs @@ -204,5 +204,25 @@ mod events { ColdkeySwapScheduleDurationSet(BlockNumberFor), /// The duration of dissolve network has been set DissolveNetworkScheduleDurationSet(BlockNumberFor), + /// Weights have been successfully committed. + /// + /// - **who**: The account ID of the user committing the weights. + /// - **netuid**: The network identifier. + /// - **commit_hash**: The hash representing the committed weights. + WeightsCommitted(T::AccountId, u16, H256), + + /// Weights have been successfully revealed. + /// + /// - **who**: The account ID of the user revealing the weights. + /// - **netuid**: The network identifier. + /// - **commit_hash**: The hash of the revealed weights. + WeightsRevealed(T::AccountId, u16, H256), + + /// Weights have been successfully batch revealed. + /// + /// - **who**: The account ID of the user revealing the weights. + /// - **netuid**: The network identifier. + /// - **revealed_hashes**: A vector of hashes representing each revealed weight set. + WeightsBatchRevealed(T::AccountId, u16, Vec), } } diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index e56c36335..082ef1638 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -73,7 +73,10 @@ impl Pallet { // --- 9. Store the updated queue back to storage. *maybe_commits = Some(commits); - // --- 10. Return ok. + // --- 10. Emit the WeightsCommitted event. + Self::deposit_event(Event::WeightsCommitted(who.clone(), netuid, commit_hash)); + + // --- 11. Return ok. Ok(()) }) } @@ -198,9 +201,15 @@ impl Pallet { } // --- 12. Proceed to set the revealed weights. - Self::do_set_weights(origin, netuid, uids, values, version_key) + Self::do_set_weights(origin, netuid, uids.clone(), values.clone(), version_key)?; + + // --- 13. Emit the WeightsRevealed event. + Self::deposit_event(Event::WeightsRevealed(who.clone(), netuid, provided_hash)); + + // --- 14. Return ok. + Ok(()) } else { - // --- 13. The provided_hash does not match any non-expired commits. + // --- 15. The provided_hash does not match any non-expired commits. if expired_hashes.contains(&provided_hash) { Err(Error::::ExpiredWeightCommit.into()) } else { @@ -302,6 +311,7 @@ impl Pallet { // --- 6. Prepare to collect all provided hashes and their corresponding reveals. let mut provided_hashes = Vec::new(); let mut reveals = Vec::new(); + let mut revealed_hashes: Vec = Vec::with_capacity(num_reveals); for ((uids, values), (salt, version_key)) in uids_list .into_iter() @@ -361,6 +371,9 @@ impl Pallet { // --- 8c. Proceed to set the revealed weights. Self::do_set_weights(origin.clone(), netuid, uids, values, version_key)?; + + // --- 8d. Collect the revealed hash. + revealed_hashes.push(provided_hash); } else if expired_hashes.contains(&provided_hash) { return Err(Error::::ExpiredWeightCommit.into()); } else { @@ -373,7 +386,14 @@ impl Pallet { *maybe_commits = None; } - // --- 10. Return ok. + // --- 10. Emit the WeightsBatchRevealed event with all revealed hashes. + Self::deposit_event(Event::WeightsBatchRevealed( + who.clone(), + netuid, + revealed_hashes, + )); + + // --- 11. Return ok. Ok(()) }) } From 96911e654d326e1469c7d2b744579025bb20f251 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 30 Oct 2024 12:42:16 -0700 Subject: [PATCH 08/15] revert hyperparam name change --- pallets/admin-utils/src/benchmarking.rs | 4 ++-- pallets/admin-utils/src/lib.rs | 12 ++++++------ pallets/admin-utils/src/weights.rs | 6 +++--- pallets/admin-utils/tests/tests.rs | 4 ++-- pallets/subtensor/src/rpc_info/subnet_info.rs | 6 +++--- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/pallets/admin-utils/src/benchmarking.rs b/pallets/admin-utils/src/benchmarking.rs index 606814ff0..65ccf629e 100644 --- a/pallets/admin-utils/src/benchmarking.rs +++ b/pallets/admin-utils/src/benchmarking.rs @@ -228,14 +228,14 @@ mod benchmarks { } #[benchmark] - fn sudo_set_commit_reveal_weights_periods() { + fn sudo_set_commit_reveal_weights_interval() { pallet_subtensor::Pallet::::init_new_network( 1u16, /*netuid*/ 1u16, /*sudo_tempo*/ ); #[extrinsic_call] - _(RawOrigin::Root, 1u16/*netuid*/, 3u64/*interval*/)/*set_commit_reveal_weights_periods()*/; + _(RawOrigin::Root, 1u16/*netuid*/, 3u64/*interval*/)/*sudo_set_commit_reveal_weights_interval()*/; } #[benchmark] diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index a287bb1b4..23121792a 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -1188,11 +1188,11 @@ pub mod pallet { /// # Weight /// Weight is handled by the `#[pallet::weight]` attribute. #[pallet::call_index(56)] - #[pallet::weight(T::WeightInfo::sudo_set_commit_reveal_weights_periods())] - pub fn sudo_set_commit_reveal_weights_periods( + #[pallet::weight(T::WeightInfo::sudo_set_commit_reveal_weights_interval())] + pub fn sudo_set_commit_reveal_weights_interval( origin: OriginFor, netuid: u16, - periods: u64, + interval: u64, ) -> DispatchResult { pallet_subtensor::Pallet::::ensure_subnet_owner_or_root(origin, netuid)?; @@ -1201,11 +1201,11 @@ pub mod pallet { Error::::SubnetDoesNotExist ); - pallet_subtensor::Pallet::::set_reveal_period(netuid, periods); + pallet_subtensor::Pallet::::set_reveal_period(netuid, interval); log::debug!( - "SetWeightCommitPeriods( netuid: {:?}, periods: {:?} ) ", + "SetWeightCommitInterval( netuid: {:?}, interval: {:?} ) ", netuid, - periods + interval ); Ok(()) } diff --git a/pallets/admin-utils/src/weights.rs b/pallets/admin-utils/src/weights.rs index db8752ff7..bda9c7916 100644 --- a/pallets/admin-utils/src/weights.rs +++ b/pallets/admin-utils/src/weights.rs @@ -60,7 +60,7 @@ pub trait WeightInfo { fn sudo_set_min_burn() -> Weight; fn sudo_set_network_registration_allowed() -> Weight; fn sudo_set_tempo() -> Weight; - fn sudo_set_commit_reveal_weights_periods() -> Weight; + fn sudo_set_commit_reveal_weights_interval() -> Weight; fn sudo_set_commit_reveal_weights_enabled() -> Weight; } @@ -413,7 +413,7 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } - fn sudo_set_commit_reveal_weights_periods() -> Weight { + fn sudo_set_commit_reveal_weights_interval() -> Weight { // Proof Size summary in bytes: // Measured: `456` // Estimated: `3921` @@ -781,7 +781,7 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } - fn sudo_set_commit_reveal_weights_periods() -> Weight { + fn sudo_set_commit_reveal_weights_interval() -> Weight { // -- Extrinsic Time -- // Model: // Time ~= 19.38 diff --git a/pallets/admin-utils/tests/tests.rs b/pallets/admin-utils/tests/tests.rs index d2c36e29f..442275052 100644 --- a/pallets/admin-utils/tests/tests.rs +++ b/pallets/admin-utils/tests/tests.rs @@ -1414,7 +1414,7 @@ fn test_sudo_set_dissolve_network_schedule_duration() { } #[test] -fn sudo_set_commit_reveal_weights_periods() { +fn sudo_set_commit_reveal_weights_interval() { new_test_ext().execute_with(|| { let netuid: u16 = 1; add_network(netuid, 10); @@ -1422,7 +1422,7 @@ fn sudo_set_commit_reveal_weights_periods() { let to_be_set = 55; let init_value = SubtensorModule::get_reveal_period(netuid); - assert_ok!(AdminUtils::sudo_set_commit_reveal_weights_periods( + assert_ok!(AdminUtils::sudo_set_commit_reveal_weights_interval( <::RuntimeOrigin>::root(), netuid, to_be_set diff --git a/pallets/subtensor/src/rpc_info/subnet_info.rs b/pallets/subtensor/src/rpc_info/subnet_info.rs index 8c79db03a..bdd420821 100644 --- a/pallets/subtensor/src/rpc_info/subnet_info.rs +++ b/pallets/subtensor/src/rpc_info/subnet_info.rs @@ -51,7 +51,7 @@ pub struct SubnetInfov2 { identity: Option, } -#[freeze_struct("4ceb81dfe8a8f96d")] +#[freeze_struct("55b472510f10e76a")] #[derive(Decode, Encode, PartialEq, Eq, Clone, Debug)] pub struct SubnetHyperparams { rho: Compact, @@ -76,7 +76,7 @@ pub struct SubnetHyperparams { max_validators: Compact, adjustment_alpha: Compact, difficulty: Compact, - commit_reveal_periods: Compact, + commit_reveal_weights_interval: Compact, commit_reveal_weights_enabled: bool, alpha_high: Compact, alpha_low: Compact, @@ -280,7 +280,7 @@ impl Pallet { max_validators: max_validators.into(), adjustment_alpha: adjustment_alpha.into(), difficulty: difficulty.into(), - commit_reveal_periods: commit_reveal_periods.into(), + commit_reveal_weights_interval: commit_reveal_periods.into(), commit_reveal_weights_enabled, alpha_high: alpha_high.into(), alpha_low: alpha_low.into(), From 498d30ae0f419c1467172e60ce894a01cddbb611 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 30 Oct 2024 14:31:16 -0700 Subject: [PATCH 09/15] add set weights rate limit to commit-reveal --- pallets/subtensor/src/macros/errors.rs | 2 + pallets/subtensor/src/subnets/weights.rs | 22 ++++++- pallets/subtensor/tests/weights.rs | 73 +++++++++++++++++++++++- 3 files changed, 92 insertions(+), 5 deletions(-) diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 1e4bf9ae0..aab849994 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -188,5 +188,7 @@ mod errors { RevealTooEarly, /// Attempted to batch reveal weights with mismatched vector input lenghts. InputLengthsUnequal, + /// A transactor exceeded the rate limit for setting weights. + CommittingWeightsTooFast, } } diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 082ef1638..e252089bc 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -39,8 +39,19 @@ impl Pallet { Error::::CommitRevealDisabled ); - // --- 3. Calculate the reveal blocks based on tempo and reveal period. + ensure!( + Self::is_hotkey_registered_on_network(netuid, &who), + Error::::HotKeyNotRegisteredInSubNet + ); + let commit_block: u64 = Self::get_current_block_as_u64(); + let neuron_uid: u16 = Self::get_uid_for_net_and_hotkey(netuid, &who)?; + ensure!( + Self::check_rate_limit(netuid, neuron_uid, commit_block), + Error::::CommittingWeightsTooFast + ); + + // --- 3. Calculate the reveal blocks based on tempo and reveal period. let (first_reveal_block, last_reveal_block) = Self::get_reveal_blocks(netuid, commit_block); // --- 4. Mutate the WeightCommits to retrieve existing commits for the user. @@ -76,7 +87,10 @@ impl Pallet { // --- 10. Emit the WeightsCommitted event. Self::deposit_event(Event::WeightsCommitted(who.clone(), netuid, commit_hash)); - // --- 11. Return ok. + // --- 11. Set last update for the UID + Self::set_last_update_for_uid(netuid, neuron_uid, commit_block); + + // --- 12. Return ok. Ok(()) }) } @@ -563,7 +577,9 @@ impl Pallet { Weights::::insert(netuid, neuron_uid, zipped_weights); // --- 18. Set the activity for the weights on this network. - Self::set_last_update_for_uid(netuid, neuron_uid, current_block); + if !Self::get_commit_reveal_weights_enabled(netuid) { + Self::set_last_update_for_uid(netuid, neuron_uid, current_block); + } // --- 19. Emit the tracking event. log::debug!( diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 0625e1e20..0f2f1a2d6 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -1815,7 +1815,7 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { #[test] fn test_tempo_change_during_commit_reveal_process() { - new_test_ext(1).execute_with(|| { + new_test_ext(0).execute_with(|| { let netuid: u16 = 1; let uids: Vec = vec![0, 1]; let weight_values: Vec = vec![10, 10]; @@ -1832,7 +1832,7 @@ fn test_tempo_change_during_commit_reveal_process() { version_key, )); - System::set_block_number(1); + System::set_block_number(0); let tempo: u16 = 100; add_network(netuid, tempo, 0); @@ -4059,3 +4059,72 @@ fn test_get_reveal_blocks() { ); }) } + +#[test] +fn test_commit_weights_rate_limit() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let uids: Vec = vec![0, 1]; + let weight_values: Vec = vec![10, 10]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; + let version_key: u64 = 0; + let hotkey: U256 = U256::from(1); + + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + System::set_block_number(11); + + let tempo: u16 = 5; + add_network(netuid, tempo, 0); + + register_ok_neuron(netuid, U256::from(3), U256::from(4), 300_000); + register_ok_neuron(netuid, U256::from(1), U256::from(2), 100_000); + SubtensorModule::set_weights_set_rate_limit(netuid, 10); // Rate limit is 10 blocks + SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + + let neuron_uid = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey).unwrap(); + SubtensorModule::set_last_update_for_uid(netuid, neuron_uid, 0); + + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + + let new_salt: Vec = vec![9; 8]; + let new_commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + new_salt.clone(), + version_key, + )); + assert_err!( + SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, new_commit_hash), + Error::::CommittingWeightsTooFast + ); + + step_block(5); + assert_err!( + SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, new_commit_hash), + Error::::CommittingWeightsTooFast + ); + + step_block(5); // Current block is now 21 + + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + new_commit_hash + )); + }); +} From f95fb5b3c955d711cdb862e07fc9eb803a3b528d Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 30 Oct 2024 15:19:47 -0700 Subject: [PATCH 10/15] clippy --- pallets/subtensor/tests/weights.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 0f2f1a2d6..30269a498 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -4090,7 +4090,8 @@ fn test_commit_weights_rate_limit() { SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - let neuron_uid = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey).unwrap(); + let neuron_uid = + SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey).expect("expected uid"); SubtensorModule::set_last_update_for_uid(netuid, neuron_uid, 0); assert_ok!(SubtensorModule::commit_weights( From 8a97a128a103eb27231cdafbaa093537e62c278d Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 30 Oct 2024 15:25:23 -0700 Subject: [PATCH 11/15] use new index previously deployed to testnet with a different name for that index --- pallets/admin-utils/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 23121792a..85c7ef62c 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -1187,7 +1187,7 @@ pub mod pallet { /// /// # Weight /// Weight is handled by the `#[pallet::weight]` attribute. - #[pallet::call_index(56)] + #[pallet::call_index(57)] #[pallet::weight(T::WeightInfo::sudo_set_commit_reveal_weights_interval())] pub fn sudo_set_commit_reveal_weights_interval( origin: OriginFor, From ba7e34c42458050ba6e71028f35597fb4a794c02 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Thu, 31 Oct 2024 08:32:49 -0700 Subject: [PATCH 12/15] update comments --- pallets/subtensor/src/subnets/weights.rs | 46 ++++++++++++++---------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index e252089bc..87042f456 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -19,61 +19,71 @@ impl Pallet { /// /// # Raises: /// * `CommitRevealDisabled`: - /// - Attempting to commit when the commit-reveal mechanism is disabled. + /// - Raised if commit-reveal is disabled for the specified network. + /// + /// * `HotKeyNotRegisteredInSubNet`: + /// - Raised if the hotkey is not registered on the specified network. + /// + /// * `CommittingWeightsTooFast`: + /// - Raised if the hotkey's commit rate exceeds the permitted limit. /// /// * `TooManyUnrevealedCommits`: - /// - Attempting to commit when the user has more than the allowed limit of unrevealed commits. + /// - Raised if the hotkey has reached the maximum number of unrevealed commits. + /// + /// # Events: + /// * `WeightsCommitted`: + /// - Emitted upon successfully storing the weight hash. pub fn do_commit_weights( origin: T::RuntimeOrigin, netuid: u16, commit_hash: H256, ) -> DispatchResult { - // --- 1. Check the caller's signature (hotkey). + // 1. Verify the caller's signature (hotkey). let who = ensure_signed(origin)?; - log::debug!("do_commit_weights( hotkey:{:?} netuid:{:?})", who, netuid); + log::debug!("do_commit_weights(hotkey: {:?}, netuid: {:?})", who, netuid); - // --- 2. Ensure commit-reveal is enabled for the network. + // 2. Ensure commit-reveal is enabled. ensure!( Self::get_commit_reveal_weights_enabled(netuid), Error::::CommitRevealDisabled ); + // 3. Ensure the hotkey is registered on the network. ensure!( Self::is_hotkey_registered_on_network(netuid, &who), Error::::HotKeyNotRegisteredInSubNet ); - let commit_block: u64 = Self::get_current_block_as_u64(); - let neuron_uid: u16 = Self::get_uid_for_net_and_hotkey(netuid, &who)?; + // 4. Check that the commit rate does not exceed the allowed frequency. + let commit_block = Self::get_current_block_as_u64(); + let neuron_uid = Self::get_uid_for_net_and_hotkey(netuid, &who)?; ensure!( Self::check_rate_limit(netuid, neuron_uid, commit_block), Error::::CommittingWeightsTooFast ); - // --- 3. Calculate the reveal blocks based on tempo and reveal period. + // 5. Calculate the reveal blocks based on network tempo and reveal period. let (first_reveal_block, last_reveal_block) = Self::get_reveal_blocks(netuid, commit_block); - // --- 4. Mutate the WeightCommits to retrieve existing commits for the user. + // 6. Retrieve or initialize the VecDeque of commits for the hotkey. WeightCommits::::try_mutate(netuid, &who, |maybe_commits| -> DispatchResult { - // --- 5. Take the existing commits or create a new VecDeque. let mut commits: VecDeque<(H256, u64, u64, u64)> = maybe_commits.take().unwrap_or_default(); - // --- 6. Remove any expired commits from the front of the queue. + // 7. Remove any expired commits from the front of the queue. while let Some((_, commit_block_existing, _, _)) = commits.front() { if Self::is_commit_expired(netuid, *commit_block_existing) { - // Remove the expired commit commits.pop_front(); } else { break; } } - // --- 7. Check if the current number of unrevealed commits is within the allowed limit. + // 8. Verify that the number of unrevealed commits is within the allowed limit. ensure!(commits.len() < 10, Error::::TooManyUnrevealedCommits); - // --- 8. Append the new commit to the queue. + // 9. Append the new commit with calculated reveal blocks. commits.push_back(( commit_hash, commit_block, @@ -81,16 +91,16 @@ impl Pallet { last_reveal_block, )); - // --- 9. Store the updated queue back to storage. + // 10. Store the updated commits queue back to storage. *maybe_commits = Some(commits); - // --- 10. Emit the WeightsCommitted event. + // 11. Emit the WeightsCommitted event Self::deposit_event(Event::WeightsCommitted(who.clone(), netuid, commit_hash)); - // --- 11. Set last update for the UID + // 12. Update the last commit block for the hotkey's UID. Self::set_last_update_for_uid(netuid, neuron_uid, commit_block); - // --- 12. Return ok. + // 13. Return success. Ok(()) }) } From 93c3f9c0730926cce9cde265838fd907111e489b Mon Sep 17 00:00:00 2001 From: johnreedv Date: Thu, 31 Oct 2024 08:58:16 -0700 Subject: [PATCH 13/15] expand test --- pallets/subtensor/tests/weights.rs | 59 ++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 30269a498..7dbeba288 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -4127,5 +4127,64 @@ fn test_commit_weights_rate_limit() { netuid, new_commit_hash )); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); + let weights_keys: Vec = vec![0]; + let weight_values: Vec = vec![1]; + + assert_err!( + SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), + netuid, + weights_keys.clone(), + weight_values.clone(), + 0 + ), + Error::::SettingWeightsTooFast + ); + + step_block(10); + + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), + netuid, + weights_keys.clone(), + weight_values.clone(), + 0 + )); + + assert_err!( + SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), + netuid, + weights_keys.clone(), + weight_values.clone(), + 0 + ), + Error::::SettingWeightsTooFast + ); + + step_block(5); + + assert_err!( + SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), + netuid, + weights_keys.clone(), + weight_values.clone(), + 0 + ), + Error::::SettingWeightsTooFast + ); + + step_block(5); + + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), + netuid, + weights_keys.clone(), + weight_values.clone(), + 0 + )); }); } From 08e62d0b025fae278bdd6bef0e0182ad35937c40 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Thu, 31 Oct 2024 09:57:22 -0700 Subject: [PATCH 14/15] bump migration version --- pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs b/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs index cd93842c9..b8b831b61 100644 --- a/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs +++ b/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs @@ -5,7 +5,7 @@ use scale_info::prelude::string::String; use sp_io::{hashing::twox_128, storage::clear_prefix, KillStorageResult}; pub fn migrate_commit_reveal_2() -> Weight { - let migration_name = b"migrate_commit_reveal_2".to_vec(); + let migration_name = b"migrate_commit_reveal_2_v2".to_vec(); let mut weight = T::DbWeight::get().reads(1); if HasMigrationRun::::get(&migration_name) { From bc08dc855ff071da4e07ed7d960b69a906f88ab7 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Thu, 31 Oct 2024 09:58:50 -0700 Subject: [PATCH 15/15] update migration test --- pallets/subtensor/tests/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index 1317bfb0f..4ddef882c 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -446,7 +446,7 @@ fn test_migrate_commit_reveal_2() { // ------------------------------ // Step 1: Simulate Old Storage Entries // ------------------------------ - const MIGRATION_NAME: &str = "migrate_commit_reveal_2"; + const MIGRATION_NAME: &str = "migrate_commit_reveal_2_v2"; let pallet_prefix = twox_128("SubtensorModule".as_bytes()); let storage_prefix_interval = twox_128("WeightCommitRevealInterval".as_bytes());