From fb0f5452df76cfac1f681f6ade33af06c4a0da00 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Fri, 4 Oct 2024 16:41:21 -0700 Subject: [PATCH 01/33] make commit-reveal based on subnet tempo --- pallets/admin-utils/src/benchmarking.rs | 11 - pallets/admin-utils/src/lib.rs | 26 - pallets/admin-utils/tests/tests.rs | 23 - pallets/subtensor/src/benchmarks.rs | 1 - pallets/subtensor/src/lib.rs | 9 - pallets/subtensor/src/macros/errors.rs | 2 + pallets/subtensor/src/rpc_info/subnet_info.rs | 5 +- pallets/subtensor/src/subnets/weights.rs | 51 +- pallets/subtensor/src/utils/misc.rs | 7 - pallets/subtensor/tests/weights.rs | 447 +++++++----------- 10 files changed, 192 insertions(+), 390 deletions(-) diff --git a/pallets/admin-utils/src/benchmarking.rs b/pallets/admin-utils/src/benchmarking.rs index 7515525f0..3d8b962f6 100644 --- a/pallets/admin-utils/src/benchmarking.rs +++ b/pallets/admin-utils/src/benchmarking.rs @@ -227,17 +227,6 @@ mod benchmarks { _(RawOrigin::Root, 1u16/*netuid*/, 1u16/*tempo*/)/*sudo_set_tempo*/; } - #[benchmark] - 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_interval()*/; - } - #[benchmark] fn sudo_set_commit_reveal_weights_enabled() { pallet_subtensor::Pallet::::init_new_network( diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 3e06b822e..501122a8e 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -960,32 +960,6 @@ pub mod pallet { Ok(()) } - /// The extrinsic sets the commit/reveal interval for a subnet. - /// It is only callable by the root account or subnet owner. - /// The extrinsic will call the Subtensor pallet to set the interval. - #[pallet::call_index(48)] - #[pallet::weight(T::WeightInfo::sudo_set_commit_reveal_weights_interval())] - pub fn sudo_set_commit_reveal_weights_interval( - origin: OriginFor, - netuid: u16, - interval: u64, - ) -> DispatchResult { - pallet_subtensor::Pallet::::ensure_subnet_owner_or_root(origin, netuid)?; - - ensure!( - pallet_subtensor::Pallet::::if_subnet_exist(netuid), - Error::::SubnetDoesNotExist - ); - - pallet_subtensor::Pallet::::set_commit_reveal_weights_interval(netuid, interval); - log::debug!( - "SetWeightCommitInterval( netuid: {:?}, interval: {:?} ) ", - netuid, - interval - ); - Ok(()) - } - /// The extrinsic enabled/disables commit/reaveal for a given subnet. /// It is only callable by the root account or subnet owner. /// The extrinsic will call the Subtensor pallet to set the value. diff --git a/pallets/admin-utils/tests/tests.rs b/pallets/admin-utils/tests/tests.rs index 8ab85f177..746dfa6f5 100644 --- a/pallets/admin-utils/tests/tests.rs +++ b/pallets/admin-utils/tests/tests.rs @@ -1113,29 +1113,6 @@ fn test_sudo_set_min_delegate_take() { }); } -#[test] -fn test_sudo_set_weight_commit_interval() { - new_test_ext().execute_with(|| { - let netuid: u16 = 1; - add_network(netuid, 10); - - let to_be_set = 55; - let init_value = SubtensorModule::get_commit_reveal_weights_interval(netuid); - - assert_ok!(AdminUtils::sudo_set_commit_reveal_weights_interval( - <::RuntimeOrigin>::root(), - netuid, - to_be_set - )); - - assert!(init_value != to_be_set); - assert_eq!( - SubtensorModule::get_commit_reveal_weights_interval(netuid), - to_be_set - ); - }); -} - #[test] fn test_sudo_set_commit_reveal_weights_enabled() { new_test_ext().execute_with(|| { diff --git a/pallets/subtensor/src/benchmarks.rs b/pallets/subtensor/src/benchmarks.rs index 4915bb3ac..bd48676b6 100644 --- a/pallets/subtensor/src/benchmarks.rs +++ b/pallets/subtensor/src/benchmarks.rs @@ -416,7 +416,6 @@ reveal_weights { ); Subtensor::::set_validator_permit_for_uid(netuid, 0, true); - Subtensor::::set_commit_reveal_weights_interval(netuid, 0); let commit_hash: H256 = BlakeTwo256::hash_of(&( hotkey.clone(), diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index cc3d7d025..92156be94 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -629,11 +629,6 @@ pub mod pallet { T::InitialServingRateLimit::get() } #[pallet::type_value] - /// Default value for weight commit reveal interval. - pub fn DefaultWeightCommitRevealInterval() -> u64 { - 1000 - } - #[pallet::type_value] /// Default value for weight commit/reveal enabled. pub fn DefaultCommitRevealWeightsEnabled() -> bool { false @@ -1032,10 +1027,6 @@ pub mod pallet { StorageMap<_, Identity, u16, u64, ValueQuery, DefaultAdjustmentAlpha>; #[pallet::storage] /// --- MAP ( netuid ) --> interval - pub type WeightCommitRevealInterval = - StorageMap<_, Identity, u16, u64, ValueQuery, DefaultWeightCommitRevealInterval>; - #[pallet::storage] - /// --- MAP ( netuid ) --> interval pub type CommitRevealWeightsEnabled = StorageMap<_, Identity, u16, bool, ValueQuery, DefaultCommitRevealWeightsEnabled>; #[pallet::storage] diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 22a0a6f89..9fd7c5fea 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -184,5 +184,7 @@ mod errors { TxChildkeyTakeRateLimitExceeded, /// Invalid identity. InvalidIdentity, + /// invalid reaveal epoch + InvalidRevealEpoch, } } diff --git a/pallets/subtensor/src/rpc_info/subnet_info.rs b/pallets/subtensor/src/rpc_info/subnet_info.rs index 9b22e0401..312a20723 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("55b472510f10e76a")] +#[freeze_struct("e8abe48842dcc8c4")] #[derive(Decode, Encode, PartialEq, Eq, Clone, Debug)] pub struct SubnetHyperparams { rho: Compact, @@ -76,7 +76,6 @@ pub struct SubnetHyperparams { max_validators: Compact, adjustment_alpha: Compact, difficulty: Compact, - commit_reveal_weights_interval: Compact, commit_reveal_weights_enabled: bool, alpha_high: Compact, alpha_low: Compact, @@ -252,7 +251,6 @@ impl Pallet { let max_validators = Self::get_max_allowed_validators(netuid); let adjustment_alpha = Self::get_adjustment_alpha(netuid); let difficulty = Self::get_difficulty_as_u64(netuid); - let commit_reveal_weights_interval = Self::get_commit_reveal_weights_interval(netuid); let commit_reveal_weights_enabled = Self::get_commit_reveal_weights_enabled(netuid); let liquid_alpha_enabled = Self::get_liquid_alpha_enabled(netuid); let (alpha_low, alpha_high): (u16, u16) = Self::get_alpha_values(netuid); @@ -280,7 +278,6 @@ impl Pallet { max_validators: max_validators.into(), adjustment_alpha: adjustment_alpha.into(), difficulty: difficulty.into(), - commit_reveal_weights_interval: commit_reveal_weights_interval.into(), commit_reveal_weights_enabled, alpha_high: alpha_high.into(), alpha_low: alpha_low.into(), diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 1a53e44cc..85f560f68 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -119,6 +119,8 @@ impl Pallet { Error::::InvalidRevealCommitHashNotMatch ); + *maybe_commit = None; + Self::do_set_weights(origin, netuid, uids, values, version_key) }) } @@ -452,50 +454,43 @@ impl Pallet { uids.len() <= subnetwork_n as usize } - #[allow(clippy::arithmetic_side_effects)] pub fn can_commit(netuid: u16, who: &T::AccountId) -> bool { if let Some((_hash, commit_block)) = WeightCommits::::get(netuid, who) { - let interval: u64 = Self::get_commit_reveal_weights_interval(netuid); - if interval == 0 { - return true; //prevent division by 0 - } - let current_block: u64 = Self::get_current_block_as_u64(); - let interval_start: u64 = current_block.saturating_sub(current_block % interval); - let last_commit_interval_start: u64 = - commit_block.saturating_sub(commit_block % interval); - - // Allow commit if we're within the interval bounds - if current_block <= interval_start.saturating_add(interval) - && interval_start > last_commit_interval_start - { + let current_epoch: u64 = Self::get_epoch_index(netuid, current_block); + let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); + + // Allow commit if we're in a new epoch + if current_epoch > commit_epoch { return true; } - false } else { true } } - #[allow(clippy::arithmetic_side_effects)] pub fn is_reveal_block_range(netuid: u16, commit_block: u64) -> bool { - let interval: u64 = Self::get_commit_reveal_weights_interval(netuid); - if interval == 0 { - return true; //prevent division by 0 - } - - let commit_interval_start: u64 = commit_block.saturating_sub(commit_block % interval); // Find the start of the interval in which the commit occurred - let reveal_interval_start: u64 = commit_interval_start.saturating_add(interval); // Start of the next interval after the commit interval let current_block: u64 = Self::get_current_block_as_u64(); + let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); + let current_epoch: u64 = Self::get_epoch_index(netuid, current_block); - // Allow reveal if the current block is within the interval following the commit's interval - if current_block >= reveal_interval_start - && current_block < reveal_interval_start.saturating_add(interval) - { + // Allow reveal if the current epoch is immediately after the commit's epoch + if current_epoch == commit_epoch + 1 { return true; } - false } + + pub fn get_epoch_index(netuid: u16, block_number: u64) -> u64 { + let tempo = Self::get_tempo(netuid); + if tempo == 0 { + return 0; + } + let tempo_plus_one = (tempo as u64).saturating_add(1); + let netuid_plus_one = (netuid as u64).saturating_add(1); + let epoch_index = + (block_number.saturating_add(netuid_plus_one)).saturating_div(tempo_plus_one); + epoch_index + } } diff --git a/pallets/subtensor/src/utils/misc.rs b/pallets/subtensor/src/utils/misc.rs index 76546a1a2..57cc38786 100644 --- a/pallets/subtensor/src/utils/misc.rs +++ b/pallets/subtensor/src/utils/misc.rs @@ -486,13 +486,6 @@ impl Pallet { Kappa::::insert(netuid, kappa); Self::deposit_event(Event::KappaSet(netuid, kappa)); } - - pub fn get_commit_reveal_weights_interval(netuid: u16) -> u64 { - WeightCommitRevealInterval::::get(netuid) - } - pub fn set_commit_reveal_weights_interval(netuid: u16, interval: u64) { - WeightCommitRevealInterval::::set(netuid, interval); - } pub fn get_commit_reveal_weights_enabled(netuid: u16) -> bool { CommitRevealWeightsEnabled::::get(netuid) } diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 214e3add0..27a43de4c 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -402,12 +402,17 @@ fn test_set_weights_is_root_error() { let uids = vec![0]; let weights = vec![1]; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let hotkey = U256::from(1); assert_err!( - commit_reveal_set_weights(hotkey, root_netuid, uids, weights, salt, version_key), + SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), + root_netuid, + uids.clone(), + weights.clone(), + version_key, + ), Error::::CanNotSetRootNetworkWeights ); }); @@ -430,14 +435,12 @@ fn test_weights_err_no_validator_permit() { let weights_keys: Vec = vec![1, 2]; let weight_values: Vec = vec![1, 2]; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; - let result = commit_reveal_set_weights( - hotkey_account_id, + let result = SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey_account_id), netuid, weights_keys, weight_values, - salt.clone(), 0, ); assert_eq!(result, Err(Error::::NeuronNoValidatorPermit.into())); @@ -448,12 +451,11 @@ fn test_weights_err_no_validator_permit() { SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey_account_id) .expect("Not registered."); SubtensorModule::set_validator_permit_for_uid(netuid, neuron_uid, true); - let result = commit_reveal_set_weights( - hotkey_account_id, + let result = SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey_account_id), netuid, weights_keys, weight_values, - salt, 0, ); assert_ok!(result); @@ -470,7 +472,7 @@ fn test_set_weights_min_stake_failed() { let version_key: u64 = 0; let hotkey = U256::from(0); let coldkey = U256::from(0); - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; + add_network(netuid, 0, 0); register_ok_neuron(netuid, hotkey, coldkey, 2143124); SubtensorModule::set_weights_min_stake(20_000_000_000_000); @@ -486,24 +488,22 @@ fn test_set_weights_min_stake_failed() { // Check that it fails at the pallet level. SubtensorModule::set_weights_min_stake(100_000_000_000_000); assert_eq!( - commit_reveal_set_weights( - hotkey, + SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), netuid, dests.clone(), weights.clone(), - salt.clone(), - version_key + version_key, ), Err(Error::::NotEnoughStakeToSetWeights.into()) ); // Now passes SubtensorModule::increase_stake_on_hotkey_account(&hotkey, 100_000_000_000_000); - assert_ok!(commit_reveal_set_weights( - hotkey, + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), netuid, dests.clone(), weights.clone(), - salt.clone(), version_key )); }); @@ -517,7 +517,7 @@ fn test_weights_version_key() { let coldkey = U256::from(66); let netuid0: u16 = 1; let netuid1: u16 = 2; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; + add_network(netuid0, 0, 0); add_network(netuid1, 0, 0); register_ok_neuron(netuid0, hotkey, coldkey, 2143124); @@ -525,20 +525,18 @@ fn test_weights_version_key() { let weights_keys: Vec = vec![0]; let weight_values: Vec = vec![1]; - assert_ok!(commit_reveal_set_weights( - hotkey, + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), netuid0, weights_keys.clone(), weight_values.clone(), - salt.clone(), 0 )); - assert_ok!(commit_reveal_set_weights( - hotkey, + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), netuid1, weights_keys.clone(), weight_values.clone(), - salt.clone(), 0 )); @@ -549,42 +547,38 @@ fn test_weights_version_key() { SubtensorModule::set_weights_version_key(netuid1, key1); // Setting works with version key. - assert_ok!(commit_reveal_set_weights( - hotkey, + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), netuid0, weights_keys.clone(), weight_values.clone(), - salt.clone(), key0 )); - assert_ok!(commit_reveal_set_weights( - hotkey, + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), netuid1, weights_keys.clone(), weight_values.clone(), - salt.clone(), key1 )); // validator:20313 >= network:12312 (accepted: validator newer) - assert_ok!(commit_reveal_set_weights( - hotkey, + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), netuid0, weights_keys.clone(), weight_values.clone(), - salt.clone(), key1 )); // Setting fails with incorrect keys. // validator:12312 < network:20313 (rejected: validator not updated) assert_eq!( - commit_reveal_set_weights( - hotkey, + SubtensorModule::set_weights( + RuntimeOrigin::signed(hotkey), netuid1, weights_keys.clone(), weight_values.clone(), - salt.clone(), key0 ), Err(Error::::IncorrectWeightVersionKey.into()) @@ -893,19 +887,23 @@ fn test_set_weight_not_enough_values() { // Should fail because we are only setting a single value and its not the self weight. let weight_keys: Vec = vec![1]; // not weight. let weight_values: Vec = vec![88]; // random value. - let result = - commit_reveal_set_weights(account_id, 1, weight_keys, weight_values, salt.clone(), 0); + let result = SubtensorModule::set_weights( + RuntimeOrigin::signed(account_id), + 1, + weight_keys, + weight_values, + 0, + ); assert_eq!(result, Err(Error::::WeightVecLengthIsLow.into())); // Shouldnt fail because we setting a single value but it is the self weight. let weight_keys: Vec = vec![0]; // self weight. let weight_values: Vec = vec![88]; // random value. - assert_ok!(commit_reveal_set_weights( - account_id, + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(account_id), 1, weight_keys, weight_values, - salt.clone(), 0 )); @@ -1390,24 +1388,39 @@ fn test_commit_reveal_weights_ok() { version_key, )); - add_network(netuid, 0, 0); - register_ok_neuron(netuid, U256::from(3), U256::from(4), 300000); - register_ok_neuron(netuid, U256::from(1), U256::from(2), 100000); + // Set block number to 0 + System::set_block_number(0); + + // Add network with tempo 5 + let tempo: u16 = 5; + add_network(netuid, tempo, 0); + + // Register neurons and set up configurations + 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_interval(netuid, 5); + // Enable commit/reveal SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + // Commit at block 0 assert_ok!(SubtensorModule::commit_weights( RuntimeOrigin::signed(hotkey), netuid, commit_hash )); - step_block(5); + // Calculate blocks to next epoch and advance + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + // Reveal in the next epoch assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, @@ -1420,7 +1433,7 @@ fn test_commit_reveal_weights_ok() { } #[test] -fn test_commit_reveal_interval() { +fn test_commit_reveal_tempo_interval() { new_test_ext(1).execute_with(|| { let netuid: u16 = 1; let uids: Vec = vec![0, 1]; @@ -1438,26 +1451,34 @@ fn test_commit_reveal_interval() { version_key, )); - add_network(netuid, 0, 0); - register_ok_neuron(netuid, U256::from(3), U256::from(4), 300000); - register_ok_neuron(netuid, U256::from(1), U256::from(2), 100000); + System::set_block_number(0); + + let tempo: u16 = 100; + 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, 5); SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); - SubtensorModule::set_commit_reveal_weights_interval(netuid, 100); + // Enable commit/reveal SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - System::set_block_number(0); + // Commit at block 0 assert_ok!(SubtensorModule::commit_weights( RuntimeOrigin::signed(hotkey), netuid, commit_hash )); + + // Attempt to commit again in the same epoch, should fail assert_err!( SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash), Error::::WeightsCommitNotAllowed ); + + // Attempt to reveal in the same epoch, should fail assert_err!( SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -1469,48 +1490,15 @@ fn test_commit_reveal_interval() { ), Error::::InvalidRevealCommitTempo ); - step_block(99); - assert_err!( - SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash), - Error::::WeightsCommitNotAllowed - ); - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt.clone(), - version_key, - ), - Error::::InvalidRevealCommitTempo - ); - step_block(1); - assert_ok!(SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt.clone(), - version_key, - )); - assert_ok!(SubtensorModule::commit_weights( - RuntimeOrigin::signed(hotkey), + + // Calculate blocks to next epoch and advance + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( netuid, - commit_hash - )); - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt.clone(), - version_key, - ), - Error::::InvalidRevealCommitTempo + tempo, + SubtensorModule::get_current_block_as_u64(), ); - step_block(100); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, @@ -1520,13 +1508,8 @@ fn test_commit_reveal_interval() { version_key, )); - // Testing that if you miss the next tempo you cannot reveal it. - assert_ok!(SubtensorModule::commit_weights( - RuntimeOrigin::signed(hotkey), - netuid, - commit_hash - )); - step_block(205); + step_block(6); + assert_err!( SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -1536,49 +1519,41 @@ fn test_commit_reveal_interval() { salt.clone(), version_key, ), - Error::::InvalidRevealCommitTempo + Error::::NoWeightsCommitFound ); - // Testing when you commit but do not reveal until later intervals assert_ok!(SubtensorModule::commit_weights( RuntimeOrigin::signed(hotkey), netuid, commit_hash )); - step_block(425); - let commit_hash_2: H256 = BlakeTwo256::hash_of(&( - hotkey, + + // step two epochs + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( netuid, - uids.clone(), - weight_values.clone(), - salt.clone(), - version_key + 1, - )); - assert_ok!(SubtensorModule::commit_weights( - RuntimeOrigin::signed(hotkey), + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( netuid, - commit_hash_2 - )); - step_block(100); + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + // Attempt to reveal previous commit in the new epoch, should fail with `InvalidRevealCommitTempo` assert_err!( SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, - uids.clone(), - weight_values.clone(), - salt.clone(), + uids, + weight_values, + salt, version_key, ), - Error::::InvalidRevealCommitHashNotMatch + Error::::InvalidRevealCommitTempo ); - assert_ok!(SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt.clone(), - version_key + 1, - )); }); } @@ -1589,17 +1564,19 @@ fn test_commit_reveal_hash() { 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 bad_salt: Vec = vec![0, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let hotkey: U256 = U256::from(1); - add_network(netuid, 0, 0); - register_ok_neuron(netuid, U256::from(3), U256::from(4), 300000); - register_ok_neuron(netuid, U256::from(1), U256::from(2), 100000); + add_network(netuid, 5, 0); + System::set_block_number(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, 5); SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); - SubtensorModule::set_commit_reveal_weights_interval(netuid, 5); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); let commit_hash: H256 = BlakeTwo256::hash_of(&( @@ -1617,8 +1594,15 @@ fn test_commit_reveal_hash() { commit_hash )); - step_block(5); + // Calculate blocks to next epoch and advance + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + SubtensorModule::get_tempo(netuid), + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + // Attempt to reveal with incorrect data, should fail assert_err!( SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -1630,46 +1614,26 @@ fn test_commit_reveal_hash() { ), Error::::InvalidRevealCommitHashNotMatch ); + assert_err!( SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, uids.clone(), weight_values.clone(), - salt.clone(), - 7, - ), - Error::::InvalidRevealCommitHashNotMatch - ); - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - vec![10, 9], - salt.clone(), + bad_salt.clone(), version_key, ), Error::::InvalidRevealCommitHashNotMatch ); - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - vec![0, 1, 2], - vec![10, 10, 33], - salt.clone(), - 9, - ), - Error::::InvalidRevealCommitHashNotMatch - ); + // Correct reveal, should succeed assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, uids, weight_values, - salt.clone(), + salt, version_key, )); }); @@ -1694,74 +1658,49 @@ fn test_commit_reveal_disabled_or_enabled() { version_key, )); - add_network(netuid, 0, 0); - register_ok_neuron(netuid, U256::from(3), U256::from(4), 300000); - register_ok_neuron(netuid, U256::from(1), U256::from(2), 100000); + add_network(netuid, 5, 0); + System::set_block_number(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, 5); SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); - SubtensorModule::set_commit_reveal_weights_interval(netuid, 5); + // Disable commit/reveal SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); + // Attempt to commit, should fail assert_err!( SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash), Error::::CommitRevealDisabled ); - step_block(5); - - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt.clone(), - version_key, - ), - Error::::CommitRevealDisabled - ); - - SubtensorModule::set_commit_reveal_weights_enabled(netuid + 1, true); - - //Should still fail because bad netuid - assert_err!( - SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash), - Error::::CommitRevealDisabled - ); - - step_block(5); - - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt.clone(), - version_key, - ), - Error::::CommitRevealDisabled - ); - - // Enable and should pass + // Enable commit/reveal SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + // Commit should now succeed assert_ok!(SubtensorModule::commit_weights( RuntimeOrigin::signed(hotkey), netuid, commit_hash )); - step_block(5); + // Calculate blocks to next epoch and advance + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + SubtensorModule::get_tempo(netuid), + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + // Reveal should succeed assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, uids, weight_values, - salt.clone(), + salt, version_key, )); }); @@ -1786,40 +1725,34 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { version_key, )); - add_network(netuid, 0, 0); - register_ok_neuron(netuid, U256::from(3), U256::from(4), 300000); - register_ok_neuron(netuid, U256::from(1), U256::from(2), 100000); + add_network(netuid, 5, 0); + System::set_block_number(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_validator_permit_for_uid(netuid, 0, true); SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); - SubtensorModule::set_weights_set_rate_limit(netuid, 5); - SubtensorModule::set_commit_reveal_weights_interval(netuid, 5); - - step_block(5); - // Set weights OK - let result = SubtensorModule::set_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - 0, - ); - assert_ok!(result); - - // Enable Commit/Reveal + // Enable commit/reveal SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - // Commit is enabled the same block + // Commit at block 0 assert_ok!(SubtensorModule::commit_weights( RuntimeOrigin::signed(hotkey), netuid, commit_hash )); - step_block(5); //Step to the next commit/reveal tempo + // Calculate blocks to next epoch and advance + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + SubtensorModule::get_tempo(netuid), + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); - // Reveal OK + // Reveal in the next epoch assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, @@ -1829,72 +1762,20 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { version_key, )); - // Disable Commit/Reveal + // Disable commit/reveal SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); - // Cannot set weights the same block due to WeightsRateLimit - step_block(5); //step to avoid settingweightstofast + // Advance to allow setting weights (due to rate limit) + step_block(5); - let result = SubtensorModule::set_weights( + // Set weights directly + assert_ok!(SubtensorModule::set_weights( RuntimeOrigin::signed(hotkey), netuid, - uids.clone(), - weight_values.clone(), - 0, - ); - assert_ok!(result); - }); -} - -#[test] -fn test_commit_reveal_bad_salt_fail() { - 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 bad_salt: Vec = vec![0, 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(), + uids, + weight_values, version_key, )); - - add_network(netuid, 0, 0); - register_ok_neuron(netuid, U256::from(3), U256::from(4), 300000); - register_ok_neuron(netuid, U256::from(1), U256::from(2), 100000); - 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_interval(netuid, 5); - SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - - assert_ok!(SubtensorModule::commit_weights( - RuntimeOrigin::signed(hotkey), - netuid, - commit_hash - )); - - step_block(5); - - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - bad_salt.clone(), - version_key, - ), - Error::::InvalidRevealCommitHashNotMatch - ); }); } @@ -1906,8 +1787,6 @@ fn commit_reveal_set_weights( salt: Vec, version_key: u64, ) -> DispatchResult { - SubtensorModule::set_commit_reveal_weights_interval(netuid, 5); - SubtensorModule::set_weights_set_rate_limit(netuid, 5); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); let commit_hash: H256 = BlakeTwo256::hash_of(&( @@ -1921,7 +1800,13 @@ fn commit_reveal_set_weights( SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash)?; - step_block(5); + // Calculate blocks to next epoch and advance + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + SubtensorModule::get_tempo(netuid), + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), From 1719e4b3df1c86054eb134f82d062a9b244f5c2c Mon Sep 17 00:00:00 2001 From: johnreedv Date: Sat, 5 Oct 2024 09:57:11 -0700 Subject: [PATCH 02/33] expand test & clippy --- pallets/subtensor/src/subnets/weights.rs | 22 ++++------- pallets/subtensor/tests/weights.rs | 49 +++++++++++++++++++----- 2 files changed, 47 insertions(+), 24 deletions(-) diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 85f560f68..d2d6cf2a0 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -475,22 +475,16 @@ impl Pallet { let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); let current_epoch: u64 = Self::get_epoch_index(netuid, current_block); - // Allow reveal if the current epoch is immediately after the commit's epoch - if current_epoch == commit_epoch + 1 { - return true; - } - false + // Reveal is allowed only in the epoch immediately after the commit's epoch + current_epoch == commit_epoch.saturating_add(1) } pub fn get_epoch_index(netuid: u16, block_number: u64) -> u64 { - let tempo = Self::get_tempo(netuid); - if tempo == 0 { - return 0; - } - let tempo_plus_one = (tempo as u64).saturating_add(1); - let netuid_plus_one = (netuid as u64).saturating_add(1); - let epoch_index = - (block_number.saturating_add(netuid_plus_one)).saturating_div(tempo_plus_one); - epoch_index + 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 block_with_offset: u64 = block_number.saturating_add(netuid_plus_one); + + block_with_offset.checked_div(tempo_plus_one).unwrap_or(0) } } diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 27a43de4c..e752c28d2 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -1388,10 +1388,8 @@ fn test_commit_reveal_weights_ok() { version_key, )); - // Set block number to 0 System::set_block_number(0); - // Add network with tempo 5 let tempo: u16 = 5; add_network(netuid, tempo, 0); @@ -1401,8 +1399,6 @@ fn test_commit_reveal_weights_ok() { 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); - - // Enable commit/reveal SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); // Commit at block 0 @@ -1461,8 +1457,6 @@ fn test_commit_reveal_tempo_interval() { 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); - - // Enable commit/reveal SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); // Commit at block 0 @@ -1542,18 +1536,53 @@ fn test_commit_reveal_tempo_interval() { ); step_block(blocks_to_next_epoch.saturating_add(1) as u16); - // Attempt to reveal previous commit in the new epoch, should fail with `InvalidRevealCommitTempo` assert_err!( SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, - uids, - weight_values, - salt, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + ), + Error::::InvalidRevealCommitTempo + ); + + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + + step_block(50); + + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), version_key, ), Error::::InvalidRevealCommitTempo ); + + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids, + weight_values, + salt, + version_key, + )); }); } From 911ffec3294088205a04781fd3dd3464ea1fa000 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Sat, 5 Oct 2024 12:51:35 -0700 Subject: [PATCH 03/33] add test_tempo_change --- pallets/subtensor/tests/weights.rs | 162 +++++++++++++++++++++++++++++ 1 file changed, 162 insertions(+) diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index e752c28d2..513421017 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -1808,6 +1808,168 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { }); } +#[test] +fn test_tempo_change_during_commit_reveal_process() { + 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(1); + + let tempo: u16 = 100; + 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, 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); + + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + log::info!( + "Commit successful at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + step_block(9); + log::info!( + "Advanced to block {}", + SubtensorModule::get_current_block_as_u64() + ); + + let tempo_before_next_reveal: u16 = 200; + log::info!("Changing tempo to {}", tempo_before_next_reveal); + SubtensorModule::set_tempo(netuid, tempo_before_next_reveal); + + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + SubtensorModule::get_tempo(netuid), + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch as u16); + log::info!( + "Advanced to block {}", + SubtensorModule::get_current_block_as_u64() + 1 + ); + assert!(SubtensorModule::should_run_epoch(netuid, SubtensorModule::get_current_block_as_u64())); + step_block(1); + + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + log::info!( + "Revealed at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + log::info!( + "Commit successful at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + let tempo: u16 = 150; + log::info!("Changing tempo to {}", tempo); + SubtensorModule::set_tempo(netuid, tempo); + + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + SubtensorModule::get_tempo(netuid), + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch as u16); + log::info!( + "Advanced to block {}", + SubtensorModule::get_current_block_as_u64() + 1 + ); + assert!(SubtensorModule::should_run_epoch(netuid, SubtensorModule::get_current_block_as_u64())); + step_block(1); + + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + log::info!( + "Revealed at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + + let tempo: u16 = 1050; + log::info!("Changing tempo to {}", tempo); + SubtensorModule::set_tempo(netuid, tempo); + + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + log::info!( + "Commit successful at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + let tempo: u16 = 805; + log::info!("Changing tempo to {}", tempo); + SubtensorModule::set_tempo(netuid, tempo); + + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + SubtensorModule::get_tempo(netuid), + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + log::info!( + "Advanced to block {}", + SubtensorModule::get_current_block_as_u64() + ); + + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + log::info!( + "Revealed at block {}", + SubtensorModule::get_current_block_as_u64() + ); + }); +} + fn commit_reveal_set_weights( hotkey: U256, netuid: u16, From c2674bf94309398e2e523f39a95f03dda507d3a7 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Sun, 6 Oct 2024 11:37:36 -0700 Subject: [PATCH 04/33] support multiple commits --- pallets/subtensor/src/lib.rs | 5 +- pallets/subtensor/src/macros/errors.rs | 8 +- pallets/subtensor/src/subnets/weights.rs | 153 +++++-- pallets/subtensor/tests/swap_hotkey.rs | 7 +- pallets/subtensor/tests/weights.rs | 510 ++++++++++++++++++++++- 5 files changed, 610 insertions(+), 73 deletions(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 92156be94..39ad90895 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -72,6 +72,7 @@ pub mod pallet { use frame_system::pallet_prelude::*; use sp_core::H256; use sp_runtime::traits::{Dispatchable, TrailingZeroInput}; + use sp_std::collections::vec_deque::VecDeque; use sp_std::vec; use sp_std::vec::Vec; use subtensor_macros::freeze_struct; @@ -1245,14 +1246,14 @@ pub mod pallet { /// ITEM( weights_min_stake ) pub type WeightsMinStake = StorageValue<_, u64, ValueQuery, DefaultWeightsMinStake>; #[pallet::storage] - /// --- MAP (netuid, who) --> (hash, weight) | Returns the hash and weight committed by an account for a given netuid. + /// --- MAP (netuid, who) --> VecDeque<(hash, commit_block)> | Stores a queue of commits for an account on a given netuid. pub type WeightCommits = StorageDoubleMap< _, Twox64Concat, u16, Twox64Concat, T::AccountId, - (H256, u64), + VecDeque<(H256, u64)>, OptionQuery, >; diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 9fd7c5fea..7ef1ec999 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -184,7 +184,11 @@ mod errors { TxChildkeyTakeRateLimitExceeded, /// Invalid identity. InvalidIdentity, - /// invalid reaveal epoch - InvalidRevealEpoch, + /// Maximum commit limit reached + TooManyUnrevealedCommits, + /// Reveal is out of order + RevealOutOfOrder, + /// Attempted to reveal weights that are expired + AttemptedToRevealExpiredWeightCommit, } } diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index d2d6cf2a0..a4730f4ea 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -2,7 +2,7 @@ use super::*; use crate::epoch::math::*; use sp_core::H256; use sp_runtime::traits::{BlakeTwo256, Hash}; -use sp_std::vec; +use sp_std::{collections::vec_deque::VecDeque, vec}; impl Pallet { /// ---- The implementation for committing weight hashes. @@ -18,34 +18,45 @@ impl Pallet { /// - The hash representing the committed weights. /// /// # Raises: - /// * `WeightsCommitNotAllowed`: - /// - Attempting to commit when it is not allowed. + /// * `CommitRevealDisabled`: + /// - Attempting to commit when the commit-reveal mechanism is disabled. /// + /// * `TooManyUnrevealedCommits`: + /// - Attempting to commit when the user has more than the allowed limit of unrevealed commits. pub fn do_commit_weights( origin: T::RuntimeOrigin, netuid: u16, commit_hash: H256, ) -> DispatchResult { + // --- 1. Check the caller's signature (hotkey). let who = ensure_signed(origin)?; log::debug!("do_commit_weights( hotkey:{:?} netuid:{:?})", who, netuid); + // --- 2. Ensure commit-reveal is enabled for the network. ensure!( Self::get_commit_reveal_weights_enabled(netuid), Error::::CommitRevealDisabled ); - ensure!( - Self::can_commit(netuid, &who), - Error::::WeightsCommitNotAllowed - ); + // --- 3. Check if the current number of unrevealed commits is within the allowed limit. + WeightCommits::::try_mutate(netuid, &who, |maybe_commits| -> DispatchResult { + if let Some(ref commits) = maybe_commits { + ensure!(commits.len() < 10, Error::::TooManyUnrevealedCommits); + } - WeightCommits::::insert( - netuid, - &who, - (commit_hash, Self::get_current_block_as_u64()), - ); - Ok(()) + // --- 4. Take the existing commits or create a new VecDeque. + let mut commits: VecDeque<(H256, u64)> = maybe_commits.take().unwrap_or_default(); + + // --- 5. Append the new commit to the queue. + commits.push_back((commit_hash, Self::get_current_block_as_u64())); + + // --- 6. Store the updated queue back to storage. + *maybe_commits = Some(commits); + + // --- 7. Return ok. + Ok(()) + }) } /// ---- The implementation for revealing committed weights. @@ -63,22 +74,30 @@ impl Pallet { /// * `values` (`Vec`): /// - The values of the weights being revealed. /// - /// * `salt` (`Vec`): + /// * `salt` (`Vec`): /// - The values of the weights being revealed. /// /// * `version_key` (`u64`): /// - The network version key. /// /// # Raises: + /// * `CommitRevealDisabled`: + /// - Attempting to reveal weights when the commit-reveal mechanism is disabled. + /// /// * `NoWeightsCommitFound`: /// - Attempting to reveal weights without an existing commit. /// - /// * `InvalidRevealCommitHashNotMatchTempo`: - /// - Attempting to reveal weights outside the valid tempo. + /// * `InvalidRevealCommitTempo`: + /// - Attempting to reveal weights outside the valid reveal period. /// - /// * `InvalidRevealCommitHashNotMatch`: - /// - The revealed hash does not match the committed hash. + /// * `AttemptedToRevealExpiredWeightCommit`: + /// - Attempting to reveal a weight commit that has expired. + /// + /// * `RevealOutOfOrder`: + /// - Attempting to reveal a commit out of the expected order. /// + /// * `InvalidRevealCommitHashNotMatch`: + /// - The revealed hash does not match any committed hash. pub fn do_reveal_weights( origin: T::RuntimeOrigin, netuid: u16, @@ -87,25 +106,36 @@ impl Pallet { salt: Vec, version_key: u64, ) -> DispatchResult { + // --- 1. Check the caller's signature (hotkey). let who = ensure_signed(origin.clone())?; log::debug!("do_reveal_weights( hotkey:{:?} netuid:{:?})", who, netuid); + // --- 2. Ensure commit-reveal is enabled for the network. ensure!( Self::get_commit_reveal_weights_enabled(netuid), Error::::CommitRevealDisabled ); - WeightCommits::::try_mutate_exists(netuid, &who, |maybe_commit| -> DispatchResult { - let (commit_hash, commit_block) = maybe_commit - .as_ref() + // --- 3. 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)?; - ensure!( - Self::is_reveal_block_range(netuid, *commit_block), - Error::::InvalidRevealCommitTempo - ); + // --- 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() { + if Self::is_commit_expired(netuid, *commit_block) { + // Collect the expired commit hash + expired_hashes.push(*hash); + commits.pop_front(); + } else { + break; + } + } + // --- 5. Hash the provided data. let provided_hash: H256 = BlakeTwo256::hash_of(&( who.clone(), netuid, @@ -114,13 +144,56 @@ impl Pallet { salt.clone(), version_key, )); + + // --- 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::::AttemptedToRevealExpiredWeightCommit.into()); + } else { + return Err(Error::::NoWeightsCommitFound.into()); + } + } + + // --- 7. Collect the hashes of the remaining (non-expired) commits. + let non_expired_hashes: Vec = commits.iter().map(|(hash, _)| *hash).collect(); + + // --- 8. Get the first commit from the VecDeque. + let (commit_hash, commit_block) = + commits.front().ok_or(Error::::NoWeightsCommitFound)?; + + // --- 9. Ensure the commit is ready to be revealed in the current block range. ensure!( - provided_hash == *commit_hash, - Error::::InvalidRevealCommitHashNotMatch + Self::is_reveal_block_range(netuid, *commit_block), + Error::::InvalidRevealCommitTempo ); - *maybe_commit = None; + // --- 10. Check if the provided hash matches the first commit's hash. + if provided_hash != *commit_hash { + // Check if the provided hash matches any other non-expired commit in the queue + if non_expired_hashes + .iter() + .skip(1) + .any(|hash| *hash == provided_hash) + { + return Err(Error::::RevealOutOfOrder.into()); + } else if expired_hashes.contains(&provided_hash) { + return Err(Error::::AttemptedToRevealExpiredWeightCommit.into()); + } else { + return Err(Error::::InvalidRevealCommitHashNotMatch.into()); + } + } + + // --- 11. Remove the first commit from the queue after passing all checks. + commits.pop_front(); + + // --- 12. If the queue is now empty, remove the storage entry for the user. + if commits.is_empty() { + *maybe_commits = None; + } + // --- 13. Proceed to set the revealed weights. Self::do_set_weights(origin, netuid, uids, values, version_key) }) } @@ -454,22 +527,6 @@ impl Pallet { uids.len() <= subnetwork_n as usize } - pub fn can_commit(netuid: u16, who: &T::AccountId) -> bool { - if let Some((_hash, commit_block)) = WeightCommits::::get(netuid, who) { - let current_block: u64 = Self::get_current_block_as_u64(); - let current_epoch: u64 = Self::get_epoch_index(netuid, current_block); - let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); - - // Allow commit if we're in a new epoch - if current_epoch > commit_epoch { - return true; - } - false - } else { - true - } - } - pub fn is_reveal_block_range(netuid: u16, commit_block: u64) -> bool { let current_block: u64 = Self::get_current_block_as_u64(); let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); @@ -487,4 +544,12 @@ impl Pallet { block_with_offset.checked_div(tempo_plus_one).unwrap_or(0) } + + pub fn is_commit_expired(netuid: u16, commit_block: u64) -> bool { + let current_block = Self::get_current_block_as_u64(); + let current_epoch = Self::get_epoch_index(netuid, current_block); + let commit_epoch = Self::get_epoch_index(netuid, commit_block); + + current_epoch > commit_epoch.saturating_add(1) + } } diff --git a/pallets/subtensor/tests/swap_hotkey.rs b/pallets/subtensor/tests/swap_hotkey.rs index 89938e3eb..bf5ecb301 100644 --- a/pallets/subtensor/tests/swap_hotkey.rs +++ b/pallets/subtensor/tests/swap_hotkey.rs @@ -342,7 +342,7 @@ fn test_swap_certificates() { ); }); } - +use sp_std::collections::vec_deque::VecDeque; // SKIP_WASM_BUILD=1 RUST_LOG=debug cargo test --test swap_hotkey -- test_swap_weight_commits --exact --nocapture #[test] fn test_swap_weight_commits() { @@ -351,12 +351,13 @@ fn test_swap_weight_commits() { let new_hotkey = U256::from(2); let coldkey = U256::from(3); let netuid = 0u16; - let weight_commits = (H256::from_low_u64_be(100), 200); + let mut weight_commits: VecDeque<(H256, u64)> = VecDeque::new(); + weight_commits.push_back((H256::from_low_u64_be(100), 200)); let mut weight = Weight::zero(); add_network(netuid, 0, 1); IsNetworkMember::::insert(old_hotkey, netuid, true); - WeightCommits::::insert(netuid, old_hotkey, weight_commits); + WeightCommits::::insert(netuid, old_hotkey, weight_commits.clone()); assert_ok!(SubtensorModule::perform_hotkey_swap( &old_hotkey, diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 513421017..2b3a5d305 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -728,7 +728,6 @@ fn test_weights_err_max_weight_limit() { // Add network. let netuid: u16 = 1; let tempo: u16 = 100; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; add_network(netuid, tempo, 0); // Set params. @@ -789,18 +788,18 @@ fn test_weights_err_max_weight_limit() { // Non self-weight fails. let uids: Vec = vec![1, 2, 3, 4]; let values: Vec = vec![u16::MAX / 4, u16::MAX / 4, u16::MAX / 54, u16::MAX / 4]; - let result = commit_reveal_set_weights(U256::from(0), 1, uids, values, salt.clone(), 0); + let result = + SubtensorModule::set_weights(RuntimeOrigin::signed(U256::from(0)), 1, uids, values, 0); assert_eq!(result, Err(Error::::MaxWeightExceeded.into())); // Self-weight is a success. let uids: Vec = vec![0]; // Self. let values: Vec = vec![u16::MAX]; // normalizes to u32::MAX - assert_ok!(commit_reveal_set_weights( - U256::from(0), + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(U256::from(0)), 1, uids, values, - salt.clone(), 0 )); }); @@ -928,7 +927,6 @@ fn test_set_weight_too_many_uids() { new_test_ext(0).execute_with(|| { let netuid: u16 = 1; let tempo: u16 = 13; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; add_network(netuid, tempo, 0); register_ok_neuron(1, U256::from(1), U256::from(2), 100_000); @@ -943,12 +941,11 @@ fn test_set_weight_too_many_uids() { // Should fail because we are setting more weights than there are neurons. let weight_keys: Vec = vec![0, 1, 2, 3, 4]; // more uids than neurons in subnet. let weight_values: Vec = vec![88, 102, 303, 1212, 11]; // random value. - let result = commit_reveal_set_weights( - U256::from(1), + let result = SubtensorModule::set_weights( + RuntimeOrigin::signed(U256::from(1)), 1, weight_keys, weight_values, - salt.clone(), 0, ); assert_eq!( @@ -959,12 +956,11 @@ fn test_set_weight_too_many_uids() { // Shouldnt fail because we are setting less weights than there are neurons. let weight_keys: Vec = vec![0, 1]; // Only on neurons that exist. let weight_values: Vec = vec![10, 10]; // random value. - assert_ok!(commit_reveal_set_weights( - U256::from(1), + assert_ok!(SubtensorModule::set_weights( + RuntimeOrigin::signed(U256::from(1)), 1, weight_keys, weight_values, - salt, 0 )); }); @@ -1369,6 +1365,71 @@ fn test_set_weights_commit_reveal_enabled_error() { }); } +#[test] +fn test_reveal_weights_when_commit_reveal_disabled() { + 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(0); + + let tempo: u16 = 5; + add_network(netuid, tempo, 0); + + // Register neurons and set up configurations + 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); + + // Enable commit-reveal and commit + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + + // Advance to the next epoch + let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + // Disable commit-reveal before reveal + SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); + + // Attempt to reveal, should fail with CommitRevealDisabled + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids, + weight_values, + salt, + version_key, + ), + Error::::CommitRevealDisabled + ); + }); +} + #[test] fn test_commit_reveal_weights_ok() { new_test_ext(1).execute_with(|| { @@ -1467,10 +1528,10 @@ fn test_commit_reveal_tempo_interval() { )); // Attempt to commit again in the same epoch, should fail - assert_err!( - SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash), - Error::::WeightsCommitNotAllowed - ); + // assert_err!( + // SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash), + // Error::::WeightsCommitNotAllowed + // ); // Attempt to reveal in the same epoch, should fail assert_err!( @@ -1545,7 +1606,7 @@ fn test_commit_reveal_tempo_interval() { salt.clone(), version_key, ), - Error::::InvalidRevealCommitTempo + Error::::AttemptedToRevealExpiredWeightCommit ); assert_ok!(SubtensorModule::commit_weights( @@ -1869,7 +1930,10 @@ fn test_tempo_change_during_commit_reveal_process() { "Advanced to block {}", SubtensorModule::get_current_block_as_u64() + 1 ); - assert!(SubtensorModule::should_run_epoch(netuid, SubtensorModule::get_current_block_as_u64())); + assert!(SubtensorModule::should_run_epoch( + netuid, + SubtensorModule::get_current_block_as_u64() + )); step_block(1); assert_ok!(SubtensorModule::reveal_weights( @@ -1909,7 +1973,10 @@ fn test_tempo_change_during_commit_reveal_process() { "Advanced to block {}", SubtensorModule::get_current_block_as_u64() + 1 ); - assert!(SubtensorModule::should_run_epoch(netuid, SubtensorModule::get_current_block_as_u64())); + assert!(SubtensorModule::should_run_epoch( + netuid, + SubtensorModule::get_current_block_as_u64() + )); step_block(1); assert_ok!(SubtensorModule::reveal_weights( @@ -1925,7 +1992,6 @@ fn test_tempo_change_during_commit_reveal_process() { SubtensorModule::get_current_block_as_u64() ); - let tempo: u16 = 1050; log::info!("Changing tempo to {}", tempo); SubtensorModule::set_tempo(netuid, tempo); @@ -1949,11 +2015,16 @@ fn test_tempo_change_during_commit_reveal_process() { SubtensorModule::get_tempo(netuid), SubtensorModule::get_current_block_as_u64(), ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_block(blocks_to_next_epoch as u16); log::info!( "Advanced to block {}", - SubtensorModule::get_current_block_as_u64() + SubtensorModule::get_current_block_as_u64() + 1 ); + assert!(SubtensorModule::should_run_epoch( + netuid, + SubtensorModule::get_current_block_as_u64() + )); + step_block(1); assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -1970,6 +2041,401 @@ fn test_tempo_change_during_commit_reveal_process() { }); } +#[test] +fn test_commit_reveal_multiple_commits() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let uids: Vec = vec![0, 1]; + let weight_values: Vec = vec![10, 10]; + let version_key: u64 = 0; + let hotkey: U256 = U256::from(1); + + System::set_block_number(0); + + let tempo: u16 = 7200; + add_network(netuid, tempo, 0); + + // Setup the network and neurons + 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, 0); + 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); + + // 1. Commit 10 times successfully + let mut commit_hashes = vec![]; + for i in 0..10 { + let salt_i: Vec = vec![i; 8]; // Unique salt for each commit + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_i.clone(), + version_key, + )); + commit_hashes.push((commit_hash, salt_i)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + } + + // 2. Attempt to commit an 11th time, should fail + let salt_11: Vec = vec![11; 8]; + let commit_hash_11: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_11.clone(), + version_key, + )); + assert_err!( + SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash_11), + Error::::TooManyUnrevealedCommits + ); + + // 3. Attempt to reveal out of order (reveal the second commit first), should fail + // Advance to the next epoch for reveals to be valid + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + // Try to reveal the second commit first + let (_commit_hash_2, salt_2) = &commit_hashes[1]; + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_2.clone(), + version_key, + ), + Error::::RevealOutOfOrder + ); + + // 4. Reveal commits in order, ensuring they succeed + for (_commit_hash_i, salt_i) in commit_hashes.iter() { + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_i.clone(), + version_key, + )); + } + + // After revealing all commits, attempt to commit again should now succeed + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_11 + )); + + // 5. Test expired commits are removed and do not block reveals + // Commit again and let the commit expire + let salt_12: Vec = vec![12; 8]; + let commit_hash_12: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_12.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_12 + )); + + // Advance two epochs so the commit expires + for _ in 0..2 { + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + } + + // Attempt to reveal the expired commit, should fail + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_12.clone(), + version_key, + ), + Error::::AttemptedToRevealExpiredWeightCommit + ); + + // Commit again and reveal after advancing to next epoch + let salt_13: Vec = vec![13; 8]; + let commit_hash_13: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_13.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_13 + )); + + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_13.clone(), + version_key, + )); + + // 6. Ensure that attempting to reveal after the valid reveal period fails + // Commit again + let salt_14: Vec = vec![14; 8]; + let commit_hash_14: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_14.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_14 + )); + + // Advance beyond the valid reveal period (more than one epoch) + for _ in 0..2 { + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + } + + // Attempt to reveal, should fail + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_14.clone(), + version_key, + ), + Error::::AttemptedToRevealExpiredWeightCommit + ); + + // 7. Attempt to reveal a commit that is not ready yet (before the reveal period) + // Commit again + let salt_15: Vec = vec![15; 8]; + let commit_hash_15: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_15.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_15 + )); + + // Attempt to reveal immediately, should fail + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_15.clone(), + version_key, + ), + Error::::InvalidRevealCommitTempo + ); + + // Advance to the next epoch + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + // Now reveal should succeed + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_15.clone(), + version_key, + )); + + // 8. Test that revealing with incorrect data (salt) fails + // Commit again + let salt_16: Vec = vec![16; 8]; + let commit_hash_16: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_16.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_16 + )); + + // Advance to the next epoch + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + // Attempt to reveal with incorrect salt + let wrong_salt: Vec = vec![99; 8]; + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + wrong_salt.clone(), + version_key, + ), + Error::::InvalidRevealCommitHashNotMatch + ); + + // Reveal with correct data should succeed + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_16.clone(), + version_key, + )); + + // 9. Test that attempting to reveal when there are no commits fails + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_16.clone(), + version_key, + ), + Error::::NoWeightsCommitFound + ); + + // 10. Commit twice and attempt to reveal out of sequence + let salt_a: Vec = vec![21; 8]; + let commit_hash_a: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_a.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_a + )); + + let salt_b: Vec = vec![22; 8]; + let commit_hash_b: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_b.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_b + )); + + // Advance to next epoch + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + // Attempt to reveal the second commit first + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_b.clone(), + version_key, + ), + Error::::RevealOutOfOrder + ); + + // Reveal the first commit + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_a.clone(), + version_key, + )); + + // Now reveal the second commit + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids, + weight_values, + salt_b, + version_key, + )); + }); +} + fn commit_reveal_set_weights( hotkey: U256, netuid: u16, From a9f60eaa68b6b858e9d95f9553d27164c487e4df Mon Sep 17 00:00:00 2001 From: johnreedv Date: Mon, 7 Oct 2024 10:46:11 -0700 Subject: [PATCH 05/33] remove expired commits when committing --- pallets/subtensor/src/subnets/weights.rs | 25 ++- pallets/subtensor/tests/weights.rs | 205 +++++++++++++++++++++++ 2 files changed, 222 insertions(+), 8 deletions(-) diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index a4730f4ea..6be6e578d 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -39,22 +39,31 @@ impl Pallet { Error::::CommitRevealDisabled ); - // --- 3. Check if the current number of unrevealed commits is within the allowed limit. + // --- 3. Mutate the WeightCommits to retrieve existing commits for the user. WeightCommits::::try_mutate(netuid, &who, |maybe_commits| -> DispatchResult { - if let Some(ref commits) = maybe_commits { - ensure!(commits.len() < 10, Error::::TooManyUnrevealedCommits); - } - // --- 4. Take the existing commits or create a new VecDeque. let mut commits: VecDeque<(H256, u64)> = maybe_commits.take().unwrap_or_default(); - // --- 5. Append the new commit to the queue. + // --- 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) { + // Remove the expired commit + commits.pop_front(); + } else { + break; + } + } + + // --- 6. 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())); - // --- 6. Store the updated queue back to storage. + // --- 8. Store the updated queue back to storage. *maybe_commits = Some(commits); - // --- 7. Return ok. + // --- 9. Return ok. Ok(()) }) } diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 2b3a5d305..786bc9b1d 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -13,6 +13,7 @@ use sp_runtime::{ traits::{BlakeTwo256, DispatchInfoOf, Hash, SignedExtension}, DispatchError, }; +use sp_std::collections::vec_deque::VecDeque; use substrate_fixed::types::I32F32; /*************************** @@ -2476,3 +2477,207 @@ fn commit_reveal_set_weights( Ok(()) } + +#[test] +fn test_expired_commits_handling_in_commit_and_reveal() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey: ::AccountId = U256::from(1); + let version_key: u64 = 0; + let uids: Vec = vec![0, 1]; + let weight_values: Vec = vec![10, 10]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + + // Register neurons + 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_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + + // 1. Commit 5 times in epoch 0 + let mut commit_info = Vec::new(); + for i in 0..5 { + let salt: Vec = vec![i; 8]; + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + commit_info.push((commit_hash, salt)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + } + + // Advance to epoch 1 + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + // 2. Commit another 5 times in epoch 1 + for i in 5..10 { + let salt: Vec = vec![i; 8]; + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + commit_info.push((commit_hash, salt)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + } + + // 3. Attempt to commit an 11th time, should fail with TooManyUnrevealedCommits + let salt_11: Vec = vec![11; 8]; + let commit_hash_11: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_11.clone(), + version_key, + )); + assert_err!( + SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash_11), + Error::::TooManyUnrevealedCommits + ); + + // 4. Advance to epoch 2 to expire the commits from epoch 0 + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); // Now at epoch 2 + + // 5. Attempt to commit again; should succeed after expired commits are removed + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_11 + )); + + // 6. Verify that the number of unrevealed, non-expired commits is now 6 + let commits: VecDeque<(H256, 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 + + // 7. Attempt to reveal an expired commit (from epoch 0) + // Previous commit removed expired commits + let (_, expired_salt) = &commit_info[0]; + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + expired_salt.clone(), + version_key, + ), + Error::::InvalidRevealCommitHashNotMatch + ); + + // 8. Reveal commits from epoch 1 at current_epoch = 2 + for (_, salt) in commit_info.iter().skip(5).take(5) { + let salt = salt.clone(); + + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + } + + // 9. Advance to epoch 3 to reveal the new commit + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + // 10. Reveal the new commit from epoch 2 + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_11.clone(), + version_key, + )); + + // 10. Verify that all commits have been revealed and the queue is empty + let commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey); + assert!(commits.is_none()); + + // 11. Attempt to reveal again, should fail with NoWeightsCommitFound + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_11.clone(), + version_key, + ), + Error::::NoWeightsCommitFound + ); + + // 12. Commit again to ensure we can continue after previous commits + let salt_12: Vec = vec![12; 8]; + let commit_hash_12: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt_12.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash_12 + )); + + // Advance to next epoch (epoch 4) and reveal + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + tempo, + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids, + weight_values, + salt_12, + version_key, + )); + }); +} From c22c3c553437f56b4844c2dbb3f913ccba00fa06 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Mon, 7 Oct 2024 14:36:56 -0700 Subject: [PATCH 06/33] add RevealPeriodEpochs and improve errors --- pallets/subtensor/src/lib.rs | 9 + pallets/subtensor/src/macros/errors.rs | 10 +- pallets/subtensor/src/subnets/weights.rs | 34 ++- pallets/subtensor/tests/weights.rs | 351 ++++++++++++++++++++++- 4 files changed, 373 insertions(+), 31 deletions(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 39ad90895..02b164c0a 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -569,6 +569,11 @@ pub mod pallet { 0 } #[pallet::type_value] + /// Default minimum stake for weights. + pub fn DefaultRevealPeriodEpochs() -> u64 { + 1 + } + #[pallet::type_value] /// Value definition for vector of u16. pub fn EmptyU16Vec() -> Vec { vec![] @@ -1256,6 +1261,10 @@ pub mod pallet { VecDeque<(H256, u64)>, OptionQuery, >; + #[pallet::storage] + /// --- Map (netuid) --> Number of epochs allowed for commit reveal periods + pub type RevealPeriodEpochs = + StorageMap<_, Twox64Concat, u16, u64, ValueQuery, DefaultRevealPeriodEpochs>; /// ================== /// ==== Genesis ===== diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 7ef1ec999..5f3712355 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -118,8 +118,6 @@ mod errors { WeightsCommitNotAllowed, /// No commit found for the provided hotkey+netuid combination when attempting to reveal the weights. NoWeightsCommitFound, - /// Not the correct block/range to reveal weights. - InvalidRevealCommitTempo, /// Committed hash does not equal the hashed reveal data. InvalidRevealCommitHashNotMatch, /// Attempting to call set_weights when commit/reveal is enabled @@ -186,9 +184,11 @@ mod errors { InvalidIdentity, /// Maximum commit limit reached TooManyUnrevealedCommits, - /// Reveal is out of order + /// Reveal is out of order. RevealOutOfOrder, - /// Attempted to reveal weights that are expired - AttemptedToRevealExpiredWeightCommit, + /// Attempted to reveal weights that are expired. + ExpiredWeightCommit, + /// Attempted to reveal weights too early. + RevealTooEarly, } } diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 6be6e578d..2cd97d5f7 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -84,7 +84,7 @@ impl Pallet { /// - The values of the weights being revealed. /// /// * `salt` (`Vec`): - /// - The values of the weights being revealed. + /// - The salt used to generate the commit hash. /// /// * `version_key` (`u64`): /// - The network version key. @@ -96,12 +96,12 @@ impl Pallet { /// * `NoWeightsCommitFound`: /// - Attempting to reveal weights without an existing commit. /// - /// * `InvalidRevealCommitTempo`: - /// - Attempting to reveal weights outside the valid reveal period. - /// - /// * `AttemptedToRevealExpiredWeightCommit`: + /// * `ExpiredWeightCommit`: /// - Attempting to reveal a weight commit that has expired. /// + /// * `RevealTooEarly`: + /// - Attempting to reveal weights outside the valid reveal period. + /// /// * `RevealOutOfOrder`: /// - Attempting to reveal a commit out of the expected order. /// @@ -159,7 +159,7 @@ impl Pallet { // No non-expired commits // Check if provided_hash matches any expired commits if expired_hashes.contains(&provided_hash) { - return Err(Error::::AttemptedToRevealExpiredWeightCommit.into()); + return Err(Error::::ExpiredWeightCommit.into()); } else { return Err(Error::::NoWeightsCommitFound.into()); } @@ -175,7 +175,7 @@ impl Pallet { // --- 9. Ensure the commit is ready to be revealed in the current block range. ensure!( Self::is_reveal_block_range(netuid, *commit_block), - Error::::InvalidRevealCommitTempo + Error::::RevealTooEarly ); // --- 10. Check if the provided hash matches the first commit's hash. @@ -188,7 +188,7 @@ impl Pallet { { return Err(Error::::RevealOutOfOrder.into()); } else if expired_hashes.contains(&provided_hash) { - return Err(Error::::AttemptedToRevealExpiredWeightCommit.into()); + return Err(Error::::ExpiredWeightCommit.into()); } else { return Err(Error::::InvalidRevealCommitHashNotMatch.into()); } @@ -540,9 +540,10 @@ impl Pallet { let current_block: u64 = Self::get_current_block_as_u64(); let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); let current_epoch: u64 = Self::get_epoch_index(netuid, current_block); + let reveal_period: u64 = RevealPeriodEpochs::::get(netuid); - // Reveal is allowed only in the epoch immediately after the commit's epoch - current_epoch == commit_epoch.saturating_add(1) + // Reveal is allowed only in the exact epoch `commit_epoch + reveal_period` + current_epoch == commit_epoch.saturating_add(reveal_period) } pub fn get_epoch_index(netuid: u16, block_number: u64) -> u64 { @@ -555,10 +556,15 @@ impl Pallet { } pub fn is_commit_expired(netuid: u16, commit_block: u64) -> bool { - let current_block = Self::get_current_block_as_u64(); - let current_epoch = Self::get_epoch_index(netuid, current_block); - let commit_epoch = Self::get_epoch_index(netuid, commit_block); + let current_block: u64 = Self::get_current_block_as_u64(); + let current_epoch: u64 = Self::get_epoch_index(netuid, current_block); + let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); + let reveal_period: u64 = RevealPeriodEpochs::::get(netuid); + + current_epoch > commit_epoch.saturating_add(reveal_period) + } - current_epoch > commit_epoch.saturating_add(1) + pub fn set_reveal_period(netuid: u16, reveal_period: u64) { + RevealPeriodEpochs::::insert(netuid, reveal_period); } } diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 786bc9b1d..c13fd0007 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -1528,12 +1528,6 @@ fn test_commit_reveal_tempo_interval() { commit_hash )); - // Attempt to commit again in the same epoch, should fail - // assert_err!( - // SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash), - // Error::::WeightsCommitNotAllowed - // ); - // Attempt to reveal in the same epoch, should fail assert_err!( SubtensorModule::reveal_weights( @@ -1544,7 +1538,7 @@ fn test_commit_reveal_tempo_interval() { salt.clone(), version_key, ), - Error::::InvalidRevealCommitTempo + Error::::RevealTooEarly ); // Calculate blocks to next epoch and advance @@ -1607,7 +1601,7 @@ fn test_commit_reveal_tempo_interval() { salt.clone(), version_key, ), - Error::::AttemptedToRevealExpiredWeightCommit + Error::::ExpiredWeightCommit ); assert_ok!(SubtensorModule::commit_weights( @@ -1627,7 +1621,7 @@ fn test_commit_reveal_tempo_interval() { salt.clone(), version_key, ), - Error::::InvalidRevealCommitTempo + Error::::RevealTooEarly ); let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( @@ -2178,7 +2172,7 @@ fn test_commit_reveal_multiple_commits() { salt_12.clone(), version_key, ), - Error::::AttemptedToRevealExpiredWeightCommit + Error::::ExpiredWeightCommit ); // Commit again and reveal after advancing to next epoch @@ -2250,7 +2244,7 @@ fn test_commit_reveal_multiple_commits() { salt_14.clone(), version_key, ), - Error::::AttemptedToRevealExpiredWeightCommit + Error::::ExpiredWeightCommit ); // 7. Attempt to reveal a commit that is not ready yet (before the reveal period) @@ -2280,7 +2274,7 @@ fn test_commit_reveal_multiple_commits() { salt_15.clone(), version_key, ), - Error::::InvalidRevealCommitTempo + Error::::RevealTooEarly ); // Advance to the next epoch @@ -2681,3 +2675,336 @@ fn test_expired_commits_handling_in_commit_and_reveal() { )); }); } + +#[test] +fn test_reveal_at_exact_epoch() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey: ::AccountId = U256::from(1); + let version_key: u64 = 0; + let uids: Vec = vec![0, 1]; + let weight_values: Vec = vec![10, 10]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + SubtensorModule::set_weights_set_rate_limit(netuid, 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_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + + let reveal_periods: Vec = vec![0, 1, 2, 7, 40, 86, 100]; + + for &reveal_period in &reveal_periods { + SubtensorModule::set_reveal_period(netuid, reveal_period); + + let salt: Vec = vec![42; 8]; + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + + // Retrieve commit information + let commit_block = SubtensorModule::get_current_block_as_u64(); + let commit_epoch = SubtensorModule::get_epoch_index(netuid, commit_block); + let reveal_epoch = commit_epoch.saturating_add(reveal_period); + + // Attempt to reveal before the allowed epoch + if reveal_period > 0 { + // Advance to epoch before the reveal epoch + if reveal_period >= 1 { + step_epochs((reveal_period - 1) as u16, netuid); + } + + // Attempt to reveal too early + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + ), + Error::::RevealTooEarly + ); + } + + // Advance to the exact reveal epoch + let current_epoch = SubtensorModule::get_epoch_index( + netuid, + SubtensorModule::get_current_block_as_u64(), + ); + if current_epoch < reveal_epoch { + step_epochs((reveal_epoch - current_epoch) as u16, netuid); + } + + // Reveal at the exact allowed epoch + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + ), + Error::::NoWeightsCommitFound + ); + + let new_salt: Vec = vec![43; 8]; + let new_commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + new_salt.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + new_commit_hash + )); + + // Advance past the reveal epoch to ensure commit expiration + step_epochs((reveal_period + 1) as u16, netuid); + + // Attempt to reveal after the allowed epoch + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + new_salt.clone(), + version_key, + ), + Error::::ExpiredWeightCommit + ); + + pallet_subtensor::WeightCommits::::remove(netuid, hotkey); + } + }); +} + +#[test] +fn test_tempo_and_reveal_period_change_during_commit_reveal_process() { + 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![42; 8]; + let version_key: u64 = 0; + let hotkey: ::AccountId = U256::from(1); + + // Compute initial commit hash + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + + System::set_block_number(0); + + 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); + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + + SubtensorModule::set_weights_set_rate_limit(netuid, 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_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + + // Step 1: Commit weights + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + log::info!( + "Commit successful at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + // Retrieve commit block and epoch + let commit_block = SubtensorModule::get_current_block_as_u64(); + let commit_epoch = SubtensorModule::get_epoch_index(netuid, commit_block); + + // Step 2: Change tempo and reveal period after commit + 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); + log::info!( + "Changed tempo to {} and reveal period to {}", + new_tempo, + new_reveal_period + ); + + // Step 3: Advance blocks to reach the reveal epoch according to new tempo and reveal period + let current_block = SubtensorModule::get_current_block_as_u64(); + let current_epoch = SubtensorModule::get_epoch_index(netuid, current_block); + let reveal_epoch = commit_epoch.saturating_add(new_reveal_period); + + // Advance to one epoch before reveal epoch + if current_epoch < reveal_epoch { + let epochs_to_advance = reveal_epoch - current_epoch - 1; + step_epochs(epochs_to_advance as u16, netuid); + } + + // Attempt to reveal too early + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key + ), + Error::::RevealTooEarly + ); + log::info!( + "Attempted to reveal too early at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + // Advance to reveal epoch + step_epochs(1, netuid); + + // Attempt to reveal at the correct epoch + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key + )); + log::info!( + "Revealed weights at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + // Step 4: Change tempo and reveal period again after reveal + 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 {} and reveal period to {} after reveal", + new_tempo_after_reveal, + new_reveal_period_after_reveal + ); + + // Step 5: Commit again + let new_salt: Vec = vec![43; 8]; + let new_commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + new_salt.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + new_commit_hash + )); + log::info!( + "Commit successful at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + // Retrieve new commit block and epoch + let new_commit_block = SubtensorModule::get_current_block_as_u64(); + let new_commit_epoch = SubtensorModule::get_epoch_index(netuid, new_commit_block); + let new_reveal_epoch = new_commit_epoch.saturating_add(new_reveal_period_after_reveal); + + // Advance to reveal epoch + let current_block = SubtensorModule::get_current_block_as_u64(); + let current_epoch = SubtensorModule::get_epoch_index(netuid, current_block); + if current_epoch < new_reveal_epoch { + let epochs_to_advance = new_reveal_epoch - current_epoch; + step_epochs(epochs_to_advance as u16, netuid); + } + + // Attempt to reveal at the correct epoch + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + new_salt.clone(), + version_key + )); + log::info!( + "Revealed weights at block {}", + SubtensorModule::get_current_block_as_u64() + ); + + // Step 6: Attempt to reveal after the allowed epoch (commit expires) + // Advance past the reveal epoch + let expiration_epochs = 1; + step_epochs(expiration_epochs as u16, netuid); + + // Attempt to reveal again (should fail due to expired commit) + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + new_salt.clone(), + version_key + ), + Error::::NoWeightsCommitFound + ); + log::info!( + "Attempted to reveal after expiration at block {}", + SubtensorModule::get_current_block_as_u64() + ); + }); +} + +pub fn step_epochs(count: u16, netuid: u16) { + for _ in 0..count { + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + SubtensorModule::get_tempo(netuid), + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch.saturating_add(1) as u16); + } +} From b0b817a108f771a5b8bc55bbf7de567086f513ed Mon Sep 17 00:00:00 2001 From: johnreedv Date: Mon, 7 Oct 2024 15:05:03 -0700 Subject: [PATCH 07/33] use fn step_epochs in tests --- pallets/subtensor/tests/weights.rs | 210 +++++------------------------ 1 file changed, 33 insertions(+), 177 deletions(-) diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index c13fd0007..168f2bb64 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -1405,13 +1405,7 @@ fn test_reveal_weights_when_commit_reveal_disabled() { commit_hash )); - // Advance to the next epoch - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // Disable commit-reveal before reveal SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); @@ -1470,13 +1464,7 @@ fn test_commit_reveal_weights_ok() { commit_hash )); - // Calculate blocks to next epoch and advance - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // Reveal in the next epoch assert_ok!(SubtensorModule::reveal_weights( @@ -1541,13 +1529,7 @@ fn test_commit_reveal_tempo_interval() { Error::::RevealTooEarly ); - // Calculate blocks to next epoch and advance - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -1579,18 +1561,7 @@ fn test_commit_reveal_tempo_interval() { )); // step two epochs - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(2, netuid); assert_err!( SubtensorModule::reveal_weights( @@ -1624,12 +1595,7 @@ fn test_commit_reveal_tempo_interval() { Error::::RevealTooEarly ); - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -1679,13 +1645,7 @@ fn test_commit_reveal_hash() { commit_hash )); - // Calculate blocks to next epoch and advance - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - SubtensorModule::get_tempo(netuid), - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // Attempt to reveal with incorrect data, should fail assert_err!( @@ -1771,13 +1731,7 @@ fn test_commit_reveal_disabled_or_enabled() { commit_hash )); - // Calculate blocks to next epoch and advance - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - SubtensorModule::get_tempo(netuid), - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // Reveal should succeed assert_ok!(SubtensorModule::reveal_weights( @@ -1829,13 +1783,7 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { commit_hash )); - // Calculate blocks to next epoch and advance - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - SubtensorModule::get_tempo(netuid), - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // Reveal in the next epoch assert_ok!(SubtensorModule::reveal_weights( @@ -1915,21 +1863,11 @@ fn test_tempo_change_during_commit_reveal_process() { log::info!("Changing tempo to {}", tempo_before_next_reveal); SubtensorModule::set_tempo(netuid, tempo_before_next_reveal); - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - SubtensorModule::get_tempo(netuid), - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch as u16); + step_epochs(1, netuid); log::info!( "Advanced to block {}", - SubtensorModule::get_current_block_as_u64() + 1 - ); - assert!(SubtensorModule::should_run_epoch( - netuid, SubtensorModule::get_current_block_as_u64() - )); - step_block(1); + ); assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -1958,21 +1896,11 @@ fn test_tempo_change_during_commit_reveal_process() { log::info!("Changing tempo to {}", tempo); SubtensorModule::set_tempo(netuid, tempo); - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - SubtensorModule::get_tempo(netuid), - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch as u16); + step_epochs(1, netuid); log::info!( "Advanced to block {}", - SubtensorModule::get_current_block_as_u64() + 1 - ); - assert!(SubtensorModule::should_run_epoch( - netuid, SubtensorModule::get_current_block_as_u64() - )); - step_block(1); + ); assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -2005,21 +1933,11 @@ fn test_tempo_change_during_commit_reveal_process() { log::info!("Changing tempo to {}", tempo); SubtensorModule::set_tempo(netuid, tempo); - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - SubtensorModule::get_tempo(netuid), - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch as u16); + step_epochs(1, netuid); log::info!( "Advanced to block {}", - SubtensorModule::get_current_block_as_u64() + 1 - ); - assert!(SubtensorModule::should_run_epoch( - netuid, SubtensorModule::get_current_block_as_u64() - )); - step_block(1); + ); assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -2095,12 +2013,7 @@ fn test_commit_reveal_multiple_commits() { // 3. Attempt to reveal out of order (reveal the second commit first), should fail // Advance to the next epoch for reveals to be valid - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // Try to reveal the second commit first let (_commit_hash_2, salt_2) = &commit_hashes[1]; @@ -2153,14 +2066,7 @@ fn test_commit_reveal_multiple_commits() { )); // Advance two epochs so the commit expires - for _ in 0..2 { - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); - } + step_epochs(2, netuid); // Attempt to reveal the expired commit, should fail assert_err!( @@ -2191,12 +2097,7 @@ fn test_commit_reveal_multiple_commits() { commit_hash_13 )); - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -2225,14 +2126,7 @@ fn test_commit_reveal_multiple_commits() { )); // Advance beyond the valid reveal period (more than one epoch) - for _ in 0..2 { - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); - } + step_epochs(2, netuid); // Attempt to reveal, should fail assert_err!( @@ -2277,13 +2171,7 @@ fn test_commit_reveal_multiple_commits() { Error::::RevealTooEarly ); - // Advance to the next epoch - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // Now reveal should succeed assert_ok!(SubtensorModule::reveal_weights( @@ -2312,13 +2200,7 @@ fn test_commit_reveal_multiple_commits() { commit_hash_16 )); - // Advance to the next epoch - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // Attempt to reveal with incorrect salt let wrong_salt: Vec = vec![99; 8]; @@ -2388,13 +2270,7 @@ fn test_commit_reveal_multiple_commits() { commit_hash_b )); - // Advance to next epoch - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // Attempt to reveal the second commit first assert_err!( @@ -2452,13 +2328,7 @@ fn commit_reveal_set_weights( SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash)?; - // Calculate blocks to next epoch and advance - let blocks_to_next_epoch: u64 = SubtensorModule::blocks_until_next_epoch( - netuid, - SubtensorModule::get_tempo(netuid), - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -2515,12 +2385,7 @@ fn test_expired_commits_handling_in_commit_and_reveal() { } // Advance to epoch 1 - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // 2. Commit another 5 times in epoch 1 for i in 5..10 { @@ -2557,12 +2422,7 @@ fn test_expired_commits_handling_in_commit_and_reveal() { ); // 4. Advance to epoch 2 to expire the commits from epoch 0 - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); // Now at epoch 2 + step_epochs(1, netuid); // Now at epoch 2 // 5. Attempt to commit again; should succeed after expired commits are removed assert_ok!(SubtensorModule::commit_weights( @@ -2607,12 +2467,7 @@ fn test_expired_commits_handling_in_commit_and_reveal() { } // 9. Advance to epoch 3 to reveal the new commit - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); // 10. Reveal the new commit from epoch 2 assert_ok!(SubtensorModule::reveal_weights( @@ -2658,12 +2513,7 @@ fn test_expired_commits_handling_in_commit_and_reveal() { )); // Advance to next epoch (epoch 4) and reveal - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - tempo, - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_epochs(1, netuid); assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -3005,6 +2855,12 @@ pub fn step_epochs(count: u16, netuid: u16) { SubtensorModule::get_tempo(netuid), SubtensorModule::get_current_block_as_u64(), ); - step_block(blocks_to_next_epoch.saturating_add(1) as u16); + step_block(blocks_to_next_epoch as u16); + + assert!(SubtensorModule::should_run_epoch( + netuid, + SubtensorModule::get_current_block_as_u64() + )); + step_block(1); } } From 625f2b390c3a05abc7dfa485ee5de22ec348c93d Mon Sep 17 00:00:00 2001 From: johnreedv Date: Tue, 8 Oct 2024 09:28:41 -0700 Subject: [PATCH 08/33] add test_commit_reveal_order_enforcement --- pallets/subtensor/tests/weights.rs | 116 +++++++++++++++++++++++++++++ 1 file changed, 116 insertions(+) diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 168f2bb64..764cfd040 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -2864,3 +2864,119 @@ pub fn step_epochs(count: u16, netuid: u16) { step_block(1); } } + +#[test] +fn test_commit_reveal_order_enforcement() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey: ::AccountId = U256::from(1); + let version_key: u64 = 0; + let uids: Vec = vec![0, 1]; + let weight_values: Vec = vec![10, 10]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + SubtensorModule::set_weights_set_rate_limit(netuid, 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_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + + // Commit three times: A, B, C + let mut commit_info = Vec::new(); + for i in 0..3 { + let salt: Vec = vec![i; 8]; + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + commit_info.push((commit_hash, salt)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + } + + step_epochs(1, netuid); + + // Attempt to reveal B first (index 1), should fail + let (_commit_hash_b, salt_b) = &commit_info[1]; + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_b.clone(), + version_key, + ), + Error::::RevealOutOfOrder + ); + + // Reveal A (index 0) + let (_commit_hash_a, salt_a) = &commit_info[0]; + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_a.clone(), + version_key, + )); + + // Attempt to reveal C (index 2) before B, should fail + let (_commit_hash_c, salt_c) = &commit_info[2]; + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_c.clone(), + version_key, + ), + Error::::RevealOutOfOrder + ); + + // Reveal B + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_b.clone(), + version_key, + )); + + // Reveal C + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_c.clone(), + version_key, + )); + + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids, + weight_values, + salt_a.clone(), + version_key, + ), + Error::::NoWeightsCommitFound + ); + }); +} From 9bd5fc9e022e3695fdc74fc1050d080350bc1208 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Tue, 8 Oct 2024 10:36:51 -0700 Subject: [PATCH 09/33] add hyperparam set_reveal_periods --- pallets/admin-utils/src/lib.rs | 39 ++++++++++++++++++++++++ pallets/admin-utils/src/weights.rs | 22 ------------- pallets/admin-utils/tests/tests.rs | 20 ++++++++++++ pallets/subtensor/src/subnets/weights.rs | 3 ++ 4 files changed, 62 insertions(+), 22 deletions(-) diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index 501122a8e..b2f59668e 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -1170,6 +1170,45 @@ pub mod pallet { Ok(()) } + + /// Sets the commit-reveal weights periods for a specific subnet. + /// + /// This extrinsic allows the subnet owner or root account to set the duration (in epochs) during which committed weights must be revealed. + /// The commit-reveal mechanism ensures that users commit weights in advance and reveal them only within a specified period. + /// + /// # Arguments + /// * `origin` - The origin of the call, which must be the subnet owner or the root account. + /// * `netuid` - The unique identifier of the subnet for which the periods are being set. + /// * `periods` - The number of epochs that define the commit-reveal period. + /// + /// # Errors + /// * `BadOrigin` - If the caller is neither the subnet owner nor the root account. + /// * `SubnetDoesNotExist` - If the specified subnet does not exist. + /// + /// # Weight + /// Weight is handled by the `#[pallet::weight]` attribute. + #[pallet::call_index(56)] + #[pallet::weight((0, DispatchClass::Operational, Pays::No))] + pub fn sudo_set_commit_reveal_weights_periods( + origin: OriginFor, + netuid: u16, + periods: u64, + ) -> DispatchResult { + pallet_subtensor::Pallet::::ensure_subnet_owner_or_root(origin, netuid)?; + + ensure!( + pallet_subtensor::Pallet::::if_subnet_exist(netuid), + Error::::SubnetDoesNotExist + ); + + pallet_subtensor::Pallet::::set_reveal_period(netuid, periods); + log::debug!( + "SetWeightCommitPeriods( netuid: {:?}, periods: {:?} ) ", + netuid, + periods + ); + Ok(()) + } } } diff --git a/pallets/admin-utils/src/weights.rs b/pallets/admin-utils/src/weights.rs index 84fe058f8..ba2247dfd 100644 --- a/pallets/admin-utils/src/weights.rs +++ b/pallets/admin-utils/src/weights.rs @@ -60,7 +60,6 @@ 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_interval() -> Weight; fn sudo_set_commit_reveal_weights_enabled() -> Weight; } @@ -413,15 +412,6 @@ 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_interval() -> Weight { - // Proof Size summary in bytes: - // Measured: `1111` - // Estimated: `4697` - // Minimum execution time: 46_450_000 picoseconds. - Weight::from_parts(47_279_000, 4697) - .saturating_add(T::DbWeight::get().reads(1_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - } fn sudo_set_commit_reveal_weights_enabled() -> Weight { // Proof Size summary in bytes: // Measured: `1111` @@ -781,18 +771,6 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } - fn sudo_set_commit_reveal_weights_interval() -> Weight { - // -- Extrinsic Time -- - // Model: - // Time ~= 20.42 - // µs - // Reads = 1 - // Writes = 1 - // Recorded proof Size = 456 - Weight::from_parts(20_420_000, 456) - .saturating_add(RocksDbWeight::get().reads(1_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - } fn sudo_set_commit_reveal_weights_enabled() -> Weight { // -- Extrinsic Time -- // Model: diff --git a/pallets/admin-utils/tests/tests.rs b/pallets/admin-utils/tests/tests.rs index 746dfa6f5..d2c36e29f 100644 --- a/pallets/admin-utils/tests/tests.rs +++ b/pallets/admin-utils/tests/tests.rs @@ -1412,3 +1412,23 @@ fn test_sudo_set_dissolve_network_schedule_duration() { System::assert_last_event(Event::DissolveNetworkScheduleDurationSet(new_duration).into()); }); } + +#[test] +fn sudo_set_commit_reveal_weights_periods() { + new_test_ext().execute_with(|| { + let netuid: u16 = 1; + add_network(netuid, 10); + + let to_be_set = 55; + let init_value = SubtensorModule::get_reveal_period(netuid); + + assert_ok!(AdminUtils::sudo_set_commit_reveal_weights_periods( + <::RuntimeOrigin>::root(), + netuid, + to_be_set + )); + + assert!(init_value != to_be_set); + assert_eq!(SubtensorModule::get_reveal_period(netuid), to_be_set); + }); +} diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 2cd97d5f7..c1ba1b1be 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -567,4 +567,7 @@ impl Pallet { pub fn set_reveal_period(netuid: u16, reveal_period: u64) { RevealPeriodEpochs::::insert(netuid, reveal_period); } + pub fn get_reveal_period(netuid: u16) -> u64 { + RevealPeriodEpochs::::get(netuid) + } } From 8d66f5f13f9e7eb07ca289b7bfe69fa0cc34bdc1 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 16 Oct 2024 15:57:12 -0700 Subject: [PATCH 10/33] tempo based & modify order enforcement --- pallets/subtensor/src/macros/errors.rs | 4 - pallets/subtensor/src/subnets/weights.rs | 69 +++-- pallets/subtensor/tests/mock.rs | 18 ++ pallets/subtensor/tests/weights.rs | 339 +++++++++++++++-------- 4 files changed, 281 insertions(+), 149 deletions(-) diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 5f3712355..ac54cff1c 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -114,8 +114,6 @@ mod errors { DelegateTakeTooLow, /// Delegate take is too high. DelegateTakeTooHigh, - /// Not allowed to commit weights. - WeightsCommitNotAllowed, /// No commit found for the provided hotkey+netuid combination when attempting to reveal the weights. NoWeightsCommitFound, /// Committed hash does not equal the hashed reveal data. @@ -184,8 +182,6 @@ mod errors { InvalidIdentity, /// Maximum commit limit reached TooManyUnrevealedCommits, - /// Reveal is out of order. - RevealOutOfOrder, /// Attempted to reveal weights that are expired. ExpiredWeightCommit, /// Attempted to reveal weights too early. diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index c1ba1b1be..690fee200 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -165,45 +165,40 @@ impl Pallet { } } - // --- 7. Collect the hashes of the remaining (non-expired) commits. - let non_expired_hashes: Vec = commits.iter().map(|(hash, _)| *hash).collect(); - - // --- 8. Get the first commit from the VecDeque. - let (commit_hash, commit_block) = - commits.front().ok_or(Error::::NoWeightsCommitFound)?; - - // --- 9. Ensure the commit is ready to be revealed in the current block range. - ensure!( - Self::is_reveal_block_range(netuid, *commit_block), - Error::::RevealTooEarly - ); - - // --- 10. Check if the provided hash matches the first commit's hash. - if provided_hash != *commit_hash { - // Check if the provided hash matches any other non-expired commit in the queue - if non_expired_hashes - .iter() - .skip(1) - .any(|hash| *hash == provided_hash) - { - return Err(Error::::RevealOutOfOrder.into()); - } else if expired_hashes.contains(&provided_hash) { - return Err(Error::::ExpiredWeightCommit.into()); - } else { - return Err(Error::::InvalidRevealCommitHashNotMatch.into()); + // --- 7. Search for the provided_hash in the non-expired commits. + 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 + .get(position) + .ok_or(Error::::NoWeightsCommitFound)?; + + // --- 9. Ensure the commit is ready to be revealed in the current block range. + ensure!( + Self::is_reveal_block_range(netuid, *commit_block), + Error::::RevealTooEarly + ); + + // --- 10. Remove all commits up to and including the one being revealed. + for _ in 0..=position { + commits.pop_front(); } - } - // --- 11. Remove the first commit from the queue after passing all checks. - commits.pop_front(); + // --- 11. If the queue is now empty, remove the storage entry for the user. + if commits.is_empty() { + *maybe_commits = None; + } - // --- 12. If the queue is now empty, remove the storage entry for the user. - if commits.is_empty() { - *maybe_commits = None; + // --- 12. Proceed to set the revealed weights. + 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 { + Err(Error::::InvalidRevealCommitHashNotMatch.into()) + } } - - // --- 13. Proceed to set the revealed weights. - Self::do_set_weights(origin, netuid, uids, values, version_key) }) } @@ -540,7 +535,7 @@ impl Pallet { let current_block: u64 = Self::get_current_block_as_u64(); let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); let current_epoch: u64 = Self::get_epoch_index(netuid, current_block); - let reveal_period: u64 = RevealPeriodEpochs::::get(netuid); + let reveal_period: u64 = Self::get_reveal_period(netuid); // Reveal is allowed only in the exact epoch `commit_epoch + reveal_period` current_epoch == commit_epoch.saturating_add(reveal_period) @@ -559,7 +554,7 @@ impl Pallet { let current_block: u64 = Self::get_current_block_as_u64(); let current_epoch: u64 = Self::get_epoch_index(netuid, current_block); let commit_epoch: u64 = Self::get_epoch_index(netuid, commit_block); - let reveal_period: u64 = RevealPeriodEpochs::::get(netuid); + let reveal_period: u64 = Self::get_reveal_period(netuid); current_epoch > commit_epoch.saturating_add(reveal_period) } diff --git a/pallets/subtensor/tests/mock.rs b/pallets/subtensor/tests/mock.rs index 6f3b44383..7a2967e8c 100644 --- a/pallets/subtensor/tests/mock.rs +++ b/pallets/subtensor/tests/mock.rs @@ -513,6 +513,24 @@ pub(crate) fn run_to_block(n: u64) { } } +#[allow(dead_code)] +pub(crate) fn step_epochs(count: u16, netuid: u16) { + for _ in 0..count { + let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( + netuid, + SubtensorModule::get_tempo(netuid), + SubtensorModule::get_current_block_as_u64(), + ); + step_block(blocks_to_next_epoch as u16); + + assert!(SubtensorModule::should_run_epoch( + netuid, + SubtensorModule::get_current_block_as_u64() + )); + step_block(1); + } +} + /// Increments current block by `1`, running all hooks associated with doing so, and asserts /// that the block number was in fact incremented. /// diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 764cfd040..3fe8bed82 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -1977,7 +1977,7 @@ fn test_commit_reveal_multiple_commits() { SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); // 1. Commit 10 times successfully - let mut commit_hashes = vec![]; + let mut commit_info = Vec::new(); for i in 0..10 { let salt_i: Vec = vec![i; 8]; // Unique salt for each commit let commit_hash: H256 = BlakeTwo256::hash_of(&( @@ -1988,7 +1988,7 @@ fn test_commit_reveal_multiple_commits() { salt_i.clone(), version_key, )); - commit_hashes.push((commit_hash, salt_i)); + commit_info.push((commit_hash, salt_i)); assert_ok!(SubtensorModule::commit_weights( RuntimeOrigin::signed(hotkey), netuid, @@ -2011,35 +2011,40 @@ fn test_commit_reveal_multiple_commits() { Error::::TooManyUnrevealedCommits ); - // 3. Attempt to reveal out of order (reveal the second commit first), should fail + // 3. Attempt to reveal out of order (reveal the second commit first) // Advance to the next epoch for reveals to be valid step_epochs(1, netuid); // Try to reveal the second commit first - let (_commit_hash_2, salt_2) = &commit_hashes[1]; - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt_2.clone(), - version_key, - ), - Error::::RevealOutOfOrder - ); + let (_commit_hash_2, salt_2) = &commit_info[1]; + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_2.clone(), + version_key, + )); - // 4. Reveal commits in order, ensuring they succeed - for (_commit_hash_i, salt_i) in commit_hashes.iter() { - assert_ok!(SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt_i.clone(), - version_key, - )); - } + // Check that commits before the revealed one are removed + let remaining_commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey) + .expect("expected 8 remaining commits"); + assert_eq!(remaining_commits.len(), 8); // 10 commits - 2 removed (index 0 and 1) + + // 4. Reveal the last commit next + let (_commit_hash_10, salt_10) = &commit_info[9]; + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt_10.clone(), + version_key, + )); + + // Remaining commits should have removed up to index 9 + let remaining_commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey); + assert!(remaining_commits.is_none()); // All commits removed // After revealing all commits, attempt to commit again should now succeed assert_ok!(SubtensorModule::commit_weights( @@ -2239,7 +2244,7 @@ fn test_commit_reveal_multiple_commits() { Error::::NoWeightsCommitFound ); - // 10. Commit twice and attempt to reveal out of sequence + // 10. Commit twice and attempt to reveal out of sequence (which is now allowed) let salt_a: Vec = vec![21; 8]; let commit_hash_a: H256 = BlakeTwo256::hash_of(&( hotkey, @@ -2272,38 +2277,32 @@ fn test_commit_reveal_multiple_commits() { step_epochs(1, netuid); - // Attempt to reveal the second commit first - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt_b.clone(), - version_key, - ), - Error::::RevealOutOfOrder - ); - - // Reveal the first commit + // Reveal the second commit first, should now succeed assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, uids.clone(), weight_values.clone(), - salt_a.clone(), + salt_b.clone(), version_key, )); - // Now reveal the second commit - assert_ok!(SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids, - weight_values, - salt_b, - version_key, - )); + // Check that the first commit has been removed + let remaining_commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey); + assert!(remaining_commits.is_none()); + + // Attempting to reveal the first commit should fail as it was removed + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids, + weight_values, + salt_a, + version_key, + ), + Error::::NoWeightsCommitFound + ); }); } @@ -2848,23 +2847,6 @@ fn test_tempo_and_reveal_period_change_during_commit_reveal_process() { }); } -pub fn step_epochs(count: u16, netuid: u16) { - for _ in 0..count { - let blocks_to_next_epoch = SubtensorModule::blocks_until_next_epoch( - netuid, - SubtensorModule::get_tempo(netuid), - SubtensorModule::get_current_block_as_u64(), - ); - step_block(blocks_to_next_epoch as u16); - - assert!(SubtensorModule::should_run_epoch( - netuid, - SubtensorModule::get_current_block_as_u64() - )); - step_block(1); - } -} - #[test] fn test_commit_reveal_order_enforcement() { new_test_ext(1).execute_with(|| { @@ -2908,46 +2890,8 @@ fn test_commit_reveal_order_enforcement() { step_epochs(1, netuid); - // Attempt to reveal B first (index 1), should fail + // Attempt to reveal B first (index 1), should now succeed let (_commit_hash_b, salt_b) = &commit_info[1]; - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt_b.clone(), - version_key, - ), - Error::::RevealOutOfOrder - ); - - // Reveal A (index 0) - let (_commit_hash_a, salt_a) = &commit_info[0]; - assert_ok!(SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt_a.clone(), - version_key, - )); - - // Attempt to reveal C (index 2) before B, should fail - let (_commit_hash_c, salt_c) = &commit_info[2]; - assert_err!( - SubtensorModule::reveal_weights( - RuntimeOrigin::signed(hotkey), - netuid, - uids.clone(), - weight_values.clone(), - salt_c.clone(), - version_key, - ), - Error::::RevealOutOfOrder - ); - - // Reveal B assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, @@ -2957,7 +2901,13 @@ fn test_commit_reveal_order_enforcement() { version_key, )); - // Reveal C + // Check that commits A and B are removed + let remaining_commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey) + .expect("expected 1 remaining commit"); + assert_eq!(remaining_commits.len(), 1); // Only commit C should remain + + // Attempt to reveal C (index 2), should succeed + let (_commit_hash_c, salt_c) = &commit_info[2]; assert_ok!(SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), netuid, @@ -2967,6 +2917,8 @@ fn test_commit_reveal_order_enforcement() { version_key, )); + // Attempting to reveal A (index 0) should fail as it's been removed + let (_commit_hash_a, salt_a) = &commit_info[0]; assert_err!( SubtensorModule::reveal_weights( RuntimeOrigin::signed(hotkey), @@ -2980,3 +2932,174 @@ fn test_commit_reveal_order_enforcement() { ); }); } + +#[test] +fn test_reveal_at_exact_block() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey: ::AccountId = U256::from(1); + let version_key: u64 = 0; + let uids: Vec = vec![0, 1]; + let weight_values: Vec = vec![10, 10]; + let tempo: u16 = 360; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + SubtensorModule::set_weights_set_rate_limit(netuid, 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_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, + ]; + + for &reveal_period in &reveal_periods { + SubtensorModule::set_reveal_period(netuid, reveal_period); + + // Step 1: Commit weights + let salt: Vec = vec![42 + (reveal_period % 100) as u16; 8]; + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + + let commit_block = SubtensorModule::get_current_block_as_u64(); + let commit_epoch = SubtensorModule::get_epoch_index(netuid, commit_block); + let reveal_epoch = commit_epoch.saturating_add(reveal_period); + + // Calculate the block number where the reveal epoch starts + let tempo_plus_one = (tempo as u64).saturating_add(1); + let netuid_plus_one = (netuid as u64).saturating_add(1); + let reveal_epoch_start_block = reveal_epoch + .saturating_mul(tempo_plus_one) + .saturating_sub(netuid_plus_one); + + // Attempt to reveal before the reveal epoch starts + let current_block = SubtensorModule::get_current_block_as_u64(); + if current_block < reveal_epoch_start_block { + // Advance to one block before the reveal epoch starts + let blocks_to_advance = reveal_epoch_start_block.saturating_sub(current_block); + if blocks_to_advance > 1 { + // Advance to one block before the reveal epoch + let new_block_number = current_block + blocks_to_advance - 1; + System::set_block_number(new_block_number); + } + + // Attempt to reveal too early + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key + ), + Error::::RevealTooEarly + ); + + // Advance one more block to reach the exact reveal epoch start block + System::set_block_number(reveal_epoch_start_block); + } else { + // If we're already at or past the reveal epoch start block + System::set_block_number(reveal_epoch_start_block); + } + + // Reveal at the exact allowed block + assert_ok!(SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key + )); + + // Attempt to reveal again; should fail with NoWeightsCommitFound + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + salt.clone(), + version_key + ), + Error::::NoWeightsCommitFound + ); + + // Commit again with new salt + let new_salt: Vec = vec![43 + (reveal_period % 100) as u16; 8]; + let new_commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weight_values.clone(), + new_salt.clone(), + version_key, + )); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + new_commit_hash + )); + + // Advance blocks to after the commit expires + let commit_block = SubtensorModule::get_current_block_as_u64(); + let commit_epoch = SubtensorModule::get_epoch_index(netuid, commit_block); + let reveal_epoch = commit_epoch.saturating_add(reveal_period); + let expiration_epoch = reveal_epoch.saturating_add(1); + let expiration_epoch_start_block = expiration_epoch + .saturating_mul(tempo_plus_one) + .saturating_sub(netuid_plus_one); + + let current_block = SubtensorModule::get_current_block_as_u64(); + if current_block < expiration_epoch_start_block { + // Advance to the block where the commit expires + System::set_block_number(expiration_epoch_start_block); + } + + // Attempt to reveal after the commit has expired + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids.clone(), + weight_values.clone(), + new_salt.clone(), + version_key + ), + Error::::ExpiredWeightCommit + ); + + // Clean up for next iteration + pallet_subtensor::WeightCommits::::remove(netuid, hotkey); + } + }); +} From 240e17ae415dd05e402875686a831e3e90ea527b Mon Sep 17 00:00:00 2001 From: johnreedv Date: Thu, 17 Oct 2024 08:16:02 -0700 Subject: [PATCH 11/33] add commit-reveal-periods to SubnetHyperparams --- pallets/subtensor/src/rpc_info/subnet_info.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pallets/subtensor/src/rpc_info/subnet_info.rs b/pallets/subtensor/src/rpc_info/subnet_info.rs index 312a20723..8c79db03a 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("e8abe48842dcc8c4")] +#[freeze_struct("4ceb81dfe8a8f96d")] #[derive(Decode, Encode, PartialEq, Eq, Clone, Debug)] pub struct SubnetHyperparams { rho: Compact, @@ -76,6 +76,7 @@ pub struct SubnetHyperparams { max_validators: Compact, adjustment_alpha: Compact, difficulty: Compact, + commit_reveal_periods: Compact, commit_reveal_weights_enabled: bool, alpha_high: Compact, alpha_low: Compact, @@ -251,6 +252,7 @@ impl Pallet { let max_validators = Self::get_max_allowed_validators(netuid); let adjustment_alpha = Self::get_adjustment_alpha(netuid); let difficulty = Self::get_difficulty_as_u64(netuid); + let commit_reveal_periods = Self::get_reveal_period(netuid); let commit_reveal_weights_enabled = Self::get_commit_reveal_weights_enabled(netuid); let liquid_alpha_enabled = Self::get_liquid_alpha_enabled(netuid); let (alpha_low, alpha_high): (u16, u16) = Self::get_alpha_values(netuid); @@ -278,6 +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_enabled, alpha_high: alpha_high.into(), alpha_low: alpha_low.into(), From 1e273a78551837017716d0a438326158670725a9 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Thu, 17 Oct 2024 09:16:43 -0700 Subject: [PATCH 12/33] Update dispatch doc comments --- pallets/subtensor/src/macros/dispatches.rs | 26 ++++++++++++++++------ 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/pallets/subtensor/src/macros/dispatches.rs b/pallets/subtensor/src/macros/dispatches.rs index c4b985a49..bdfd3e6db 100644 --- a/pallets/subtensor/src/macros/dispatches.rs +++ b/pallets/subtensor/src/macros/dispatches.rs @@ -102,8 +102,11 @@ mod dispatches { /// - The hash representing the committed weights. /// /// # Raises: - /// * `WeightsCommitNotAllowed`: - /// - Attempting to commit when it is not allowed. + /// * `CommitRevealDisabled`: + /// - Attempting to commit when the commit-reveal mechanism is disabled. + /// + /// * `TooManyUnrevealedCommits`: + /// - Attempting to commit when the user has more than the allowed limit of unrevealed commits. /// #[pallet::call_index(96)] #[pallet::weight((Weight::from_parts(46_000_000, 0) @@ -132,21 +135,30 @@ mod dispatches { /// * `values` (`Vec`): /// - The values of the weights being revealed. /// - /// * `salt` (`Vec`): - /// - The random salt to protect from brute-force guessing attack in case of small weight changes bit-wise. + /// * `salt` (`Vec`): + /// - The salt used to generate the commit hash. /// /// * `version_key` (`u64`): /// - The network version key. /// /// # Raises: + /// * `CommitRevealDisabled`: + /// - Attempting to reveal weights when the commit-reveal mechanism is disabled. + /// /// * `NoWeightsCommitFound`: /// - Attempting to reveal weights without an existing commit. /// - /// * `InvalidRevealCommitHashNotMatchTempo`: - /// - Attempting to reveal weights outside the valid tempo. + /// * `ExpiredWeightCommit`: + /// - Attempting to reveal a weight commit that has expired. + /// + /// * `RevealTooEarly`: + /// - Attempting to reveal weights outside the valid reveal period. + /// + /// * `RevealOutOfOrder`: + /// - Attempting to reveal a commit out of the expected order. /// /// * `InvalidRevealCommitHashNotMatch`: - /// - The revealed hash does not match the committed hash. + /// - The revealed hash does not match any committed hash. /// #[pallet::call_index(97)] #[pallet::weight((Weight::from_parts(103_000_000, 0) From 36810269b0e7fa66b5995626cb7dc6c74139dc55 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Thu, 17 Oct 2024 14:14:54 -0700 Subject: [PATCH 13/33] add extrinsic batch_reveal_weights --- pallets/subtensor/src/macros/dispatches.rs | 61 +++ pallets/subtensor/src/macros/errors.rs | 2 + pallets/subtensor/src/subnets/weights.rs | 147 ++++++ pallets/subtensor/tests/weights.rs | 572 +++++++++++++++++++++ 4 files changed, 782 insertions(+) diff --git a/pallets/subtensor/src/macros/dispatches.rs b/pallets/subtensor/src/macros/dispatches.rs index bdfd3e6db..19baa2a4b 100644 --- a/pallets/subtensor/src/macros/dispatches.rs +++ b/pallets/subtensor/src/macros/dispatches.rs @@ -175,6 +175,67 @@ mod dispatches { Self::do_reveal_weights(origin, netuid, uids, values, salt, version_key) } + /// ---- 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. + #[pallet::call_index(98)] + #[pallet::weight((Weight::from_parts(103_000_000, 0) + .saturating_add(T::DbWeight::get().reads(11)) + .saturating_add(T::DbWeight::get().writes(3)), DispatchClass::Normal, Pays::No))] + pub fn batch_reveal_weights( + origin: T::RuntimeOrigin, + netuid: u16, + uids_list: Vec>, + values_list: Vec>, + salts_list: Vec>, + version_keys: Vec, + ) -> DispatchResult { + Self::do_batch_reveal_weights( + origin, + netuid, + uids_list, + values_list, + salts_list, + version_keys, + ) + } + /// # Args: /// * `origin`: (Origin): /// - The caller, a hotkey who wishes to set their weights. diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index ac54cff1c..1e4bf9ae0 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -186,5 +186,7 @@ mod errors { ExpiredWeightCommit, /// Attempted to reveal weights too early. RevealTooEarly, + /// Attempted to batch reveal weights with mismatched vector input lenghts. + InputLengthsUnequal, } } diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 690fee200..61130d18a 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -202,6 +202,153 @@ 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 + ); + + // --- 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; + } + } + + // --- 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)?; + } 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()); + } + } + } + + // --- 7. If the queue is now empty, remove the storage entry for the user. + if commits.is_empty() { + *maybe_commits = None; + } + + // --- 8. Return ok. + Ok(()) + }) + } + /// ---- The implementation for the extrinsic set_weights. /// /// # Args: diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 3fe8bed82..c52739482 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -3103,3 +3103,575 @@ fn test_reveal_at_exact_block() { } }); } + +#[test] +fn test_successful_batch_reveal() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey = U256::from(1); + let version_keys: Vec = vec![0, 0, 0]; + let uids_list: Vec> = vec![vec![0, 1], vec![1, 0], vec![0, 1]]; + let weight_values_list: Vec> = vec![vec![10, 20], vec![30, 40], vec![50, 60]]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + + register_ok_neuron(netuid, U256::from(3), U256::from(4), 300_000); + register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); + SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + + // 1. Commit multiple times + let mut commit_info = Vec::new(); + for i in 0..3 { + let salt: Vec = vec![i as u16; 8]; + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids_list[i].clone(), + weight_values_list[i].clone(), + salt.clone(), + version_keys[i], + )); + commit_info.push((commit_hash, salt)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + } + + step_epochs(1, netuid); + + // 2. Prepare batch reveal parameters + let salts_list: Vec> = commit_info.iter().map(|(_, salt)| salt.clone()).collect(); + + // 3. Perform batch reveal + assert_ok!(SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list.clone(), + weight_values_list.clone(), + salts_list.clone(), + version_keys.clone(), + )); + + // 4. Ensure all commits are removed + let commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey); + assert!(commits.is_none()); + }); +} + +#[test] +fn test_batch_reveal_with_expired_commits() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey = U256::from(1); + let version_keys: Vec = vec![0, 0, 0]; + let uids_list: Vec> = vec![vec![0, 1], vec![1, 0], vec![0, 1]]; + let weight_values_list: Vec> = vec![vec![10, 20], vec![30, 40], vec![50, 60]]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + + register_ok_neuron(netuid, U256::from(3), U256::from(4), 300_000); + register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); + SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + + let mut commit_info = Vec::new(); + + // 1. Commit the first weight in epoch 0 + let salt0: Vec = vec![0u16; 8]; + let commit_hash0: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids_list[0].clone(), + weight_values_list[0].clone(), + salt0.clone(), + version_keys[0], + )); + commit_info.push((commit_hash0, salt0)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash0 + )); + + // Advance to epoch 1 + step_epochs(1, netuid); + + // 2. Commit the next two weights in epoch 1 + for i in 1..3 { + let salt: Vec = vec![i as u16; 8]; + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids_list[i].clone(), + weight_values_list[i].clone(), + salt.clone(), + version_keys[i], + )); + commit_info.push((commit_hash, salt)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + } + + // Advance to epoch 2 (after reveal period for first commit) + step_epochs(1, netuid); + + // 3. Prepare batch reveal parameters + let salts_list: Vec> = commit_info.iter().map(|(_, salt)| salt.clone()).collect(); + + // 4. Perform batch reveal + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list.clone(), + weight_values_list.clone(), + salts_list.clone(), + version_keys.clone(), + ); + assert_err!(result, Error::::ExpiredWeightCommit); + + // 5. Expired commit is not removed until a successful call + let commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey) + .expect("Expected remaining commits"); + assert_eq!(commits.len(), 3); + + // 6. Try revealing the remaining commits + let valid_uids_list = uids_list[1..].to_vec(); + let valid_weight_values_list = weight_values_list[1..].to_vec(); + let valid_salts_list = salts_list[1..].to_vec(); + let valid_version_keys = version_keys[1..].to_vec(); + + assert_ok!(SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + valid_uids_list, + valid_weight_values_list, + valid_salts_list, + valid_version_keys, + )); + + // 7. Ensure all commits are removed + let commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey); + assert!(commits.is_none()); + }); +} + +#[test] +fn test_batch_reveal_with_invalid_input_lengths() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey = U256::from(1); + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + + // Base data for valid inputs + let uids_list: Vec> = vec![vec![0, 1], vec![1, 0]]; + let weight_values_list: Vec> = vec![vec![10, 20], vec![30, 40]]; + let salts_list: Vec> = vec![vec![0u16; 8], vec![1u16; 8]]; + let version_keys: Vec = vec![0, 0]; + + // Test cases with mismatched input lengths + + // Case 1: uids_list has an extra element + let uids_list_case = vec![vec![0, 1], vec![1, 0], vec![2, 3]]; + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list_case.clone(), + weight_values_list.clone(), + salts_list.clone(), + version_keys.clone(), + ); + assert_err!(result, Error::::InputLengthsUnequal); + + // Case 2: weight_values_list has an extra element + let weight_values_list_case = vec![vec![10, 20], vec![30, 40], vec![50, 60]]; + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list.clone(), + weight_values_list_case.clone(), + salts_list.clone(), + version_keys.clone(), + ); + assert_err!(result, Error::::InputLengthsUnequal); + + // Case 3: salts_list has an extra element + let salts_list_case = vec![vec![0u16; 8], vec![1u16; 8], vec![2u16; 8]]; + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list.clone(), + weight_values_list.clone(), + salts_list_case.clone(), + version_keys.clone(), + ); + assert_err!(result, Error::::InputLengthsUnequal); + + // Case 4: version_keys has an extra element + let version_keys_case = vec![0, 0, 0]; + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list.clone(), + weight_values_list.clone(), + salts_list.clone(), + version_keys_case.clone(), + ); + assert_err!(result, Error::::InputLengthsUnequal); + + // Case 5: All input vectors have mismatched lengths + let uids_list_case = vec![vec![0, 1]]; + let weight_values_list_case = vec![vec![10, 20], vec![30, 40]]; + let salts_list_case = vec![vec![0u16; 8]]; + let version_keys_case = vec![0, 0, 0]; + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list_case, + weight_values_list_case, + salts_list_case, + version_keys_case, + ); + assert_err!(result, Error::::InputLengthsUnequal); + + // Case 6: Valid input lengths (should not return an error) + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list.clone(), + weight_values_list.clone(), + salts_list.clone(), + version_keys.clone(), + ); + // We expect an error because no commits have been made, but it should not be InputLengthsUnequal + assert_err!(result, Error::::NoWeightsCommitFound); + }); +} + +#[test] +fn test_batch_reveal_with_no_commits() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey = U256::from(1); + let version_keys: Vec = vec![0]; + let uids_list: Vec> = vec![vec![0, 1]]; + let weight_values_list: Vec> = vec![vec![10, 20]]; + let salts_list: Vec> = vec![vec![0u16; 8]]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + + // 1. Attempt to perform batch reveal without any commits + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list, + weight_values_list, + salts_list, + version_keys, + ); + assert_err!(result, Error::::NoWeightsCommitFound); + }); +} + +#[test] +fn test_batch_reveal_before_reveal_period() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey = U256::from(1); + let version_keys: Vec = vec![0, 0]; + let uids_list: Vec> = vec![vec![0, 1], vec![1, 0]]; + let weight_values_list: Vec> = vec![vec![10, 20], vec![30, 40]]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + + register_ok_neuron(netuid, U256::from(3), U256::from(4), 300_000); + register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + + // 1. Commit multiple times in the same epoch + let mut commit_info = Vec::new(); + for i in 0..2 { + let salt: Vec = vec![i as u16; 8]; + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids_list[i].clone(), + weight_values_list[i].clone(), + salt.clone(), + version_keys[i], + )); + commit_info.push((commit_hash, salt)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + } + + // 2. Prepare batch reveal parameters + let salts_list: Vec> = commit_info.iter().map(|(_, salt)| salt.clone()).collect(); + + // 3. Attempt to reveal before reveal period + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list.clone(), + weight_values_list.clone(), + salts_list.clone(), + version_keys.clone(), + ); + assert_err!(result, Error::::RevealTooEarly); + }); +} + +#[test] +fn test_batch_reveal_after_commits_expired() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey = U256::from(1); + let version_keys: Vec = vec![0, 0]; + let uids_list: Vec> = vec![vec![0, 1], vec![1, 0]]; + let weight_values_list: Vec> = vec![vec![10, 20], vec![30, 40]]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + + register_ok_neuron(netuid, U256::from(3), U256::from(4), 300_000); + register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + + let mut commit_info = Vec::new(); + + // 1. Commit the first weight in epoch 0 + let salt0: Vec = vec![0u16; 8]; + let commit_hash0: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids_list[0].clone(), + weight_values_list[0].clone(), + salt0.clone(), + version_keys[0], + )); + commit_info.push((commit_hash0, salt0)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash0 + )); + + // Advance to epoch 1 + step_epochs(1, netuid); + + // 2. Commit the second weight in epoch 1 + let salt1: Vec = vec![1u16; 8]; + let commit_hash1: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids_list[1].clone(), + weight_values_list[1].clone(), + salt1.clone(), + version_keys[1], + )); + commit_info.push((commit_hash1, salt1)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash1 + )); + + // Advance to epoch 4 to ensure both commits have expired (assuming reveal_period is 1) + step_epochs(3, netuid); + + // 3. Prepare batch reveal parameters + let salts_list: Vec> = commit_info.iter().map(|(_, salt)| salt.clone()).collect(); + + // 4. Attempt to reveal after commits have expired + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list.clone(), + weight_values_list.clone(), + salts_list, + version_keys.clone(), + ); + assert_err!(result, Error::::ExpiredWeightCommit); + }); +} + +#[test] +fn test_batch_reveal_when_commit_reveal_disabled() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey = U256::from(1); + let version_keys: Vec = vec![0]; + let uids_list: Vec> = vec![vec![0, 1]]; + let weight_values_list: Vec> = vec![vec![10, 20]]; + let salts_list: Vec> = vec![vec![0u16; 8]]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, false); + + // 1. Attempt to perform batch reveal when commit-reveal is disabled + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list, + weight_values_list, + salts_list, + version_keys, + ); + assert_err!(result, Error::::CommitRevealDisabled); + }); +} + +#[test] +fn test_batch_reveal_with_out_of_order_commits() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let hotkey = U256::from(1); + let version_keys: Vec = vec![0, 0, 0]; + let uids_list: Vec> = vec![vec![0, 1], vec![1, 0], vec![0, 1]]; + let weight_values_list: Vec> = vec![vec![10, 20], vec![30, 40], vec![50, 60]]; + let tempo: u16 = 100; + + System::set_block_number(0); + add_network(netuid, tempo, 0); + + SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + + register_ok_neuron(netuid, U256::from(3), U256::from(4), 300_000); + register_ok_neuron(netuid, hotkey, U256::from(2), 100_000); + SubtensorModule::set_validator_permit_for_uid(netuid, 0, true); + SubtensorModule::set_validator_permit_for_uid(netuid, 1, true); + + // 1. Commit multiple times + let mut commit_info = Vec::new(); + for i in 0..3 { + let salt: Vec = vec![i as u16; 8]; + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids_list[i].clone(), + weight_values_list[i].clone(), + salt.clone(), + version_keys[i], + )); + commit_info.push((commit_hash, salt)); + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(hotkey), + netuid, + commit_hash + )); + } + + step_epochs(1, netuid); + + // 2. Prepare batch reveal parameters 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 + ]; + let uids_list_out_of_order = vec![ + uids_list[2].clone(), + uids_list[0].clone(), + uids_list[1].clone(), + ]; + let weight_values_list_out_of_order = vec![ + weight_values_list[2].clone(), + weight_values_list[0].clone(), + weight_values_list[1].clone(), + ]; + let version_keys_out_of_order = vec![version_keys[2], version_keys[0], version_keys[1]]; + + // 3. Attempt batch reveal out of order + let result = SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + uids_list_out_of_order, + weight_values_list_out_of_order, + salts_list, + 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], + )); + + // 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]]; + + assert_ok!(SubtensorModule::do_batch_reveal_weights( + RuntimeOrigin::signed(hotkey), + netuid, + remaining_uids_list, + remaining_weight_values_list, + remaining_salts, + remaining_version_keys, + )); + + // 7. Ensure all commits are removed + let commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey); + assert!(commits.is_none()); + }); +} From adeac4cfd508282c8e89273f83cbbeef98dc5bb2 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Fri, 18 Oct 2024 18:42:08 -0700 Subject: [PATCH 14/33] fix doc comment --- pallets/subtensor/src/macros/dispatches.rs | 3 --- pallets/subtensor/src/subnets/weights.rs | 3 --- 2 files changed, 6 deletions(-) diff --git a/pallets/subtensor/src/macros/dispatches.rs b/pallets/subtensor/src/macros/dispatches.rs index 19baa2a4b..42c8e3c1a 100644 --- a/pallets/subtensor/src/macros/dispatches.rs +++ b/pallets/subtensor/src/macros/dispatches.rs @@ -154,9 +154,6 @@ mod dispatches { /// * `RevealTooEarly`: /// - Attempting to reveal weights outside the valid reveal period. /// - /// * `RevealOutOfOrder`: - /// - Attempting to reveal a commit out of the expected order. - /// /// * `InvalidRevealCommitHashNotMatch`: /// - The revealed hash does not match any committed hash. /// diff --git a/pallets/subtensor/src/subnets/weights.rs b/pallets/subtensor/src/subnets/weights.rs index 61130d18a..ba673afa4 100644 --- a/pallets/subtensor/src/subnets/weights.rs +++ b/pallets/subtensor/src/subnets/weights.rs @@ -102,9 +102,6 @@ impl Pallet { /// * `RevealTooEarly`: /// - Attempting to reveal weights outside the valid reveal period. /// - /// * `RevealOutOfOrder`: - /// - Attempting to reveal a commit out of the expected order. - /// /// * `InvalidRevealCommitHashNotMatch`: /// - The revealed hash does not match any committed hash. pub fn do_reveal_weights( From d7a64ba809af85cbfecbea1ccab1a61edc916af6 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Mon, 21 Oct 2024 09:53:14 -0700 Subject: [PATCH 15/33] update benchmarks --- pallets/admin-utils/src/benchmarking.rs | 11 +++ pallets/admin-utils/src/lib.rs | 2 +- pallets/admin-utils/src/weights.rs | 22 ++++++ pallets/subtensor/src/benchmarks.rs | 78 ++++++++++++++++++++++ pallets/subtensor/src/macros/dispatches.rs | 4 +- 5 files changed, 114 insertions(+), 3 deletions(-) diff --git a/pallets/admin-utils/src/benchmarking.rs b/pallets/admin-utils/src/benchmarking.rs index 3d8b962f6..606814ff0 100644 --- a/pallets/admin-utils/src/benchmarking.rs +++ b/pallets/admin-utils/src/benchmarking.rs @@ -227,6 +227,17 @@ mod benchmarks { _(RawOrigin::Root, 1u16/*netuid*/, 1u16/*tempo*/)/*sudo_set_tempo*/; } + #[benchmark] + fn sudo_set_commit_reveal_weights_periods() { + pallet_subtensor::Pallet::::init_new_network( + 1u16, /*netuid*/ + 1u16, /*sudo_tempo*/ + ); + + #[extrinsic_call] + _(RawOrigin::Root, 1u16/*netuid*/, 3u64/*interval*/)/*set_commit_reveal_weights_periods()*/; + } + #[benchmark] fn sudo_set_commit_reveal_weights_enabled() { pallet_subtensor::Pallet::::init_new_network( diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index b2f59668e..a287bb1b4 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -1188,7 +1188,7 @@ pub mod pallet { /// # Weight /// Weight is handled by the `#[pallet::weight]` attribute. #[pallet::call_index(56)] - #[pallet::weight((0, DispatchClass::Operational, Pays::No))] + #[pallet::weight(T::WeightInfo::sudo_set_commit_reveal_weights_periods())] pub fn sudo_set_commit_reveal_weights_periods( origin: OriginFor, netuid: u16, diff --git a/pallets/admin-utils/src/weights.rs b/pallets/admin-utils/src/weights.rs index ba2247dfd..db8752ff7 100644 --- a/pallets/admin-utils/src/weights.rs +++ b/pallets/admin-utils/src/weights.rs @@ -60,6 +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_enabled() -> Weight; } @@ -412,6 +413,15 @@ 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 { + // Proof Size summary in bytes: + // Measured: `456` + // Estimated: `3921` + // Minimum execution time: 19_070_000 picoseconds. + Weight::from_parts(19_380_000, 456) + .saturating_add(T::DbWeight::get().reads(1_u64)) + .saturating_add(T::DbWeight::get().writes(1_u64)) + } fn sudo_set_commit_reveal_weights_enabled() -> Weight { // Proof Size summary in bytes: // Measured: `1111` @@ -771,6 +781,18 @@ 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 { + // -- Extrinsic Time -- + // Model: + // Time ~= 19.38 + // µs + // Reads = 1 + // Writes = 1 + // Recorded proof Size = 456 + Weight::from_parts(19_380_000, 456) + .saturating_add(RocksDbWeight::get().reads(1)) + .saturating_add(RocksDbWeight::get().writes(1)) + } fn sudo_set_commit_reveal_weights_enabled() -> Weight { // -- Extrinsic Time -- // Model: diff --git a/pallets/subtensor/src/benchmarks.rs b/pallets/subtensor/src/benchmarks.rs index bd48676b6..6fd1cbf8b 100644 --- a/pallets/subtensor/src/benchmarks.rs +++ b/pallets/subtensor/src/benchmarks.rs @@ -520,4 +520,82 @@ reveal_weights { // Benchmark setup complete, now execute the extrinsic }: swap_coldkey(RawOrigin::Root, old_coldkey.clone(), new_coldkey.clone()) +batch_reveal_weights { + let tempo: u16 = 0; + let netuid: u16 = 1; + let num_commits: usize = 10; + + let hotkey: T::AccountId = account("hot", 0, 1); + let coldkey: T::AccountId = account("cold", 0, 2); + + Subtensor::::init_new_network(netuid, tempo); + Subtensor::::set_network_registration_allowed(netuid, true); + Subtensor::::set_network_pow_registration_allowed(netuid, true); + Subtensor::::set_commit_reveal_weights_enabled(netuid, true); + Subtensor::::set_weights_set_rate_limit(netuid, 0); // Disable rate limiting for benchmarking + + let block_number: u64 = Subtensor::::get_current_block_as_u64(); + let (nonce, work): (u64, Vec) = Subtensor::::create_work_for_block_number( + netuid, + block_number, + 3, + &hotkey, + ); + + let origin = T::RuntimeOrigin::from(RawOrigin::Signed(hotkey.clone())); + assert_ok!(Subtensor::::register( + origin.clone(), + netuid, + block_number, + nonce, + work.clone(), + hotkey.clone(), + coldkey.clone(), + )); + + let uid: u16 = 0; + + Subtensor::::set_validator_permit_for_uid(netuid, uid, true); + + let mut uids_list = Vec::new(); + let mut values_list = Vec::new(); + let mut salts_list = Vec::new(); + let mut version_keys = Vec::new(); + + for i in 0..num_commits { + let uids: Vec = vec![uid]; + let values: Vec = vec![i as u16]; + let salt: Vec = vec![i as u16]; + let version_key_i: u64 = i as u64; + + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey.clone(), + netuid, + uids.clone(), + values.clone(), + salt.clone(), + version_key_i, + )); + + assert_ok!(Subtensor::::commit_weights( + T::RuntimeOrigin::from(RawOrigin::Signed(hotkey.clone())), + netuid, + commit_hash, + )); + + uids_list.push(uids); + values_list.push(values); + salts_list.push(salt); + version_keys.push(version_key_i); + } +}: batch_reveal_weights( + RawOrigin::Signed(hotkey.clone()), + netuid, + uids_list, + values_list, + salts_list, + version_keys +) + + } diff --git a/pallets/subtensor/src/macros/dispatches.rs b/pallets/subtensor/src/macros/dispatches.rs index 42c8e3c1a..e98ecbd6a 100644 --- a/pallets/subtensor/src/macros/dispatches.rs +++ b/pallets/subtensor/src/macros/dispatches.rs @@ -212,8 +212,8 @@ mod dispatches { /// * `InvalidInputLengths`: /// - The input vectors are of mismatched lengths. #[pallet::call_index(98)] - #[pallet::weight((Weight::from_parts(103_000_000, 0) - .saturating_add(T::DbWeight::get().reads(11)) + #[pallet::weight((Weight::from_parts(367_612_000, 0) + .saturating_add(T::DbWeight::get().reads(14)) .saturating_add(T::DbWeight::get().writes(3)), DispatchClass::Normal, Pays::No))] pub fn batch_reveal_weights( origin: T::RuntimeOrigin, From 75ccbf167bf1f8732b7c053283213c9d39cba820 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 23 Oct 2024 09:25:16 -0700 Subject: [PATCH 16/33] set priority for batch reveals --- pallets/subtensor/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 02b164c0a..5081d69d3 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -1461,6 +1461,18 @@ where Err(InvalidTransaction::Custom(2).into()) } } + Some(Call::batch_reveal_weights { netuid, .. }) => { + if Self::check_weights_min_stake(who) { + let priority: u64 = Self::get_priority_set_weights(who, *netuid); + Ok(ValidTransaction { + priority, + longevity: 1, + ..Default::default() + }) + } else { + Err(InvalidTransaction::Custom(6).into()) + } + } Some(Call::set_weights { netuid, .. }) => { if Self::check_weights_min_stake(who) { let priority: u64 = Self::get_priority_set_weights(who, *netuid); From 7385f10a86fd6f1d30ba0a0f6d4c1f1b8fcef432 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 23 Oct 2024 14:42:38 -0700 Subject: [PATCH 17/33] add migrate_commit_reveal_2 --- pallets/subtensor/src/macros/hooks.rs | 4 +- .../migrations/migrate_commit_reveal_v2.rs | 82 +++++++++++++++++ pallets/subtensor/src/migrations/mod.rs | 1 + pallets/subtensor/tests/migration.rs | 88 ++++++++++++++++++- 4 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs diff --git a/pallets/subtensor/src/macros/hooks.rs b/pallets/subtensor/src/macros/hooks.rs index 76f140002..1077acb76 100644 --- a/pallets/subtensor/src/macros/hooks.rs +++ b/pallets/subtensor/src/macros/hooks.rs @@ -70,7 +70,9 @@ mod hooks { // Storage version v8 -> v9 .saturating_add(migrations::migrate_fix_total_coldkey_stake::migrate_fix_total_coldkey_stake::()) // Migrate Delegate Ids on chain - .saturating_add(migrations::migrate_chain_identity::migrate_set_hotkey_identities::()); + .saturating_add(migrations::migrate_chain_identity::migrate_set_hotkey_identities::()) + // Migrate Commit-Reval 2.0 + .saturating_add(migrations::migrate_commit_reveal_v2::migrate_commit_reveal_2::()); weight } diff --git a/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs b/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs new file mode 100644 index 000000000..6d0ef09f0 --- /dev/null +++ b/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs @@ -0,0 +1,82 @@ +use super::*; +use crate::HasMigrationRun; +use frame_support::{traits::Get, weights::Weight}; +use scale_info::prelude::string::String; +use sp_io::{storage::clear_prefix, KillStorageResult}; + +pub fn migrate_commit_reveal_2() -> Weight { + let migration_name = b"migrate_commit_reveal_2".to_vec(); + let mut weight = T::DbWeight::get().reads(1); + + if HasMigrationRun::::get(&migration_name) { + log::info!( + "Migration '{:?}' has already run. Skipping.", + migration_name + ); + return weight; + } + + log::info!( + "Running migration '{}'", + String::from_utf8_lossy(&migration_name) + ); + + // ------------------------------ + // Step 1: Remove WeightCommitRevealInterval entries + // ------------------------------ + + const WEIGHT_COMMIT_REVEAL_INTERVAL_PREFIX: &[u8] = + b"pallet_subtensor::WeightCommitRevealInterval"; + let removal_results = clear_prefix(WEIGHT_COMMIT_REVEAL_INTERVAL_PREFIX, Some(u32::MAX)); + + let removed_entries_count = match removal_results { + KillStorageResult::AllRemoved(removed) => removed as u64, + KillStorageResult::SomeRemaining(removed) => { + log::info!("Failed To Remove Some Items During migrate_commit_reveal_v2",); + removed as u64 + } + }; + + weight = weight.saturating_add(T::DbWeight::get().writes(removed_entries_count)); + + log::info!( + "Removed {:?} entries from WeightCommitRevealInterval.", + removed_entries_count + ); + + // ------------------------------ + // Step 2: Remove WeightCommits entries + // ------------------------------ + + const WEIGHT_COMMITS_PREFIX: &[u8] = b"pallet_subtensor::WeightCommits"; + let removal_results_commits = clear_prefix(WEIGHT_COMMITS_PREFIX, Some(u32::MAX)); + + let removed_commits_entries = match removal_results_commits { + KillStorageResult::AllRemoved(removed) => removed as u64, + KillStorageResult::SomeRemaining(removed) => { + log::info!("Failed To Remove Some Items During migrate_commit_reveal_v2",); + removed as u64 + } + }; + + weight = weight.saturating_add(T::DbWeight::get().writes(removed_commits_entries)); + + log::info!( + "Removed {} entries from WeightCommits.", + removed_commits_entries + ); + + // ------------------------------ + // Step 3: Mark Migration as Completed + // ------------------------------ + + HasMigrationRun::::insert(&migration_name, true); + weight = weight.saturating_add(T::DbWeight::get().writes(1)); + + log::info!( + "Migration '{:?}' completed successfully.", + String::from_utf8_lossy(&migration_name) + ); + + weight +} diff --git a/pallets/subtensor/src/migrations/mod.rs b/pallets/subtensor/src/migrations/mod.rs index 6036b23e0..a0ee65998 100644 --- a/pallets/subtensor/src/migrations/mod.rs +++ b/pallets/subtensor/src/migrations/mod.rs @@ -1,5 +1,6 @@ use super::*; pub mod migrate_chain_identity; +pub mod migrate_commit_reveal_v2; pub mod migrate_create_root_network; pub mod migrate_delete_subnet_21; pub mod migrate_delete_subnet_3; diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index 6c40d7d78..4ce21dcf6 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -1,10 +1,18 @@ #![allow(unused, clippy::indexing_slicing, clippy::panic, clippy::unwrap_used)] mod mock; -use frame_support::{assert_ok, weights::Weight}; +use codec::{Decode, Encode}; +use frame_support::{ + assert_ok, + storage::unhashed::{get_raw, put_raw}, + traits::{StorageInstance, StoredMap}, + weights::Weight, + StorageHasher, Twox64Concat, +}; use frame_system::Config; use mock::*; use pallet_subtensor::*; -use sp_core::U256; +use sp_core::{H256, U256}; +use sp_runtime::traits::Zero; #[test] fn test_initialise_ti() { @@ -430,3 +438,79 @@ fn run_migration_and_check(migration_name: &'static str) -> frame_support::weigh // Return the weight of the executed migration weight } + +#[test] +fn test_migrate_commit_reveal_2() { + new_test_ext(1).execute_with(|| { + // ------------------------------ + // Step 1: Simulate Old Storage Entries + // ------------------------------ + const MIGRATION_NAME: &str = "migrate_commit_reveal_2"; + const WEIGHT_COMMIT_REVEAL_INTERVAL_PREFIX: &[u8] = + b"pallet_subtensor::WeightCommitRevealInterval"; + const WEIGHT_COMMITS_PREFIX: &[u8] = b"pallet_subtensor::WeightCommits"; + + let netuid: u16 = 1; + let interval_value: u64 = 50u64; + + let mut interval_key = WEIGHT_COMMIT_REVEAL_INTERVAL_PREFIX.to_vec(); + interval_key.extend_from_slice(&netuid.encode()); + + put_raw(&interval_key, &interval_value.encode()); + + let test_account: U256 = U256::from(1); + + let mut commit_key = WEIGHT_COMMITS_PREFIX.to_vec(); + commit_key.extend_from_slice(&Twox64Concat::hash(&netuid.encode())); + commit_key.extend_from_slice(&Twox64Concat::hash(&test_account.encode())); + + let commit_value: (H256, u64) = (H256::from_low_u64_be(42), 100); + put_raw(&commit_key, &commit_value.encode()); + + let stored_interval = get_raw(&interval_key).expect("Expected to get a value"); + assert_eq!( + u64::decode(&mut &stored_interval[..]).expect("Failed to decode interval value"), + interval_value + ); + + let stored_commit = get_raw(&commit_key).expect("Expected to get a value"); + assert_eq!( + <(H256, u64)>::decode(&mut &stored_commit[..]).expect("Failed to decode commit value"), + commit_value + ); + + assert!( + !HasMigrationRun::::get(MIGRATION_NAME.as_bytes().to_vec()), + "Migration should not have run yet" + ); + + // ------------------------------ + // Step 2: Run the Migration + // ------------------------------ + let weight = + pallet_subtensor::migrations::migrate_commit_reveal_v2::migrate_commit_reveal_2::( + ); + + assert!( + HasMigrationRun::::get(MIGRATION_NAME.as_bytes().to_vec()), + "Migration should be marked as run" + ); + + // ------------------------------ + // Step 3: Verify Migration Effects + // ------------------------------ + let stored_interval_after = get_raw(&interval_key); + assert!( + stored_interval_after.is_none(), + "WeightCommitRevealInterval should be cleared" + ); + + let stored_commit_after = get_raw(&commit_key); + assert!( + stored_commit_after.is_none(), + "WeightCommits entry should be cleared" + ); + + assert!(!weight.is_zero(), "Migration weight should be non-zero"); + }); +} From 8d5de806df593b5831f8bedae9702928b0ea9e3d Mon Sep 17 00:00:00 2001 From: johnreedv Date: Wed, 23 Oct 2024 15:46:06 -0700 Subject: [PATCH 18/33] fix storage item migration keys --- .../migrations/migrate_commit_reveal_v2.rs | 22 +++++++++------ pallets/subtensor/tests/migration.rs | 28 ++++++++++++++----- 2 files changed, 35 insertions(+), 15 deletions(-) diff --git a/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs b/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs index 6d0ef09f0..cd93842c9 100644 --- a/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs +++ b/pallets/subtensor/src/migrations/migrate_commit_reveal_v2.rs @@ -2,7 +2,7 @@ use super::*; use crate::HasMigrationRun; use frame_support::{traits::Get, weights::Weight}; use scale_info::prelude::string::String; -use sp_io::{storage::clear_prefix, KillStorageResult}; +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(); @@ -25,14 +25,17 @@ pub fn migrate_commit_reveal_2() -> Weight { // Step 1: Remove WeightCommitRevealInterval entries // ------------------------------ - const WEIGHT_COMMIT_REVEAL_INTERVAL_PREFIX: &[u8] = - b"pallet_subtensor::WeightCommitRevealInterval"; - let removal_results = clear_prefix(WEIGHT_COMMIT_REVEAL_INTERVAL_PREFIX, Some(u32::MAX)); + let mut weight_commit_reveal_interval_prefix = Vec::new(); + weight_commit_reveal_interval_prefix.extend_from_slice(&twox_128("SubtensorModule".as_bytes())); + weight_commit_reveal_interval_prefix + .extend_from_slice(&twox_128("WeightCommitRevealInterval".as_bytes())); + + let removal_results = clear_prefix(&weight_commit_reveal_interval_prefix, Some(u32::MAX)); let removed_entries_count = match removal_results { KillStorageResult::AllRemoved(removed) => removed as u64, KillStorageResult::SomeRemaining(removed) => { - log::info!("Failed To Remove Some Items During migrate_commit_reveal_v2",); + log::info!("Failed To Remove Some Items During migrate_commit_reveal_v2"); removed as u64 } }; @@ -48,13 +51,16 @@ pub fn migrate_commit_reveal_2() -> Weight { // Step 2: Remove WeightCommits entries // ------------------------------ - const WEIGHT_COMMITS_PREFIX: &[u8] = b"pallet_subtensor::WeightCommits"; - let removal_results_commits = clear_prefix(WEIGHT_COMMITS_PREFIX, Some(u32::MAX)); + let mut weight_commits_prefix = Vec::new(); + weight_commits_prefix.extend_from_slice(&twox_128("SubtensorModule".as_bytes())); + weight_commits_prefix.extend_from_slice(&twox_128("WeightCommits".as_bytes())); + + let removal_results_commits = clear_prefix(&weight_commits_prefix, Some(u32::MAX)); let removed_commits_entries = match removal_results_commits { KillStorageResult::AllRemoved(removed) => removed as u64, KillStorageResult::SomeRemaining(removed) => { - log::info!("Failed To Remove Some Items During migrate_commit_reveal_v2",); + log::info!("Failed To Remove Some Items During migrate_commit_reveal_v2"); removed as u64 } }; diff --git a/pallets/subtensor/tests/migration.rs b/pallets/subtensor/tests/migration.rs index 4ce21dcf6..1317bfb0f 100644 --- a/pallets/subtensor/tests/migration.rs +++ b/pallets/subtensor/tests/migration.rs @@ -12,6 +12,7 @@ use frame_system::Config; use mock::*; use pallet_subtensor::*; use sp_core::{H256, U256}; +use sp_io::hashing::twox_128; use sp_runtime::traits::Zero; #[test] @@ -446,23 +447,36 @@ fn test_migrate_commit_reveal_2() { // Step 1: Simulate Old Storage Entries // ------------------------------ const MIGRATION_NAME: &str = "migrate_commit_reveal_2"; - const WEIGHT_COMMIT_REVEAL_INTERVAL_PREFIX: &[u8] = - b"pallet_subtensor::WeightCommitRevealInterval"; - const WEIGHT_COMMITS_PREFIX: &[u8] = b"pallet_subtensor::WeightCommits"; + + let pallet_prefix = twox_128("SubtensorModule".as_bytes()); + let storage_prefix_interval = twox_128("WeightCommitRevealInterval".as_bytes()); + let storage_prefix_commits = twox_128("WeightCommits".as_bytes()); let netuid: u16 = 1; let interval_value: u64 = 50u64; - let mut interval_key = WEIGHT_COMMIT_REVEAL_INTERVAL_PREFIX.to_vec(); + // Construct the full key for WeightCommitRevealInterval + let mut interval_key = Vec::new(); + interval_key.extend_from_slice(&pallet_prefix); + interval_key.extend_from_slice(&storage_prefix_interval); interval_key.extend_from_slice(&netuid.encode()); put_raw(&interval_key, &interval_value.encode()); let test_account: U256 = U256::from(1); - let mut commit_key = WEIGHT_COMMITS_PREFIX.to_vec(); - commit_key.extend_from_slice(&Twox64Concat::hash(&netuid.encode())); - commit_key.extend_from_slice(&Twox64Concat::hash(&test_account.encode())); + // Construct the full key for WeightCommits (DoubleMap) + let mut commit_key = Vec::new(); + commit_key.extend_from_slice(&pallet_prefix); + commit_key.extend_from_slice(&storage_prefix_commits); + + // First key (netuid) hashed with Twox64Concat + let netuid_hashed = Twox64Concat::hash(&netuid.encode()); + commit_key.extend_from_slice(&netuid_hashed); + + // Second key (account) hashed with Twox64Concat + let account_hashed = Twox64Concat::hash(&test_account.encode()); + commit_key.extend_from_slice(&account_hashed); let commit_value: (H256, u64) = (H256::from_low_u64_be(42), 100); put_raw(&commit_key, &commit_value.encode()); From ecd3612d2af95111a077a2ebd1bbbdeb743aa8aa Mon Sep 17 00:00:00 2001 From: johnreedv Date: Thu, 24 Oct 2024 07:54:37 -0700 Subject: [PATCH 19/33] add highly_concurrent test --- pallets/subtensor/tests/weights.rs | 279 +++++++++++++++++++++++++++++ 1 file changed, 279 insertions(+) diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index c52739482..4f925a80d 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -15,6 +15,7 @@ use sp_runtime::{ }; use sp_std::collections::vec_deque::VecDeque; use substrate_fixed::types::I32F32; +use scale_info::prelude::collections::HashMap; /*************************** pub fn set_weights() tests @@ -3675,3 +3676,281 @@ fn test_batch_reveal_with_out_of_order_commits() { assert!(commits.is_none()); }); } + +#[test] +fn test_highly_concurrent_commits_and_reveals_with_multiple_hotkeys() { + new_test_ext(1).execute_with(|| { + // ==== Test Configuration ==== + let netuid: u16 = 1; + let num_hotkeys: usize = 10; + let max_unrevealed_commits: usize = 10; + let commits_per_hotkey: usize = 20; + let initial_reveal_period: u64 = 5; + let initial_tempo: u16 = 100; + + // ==== Setup Network ==== + 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); + SubtensorModule::set_max_registrations_per_block(netuid, u16::MAX); + SubtensorModule::set_target_registrations_per_interval(netuid, u16::MAX); + + // ==== Register Validators ==== + for uid in 0..5 { + let validator_id = U256::from(100 + uid as u64); + register_ok_neuron(netuid, validator_id, U256::from(200 + uid as u64), 300_000); + SubtensorModule::set_validator_permit_for_uid(netuid, uid, true); + } + + // ==== Register Hotkeys ==== + let mut hotkeys: Vec<::AccountId> = Vec::new(); + for i in 0..num_hotkeys { + let hotkey_id = U256::from(1000 + i as u64); + register_ok_neuron(netuid, hotkey_id, U256::from(2000 + i as u64), 100_000); + hotkeys.push(hotkey_id); + } + + // ==== Initialize Commit Information ==== + let mut commit_info_map: HashMap< + ::AccountId, + Vec<(H256, Vec, Vec, Vec, u64)>, + > = HashMap::new(); + + // Initialize the map + for hotkey in &hotkeys { + commit_info_map.insert(*hotkey, Vec::new()); + } + + // ==== Function to Generate Unique Data ==== + fn generate_unique_data(index: usize) -> (Vec, Vec, Vec, u64) { + let uids = vec![index as u16, (index + 1) as u16]; + let values = vec![(index * 10) as u16, ((index + 1) * 10) as u16]; + let salt = vec![(index % 100) as u16; 8]; + let version_key = index as u64; + (uids, values, salt, version_key) + } + + // ==== Simulate Concurrent Commits and Reveals ==== + for i in 0..commits_per_hotkey { + for hotkey in &hotkeys { + + let current_commits = pallet_subtensor::WeightCommits::::get(netuid, hotkey) + .unwrap_or_default(); + if current_commits.len() >= max_unrevealed_commits { + continue; + } + + let (uids, values, salt, version_key) = generate_unique_data(i); + let commit_hash: H256 = BlakeTwo256::hash_of(&( + *hotkey, + netuid, + uids.clone(), + values.clone(), + salt.clone(), + version_key, + )); + + if let Some(commits) = commit_info_map.get_mut(hotkey) { + commits.push((commit_hash, salt.clone(), uids.clone(), values.clone(), version_key)); + } + + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(*hotkey), + netuid, + commit_hash + )); + } + + // ==== Reveal Phase ==== + for hotkey in &hotkeys { + if let Some(commits) = commit_info_map.get_mut(hotkey) { + if commits.is_empty() { + continue; // No commits to reveal + } + + let (_commit_hash, salt, uids, values, version_key) = commits.first().expect("expected a value"); + + let reveal_result = SubtensorModule::reveal_weights( + RuntimeOrigin::signed(*hotkey), + netuid, + uids.clone(), + values.clone(), + salt.clone(), + *version_key, + ); + + match reveal_result { + Ok(_) => { + commits.remove(0); + } + Err(e) => { + if e == Error::::RevealTooEarly.into() + || e == Error::::ExpiredWeightCommit.into() + || e == Error::::InvalidRevealCommitHashNotMatch.into() + { + log::info!("Expected error during reveal after epoch advancement: {:?}", e); + } else { + panic!( + "Unexpected error during reveal: {:?}, expected RevealTooEarly, ExpiredWeightCommit, or InvalidRevealCommitHashNotMatch", + e + ); + } + } + } + } + } + } + + // ==== Modify Network Parameters During Commits ==== + SubtensorModule::set_tempo(netuid, 150); + SubtensorModule::set_reveal_period(netuid, 7); + log::info!("Changed tempo to 150 and reveal_period to 7 during commits."); + + step_epochs(3, netuid); + + // ==== Continue Reveals After Epoch Advancement ==== + for hotkey in &hotkeys { + if let Some(commits) = commit_info_map.get_mut(hotkey) { + while !commits.is_empty() { + let (_commit_hash, salt, uids, values, version_key) = &commits[0]; + + // Attempt to reveal + let reveal_result = SubtensorModule::reveal_weights( + RuntimeOrigin::signed(*hotkey), + netuid, + uids.clone(), + values.clone(), + salt.clone(), + *version_key, + ); + + match reveal_result { + Ok(_) => { + commits.remove(0); + } + Err(e) => { + // Check if the error is due to reveal being too early or commit expired + if e == Error::::RevealTooEarly.into() + || e == Error::::ExpiredWeightCommit.into() + || e == Error::::InvalidRevealCommitHashNotMatch.into() + { + log::info!("Expected error during reveal after epoch advancement: {:?}", e); + break; + } else { + panic!( + "Unexpected error during reveal after epoch advancement: {:?}, expected RevealTooEarly, ExpiredWeightCommit, or InvalidRevealCommitHashNotMatch", + e + ); + } + } + } + } + } + } + + // ==== Change Network Parameters Again ==== + SubtensorModule::set_tempo(netuid, 200); + SubtensorModule::set_reveal_period(netuid, 10); + log::info!("Changed tempo to 200 and reveal_period to 10 after initial reveals."); + + step_epochs(10, netuid); + + // ==== Final Reveal Attempts ==== + for (hotkey, commits) in commit_info_map.iter_mut() { + for (_commit_hash, salt, uids, values, version_key) in commits.iter() { + let reveal_result = SubtensorModule::reveal_weights( + RuntimeOrigin::signed(*hotkey), + netuid, + uids.clone(), + values.clone(), + salt.clone(), + *version_key, + ); + + assert_eq!( + reveal_result, + Err(Error::::ExpiredWeightCommit.into()), + "Expected ExpiredWeightCommit error, got {:?}", + reveal_result + ); + } + } + + for hotkey in &hotkeys { + commit_info_map.insert(*hotkey, Vec::new()); + + for i in 0..max_unrevealed_commits { + let (uids, values, salt, version_key) = generate_unique_data(i + commits_per_hotkey); + let commit_hash: H256 = BlakeTwo256::hash_of(&( + *hotkey, + netuid, + uids.clone(), + values.clone(), + salt.clone(), + version_key, + )); + + assert_ok!(SubtensorModule::commit_weights( + RuntimeOrigin::signed(*hotkey), + netuid, + commit_hash + )); + } + + let (uids, values, salt, version_key) = generate_unique_data(max_unrevealed_commits + commits_per_hotkey); + let commit_hash: H256 = BlakeTwo256::hash_of(&( + *hotkey, + netuid, + uids.clone(), + values.clone(), + salt.clone(), + version_key, + )); + + assert_err!( + SubtensorModule::commit_weights( + RuntimeOrigin::signed(*hotkey), + netuid, + commit_hash + ), + Error::::TooManyUnrevealedCommits + ); + } + + // Attempt unauthorized reveal + let unauthorized_hotkey = hotkeys[0]; + let target_hotkey = hotkeys[1]; + if let Some(commits) = commit_info_map.get(&target_hotkey) { + if let Some((_commit_hash, salt, uids, values, version_key)) = commits.first() { + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(unauthorized_hotkey), + netuid, + uids.clone(), + values.clone(), + salt.clone(), + *version_key, + ), + Error::::InvalidRevealCommitHashNotMatch + ); + } + } + + let non_committing_hotkey: ::AccountId = U256::from(9999); + assert_err!( + SubtensorModule::reveal_weights( + RuntimeOrigin::signed(non_committing_hotkey), + netuid, + vec![0, 1], + vec![10, 20], + vec![0; 8], + 0, + ), + Error::::NoWeightsCommitFound + ); + + assert_eq!(SubtensorModule::get_reveal_period(netuid), 10); + assert_eq!(SubtensorModule::get_tempo(netuid), 200); + }) +} From 6e21d6acb36837690a9f9f919f55f3725379fac1 Mon Sep 17 00:00:00 2001 From: johnreedv Date: Thu, 24 Oct 2024 07:57:54 -0700 Subject: [PATCH 20/33] cargo fmt --- pallets/subtensor/tests/weights.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 4f925a80d..ddc2ccd77 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -8,6 +8,7 @@ use frame_support::{ }; use mock::*; use pallet_subtensor::{Error, Owner}; +use scale_info::prelude::collections::HashMap; use sp_core::{H256, U256}; use sp_runtime::{ traits::{BlakeTwo256, DispatchInfoOf, Hash, SignedExtension}, @@ -15,7 +16,6 @@ use sp_runtime::{ }; use sp_std::collections::vec_deque::VecDeque; use substrate_fixed::types::I32F32; -use scale_info::prelude::collections::HashMap; /*************************** pub fn set_weights() tests From 8b1f71ea55421ae124835dee41d03a06d7e1e467 Mon Sep 17 00:00:00 2001 From: Liam Date: Thu, 24 Oct 2024 21:36:04 +0400 Subject: [PATCH 21/33] better try-runtime ci --- .github/workflows/check-devnet.yml | 15 -------- .github/workflows/check-finney.yml | 14 -------- .github/workflows/check-testnet.yml | 15 -------- .github/workflows/try-runtime.yml | 54 +++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 44 deletions(-) create mode 100644 .github/workflows/try-runtime.yml diff --git a/.github/workflows/check-devnet.yml b/.github/workflows/check-devnet.yml index 2cb586348..8f04d78cf 100644 --- a/.github/workflows/check-devnet.yml +++ b/.github/workflows/check-devnet.yml @@ -39,18 +39,3 @@ jobs: echo "network spec_version: $spec_version" if (( $(echo "$local_spec_version <= $spec_version" | bc -l) )); then echo "$local_spec_version ≯ $spec_version ❌"; exit 1; fi echo "$local_spec_version > $spec_version ✅" - - check-devnet-migrations: - name: check devnet migrations - runs-on: ubuntu-22.04 - steps: - - name: Checkout sources - uses: actions/checkout@v3 - - - name: Run Try Runtime Checks - uses: "paritytech/try-runtime-gha@v0.1.0" - with: - runtime-package: "node-subtensor-runtime" - node-uri: "wss://dev.chain.opentensor.ai:443" - checks: "pre-and-post" - extra-args: "--disable-spec-version-check --no-weight-warnings" diff --git a/.github/workflows/check-finney.yml b/.github/workflows/check-finney.yml index 665c9c8a9..947b9a902 100644 --- a/.github/workflows/check-finney.yml +++ b/.github/workflows/check-finney.yml @@ -39,17 +39,3 @@ jobs: echo "network spec_version: $spec_version" if (( $(echo "$local_spec_version <= $spec_version" | bc -l) )); then echo "$local_spec_version ≯ $spec_version ❌"; exit 1; fi echo "$local_spec_version > $spec_version ✅" - - check-finney-migrations: - name: check finney migrations - runs-on: SubtensorCI - steps: - - name: Checkout sources - uses: actions/checkout@v4 - - name: Run Try Runtime Checks - uses: "paritytech/try-runtime-gha@v0.1.0" - with: - runtime-package: "node-subtensor-runtime" - node-uri: "wss://entrypoint-finney.opentensor.ai:443" - checks: "pre-and-post" - extra-args: "--disable-spec-version-check --no-weight-warnings" diff --git a/.github/workflows/check-testnet.yml b/.github/workflows/check-testnet.yml index 95277c94a..a869129ab 100644 --- a/.github/workflows/check-testnet.yml +++ b/.github/workflows/check-testnet.yml @@ -39,18 +39,3 @@ jobs: echo "network spec_version: $spec_version" if (( $(echo "$local_spec_version <= $spec_version" | bc -l) )); then echo "$local_spec_version ≯ $spec_version ❌"; exit 1; fi echo "$local_spec_version > $spec_version ✅" - - check-testnet-migrations: - name: check testnet migrations - runs-on: ubuntu-22.04 - steps: - - name: Checkout sources - uses: actions/checkout@v3 - - - name: Run Try Runtime Checks - uses: "paritytech/try-runtime-gha@v0.1.0" - with: - runtime-package: "node-subtensor-runtime" - node-uri: "wss://test.chain.opentensor.ai:443" - checks: "pre-and-post" - extra-args: "--disable-spec-version-check --no-weight-warnings" diff --git a/.github/workflows/try-runtime.yml b/.github/workflows/try-runtime.yml new file mode 100644 index 000000000..174e6db37 --- /dev/null +++ b/.github/workflows/try-runtime.yml @@ -0,0 +1,54 @@ +name: Try Runtime + +on: + pull_request: + branches: [main, devnet-ready, devnet, testnet, finney] + types: [labeled, unlabeled, synchronize] + +env: + CARGO_TERM_COLOR: always + +jobs: + check-devnet: + name: check devnet + runs-on: SubtensorCI + steps: + - name: Checkout sources + uses: actions/checkout@v3 + + - name: Run Try Runtime Checks + uses: "paritytech/try-runtime-gha@v0.1.0" + with: + runtime-package: "node-subtensor-runtime" + node-uri: "wss://dev.chain.opentensor.ai:443" + checks: "all" + extra-args: "--disable-spec-version-check --no-weight-warnings" + + check-testnet: + name: check testnet + runs-on: SubtensorCI + steps: + - name: Checkout sources + uses: actions/checkout@v3 + + - name: Run Try Runtime Checks + uses: "paritytech/try-runtime-gha@v0.1.0" + with: + runtime-package: "node-subtensor-runtime" + node-uri: "wss://test.chain.opentensor.ai:443" + checks: "all" + extra-args: "--disable-spec-version-check --no-weight-warnings" + + check-finney: + name: check finney + runs-on: SubtensorCI + steps: + - name: Checkout sources + uses: actions/checkout@v4 + - name: Run Try Runtime Checks + uses: "paritytech/try-runtime-gha@v0.1.0" + with: + runtime-package: "node-subtensor-runtime" + node-uri: "wss://archive.chain.opentensor.ai:443" + checks: "all" + extra-args: "--disable-spec-version-check --no-weight-warnings" From d81cbaa2b044ab1c8ce571def7015fa018b22267 Mon Sep 17 00:00:00 2001 From: Greg Zaitsev Date: Tue, 29 Oct 2024 15:32:06 -0400 Subject: [PATCH 22/33] Make devnet-ready mergable with main --- .../subtensor/src/coinbase/run_coinbase.rs | 64 +++++++++++-------- pallets/subtensor/src/lib.rs | 13 ++-- pallets/subtensor/src/staking/add_stake.rs | 6 +- pallets/subtensor/src/swap/swap_hotkey.rs | 48 ++++++++++---- 4 files changed, 85 insertions(+), 46 deletions(-) diff --git a/pallets/subtensor/src/coinbase/run_coinbase.rs b/pallets/subtensor/src/coinbase/run_coinbase.rs index 723edc423..badb811fa 100644 --- a/pallets/subtensor/src/coinbase/run_coinbase.rs +++ b/pallets/subtensor/src/coinbase/run_coinbase.rs @@ -265,69 +265,65 @@ impl Pallet { // --- 1.0 Drain the hotkey emission. PendingdHotkeyEmission::::insert(hotkey, 0); - // --- 2 Retrieve the last time this hotkey's emissions were drained. - let last_emission_drain: u64 = LastHotkeyEmissionDrain::::get(hotkey); - - // --- 3 Update the block value to the current block number. + // --- 2 Update the block value to the current block number. LastHotkeyEmissionDrain::::insert(hotkey, block_number); - // --- 4 Retrieve the total stake for the hotkey from all nominations. + // --- 3 Retrieve the total stake for the hotkey from all nominations. let total_hotkey_stake: u64 = Self::get_total_stake_for_hotkey(hotkey); - // --- 5 Calculate the emission take for the hotkey. + // --- 4 Calculate the emission take for the hotkey. let take_proportion: I64F64 = I64F64::from_num(Delegates::::get(hotkey)) .saturating_div(I64F64::from_num(u16::MAX)); let hotkey_take: u64 = (take_proportion.saturating_mul(I64F64::from_num(emission))).to_num::(); - // --- 6 Compute the remaining emission after deducting the hotkey's take. + // --- 5 Compute the remaining emission after deducting the hotkey's take. let emission_minus_take: u64 = emission.saturating_sub(hotkey_take); - // --- 7 Calculate the remaining emission after the hotkey's take. + // --- 6 Calculate the remaining emission after the hotkey's take. let mut remainder: u64 = emission_minus_take; - // --- 8 Iterate over each nominator and get all viable stake. + // --- 7 Iterate over each nominator and get all viable stake. let mut total_viable_nominator_stake: u64 = total_hotkey_stake; - for (nominator, nominator_stake) in Stake::::iter_prefix(hotkey) { - if LastAddStakeIncrease::::get(hotkey, nominator) > last_emission_drain { - total_viable_nominator_stake = - total_viable_nominator_stake.saturating_sub(nominator_stake); - } + for (nominator, _) in Stake::::iter_prefix(hotkey) { + let nonviable_nomintaor_stake = Self::get_nonviable_stake(hotkey, &nominator); + + total_viable_nominator_stake = + total_viable_nominator_stake.saturating_sub(nonviable_nomintaor_stake); } - // --- 9 Iterate over each nominator. + // --- 8 Iterate over each nominator. if total_viable_nominator_stake != 0 { for (nominator, nominator_stake) in Stake::::iter_prefix(hotkey) { - // --- 10 Check if the stake was manually increased by the user since the last emission drain for this hotkey. + // --- 9 Check if the stake was manually increased by the user since the last emission drain for this hotkey. // If it was, skip this nominator as they will not receive their proportion of the emission. - if LastAddStakeIncrease::::get(hotkey, nominator.clone()) > last_emission_drain { - continue; - } + let viable_nominator_stake = + nominator_stake.saturating_sub(Self::get_nonviable_stake(hotkey, &nominator)); - // --- 11 Calculate this nominator's share of the emission. - let nominator_emission: I64F64 = I64F64::from_num(emission_minus_take) - .saturating_mul(I64F64::from_num(nominator_stake)) + // --- 10 Calculate this nominator's share of the emission. + let nominator_emission: I64F64 = I64F64::from_num(viable_nominator_stake) .checked_div(I64F64::from_num(total_viable_nominator_stake)) - .unwrap_or(I64F64::from_num(0)); + .unwrap_or(I64F64::from_num(0)) + .saturating_mul(I64F64::from_num(emission_minus_take)); - // --- 12 Increase the stake for the nominator. + // --- 11 Increase the stake for the nominator. Self::increase_stake_on_coldkey_hotkey_account( &nominator, hotkey, nominator_emission.to_num::(), ); - // --- 13* Record event and Subtract the nominator's emission from the remainder. + // --- 12* Record event and Subtract the nominator's emission from the remainder. total_new_tao = total_new_tao.saturating_add(nominator_emission.to_num::()); remainder = remainder.saturating_sub(nominator_emission.to_num::()); } } - // --- 14 Finally, add the stake to the hotkey itself, including its take and the remaining emission. + // --- 13 Finally, add the stake to the hotkey itself, including its take and the remaining emission. let hotkey_new_tao: u64 = hotkey_take.saturating_add(remainder); Self::increase_stake_on_hotkey_account(hotkey, hotkey_new_tao); - // --- 15 Record new tao creation event and return the amount created. + // --- 14 Record new tao creation event and return the amount created. total_new_tao = total_new_tao.saturating_add(hotkey_new_tao); total_new_tao } @@ -382,4 +378,18 @@ impl Pallet { let remainder = block_plus_netuid.rem_euclid(tempo_plus_one); (tempo as u64).saturating_sub(remainder) } + + /// Calculates the nonviable stake for a nominator. + /// The nonviable stake is the stake that was added by the nominator since the last emission drain. + /// This stake will not receive emission until the next emission drain. + /// Note: if the stake delta is below zero, we return zero. We don't allow more stake than the nominator has. + pub fn get_nonviable_stake(hotkey: &T::AccountId, nominator: &T::AccountId) -> u64 { + let stake_delta = StakeDeltaSinceLastEmissionDrain::::get(hotkey, nominator); + if stake_delta.is_negative() { + 0 + } else { + // Should never fail the into, but we handle it anyway. + stake_delta.try_into().unwrap_or(u64::MAX) + } + } } diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 5081d69d3..8f2910222 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -259,6 +259,11 @@ pub mod pallet { 0 } #[pallet::type_value] + /// Default stake delta. + pub fn DefaultStakeDelta() -> i128 { + 0 + } + #[pallet::type_value] /// Default stakes per interval. pub fn DefaultStakesPerInterval() -> (u64, u64) { (0, 0) @@ -791,16 +796,16 @@ pub mod pallet { DefaultAccumulatedEmission, >; #[pallet::storage] - /// Map ( hot, cold ) --> block_number | Last add stake increase. - pub type LastAddStakeIncrease = StorageDoubleMap< + /// Map ( hot, cold ) --> stake: i128 | Stake added/removed since last emission drain. + pub type StakeDeltaSinceLastEmissionDrain = StorageDoubleMap< _, Blake2_128Concat, T::AccountId, Identity, T::AccountId, - u64, + i128, ValueQuery, - DefaultAccountTake, + DefaultStakeDelta, >; #[pallet::storage] /// DMAP ( parent, netuid ) --> Vec<(proportion,child)> diff --git a/pallets/subtensor/src/staking/add_stake.rs b/pallets/subtensor/src/staking/add_stake.rs index c9cbd7e04..72d8374bc 100644 --- a/pallets/subtensor/src/staking/add_stake.rs +++ b/pallets/subtensor/src/staking/add_stake.rs @@ -70,8 +70,10 @@ impl Pallet { Error::::StakeRateLimitExceeded ); - // Set the last time the stake increased for nominator drain protection. - LastAddStakeIncrease::::insert(&hotkey, &coldkey, Self::get_current_block_as_u64()); + // Track this addition in the stake delta. + StakeDeltaSinceLastEmissionDrain::::mutate(&hotkey, &coldkey, |stake_delta| { + *stake_delta = stake_delta.saturating_add_unsigned(stake_to_be_added as u128); + }); // If coldkey is not owner of the hotkey, it's a nomination stake. if !Self::coldkey_owns_hotkey(&coldkey, &hotkey) { diff --git a/pallets/subtensor/src/swap/swap_hotkey.rs b/pallets/subtensor/src/swap/swap_hotkey.rs index ca3d0b5a7..efa6c53f8 100644 --- a/pallets/subtensor/src/swap/swap_hotkey.rs +++ b/pallets/subtensor/src/swap/swap_hotkey.rs @@ -206,33 +206,42 @@ impl Pallet { Delegates::::insert(new_hotkey, old_delegate_take); weight.saturating_accrue(T::DbWeight::get().reads_writes(2, 2)); } - // 9. Swap all subnet specific info. + + // 9. swap PendingdHotkeyEmission + if PendingdHotkeyEmission::::contains_key(old_hotkey) { + let old_pending_hotkey_emission = PendingdHotkeyEmission::::get(old_hotkey); + PendingdHotkeyEmission::::remove(old_hotkey); + PendingdHotkeyEmission::::insert(new_hotkey, old_pending_hotkey_emission); + weight.saturating_accrue(T::DbWeight::get().reads_writes(2, 2)); + } + + // 10. Swap all subnet specific info. let all_netuids: Vec = Self::get_all_subnet_netuids(); for netuid in all_netuids { - // 9.1 Remove the previous hotkey and insert the new hotkey from membership. + // 10.1 Remove the previous hotkey and insert the new hotkey from membership. // IsNetworkMember( hotkey, netuid ) -> bool -- is the hotkey a subnet member. let is_network_member: bool = IsNetworkMember::::get(old_hotkey, netuid); IsNetworkMember::::remove(old_hotkey, netuid); IsNetworkMember::::insert(new_hotkey, netuid, is_network_member); weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 2)); - // 9.2 Swap Uids + Keys. + // 10.2 Swap Uids + Keys. // Keys( netuid, hotkey ) -> uid -- the uid the hotkey has in the network if it is a member. // Uids( netuid, hotkey ) -> uid -- the uids that the hotkey has. if is_network_member { - // 9.2.1 Swap the UIDS + // 10.2.1 Swap the UIDS if let Ok(old_uid) = Uids::::try_get(netuid, old_hotkey) { Uids::::remove(netuid, old_hotkey); Uids::::insert(netuid, new_hotkey, old_uid); weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 2)); - // 9.2.2 Swap the keys. + // 10.2.2 Swap the keys. Keys::::insert(netuid, old_uid, new_hotkey.clone()); weight.saturating_accrue(T::DbWeight::get().reads_writes(0, 1)); } } - // 9.3 Swap Prometheus. + // 10.3 Swap Prometheus. // Prometheus( netuid, hotkey ) -> prometheus -- the prometheus data that a hotkey has in the network. if is_network_member { if let Ok(old_prometheus_info) = Prometheus::::try_get(netuid, old_hotkey) { @@ -242,7 +251,7 @@ impl Pallet { } } - // 9.4. Swap axons. + // 10.4. Swap axons. // Axons( netuid, hotkey ) -> axon -- the axon that the hotkey has. if is_network_member { if let Ok(old_axon_info) = Axons::::try_get(netuid, old_hotkey) { @@ -252,7 +261,7 @@ impl Pallet { } } - // 9.5 Swap WeightCommits + // 10.5 Swap WeightCommits // WeightCommits( hotkey ) --> Vec -- the weight commits for the hotkey. if is_network_member { if let Ok(old_weight_commits) = WeightCommits::::try_get(netuid, old_hotkey) { @@ -262,7 +271,7 @@ impl Pallet { } } - // 9.6. Swap the subnet loaded emission. + // 10.6. Swap the subnet loaded emission. // LoadedEmission( netuid ) --> Vec<(hotkey, u64)> -- the loaded emission for the subnet. if is_network_member { if let Some(mut old_loaded_emission) = LoadedEmission::::get(netuid) { @@ -277,7 +286,7 @@ impl Pallet { } } - // 9.7. Swap neuron TLS certificates. + // 10.7. Swap neuron TLS certificates. // NeuronCertificates( netuid, hotkey ) -> Vec -- the neuron certificate for the hotkey. if is_network_member { if let Ok(old_neuron_certificates) = @@ -290,7 +299,7 @@ impl Pallet { } } - // 10. Swap Stake. + // 11. Swap Stake. // Stake( hotkey, coldkey ) -> stake -- the stake that the hotkey controls on behalf of the coldkey. let stakes: Vec<(T::AccountId, u64)> = Stake::::iter_prefix(old_hotkey).collect(); // Clear the entire old prefix here. @@ -320,7 +329,7 @@ impl Pallet { weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); } - // 11. Swap ChildKeys. + // 12. Swap ChildKeys. // ChildKeys( parent, netuid ) --> Vec<(proportion,child)> -- the child keys of the parent. for netuid in Self::get_all_subnet_netuids() { // Get the children of the old hotkey for this subnet @@ -331,7 +340,7 @@ impl Pallet { ChildKeys::::insert(new_hotkey, netuid, my_children); } - // 12. Swap ParentKeys. + // 13. Swap ParentKeys. // ParentKeys( child, netuid ) --> Vec<(proportion,parent)> -- the parent keys of the child. for netuid in Self::get_all_subnet_netuids() { // Get the parents of the old hotkey for this subnet @@ -355,6 +364,19 @@ impl Pallet { } } + // 14. Swap Stake Delta for all coldkeys. + for (coldkey, stake_delta) in StakeDeltaSinceLastEmissionDrain::::iter_prefix(old_hotkey) + { + let new_stake_delta = StakeDeltaSinceLastEmissionDrain::::get(new_hotkey, &coldkey); + StakeDeltaSinceLastEmissionDrain::::insert( + new_hotkey, + &coldkey, + new_stake_delta.saturating_add(stake_delta), + ); + StakeDeltaSinceLastEmissionDrain::::remove(old_hotkey, &coldkey); + weight.saturating_accrue(T::DbWeight::get().reads_writes(2, 2)); + } + // Return successful after swapping all the relevant terms. Ok(()) } From 653ba75dde85712f140956eeefb2a2fec29e9451 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 29 Oct 2024 16:14:22 -0400 Subject: [PATCH 23/33] fix cargo audit label --- .github/workflows/check-rust.yml | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/.github/workflows/check-rust.yml b/.github/workflows/check-rust.yml index e42e369cf..4ebdf6d56 100644 --- a/.github/workflows/check-rust.yml +++ b/.github/workflows/check-rust.yml @@ -212,16 +212,7 @@ jobs: cargo-audit: name: cargo audit runs-on: SubtensorCI - strategy: - matrix: - rust-branch: - - stable - rust-target: - - x86_64-unknown-linux-gnu - # - x86_64-apple-darwin - os: - - ubuntu-latest - # - macos-latest + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'audit')) }} steps: - name: Check-out repositoroy under $GITHUB_WORKSPACE uses: actions/checkout@v4 From f94b4be77997045e9755d123cbef8d905fa606f4 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 29 Oct 2024 16:20:54 -0400 Subject: [PATCH 24/33] fix --- .github/workflows/check-rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check-rust.yml b/.github/workflows/check-rust.yml index 4ebdf6d56..6723a8d73 100644 --- a/.github/workflows/check-rust.yml +++ b/.github/workflows/check-rust.yml @@ -212,7 +212,7 @@ jobs: cargo-audit: name: cargo audit runs-on: SubtensorCI - if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'audit')) }} + if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'skip-cargo-audit')) }} steps: - name: Check-out repositoroy under $GITHUB_WORKSPACE uses: actions/checkout@v4 From c64caf1a556356c3ab94474219de29bddad916e3 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 29 Oct 2024 16:29:45 -0400 Subject: [PATCH 25/33] bump spec version --- runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 6990cb907..54de0e56c 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -146,7 +146,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: 197, + spec_version: 203, impl_version: 1, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From 05de4a511edee9ca8a2c63474db1777bb8b58f68 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 29 Oct 2024 16:51:48 -0400 Subject: [PATCH 26/33] fix toolchain --- .github/workflows/check-rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check-rust.yml b/.github/workflows/check-rust.yml index 6723a8d73..5fe92f607 100644 --- a/.github/workflows/check-rust.yml +++ b/.github/workflows/check-rust.yml @@ -225,7 +225,7 @@ jobs: - name: Install Rust ${{ matrix.rust-branch }} uses: actions-rs/toolchain@v1.0.6 with: - toolchain: ${{ matrix.rust-branch }} + toolchain: stable components: rustfmt, clippy profile: minimal From beca39b4721c6cb9210d4f62b923c9684e746124 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 29 Oct 2024 16:53:38 -0400 Subject: [PATCH 27/33] bump CI From 50f8a9dbcfea2257a6ae210db59f1f9b5cbaf7e4 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 29 Oct 2024 16:56:12 -0400 Subject: [PATCH 28/33] fix --- .github/workflows/check-rust.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/check-rust.yml b/.github/workflows/check-rust.yml index 5fe92f607..116ffe282 100644 --- a/.github/workflows/check-rust.yml +++ b/.github/workflows/check-rust.yml @@ -222,7 +222,7 @@ jobs: sudo apt-get update && sudo apt-get install -y clang curl libssl-dev llvm libudev-dev protobuf-compiler - - name: Install Rust ${{ matrix.rust-branch }} + - name: Install Rust Stable uses: actions-rs/toolchain@v1.0.6 with: toolchain: stable @@ -232,7 +232,7 @@ jobs: - name: Utilize Shared Rust Cache uses: Swatinem/rust-cache@v2.2.1 with: - key: ${{ matrix.os }}-${{ env.RUST_BIN_DIR }} + key: ubuntu-latest-${{ env.RUST_BIN_DIR }} - name: Install cargo-audit run: cargo install cargo-audit From f37dd7a83be6f34786e012f16694e673db6d90d9 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 29 Oct 2024 17:44:41 -0400 Subject: [PATCH 29/33] hotfix --- .github/workflows/check-rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check-rust.yml b/.github/workflows/check-rust.yml index 116ffe282..03dc22496 100644 --- a/.github/workflows/check-rust.yml +++ b/.github/workflows/check-rust.yml @@ -212,7 +212,7 @@ jobs: cargo-audit: name: cargo audit runs-on: SubtensorCI - if: ${{ github.event_name == 'push' || (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'skip-cargo-audit')) }} + if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-cargo-audit') }} steps: - name: Check-out repositoroy under $GITHUB_WORKSPACE uses: actions/checkout@v4 From a389bcde2956bf578e6fc33b104b2b7c7abe8789 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 29 Oct 2024 17:52:02 -0400 Subject: [PATCH 30/33] bump CI From 69b7ac85e78fc3fc313bbb6bf320c5876b6aeda1 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Tue, 29 Oct 2024 17:56:13 -0400 Subject: [PATCH 31/33] fix again --- .github/workflows/check-rust.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check-rust.yml b/.github/workflows/check-rust.yml index 03dc22496..b6a627314 100644 --- a/.github/workflows/check-rust.yml +++ b/.github/workflows/check-rust.yml @@ -212,7 +212,7 @@ jobs: cargo-audit: name: cargo audit runs-on: SubtensorCI - if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-cargo-audit') }} + if: ${{ github.event_name != 'push' && !contains(github.event.pull_request.labels.*.name, 'skip-cargo-audit') }} steps: - name: Check-out repositoroy under $GITHUB_WORKSPACE uses: actions/checkout@v4 From dbb3ad33c9c61fb166590e03ee4dd342b6a6c2ed Mon Sep 17 00:00:00 2001 From: Greg Zaitsev Date: Wed, 30 Oct 2024 12:19:07 -0400 Subject: [PATCH 32/33] Reapply hotkey swap fix for childkey's parents --- pallets/subtensor/src/swap/swap_hotkey.rs | 15 +++- pallets/subtensor/tests/swap_hotkey.rs | 97 +++++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/pallets/subtensor/src/swap/swap_hotkey.rs b/pallets/subtensor/src/swap/swap_hotkey.rs index efa6c53f8..b16c180c4 100644 --- a/pallets/subtensor/src/swap/swap_hotkey.rs +++ b/pallets/subtensor/src/swap/swap_hotkey.rs @@ -337,7 +337,20 @@ impl Pallet { // Remove the old hotkey's child entries ChildKeys::::remove(old_hotkey, netuid); // Insert the same child entries for the new hotkey - ChildKeys::::insert(new_hotkey, netuid, my_children); + ChildKeys::::insert(new_hotkey, netuid, my_children.clone()); + for (_, child_key_i) in my_children { + // For each child, update their parent list + let mut child_parents: Vec<(u64, T::AccountId)> = + ParentKeys::::get(child_key_i.clone(), netuid); + for parent in child_parents.iter_mut() { + // If the parent is the old hotkey, replace it with the new hotkey + if parent.1 == *old_hotkey { + parent.1 = new_hotkey.clone(); + } + } + // Update the child's parent list + ParentKeys::::insert(child_key_i, netuid, child_parents); + } } // 13. Swap ParentKeys. diff --git a/pallets/subtensor/tests/swap_hotkey.rs b/pallets/subtensor/tests/swap_hotkey.rs index bf5ecb301..53524c72c 100644 --- a/pallets/subtensor/tests/swap_hotkey.rs +++ b/pallets/subtensor/tests/swap_hotkey.rs @@ -1148,3 +1148,100 @@ fn test_swap_complex_parent_child_structure() { ); }); } + +#[test] +fn test_swap_parent_hotkey_childkey_maps() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let parent_old = U256::from(1); + let coldkey = U256::from(2); + let child = U256::from(3); + let parent_new = U256::from(4); + add_network(netuid, 0, 0); + SubtensorModule::create_account_if_non_existent(&coldkey, &parent_old); + + // Set child and verify state maps + assert_ok!(SubtensorModule::do_set_children( + RuntimeOrigin::signed(coldkey), + parent_old, + netuid, + vec![(u64::MAX, child)] + )); + assert_eq!( + ParentKeys::::get(child, netuid), + vec![(u64::MAX, parent_old)] + ); + assert_eq!( + ChildKeys::::get(parent_old, netuid), + vec![(u64::MAX, child)] + ); + + // Swap + let mut weight = Weight::zero(); + assert_ok!(SubtensorModule::perform_hotkey_swap( + &parent_old, + &parent_new, + &coldkey, + &mut weight + )); + + // Verify parent and child keys updates + assert_eq!( + ParentKeys::::get(child, netuid), + vec![(u64::MAX, parent_new)] + ); + assert_eq!( + ChildKeys::::get(parent_new, netuid), + vec![(u64::MAX, child)] + ); + }) +} + +#[test] +fn test_swap_child_hotkey_childkey_maps() { + new_test_ext(1).execute_with(|| { + let netuid: u16 = 1; + let parent = U256::from(1); + let coldkey = U256::from(2); + let child_old = U256::from(3); + let child_new = U256::from(4); + add_network(netuid, 0, 0); + SubtensorModule::create_account_if_non_existent(&coldkey, &child_old); + SubtensorModule::create_account_if_non_existent(&coldkey, &parent); + + // Set child and verify state maps + assert_ok!(SubtensorModule::do_set_children( + RuntimeOrigin::signed(coldkey), + parent, + netuid, + vec![(u64::MAX, child_old)] + )); + assert_eq!( + ParentKeys::::get(child_old, netuid), + vec![(u64::MAX, parent)] + ); + assert_eq!( + ChildKeys::::get(parent, netuid), + vec![(u64::MAX, child_old)] + ); + + // Swap + let mut weight = Weight::zero(); + assert_ok!(SubtensorModule::perform_hotkey_swap( + &child_old, + &child_new, + &coldkey, + &mut weight + )); + + // Verify parent and child keys updates + assert_eq!( + ParentKeys::::get(child_new, netuid), + vec![(u64::MAX, parent)] + ); + assert_eq!( + ChildKeys::::get(parent, netuid), + vec![(u64::MAX, child_new)] + ); + }) +} From 2ad1381d50d48e99a2c196560087ffc853c74232 Mon Sep 17 00:00:00 2001 From: Sam Johnson Date: Wed, 30 Oct 2024 13:18:08 -0400 Subject: [PATCH 33/33] separate out cargo-audit so it can be canceled via label --- .github/workflows/cargo-audit.yml | 42 +++++++++++++++++++++++++++++++ .github/workflows/check-rust.yml | 31 ----------------------- 2 files changed, 42 insertions(+), 31 deletions(-) create mode 100644 .github/workflows/cargo-audit.yml diff --git a/.github/workflows/cargo-audit.yml b/.github/workflows/cargo-audit.yml new file mode 100644 index 000000000..4687eee59 --- /dev/null +++ b/.github/workflows/cargo-audit.yml @@ -0,0 +1,42 @@ +name: cargo audit +on: + pull_request: + types: + - labeled + - unlabeled + - synchronize +concurrency: + group: cargo-audit-${{ github.ref }} + cancel-in-progress: true + +jobs: + cargo-audit: + name: cargo audit + runs-on: SubtensorCI + if: ${{ !contains(github.event.pull_request.labels.*.name, 'skip-cargo-audit') }} + steps: + - name: Check-out repositoroy under $GITHUB_WORKSPACE + uses: actions/checkout@v4 + + - name: Install dependencies + run: | + sudo apt-get update && + sudo apt-get install -y clang curl libssl-dev llvm libudev-dev protobuf-compiler + + - name: Install Rust Stable + uses: actions-rs/toolchain@v1.0.6 + with: + toolchain: stable + components: rustfmt, clippy + profile: minimal + + - name: Utilize Shared Rust Cache + uses: Swatinem/rust-cache@v2.2.1 + with: + key: ubuntu-latest-${{ env.RUST_BIN_DIR }} + + - name: Install cargo-audit + run: cargo install cargo-audit + + - name: cargo audit + run: cargo audit --ignore RUSTSEC-2024-0336 # rustls issue; wait for upstream to resolve this diff --git a/.github/workflows/check-rust.yml b/.github/workflows/check-rust.yml index b6a627314..80d543163 100644 --- a/.github/workflows/check-rust.yml +++ b/.github/workflows/check-rust.yml @@ -208,37 +208,6 @@ jobs: - name: cargo clippy --workspace --all-targets --all-features -- -D warnings run: cargo clippy --workspace --all-targets --all-features -- -D warnings - # runs cargo audit - cargo-audit: - name: cargo audit - runs-on: SubtensorCI - if: ${{ github.event_name != 'push' && !contains(github.event.pull_request.labels.*.name, 'skip-cargo-audit') }} - steps: - - name: Check-out repositoroy under $GITHUB_WORKSPACE - uses: actions/checkout@v4 - - - name: Install dependencies - run: | - sudo apt-get update && - sudo apt-get install -y clang curl libssl-dev llvm libudev-dev protobuf-compiler - - - name: Install Rust Stable - uses: actions-rs/toolchain@v1.0.6 - with: - toolchain: stable - components: rustfmt, clippy - profile: minimal - - - name: Utilize Shared Rust Cache - uses: Swatinem/rust-cache@v2.2.1 - with: - key: ubuntu-latest-${{ env.RUST_BIN_DIR }} - - - name: Install cargo-audit - run: cargo install cargo-audit - - - name: cargo audit - run: cargo audit --ignore RUSTSEC-2024-0336 # rustls issue; wait for upstream to resolve this # runs cargo test --workspace cargo-test: