From ca76284c06b0da8751c7ca2b95a604f541d81443 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 29 Apr 2026 17:43:28 -0300 Subject: [PATCH 01/14] Split polls traits and prepare for benchmarking --- common/src/traits.rs | 35 +++++++++++++++++++++++++-- pallets/referenda/src/lib.rs | 39 +++++++++++++++++++------------ pallets/referenda/src/mock.rs | 3 ++- pallets/signed-voting/src/lib.rs | 14 +++++++++-- pallets/signed-voting/src/mock.rs | 10 +++++--- runtime/src/lib.rs | 3 ++- 6 files changed, 80 insertions(+), 24 deletions(-) diff --git a/common/src/traits.rs b/common/src/traits.rs index d4dc0e79e1..015281960f 100644 --- a/common/src/traits.rs +++ b/common/src/traits.rs @@ -9,6 +9,9 @@ pub trait SetLike { } } +/// Poll provider seen from the voting pallet's side. Carries the +/// read-only queries plus the tally-update notification fired when a +/// vote moves the tally. pub trait Polls { type Index: Parameter + Copy; type VotingScheme: PartialEq; @@ -17,20 +20,48 @@ pub trait Polls { fn is_ongoing(index: Self::Index) -> bool; fn voting_scheme_of(index: Self::Index) -> Option; fn voter_set_of(index: Self::Index) -> Option; + fn on_tally_updated(index: Self::Index, tally: &VoteTally); + /// Worst-case upper bound on `on_tally_updated`'s weight. + fn on_tally_updated_weight() -> Weight; } -pub trait PollHooks { +/// Notification fired when a poll is created. +pub trait OnPollCreated { fn on_poll_created(poll_index: PollIndex); + /// Returns the worst-case upper bound on `on_poll_created`'s weight. + fn weight() -> Weight; +} + +/// Notification fired when a poll reaches a terminal status. +pub trait OnPollCompleted { fn on_poll_completed(poll_index: PollIndex); + /// Returns the worst-case upper bound on `on_poll_completed`'s weight. + fn weight() -> Weight; } #[impl_trait_for_tuples::impl_for_tuples(10)] -impl PollHooks for Tuple { +impl OnPollCreated for Tuple { fn on_poll_created(poll_index: I) { for_tuples!( #( Tuple::on_poll_created(poll_index); )* ); } + + fn weight() -> Weight { + let mut weight = Weight::zero(); + for_tuples!( #( weight = weight.saturating_add(Tuple::weight()); )* ); + weight + } +} + +#[impl_trait_for_tuples::impl_for_tuples(10)] +impl OnPollCompleted for Tuple { fn on_poll_completed(poll_index: I) { for_tuples!( #( Tuple::on_poll_completed(poll_index); )* ); } + + fn weight() -> Weight { + let mut weight = Weight::zero(); + for_tuples!( #( weight = weight.saturating_add(Tuple::weight()); )* ); + weight + } } diff --git a/pallets/referenda/src/lib.rs b/pallets/referenda/src/lib.rs index 3a2fe091ee..783e7fdfc6 100644 --- a/pallets/referenda/src/lib.rs +++ b/pallets/referenda/src/lib.rs @@ -22,7 +22,7 @@ //! `submit` records a referendum, schedules the relevant scheduler entries //! (an alarm for `PassOrFail`; an enactment task plus a reaper alarm for //! `Adjustable`), and notifies subscribers via -//! [`PollHooks::on_poll_created`]. +//! [`OnPollCreated::on_poll_created`]. //! //! Tally updates arrive through [`Polls::on_tally_updated`]. The hook is //! intentionally side-effect-light: it stores the new tally and arms an @@ -140,7 +140,7 @@ use frame_support::{ }, }; use frame_system::pallet_prelude::*; -use subtensor_runtime_common::{PollHooks, Polls, SetLike, VoteTally}; +use subtensor_runtime_common::{OnPollCompleted, OnPollCreated, Polls, SetLike, VoteTally}; pub use pallet::*; pub use types::*; @@ -201,10 +201,14 @@ pub mod pallet { /// expose a different block-number authority. type BlockNumberProvider: BlockNumberProvider>; - /// Lifecycle hooks invoked when a referendum is created or - /// completed. Notifies any subscriber that needs to react to those - /// events. - type PollHooks: PollHooks; + /// Subscriber notified when a new referendum is created. The hook + /// returns its actual weight; the pallet pre-charges + /// `OnPollCreated::weight()` and refunds the unused portion. + type OnPollCreated: OnPollCreated; + + /// Subscriber notified when a referendum reaches a terminal status. + /// Same weight contract as [`OnPollCreated`]. + type OnPollCompleted: OnPollCompleted; } /// Monotonic referendum id generator. Incremented by `submit`; never @@ -368,7 +372,7 @@ pub mod pallet { }; ReferendumStatusFor::::insert(index, ReferendumStatus::Ongoing(info)); - T::PollHooks::on_poll_created(index); + T::OnPollCreated::on_poll_created(index); Self::deposit_event(Event::::Submitted { index, @@ -421,7 +425,7 @@ pub mod pallet { // Terminal state: nothing further to do. Reached when an // alarm fires after a manual kill or a delegated handoff. } - }; + } Ok(()) } @@ -537,16 +541,17 @@ impl Pallet { } /// Move a referendum to a terminal status: cancel any pending alarm, - /// store the new status, decrement `ActiveCount`, notify voting pallets, - /// and emit `event`. Callers that need a follow-up alarm (the - /// `Approved -> Enacted` and `FastTracked -> Enacted` transitions) must - /// call `set_alarm` AFTER this function, since `conclude` cancels - /// whatever alarm is currently scheduled. + /// store the new status, decrement `ActiveCount`, notify subscribers + /// via `OnPollCompleted`, and emit `event`. Callers that need a + /// follow-up alarm (the `Approved -> Enacted` and + /// `FastTracked -> Enacted` transitions) must call `set_alarm` AFTER + /// this function, since `conclude` cancels whatever alarm is currently + /// scheduled. fn conclude(index: ReferendumIndex, status: ReferendumStatusOf, event: Event) { let _ = T::Scheduler::cancel_named(alarm_name(index)); ReferendumStatusFor::::insert(index, status); ActiveCount::::mutate(|c| *c = c.saturating_sub(1)); - T::PollHooks::on_poll_completed(index); + T::OnPollCompleted::on_poll_completed(index); Self::deposit_event(event); } @@ -662,7 +667,7 @@ impl Pallet { }; ReferendumStatusFor::::insert(new_index, ReferendumStatus::Ongoing(new_info)); - T::PollHooks::on_poll_created(new_index); + T::OnPollCreated::on_poll_created(new_index); Some(new_index) } @@ -884,4 +889,8 @@ impl Polls for Pallet { Self::report_scheduler_error(index, "set_alarm", err); } } + + fn on_tally_updated_weight() -> Weight { + T::WeightInfo::on_tally_updated() + } } diff --git a/pallets/referenda/src/mock.rs b/pallets/referenda/src/mock.rs index 89b462ef4d..c3ccf1e102 100644 --- a/pallets/referenda/src/mock.rs +++ b/pallets/referenda/src/mock.rs @@ -363,7 +363,8 @@ impl pallet_referenda::Config for Test { type KillOrigin = EnsureRoot; type Tracks = TestTracks; type BlockNumberProvider = System; - type PollHooks = SignedVoting; + type OnPollCreated = SignedVoting; + type OnPollCompleted = SignedVoting; } pub struct TestState { diff --git a/pallets/signed-voting/src/lib.rs b/pallets/signed-voting/src/lib.rs index 5d74400376..a4b17d7094 100644 --- a/pallets/signed-voting/src/lib.rs +++ b/pallets/signed-voting/src/lib.rs @@ -8,7 +8,7 @@ use frame_support::{ sp_runtime::{Perbill, Saturating}, }; use frame_system::pallet_prelude::*; -use subtensor_runtime_common::{PollHooks, Polls, SetLike, VoteTally}; +use subtensor_runtime_common::{OnPollCompleted, OnPollCreated, Polls, SetLike, VoteTally}; pub use pallet::*; @@ -286,7 +286,7 @@ impl Pallet { } } -impl PollHooks> for Pallet { +impl OnPollCreated> for Pallet { fn on_poll_created(poll_index: PollIndexOf) { let total = T::Polls::voter_set_of(poll_index) .map(|voter_set| voter_set.len()) @@ -302,10 +302,20 @@ impl PollHooks> for Pallet { ); } + fn weight() -> Weight { + Weight::zero() + } +} + +impl OnPollCompleted> for Pallet { fn on_poll_completed(poll_index: PollIndexOf) { // `u32::MAX` is effectively unbounded. `VotingFor` entries per poll // are bounded by the voter-set size, so one call clears everything. let _ = VotingFor::::clear_prefix(poll_index, u32::MAX, None); TallyOf::::remove(poll_index); } + + fn weight() -> Weight { + Weight::zero() + } } diff --git a/pallets/signed-voting/src/mock.rs b/pallets/signed-voting/src/mock.rs index 85168a94ac..bc18a6b3fd 100644 --- a/pallets/signed-voting/src/mock.rs +++ b/pallets/signed-voting/src/mock.rs @@ -14,7 +14,7 @@ use frame_support::{ sp_runtime::{BuildStorage, traits::IdentityLookup}, }; use sp_core::U256; -use subtensor_runtime_common::{PollHooks, Polls, SetLike, VoteTally}; +use subtensor_runtime_common::{OnPollCompleted, OnPollCreated, Polls, SetLike, VoteTally}; use crate::{self as pallet_signed_voting}; @@ -108,6 +108,10 @@ impl Polls for MockPolls { fn on_tally_updated(index: Self::Index, tally: &VoteTally) { TALLY_UPDATES.with(|t| t.borrow_mut().push((index, *tally))); } + + fn on_tally_updated_weight() -> Weight { + Weight::zero() + } } // --- Helpers --- @@ -125,7 +129,7 @@ pub fn start_poll(index: u32, scheme: VotingScheme, voter_set: Vec) { }, ); }); - >::on_poll_created(index); + >::on_poll_created(index); } /// Mark the poll inactive and fire `on_poll_completed` to clean up storage. @@ -135,7 +139,7 @@ pub fn complete_poll(index: u32) { s.is_ongoing = false; } }); - >::on_poll_completed(index); + >::on_poll_completed(index); } /// Simulate membership rotation by removing `who` from a poll's voter set diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 4dfb392a90..c3880b7a3e 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1873,7 +1873,8 @@ impl pallet_referenda::Config for Runtime { type KillOrigin = EnsureRoot; type Tracks = governance::tracks::SubtensorTracks; type BlockNumberProvider = System; - type PollHooks = SignedVoting; + type OnPollCreated = SignedVoting; + type OnPollCompleted = SignedVoting; } // Create the runtime by composing the FRAME pallets that were previously configured. From 20b97d0fffb08a856c477305a1ca0486ae7d3d59 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 29 Apr 2026 17:55:57 -0300 Subject: [PATCH 02/14] Added benchmarks for referenda --- Cargo.lock | 1 + pallets/referenda/Cargo.toml | 17 ++- pallets/referenda/src/benchmarking.rs | 112 ++++++++++++++ pallets/referenda/src/lib.rs | 48 +++++- pallets/referenda/src/mock.rs | 42 ++++- pallets/referenda/src/weights.rs | 212 ++++++++++++++++++++++++++ runtime/Cargo.toml | 3 +- runtime/src/lib.rs | 36 +++++ scripts/benchmark_all.sh | 6 +- 9 files changed, 469 insertions(+), 8 deletions(-) create mode 100644 pallets/referenda/src/benchmarking.rs create mode 100644 pallets/referenda/src/weights.rs diff --git a/Cargo.lock b/Cargo.lock index eca72a02f8..d509abab55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10366,6 +10366,7 @@ dependencies = [ name = "pallet-referenda" version = "1.0.0" dependencies = [ + "frame-benchmarking", "frame-support", "frame-system", "log", diff --git a/pallets/referenda/Cargo.toml b/pallets/referenda/Cargo.toml index b1501550fc..d2753a09ac 100644 --- a/pallets/referenda/Cargo.toml +++ b/pallets/referenda/Cargo.toml @@ -19,6 +19,7 @@ codec = { workspace = true, features = ["max-encoded-len"] } scale-info = { workspace = true, features = ["derive"] } frame-system = { workspace = true } frame-support = { workspace = true } +frame-benchmarking = { workspace = true, optional = true } sp-runtime = { workspace = true } sp-io = { workspace = true } subtensor-macros.workspace = true @@ -42,9 +43,21 @@ std = [ "scale-info/std", "frame-system/std", "frame-support/std", + "frame-benchmarking?/std", "sp-runtime/std", + "sp-io/std", "subtensor-runtime-common/std", "log/std", ] -runtime-benchmarks = [] -try-runtime = [] +runtime-benchmarks = [ + "frame-benchmarking/runtime-benchmarks", + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", + "sp-runtime/runtime-benchmarks", + "subtensor-runtime-common/runtime-benchmarks", +] +try-runtime = [ + "frame-support/try-runtime", + "frame-system/try-runtime", + "sp-runtime/try-runtime", +] diff --git a/pallets/referenda/src/benchmarking.rs b/pallets/referenda/src/benchmarking.rs new file mode 100644 index 0000000000..30b7226809 --- /dev/null +++ b/pallets/referenda/src/benchmarking.rs @@ -0,0 +1,112 @@ +//! Benchmarks for `pallet_referenda`. +//! +//! Setup is parameterised through [`Config::BenchmarkHelper`]: the runtime +//! supplies track ids of each strategy variant plus a proposer that's +//! already in the relevant proposer set. +//! +//! `advance_referendum` is benchmarked on its worst-case branch +//! (approve-with-`Review`): the parent fires `OnPollCompleted`, the child +//! fires `OnPollCreated`, and two scheduler operations run. Every other +//! branch is strictly cheaper, so a single figure soundly bounds them all. + +#![cfg(feature = "runtime-benchmarks")] + +use super::*; +use alloc::boxed::Box; +use frame_benchmarking::v2::*; +use frame_system::RawOrigin; +use sp_runtime::Perbill; + +#[benchmarks] +mod benches { + use super::*; + + #[benchmark] + fn submit() { + let proposer = T::BenchmarkHelper::proposer(); + let track = T::BenchmarkHelper::track_passorfail(); + let call = Box::new(T::BenchmarkHelper::call()); + + #[extrinsic_call] + submit(RawOrigin::Signed(proposer), track, call); + + assert_eq!(ActiveCount::::get(), 1); + } + + #[benchmark] + fn kill() { + let proposer = T::BenchmarkHelper::proposer(); + let track = T::BenchmarkHelper::track_passorfail(); + let call = Box::new(T::BenchmarkHelper::call()); + let index = ReferendumCount::::get(); + Pallet::::submit(RawOrigin::Signed(proposer).into(), track, call) + .expect("submit must succeed in benchmark setup"); + + #[extrinsic_call] + kill(RawOrigin::Root, index); + + assert!(matches!( + ReferendumStatusFor::::get(index), + Some(ReferendumStatus::Killed(_)) + )); + } + + /// Worst-case `advance_referendum`: PassOrFail with `Review` outcome. + /// Fires both `OnPollCreated` (for the child) and `OnPollCompleted` + /// (parent), runs two scheduler operations. + #[benchmark] + fn advance_referendum() { + let proposer = T::BenchmarkHelper::proposer(); + let track = T::BenchmarkHelper::track_passorfail(); + let call = Box::new(T::BenchmarkHelper::call()); + let index = ReferendumCount::::get(); + Pallet::::submit(RawOrigin::Signed(proposer).into(), track, call) + .expect("submit must succeed in benchmark setup"); + + // Force the approve-with-Review branch by overwriting the tally. + let mut info = match ReferendumStatusFor::::get(index) { + Some(ReferendumStatus::Ongoing(info)) => info, + _ => panic!("expected ongoing referendum"), + }; + info.tally = VoteTally { + approval: Perbill::one(), + rejection: Perbill::zero(), + abstention: Perbill::zero(), + }; + ReferendumStatusFor::::insert(index, ReferendumStatus::Ongoing(info)); + + #[extrinsic_call] + advance_referendum(RawOrigin::Root, index); + + // Either Delegated (Review path) or Approved (Execute fallback). + assert!(!matches!( + ReferendumStatusFor::::get(index), + Some(ReferendumStatus::Ongoing(_)) + )); + } + + /// `OnTallyUpdated` hook: stores the new tally and arms an alarm at + /// `now + 1`. Benchmarked as a function call rather than an extrinsic. + #[benchmark] + fn on_tally_updated() { + let proposer = T::BenchmarkHelper::proposer(); + let track = T::BenchmarkHelper::track_passorfail(); + let call = Box::new(T::BenchmarkHelper::call()); + let index = ReferendumCount::::get(); + Pallet::::submit(RawOrigin::Signed(proposer).into(), track, call) + .expect("submit must succeed in benchmark setup"); + + let tally = VoteTally { + approval: Perbill::from_percent(50), + rejection: Perbill::from_percent(10), + abstention: Perbill::from_percent(40), + }; + + #[block] + { + as Polls>::on_tally_updated(index, &tally); + } + } + + impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test); +} diff --git a/pallets/referenda/src/lib.rs b/pallets/referenda/src/lib.rs index 783e7fdfc6..2e60ddd046 100644 --- a/pallets/referenda/src/lib.rs +++ b/pallets/referenda/src/lib.rs @@ -144,15 +144,19 @@ use subtensor_runtime_common::{OnPollCompleted, OnPollCreated, Polls, SetLike, V pub use pallet::*; pub use types::*; +pub use weights::WeightInfo; +#[cfg(feature = "runtime-benchmarks")] +mod benchmarking; mod types; +pub mod weights; #[cfg(test)] mod mock; #[cfg(test)] mod tests; -#[frame_support::pallet(dev_mode)] +#[frame_support::pallet] #[allow(clippy::expect_used)] pub mod pallet { use super::*; @@ -209,6 +213,36 @@ pub mod pallet { /// Subscriber notified when a referendum reaches a terminal status. /// Same weight contract as [`OnPollCreated`]. type OnPollCompleted: OnPollCompleted; + + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; + + /// Helper for setting up cross-pallet state needed by benchmarks. + /// The runtime provides track ids of each strategy variant plus a + /// proposer guaranteed to be in those tracks' proposer sets. + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper: BenchmarkHelper, Self::AccountId, CallOf>; + } + + /// Benchmark setup helper. The runtime wires this with track ids and a + /// proposer that match its track table; the mock provides defaults + /// matching `pallet-referenda::mock::TestTracks`. + /// + /// Note: only a `PassOrFail` track is needed for the approve benchmark + /// because the `Review` outcome is the worst case and bounds `Execute` + /// from above (see [`weights::WeightInfo`]). + #[cfg(feature = "runtime-benchmarks")] + pub trait BenchmarkHelper { + /// Track id of a `PassOrFail` track. The benchmark drives both the + /// approve and reject paths through it. + fn track_passorfail() -> TrackId; + /// Track id of an `Adjustable` track. + fn track_adjustable() -> TrackId; + /// Account in the proposer set of both tracks returned above. + fn proposer() -> AccountId; + /// A call that `T::Tracks::authorize_proposal` accepts. Should be + /// cheap to bound (e.g. `frame_system::remark`). + fn call() -> Call; } /// Monotonic referendum id generator. Incremented by `submit`; never @@ -314,6 +348,9 @@ pub mod pallet { /// `PassOrFail`, `Review` for `Adjustable` (with the call scheduled /// for dispatch after `initial_delay`). #[pallet::call_index(0)] + #[pallet::weight( + T::WeightInfo::submit().saturating_add(T::OnPollCreated::weight()) + )] pub fn submit( origin: OriginFor, track: TrackIdOf, @@ -386,6 +423,9 @@ pub mod pallet { /// Privileged termination of an ongoing referendum. Cancels any /// pending scheduler entries and concludes as `Killed`. #[pallet::call_index(1)] + #[pallet::weight( + T::WeightInfo::kill().saturating_add(T::OnPollCompleted::weight()) + )] pub fn kill(origin: OriginFor, index: ReferendumIndex) -> DispatchResult { T::KillOrigin::ensure_origin(origin)?; @@ -409,6 +449,12 @@ pub mod pallet { /// Drive the state machine for `index`. Invoked by the alarm and /// available as a privileged extrinsic for manual recovery. #[pallet::call_index(2)] + #[pallet::weight( + // Worst-case bound: the approve-with-`Review` branch fires both hooks. + T::WeightInfo::advance_referendum() + .saturating_add(T::OnPollCreated::weight()) + .saturating_add(T::OnPollCompleted::weight()) + )] pub fn advance_referendum(origin: OriginFor, index: ReferendumIndex) -> DispatchResult { ensure_root(origin)?; diff --git a/pallets/referenda/src/mock.rs b/pallets/referenda/src/mock.rs index c3ccf1e102..e25a416ef3 100644 --- a/pallets/referenda/src/mock.rs +++ b/pallets/referenda/src/mock.rs @@ -365,6 +365,30 @@ impl pallet_referenda::Config for Test { type BlockNumberProvider = System; type OnPollCreated = SignedVoting; type OnPollCompleted = SignedVoting; + type WeightInfo = (); + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = TestBenchmarkHelper; +} + +#[cfg(feature = "runtime-benchmarks")] +pub struct TestBenchmarkHelper; + +#[cfg(feature = "runtime-benchmarks")] +impl pallet_referenda::BenchmarkHelper for TestBenchmarkHelper { + /// Track 2: `PassOrFail` with `Review { track: 1 }`. Worst case for + /// the approve benchmark (creates a child referendum). + fn track_passorfail() -> u8 { + 2 + } + fn track_adjustable() -> u8 { + 1 + } + fn proposer() -> U256 { + U256::from(1) + } + fn call() -> RuntimeCall { + RuntimeCall::System(frame_system::Call::remark { remark: vec![] }) + } } pub struct TestState { @@ -383,6 +407,14 @@ impl Default for TestState { impl TestState { pub fn build_and_execute(self, test: impl FnOnce()) { + let mut ext = self.into_test_ext(); + ext.execute_with(test); + } + + /// Build the externalities object pre-populated with collectives. + /// Exposed for `impl_benchmark_test_suite!`, which expects a builder + /// that returns `sp_io::TestExternalities` rather than a `FnOnce`. + pub fn into_test_ext(self) -> sp_io::TestExternalities { let mut ext: sp_io::TestExternalities = RuntimeGenesisConfig { system: frame_system::GenesisConfig::default(), balances: pallet_balances::GenesisConfig::default(), @@ -412,12 +444,18 @@ impl TestState { ) .unwrap(); } - - test(); }); + + ext } } +/// Externalities builder for `impl_benchmark_test_suite!`. +#[cfg(feature = "runtime-benchmarks")] +pub fn new_test_ext() -> sp_io::TestExternalities { + TestState::default().into_test_ext() +} + pub fn run_to_block(n: u64) { System::run_to_block::(n); } diff --git a/pallets/referenda/src/weights.rs b/pallets/referenda/src/weights.rs new file mode 100644 index 0000000000..69bcdf3f5e --- /dev/null +++ b/pallets/referenda/src/weights.rs @@ -0,0 +1,212 @@ + +//! Autogenerated weights for `pallet_referenda` +//! +//! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 49.1.0 +//! DATE: 2026-04-29, STEPS: `50`, REPEAT: `20`, LOW RANGE: `[]`, HIGH RANGE: `[]` +//! WORST CASE MAP SIZE: `1000000` +//! HOSTNAME: `users-MacBook-Air.local`, CPU: `` +//! WASM-EXECUTION: `Compiled`, CHAIN: `None`, DB CACHE: `1024` + +// Executed Command: +// /Users/user/Work/subtensor/target/production/node-subtensor +// benchmark +// pallet +// --runtime=/Users/user/Work/subtensor/target/production/wbuild/node-subtensor-runtime/node_subtensor_runtime.compact.compressed.wasm +// --genesis-builder=runtime +// --genesis-builder-preset=benchmark +// --wasm-execution=compiled +// --pallet=pallet_referenda +// --extrinsic=* +// --steps=50 +// --repeat=20 +// --no-storage-info +// --no-min-squares +// --no-median-slopes +// --output=/Users/user/Work/subtensor/pallets/referenda/src/weights.rs +// --template=/Users/user/Work/subtensor/.maintain/frame-weight-template.hbs + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] +#![allow(missing_docs)] +#![allow(dead_code)] + +use frame_support::{traits::Get, weights::{Weight, constants::ParityDbWeight}}; +use core::marker::PhantomData; + +/// Weight functions needed for `pallet_referenda`. +pub trait WeightInfo { + fn submit() -> Weight; + fn kill() -> Weight; + fn advance_referendum() -> Weight; + fn on_tally_updated() -> Weight; +} + +/// Weights for `pallet_referenda` using the Substrate node and recommended hardware. +pub struct SubstrateWeight(PhantomData); +impl WeightInfo for SubstrateWeight { + /// Storage: `MultiCollective::Members` (r:2 w:0) + /// Proof: `MultiCollective::Members` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Referenda::ActiveCount` (r:1 w:1) + /// Proof: `Referenda::ActiveCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ReferendumCount` (r:1 w:1) + /// Proof: `Referenda::ReferendumCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Lookup` (r:1 w:1) + /// Proof: `Scheduler::Lookup` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Agenda` (r:1 w:1) + /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10463), added: 12938, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ReferendumStatusFor` (r:0 w:1) + /// Proof: `Referenda::ReferendumStatusFor` (`max_values`: None, `max_size`: Some(202), added: 2677, mode: `MaxEncodedLen`) + /// Storage: `SignedVoting::TallyOf` (r:0 w:1) + /// Proof: `SignedVoting::TallyOf` (`max_values`: None, `max_size`: None, mode: `Measured`) + fn submit() -> Weight { + // Proof Size summary in bytes: + // Measured: `203` + // Estimated: `13928` + // Minimum execution time: 17_000_000 picoseconds. + Weight::from_parts(17_000_000, 13928) + .saturating_add(T::DbWeight::get().reads(6_u64)) + .saturating_add(T::DbWeight::get().writes(6_u64)) + } + /// Storage: `Referenda::ReferendumStatusFor` (r:1 w:1) + /// Proof: `Referenda::ReferendumStatusFor` (`max_values`: None, `max_size`: Some(202), added: 2677, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Lookup` (r:2 w:1) + /// Proof: `Scheduler::Lookup` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Agenda` (r:1 w:1) + /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10463), added: 12938, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ActiveCount` (r:1 w:1) + /// Proof: `Referenda::ActiveCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `SignedVoting::TallyOf` (r:0 w:1) + /// Proof: `SignedVoting::TallyOf` (`max_values`: None, `max_size`: None, mode: `Measured`) + fn kill() -> Weight { + // Proof Size summary in bytes: + // Measured: `471` + // Estimated: `13928` + // Minimum execution time: 20_000_000 picoseconds. + Weight::from_parts(20_000_000, 13928) + .saturating_add(T::DbWeight::get().reads(5_u64)) + .saturating_add(T::DbWeight::get().writes(5_u64)) + } + /// Storage: `Referenda::ReferendumStatusFor` (r:1 w:2) + /// Proof: `Referenda::ReferendumStatusFor` (`max_values`: None, `max_size`: Some(202), added: 2677, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ReferendumCount` (r:1 w:1) + /// Proof: `Referenda::ReferendumCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Lookup` (r:3 w:3) + /// Proof: `Scheduler::Lookup` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Agenda` (r:3 w:3) + /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10463), added: 12938, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ActiveCount` (r:1 w:1) + /// Proof: `Referenda::ActiveCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `MultiCollective::Members` (r:2 w:0) + /// Proof: `MultiCollective::Members` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `SignedVoting::TallyOf` (r:0 w:2) + /// Proof: `SignedVoting::TallyOf` (`max_values`: None, `max_size`: None, mode: `Measured`) + fn advance_referendum() -> Weight { + // Proof Size summary in bytes: + // Measured: `587` + // Estimated: `39804` + // Minimum execution time: 36_000_000 picoseconds. + Weight::from_parts(37_000_000, 39804) + .saturating_add(T::DbWeight::get().reads(11_u64)) + .saturating_add(T::DbWeight::get().writes(12_u64)) + } + /// Storage: `Referenda::ReferendumStatusFor` (r:1 w:1) + /// Proof: `Referenda::ReferendumStatusFor` (`max_values`: None, `max_size`: Some(202), added: 2677, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Lookup` (r:1 w:1) + /// Proof: `Scheduler::Lookup` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Agenda` (r:2 w:2) + /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10463), added: 12938, mode: `MaxEncodedLen`) + fn on_tally_updated() -> Weight { + // Proof Size summary in bytes: + // Measured: `391` + // Estimated: `26866` + // Minimum execution time: 16_000_000 picoseconds. + Weight::from_parts(17_000_000, 26866) + .saturating_add(T::DbWeight::get().reads(4_u64)) + .saturating_add(T::DbWeight::get().writes(4_u64)) + } +} + +// For backwards compatibility and tests. +impl WeightInfo for () { + /// Storage: `MultiCollective::Members` (r:2 w:0) + /// Proof: `MultiCollective::Members` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `Referenda::ActiveCount` (r:1 w:1) + /// Proof: `Referenda::ActiveCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ReferendumCount` (r:1 w:1) + /// Proof: `Referenda::ReferendumCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Lookup` (r:1 w:1) + /// Proof: `Scheduler::Lookup` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Agenda` (r:1 w:1) + /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10463), added: 12938, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ReferendumStatusFor` (r:0 w:1) + /// Proof: `Referenda::ReferendumStatusFor` (`max_values`: None, `max_size`: Some(202), added: 2677, mode: `MaxEncodedLen`) + /// Storage: `SignedVoting::TallyOf` (r:0 w:1) + /// Proof: `SignedVoting::TallyOf` (`max_values`: None, `max_size`: None, mode: `Measured`) + fn submit() -> Weight { + // Proof Size summary in bytes: + // Measured: `203` + // Estimated: `13928` + // Minimum execution time: 17_000_000 picoseconds. + Weight::from_parts(17_000_000, 13928) + .saturating_add(ParityDbWeight::get().reads(6_u64)) + .saturating_add(ParityDbWeight::get().writes(6_u64)) + } + /// Storage: `Referenda::ReferendumStatusFor` (r:1 w:1) + /// Proof: `Referenda::ReferendumStatusFor` (`max_values`: None, `max_size`: Some(202), added: 2677, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Lookup` (r:2 w:1) + /// Proof: `Scheduler::Lookup` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Agenda` (r:1 w:1) + /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10463), added: 12938, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ActiveCount` (r:1 w:1) + /// Proof: `Referenda::ActiveCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `SignedVoting::TallyOf` (r:0 w:1) + /// Proof: `SignedVoting::TallyOf` (`max_values`: None, `max_size`: None, mode: `Measured`) + fn kill() -> Weight { + // Proof Size summary in bytes: + // Measured: `471` + // Estimated: `13928` + // Minimum execution time: 20_000_000 picoseconds. + Weight::from_parts(20_000_000, 13928) + .saturating_add(ParityDbWeight::get().reads(5_u64)) + .saturating_add(ParityDbWeight::get().writes(5_u64)) + } + /// Storage: `Referenda::ReferendumStatusFor` (r:1 w:2) + /// Proof: `Referenda::ReferendumStatusFor` (`max_values`: None, `max_size`: Some(202), added: 2677, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ReferendumCount` (r:1 w:1) + /// Proof: `Referenda::ReferendumCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Lookup` (r:3 w:3) + /// Proof: `Scheduler::Lookup` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Agenda` (r:3 w:3) + /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10463), added: 12938, mode: `MaxEncodedLen`) + /// Storage: `Referenda::ActiveCount` (r:1 w:1) + /// Proof: `Referenda::ActiveCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) + /// Storage: `MultiCollective::Members` (r:2 w:0) + /// Proof: `MultiCollective::Members` (`max_values`: None, `max_size`: None, mode: `Measured`) + /// Storage: `SignedVoting::TallyOf` (r:0 w:2) + /// Proof: `SignedVoting::TallyOf` (`max_values`: None, `max_size`: None, mode: `Measured`) + fn advance_referendum() -> Weight { + // Proof Size summary in bytes: + // Measured: `587` + // Estimated: `39804` + // Minimum execution time: 36_000_000 picoseconds. + Weight::from_parts(37_000_000, 39804) + .saturating_add(ParityDbWeight::get().reads(11_u64)) + .saturating_add(ParityDbWeight::get().writes(12_u64)) + } + /// Storage: `Referenda::ReferendumStatusFor` (r:1 w:1) + /// Proof: `Referenda::ReferendumStatusFor` (`max_values`: None, `max_size`: Some(202), added: 2677, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Lookup` (r:1 w:1) + /// Proof: `Scheduler::Lookup` (`max_values`: None, `max_size`: Some(48), added: 2523, mode: `MaxEncodedLen`) + /// Storage: `Scheduler::Agenda` (r:2 w:2) + /// Proof: `Scheduler::Agenda` (`max_values`: None, `max_size`: Some(10463), added: 12938, mode: `MaxEncodedLen`) + fn on_tally_updated() -> Weight { + // Proof Size summary in bytes: + // Measured: `391` + // Estimated: `26866` + // Minimum execution time: 16_000_000 picoseconds. + Weight::from_parts(17_000_000, 26866) + .saturating_add(ParityDbWeight::get().reads(4_u64)) + .saturating_add(ParityDbWeight::get().writes(4_u64)) + } +} diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 95a6b807a9..b811a144ba 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -333,7 +333,8 @@ runtime-benchmarks = [ # Smart Tx fees pallet "subtensor-transaction-fee/runtime-benchmarks", "pallet-shield/runtime-benchmarks", - + "pallet-referenda/runtime-benchmarks", + "subtensor-runtime-common/runtime-benchmarks", "subtensor-chain-extensions/runtime-benchmarks" ] diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index c3880b7a3e..6a3d815f08 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1875,6 +1875,41 @@ impl pallet_referenda::Config for Runtime { type BlockNumberProvider = System; type OnPollCreated = SignedVoting; type OnPollCompleted = SignedVoting; + type WeightInfo = pallet_referenda::weights::SubstrateWeight; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = ReferendaBenchmarkHelper; +} + +#[cfg(feature = "runtime-benchmarks")] +pub struct ReferendaBenchmarkHelper; + +#[cfg(feature = "runtime-benchmarks")] +impl pallet_referenda::BenchmarkHelper for ReferendaBenchmarkHelper { + /// Track 0: `triumvirate` (PassOrFail with `Review { track: 1 }`). + fn track_passorfail() -> u8 { + 0 + } + /// Track 1: `review` (Adjustable). + fn track_adjustable() -> u8 { + 1 + } + /// Adds a fresh account to the `Proposers` collective on every call so + /// `submit` finds it in the proposer set. Idempotent failures (already + /// a member) are ignored so multiple benchmarks can call it. + fn proposer() -> AccountId { + let proposer: AccountId = sp_core::crypto::AccountId32::new([1u8; 32]).into(); + let _ = pallet_multi_collective::Pallet::::add_member( + frame_system::RawOrigin::Root.into(), + GovernanceCollectiveId::Proposers, + proposer.clone(), + ); + proposer + } + fn call() -> RuntimeCall { + RuntimeCall::System(frame_system::Call::remark { + remark: alloc::vec![], + }) + } } // Create the runtime by composing the FRAME pallets that were previously configured. @@ -2005,6 +2040,7 @@ mod benches { [pallet_shield, MevShield] [pallet_subtensor_proxy, Proxy] [pallet_subtensor_utility, Utility] + [pallet_referenda, Referenda] ); } diff --git a/scripts/benchmark_all.sh b/scripts/benchmark_all.sh index 6432c3d5a7..405265eb51 100755 --- a/scripts/benchmark_all.sh +++ b/scripts/benchmark_all.sh @@ -16,6 +16,8 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${0}")" && pwd)" ROOT_DIR="$(cd "$SCRIPT_DIR/.." && pwd)" +export PATH="$HOME/.cargo/bin:$PATH" + RUNTIME_WASM="$ROOT_DIR/target/production/wbuild/node-subtensor-runtime/node_subtensor_runtime.compact.compressed.wasm" NODE_BIN="$ROOT_DIR/target/production/node-subtensor" TEMPLATE="$ROOT_DIR/.maintain/frame-weight-template.hbs" @@ -27,8 +29,8 @@ die() { echo "ERROR: $1" >&2; exit 1; } # ── Auto-discover pallets ──────────────────────────────────────────────────── typeset -A PALLET_OUTPUTS -while read -r name path; do - PALLET_OUTPUTS[$name]="$path" +while read -r name out; do + PALLET_OUTPUTS[$name]="$out" done < <("$SCRIPT_DIR/discover_pallets.sh") (( ${#PALLET_OUTPUTS} > 0 )) || die "no benchmarked pallets found" From 8e2a6a379b5d8ad3e1f288b81ba7b00c2d86e0cc Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Wed, 29 Apr 2026 18:27:52 -0300 Subject: [PATCH 03/14] zepter run default --- pallets/multi-collective/Cargo.toml | 12 ++++++++++-- pallets/referenda/Cargo.toml | 26 ++++++++++++++++++-------- pallets/signed-voting/Cargo.toml | 13 +++++++++++-- primitives/crypto/Cargo.toml | 11 ++++++----- runtime/Cargo.toml | 4 +++- 5 files changed, 48 insertions(+), 18 deletions(-) diff --git a/pallets/multi-collective/Cargo.toml b/pallets/multi-collective/Cargo.toml index d34f9bd59d..411bb8eae5 100644 --- a/pallets/multi-collective/Cargo.toml +++ b/pallets/multi-collective/Cargo.toml @@ -36,5 +36,13 @@ std = [ "frame-support/std", "num-traits/std", ] -runtime-benchmarks = [] -try-runtime = [] +runtime-benchmarks = [ + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", + "sp-runtime/runtime-benchmarks" +] +try-runtime = [ + "frame-support/try-runtime", + "frame-system/try-runtime", + "sp-runtime/try-runtime" +] diff --git a/pallets/referenda/Cargo.toml b/pallets/referenda/Cargo.toml index d2753a09ac..17fbe7a7d8 100644 --- a/pallets/referenda/Cargo.toml +++ b/pallets/referenda/Cargo.toml @@ -50,14 +50,24 @@ std = [ "log/std", ] runtime-benchmarks = [ - "frame-benchmarking/runtime-benchmarks", - "frame-support/runtime-benchmarks", - "frame-system/runtime-benchmarks", - "sp-runtime/runtime-benchmarks", - "subtensor-runtime-common/runtime-benchmarks", + "frame-benchmarking/runtime-benchmarks", + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", + "sp-runtime/runtime-benchmarks", + "subtensor-runtime-common/runtime-benchmarks", + "pallet-balances/runtime-benchmarks", + "pallet-multi-collective/runtime-benchmarks", + "pallet-preimage/runtime-benchmarks", + "pallet-scheduler/runtime-benchmarks", + "pallet-signed-voting/runtime-benchmarks" ] try-runtime = [ - "frame-support/try-runtime", - "frame-system/try-runtime", - "sp-runtime/try-runtime", + "frame-support/try-runtime", + "frame-system/try-runtime", + "sp-runtime/try-runtime", + "pallet-balances/try-runtime", + "pallet-multi-collective/try-runtime", + "pallet-preimage/try-runtime", + "pallet-scheduler/try-runtime", + "pallet-signed-voting/try-runtime" ] diff --git a/pallets/signed-voting/Cargo.toml b/pallets/signed-voting/Cargo.toml index ad6074a774..faa32abb2e 100644 --- a/pallets/signed-voting/Cargo.toml +++ b/pallets/signed-voting/Cargo.toml @@ -36,5 +36,14 @@ std = [ "frame-support/std", "subtensor-runtime-common/std", ] -runtime-benchmarks = [] -try-runtime = [] +runtime-benchmarks = [ + "frame-support/runtime-benchmarks", + "frame-system/runtime-benchmarks", + "sp-runtime/runtime-benchmarks", + "subtensor-runtime-common/runtime-benchmarks" +] +try-runtime = [ + "frame-support/try-runtime", + "frame-system/try-runtime", + "sp-runtime/try-runtime" +] diff --git a/primitives/crypto/Cargo.toml b/primitives/crypto/Cargo.toml index 8f12b46850..45c44f8718 100644 --- a/primitives/crypto/Cargo.toml +++ b/primitives/crypto/Cargo.toml @@ -25,11 +25,12 @@ rand = "0.8" [features] default = ["std"] std = [ - "blake2/std", - "codec/std", - "digest/std", - "rand_core?/std", - "scale-info/std", + "blake2/std", + "codec/std", + "digest/std", + "rand_core?/std", + "scale-info/std", + "zeroize?/std" ] # Enables sign() and generate_key_image(). Not needed for on-chain verification. signing = ["rand_core", "zeroize"] diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index b811a144ba..e27eedbedd 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -336,7 +336,9 @@ runtime-benchmarks = [ "pallet-referenda/runtime-benchmarks", "subtensor-runtime-common/runtime-benchmarks", - "subtensor-chain-extensions/runtime-benchmarks" + "subtensor-chain-extensions/runtime-benchmarks", + "pallet-multi-collective/runtime-benchmarks", + "pallet-signed-voting/runtime-benchmarks" ] try-runtime = [ "frame-try-runtime/try-runtime", From fd42f132291b68566067bb05c98778db0b2e41ee Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Thu, 30 Apr 2026 13:38:09 +0300 Subject: [PATCH 04/14] Adjust minimum collective members --- runtime/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 7ce3461d7e..3a1a5a3f4d 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1794,7 +1794,7 @@ impl McCollectivesInfo for SubtensorCollectives { id: GovernanceCollectiveId::Proposers, info: McCollectiveInfo { name: name(b"otf"), - min_members: 0, + min_members: 1, max_members: Some(20), term_duration: None, }, @@ -1803,7 +1803,7 @@ impl McCollectivesInfo for SubtensorCollectives { id: GovernanceCollectiveId::Triumvirate, info: McCollectiveInfo { name: name(b"triumvirate"), - min_members: 0, + min_members: 3, max_members: Some(3), term_duration: None, }, @@ -1812,7 +1812,7 @@ impl McCollectivesInfo for SubtensorCollectives { id: GovernanceCollectiveId::Economic, info: McCollectiveInfo { name: name(b"economic"), - min_members: 0, + min_members: 1, max_members: Some(16), term_duration: Some(GovernanceCollectiveTermDuration::get()), }, @@ -1821,7 +1821,7 @@ impl McCollectivesInfo for SubtensorCollectives { id: GovernanceCollectiveId::Building, info: McCollectiveInfo { name: name(b"building"), - min_members: 0, + min_members: 1, max_members: Some(16), term_duration: Some(GovernanceCollectiveTermDuration::get()), }, From 55e6ef86bebada1ac4d91f8bc6b1380f279adecb Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Thu, 30 Apr 2026 15:56:50 +0300 Subject: [PATCH 05/14] Add scheduler error handling --- pallets/referenda/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pallets/referenda/src/lib.rs b/pallets/referenda/src/lib.rs index 3a2fe091ee..357b6df63f 100644 --- a/pallets/referenda/src/lib.rs +++ b/pallets/referenda/src/lib.rs @@ -543,7 +543,9 @@ impl Pallet { /// call `set_alarm` AFTER this function, since `conclude` cancels /// whatever alarm is currently scheduled. fn conclude(index: ReferendumIndex, status: ReferendumStatusOf, event: Event) { - let _ = T::Scheduler::cancel_named(alarm_name(index)); + if let Err(err) = T::Scheduler::cancel_named(alarm_name(index)) { + Self::report_scheduler_error(index, "cancel_alarm", err); + } ReferendumStatusFor::::insert(index, status); ActiveCount::::mutate(|c| *c = c.saturating_sub(1)); T::PollHooks::on_poll_completed(index); From f06e8487c900487f7cddef02d0b5eedd0c270e95 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Thu, 30 Apr 2026 16:25:26 +0300 Subject: [PATCH 06/14] Adjust reaper alarm logic --- pallets/referenda/src/lib.rs | 28 ++++++++++++++++++++++++---- runtime/src/lib.rs | 2 +- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/pallets/referenda/src/lib.rs b/pallets/referenda/src/lib.rs index 357b6df63f..584e1aea3b 100644 --- a/pallets/referenda/src/lib.rs +++ b/pallets/referenda/src/lib.rs @@ -387,11 +387,13 @@ pub mod pallet { Self::ensure_ongoing(index)?; - // Best-effort cleanup. Either entry may be absent: `PassOrFail` - // has no enactment task before approval, and the alarm may have - // just fired. Failures here are expected and not reported. + // Best-effort cleanup. The task entry may be absent (`PassOrFail` + // has no enactment task before approval); a missing task is + // expected and not reported. let _ = T::Scheduler::cancel_named(task_name(index)); - let _ = T::Scheduler::cancel_named(alarm_name(index)); + if let Err(err) = T::Scheduler::cancel_named(alarm_name(index)) { + Self::report_scheduler_error(index, "cancel_alarm", err); + } let now = T::BlockNumberProvider::current_block_number(); Self::conclude( @@ -499,6 +501,24 @@ impl Pallet { return Ok(()); } + // Reaper position reached but the task is still queued — + // it was postponed by the scheduler under weight pressure. + // Don't run threshold logic here (with no votes, + // `do_adjust_delay` would fall through to `do_fast_track` + // and conclude as `FastTracked` even though no member + // fast-tracked); re-arm and wait for the task to dispatch. + let reaper_at = info + .submitted + .saturating_add(*initial_delay) + .saturating_add(One::one()); + let now = T::BlockNumberProvider::current_block_number(); + if now >= reaper_at { + if let Err(err) = Self::set_alarm(index, now.saturating_add(One::one())) { + Self::report_scheduler_error(index, "set_alarm", err); + } + return Ok(()); + } + if tally.approval >= *fast_track_threshold { Self::do_fast_track(index); } else if tally.rejection >= *cancel_threshold { diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 3a1a5a3f4d..d07e6d0721 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -880,7 +880,7 @@ impl CommitmentsInterface for CommitmentsI { parameter_types! { pub MaximumSchedulerWeight: Weight = Perbill::from_percent(80) * BlockWeights::get().max_block; - pub const MaxScheduledPerBlock: u32 = 50; + pub const MaxScheduledPerBlock: u32 = 70; } /// Used the compare the privilege of an origin inside the scheduler. From cc5b4d87d8a8a4fd71d772c2935b58ba79ffb2a0 Mon Sep 17 00:00:00 2001 From: Shamil Gadelshin Date: Thu, 30 Apr 2026 17:06:39 +0300 Subject: [PATCH 07/14] Remove double vote for duplicated members --- pallets/referenda/src/mock.rs | 27 ++++++++++++++++++--------- runtime/src/lib.rs | 26 +++++++++++++++++++------- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/pallets/referenda/src/mock.rs b/pallets/referenda/src/mock.rs index afbc6994bd..010a321737 100644 --- a/pallets/referenda/src/mock.rs +++ b/pallets/referenda/src/mock.rs @@ -100,15 +100,24 @@ impl subtensor_runtime_common::SetLike for MemberSet { U256, CollectiveId, >>::member_count(*id), - MemberSet::Union(ids) => ids - .iter() - .map(|id| { - as CollectiveInspect< - U256, - CollectiveId, - >>::member_count(*id) - }) - .sum(), + // Mirrors the production `GovernanceMemberSet` impl: members can + // overlap across collectives but a dual member can only vote + // once. Sum-of-`member_count` would inflate `total` and bias + // thresholds upward; dedup so `len()` is the true cardinality. + MemberSet::Union(ids) => { + let mut accounts: Vec = Vec::new(); + for id in ids { + accounts.extend( + as CollectiveInspect< + U256, + CollectiveId, + >>::members_of(*id), + ); + } + accounts.sort(); + accounts.dedup(); + accounts.len() as u32 + } } } } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index d07e6d0721..e2b178ebec 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1736,15 +1736,27 @@ impl SetLike for GovernanceMemberSet { AccountId, GovernanceCollectiveId, >>::member_count(*id), - Self::Union(ids) => ids - .iter() - .map(|id| { - { + let mut accounts: Vec = Vec::new(); + for id in ids { + accounts.extend(>::member_count(*id) - }) - .sum(), + >>::members_of(*id)); + } + accounts.sort(); + accounts.dedup(); + accounts.len() as u32 + } } } } From af5b092b8fcbe55a5884e53b934913e0404ca0e5 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 30 Apr 2026 11:14:13 -0300 Subject: [PATCH 08/14] Fix some clippy issues --- common/src/traits.rs | 6 ++++-- pallets/referenda/src/benchmarking.rs | 3 +-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/common/src/traits.rs b/common/src/traits.rs index 015281960f..e8c2115ac8 100644 --- a/common/src/traits.rs +++ b/common/src/traits.rs @@ -47,8 +47,9 @@ impl OnPollCreated for Tuple { } fn weight() -> Weight { + #[allow(clippy::let_and_return)] let mut weight = Weight::zero(); - for_tuples!( #( weight = weight.saturating_add(Tuple::weight()); )* ); + for_tuples!( #( weight.saturating_accrue(Tuple::weight()); )* ); weight } } @@ -60,8 +61,9 @@ impl OnPollCompleted for Tuple { } fn weight() -> Weight { + #[allow(clippy::let_and_return)] let mut weight = Weight::zero(); - for_tuples!( #( weight = weight.saturating_add(Tuple::weight()); )* ); + for_tuples!( #( weight.saturating_accrue(Tuple::weight()); )* ); weight } } diff --git a/pallets/referenda/src/benchmarking.rs b/pallets/referenda/src/benchmarking.rs index 30b7226809..189864b339 100644 --- a/pallets/referenda/src/benchmarking.rs +++ b/pallets/referenda/src/benchmarking.rs @@ -8,8 +8,7 @@ //! (approve-with-`Review`): the parent fires `OnPollCompleted`, the child //! fires `OnPollCreated`, and two scheduler operations run. Every other //! branch is strictly cheaper, so a single figure soundly bounds them all. - -#![cfg(feature = "runtime-benchmarks")] +#![allow(clippy::unwrap_used, clippy::expect_used)] use super::*; use alloc::boxed::Box; From 7a0a5694258ca97c37404d39d9c12c540ad219c6 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 30 Apr 2026 11:35:40 -0300 Subject: [PATCH 09/14] Fix worst case referenda benchmarks --- pallets/referenda/src/benchmarking.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pallets/referenda/src/benchmarking.rs b/pallets/referenda/src/benchmarking.rs index 189864b339..71e311d7a9 100644 --- a/pallets/referenda/src/benchmarking.rs +++ b/pallets/referenda/src/benchmarking.rs @@ -20,10 +20,13 @@ use sp_runtime::Perbill; mod benches { use super::*; + /// Worst-case `submit`: `Adjustable` track schedules both the + /// enactment task and the reaper alarm. `PassOrFail` only schedules + /// the deadline alarm, so it is strictly cheaper. #[benchmark] fn submit() { let proposer = T::BenchmarkHelper::proposer(); - let track = T::BenchmarkHelper::track_passorfail(); + let track = T::BenchmarkHelper::track_adjustable(); let call = Box::new(T::BenchmarkHelper::call()); #[extrinsic_call] @@ -32,10 +35,13 @@ mod benches { assert_eq!(ActiveCount::::get(), 1); } + /// Worst-case `kill`: `Adjustable` has both an enactment task and an + /// alarm to cancel. `PassOrFail` only has an alarm before approval, so + /// one of the two `cancel_named` calls is a no-op. #[benchmark] fn kill() { let proposer = T::BenchmarkHelper::proposer(); - let track = T::BenchmarkHelper::track_passorfail(); + let track = T::BenchmarkHelper::track_adjustable(); let call = Box::new(T::BenchmarkHelper::call()); let index = ReferendumCount::::get(); Pallet::::submit(RawOrigin::Signed(proposer).into(), track, call) @@ -77,10 +83,9 @@ mod benches { #[extrinsic_call] advance_referendum(RawOrigin::Root, index); - // Either Delegated (Review path) or Approved (Execute fallback). - assert!(!matches!( + assert!(matches!( ReferendumStatusFor::::get(index), - Some(ReferendumStatus::Ongoing(_)) + Some(ReferendumStatus::Delegated(_)) )); } From 544b1882a19d866e246643bbaf6a40d2ac9003c8 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 30 Apr 2026 15:04:46 -0300 Subject: [PATCH 10/14] Make members sorted in multi collectiv --- Cargo.toml | 2 +- pallets/multi-collective/src/lib.rs | 77 +++++++++++++++++---------- pallets/multi-collective/src/tests.rs | 14 ++--- 3 files changed, 57 insertions(+), 36 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 17aac22fca..abc6d50a00 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -122,7 +122,7 @@ toml_edit = "0.22" derive-syn-parse = "0.2" Inflector = "0.11" cfg-expr = "0.15" -itertools = "0.10" +itertools = { version = "0.10", default-features = false } macro_magic = { version = "0.5", default-features = false } frame-support-procedural-tools = { version = "10.0.0", default-features = false } proc-macro-warning = { version = "1", default-features = false } diff --git a/pallets/multi-collective/src/lib.rs b/pallets/multi-collective/src/lib.rs index beeb485b06..2f324d36e0 100644 --- a/pallets/multi-collective/src/lib.rs +++ b/pallets/multi-collective/src/lib.rs @@ -3,7 +3,11 @@ extern crate alloc; use alloc::vec::Vec; -use frame_support::{dispatch::DispatchResult, pallet_prelude::*, traits::EnsureOriginWithArg}; +use frame_support::{ + dispatch::DispatchResult, + pallet_prelude::*, + traits::{ChangeMembers, EnsureOriginWithArg}, +}; use frame_system::pallet_prelude::*; use num_traits::ops::checked::CheckedRem; pub use pallet::*; @@ -58,6 +62,12 @@ pub mod pallet { type MaxMembers: Get; } + /// Members of each collective, kept sorted by `AccountId`. + /// + /// The sorted invariant is maintained by every write path + /// (`add_member`, `remove_member`, `swap_member`, `reset_members`) so + /// that membership lookups can use `binary_search` and the + /// reset-time diff against the previous set is a linear merge. #[pallet::storage] pub(super) type Members = StorageMap< _, @@ -200,12 +210,15 @@ pub mod pallet { let info = T::Collectives::info(collective_id).ok_or(Error::::CollectiveNotFound)?; Members::::try_mutate(collective_id, |members| -> DispatchResult { - ensure!(!members.contains(&who), Error::::AlreadyMember); + let pos = members + .binary_search(&who) + .err() + .ok_or(Error::::AlreadyMember)?; if let Some(max) = info.max_members { ensure!(members.len() < max as usize, Error::::TooManyMembers); } members - .try_push(who.clone()) + .try_insert(pos, who.clone()) .map_err(|_| Error::::TooManyMembers)?; Ok(()) })?; @@ -229,12 +242,14 @@ pub mod pallet { let info = T::Collectives::info(collective_id).ok_or(Error::::CollectiveNotFound)?; Members::::try_mutate(collective_id, |members| -> DispatchResult { - ensure!(members.contains(&who), Error::::NotMember); + let pos = members + .binary_search(&who) + .map_err(|_| Error::::NotMember)?; ensure!( members.len() > info.min_members as usize, Error::::TooFewMembers ); - members.retain(|m| m != &who); + members.remove(pos); Ok(()) })?; @@ -258,12 +273,22 @@ pub mod pallet { T::Collectives::info(collective_id).ok_or(Error::::CollectiveNotFound)?; Members::::try_mutate(collective_id, |members| -> DispatchResult { - let pos = members - .iter() - .position(|m| m == &remove) - .ok_or(Error::::NotMember)?; - ensure!(!members.contains(&add), Error::::AlreadyMember); - *members.get_mut(pos).ok_or(Error::::NotMember)? = add.clone(); + let pos_remove = members + .binary_search(&remove) + .map_err(|_| Error::::NotMember)?; + ensure!( + members.binary_search(&add).is_err(), + Error::::AlreadyMember + ); + members.remove(pos_remove); + // `add` was absent before the removal, so it is still + // absent now; the search must return `Err(idx)`. + let pos_add = members + .binary_search(&add) + .expect_err("add was checked absent above"); + members + .try_insert(pos_add, add.clone()) + .map_err(|_| Error::::TooManyMembers)?; Ok(()) })?; @@ -298,33 +323,29 @@ pub mod pallet { ensure!(members.len() <= max as usize, Error::::TooManyMembers); } - // Check for duplicates - let mut sorted = members.clone(); + // Sort + dedup; the sorted form is what we store, so the + // dedup pass and the storage write share the same buffer. + let len_before = members.len(); + let mut sorted = members; sorted.sort(); sorted.dedup(); - ensure!(sorted.len() == members.len(), Error::::DuplicateAccounts); + ensure!(sorted.len() == len_before, Error::::DuplicateAccounts); let old_members = Members::::get(collective_id); let bounded = - BoundedVec::try_from(members.clone()).map_err(|_| Error::::TooManyMembers)?; + BoundedVec::try_from(sorted.clone()).map_err(|_| Error::::TooManyMembers)?; Members::::insert(collective_id, bounded); - // Compute incoming/outgoing - let incoming: Vec<_> = members - .iter() - .filter(|m| !old_members.contains(m)) - .cloned() - .collect(); - let outgoing: Vec<_> = old_members - .iter() - .filter(|m| !members.contains(m)) - .cloned() - .collect(); + let (incoming, outgoing) = + <() as ChangeMembers>::compute_members_diff_sorted( + &sorted, + &old_members, + ); T::OnMembersChanged::on_members_changed(collective_id, &incoming, &outgoing); Self::deposit_event(Event::MembersReset { collective_id, - members, + members: sorted, }); Ok(()) } @@ -469,7 +490,7 @@ impl CollectiveInspect for Pallet { Members::::get(collective_id).to_vec() } fn is_member(collective_id: T::CollectiveId, who: &T::AccountId) -> bool { - Members::::get(collective_id).contains(who) + Members::::get(collective_id).binary_search(who).is_ok() } fn member_count(collective_id: T::CollectiveId) -> u32 { Members::::get(collective_id).len() as u32 diff --git a/pallets/multi-collective/src/tests.rs b/pallets/multi-collective/src/tests.rs index da97000493..622092d836 100644 --- a/pallets/multi-collective/src/tests.rs +++ b/pallets/multi-collective/src/tests.rs @@ -424,10 +424,10 @@ fn swap_member_happy_path() { dave, )); - // Dave takes bob's slot at index 1 — position preserved. + // Members are kept sorted: dave (4) goes after charlie (3). assert_eq!( MultiCollective::::members_of(CollectiveId::Alpha), - vec![alice, dave, charlie] + vec![alice, charlie, dave] ); assert!(!MultiCollective::::is_member( CollectiveId::Alpha, @@ -450,7 +450,7 @@ fn swap_member_happy_path() { } #[test] -fn swap_member_preserves_position_on_head_and_tail() { +fn swap_member_keeps_sorted_order() { TestState::build_and_execute(|| { let a = U256::from(1); let b = U256::from(2); @@ -466,7 +466,7 @@ fn swap_member_preserves_position_on_head_and_tail() { )); } - // Swap head slot. + // Swap the head member out for an account that sorts to the tail. assert_ok!(MultiCollective::::swap_member( RuntimeOrigin::root(), CollectiveId::Alpha, @@ -475,10 +475,10 @@ fn swap_member_preserves_position_on_head_and_tail() { )); assert_eq!( MultiCollective::::members_of(CollectiveId::Alpha), - vec![x, b, c] + vec![b, c, x] ); - // Swap tail slot. + // Swap the (former) tail member; the new account also sorts to the tail. assert_ok!(MultiCollective::::swap_member( RuntimeOrigin::root(), CollectiveId::Alpha, @@ -487,7 +487,7 @@ fn swap_member_preserves_position_on_head_and_tail() { )); assert_eq!( MultiCollective::::members_of(CollectiveId::Alpha), - vec![x, b, y] + vec![b, x, y] ); }); } From 537928a5e74745eb32d49db9598f970a4b47e22f Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 30 Apr 2026 15:11:28 -0300 Subject: [PATCH 11/14] reset_members to set_members --- pallets/multi-collective/src/lib.rs | 16 +++--- pallets/multi-collective/src/mock.rs | 4 +- pallets/multi-collective/src/tests.rs | 52 +++++++++---------- pallets/referenda/src/mock.rs | 2 +- pallets/signed-voting/src/lib.rs | 2 +- pallets/signed-voting/src/tests.rs | 2 +- .../src/governance/collective_management.rs | 8 +-- runtime/src/lib.rs | 2 +- 8 files changed, 42 insertions(+), 46 deletions(-) diff --git a/pallets/multi-collective/src/lib.rs b/pallets/multi-collective/src/lib.rs index 2f324d36e0..990276a48c 100644 --- a/pallets/multi-collective/src/lib.rs +++ b/pallets/multi-collective/src/lib.rs @@ -45,7 +45,7 @@ pub mod pallet { type SwapOrigin: EnsureOriginWithArg; /// Required origin for resetting the members of a collective. - type ResetOrigin: EnsureOriginWithArg; + type SetOrigin: EnsureOriginWithArg; /// The receiver of the signal for when the members of a collective have changed. type OnMembersChanged: OnMembersChanged; @@ -65,7 +65,7 @@ pub mod pallet { /// Members of each collective, kept sorted by `AccountId`. /// /// The sorted invariant is maintained by every write path - /// (`add_member`, `remove_member`, `swap_member`, `reset_members`) so + /// (`add_member`, `remove_member`, `swap_member`, `set_members`) so /// that membership lookups can use `binary_search` and the /// reset-time diff against the previous set is a linear merge. #[pallet::storage] @@ -93,7 +93,7 @@ pub mod pallet { removed: T::AccountId, added: T::AccountId, }, - MembersReset { + MembersSet { collective_id: T::CollectiveId, members: Vec, }, @@ -147,7 +147,7 @@ pub mod pallet { // Guards against `CollectiveInfo` / `T::MaxMembers` mismatch: a runtime // declaring `max_members` (or `min_members`) greater than // `T::MaxMembers` would pass the per-collective cap check in - // `add_member` / `reset_members` but then fail the `BoundedVec` bound + // `add_member` / `set_members` but then fail the `BoundedVec` bound // with a confusing `TooManyMembers` at the storage ceiling. Failing // construction here makes the inconsistent config unreachable at // runtime. @@ -306,12 +306,12 @@ pub mod pallet { } #[pallet::call_index(3)] - pub fn reset_members( + pub fn set_members( origin: OriginFor, collective_id: T::CollectiveId, members: Vec, ) -> DispatchResult { - T::ResetOrigin::ensure_origin(origin, &collective_id)?; + T::SetOrigin::ensure_origin(origin, &collective_id)?; let info = T::Collectives::info(collective_id).ok_or(Error::::CollectiveNotFound)?; // Validate new member list @@ -343,7 +343,7 @@ pub mod pallet { ); T::OnMembersChanged::on_members_changed(collective_id, &incoming, &outgoing); - Self::deposit_event(Event::MembersReset { + Self::deposit_event(Event::MembersSet { collective_id, members: sorted, }); @@ -360,7 +360,7 @@ pub mod pallet { /// Restricted to collectives whose `CollectiveId::can_rotate()` /// is true. Curated collectives (Triumvirate, Proposers) are /// managed directly via `add_member` / `remove_member` / - /// `swap_member` / `reset_members` and have no rotation hook + /// `swap_member` / `set_members` and have no rotation hook /// — refusing the call here surfaces a misconfigured Root /// extrinsic as `CollectiveDoesNotRotate` instead of silently /// consuming weight. diff --git a/pallets/multi-collective/src/mock.rs b/pallets/multi-collective/src/mock.rs index 3e5441cfd4..ccfe838e03 100644 --- a/pallets/multi-collective/src/mock.rs +++ b/pallets/multi-collective/src/mock.rs @@ -173,7 +173,7 @@ impl CollectivesInfo for TestCollectives { // --- Recording stub for the `OnNewTerm` hook --- // // `OnMembersChanged` observations go through the pallet's `Event` enum -// (MemberAdded / MemberRemoved / MemberSwapped / MembersReset) — see +// (MemberAdded / MemberRemoved / MemberSwapped / MembersSet) — see // `multi_collective_events()` below. `OnNewTerm` has no corresponding event, // so we keep a thread_local log for the rotation tests in Section 6. @@ -228,7 +228,7 @@ impl pallet_multi_collective::Config for Test { type AddOrigin = AsEnsureOriginWithArg>; type RemoveOrigin = AsEnsureOriginWithArg>; type SwapOrigin = AsEnsureOriginWithArg>; - type ResetOrigin = AsEnsureOriginWithArg>; + type SetOrigin = AsEnsureOriginWithArg>; type OnMembersChanged = (); type OnNewTerm = TestOnNewTerm; type MaxMembers = MaxMembers; diff --git a/pallets/multi-collective/src/tests.rs b/pallets/multi-collective/src/tests.rs index 622092d836..a6ce9a9ebb 100644 --- a/pallets/multi-collective/src/tests.rs +++ b/pallets/multi-collective/src/tests.rs @@ -690,10 +690,10 @@ fn swap_member_works_at_max_bound() { }); } -// -------- Section 5: reset_members -------- +// -------- Section 5: set_members -------- #[test] -fn reset_members_replaces_list() { +fn set_members_replaces_list() { TestState::build_and_execute(|| { let a = U256::from(1); let b = U256::from(2); @@ -709,7 +709,7 @@ fn reset_members_replaces_list() { )); } - assert_ok!(MultiCollective::::reset_members( + assert_ok!(MultiCollective::::set_members( RuntimeOrigin::root(), CollectiveId::Alpha, vec![c, d, e], @@ -725,7 +725,7 @@ fn reset_members_replaces_list() { assert_eq!( multi_collective_events().last(), - Some(&CollectiveEvent::MembersReset { + Some(&CollectiveEvent::MembersSet { collective_id: CollectiveId::Alpha, members: vec![c, d, e], }) @@ -734,7 +734,7 @@ fn reset_members_replaces_list() { } #[test] -fn reset_members_handles_overlap() { +fn set_members_handles_overlap() { TestState::build_and_execute(|| { let a = U256::from(1); let b = U256::from(2); @@ -751,7 +751,7 @@ fn reset_members_handles_overlap() { // [b, c, d] overlaps with the old [a, b, c]: b and c stay, a goes out, // d comes in. Final storage reflects the new list verbatim. - assert_ok!(MultiCollective::::reset_members( + assert_ok!(MultiCollective::::set_members( RuntimeOrigin::root(), CollectiveId::Alpha, vec![b, c, d], @@ -764,7 +764,7 @@ fn reset_members_handles_overlap() { assert_eq!( multi_collective_events().last(), - Some(&CollectiveEvent::MembersReset { + Some(&CollectiveEvent::MembersSet { collective_id: CollectiveId::Alpha, members: vec![b, c, d], }) @@ -773,10 +773,10 @@ fn reset_members_handles_overlap() { } #[test] -fn reset_members_requires_origin() { +fn set_members_requires_origin() { TestState::build_and_execute(|| { assert_noop!( - MultiCollective::::reset_members( + MultiCollective::::set_members( RuntimeOrigin::signed(U256::from(999)), CollectiveId::Alpha, vec![U256::from(1)], @@ -790,10 +790,10 @@ fn reset_members_requires_origin() { } #[test] -fn reset_members_fails_for_unknown_collective() { +fn set_members_fails_for_unknown_collective() { TestState::build_and_execute(|| { assert_noop!( - MultiCollective::::reset_members( + MultiCollective::::set_members( RuntimeOrigin::root(), CollectiveId::Unknown, vec![U256::from(1)], @@ -806,11 +806,11 @@ fn reset_members_fails_for_unknown_collective() { } #[test] -fn reset_members_rejects_too_few() { +fn set_members_rejects_too_few() { TestState::build_and_execute(|| { // Beta declares min_members = 2. assert_noop!( - MultiCollective::::reset_members( + MultiCollective::::set_members( RuntimeOrigin::root(), CollectiveId::Beta, vec![U256::from(1)], @@ -824,12 +824,12 @@ fn reset_members_rejects_too_few() { } #[test] -fn reset_members_rejects_too_many_via_info() { +fn set_members_rejects_too_many_via_info() { TestState::build_and_execute(|| { // Beta declares max_members = Some(3); four accounts is one over. let list: Vec = (1..=4u32).map(U256::from).collect(); assert_noop!( - MultiCollective::::reset_members(RuntimeOrigin::root(), CollectiveId::Beta, list,), + MultiCollective::::set_members(RuntimeOrigin::root(), CollectiveId::Beta, list,), Error::::TooManyMembers ); @@ -839,17 +839,13 @@ fn reset_members_rejects_too_many_via_info() { } #[test] -fn reset_members_rejects_too_many_via_storage() { +fn set_members_rejects_too_many_via_storage() { TestState::build_and_execute(|| { // Gamma's info.max_members is None; only T::MaxMembers = 32 applies. // 33 accounts exceed the BoundedVec bound, caught by try_from. let list: Vec = (1..=33u32).map(U256::from).collect(); assert_noop!( - MultiCollective::::reset_members( - RuntimeOrigin::root(), - CollectiveId::Gamma, - list, - ), + MultiCollective::::set_members(RuntimeOrigin::root(), CollectiveId::Gamma, list,), Error::::TooManyMembers ); @@ -858,13 +854,13 @@ fn reset_members_rejects_too_many_via_storage() { } #[test] -fn reset_members_rejects_duplicates() { +fn set_members_rejects_duplicates() { TestState::build_and_execute(|| { let a = U256::from(1); let b = U256::from(2); assert_noop!( - MultiCollective::::reset_members( + MultiCollective::::set_members( RuntimeOrigin::root(), CollectiveId::Alpha, vec![a, b, a], @@ -877,10 +873,10 @@ fn reset_members_rejects_duplicates() { } /// Reset with a list identical to the current membership still emits a -/// `MembersReset` event — the pallet doesn't short-circuit no-op resets. +/// `MembersSet` event — the pallet doesn't short-circuit no-op resets. /// Pinned so downstream consumers know they must tolerate empty-diff calls. #[test] -fn reset_members_noop_still_fires_event() { +fn set_members_noop_still_fires_event() { TestState::build_and_execute(|| { let a = U256::from(1); let b = U256::from(2); @@ -893,7 +889,7 @@ fn reset_members_noop_still_fires_event() { )); } - assert_ok!(MultiCollective::::reset_members( + assert_ok!(MultiCollective::::set_members( RuntimeOrigin::root(), CollectiveId::Alpha, vec![a, b], @@ -906,7 +902,7 @@ fn reset_members_noop_still_fires_event() { assert_eq!( multi_collective_events().last(), - Some(&CollectiveEvent::MembersReset { + Some(&CollectiveEvent::MembersSet { collective_id: CollectiveId::Alpha, members: vec![a, b], }) @@ -1166,7 +1162,7 @@ fn inspect_member_count_matches_mutations() { ); // Reset replaces wholesale — count reflects the new list length. - assert_ok!(MultiCollective::::reset_members( + assert_ok!(MultiCollective::::set_members( RuntimeOrigin::root(), CollectiveId::Alpha, vec![a, b, c, d], diff --git a/pallets/referenda/src/mock.rs b/pallets/referenda/src/mock.rs index e25a416ef3..0a7372b1f8 100644 --- a/pallets/referenda/src/mock.rs +++ b/pallets/referenda/src/mock.rs @@ -336,7 +336,7 @@ impl pallet_multi_collective::Config for Test { type AddOrigin = frame_support::traits::AsEnsureOriginWithArg>; type RemoveOrigin = frame_support::traits::AsEnsureOriginWithArg>; type SwapOrigin = frame_support::traits::AsEnsureOriginWithArg>; - type ResetOrigin = frame_support::traits::AsEnsureOriginWithArg>; + type SetOrigin = frame_support::traits::AsEnsureOriginWithArg>; type OnMembersChanged = VoteCleanup; type OnNewTerm = (); type MaxMembers = MaxMembers; diff --git a/pallets/signed-voting/src/lib.rs b/pallets/signed-voting/src/lib.rs index a4b17d7094..bbf0e2dda9 100644 --- a/pallets/signed-voting/src/lib.rs +++ b/pallets/signed-voting/src/lib.rs @@ -247,7 +247,7 @@ impl Pallet { /// Called when a member is rotated out of a collective. /// /// `total` is intentionally left unchanged: the runtime is expected to - /// replace departing voters via `swap_member` or `reset_members`, which + /// replace departing voters via `swap_member` or `set_members`, which /// preserve voter-set size. The `outgoing`-only iteration in typical /// `OnMembersChanged` wiring (e.g. referenda's `VoteCleanup`) has no /// symmetric counterpart for incoming members, so decrementing `total` diff --git a/pallets/signed-voting/src/tests.rs b/pallets/signed-voting/src/tests.rs index 9e15201662..5a7eecf374 100644 --- a/pallets/signed-voting/src/tests.rs +++ b/pallets/signed-voting/src/tests.rs @@ -638,7 +638,7 @@ fn remove_votes_for_emits_invalidated_event() { } /// `remove_votes_for` preserves `total`: the runtime rotates voters via -/// `swap_member` / `reset_members`, which keep the voter-set size constant +/// `swap_member` / `set_members`, which keep the voter-set size constant /// and fill the slot a departing voter leaves. Decrementing `total` here /// would break the denominator on swap (incoming member present but uncounted). #[test] diff --git a/runtime/src/governance/collective_management.rs b/runtime/src/governance/collective_management.rs index 6323487338..9fd03d532a 100644 --- a/runtime/src/governance/collective_management.rs +++ b/runtime/src/governance/collective_management.rs @@ -132,17 +132,17 @@ impl CollectiveManagement { } /// Push a new membership list into multi-collective storage. - /// Goes through `reset_members` (rather than direct storage writes) + /// Goes through `set_members` (rather than direct storage writes) /// so size validation, the `OnMembersChanged` hook (which routes to /// `SignedVoting::remove_votes_for`), and the canonical - /// `MembersReset` event all fire on every rotation. + /// `MembersSet` event all fire on every rotation. fn apply_rotation( collective_id: GovernanceCollectiveId, members: Vec, query_weight: Weight, ) -> Weight { let len = members.len() as u64; - let result = pallet_multi_collective::Pallet::::reset_members( + let result = pallet_multi_collective::Pallet::::set_members( frame_system::RawOrigin::Root.into(), collective_id, members, @@ -151,7 +151,7 @@ impl CollectiveManagement { if let Err(err) = result { log::error!( target: "runtime::collective-management", - "reset_members failed for {:?}: {:?}", + "set_members failed for {:?}: {:?}", collective_id, err, ); diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 6a3d815f08..aaa80b20ac 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1854,7 +1854,7 @@ impl pallet_multi_collective::Config for Runtime { type AddOrigin = AsEnsureOriginWithArg>; type RemoveOrigin = AsEnsureOriginWithArg>; type SwapOrigin = AsEnsureOriginWithArg>; - type ResetOrigin = AsEnsureOriginWithArg>; + type SetOrigin = AsEnsureOriginWithArg>; type OnMembersChanged = GovernanceVoteCleanup; type OnNewTerm = governance::collective_management::CollectiveManagement; type MaxMembers = MultiCollectiveMaxMembers; From 8c099b126b4a409a1d5c668c0ef495c0d9ba1c3f Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Thu, 30 Apr 2026 17:18:41 -0300 Subject: [PATCH 12/14] Move OnMembersChanged to subtensor common --- Cargo.lock | 1 + common/src/traits.rs | 22 ++++++++++++++++++++++ pallets/multi-collective/Cargo.toml | 5 ++++- pallets/multi-collective/src/lib.rs | 23 +---------------------- pallets/referenda/src/mock.rs | 3 ++- runtime/src/lib.rs | 2 +- 6 files changed, 31 insertions(+), 25 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d509abab55..e7277d9a6e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10094,6 +10094,7 @@ dependencies = [ "sp-core", "sp-io", "sp-runtime", + "subtensor-runtime-common", ] [[package]] diff --git a/common/src/traits.rs b/common/src/traits.rs index e8c2115ac8..c8773deae7 100644 --- a/common/src/traits.rs +++ b/common/src/traits.rs @@ -67,3 +67,25 @@ impl OnPollCompleted for Tuple { weight } } + +/// Handler for when the members of a collective have changed. +pub trait OnMembersChanged { + /// A collective's members have changed, `incoming` members have joined and + /// `outgoing` members have left. + fn on_members_changed( + collective_id: CollectiveId, + incoming: &[AccountId], + outgoing: &[AccountId], + ); +} + +#[impl_trait_for_tuples::impl_for_tuples(10)] +impl OnMembersChanged for Tuple { + fn on_members_changed( + collective_id: CollectiveId, + incoming: &[AccountId], + outgoing: &[AccountId], + ) { + for_tuples!( #( Tuple::on_members_changed(collective_id.clone(), incoming, outgoing); )* ); + } +} diff --git a/pallets/multi-collective/Cargo.toml b/pallets/multi-collective/Cargo.toml index 411bb8eae5..8eb4cd3372 100644 --- a/pallets/multi-collective/Cargo.toml +++ b/pallets/multi-collective/Cargo.toml @@ -21,6 +21,7 @@ frame-system = { workspace = true } frame-support = { workspace = true } impl-trait-for-tuples = { workspace = true } num-traits = { workspace = true } +subtensor-runtime-common = { workspace = true } [dev-dependencies] sp-io = { workspace = true, default-features = true } @@ -35,11 +36,13 @@ std = [ "frame-system/std", "frame-support/std", "num-traits/std", + "subtensor-runtime-common/std", ] runtime-benchmarks = [ "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", - "sp-runtime/runtime-benchmarks" + "sp-runtime/runtime-benchmarks", + "subtensor-runtime-common/runtime-benchmarks", ] try-runtime = [ "frame-support/try-runtime", diff --git a/pallets/multi-collective/src/lib.rs b/pallets/multi-collective/src/lib.rs index 990276a48c..95167e7b63 100644 --- a/pallets/multi-collective/src/lib.rs +++ b/pallets/multi-collective/src/lib.rs @@ -11,6 +11,7 @@ use frame_support::{ use frame_system::pallet_prelude::*; use num_traits::ops::checked::CheckedRem; pub use pallet::*; +pub use subtensor_runtime_common::OnMembersChanged; #[cfg(test)] mod mock; @@ -436,28 +437,6 @@ pub trait CollectivesInfo { } } -/// Handler for when the members of a collective have changed. -pub trait OnMembersChanged { - /// A collective's members have changed, `incoming` members have joined and - /// `outgoing` members have left. - fn on_members_changed( - collective_id: CollectiveId, - incoming: &[AccountId], - outgoing: &[AccountId], - ); -} - -#[impl_trait_for_tuples::impl_for_tuples(10)] -impl OnMembersChanged for Tuple { - fn on_members_changed( - collective_id: CollectiveId, - incoming: &[AccountId], - outgoing: &[AccountId], - ) { - for_tuples!( #( Tuple::on_members_changed(collective_id.clone(), incoming, outgoing); )* ); - } -} - /// Handler for when a new term of a collective has started. pub trait OnNewTerm { /// A new term of a collective has started. diff --git a/pallets/referenda/src/mock.rs b/pallets/referenda/src/mock.rs index 0a7372b1f8..827cbd8d81 100644 --- a/pallets/referenda/src/mock.rs +++ b/pallets/referenda/src/mock.rs @@ -13,8 +13,9 @@ use sp_runtime::{BuildStorage, Perbill, traits::IdentityLookup}; use crate::{self as pallet_referenda, *}; use pallet_multi_collective::{ - self, Collective, CollectiveInfo, CollectiveInspect, CollectivesInfo, OnMembersChanged, + self, Collective, CollectiveInfo, CollectiveInspect, CollectivesInfo, }; +use subtensor_runtime_common::OnMembersChanged; type Block = frame_system::mocking::MockBlock; diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index aaa80b20ac..aad2c165dc 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1647,8 +1647,8 @@ use frame_support::traits::AsEnsureOriginWithArg; use pallet_multi_collective::{ Collective as McCollective, CollectiveInfo as McCollectiveInfo, CollectiveInspect as McCollectiveInspect, CollectivesInfo as McCollectivesInfo, - OnMembersChanged as McOnMembersChanged, }; +use subtensor_runtime_common::OnMembersChanged as McOnMembersChanged; /// Identifier of a collective managed by `pallet-multi-collective`. #[derive( Copy, From 82384e6bb35a3da24446b347e5d7d82dca8a3a11 Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Fri, 1 May 2026 12:36:12 -0300 Subject: [PATCH 13/14] Benchmarks for the multi collective pallet --- Cargo.lock | 1 + common/src/traits.rs | 11 ++ pallets/multi-collective/Cargo.toml | 3 + pallets/multi-collective/src/benchmarking.rs | 126 ++++++++++++ pallets/multi-collective/src/lib.rs | 182 ++++++++++++------ pallets/multi-collective/src/mock.rs | 39 +++- pallets/multi-collective/src/tests.rs | 8 +- pallets/multi-collective/src/weights.rs | 43 +++++ pallets/referenda/src/mock.rs | 21 ++ pallets/signed-voting/src/lib.rs | 2 +- .../src/governance/collective_management.rs | 27 +++ runtime/src/lib.rs | 41 ++++ 12 files changed, 437 insertions(+), 67 deletions(-) create mode 100644 pallets/multi-collective/src/benchmarking.rs create mode 100644 pallets/multi-collective/src/weights.rs diff --git a/Cargo.lock b/Cargo.lock index e7277d9a6e..8e2859a6b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10085,6 +10085,7 @@ dependencies = [ name = "pallet-multi-collective" version = "1.0.0" dependencies = [ + "frame-benchmarking", "frame-support", "frame-system", "impl-trait-for-tuples", diff --git a/common/src/traits.rs b/common/src/traits.rs index c8773deae7..c2d84b460b 100644 --- a/common/src/traits.rs +++ b/common/src/traits.rs @@ -77,6 +77,10 @@ pub trait OnMembersChanged { incoming: &[AccountId], outgoing: &[AccountId], ); + /// Worst-case upper bound on `on_members_changed`'s weight. The + /// implementation is responsible for bounding its own iteration over + /// `incoming`/`outgoing` against the relevant `MaxMembers` constant. + fn weight() -> Weight; } #[impl_trait_for_tuples::impl_for_tuples(10)] @@ -88,4 +92,11 @@ impl OnMembersChanged f ) { for_tuples!( #( Tuple::on_members_changed(collective_id.clone(), incoming, outgoing); )* ); } + + fn weight() -> Weight { + #[allow(clippy::let_and_return)] + let mut weight = Weight::zero(); + for_tuples!( #( weight.saturating_accrue(Tuple::weight()); )* ); + weight + } } diff --git a/pallets/multi-collective/Cargo.toml b/pallets/multi-collective/Cargo.toml index 8eb4cd3372..116a6cba5a 100644 --- a/pallets/multi-collective/Cargo.toml +++ b/pallets/multi-collective/Cargo.toml @@ -17,6 +17,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { workspace = true, features = ["max-encoded-len"] } scale-info = { workspace = true, features = ["derive"] } +frame-benchmarking = { workspace = true, optional = true } frame-system = { workspace = true } frame-support = { workspace = true } impl-trait-for-tuples = { workspace = true } @@ -33,12 +34,14 @@ default = ["std"] std = [ "codec/std", "scale-info/std", + "frame-benchmarking?/std", "frame-system/std", "frame-support/std", "num-traits/std", "subtensor-runtime-common/std", ] runtime-benchmarks = [ + "frame-benchmarking/runtime-benchmarks", "frame-support/runtime-benchmarks", "frame-system/runtime-benchmarks", "sp-runtime/runtime-benchmarks", diff --git a/pallets/multi-collective/src/benchmarking.rs b/pallets/multi-collective/src/benchmarking.rs new file mode 100644 index 0000000000..60d99a4aae --- /dev/null +++ b/pallets/multi-collective/src/benchmarking.rs @@ -0,0 +1,126 @@ +//! Benchmarks for `pallet-multi-collective`. +//! +//! Setup is parameterised through [`Config::BenchmarkHelper`]: the runtime +//! supplies a non-rotatable collective whose bounds allow the pallet to +//! fill and drain it freely, plus a separate rotatable collective for +//! `force_rotate`. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use super::*; +use frame_benchmarking::v2::*; +use frame_system::RawOrigin; + +/// Stable seed for `frame_benchmarking::account` so accounts generated +/// across benchmark setup steps round-trip the same value. +const SEED: u32 = 0; + +/// Pre-fill a collective's `Members` storage with `count` distinct +/// accounts, returning them sorted by `AccountId` (the canonical storage +/// order). +fn fill_members(collective_id: T::CollectiveId, count: u32) -> Vec { + let mut members: Vec = (0..count) + .map(|i| account::("member", i, SEED)) + .collect(); + members.sort(); + + // Bypass `add_member` to avoid paying the per-call binary_search cost + // during setup: we know the list is sorted and unique, so we can + // write the storage directly. + let bounded = BoundedVec::try_from(members.clone()) + .expect("benchmark fill must respect MaxMembers"); + Members::::insert(collective_id, bounded); + members +} + +#[benchmarks] +mod benches { + use super::*; + + /// Worst case: pre-fill to `MaxMembers - 1` so the binary_search + /// runs at full depth. The new account's insert position depends on + /// its `AccountId` hash — uniformly distributed but deterministic + /// across benchmark runs, and the per-element shift cost is + /// constant-bounded by `MaxMembers × sizeof::`. + #[benchmark] + fn add_member() { + let collective = T::BenchmarkHelper::collective(); + let max = T::MaxMembers::get(); + let _existing = fill_members::(collective, max.saturating_sub(1)); + let new_member = account::("new", 0, SEED); + + #[extrinsic_call] + add_member(RawOrigin::Root, collective, new_member); + + assert_eq!(Members::::get(collective).len(), max as usize); + } + + /// Worst case: full collective; binary_search at max depth, remove + /// shifts the maximum number of trailing elements. + #[benchmark] + fn remove_member() { + let collective = T::BenchmarkHelper::collective(); + let max = T::MaxMembers::get(); + let members = fill_members::(collective, max); + // Remove the head: `remove(0)` shifts every other element. + let to_remove = members[0].clone(); + + #[extrinsic_call] + remove_member(RawOrigin::Root, collective, to_remove); + + assert_eq!( + Members::::get(collective).len(), + (max as usize).saturating_sub(1), + ); + } + + /// Worst case: full collective; two binary_searches at max depth, + /// then a remove + insert each shifting the maximum trailing slice. + #[benchmark] + fn swap_member() { + let collective = T::BenchmarkHelper::collective(); + let max = T::MaxMembers::get(); + let members = fill_members::(collective, max); + let to_remove = members[0].clone(); + // A fresh account, distinct from the existing set. + let to_add = account::("new", 0, SEED); + + #[extrinsic_call] + swap_member(RawOrigin::Root, collective, to_remove, to_add); + + assert_eq!(Members::::get(collective).len(), max as usize); + } + + /// Worst case: replace a fully-populated collective with a + /// completely disjoint set of `MaxMembers` new accounts. Sort, dedup, + /// and the linear merge all run at maximum length. + #[benchmark] + fn set_members() { + let collective = T::BenchmarkHelper::collective(); + let max = T::MaxMembers::get(); + let _existing = fill_members::(collective, max); + + let new_members: Vec = (0..max) + .map(|i| account::("new", i, SEED)) + .collect(); + + #[extrinsic_call] + set_members(RawOrigin::Root, collective, new_members.clone()); + + assert_eq!(Members::::get(collective).len(), max as usize); + } + + /// `force_rotate` itself does only validation + a hook dispatch; + /// this benchmark measures just the extrinsic-side overhead. The + /// hook's worst-case cost is added separately via + /// `T::OnNewTerm::weight()` in the `#[pallet::weight(...)]` + /// annotation. + #[benchmark] + fn force_rotate() { + let collective = T::BenchmarkHelper::rotatable_collective(); + + #[extrinsic_call] + force_rotate(RawOrigin::Root, collective); + } + + impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test); +} diff --git a/pallets/multi-collective/src/lib.rs b/pallets/multi-collective/src/lib.rs index 95167e7b63..f41627235c 100644 --- a/pallets/multi-collective/src/lib.rs +++ b/pallets/multi-collective/src/lib.rs @@ -13,10 +13,14 @@ use num_traits::ops::checked::CheckedRem; pub use pallet::*; pub use subtensor_runtime_common::OnMembersChanged; +#[cfg(feature = "runtime-benchmarks")] +mod benchmarking; #[cfg(test)] mod mock; #[cfg(test)] mod tests; +pub mod weights; +pub use weights::WeightInfo; pub const MAX_COLLECTIVE_NAME_LEN: usize = 32; type CollectiveName = [u8; MAX_COLLECTIVE_NAME_LEN]; @@ -45,7 +49,7 @@ pub mod pallet { /// Required origin for swapping a member in a collective. type SwapOrigin: EnsureOriginWithArg; - /// Required origin for resetting the members of a collective. + /// Required origin for setting the full member list of a collective. type SetOrigin: EnsureOriginWithArg; /// The receiver of the signal for when the members of a collective have changed. @@ -61,14 +65,35 @@ pub mod pallet { /// This is enforced in the code; the membership size can not exceed this limit. #[pallet::constant] type MaxMembers: Get; + + /// Weight information for extrinsics in this pallet. + type WeightInfo: WeightInfo; + + /// Helper for setting up cross-pallet state needed by benchmarks. + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper: BenchmarkHelper; + } + + /// Benchmark setup helper. The runtime supplies a non-rotatable + /// collective for member-management benchmarks and a rotatable one for + /// `force_rotate`. + #[cfg(feature = "runtime-benchmarks")] + pub trait BenchmarkHelper { + /// A collective whose `info.max_members` allows reaching `MaxMembers` + /// and whose `info.min_members == 0`, so member-management + /// benchmarks can fill and drain freely. + fn collective() -> CollectiveId; + /// A collective whose `CollectiveId::can_rotate()` is `true`, + /// for the `force_rotate` benchmark. + fn rotatable_collective() -> CollectiveId; } /// Members of each collective, kept sorted by `AccountId`. /// /// The sorted invariant is maintained by every write path /// (`add_member`, `remove_member`, `swap_member`, `set_members`) so - /// that membership lookups can use `binary_search` and the - /// reset-time diff against the previous set is a linear merge. + /// that membership lookups can use `binary_search` and `set_members` + /// can diff against the previous set with a linear merge. #[pallet::storage] pub(super) type Members = StorageMap< _, @@ -145,63 +170,16 @@ pub mod pallet { } fn integrity_test() { - // Guards against `CollectiveInfo` / `T::MaxMembers` mismatch: a runtime - // declaring `max_members` (or `min_members`) greater than - // `T::MaxMembers` would pass the per-collective cap check in - // `add_member` / `set_members` but then fail the `BoundedVec` bound - // with a confusing `TooManyMembers` at the storage ceiling. Failing - // construction here makes the inconsistent config unreachable at - // runtime. - // - // Alternative structural fix (not taken): drop `max_members` from - // `CollectiveInfo` and expose it via a per-collective method on - // `CollectivesInfo` computed against `T::MaxMembers` (e.g. - // `fn max_members_of(id) -> u32`). That eliminates the field mismatch - // by construction at the cost of a `CollectivesInfo` trait-shape change. - let storage_max = T::MaxMembers::get(); - for collective in T::Collectives::collectives() { - let info = collective.info; - - assert!( - info.min_members <= storage_max, - "CollectiveInfo::min_members ({}) exceeds T::MaxMembers ({}) — collective cannot reach its min", - info.min_members, - storage_max, - ); - - if let Some(max) = info.max_members { - assert!( - max <= storage_max, - "CollectiveInfo::max_members ({}) exceeds T::MaxMembers ({}) — storage cannot hold this many", - max, - storage_max, - ); - assert!( - info.min_members <= max, - "CollectiveInfo::min_members ({}) exceeds max_members ({}) — collective is unreachable", - info.min_members, - max, - ); - } - - // `Some(0)` for term_duration is indistinguishable from "rotate - // every block" at the type level, but the `n % td` check in - // `on_initialize` short-circuits via `checked_rem` and never - // fires. Reject it here rather than let a misconfigured runtime - // silently disable rotations. Use `None` to opt out. - if let Some(td) = info.term_duration { - assert!( - !td.is_zero(), - "CollectiveInfo::term_duration = Some(0) silently disables rotations; use None to opt out", - ); - } - } + Pallet::::check_integrity(); } } #[pallet::call] impl Pallet { #[pallet::call_index(0)] + #[pallet::weight( + T::WeightInfo::add_member().saturating_add(T::OnMembersChanged::weight()) + )] pub fn add_member( origin: OriginFor, collective_id: T::CollectiveId, @@ -234,6 +212,9 @@ pub mod pallet { } #[pallet::call_index(1)] + #[pallet::weight( + T::WeightInfo::remove_member().saturating_add(T::OnMembersChanged::weight()) + )] pub fn remove_member( origin: OriginFor, collective_id: T::CollectiveId, @@ -264,6 +245,9 @@ pub mod pallet { } #[pallet::call_index(2)] + #[pallet::weight( + T::WeightInfo::swap_member().saturating_add(T::OnMembersChanged::weight()) + )] pub fn swap_member( origin: OriginFor, collective_id: T::CollectiveId, @@ -307,6 +291,9 @@ pub mod pallet { } #[pallet::call_index(3)] + #[pallet::weight( + T::WeightInfo::set_members().saturating_add(T::OnMembersChanged::weight()) + )] pub fn set_members( origin: OriginFor, collective_id: T::CollectiveId, @@ -368,10 +355,13 @@ pub mod pallet { /// /// Origin: Root. #[pallet::call_index(4)] + #[pallet::weight( + T::WeightInfo::force_rotate().saturating_add(T::OnNewTerm::weight()) + )] pub fn force_rotate( origin: OriginFor, collective_id: T::CollectiveId, - ) -> DispatchResultWithPostInfo { + ) -> DispatchResult { ensure_root(origin)?; ensure!( collective_id.can_rotate(), @@ -381,8 +371,72 @@ pub mod pallet { // id still surfaces `CollectiveNotFound` if it was meant to // be rotatable. T::Collectives::info(collective_id).ok_or(Error::::CollectiveNotFound)?; - let weight = T::OnNewTerm::on_new_term(collective_id); - Ok(Some(weight).into()) + // The hook returns `Weight` so `on_initialize` can accumulate + // actual block weight; `force_rotate` is Root-only and just + // pays the worst-case bound, no refund. + let _ = T::OnNewTerm::on_new_term(collective_id); + Ok(()) + } + } +} + +impl Pallet { + /// Validates the `CollectivesInfo` configuration against the + /// pallet's storage cap. Called from the `integrity_test` hook + /// at construction; extracted so tests can drive it directly. + /// + /// Guards against `CollectiveInfo` / `T::MaxMembers` mismatch: a + /// runtime declaring `max_members` (or `min_members`) greater + /// than `T::MaxMembers` would pass the per-collective cap check + /// in `add_member` / `set_members` but then fail the `BoundedVec` + /// bound with a confusing `TooManyMembers` at the storage + /// ceiling. Failing construction here makes the inconsistent + /// config unreachable at runtime. + /// + /// Alternative structural fix (not taken): drop `max_members` + /// from `CollectiveInfo` and expose it via a per-collective + /// method on `CollectivesInfo` computed against `T::MaxMembers` + /// (e.g. `fn max_members_of(id) -> u32`). That eliminates the + /// field mismatch by construction at the cost of a + /// `CollectivesInfo` trait-shape change. + pub fn check_integrity() { + let storage_max = T::MaxMembers::get(); + for collective in T::Collectives::collectives() { + let info = collective.info; + + assert!( + info.min_members <= storage_max, + "CollectiveInfo::min_members ({}) exceeds T::MaxMembers ({}) — collective cannot reach its min", + info.min_members, + storage_max, + ); + + if let Some(max) = info.max_members { + assert!( + max <= storage_max, + "CollectiveInfo::max_members ({}) exceeds T::MaxMembers ({}) — storage cannot hold this many", + max, + storage_max, + ); + assert!( + info.min_members <= max, + "CollectiveInfo::min_members ({}) exceeds max_members ({}) — collective is unreachable", + info.min_members, + max, + ); + } + + // `Some(0)` for term_duration is indistinguishable from "rotate + // every block" at the type level, but the `n % td` check in + // `on_initialize` short-circuits via `checked_rem` and never + // fires. Reject it here rather than let a misconfigured runtime + // silently disable rotations. Use `None` to opt out. + if let Some(td) = info.term_duration { + assert!( + !td.is_zero(), + "CollectiveInfo::term_duration = Some(0) silently disables rotations; use None to opt out", + ); + } } } } @@ -439,8 +493,13 @@ pub trait CollectivesInfo { /// Handler for when a new term of a collective has started. pub trait OnNewTerm { - /// A new term of a collective has started. + /// A new term of a collective has started. Returns the actual weight + /// consumed so `on_initialize` can accumulate per-block hook weight + /// across all rotating collectives. fn on_new_term(collective_id: CollectiveId) -> Weight; + /// Worst-case upper bound on `on_new_term`'s weight, used to + /// pre-charge `force_rotate`. + fn weight() -> Weight; } #[impl_trait_for_tuples::impl_for_tuples(10)] @@ -452,6 +511,13 @@ impl OnNewTerm for Tuple { for_tuples!( #( weight = weight.saturating_add(Tuple::on_new_term(collective_id.clone())); )* ); weight } + + fn weight() -> Weight { + #[allow(clippy::let_and_return)] + let mut weight = Weight::zero(); + for_tuples!( #( weight.saturating_accrue(Tuple::weight()); )* ); + weight + } } /// Trait for inspecting a collective. diff --git a/pallets/multi-collective/src/mock.rs b/pallets/multi-collective/src/mock.rs index ccfe838e03..c762aa78cb 100644 --- a/pallets/multi-collective/src/mock.rs +++ b/pallets/multi-collective/src/mock.rs @@ -188,6 +188,10 @@ impl OnNewTerm for TestOnNewTerm { NEW_TERM_LOG.with(|log| log.borrow_mut().push(id)); Weight::zero() } + + fn weight() -> Weight { + Weight::zero() + } } /// Drain and return the recorded `OnNewTerm` calls since the last drain. @@ -232,18 +236,45 @@ impl pallet_multi_collective::Config for Test { type OnMembersChanged = (); type OnNewTerm = TestOnNewTerm; type MaxMembers = MaxMembers; + type WeightInfo = (); + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = TestBenchmarkHelper; +} + +#[cfg(feature = "runtime-benchmarks")] +pub struct TestBenchmarkHelper; + +#[cfg(feature = "runtime-benchmarks")] +impl pallet_multi_collective::BenchmarkHelper for TestBenchmarkHelper { + fn collective() -> CollectiveId { + // Gamma: max_members = None, min_members = 0 → can fill to MaxMembers + // and drain to empty without tripping the per-collective bounds. + CollectiveId::Gamma + } + + fn rotatable_collective() -> CollectiveId { + // Beta has term_duration = Some(100); see `CollectiveId::can_rotate`. + CollectiveId::Beta + } } // --- Test externality builder --- +/// Build a fresh `TestExternalities` for the mock runtime. Used directly +/// by `impl_benchmark_test_suite!`; `TestState::build_and_execute` wraps +/// this with the per-test bootstrap unit tests rely on. +pub fn new_test_ext() -> sp_io::TestExternalities { + RuntimeGenesisConfig::default() + .build_storage() + .unwrap() + .into() +} + pub struct TestState; impl TestState { pub fn build_and_execute(test: impl FnOnce()) { - let mut ext: sp_io::TestExternalities = RuntimeGenesisConfig::default() - .build_storage() - .unwrap() - .into(); + let mut ext = new_test_ext(); ext.execute_with(|| { // System::events() only records events from block >= 1, so diff --git a/pallets/multi-collective/src/tests.rs b/pallets/multi-collective/src/tests.rs index a6ce9a9ebb..15c411afb3 100644 --- a/pallets/multi-collective/src/tests.rs +++ b/pallets/multi-collective/src/tests.rs @@ -211,7 +211,7 @@ fn add_member_respects_storage_max_when_info_max_none() { 32 ); - // 33rd add fails via `try_push` (BoundedVec bound) rather than the info cap. + // 33rd add fails via `try_insert` (BoundedVec bound) rather than the info cap. assert_noop!( MultiCollective::::add_member( RuntimeOrigin::root(), @@ -872,8 +872,8 @@ fn set_members_rejects_duplicates() { }); } -/// Reset with a list identical to the current membership still emits a -/// `MembersSet` event — the pallet doesn't short-circuit no-op resets. +/// Setting a list identical to the current membership still emits a +/// `MembersSet` event — the pallet doesn't short-circuit no-op sets. /// Pinned so downstream consumers know they must tolerate empty-diff calls. #[test] fn set_members_noop_still_fires_event() { @@ -1161,7 +1161,7 @@ fn inspect_member_count_matches_mutations() { 1 ); - // Reset replaces wholesale — count reflects the new list length. + // `set_members` replaces wholesale — count reflects the new list length. assert_ok!(MultiCollective::::set_members( RuntimeOrigin::root(), CollectiveId::Alpha, diff --git a/pallets/multi-collective/src/weights.rs b/pallets/multi-collective/src/weights.rs new file mode 100644 index 0000000000..5500a2f7a5 --- /dev/null +++ b/pallets/multi-collective/src/weights.rs @@ -0,0 +1,43 @@ +//! Weights for `pallet-multi-collective`. +//! +//! Replace `SubstrateWeight`'s body with the autogenerated output once the +//! benchmarks are run via `frame-omni-bencher`. + +#![cfg_attr(rustfmt, rustfmt_skip)] +#![allow(unused_parens)] +#![allow(unused_imports)] +#![allow(missing_docs)] +#![allow(dead_code)] + +use frame_support::weights::Weight; +use core::marker::PhantomData; + +/// Weight functions needed for `pallet-multi-collective`. Each method +/// returns the worst-case weight at `MaxMembers`; the per-extrinsic CPU +/// cost varies linearly with the actual member count, but the storage +/// reads/writes don't, so we don't parameterise or refund. +pub trait WeightInfo { + fn add_member() -> Weight; + fn remove_member() -> Weight; + fn swap_member() -> Weight; + fn set_members() -> Weight; + fn force_rotate() -> Weight; +} + +/// Placeholder zero weights — overwritten by the benchmark output. +pub struct SubstrateWeight(PhantomData); +impl WeightInfo for SubstrateWeight { + fn add_member() -> Weight { Weight::zero() } + fn remove_member() -> Weight { Weight::zero() } + fn swap_member() -> Weight { Weight::zero() } + fn set_members() -> Weight { Weight::zero() } + fn force_rotate() -> Weight { Weight::zero() } +} + +impl WeightInfo for () { + fn add_member() -> Weight { Weight::zero() } + fn remove_member() -> Weight { Weight::zero() } + fn swap_member() -> Weight { Weight::zero() } + fn set_members() -> Weight { Weight::zero() } + fn force_rotate() -> Weight { Weight::zero() } +} diff --git a/pallets/referenda/src/mock.rs b/pallets/referenda/src/mock.rs index 827cbd8d81..ed66dfada2 100644 --- a/pallets/referenda/src/mock.rs +++ b/pallets/referenda/src/mock.rs @@ -325,6 +325,11 @@ impl OnMembersChanged for VoteCleanup { SignedVoting::remove_votes_for(who); } } + + fn weight() -> Weight { + // Test mock: weights aren't billed in unit tests, return zero. + Weight::zero() + } } parameter_types! { @@ -341,6 +346,22 @@ impl pallet_multi_collective::Config for Test { type OnMembersChanged = VoteCleanup; type OnNewTerm = (); type MaxMembers = MaxMembers; + type WeightInfo = (); + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = ReferendaMockMcBenchmarkHelper; +} + +#[cfg(feature = "runtime-benchmarks")] +pub struct ReferendaMockMcBenchmarkHelper; + +#[cfg(feature = "runtime-benchmarks")] +impl pallet_multi_collective::BenchmarkHelper for ReferendaMockMcBenchmarkHelper { + fn collective() -> CollectiveId { + CollectiveId::Alpha + } + fn rotatable_collective() -> CollectiveId { + CollectiveId::Alpha + } } parameter_types! { diff --git a/pallets/signed-voting/src/lib.rs b/pallets/signed-voting/src/lib.rs index bbf0e2dda9..d181948294 100644 --- a/pallets/signed-voting/src/lib.rs +++ b/pallets/signed-voting/src/lib.rs @@ -252,7 +252,7 @@ impl Pallet { /// `OnMembersChanged` wiring (e.g. referenda's `VoteCleanup`) has no /// symmetric counterpart for incoming members, so decrementing `total` /// here would make the denominator diverge from the actual voter-set - /// size on swap or reset. Pure `remove_member` of a voter in an active + /// size on swap or set. Pure `remove_member` of a voter in an active /// poll is therefore a known operational limitation — leaves `total` /// stale (denominator too high, conservative for thresholds). pub fn remove_votes_for(who: &T::AccountId) { diff --git a/runtime/src/governance/collective_management.rs b/runtime/src/governance/collective_management.rs index 9fd03d532a..7c84bc38bd 100644 --- a/runtime/src/governance/collective_management.rs +++ b/runtime/src/governance/collective_management.rs @@ -26,6 +26,33 @@ use crate::{ pub struct CollectiveManagement; impl pallet_multi_collective::OnNewTerm for CollectiveManagement { + fn weight() -> Weight { + // Worst-case bound used to pre-charge `force_rotate`. + // `on_initialize` separately accumulates the *actual* weight + // returned by `on_new_term`, so this bound is only consulted + // at extrinsic dispatch. + // + // The dominant cost is the ranking pass (`top_validators` or + // `top_subnet_owners`) which iterates an unbounded storage map + // and, today, charges 8 reads per staking hotkey or 3 per + // subnet. We size the bound generously: 5_000 iterations × 8 + // reads, plus the `apply_rotation` storage cost (1 read + 1 + // write for the membership update, plus per-outgoing-member + // cleanup work counted separately by `OnMembersChanged::weight`). + // + // TODO(weights): tighten once `StakingHotkeys` has an explicit + // size bound or once the ranking helpers move to a bounded + // iterator. + const RANKING_ITERATIONS_BOUND: u64 = 5_000; + const READS_PER_ITERATION: u64 = 8; + let db = ::DbWeight::get(); + let ranking = db.reads( + RANKING_ITERATIONS_BOUND.saturating_mul(READS_PER_ITERATION), + ); + let apply = db.reads_writes(1, 1); + ranking.saturating_add(apply) + } + fn on_new_term(collective_id: GovernanceCollectiveId) -> Weight { // Gate via the inherent `GovernanceCollectiveId::can_rotate()`. // The pallet is policy-agnostic — `force_rotate` will route any diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index aad2c165dc..8b787e86a3 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1846,6 +1846,25 @@ impl McOnMembersChanged for GovernanceVoteCle SignedVoting::remove_votes_for(who); } } + + fn weight() -> Weight { + // Worst-case `remove_votes_for` for every outgoing member. For + // each, the implementation iterates every entry in `TallyOf` + // (bounded by `ReferendaMaxQueued`) and, for each poll where the + // voter is recorded, takes the vote and updates the tally — + // which in turn calls `Polls::on_tally_updated`. + let outgoing_max = MultiCollectiveMaxMembers::get() as u64; + let polls_max = ReferendaMaxQueued::get() as u64; + let db = ::DbWeight::get(); + // Per-poll: VotingFor::take + TallyOf::get + TallyOf::insert + // (= 2 reads + 2 writes), plus the cost of `on_tally_updated`. + let per_poll = db + .reads_writes(2, 2) + .saturating_add( + >::on_tally_updated_weight(), + ); + per_poll.saturating_mul(outgoing_max.saturating_mul(polls_max)) + } } impl pallet_multi_collective::Config for Runtime { @@ -1858,6 +1877,28 @@ impl pallet_multi_collective::Config for Runtime { type OnMembersChanged = GovernanceVoteCleanup; type OnNewTerm = governance::collective_management::CollectiveManagement; type MaxMembers = MultiCollectiveMaxMembers; + type WeightInfo = pallet_multi_collective::weights::SubstrateWeight; + #[cfg(feature = "runtime-benchmarks")] + type BenchmarkHelper = MultiCollectiveBenchmarkHelper; +} + +#[cfg(feature = "runtime-benchmarks")] +pub struct MultiCollectiveBenchmarkHelper; + +#[cfg(feature = "runtime-benchmarks")] +impl pallet_multi_collective::BenchmarkHelper + for MultiCollectiveBenchmarkHelper +{ + fn collective() -> GovernanceCollectiveId { + // Proposers: max_members = MultiCollectiveMaxMembers, min_members = 0, + // and not rotatable — so the pallet's member-management benchmarks + // can fill and drain freely. + GovernanceCollectiveId::Proposers + } + + fn rotatable_collective() -> GovernanceCollectiveId { + GovernanceCollectiveId::Economic + } } impl pallet_signed_voting::Config for Runtime { From ea43f56987af90999193f31809f72d0538149ecf Mon Sep 17 00:00:00 2001 From: Loris Moulin Date: Mon, 4 May 2026 10:08:34 -0300 Subject: [PATCH 14/14] cargo fmt --- pallets/multi-collective/src/benchmarking.rs | 4 ++-- runtime/src/governance/collective_management.rs | 4 +--- runtime/src/lib.rs | 10 +++++----- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/pallets/multi-collective/src/benchmarking.rs b/pallets/multi-collective/src/benchmarking.rs index 60d99a4aae..b97a90d12a 100644 --- a/pallets/multi-collective/src/benchmarking.rs +++ b/pallets/multi-collective/src/benchmarking.rs @@ -26,8 +26,8 @@ fn fill_members(collective_id: T::CollectiveId, count: u32) -> Vec::insert(collective_id, bounded); members } diff --git a/runtime/src/governance/collective_management.rs b/runtime/src/governance/collective_management.rs index 7c84bc38bd..77482c1ef1 100644 --- a/runtime/src/governance/collective_management.rs +++ b/runtime/src/governance/collective_management.rs @@ -46,9 +46,7 @@ impl pallet_multi_collective::OnNewTerm for CollectiveMa const RANKING_ITERATIONS_BOUND: u64 = 5_000; const READS_PER_ITERATION: u64 = 8; let db = ::DbWeight::get(); - let ranking = db.reads( - RANKING_ITERATIONS_BOUND.saturating_mul(READS_PER_ITERATION), - ); + let ranking = db.reads(RANKING_ITERATIONS_BOUND.saturating_mul(READS_PER_ITERATION)); let apply = db.reads_writes(1, 1); ranking.saturating_add(apply) } diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index a1e9157f7b..3af57da156 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -1870,11 +1870,11 @@ impl McOnMembersChanged for GovernanceVoteCle let db = ::DbWeight::get(); // Per-poll: VotingFor::take + TallyOf::get + TallyOf::insert // (= 2 reads + 2 writes), plus the cost of `on_tally_updated`. - let per_poll = db - .reads_writes(2, 2) - .saturating_add( - >::on_tally_updated_weight(), - ); + let per_poll = + db.reads_writes(2, 2) + .saturating_add(>::on_tally_updated_weight()); per_poll.saturating_mul(outgoing_max.saturating_mul(polls_max)) } }