From fa7c922f3d3777f68f2ff7dd19f8f757fbaf05a6 Mon Sep 17 00:00:00 2001 From: UnArbosSix Date: Tue, 4 Aug 2026 09:03:08 -0700 Subject: [PATCH 1/3] reject invalid during transaction validation --- pallets/subtensor/src/extensions/subtensor.rs | 138 +++++++++++++++++- .../subtensor/src/guards/check_rate_limits.rs | 58 +++++++- 2 files changed, 189 insertions(+), 7 deletions(-) diff --git a/pallets/subtensor/src/extensions/subtensor.rs b/pallets/subtensor/src/extensions/subtensor.rs index 7899ed855e..2b1844fe8c 100644 --- a/pallets/subtensor/src/extensions/subtensor.rs +++ b/pallets/subtensor/src/extensions/subtensor.rs @@ -1,11 +1,11 @@ use crate::{ Call, CheckColdkeySwap, CheckDelegateTake, CheckEvmKeyAssociation, CheckRateLimits, - CheckServingEndpoints, CheckWeights, Config, Error, guards::applicable_call, + CheckServingEndpoints, CheckWeights, Config, Error, Pallet, guards::applicable_call, }; use codec::{Decode, DecodeWithMemTracking, Encode}; use frame_support::{ dispatch::{DispatchExtension, DispatchInfo, PostDispatchInfo}, - traits::{IsSubType, OriginTrait}, + traits::{Get, IsSubType, OriginTrait}, weights::Weight, }; use scale_info::TypeInfo; @@ -14,7 +14,7 @@ use sp_runtime::traits::{ }; use sp_runtime::{ impl_tx_ext_default, - transaction_validity::{TransactionSource, TransactionValidityError}, + transaction_validity::{TransactionSource, TransactionValidityError, ValidTransaction}, }; use sp_std::marker::PhantomData; use subtensor_macros::freeze_struct; @@ -77,9 +77,10 @@ impl SubtensorTransactionExtension { fn check(origin: &OriginOf, call: &CallOf) -> Result<(), Error> where - T: pallet_shield::Config, + T: pallet_commitments::Config + pallet_shield::Config, CallOf: Dispatchable> + IsSubType> + + IsSubType> + IsSubType>, OriginOf: OriginTrait, { @@ -89,6 +90,16 @@ impl SubtensorTransactionExtension { CheckColdkeySwap::::check(who, call)?; + let commitment_call: Option<&pallet_commitments::Call> = call.is_sub_type(); + if let Some(pallet_commitments::Call::set_commitment { netuid, .. }) = commitment_call { + if !Pallet::::if_subnet_exist(*netuid) { + return Err(Error::::SubnetNotExists); + } + if !Pallet::::is_hotkey_registered_on_network(*netuid, who) { + return Err(Error::::HotKeyNotRegisteredInSubNet); + } + } + if let Some(call) = applicable_call(call, CheckWeights::::applies_to) { CheckWeights::::check(who, call)?; } @@ -107,13 +118,30 @@ impl SubtensorTransactionExtension { Ok(()) } + + fn commitment_weight(call: &CallOf) -> Weight + where + T: pallet_commitments::Config, + CallOf: IsSubType>, + { + let commitment_call: Option<&pallet_commitments::Call> = call.is_sub_type(); + if matches!( + commitment_call, + Some(pallet_commitments::Call::set_commitment { .. }) + ) { + T::DbWeight::get().reads(2) + } else { + Weight::zero() + } + } } impl TransactionExtension> for SubtensorTransactionExtension where - T: Config + pallet_shield::Config + Send + Sync + TypeInfo, + T: Config + pallet_commitments::Config + pallet_shield::Config + Send + Sync + TypeInfo, CallOf: Dispatchable, Info = DispatchInfo, PostInfo = PostDispatchInfo> + IsSubType> + + IsSubType> + IsSubType>, OriginOf: Clone + OriginTrait, { @@ -131,6 +159,7 @@ where .saturating_add( as DE>>::weight(call)) .saturating_add( as DE>>::weight(call)) .saturating_add( as DE>>::weight(call)) + .saturating_add(Self::commitment_weight(call)) } fn validate( @@ -144,7 +173,17 @@ where _source: TransactionSource, ) -> ValidateResult> { Self::check(&origin, call) - .map(|()| (Default::default(), (), origin)) + .map(|()| { + let mut validity = ValidTransaction::default(); + if let Some(who) = origin.as_signer() + && let Some(call) = applicable_call(call, CheckRateLimits::::applies_to) + { + validity + .provides + .extend(CheckRateLimits::::provides_tags(who, call)); + } + (validity, (), origin) + }) .map_err(|error| TransactionValidityError::from(CustomTransactionError::from(error))) } @@ -206,6 +245,9 @@ mod tests { .saturating_add( as DE>::weight( call, )) + .saturating_add(SubtensorTransactionExtension::::commitment_weight( + call, + )) } #[test] @@ -278,6 +320,84 @@ mod tests { }); } + #[test] + fn validate_rejects_ineligible_metadata_commitment() { + new_test_ext(0).execute_with(|| { + let netuid = NetUid::from(1); + let hotkey = U256::from(1); + let coldkey = U256::from(2); + let commitment_call = || { + RuntimeCall::Commitments(pallet_commitments::Call::set_commitment { + netuid, + info: Box::new(pallet_commitments::CommitmentInfo { + fields: frame_support::BoundedVec::default(), + }), + }) + }; + + assert_eq!( + validate_signed(hotkey, &commitment_call()).unwrap_err(), + CustomTransactionError::SubnetNotExists.into() + ); + + add_network(netuid, 1, 0); + assert_eq!( + validate_signed(hotkey, &commitment_call()).unwrap_err(), + CustomTransactionError::UidNotFound.into() + ); + + setup_reserves( + netuid, + 1_000_000_000_000_u64.into(), + 1_000_000_000_000_u64.into(), + ); + register_ok_neuron(netuid, hotkey, coldkey, 0); + assert_ok!(validate_signed(hotkey, &commitment_call())); + }); + } + + #[test] + fn timelocked_commits_reject_at_validity_and_conflict_in_pool() { + new_test_ext(0).execute_with(|| { + let netuid = NetUid::from(1); + let hotkey = U256::from(1); + let coldkey = U256::from(2); + + add_network(netuid, 1, 0); + setup_reserves( + netuid, + 1_000_000_000_000_u64.into(), + 1_000_000_000_000_u64.into(), + ); + register_ok_neuron(netuid, hotkey, coldkey, 0); + SubtensorModule::set_stake_threshold(0); + SubtensorModule::set_weights_set_rate_limit(netuid, 100); + System::set_block_number(10_u64); + let uid = SubtensorModule::get_uid_for_net_and_hotkey(netuid, &hotkey).unwrap(); + let netuid_index = SubtensorModule::get_mechanism_storage_index(netuid, MechId::MAIN); + SubtensorModule::set_last_update_for_uid(netuid_index, uid, 10); + + let call = + RuntimeCall::SubtensorModule(SubtensorCall::commit_timelocked_mechanism_weights { + netuid, + mecid: MechId::MAIN, + commit: Default::default(), + reveal_round: 1, + commit_reveal_version: 4, + }); + assert_eq!( + validate_signed(hotkey, &call).unwrap_err(), + CustomTransactionError::RateLimitExceeded.into() + ); + + System::set_block_number(200_u64); + let first = validate_signed(hotkey, &call).unwrap(); + let second = validate_signed(hotkey, &call).unwrap(); + assert_eq!(first.provides.len(), 1); + assert_eq!(first.provides, second.provides); + }); + } + #[test] fn weight_matches_top_level_dispatch_extension_checks() { new_test_ext(1).execute_with(|| { @@ -293,6 +413,12 @@ mod tests { RuntimeCall::SubtensorModule(SubtensorCall::register_network { hotkey: U256::from(9), }), + RuntimeCall::Commitments(pallet_commitments::Call::set_commitment { + netuid: NetUid::from(1), + info: Box::new(pallet_commitments::CommitmentInfo { + fields: frame_support::BoundedVec::default(), + }), + }), ]; for call in calls { diff --git a/pallets/subtensor/src/guards/check_rate_limits.rs b/pallets/subtensor/src/guards/check_rate_limits.rs index e12c9d064b..7e958a1661 100644 --- a/pallets/subtensor/src/guards/check_rate_limits.rs +++ b/pallets/subtensor/src/guards/check_rate_limits.rs @@ -1,13 +1,14 @@ use super::{CallOf, DispatchableOriginOf, applicable_call}; use crate::weights::WeightInfo; use crate::{Call, Config, Error, Pallet, TransactionType}; +use codec::Encode; use frame_support::{ dispatch::{DispatchErrorWithPostInfo, DispatchExtension, DispatchInfo, PostDispatchInfo}, pallet_prelude::*, traits::{IsSubType, OriginTrait}, }; use sp_runtime::traits::Dispatchable; -use sp_std::marker::PhantomData; +use sp_std::{marker::PhantomData, vec, vec::Vec}; use subtensor_runtime_common::{NetUid, NetUidStorageIndex}; /// Dispatch extension for rate-limit checks that are safe to reject before dispatch. @@ -22,6 +23,9 @@ impl CheckRateLimits { call, Call::commit_weights { .. } | Call::commit_mechanism_weights { .. } + | Call::commit_timelocked_weights { .. } + | Call::commit_timelocked_mechanism_weights { .. } + | Call::commit_crv3_mechanism_weights { .. } | Call::set_weights { .. } | Call::set_mechanism_weights { .. } | Call::register_network { .. } @@ -60,6 +64,21 @@ impl CheckRateLimits { Pallet::::get_mechanism_storage_index(*netuid, *mecid), Error::::CommittingWeightsTooFast, ), + Call::commit_timelocked_weights { netuid, .. } => Self::check_weights_rate_limit( + who, + *netuid, + NetUidStorageIndex::from(*netuid), + Error::::CommittingWeightsTooFast, + ), + Call::commit_timelocked_mechanism_weights { netuid, mecid, .. } + | Call::commit_crv3_mechanism_weights { netuid, mecid, .. } => { + Self::check_weights_rate_limit( + who, + *netuid, + Pallet::::get_mechanism_storage_index(*netuid, *mecid), + Error::::CommittingWeightsTooFast, + ) + } Call::set_weights { netuid, .. } if !Pallet::::get_commit_reveal_weights_enabled(*netuid) => { @@ -88,6 +107,24 @@ impl CheckRateLimits { _ => Ok(()), } } + + /// One pending commit per hotkey and mechanism. Calls sharing this tag also share the same + /// on-chain rate limit, so the pool keeps only one candidate instead of landing the rest as + /// deterministic `CommittingWeightsTooFast` failures. + pub(crate) fn provides_tags(who: &T::AccountId, call: &Call) -> Vec> { + let netuid_index = match call { + Call::commit_weights { netuid, .. } + | Call::commit_timelocked_weights { netuid, .. } => NetUidStorageIndex::from(*netuid), + Call::commit_mechanism_weights { netuid, mecid, .. } + | Call::commit_timelocked_mechanism_weights { netuid, mecid, .. } + | Call::commit_crv3_mechanism_weights { netuid, mecid, .. } => { + Pallet::::get_mechanism_storage_index(*netuid, *mecid) + } + _ => return Vec::new(), + }; + + vec![(b"weight-commit", who, netuid_index).encode()] + } } impl DispatchExtension> for CheckRateLimits @@ -191,6 +228,25 @@ mod tests { mecid: MechId::MAIN, commit_hash: sp_core::H256::zero(), }), + RuntimeCall::SubtensorModule(SubtensorCall::commit_timelocked_weights { + netuid, + commit: Default::default(), + reveal_round: 1, + commit_reveal_version: 4, + }), + RuntimeCall::SubtensorModule(SubtensorCall::commit_timelocked_mechanism_weights { + netuid, + mecid: MechId::MAIN, + commit: Default::default(), + reveal_round: 1, + commit_reveal_version: 4, + }), + RuntimeCall::SubtensorModule(SubtensorCall::commit_crv3_mechanism_weights { + netuid, + mecid: MechId::MAIN, + commit: Default::default(), + reveal_round: 1, + }), set_weights_call(netuid, 0), RuntimeCall::SubtensorModule(SubtensorCall::set_mechanism_weights { netuid, From 2769b73a8a94b369c8c712553a0a28b17125f54f Mon Sep 17 00:00:00 2001 From: UnArbosSix Date: Tue, 4 Aug 2026 10:16:18 -0700 Subject: [PATCH 2/3] fix provides tag --- pallets/subtensor/src/extensions/subtensor.rs | 33 +++++++++++++++++++ .../subtensor/src/guards/check_rate_limits.rs | 23 ++++++++----- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/pallets/subtensor/src/extensions/subtensor.rs b/pallets/subtensor/src/extensions/subtensor.rs index 2b1844fe8c..3842332cf1 100644 --- a/pallets/subtensor/src/extensions/subtensor.rs +++ b/pallets/subtensor/src/extensions/subtensor.rs @@ -398,6 +398,39 @@ mod tests { }); } + #[test] + fn timelocked_commits_with_zero_rate_limit_do_not_conflict_in_pool() { + new_test_ext(0).execute_with(|| { + let netuid = NetUid::from(1); + let hotkey = U256::from(1); + let coldkey = U256::from(2); + + add_network(netuid, 1, 0); + setup_reserves( + netuid, + 1_000_000_000_000_u64.into(), + 1_000_000_000_000_u64.into(), + ); + register_ok_neuron(netuid, hotkey, coldkey, 0); + SubtensorModule::set_stake_threshold(0); + SubtensorModule::set_weights_set_rate_limit(netuid, 0); + + let call = + RuntimeCall::SubtensorModule(SubtensorCall::commit_timelocked_mechanism_weights { + netuid, + mecid: MechId::MAIN, + commit: Default::default(), + reveal_round: 1, + commit_reveal_version: 4, + }); + + let first = validate_signed(hotkey, &call).unwrap(); + let second = validate_signed(hotkey, &call).unwrap(); + assert!(first.provides.is_empty()); + assert!(second.provides.is_empty()); + }); + } + #[test] fn weight_matches_top_level_dispatch_extension_checks() { new_test_ext(1).execute_with(|| { diff --git a/pallets/subtensor/src/guards/check_rate_limits.rs b/pallets/subtensor/src/guards/check_rate_limits.rs index 7e958a1661..b241903ce5 100644 --- a/pallets/subtensor/src/guards/check_rate_limits.rs +++ b/pallets/subtensor/src/guards/check_rate_limits.rs @@ -108,21 +108,28 @@ impl CheckRateLimits { } } - /// One pending commit per hotkey and mechanism. Calls sharing this tag also share the same - /// on-chain rate limit, so the pool keeps only one candidate instead of landing the rest as - /// deterministic `CommittingWeightsTooFast` failures. + /// One pending commit per hotkey and mechanism when commits are rate limited. Calls sharing + /// this tag also share the same on-chain rate limit, so the pool keeps only one candidate + /// instead of landing the rest as deterministic `CommittingWeightsTooFast` failures. pub(crate) fn provides_tags(who: &T::AccountId, call: &Call) -> Vec> { - let netuid_index = match call { + let (netuid, netuid_index) = match call { Call::commit_weights { netuid, .. } - | Call::commit_timelocked_weights { netuid, .. } => NetUidStorageIndex::from(*netuid), + | Call::commit_timelocked_weights { netuid, .. } => { + (*netuid, NetUidStorageIndex::from(*netuid)) + } Call::commit_mechanism_weights { netuid, mecid, .. } | Call::commit_timelocked_mechanism_weights { netuid, mecid, .. } - | Call::commit_crv3_mechanism_weights { netuid, mecid, .. } => { - Pallet::::get_mechanism_storage_index(*netuid, *mecid) - } + | Call::commit_crv3_mechanism_weights { netuid, mecid, .. } => ( + *netuid, + Pallet::::get_mechanism_storage_index(*netuid, *mecid), + ), _ => return Vec::new(), }; + if Pallet::::get_weights_set_rate_limit(netuid) == 0 { + return Vec::new(); + } + vec![(b"weight-commit", who, netuid_index).encode()] } } From ef6a09b8da910f598b9f1f58cbd955f4799a3fee Mon Sep 17 00:00:00 2001 From: UnArbosSix Date: Tue, 4 Aug 2026 10:47:05 -0700 Subject: [PATCH 3/3] use existing helper --- pallets/commitments/src/lib.rs | 24 ++++++++++----- pallets/commitments/src/mock.rs | 10 +++++-- pallets/subtensor/src/extensions/subtensor.rs | 17 +++++------ pallets/subtensor/src/tests/mock.rs | 16 ++++++++-- runtime/src/lib.rs | 29 +++++++++++++++---- 5 files changed, 70 insertions(+), 26 deletions(-) diff --git a/pallets/commitments/src/lib.rs b/pallets/commitments/src/lib.rs index 5ed05744ed..d1f085747d 100644 --- a/pallets/commitments/src/lib.rs +++ b/pallets/commitments/src/lib.rs @@ -211,7 +211,8 @@ pub mod pallet { /// Set the commitment for a given netuid #[pallet::call_index(0)] #[pallet::weight(( - ::WeightInfo::set_commitment(), + ::WeightInfo::set_commitment() + .saturating_add(T::CanCommit::validation_weight()), DispatchClass::Normal, Pays::No ))] @@ -221,10 +222,8 @@ pub mod pallet { info: Box>, ) -> DispatchResult { let who = ensure_signed(origin.clone())?; - ensure!( - T::CanCommit::can_commit(netuid, &who), - Error::::AccountNotAllowedCommit - ); + T::CanCommit::validate(netuid, &who) + .map_err(|_| Error::::AccountNotAllowedCommit)?; let extra_fields = info.fields.len() as u32; ensure!( @@ -356,12 +355,21 @@ pub mod pallet { // Interfaces to interact with other pallets pub trait CanCommit { - fn can_commit(netuid: NetUid, who: &AccountId) -> bool; + type Error; + + fn validate(netuid: NetUid, who: &AccountId) -> Result<(), Self::Error>; + fn validation_weight() -> frame_support::weights::Weight; } impl CanCommit for () { - fn can_commit(_: NetUid, _: &A) -> bool { - false + type Error = (); + + fn validate(_: NetUid, _: &A) -> Result<(), Self::Error> { + Err(()) + } + + fn validation_weight() -> frame_support::weights::Weight { + frame_support::weights::Weight::zero() } } diff --git a/pallets/commitments/src/mock.rs b/pallets/commitments/src/mock.rs index 58ed8cd863..c626a6f784 100644 --- a/pallets/commitments/src/mock.rs +++ b/pallets/commitments/src/mock.rs @@ -90,8 +90,14 @@ impl TypeInfo for TestMaxFields { pub struct TestCanCommit; impl pallet_commitments::CanCommit for TestCanCommit { - fn can_commit(_netuid: NetUid, _who: &u64) -> bool { - true + type Error = (); + + fn validate(_netuid: NetUid, _who: &u64) -> Result<(), Self::Error> { + Ok(()) + } + + fn validation_weight() -> Weight { + Weight::zero() } } diff --git a/pallets/subtensor/src/extensions/subtensor.rs b/pallets/subtensor/src/extensions/subtensor.rs index 3842332cf1..983a1ce97d 100644 --- a/pallets/subtensor/src/extensions/subtensor.rs +++ b/pallets/subtensor/src/extensions/subtensor.rs @@ -1,13 +1,14 @@ use crate::{ Call, CheckColdkeySwap, CheckDelegateTake, CheckEvmKeyAssociation, CheckRateLimits, - CheckServingEndpoints, CheckWeights, Config, Error, Pallet, guards::applicable_call, + CheckServingEndpoints, CheckWeights, Config, Error, guards::applicable_call, }; use codec::{Decode, DecodeWithMemTracking, Encode}; use frame_support::{ dispatch::{DispatchExtension, DispatchInfo, PostDispatchInfo}, - traits::{Get, IsSubType, OriginTrait}, + traits::{IsSubType, OriginTrait}, weights::Weight, }; +use pallet_commitments::CanCommit; use scale_info::TypeInfo; use sp_runtime::traits::{ DispatchInfoOf, Dispatchable, Implication, TransactionExtension, ValidateResult, @@ -22,6 +23,7 @@ use subtensor_runtime_common::CustomTransactionError; type CallOf = ::RuntimeCall; type OriginOf = ::RuntimeOrigin; +type CommitmentPolicy = ::CanCommit; #[allow(deprecated)] impl From> for CustomTransactionError { @@ -83,6 +85,7 @@ impl SubtensorTransactionExtension { + IsSubType> + IsSubType>, OriginOf: OriginTrait, + CommitmentPolicy: CanCommit>, { let Some(who) = origin.as_signer() else { return Ok(()); @@ -92,12 +95,7 @@ impl SubtensorTransactionExtension { let commitment_call: Option<&pallet_commitments::Call> = call.is_sub_type(); if let Some(pallet_commitments::Call::set_commitment { netuid, .. }) = commitment_call { - if !Pallet::::if_subnet_exist(*netuid) { - return Err(Error::::SubnetNotExists); - } - if !Pallet::::is_hotkey_registered_on_network(*netuid, who) { - return Err(Error::::HotKeyNotRegisteredInSubNet); - } + CommitmentPolicy::::validate(*netuid, who)?; } if let Some(call) = applicable_call(call, CheckWeights::::applies_to) { @@ -129,7 +127,7 @@ impl SubtensorTransactionExtension { commitment_call, Some(pallet_commitments::Call::set_commitment { .. }) ) { - T::DbWeight::get().reads(2) + CommitmentPolicy::::validation_weight() } else { Weight::zero() } @@ -144,6 +142,7 @@ where + IsSubType> + IsSubType>, OriginOf: Clone + OriginTrait, + CommitmentPolicy: CanCommit>, { const IDENTIFIER: &'static str = "SubtensorTransactionExtension"; diff --git a/pallets/subtensor/src/tests/mock.rs b/pallets/subtensor/src/tests/mock.rs index 0452471018..9fcc28f866 100644 --- a/pallets/subtensor/src/tests/mock.rs +++ b/pallets/subtensor/src/tests/mock.rs @@ -52,8 +52,20 @@ impl Get for TestMaxFields { pub struct TestCanCommit; impl pallet_commitments::CanCommit for TestCanCommit { - fn can_commit(_netuid: NetUid, _who: &U256) -> bool { - true + type Error = crate::Error; + + fn validate(netuid: NetUid, who: &U256) -> Result<(), Self::Error> { + if !SubtensorModule::if_subnet_exist(netuid) { + return Err(crate::Error::::SubnetNotExists); + } + if !SubtensorModule::is_hotkey_registered_on_network(netuid, who) { + return Err(crate::Error::::HotKeyNotRegisteredInSubNet); + } + Ok(()) + } + + fn validation_weight() -> Weight { + ::DbWeight::get().reads(2) } } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index f4ad13bf81..56fbc7a287 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -712,15 +712,34 @@ impl Get for MaxCommitFields { #[subtensor_macros::freeze_struct("c39297f5eb97ee82")] pub struct AllowCommitments; impl CanCommit for AllowCommitments { + type Error = pallet_subtensor::Error; + #[cfg(not(feature = "runtime-benchmarks"))] - fn can_commit(netuid: NetUid, address: &AccountId) -> bool { - SubtensorModule::if_subnet_exist(netuid) - && SubtensorModule::is_hotkey_registered_on_network(netuid, address) + fn validate(netuid: NetUid, address: &AccountId) -> Result<(), Self::Error> { + if !SubtensorModule::if_subnet_exist(netuid) { + return Err(pallet_subtensor::Error::::SubnetNotExists); + } + if !SubtensorModule::is_hotkey_registered_on_network(netuid, address) { + return Err(pallet_subtensor::Error::::HotKeyNotRegisteredInSubNet); + } + Ok(()) } #[cfg(feature = "runtime-benchmarks")] - fn can_commit(_: NetUid, _: &AccountId) -> bool { - true + fn validate(_: NetUid, _: &AccountId) -> Result<(), Self::Error> { + Ok(()) + } + + fn validation_weight() -> frame_support::weights::Weight { + #[cfg(not(feature = "runtime-benchmarks"))] + { + ::DbWeight::get().reads(2) + } + + #[cfg(feature = "runtime-benchmarks")] + { + frame_support::weights::Weight::zero() + } } }