diff --git a/Cargo.lock b/Cargo.lock index 398a6bc426..b4fee79839 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", @@ -10094,6 +10095,7 @@ dependencies = [ "sp-core", "sp-io", "sp-runtime", + "subtensor-runtime-common", ] [[package]] @@ -10366,6 +10368,7 @@ dependencies = [ name = "pallet-referenda" version = "1.0.0" dependencies = [ + "frame-benchmarking", "frame-support", "frame-system", "log", diff --git a/Cargo.toml b/Cargo.toml index 10d64709b5..975ef9a52d 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/common/src/traits.rs b/common/src/traits.rs index d4dc0e79e1..c2d84b460b 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,83 @@ 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 { + #[allow(clippy::let_and_return)] + let mut weight = Weight::zero(); + for_tuples!( #( weight.saturating_accrue(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 { + #[allow(clippy::let_and_return)] + let mut weight = Weight::zero(); + for_tuples!( #( weight.saturating_accrue(Tuple::weight()); )* ); + 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], + ); + /// 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)] +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); )* ); + } + + 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 411bb8eae5..116a6cba5a 100644 --- a/pallets/multi-collective/Cargo.toml +++ b/pallets/multi-collective/Cargo.toml @@ -17,10 +17,12 @@ 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 } num-traits = { workspace = true } +subtensor-runtime-common = { workspace = true } [dev-dependencies] sp-io = { workspace = true, default-features = true } @@ -32,14 +34,18 @@ 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" + "sp-runtime/runtime-benchmarks", + "subtensor-runtime-common/runtime-benchmarks", ] try-runtime = [ "frame-support/try-runtime", diff --git a/pallets/multi-collective/src/benchmarking.rs b/pallets/multi-collective/src/benchmarking.rs new file mode 100644 index 0000000000..b97a90d12a --- /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 d32ad5bced..f41627235c 100644 --- a/pallets/multi-collective/src/lib.rs +++ b/pallets/multi-collective/src/lib.rs @@ -3,15 +3,24 @@ 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::*; +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]; @@ -40,8 +49,8 @@ pub mod pallet { /// Required origin for swapping a member in a collective. type SwapOrigin: EnsureOriginWithArg; - /// Required origin for resetting the members of a collective. - type SetMembersOrigin: EnsureOriginWithArg; + /// 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. type OnMembersChanged: OnMembersChanged; @@ -56,8 +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 `set_members` + /// can diff against the previous set with a linear merge. #[pallet::storage] pub(super) type Members = StorageMap< _, @@ -134,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, @@ -200,12 +189,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(()) })?; @@ -220,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, @@ -229,12 +224,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(()) })?; @@ -248,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, @@ -258,12 +258,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(()) })?; @@ -281,12 +291,15 @@ 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, members: Vec, ) -> DispatchResult { - T::SetMembersOrigin::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 @@ -298,33 +311,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::MembersSet { collective_id, - members, + members: sorted, }); Ok(()) } @@ -346,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(), @@ -359,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", + ); + } } } } @@ -415,32 +491,15 @@ 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. + /// 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. @@ -469,7 +535,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/mock.rs b/pallets/multi-collective/src/mock.rs index a166501476..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. @@ -228,22 +232,49 @@ impl pallet_multi_collective::Config for Test { type AddOrigin = AsEnsureOriginWithArg>; type RemoveOrigin = AsEnsureOriginWithArg>; type SwapOrigin = AsEnsureOriginWithArg>; - type SetMembersOrigin = AsEnsureOriginWithArg>; + type SetOrigin = AsEnsureOriginWithArg>; 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 f4f5872fa4..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(), @@ -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] ); }); } @@ -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/Cargo.toml b/pallets/referenda/Cargo.toml index 4892bd53d6..17fbe7a7d8 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,28 +43,31 @@ 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 = [ + "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", - "sp-runtime/runtime-benchmarks", - "subtensor-runtime-common/runtime-benchmarks" + "pallet-signed-voting/runtime-benchmarks" ] 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", - "sp-runtime/try-runtime" + "pallet-signed-voting/try-runtime" ] diff --git a/pallets/referenda/src/benchmarking.rs b/pallets/referenda/src/benchmarking.rs new file mode 100644 index 0000000000..71e311d7a9 --- /dev/null +++ b/pallets/referenda/src/benchmarking.rs @@ -0,0 +1,116 @@ +//! 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. +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use super::*; +use alloc::boxed::Box; +use frame_benchmarking::v2::*; +use frame_system::RawOrigin; +use sp_runtime::Perbill; + +#[benchmarks] +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_adjustable(); + let call = Box::new(T::BenchmarkHelper::call()); + + #[extrinsic_call] + submit(RawOrigin::Signed(proposer), track, call); + + 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_adjustable(); + 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); + + assert!(matches!( + ReferendumStatusFor::::get(index), + Some(ReferendumStatus::Delegated(_)) + )); + } + + /// `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 3a2fe091ee..3d956c0195 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,19 +140,23 @@ 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::*; +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::*; @@ -201,10 +205,44 @@ 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; + + /// 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 @@ -310,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, @@ -368,7 +409,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, @@ -382,16 +423,21 @@ 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)?; 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( @@ -405,6 +451,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)?; @@ -421,7 +473,7 @@ pub mod pallet { // Terminal state: nothing further to do. Reached when an // alarm fires after a manual kill or a delegated handoff. } - }; + } Ok(()) } @@ -499,6 +551,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 { @@ -537,16 +607,19 @@ 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)); + 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); + T::OnPollCompleted::on_poll_completed(index); Self::deposit_event(event); } @@ -662,7 +735,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 +957,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 afbc6994bd..9d0c34fecf 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; @@ -100,15 +101,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 + } } } } @@ -324,6 +334,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! { @@ -336,10 +351,26 @@ 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 SetMembersOrigin = frame_support::traits::AsEnsureOriginWithArg>; + type SetOrigin = frame_support::traits::AsEnsureOriginWithArg>; 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! { @@ -363,7 +394,32 @@ impl pallet_referenda::Config for Test { type KillOrigin = EnsureRoot; type Tracks = TestTracks; type BlockNumberProvider = System; - type PollHooks = SignedVoting; + 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 { @@ -382,6 +438,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(), @@ -411,12 +475,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/pallets/signed-voting/src/lib.rs b/pallets/signed-voting/src/lib.rs index 0110cee107..d181948294 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::*; @@ -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) { @@ -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/Cargo.toml b/runtime/Cargo.toml index a0d41c991b..71c38d8ca4 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -334,11 +334,11 @@ 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", "pallet-multi-collective/runtime-benchmarks", - "pallet-referenda/runtime-benchmarks", "pallet-signed-voting/runtime-benchmarks" ] try-runtime = [ diff --git a/runtime/src/governance/collective_management.rs b/runtime/src/governance/collective_management.rs index 9fd03d532a..77482c1ef1 100644 --- a/runtime/src/governance/collective_management.rs +++ b/runtime/src/governance/collective_management.rs @@ -26,6 +26,31 @@ 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 db0eff9769..86ecdfc6a2 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -882,7 +882,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. @@ -1653,8 +1653,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, @@ -1742,15 +1742,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 + } } } } @@ -1800,7 +1812,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, }, @@ -1809,7 +1821,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, }, @@ -1818,7 +1830,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()), }, @@ -1827,7 +1839,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()), }, @@ -1852,6 +1864,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 { @@ -1860,10 +1891,32 @@ impl pallet_multi_collective::Config for Runtime { type AddOrigin = AsEnsureOriginWithArg>; type RemoveOrigin = AsEnsureOriginWithArg>; type SwapOrigin = AsEnsureOriginWithArg>; - type SetMembersOrigin = AsEnsureOriginWithArg>; + type SetOrigin = AsEnsureOriginWithArg>; 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 { @@ -1879,7 +1932,43 @@ 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; + 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. @@ -2010,6 +2099,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"