From b8364014f36b6a72357881cacb0f3462a3fde5a0 Mon Sep 17 00:00:00 2001 From: Samuel Dare Date: Sun, 23 Jun 2024 04:53:02 +0400 Subject: [PATCH] lints , use sat maths for 256 miner alpha test --- pallets/admin-utils/src/lib.rs | 14 ++- pallets/admin-utils/tests/mock.rs | 7 +- pallets/admin-utils/tests/tests.rs | 106 ++++++++++++++------ pallets/subtensor/src/epoch.rs | 9 +- pallets/subtensor/src/lib.rs | 5 +- pallets/subtensor/src/utils.rs | 2 +- pallets/subtensor/tests/epoch.rs | 153 ++++++++++++++++++----------- runtime/src/lib.rs | 7 +- 8 files changed, 209 insertions(+), 94 deletions(-) diff --git a/pallets/admin-utils/src/lib.rs b/pallets/admin-utils/src/lib.rs index f85f4be98..9a8744dc6 100644 --- a/pallets/admin-utils/src/lib.rs +++ b/pallets/admin-utils/src/lib.rs @@ -1026,7 +1026,12 @@ pub mod pallet { /// Sets values for liquid alpha #[pallet::call_index(51)] #[pallet::weight((0, DispatchClass::Operational, Pays::No))] - pub fn sudo_set_alpha_values(origin: OriginFor, netuid: u16, alpha_low: u16, alpha_high: u16) -> DispatchResult { + pub fn sudo_set_alpha_values( + origin: OriginFor, + netuid: u16, + alpha_low: u16, + alpha_high: u16, + ) -> DispatchResult { T::Subtensor::ensure_subnet_owner_or_root(origin.clone(), netuid)?; T::Subtensor::do_set_alpha_values(origin, netuid, alpha_low, alpha_high) } @@ -1126,5 +1131,10 @@ pub trait SubtensorInterface { fn set_commit_reveal_weights_interval(netuid: u16, interval: u64); fn set_commit_reveal_weights_enabled(netuid: u16, enabled: bool); fn set_liquid_alpha_enabled(netuid: u16, enabled: bool); - fn do_set_alpha_values(origin: RuntimeOrigin, netuid: u16, alpha_low: u16, alpha_high: u16) -> Result<(), DispatchError>; + fn do_set_alpha_values( + origin: RuntimeOrigin, + netuid: u16, + alpha_low: u16, + alpha_high: u16, + ) -> Result<(), DispatchError>; } diff --git a/pallets/admin-utils/tests/mock.rs b/pallets/admin-utils/tests/mock.rs index 79fbd5b56..3dc08170b 100644 --- a/pallets/admin-utils/tests/mock.rs +++ b/pallets/admin-utils/tests/mock.rs @@ -471,7 +471,12 @@ impl pallet_admin_utils::SubtensorInterface f fn set_liquid_alpha_enabled(netuid: u16, enabled: bool) { SubtensorModule::set_liquid_alpha_enabled(netuid, enabled); } - fn do_set_alpha_values(origin: RuntimeOrigin, netuid: u16, alpha_low: u16, alpha_high: u16) -> Result<(), DispatchError> { + fn do_set_alpha_values( + origin: RuntimeOrigin, + netuid: u16, + alpha_low: u16, + alpha_high: u16, + ) -> Result<(), DispatchError> { SubtensorModule::do_set_alpha_values(origin, netuid, alpha_low, alpha_high) } } diff --git a/pallets/admin-utils/tests/tests.rs b/pallets/admin-utils/tests/tests.rs index 047314af2..9df59978f 100644 --- a/pallets/admin-utils/tests/tests.rs +++ b/pallets/admin-utils/tests/tests.rs @@ -1,5 +1,8 @@ -use frame_support::{assert_ok, assert_err, dispatch::{DispatchClass, GetDispatchInfo, Pays}}; use frame_support::sp_runtime::DispatchError; +use frame_support::{ + assert_err, assert_ok, + dispatch::{DispatchClass, GetDispatchInfo, Pays}, +}; use frame_system::Config; use pallet_admin_utils::Error; use pallet_subtensor::Error as SubtensorError; @@ -1225,24 +1228,14 @@ fn test_sudo_get_set_alpha() { let hotkey: U256 = U256::from(1); let coldkey: U256 = U256::from(1 + 456); - let signer = <::RuntimeOrigin>::signed(coldkey.clone()); + let signer = <::RuntimeOrigin>::signed(coldkey); // Enable Liquid Alpha and setup SubtensorModule::set_liquid_alpha_enabled(netuid, true); migration::migrate_create_root_network::(); - SubtensorModule::add_balance_to_coldkey_account( - &coldkey, - 1_000_000_000_000_000, - ); - assert_ok!(SubtensorModule::root_register( - signer.clone(), - hotkey, - )); - assert_ok!(SubtensorModule::add_stake( - signer.clone(), - hotkey, - 1000 - )); + SubtensorModule::add_balance_to_coldkey_account(&coldkey, 1_000_000_000_000_000); + assert_ok!(SubtensorModule::root_register(signer.clone(), hotkey,)); + assert_ok!(SubtensorModule::add_stake(signer.clone(), hotkey, 1000)); // Should fail as signer does not own the subnet assert_err!( @@ -1250,14 +1243,22 @@ fn test_sudo_get_set_alpha() { DispatchError::BadOrigin ); - assert_ok!(SubtensorModule::register_network( - signer.clone() - )); + assert_ok!(SubtensorModule::register_network(signer.clone())); - assert_ok!(AdminUtils::sudo_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); - let (grabbed_alpha_low, grabbed_alpha_high): (u16, u16) = SubtensorModule::get_alpha_values(netuid); + assert_ok!(AdminUtils::sudo_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); + let (grabbed_alpha_low, grabbed_alpha_high): (u16, u16) = + SubtensorModule::get_alpha_values(netuid); - log::info!("alpha_low: {:?} alpha_high: {:?}", grabbed_alpha_low, grabbed_alpha_high); + log::info!( + "alpha_low: {:?} alpha_high: {:?}", + grabbed_alpha_low, + grabbed_alpha_high + ); assert_eq!(grabbed_alpha_low, alpha_low); assert_eq!(grabbed_alpha_high, alpha_high); @@ -1275,8 +1276,18 @@ fn test_sudo_get_set_alpha() { let tolerance: f32 = 1e-6; // 0.000001 // Check if the values are equal to the sixth decimal - assert!((alpha_low_32.to_num::() - alpha_low_decimal).abs() < tolerance, "alpha_low mismatch: {} != {}", alpha_low_32.to_num::(), alpha_low_decimal); - assert!((alpha_high_32.to_num::() - alpha_high_decimal).abs() < tolerance, "alpha_high mismatch: {} != {}", alpha_high_32.to_num::(), alpha_high_decimal); + assert!( + (alpha_low_32.to_num::() - alpha_low_decimal).abs() < tolerance, + "alpha_low mismatch: {} != {}", + alpha_low_32.to_num::(), + alpha_low_decimal + ); + assert!( + (alpha_high_32.to_num::() - alpha_high_decimal).abs() < tolerance, + "alpha_high mismatch: {} != {}", + alpha_high_32.to_num::(), + alpha_high_decimal + ); // 1. Liquid alpha disabled SubtensorModule::set_liquid_alpha_enabled(netuid, false); @@ -1286,32 +1297,67 @@ fn test_sudo_get_set_alpha() { ); // Correct scenario after error SubtensorModule::set_liquid_alpha_enabled(netuid, true); // Re-enable for further tests - assert_ok!(AdminUtils::sudo_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); + assert_ok!(AdminUtils::sudo_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); // 2. Alpha high too low let alpha_high_too_low = (u16::MAX as u32 * 4 / 5) as u16 - 1; // One less than the minimum acceptable value assert_err!( - AdminUtils::sudo_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high_too_low), + AdminUtils::sudo_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high_too_low + ), SubtensorError::::AlphaHighTooLow ); // Correct scenario after error - assert_ok!(AdminUtils::sudo_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); + assert_ok!(AdminUtils::sudo_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); // 3. Alpha low too low or too high let alpha_low_too_low = 0_u16; assert_err!( - AdminUtils::sudo_set_alpha_values(signer.clone(), netuid, alpha_low_too_low, alpha_high), + AdminUtils::sudo_set_alpha_values( + signer.clone(), + netuid, + alpha_low_too_low, + alpha_high + ), SubtensorError::::AlphaLowOutOfRange ); // Correct scenario after error - assert_ok!(AdminUtils::sudo_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); + assert_ok!(AdminUtils::sudo_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); let alpha_low_too_high = (u16::MAX as u32 * 4 / 5) as u16 + 1; // One more than the maximum acceptable value assert_err!( - AdminUtils::sudo_set_alpha_values(signer.clone(), netuid, alpha_low_too_high, alpha_high), + AdminUtils::sudo_set_alpha_values( + signer.clone(), + netuid, + alpha_low_too_high, + alpha_high + ), SubtensorError::::AlphaLowOutOfRange ); // Correct scenario after error - assert_ok!(AdminUtils::sudo_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); + assert_ok!(AdminUtils::sudo_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); }); } diff --git a/pallets/subtensor/src/epoch.rs b/pallets/subtensor/src/epoch.rs index 1bb637caf..9881b809e 100644 --- a/pallets/subtensor/src/epoch.rs +++ b/pallets/subtensor/src/epoch.rs @@ -1182,13 +1182,18 @@ impl Pallet { } } - pub fn do_set_alpha_values(origin: T::RuntimeOrigin, netuid: u16, alpha_low: u16, alpha_high: u16) -> Result<(), DispatchError> { + pub fn do_set_alpha_values( + origin: T::RuntimeOrigin, + netuid: u16, + alpha_low: u16, + alpha_high: u16, + ) -> Result<(), DispatchError> { // --- 1. Ensure the function caller is a signed user. ensure_signed(origin.clone())?; // --- 2. Ensure the function caller is the subnet owner or root. Self::ensure_subnet_owner_or_root(origin, netuid)?; - + // --- 3. Ensure liquid alpha is enabled ensure!( Self::get_liquid_alpha_enabled(netuid), diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 4eb8ef474..76c553ab7 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -868,7 +868,7 @@ pub mod pallet { #[pallet::type_value] pub fn DefaultAlphaValues() -> (u16, u16) { (45875, 58982) // (alpha_low: 0.7, alpha_high: 0.9) - } + } #[pallet::storage] // ITEM( weights_min_stake ) pub type WeightsMinStake = StorageValue<_, u64, ValueQuery, DefaultWeightsMinStake>; @@ -942,7 +942,8 @@ pub mod pallet { // MAP ( netuid ) --> (alpha_low, alpha_high) #[pallet::storage] - pub type AlphaValues = StorageMap<_, Identity, u16, (u16, u16), ValueQuery, DefaultAlphaValues>; + pub type AlphaValues = + StorageMap<_, Identity, u16, (u16, u16), ValueQuery, DefaultAlphaValues>; #[pallet::storage] // --- MAP (netuid, who) --> (hash, weight) | Returns the hash and weight committed by an account for a given netuid. pub type WeightCommits = StorageDoubleMap< diff --git a/pallets/subtensor/src/utils.rs b/pallets/subtensor/src/utils.rs index e97d29534..b5bf59311 100644 --- a/pallets/subtensor/src/utils.rs +++ b/pallets/subtensor/src/utils.rs @@ -672,7 +672,7 @@ impl Pallet { let converted_low = I32F32::from_num(alpha_low) / I32F32::from_num(u16::MAX); let converted_high = I32F32::from_num(alpha_high) / I32F32::from_num(u16::MAX); - return (converted_low, converted_high); + (converted_low, converted_high) } pub fn set_liquid_alpha_enabled(netuid: u16, enabled: bool) { diff --git a/pallets/subtensor/tests/epoch.rs b/pallets/subtensor/tests/epoch.rs index 801a7f74f..b4170b0ac 100644 --- a/pallets/subtensor/tests/epoch.rs +++ b/pallets/subtensor/tests/epoch.rs @@ -1,13 +1,13 @@ use crate::mock::*; use frame_support::{assert_err, assert_ok}; use frame_system::Config; +use pallet_subtensor::math::safe_exp; use pallet_subtensor::*; use rand::{distributions::Uniform, rngs::StdRng, seq::SliceRandom, thread_rng, Rng, SeedableRng}; use sp_core::U256; use sp_runtime::DispatchError; use std::time::Instant; use substrate_fixed::types::I32F32; -use substrate_fixed::transcendental::exp; mod mock; @@ -1486,28 +1486,16 @@ fn test_set_alpha_disabled() { let netuid: u16 = 1; let hotkey: U256 = U256::from(1); let coldkey: U256 = U256::from(1 + 456); - let signer = <::RuntimeOrigin>::signed(coldkey.clone()); + let signer = <::RuntimeOrigin>::signed(coldkey); // Enable Liquid Alpha and setup SubtensorModule::set_liquid_alpha_enabled(netuid, true); migration::migrate_create_root_network::(); - SubtensorModule::add_balance_to_coldkey_account( - &coldkey, - 1_000_000_000_000_000, - ); - assert_ok!(SubtensorModule::root_register( - signer.clone(), - hotkey, - )); - assert_ok!(SubtensorModule::add_stake( - signer.clone(), - hotkey, - 1000 - )); + SubtensorModule::add_balance_to_coldkey_account(&coldkey, 1_000_000_000_000_000); + assert_ok!(SubtensorModule::root_register(signer.clone(), hotkey,)); + assert_ok!(SubtensorModule::add_stake(signer.clone(), hotkey, 1000)); // Only owner can set alpha values - assert_ok!(SubtensorModule::register_network( - signer.clone() - )); + assert_ok!(SubtensorModule::register_network(signer.clone())); // Explicitly set to false SubtensorModule::set_liquid_alpha_enabled(netuid, false); @@ -1517,7 +1505,12 @@ fn test_set_alpha_disabled() { ); SubtensorModule::set_liquid_alpha_enabled(netuid, true); - assert_ok!(SubtensorModule::do_set_alpha_values(signer.clone(), netuid, 12_u16, u16::MAX)); + assert_ok!(SubtensorModule::do_set_alpha_values( + signer.clone(), + netuid, + 12_u16, + u16::MAX + )); }); } @@ -2286,7 +2279,9 @@ fn test_compute_alpha_values() { #[test] fn test_compute_alpha_values_256_miners() { // Define the consensus values for 256 miners. - let consensus: Vec = (0..256).map(|i| I32F32::from_num(i as f32 / 255.0)).collect(); + let consensus: Vec = (0..256) + .map(|i| I32F32::from_num(i as f32 / 255.0)) + .collect(); // Define the logistic function parameters 'a' and 'b'. let a = I32F32::from_num(1.0); let b = I32F32::from_num(0.0); @@ -2297,20 +2292,25 @@ fn test_compute_alpha_values_256_miners() { // Ensure the length of the alpha vector matches the consensus vector. assert_eq!(alpha.len(), consensus.len()); - // Manually compute the expected alpha values for each consensus value. - // The logistic function is: 1 / (1 + exp(b - a * c)) - // where c is the consensus value. - // Define an epsilon for approximate equality checks. let epsilon = I32F32::from_num(1e-6); for (i, &c) in consensus.iter().enumerate() { - let exp_val = exp(b - a * c).unwrap_or(I32F32::from_num(0.0)); - let expected_alpha = I32F32::from_num(1.0).saturating_div(I32F32::from_num(1.0).saturating_add(exp_val)); + // Use saturating subtraction and multiplication + let exponent = b.saturating_sub(a.saturating_mul(c)); + + // Use safe_exp instead of exp + let exp_val = safe_exp(exponent); + + // Use saturating addition and division + let expected_alpha = + I32F32::from_num(1.0).saturating_div(I32F32::from_num(1.0).saturating_add(exp_val)); + // Assert that the computed alpha values match the expected values within the epsilon. assert_approx_eq(alpha[i], expected_alpha, epsilon); } } + #[test] fn test_clamp_alpha_values() { // Define the alpha values. @@ -2564,24 +2564,14 @@ fn test_get_set_alpha() { let hotkey: U256 = U256::from(1); let coldkey: U256 = U256::from(1 + 456); - let signer = <::RuntimeOrigin>::signed(coldkey.clone()); + let signer = <::RuntimeOrigin>::signed(coldkey); // Enable Liquid Alpha and setup SubtensorModule::set_liquid_alpha_enabled(netuid, true); migration::migrate_create_root_network::(); - SubtensorModule::add_balance_to_coldkey_account( - &coldkey, - 1_000_000_000_000_000, - ); - assert_ok!(SubtensorModule::root_register( - signer.clone(), - hotkey, - )); - assert_ok!(SubtensorModule::add_stake( - signer.clone(), - hotkey, - 1000 - )); + SubtensorModule::add_balance_to_coldkey_account(&coldkey, 1_000_000_000_000_000); + assert_ok!(SubtensorModule::root_register(signer.clone(), hotkey,)); + assert_ok!(SubtensorModule::add_stake(signer.clone(), hotkey, 1000)); // Should fail as signer does not own the subnet assert_err!( @@ -2589,14 +2579,22 @@ fn test_get_set_alpha() { DispatchError::BadOrigin ); - assert_ok!(SubtensorModule::register_network( - signer.clone() - )); + assert_ok!(SubtensorModule::register_network(signer.clone())); - assert_ok!(SubtensorModule::do_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); - let (grabbed_alpha_low, grabbed_alpha_high): (u16, u16) = SubtensorModule::get_alpha_values(netuid); + assert_ok!(SubtensorModule::do_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); + let (grabbed_alpha_low, grabbed_alpha_high): (u16, u16) = + SubtensorModule::get_alpha_values(netuid); - log::info!("alpha_low: {:?} alpha_high: {:?}", grabbed_alpha_low, grabbed_alpha_high); + log::info!( + "alpha_low: {:?} alpha_high: {:?}", + grabbed_alpha_low, + grabbed_alpha_high + ); assert_eq!(grabbed_alpha_low, alpha_low); assert_eq!(grabbed_alpha_high, alpha_high); @@ -2614,8 +2612,18 @@ fn test_get_set_alpha() { let tolerance: f32 = 1e-6; // 0.000001 // Check if the values are equal to the sixth decimal - assert!((alpha_low_32.to_num::() - alpha_low_decimal).abs() < tolerance, "alpha_low mismatch: {} != {}", alpha_low_32.to_num::(), alpha_low_decimal); - assert!((alpha_high_32.to_num::() - alpha_high_decimal).abs() < tolerance, "alpha_high mismatch: {} != {}", alpha_high_32.to_num::(), alpha_high_decimal); + assert!( + (alpha_low_32.to_num::() - alpha_low_decimal).abs() < tolerance, + "alpha_low mismatch: {} != {}", + alpha_low_32.to_num::(), + alpha_low_decimal + ); + assert!( + (alpha_high_32.to_num::() - alpha_high_decimal).abs() < tolerance, + "alpha_high mismatch: {} != {}", + alpha_high_32.to_num::(), + alpha_high_decimal + ); // 1. Liquid alpha disabled SubtensorModule::set_liquid_alpha_enabled(netuid, false); @@ -2625,33 +2633,68 @@ fn test_get_set_alpha() { ); // Correct scenario after error SubtensorModule::set_liquid_alpha_enabled(netuid, true); // Re-enable for further tests - assert_ok!(SubtensorModule::do_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); + assert_ok!(SubtensorModule::do_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); // 2. Alpha high too low let alpha_high_too_low = (u16::MAX as u32 * 4 / 5) as u16 - 1; // One less than the minimum acceptable value assert_err!( - SubtensorModule::do_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high_too_low), + SubtensorModule::do_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high_too_low + ), Error::::AlphaHighTooLow ); // Correct scenario after error - assert_ok!(SubtensorModule::do_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); + assert_ok!(SubtensorModule::do_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); // 3. Alpha low too low or too high let alpha_low_too_low = 0_u16; assert_err!( - SubtensorModule::do_set_alpha_values(signer.clone(), netuid, alpha_low_too_low, alpha_high), + SubtensorModule::do_set_alpha_values( + signer.clone(), + netuid, + alpha_low_too_low, + alpha_high + ), Error::::AlphaLowOutOfRange ); // Correct scenario after error - assert_ok!(SubtensorModule::do_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); + assert_ok!(SubtensorModule::do_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); let alpha_low_too_high = (u16::MAX as u32 * 4 / 5) as u16 + 1; // One more than the maximum acceptable value assert_err!( - SubtensorModule::do_set_alpha_values(signer.clone(), netuid, alpha_low_too_high, alpha_high), + SubtensorModule::do_set_alpha_values( + signer.clone(), + netuid, + alpha_low_too_high, + alpha_high + ), Error::::AlphaLowOutOfRange ); // Correct scenario after error - assert_ok!(SubtensorModule::do_set_alpha_values(signer.clone(), netuid, alpha_low, alpha_high)); + assert_ok!(SubtensorModule::do_set_alpha_values( + signer.clone(), + netuid, + alpha_low, + alpha_high + )); }); } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 96a1bf033..3e66513c9 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1153,7 +1153,12 @@ impl SubtensorModule::set_liquid_alpha_enabled(netuid, enabled); } - fn do_set_alpha_values(origin: RuntimeOrigin, netuid: u16, alpha_low: u16, alpha_high: u16) -> Result<(), DispatchError> { + fn do_set_alpha_values( + origin: RuntimeOrigin, + netuid: u16, + alpha_low: u16, + alpha_high: u16, + ) -> Result<(), DispatchError> { SubtensorModule::do_set_alpha_values(origin, netuid, alpha_low, alpha_high) } }