From cbc5e4badd045db46b3d6eda94d7b54c2cd9a6f9 Mon Sep 17 00:00:00 2001 From: Greg Zaitsev Date: Sun, 26 May 2024 21:35:09 -0400 Subject: [PATCH 1/3] Add salt to weights commit-reveal --- pallets/subtensor/src/benchmarks.rs | 4 +- pallets/subtensor/src/lib.rs | 9 +- pallets/subtensor/src/weights.rs | 5 + pallets/subtensor/tests/weights.rs | 136 +++++++++++++++++++++++++--- 4 files changed, 138 insertions(+), 16 deletions(-) diff --git a/pallets/subtensor/src/benchmarks.rs b/pallets/subtensor/src/benchmarks.rs index 8d35cd291..e2d2a29b6 100644 --- a/pallets/subtensor/src/benchmarks.rs +++ b/pallets/subtensor/src/benchmarks.rs @@ -388,6 +388,7 @@ reveal_weights { let version_key: u64 = 0; let uids: Vec = vec![0]; let weight_values: Vec = vec![10]; + let salt: Vec = vec![8]; let hotkey: T::AccountId = account("hot", 0, 1); let coldkey: T::AccountId = account("cold", 1, 2); @@ -421,9 +422,10 @@ reveal_weights { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, )); let _ = Subtensor::::commit_weights(::RuntimeOrigin::from(RawOrigin::Signed(hotkey.clone())), netuid, commit_hash); - }: reveal_weights(RawOrigin::Signed(hotkey.clone()), netuid, uids, weight_values, version_key) + }: reveal_weights(RawOrigin::Signed(hotkey.clone()), netuid, uids, weight_values, salt, version_key) } diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index a4dfcdbec..b1122b965 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -1399,7 +1399,7 @@ pub mod pallet { Self::do_commit_weights(origin, netuid, commit_hash) } - /// ---- Used to reveal the weights for a previously committed hash. + /// ---- Used to reveal the weights for a previously committed hash. /// /// # Args: /// * `origin`: (`::RuntimeOrigin`): @@ -1413,6 +1413,9 @@ pub mod pallet { /// /// * `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. /// /// * `version_key` (`u64`): /// - The network version key. @@ -1436,11 +1439,13 @@ pub mod pallet { netuid: u16, uids: Vec, values: Vec, + salt: Vec, version_key: u64, ) -> DispatchResult { - Self::do_reveal_weights(origin, netuid, uids, values, version_key) + Self::do_reveal_weights(origin, netuid, uids, values, salt, version_key) } + /// # Args: /// * `origin`: (Origin): /// - The caller, a hotkey who wishes to set their weights. diff --git a/pallets/subtensor/src/weights.rs b/pallets/subtensor/src/weights.rs index c7e31efd9..d99cb8542 100644 --- a/pallets/subtensor/src/weights.rs +++ b/pallets/subtensor/src/weights.rs @@ -60,6 +60,9 @@ impl Pallet { /// * `values` (`Vec`): /// - The values of the weights being revealed. /// + /// * `salt` (`Vec`): + /// - The values of the weights being revealed. + /// /// * `version_key` (`u64`): /// - The network version key. /// @@ -78,6 +81,7 @@ impl Pallet { netuid: u16, uids: Vec, values: Vec, + salt: Vec, version_key: u64, ) -> DispatchResult { let who = ensure_signed(origin.clone())?; @@ -103,6 +107,7 @@ impl Pallet { netuid, uids.clone(), values.clone(), + salt.clone(), version_key, )); ensure!(provided_hash == *commit_hash, Error::::InvalidReveal); diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 57fc897d6..f227c1838 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -44,11 +44,12 @@ fn test_commit_weights_dispatch_info_ok() { let dests = vec![1, 1]; let weights = vec![1, 1]; let netuid: u16 = 1; + 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, dests, weights, version_key)); + BlakeTwo256::hash_of(&(hotkey, netuid, dests, weights, salt, version_key)); let call = RuntimeCall::SubtensorModule(SubtensorCall::commit_weights { netuid, @@ -67,12 +68,14 @@ fn test_reveal_weights_dispatch_info_ok() { let dests = vec![1, 1]; let weights = vec![1, 1]; let netuid: u16 = 1; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let call = RuntimeCall::SubtensorModule(SubtensorCall::reveal_weights { netuid, uids: dests, values: weights, + salt: salt, version_key, }); let dispatch_info = call.get_dispatch_info(); @@ -89,11 +92,12 @@ 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, version_key), + commit_reveal_set_weights(hotkey, root_netuid, uids, weights, salt, version_key), Error::::IsRoot ); }); @@ -116,9 +120,10 @@ 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, netuid, weights_keys, weight_values, 0); + commit_reveal_set_weights(hotkey_account_id, netuid, weights_keys, weight_values, salt.clone(), 0); assert_eq!(result, Err(Error::::NoValidatorPermit.into())); let weights_keys: Vec = vec![1, 2]; @@ -128,7 +133,7 @@ fn test_weights_err_no_validator_permit() { .expect("Not registered."); SubtensorModule::set_validator_permit_for_uid(netuid, neuron_uid, true); let result = - commit_reveal_set_weights(hotkey_account_id, netuid, weights_keys, weight_values, 0); + commit_reveal_set_weights(hotkey_account_id, netuid, weights_keys, weight_values, salt, 0); assert_ok!(result); }); } @@ -144,6 +149,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); @@ -159,7 +165,7 @@ 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, netuid, dests.clone(), weights.clone(), version_key), + commit_reveal_set_weights(hotkey, netuid, dests.clone(), weights.clone(), salt.clone(), version_key), Err(Error::::NotEnoughStakeToSetWeights.into()) ); // Now passes @@ -169,6 +175,7 @@ fn test_set_weights_min_stake_failed() { netuid, dests.clone(), weights.clone(), + salt.clone(), version_key )); }); @@ -182,6 +189,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); @@ -194,6 +202,7 @@ fn test_weights_version_key() { netuid0, weights_keys.clone(), weight_values.clone(), + salt.clone(), 0 )); assert_ok!(commit_reveal_set_weights( @@ -201,6 +210,7 @@ fn test_weights_version_key() { netuid1, weights_keys.clone(), weight_values.clone(), + salt.clone(), 0 )); @@ -216,6 +226,7 @@ fn test_weights_version_key() { netuid0, weights_keys.clone(), weight_values.clone(), + salt.clone(), key0 )); assert_ok!(commit_reveal_set_weights( @@ -223,6 +234,7 @@ fn test_weights_version_key() { netuid1, weights_keys.clone(), weight_values.clone(), + salt.clone(), key1 )); @@ -232,6 +244,7 @@ fn test_weights_version_key() { netuid0, weights_keys.clone(), weight_values.clone(), + salt.clone(), key1 )); @@ -243,6 +256,7 @@ fn test_weights_version_key() { netuid1, weights_keys.clone(), weight_values.clone(), + salt.clone(), key0 ), Err(Error::::IncorrectNetworkVersionKey.into()) @@ -312,6 +326,7 @@ fn test_weights_err_weights_vec_not_equal_size() { let hotkey_account_id = U256::from(55); 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, hotkey_account_id, U256::from(66), 0); let neuron_uid: u16 = @@ -325,6 +340,7 @@ fn test_weights_err_weights_vec_not_equal_size() { 1, weights_keys.clone(), weight_values.clone(), + salt.clone(), 0, ); assert_eq!(result, Err(Error::::WeightVecNotEqualSize.into())); @@ -338,6 +354,7 @@ fn test_weights_err_has_duplicate_ids() { let hotkey_account_id = U256::from(666); 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); SubtensorModule::set_max_allowed_uids(netuid, 100); // Allow many registrations per block. @@ -374,6 +391,7 @@ fn test_weights_err_has_duplicate_ids() { netuid, weights_keys.clone(), weight_values.clone(), + salt.clone(), 0, ); assert_eq!(result, Err(Error::::DuplicateUids.into())); @@ -388,6 +406,7 @@ 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. @@ -448,13 +467,13 @@ 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, 0); + let result = commit_reveal_set_weights(U256::from(0), 1, uids, values, salt.clone(), 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), 1, uids, values, 0)); + assert_ok!(commit_reveal_set_weights(U256::from(0), 1, uids, values, salt.clone(), 0)); }); } @@ -475,6 +494,7 @@ fn test_set_weights_err_not_active() { 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 one neuron. Should have uid 0 @@ -485,7 +505,7 @@ fn test_set_weights_err_not_active() { let weights_keys: Vec = vec![0]; // Uid 0 is valid. let weight_values: Vec = vec![1]; // This hotkey is NOT registered. - let result = commit_reveal_set_weights(U256::from(1), 1, weights_keys, weight_values, 0); + let result = commit_reveal_set_weights(U256::from(1), 1, weights_keys, weight_values, salt, 0); assert_eq!(result, Err(Error::::NotRegistered.into())); }); } @@ -497,6 +517,7 @@ fn test_set_weights_err_invalid_uid() { let hotkey_account_id = U256::from(55); 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, hotkey_account_id, U256::from(66), 0); let neuron_uid: u16 = @@ -505,7 +526,7 @@ fn test_set_weights_err_invalid_uid() { SubtensorModule::set_validator_permit_for_uid(netuid, neuron_uid, true); let weight_keys: Vec = vec![9999]; // Does not exist let weight_values: Vec = vec![88]; // random value - let result = commit_reveal_set_weights(hotkey_account_id, 1, weight_keys, weight_values, 0); + let result = commit_reveal_set_weights(hotkey_account_id, 1, weight_keys, weight_values, salt, 0); assert_eq!(result, Err(Error::::InvalidUid.into())); }); } @@ -516,6 +537,7 @@ fn test_set_weight_not_enough_values() { 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]; let account_id = U256::from(1); add_network(netuid, tempo, 0); @@ -531,7 +553,7 @@ 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, 0); + let result = commit_reveal_set_weights(account_id, 1, weight_keys, weight_values, salt.clone(), 0); assert_eq!(result, Err(Error::::NotSettingEnoughWeights.into())); // Shouldnt fail because we setting a single value but it is the self weight. @@ -542,6 +564,7 @@ fn test_set_weight_not_enough_values() { 1, weight_keys, weight_values, + salt.clone(), 0 )); @@ -554,6 +577,7 @@ fn test_set_weight_not_enough_values() { 1, weight_keys, weight_values, + salt, 0 )); }); @@ -565,6 +589,7 @@ 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); @@ -579,7 +604,7 @@ 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), 1, weight_keys, weight_values, 0); + let result = commit_reveal_set_weights(U256::from(1), 1, weight_keys, weight_values, salt.clone(), 0); assert_eq!(result, Err(Error::::TooManyUids.into())); // Shouldnt fail because we are setting less weights than there are neurons. @@ -590,6 +615,7 @@ fn test_set_weight_too_many_uids() { 1, weight_keys, weight_values, + salt, 0 )); }); @@ -601,6 +627,7 @@ fn test_set_weights_sum_larger_than_u16_max() { 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); @@ -618,7 +645,7 @@ fn test_set_weights_sum_larger_than_u16_max() { // sum of weights is larger than u16 max. assert!(weight_values.iter().map(|x| *x as u64).sum::() > (u16::MAX as u64)); - let result = commit_reveal_set_weights(U256::from(1), 1, weight_keys, weight_values, 0); + let result = commit_reveal_set_weights(U256::from(1), 1, weight_keys, weight_values, salt, 0); assert_ok!(result); // Get max-upscaled unnormalized weights. @@ -998,6 +1025,7 @@ fn test_commit_reveal_weights_ok() { 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); @@ -1006,6 +1034,7 @@ fn test_commit_reveal_weights_ok() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, )); @@ -1032,6 +1061,7 @@ fn test_commit_reveal_weights_ok() { netuid, uids, weight_values, + salt, version_key, )); }); @@ -1043,6 +1073,7 @@ fn test_commit_reveal_interval() { 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); @@ -1051,6 +1082,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, )); @@ -1080,6 +1112,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, ), Error::::InvalidRevealTempo @@ -1095,6 +1128,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, ), Error::::InvalidRevealTempo @@ -1105,6 +1139,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, )); assert_ok!(SubtensorModule::commit_weights( @@ -1118,6 +1153,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, ), Error::::InvalidRevealTempo @@ -1128,6 +1164,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, )); @@ -1144,6 +1181,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, ), Error::::InvalidRevealTempo @@ -1161,6 +1199,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key + 1, )); assert_ok!(SubtensorModule::commit_weights( @@ -1175,6 +1214,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, ), Error::::InvalidReveal @@ -1184,6 +1224,7 @@ fn test_commit_reveal_interval() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key + 1, )); }); @@ -1195,6 +1236,7 @@ fn test_commit_reveal_hash() { 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); @@ -1213,6 +1255,7 @@ fn test_commit_reveal_hash() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, )); @@ -1230,6 +1273,7 @@ fn test_commit_reveal_hash() { netuid, vec![0, 2], weight_values.clone(), + salt.clone(), version_key ), Error::::InvalidReveal @@ -1240,6 +1284,7 @@ fn test_commit_reveal_hash() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), 7, ), Error::::InvalidReveal @@ -1250,6 +1295,7 @@ fn test_commit_reveal_hash() { netuid, uids.clone(), vec![10, 9], + salt.clone(), version_key, ), Error::::InvalidReveal @@ -1260,6 +1306,7 @@ fn test_commit_reveal_hash() { netuid, vec![0, 1, 2], vec![10, 10, 33], + salt.clone(), 9, ), Error::::InvalidReveal @@ -1270,6 +1317,7 @@ fn test_commit_reveal_hash() { netuid, uids, weight_values, + salt.clone(), version_key, )); }); @@ -1281,6 +1329,7 @@ fn test_commit_reveal_disabled_or_enabled() { 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); @@ -1289,6 +1338,7 @@ fn test_commit_reveal_disabled_or_enabled() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, )); @@ -1315,6 +1365,7 @@ fn test_commit_reveal_disabled_or_enabled() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, ), Error::::CommitRevealDisabled @@ -1336,6 +1387,7 @@ fn test_commit_reveal_disabled_or_enabled() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, ), Error::::CommitRevealDisabled @@ -1357,6 +1409,7 @@ fn test_commit_reveal_disabled_or_enabled() { netuid, uids, weight_values, + salt.clone(), version_key, )); }); @@ -1368,6 +1421,7 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { 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); @@ -1376,6 +1430,7 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, )); @@ -1418,6 +1473,7 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { netuid, uids.clone(), weight_values.clone(), + salt.clone(), version_key, )); @@ -1438,11 +1494,64 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { }); } +#[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(), + 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::::InvalidReveal + ); + }); +} + fn commit_reveal_set_weights( hotkey: U256, netuid: u16, uids: Vec, weights: Vec, + salt: Vec, version_key: u64, ) -> DispatchResult { SubtensorModule::set_commit_reveal_weights_interval(netuid, 5); @@ -1450,7 +1559,7 @@ fn commit_reveal_set_weights( SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); let commit_hash: H256 = - BlakeTwo256::hash_of(&(hotkey, netuid, uids.clone(), weights.clone(), version_key)); + BlakeTwo256::hash_of(&(hotkey, netuid, uids.clone(), weights.clone(), salt.clone(), version_key)); SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash)?; @@ -1461,6 +1570,7 @@ fn commit_reveal_set_weights( netuid, uids, weights, + salt, version_key, )?; From b89bbdfc3c6c4efe701e64cbc53efabe43da3540 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Tue, 28 May 2024 09:28:27 -0500 Subject: [PATCH 2/3] cargo fmt & clippy --- pallets/subtensor/src/lib.rs | 5 +- pallets/subtensor/src/weights.rs | 2 +- pallets/subtensor/tests/weights.rs | 75 +++++++++++++++++++++++------- 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index b1122b965..bb49c49fe 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -1399,7 +1399,7 @@ pub mod pallet { Self::do_commit_weights(origin, netuid, commit_hash) } - /// ---- Used to reveal the weights for a previously committed hash. + /// ---- Used to reveal the weights for a previously committed hash. /// /// # Args: /// * `origin`: (`::RuntimeOrigin`): @@ -1413,7 +1413,7 @@ pub mod pallet { /// /// * `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. /// @@ -1445,7 +1445,6 @@ pub mod pallet { Self::do_reveal_weights(origin, netuid, uids, values, salt, version_key) } - /// # Args: /// * `origin`: (Origin): /// - The caller, a hotkey who wishes to set their weights. diff --git a/pallets/subtensor/src/weights.rs b/pallets/subtensor/src/weights.rs index d99cb8542..75a75ed12 100644 --- a/pallets/subtensor/src/weights.rs +++ b/pallets/subtensor/src/weights.rs @@ -62,7 +62,7 @@ impl Pallet { /// /// * `salt` (`Vec`): /// - The values of the weights being revealed. - /// + /// /// * `version_key` (`u64`): /// - The network version key. /// diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index f227c1838..508b61f8d 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -75,7 +75,7 @@ fn test_reveal_weights_dispatch_info_ok() { netuid, uids: dests, values: weights, - salt: salt, + salt, version_key, }); let dispatch_info = call.get_dispatch_info(); @@ -122,8 +122,14 @@ fn test_weights_err_no_validator_permit() { 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, netuid, weights_keys, weight_values, salt.clone(), 0); + let result = commit_reveal_set_weights( + hotkey_account_id, + netuid, + weights_keys, + weight_values, + salt.clone(), + 0, + ); assert_eq!(result, Err(Error::::NoValidatorPermit.into())); let weights_keys: Vec = vec![1, 2]; @@ -132,8 +138,14 @@ 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, netuid, weights_keys, weight_values, salt, 0); + let result = commit_reveal_set_weights( + hotkey_account_id, + netuid, + weights_keys, + weight_values, + salt, + 0, + ); assert_ok!(result); }); } @@ -165,7 +177,14 @@ 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, netuid, dests.clone(), weights.clone(), salt.clone(), version_key), + commit_reveal_set_weights( + hotkey, + netuid, + dests.clone(), + weights.clone(), + salt.clone(), + version_key + ), Err(Error::::NotEnoughStakeToSetWeights.into()) ); // Now passes @@ -473,7 +492,14 @@ fn test_weights_err_max_weight_limit() { // 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), 1, uids, values, salt.clone(), 0)); + assert_ok!(commit_reveal_set_weights( + U256::from(0), + 1, + uids, + values, + salt.clone(), + 0 + )); }); } @@ -505,7 +531,8 @@ fn test_set_weights_err_not_active() { let weights_keys: Vec = vec![0]; // Uid 0 is valid. let weight_values: Vec = vec![1]; // This hotkey is NOT registered. - let result = commit_reveal_set_weights(U256::from(1), 1, weights_keys, weight_values, salt, 0); + let result = + commit_reveal_set_weights(U256::from(1), 1, weights_keys, weight_values, salt, 0); assert_eq!(result, Err(Error::::NotRegistered.into())); }); } @@ -526,7 +553,8 @@ fn test_set_weights_err_invalid_uid() { SubtensorModule::set_validator_permit_for_uid(netuid, neuron_uid, true); let weight_keys: Vec = vec![9999]; // Does not exist let weight_values: Vec = vec![88]; // random value - let result = commit_reveal_set_weights(hotkey_account_id, 1, weight_keys, weight_values, salt, 0); + let result = + commit_reveal_set_weights(hotkey_account_id, 1, weight_keys, weight_values, salt, 0); assert_eq!(result, Err(Error::::InvalidUid.into())); }); } @@ -553,7 +581,8 @@ 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 = + commit_reveal_set_weights(account_id, 1, weight_keys, weight_values, salt.clone(), 0); assert_eq!(result, Err(Error::::NotSettingEnoughWeights.into())); // Shouldnt fail because we setting a single value but it is the self weight. @@ -604,7 +633,14 @@ 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), 1, weight_keys, weight_values, salt.clone(), 0); + let result = commit_reveal_set_weights( + U256::from(1), + 1, + weight_keys, + weight_values, + salt.clone(), + 0, + ); assert_eq!(result, Err(Error::::TooManyUids.into())); // Shouldnt fail because we are setting less weights than there are neurons. @@ -645,7 +681,8 @@ fn test_set_weights_sum_larger_than_u16_max() { // sum of weights is larger than u16 max. assert!(weight_values.iter().map(|x| *x as u64).sum::() > (u16::MAX as u64)); - let result = commit_reveal_set_weights(U256::from(1), 1, weight_keys, weight_values, salt, 0); + let result = + commit_reveal_set_weights(U256::from(1), 1, weight_keys, weight_values, salt, 0); assert_ok!(result); // Get max-upscaled unnormalized weights. @@ -1525,8 +1562,8 @@ fn test_commit_reveal_bad_salt_fail() { SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); assert_ok!(SubtensorModule::commit_weights( - RuntimeOrigin::signed(hotkey), - netuid, + RuntimeOrigin::signed(hotkey), + netuid, commit_hash )); @@ -1558,8 +1595,14 @@ fn commit_reveal_set_weights( SubtensorModule::set_weights_set_rate_limit(netuid, 5); SubtensorModule::set_commit_reveal_weights_enabled(netuid, true); - let commit_hash: H256 = - BlakeTwo256::hash_of(&(hotkey, netuid, uids.clone(), weights.clone(), salt.clone(), version_key)); + let commit_hash: H256 = BlakeTwo256::hash_of(&( + hotkey, + netuid, + uids.clone(), + weights.clone(), + salt.clone(), + version_key, + )); SubtensorModule::commit_weights(RuntimeOrigin::signed(hotkey), netuid, commit_hash)?; From 06295e41d5b094347a35fa028ed0ebd66041f723 Mon Sep 17 00:00:00 2001 From: John Reed <87283488+JohnReedV@users.noreply.github.com> Date: Tue, 28 May 2024 15:03:47 -0500 Subject: [PATCH 3/3] salt u8 -> u16 --- pallets/subtensor/src/benchmarks.rs | 2 +- pallets/subtensor/src/lib.rs | 2 +- pallets/subtensor/src/weights.rs | 2 +- pallets/subtensor/tests/weights.rs | 44 ++++++++++++++--------------- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/pallets/subtensor/src/benchmarks.rs b/pallets/subtensor/src/benchmarks.rs index e2d2a29b6..a7dd29fbb 100644 --- a/pallets/subtensor/src/benchmarks.rs +++ b/pallets/subtensor/src/benchmarks.rs @@ -388,7 +388,7 @@ reveal_weights { let version_key: u64 = 0; let uids: Vec = vec![0]; let weight_values: Vec = vec![10]; - let salt: Vec = vec![8]; + let salt: Vec = vec![8]; let hotkey: T::AccountId = account("hot", 0, 1); let coldkey: T::AccountId = account("cold", 1, 2); diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index bb49c49fe..c8af548af 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -1439,7 +1439,7 @@ pub mod pallet { netuid: u16, uids: Vec, values: Vec, - salt: Vec, + salt: Vec, version_key: u64, ) -> DispatchResult { Self::do_reveal_weights(origin, netuid, uids, values, salt, version_key) diff --git a/pallets/subtensor/src/weights.rs b/pallets/subtensor/src/weights.rs index 75a75ed12..c21fe6bbd 100644 --- a/pallets/subtensor/src/weights.rs +++ b/pallets/subtensor/src/weights.rs @@ -81,7 +81,7 @@ impl Pallet { netuid: u16, uids: Vec, values: Vec, - salt: Vec, + salt: Vec, version_key: u64, ) -> DispatchResult { let who = ensure_signed(origin.clone())?; diff --git a/pallets/subtensor/tests/weights.rs b/pallets/subtensor/tests/weights.rs index 508b61f8d..c4085c5d7 100644 --- a/pallets/subtensor/tests/weights.rs +++ b/pallets/subtensor/tests/weights.rs @@ -44,7 +44,7 @@ fn test_commit_weights_dispatch_info_ok() { let dests = vec![1, 1]; let weights = vec![1, 1]; let netuid: u16 = 1; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let hotkey: U256 = U256::from(1); @@ -68,7 +68,7 @@ fn test_reveal_weights_dispatch_info_ok() { let dests = vec![1, 1]; let weights = vec![1, 1]; let netuid: u16 = 1; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let call = RuntimeCall::SubtensorModule(SubtensorCall::reveal_weights { @@ -92,7 +92,7 @@ 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 salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let hotkey = U256::from(1); @@ -120,7 +120,7 @@ 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 salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let result = commit_reveal_set_weights( hotkey_account_id, @@ -161,7 +161,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]; + 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); @@ -208,7 +208,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]; + 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); @@ -345,7 +345,7 @@ fn test_weights_err_weights_vec_not_equal_size() { let hotkey_account_id = U256::from(55); let netuid: u16 = 1; let tempo: u16 = 13; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; add_network(netuid, tempo, 0); register_ok_neuron(1, hotkey_account_id, U256::from(66), 0); let neuron_uid: u16 = @@ -373,7 +373,7 @@ fn test_weights_err_has_duplicate_ids() { let hotkey_account_id = U256::from(666); let netuid: u16 = 1; let tempo: u16 = 13; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; add_network(netuid, tempo, 0); SubtensorModule::set_max_allowed_uids(netuid, 100); // Allow many registrations per block. @@ -425,7 +425,7 @@ 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]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; add_network(netuid, tempo, 0); // Set params. @@ -520,7 +520,7 @@ fn test_set_weights_err_not_active() { 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]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; add_network(netuid, tempo, 0); // Register one neuron. Should have uid 0 @@ -544,7 +544,7 @@ fn test_set_weights_err_invalid_uid() { let hotkey_account_id = U256::from(55); let netuid: u16 = 1; let tempo: u16 = 13; - let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; add_network(netuid, tempo, 0); register_ok_neuron(1, hotkey_account_id, U256::from(66), 0); let neuron_uid: u16 = @@ -565,7 +565,7 @@ fn test_set_weight_not_enough_values() { 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]; + let salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let account_id = U256::from(1); add_network(netuid, tempo, 0); @@ -618,7 +618,7 @@ 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]; + 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); @@ -663,7 +663,7 @@ fn test_set_weights_sum_larger_than_u16_max() { 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]; + 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); @@ -1062,7 +1062,7 @@ fn test_commit_reveal_weights_ok() { 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 salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let hotkey: U256 = U256::from(1); @@ -1110,7 +1110,7 @@ fn test_commit_reveal_interval() { 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 salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let hotkey: U256 = U256::from(1); @@ -1273,7 +1273,7 @@ fn test_commit_reveal_hash() { 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 salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let hotkey: U256 = U256::from(1); @@ -1366,7 +1366,7 @@ fn test_commit_reveal_disabled_or_enabled() { 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 salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let hotkey: U256 = U256::from(1); @@ -1458,7 +1458,7 @@ fn test_toggle_commit_reveal_weights_and_set_weights() { 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 salt: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8]; let version_key: u64 = 0; let hotkey: U256 = U256::from(1); @@ -1537,8 +1537,8 @@ fn test_commit_reveal_bad_salt_fail() { 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 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); @@ -1588,7 +1588,7 @@ fn commit_reveal_set_weights( netuid: u16, uids: Vec, weights: Vec, - salt: Vec, + salt: Vec, version_key: u64, ) -> DispatchResult { SubtensorModule::set_commit_reveal_weights_interval(netuid, 5);