diff --git a/pallets/subtensor/src/benchmarks/benchmarks.rs b/pallets/subtensor/src/benchmarks/benchmarks.rs index 5a854695f8..3d514902ec 100644 --- a/pallets/subtensor/src/benchmarks/benchmarks.rs +++ b/pallets/subtensor/src/benchmarks/benchmarks.rs @@ -1962,102 +1962,67 @@ mod pallet_benchmarks { } #[benchmark] - fn claim_root() { + fn claim_root(h: Linear<1, { crate::MAX_ROOT_CLAIM_WORK }>) { let coldkey: T::AccountId = whitelisted_caller(); - let hotkey: T::AccountId = account("A", 0, 1); - + let owner_coldkey: T::AccountId = account("claim_owner_cold", 0, 0); + let owner_hotkey: T::AccountId = account("claim_owner_hot", 0, 1); let netuid = Subtensor::::get_next_netuid(); let lock_cost = Subtensor::::get_network_lock_cost(); - add_balance_to_coldkey_account::(&coldkey, lock_cost.into()); + add_balance_to_coldkey_account::(&owner_coldkey, lock_cost.into()); assert_ok!(Subtensor::::register_network( - RawOrigin::Signed(coldkey.clone()).into(), - hotkey.clone() + RawOrigin::Signed(owner_coldkey).into(), + owner_hotkey )); - SubtokenEnabled::::insert(netuid, true); - Subtensor::::set_network_registration_allowed(netuid, true); - NetworkRegistrationAllowed::::insert(netuid, true); FirstEmissionBlockNumber::::insert(netuid, 0); - SubnetMechanism::::insert(netuid, 1); SubnetworkN::::insert(netuid, 1); Subtensor::::set_tao_weight(u64::MAX); - - let root_stake = 100_000_000u64; - Subtensor::::increase_stake_for_hotkey_and_coldkey_on_subnet( - &hotkey, - &coldkey, - NetUid::ROOT, - root_stake.into(), - ); - - let initial_total_hotkey_alpha = 100_000_000u64; - Subtensor::::increase_stake_for_hotkey_and_coldkey_on_subnet( - &hotkey, - &coldkey, + set_reserves::( netuid, - initial_total_hotkey_alpha.into(), + TaoBalance::from(100_000_000_000_000_u64), + AlphaBalance::from(100_000_000_000_000_u64), ); + RootClaimableThreshold::::insert(NetUid::ROOT, I96F32::from_num(0)); - // Point the validator's basket weight vector at the subnet so the distributed root - // dividend is deposited into its fund (instead of being recycled for lack of weights). - if let Ok(root_uid) = Uids::::try_get(NetUid::ROOT, &hotkey) { - Weights::::insert( - NetUidStorageIndex::ROOT, - root_uid, - vec![(u16::from(netuid), 1u16)], + let escrow = Subtensor::::get_beta_escrow_account_id(); + let holding_alpha = AlphaBalance::from(1_000_000_u64); + for i in 0..h { + let hotkey: T::AccountId = account("claim_hot", i, 1); + Subtensor::::increase_stake_for_hotkey_and_coldkey_on_subnet( + &hotkey, + &coldkey, + NetUid::ROOT, + AlphaBalance::from(1_u64), + ); + Subtensor::::increase_stake_for_hotkey_and_coldkey_on_subnet( + &hotkey, + &escrow, + netuid, + holding_alpha, ); + BasketShares::::insert(&hotkey, 1_u64); + BasketRate::::insert(&hotkey, I96F32::from_num(1)); } - let pending_root_alpha = 10_000_000u64; - Subtensor::::distribute_emission( - netuid, - AlphaBalance::ZERO, - pending_root_alpha.into(), - pending_root_alpha.into(), - AlphaBalance::ZERO, - ); - - let initial_stake = Subtensor::::get_stake_for_hotkey_and_coldkey_on_subnet( - &hotkey, - &coldkey, - NetUid::ROOT, - ); - #[extrinsic_call] _(RawOrigin::Signed(coldkey.clone())); - let new_stake = Subtensor::::get_stake_for_hotkey_and_coldkey_on_subnet( - &hotkey, - &coldkey, - NetUid::ROOT, - ); - - // The claim must actually pay out (strict: a no-op claim is a broken benchmark). - assert!(new_stake > initial_stake); + // Every work unit must execute the active holding-redemption path. + let first_hotkey: T::AccountId = account("claim_hot", 0, 1); + let last_hotkey: T::AccountId = account("claim_hot", h.saturating_sub(1), 1); + assert_eq!(BasketShares::::get(first_hotkey), 0); + assert_eq!(BasketShares::::get(last_hotkey), 0); } #[benchmark] fn sudo_set_root_claim_threshold() { - let coldkey: T::AccountId = whitelisted_caller(); - let hotkey: T::AccountId = account("A", 0, 1); - - let netuid = Subtensor::::get_next_netuid(); - - let lock_cost = Subtensor::::get_network_lock_cost(); - add_balance_to_coldkey_account::(&coldkey, lock_cost.into()); - - assert_ok!(Subtensor::::register_network( - RawOrigin::Signed(coldkey.clone()).into(), - hotkey.clone() - )); - #[extrinsic_call] - _(RawOrigin::Root, netuid, 100); + _(RawOrigin::Root, NetUid::ROOT, 100); } #[benchmark] diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 9354a6023b..41a732b5bd 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -65,6 +65,10 @@ pub const ALPHA_MAP_BATCH_SIZE: usize = 30; pub const MAX_ROOT_CLAIM_THRESHOLD: u64 = 10_000_000; +/// Maximum number of validator hotkeys or raw basket-position rows one `claim_root` call may +/// inspect. +pub const MAX_ROOT_CLAIM_WORK: u32 = 256; + pub struct SubtensorDustRemoval(PhantomData); impl frame_support::traits::OnUnbalanced> for SubtensorDustRemoval diff --git a/pallets/subtensor/src/macros/dispatches.rs b/pallets/subtensor/src/macros/dispatches.rs index a2cadab9c8..a362978708 100644 --- a/pallets/subtensor/src/macros/dispatches.rs +++ b/pallets/subtensor/src/macros/dispatches.rs @@ -13,6 +13,7 @@ mod dispatches { use crate::MAX_CRV3_COMMIT_SIZE_BYTES; use crate::MAX_ROOT_CLAIM_THRESHOLD; + use crate::MAX_ROOT_CLAIM_WORK; /// Dispatchable functions allow users to interact with the pallet and invoke state changes. /// These functions materialize as "extrinsics", which are often compared to transactions. /// Dispatchable functions must be annotated with a weight and must return a DispatchResult. @@ -1907,16 +1908,20 @@ mod dispatches { /// * `RootClaimed`: On the successfully claiming the root emissions for a coldkey. /// /// # Errors - /// * `InvalidSubnetNumber`: The subnet set is empty or exceeds the maximum number of claims. - /// + /// * `TooManyRootClaimHotkeys`: The coldkey exceeds the benchmarked hotkey bound. + /// * `TooManyRootClaimHoldings`: The coldkey exceeds the benchmarked basket-position bound. #[pallet::call_index(121)] - #[pallet::weight(::WeightInfo::claim_root())] + #[pallet::weight( + ::WeightInfo::claim_root(MAX_ROOT_CLAIM_WORK) + )] pub fn claim_root(origin: OriginFor) -> DispatchResultWithPostInfo { let coldkey: T::AccountId = ensure_signed(origin)?; + let (hotkeys, work) = Self::bounded_root_claim_work(&coldkey)?; + Self::do_root_claim(coldkey.clone(), hotkeys)?; Self::maybe_add_coldkey_index(&coldkey); - let weight = Self::do_root_claim(coldkey)?; + let weight = ::WeightInfo::claim_root(work); Ok((Some(weight), Pays::Yes).into()) } diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 85ac21da6b..837d623f45 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -350,5 +350,9 @@ mod errors { /// (measured from the last stake add/remove) and cannot be unstaked yet. Prevents /// epoch-boundary just-in-time dividend sniping. RootStakeLocked, + /// A root claim would inspect more validator hotkeys than its benchmarked bound. + TooManyRootClaimHotkeys, + /// A root claim would inspect more basket-position rows than its benchmarked bound. + TooManyRootClaimHoldings, } } diff --git a/pallets/subtensor/src/staking/claim_root.rs b/pallets/subtensor/src/staking/claim_root.rs index aefea81a64..ed70a8fe5f 100644 --- a/pallets/subtensor/src/staking/claim_root.rs +++ b/pallets/subtensor/src/staking/claim_root.rs @@ -403,10 +403,7 @@ impl Pallet { Self::hotkey_account_exists(&hotkey), Error::::HotKeyAccountNotExists ); - ensure!( - tao >= DefaultMinStake::::get(), - Error::::AmountTooLow - ); + ensure!(tao >= DefaultMinStake::::get(), Error::::AmountTooLow); ensure!( Self::can_remove_balance_from_coldkey_account(&coldkey, tao.into()), Error::::NotEnoughBalanceToStake @@ -417,8 +414,8 @@ impl Pallet { // Each weight slot can add at most one new holding, so pre-deploy holdings plus the // slot count bounds the holdings the two NAV valuations will sweep. - let num_holdings = (Self::get_basket_holdings(&hotkey).len() as u64) - .saturating_add(valid.len() as u64); + let num_holdings = + (Self::get_basket_holdings(&hotkey).len() as u64).saturating_add(valid.len() as u64); with_transaction( || match Self::try_stake_into_basket(&coldkey, &hotkey, tao, &valid) { @@ -663,34 +660,59 @@ impl Pallet { }) } - fn root_claim_weight(num_holdings: u64) -> Weight { - // Per-holding: escrow stake read/write + swap + protocol-flow bookkeeping. - Weight::from_parts(20_000_000, 3000) - .saturating_add(T::DbWeight::get().reads(4_u64)) - .saturating_add(T::DbWeight::get().writes(3_u64)) - .saturating_mul(num_holdings.max(1)) - .saturating_add(T::DbWeight::get().reads_writes(4_u64, 3_u64)) + /// Returns the bounded hotkey set and benchmark work parameter for a root claim. + /// + /// The benchmark uses one hotkey with one basket position per work unit. Bounding both + /// independent dimensions by the same parameter therefore covers callers with many empty + /// hotkeys, callers with many holdings on one hotkey, and mixtures of the two. + pub(crate) fn bounded_root_claim_work( + coldkey: &T::AccountId, + ) -> Result<(Vec, u32), DispatchError> { + let hotkey_count = StakingHotkeys::::decode_len(coldkey).unwrap_or_default(); + ensure!( + hotkey_count <= crate::MAX_ROOT_CLAIM_WORK as usize, + Error::::TooManyRootClaimHotkeys + ); + let hotkey_count = hotkey_count as u32; + + let hotkeys = StakingHotkeys::::get(coldkey); + let escrow = Self::get_beta_escrow_account_id(); + let mut holding_rows = 0_u32; + + for hotkey in &hotkeys { + for _ in Alpha::::iter_prefix((hotkey, &escrow)) + .map(|_| ()) + .chain(AlphaV2::::iter_prefix((hotkey, &escrow)).map(|_| ())) + { + holding_rows = holding_rows.saturating_add(1); + ensure!( + holding_rows <= crate::MAX_ROOT_CLAIM_WORK, + Error::::TooManyRootClaimHoldings + ); + } + } + + Ok((hotkeys, hotkey_count.max(holding_rows).max(1))) } - pub fn do_root_claim(coldkey: T::AccountId) -> Result { - with_transaction(|| match Self::try_do_root_claim(coldkey) { - Ok(weight) => TransactionOutcome::Commit(Ok(weight)), + pub fn do_root_claim( + coldkey: T::AccountId, + hotkeys: Vec, + ) -> Result<(), DispatchError> { + with_transaction(|| match Self::try_do_root_claim(coldkey, &hotkeys) { + Ok(()) => TransactionOutcome::Commit(Ok(())), Err(err) => TransactionOutcome::Rollback(Err(err)), }) } - fn try_do_root_claim(coldkey: T::AccountId) -> Result { - let mut weight = Weight::default(); - - let hotkeys = StakingHotkeys::::get(&coldkey); - weight.saturating_accrue(T::DbWeight::get().reads(1)); - + fn try_do_root_claim( + coldkey: T::AccountId, + hotkeys: &[T::AccountId], + ) -> Result<(), DispatchError> { let mut total_tao: u64 = 0; - for hotkey in hotkeys.iter() { - let num_holdings = Self::get_basket_holdings(hotkey).len() as u64; + for hotkey in hotkeys { let realized = Self::root_claim_for_hotkey(hotkey, &coldkey, false)?; total_tao = total_tao.saturating_add(realized); - weight.saturating_accrue(Self::root_claim_weight(num_holdings)); } Self::deposit_event(Event::RootClaimed { @@ -698,7 +720,7 @@ impl Pallet { tao: total_tao.into(), }); - Ok(weight) + Ok(()) } pub fn maybe_add_coldkey_index(coldkey: &T::AccountId) { diff --git a/pallets/subtensor/src/tests/claim_root.rs b/pallets/subtensor/src/tests/claim_root.rs index ff245a818f..98d4bd25d1 100644 --- a/pallets/subtensor/src/tests/claim_root.rs +++ b/pallets/subtensor/src/tests/claim_root.rs @@ -3,12 +3,13 @@ use crate::tests::mock::*; use crate::{ BasketClaimed, BasketRate, BasketShares, DefaultMinRootClaimAmount, Error, Keys, - MAX_ROOT_CLAIM_THRESHOLD, NetworksAdded, NumStakingColdkeys, RootClaimableThreshold, - StakingColdkeys, StakingColdkeysByIndex, SubnetAlphaIn, SubnetMovingPrice, - SubnetProtocolFlow, SubnetTAO, SubnetworkN, Tempo, TotalStake, Uids, Weights, + MAX_ROOT_CLAIM_THRESHOLD, MAX_ROOT_CLAIM_WORK, NetworksAdded, NumStakingColdkeys, + RootClaimableThreshold, StakingColdkeys, StakingColdkeysByIndex, StakingHotkeys, SubnetAlphaIn, + SubnetMovingPrice, SubnetProtocolFlow, SubnetTAO, SubnetworkN, Tempo, TotalStake, Uids, + Weights, }; use approx::assert_abs_diff_eq; -use frame_support::dispatch::RawOrigin; +use frame_support::dispatch::{DispatchClass, GetDispatchInfo, RawOrigin}; use frame_support::pallet_prelude::Weight; use frame_support::traits::Get; use frame_support::{assert_err, assert_noop, assert_ok}; @@ -167,6 +168,94 @@ fn test_claim_root_threshold() { }); } +#[test] +fn test_claim_root_declared_weight_covers_bounded_work() { + new_test_ext(1).execute_with(|| { + let coldkey = U256::from(1001); + let hotkey = U256::from(1002); + let escrow = SubtensorModule::get_beta_escrow_account_id(); + + mock_increase_stake_for_hotkey_and_coldkey_on_subnet( + &hotkey, + &coldkey, + NetUid::ROOT, + 1_u64.into(), + ); + mock_increase_stake_for_hotkey_and_coldkey_on_subnet( + &hotkey, + &escrow, + NetUid::ROOT, + 1_000_000_u64.into(), + ); + BasketShares::::insert(hotkey, 1); + BasketRate::::insert(hotkey, I96F32::from_num(1)); + zero_claim_threshold(); + + let call = RuntimeCall::SubtensorModule(crate::Call::claim_root {}); + let declared_weight = call.get_dispatch_info().call_weight; + let actual_weight = SubtensorModule::claim_root(RuntimeOrigin::signed(coldkey)) + .expect("claim succeeds") + .actual_weight + .expect("claim reports benchmark-derived actual weight"); + + assert!(actual_weight.all_lte(declared_weight)); + + let max_extrinsic = BlockWeights::get() + .get(DispatchClass::Normal) + .max_extrinsic + .expect("normal extrinsics have a configured maximum"); + assert!( + declared_weight.all_lte(max_extrinsic), + "declared weight {declared_weight:?} exceeds max extrinsic {max_extrinsic:?}" + ); + }); +} + +#[test] +fn test_claim_root_rejects_hotkeys_above_bound() { + new_test_ext(1).execute_with(|| { + let coldkey = U256::from(1001); + let hotkeys = (0..=MAX_ROOT_CLAIM_WORK) + .map(|index| U256::from(u64::from(index) + 10_000)) + .collect::>(); + StakingHotkeys::::insert(coldkey, hotkeys); + + assert_err!( + SubtensorModule::claim_root(RuntimeOrigin::signed(coldkey)), + Error::::TooManyRootClaimHotkeys + ); + }); +} + +#[test] +fn test_claim_root_rejects_holdings_above_bound() { + new_test_ext(1).execute_with(|| { + let coldkey = U256::from(1001); + let hotkey = U256::from(1002); + let escrow = SubtensorModule::get_beta_escrow_account_id(); + + mock_increase_stake_for_hotkey_and_coldkey_on_subnet( + &hotkey, + &coldkey, + NetUid::ROOT, + 1_u64.into(), + ); + for index in 0..=MAX_ROOT_CLAIM_WORK { + mock_increase_stake_for_hotkey_and_coldkey_on_subnet( + &hotkey, + &escrow, + NetUid::from(index as u16), + 1_u64.into(), + ); + } + + assert_err!( + SubtensorModule::claim_root(RuntimeOrigin::signed(coldkey)), + Error::::TooManyRootClaimHoldings + ); + }); +} + // ============================================================================= // Beta basket: setting weights (extrinsic validation) // ============================================================================= @@ -2647,4 +2736,3 @@ fn test_root_basket_uid0_excludes_escrow_from_denominator() { ); }); } - diff --git a/pallets/subtensor/src/weights.rs b/pallets/subtensor/src/weights.rs index 6239c31428..fd2cc32250 100644 --- a/pallets/subtensor/src/weights.rs +++ b/pallets/subtensor/src/weights.rs @@ -88,7 +88,7 @@ pub trait WeightInfo { fn commit_timelocked_weights() -> Weight; fn set_coldkey_auto_stake_hotkey() -> Weight; fn set_root_claim_type() -> Weight; - fn claim_root() -> Weight; + fn claim_root(h: u32, ) -> Weight; fn sudo_set_num_root_claims() -> Weight; fn sudo_set_root_claim_threshold() -> Weight; fn set_auto_parent_delegation_enabled() -> Weight; @@ -2738,7 +2738,7 @@ impl WeightInfo for SubstrateWeight { /// Proof: `SubtensorModule::RootClaimed` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `SubtensorModule::RootClaimableThreshold` (r:1 w:0) /// Proof: `SubtensorModule::RootClaimableThreshold` (`max_values`: None, `max_size`: None, mode: `Measured`) - fn claim_root() -> Weight { + fn claim_root(h: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `1969` // Estimated: `7909` @@ -2746,6 +2746,7 @@ impl WeightInfo for SubstrateWeight { Weight::from_parts(70_000_000, 7909) .saturating_add(T::DbWeight::get().reads(17_u64)) .saturating_add(T::DbWeight::get().writes(4_u64)) + .saturating_mul(h.into()) } /// Storage: `SubtensorModule::NumRootClaim` (r:0 w:1) /// Proof: `SubtensorModule::NumRootClaim` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) @@ -6338,7 +6339,7 @@ impl WeightInfo for () { /// Proof: `SubtensorModule::RootClaimed` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `SubtensorModule::RootClaimableThreshold` (r:1 w:0) /// Proof: `SubtensorModule::RootClaimableThreshold` (`max_values`: None, `max_size`: None, mode: `Measured`) - fn claim_root() -> Weight { + fn claim_root(h: u32, ) -> Weight { // Proof Size summary in bytes: // Measured: `1969` // Estimated: `7909` @@ -6346,6 +6347,7 @@ impl WeightInfo for () { Weight::from_parts(70_000_000, 7909) .saturating_add(RocksDbWeight::get().reads(17_u64)) .saturating_add(RocksDbWeight::get().writes(4_u64)) + .saturating_mul(h.into()) } /// Storage: `SubtensorModule::NumRootClaim` (r:0 w:1) /// Proof: `SubtensorModule::NumRootClaim` (`max_values`: Some(1), `max_size`: None, mode: `Measured`) diff --git a/runtime/tests/claim_root_weight.rs b/runtime/tests/claim_root_weight.rs new file mode 100644 index 0000000000..38f0524e97 --- /dev/null +++ b/runtime/tests/claim_root_weight.rs @@ -0,0 +1,44 @@ +use frame_support::dispatch::{DispatchClass, GetDispatchInfo}; +use node_subtensor_runtime::{ + BlockWeights, Runtime, RuntimeCall, TxExtension, check_mortality, check_nonce, sudo_wrapper, + transaction_payment_wrapper::ChargeTransactionPaymentWrapper, +}; +use sp_runtime::{generic::Era, traits::TransactionExtension}; +use subtensor_runtime_common::TaoBalance; + +#[test] +fn claim_root_with_extensions_fits_normal_extrinsic_limit() { + let call = RuntimeCall::SubtensorModule(pallet_subtensor::Call::claim_root {}); + let extensions: TxExtension = ( + ( + frame_system::CheckNonZeroSender::::new(), + frame_system::CheckSpecVersion::::new(), + frame_system::CheckTxVersion::::new(), + frame_system::CheckGenesis::::new(), + check_mortality::CheckMortality::::from(Era::Immortal), + check_nonce::CheckNonce::::from(0), + frame_system::CheckWeight::::new(), + ), + ( + ChargeTransactionPaymentWrapper::::new(TaoBalance::new(0)), + sudo_wrapper::SudoTransactionExtension::::new(), + pallet_shield::CheckShieldedTxValidity::::new(), + pallet_subtensor::SubtensorTransactionExtension::::new(), + pallet_drand::drand_priority::DrandPriority::::new(), + ), + frame_metadata_hash_extension::CheckMetadataHash::::new(true), + ); + + let mut dispatch_info = call.get_dispatch_info(); + dispatch_info.extension_weight = extensions.weight(&call); + let max_extrinsic = BlockWeights::get() + .get(DispatchClass::Normal) + .max_extrinsic + .expect("normal extrinsics have a configured maximum"); + + assert!( + dispatch_info.total_weight().all_lte(max_extrinsic), + "claim_root total weight {:?} exceeds normal max extrinsic {max_extrinsic:?}", + dispatch_info.total_weight() + ); +}