From 58d23ef97bdb8df6a74d66841e32dd2d8d448d67 Mon Sep 17 00:00:00 2001 From: Gavin Wood Date: Thu, 2 Jun 2022 12:41:05 +0100 Subject: [PATCH] Reduce call size of Referenda pallet (#11578) * Reduce call size of Referenda pallet * Fixes * Fixes * Fixes * Docs --- substrate/frame/referenda/src/benchmarking.rs | 4 +-- substrate/frame/referenda/src/lib.rs | 4 +-- substrate/frame/referenda/src/mock.rs | 4 +-- substrate/frame/referenda/src/tests.rs | 33 +++++++++-------- .../src/construct_runtime/expand/call.rs | 36 +++++++++++++++++++ 5 files changed, 61 insertions(+), 20 deletions(-) diff --git a/substrate/frame/referenda/src/benchmarking.rs b/substrate/frame/referenda/src/benchmarking.rs index e4cbf2ca52..9abd3768f7 100644 --- a/substrate/frame/referenda/src/benchmarking.rs +++ b/substrate/frame/referenda/src/benchmarking.rs @@ -46,7 +46,7 @@ fn create_referendum() -> (T::AccountId, ReferendumIndex) { whitelist_account!(caller); assert_ok!(Referenda::::submit( RawOrigin::Signed(caller.clone()).into(), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), T::Hashing::hash_of(&0), DispatchTime::After(0u32.into()) )); @@ -177,7 +177,7 @@ benchmarks! { whitelist_account!(caller); }: _( RawOrigin::Signed(caller), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), T::Hashing::hash_of(&0), DispatchTime::After(0u32.into()) ) verify { diff --git a/substrate/frame/referenda/src/lib.rs b/substrate/frame/referenda/src/lib.rs index f21a78bcb7..ab8c3ad0ef 100644 --- a/substrate/frame/referenda/src/lib.rs +++ b/substrate/frame/referenda/src/lib.rs @@ -355,7 +355,7 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::submit())] pub fn submit( origin: OriginFor, - proposal_origin: PalletsOriginOf, + proposal_origin: Box>, proposal_hash: T::Hash, enactment_moment: DispatchTime, ) -> DispatchResult { @@ -373,7 +373,7 @@ pub mod pallet { let nudge_call = Call::nudge_referendum { index }; let status = ReferendumStatus { track, - origin: proposal_origin, + origin: *proposal_origin, proposal_hash, enactment: enactment_moment, submitted: now, diff --git a/substrate/frame/referenda/src/mock.rs b/substrate/frame/referenda/src/mock.rs index 8b06fb3205..1a24911603 100644 --- a/substrate/frame/referenda/src/mock.rs +++ b/substrate/frame/referenda/src/mock.rs @@ -310,7 +310,7 @@ pub fn set_balance_proposal_hash(value: u64) -> H256 { pub fn propose_set_balance(who: u64, value: u64, delay: u64) -> DispatchResult { Referenda::submit( Origin::signed(who), - frame_system::RawOrigin::Root.into(), + Box::new(frame_system::RawOrigin::Root.into()), set_balance_proposal_hash(value), DispatchTime::After(delay), ) @@ -438,7 +438,7 @@ impl RefState { pub fn create(self) -> ReferendumIndex { assert_ok!(Referenda::submit( Origin::signed(1), - frame_support::dispatch::RawOrigin::Root.into(), + Box::new(frame_support::dispatch::RawOrigin::Root.into()), set_balance_proposal_hash(1), DispatchTime::At(10), )); diff --git a/substrate/frame/referenda/src/tests.rs b/substrate/frame/referenda/src/tests.rs index db55fdb8c4..d5435daf18 100644 --- a/substrate/frame/referenda/src/tests.rs +++ b/substrate/frame/referenda/src/tests.rs @@ -43,7 +43,7 @@ fn basic_happy_path_works() { // #1: submit assert_ok!(Referenda::submit( Origin::signed(1), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), set_balance_proposal_hash(1), DispatchTime::At(10), )); @@ -174,7 +174,7 @@ fn queueing_works() { // Submit a proposal into a track with a queue len of 1. assert_ok!(Referenda::submit( Origin::signed(5), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), set_balance_proposal_hash(0), DispatchTime::After(0), )); @@ -186,7 +186,7 @@ fn queueing_works() { for i in 1..=4 { assert_ok!(Referenda::submit( Origin::signed(i), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), set_balance_proposal_hash(i), DispatchTime::After(0), )); @@ -271,7 +271,7 @@ fn auto_timeout_should_happen_with_nothing_but_submit() { // #1: submit assert_ok!(Referenda::submit( Origin::signed(1), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), set_balance_proposal_hash(1), DispatchTime::At(20), )); @@ -291,13 +291,13 @@ fn tracks_are_distinguished() { new_test_ext().execute_with(|| { assert_ok!(Referenda::submit( Origin::signed(1), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), set_balance_proposal_hash(1), DispatchTime::At(10), )); assert_ok!(Referenda::submit( Origin::signed(2), - RawOrigin::None.into(), + Box::new(RawOrigin::None.into()), set_balance_proposal_hash(2), DispatchTime::At(20), )); @@ -355,7 +355,7 @@ fn submit_errors_work() { assert_noop!( Referenda::submit( Origin::signed(1), - RawOrigin::Signed(2).into(), + Box::new(RawOrigin::Signed(2).into()), h, DispatchTime::At(10), ), @@ -364,7 +364,12 @@ fn submit_errors_work() { // No funds for deposit assert_noop!( - Referenda::submit(Origin::signed(10), RawOrigin::Root.into(), h, DispatchTime::At(10),), + Referenda::submit( + Origin::signed(10), + Box::new(RawOrigin::Root.into()), + h, + DispatchTime::At(10), + ), BalancesError::::InsufficientBalance ); }); @@ -379,7 +384,7 @@ fn decision_deposit_errors_work() { let h = set_balance_proposal_hash(1); assert_ok!(Referenda::submit( Origin::signed(1), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), h, DispatchTime::At(10), )); @@ -401,7 +406,7 @@ fn refund_deposit_works() { let h = set_balance_proposal_hash(1); assert_ok!(Referenda::submit( Origin::signed(1), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), h, DispatchTime::At(10), )); @@ -423,7 +428,7 @@ fn cancel_works() { let h = set_balance_proposal_hash(1); assert_ok!(Referenda::submit( Origin::signed(1), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), h, DispatchTime::At(10), )); @@ -442,7 +447,7 @@ fn cancel_errors_works() { let h = set_balance_proposal_hash(1); assert_ok!(Referenda::submit( Origin::signed(1), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), h, DispatchTime::At(10), )); @@ -460,7 +465,7 @@ fn kill_works() { let h = set_balance_proposal_hash(1); assert_ok!(Referenda::submit( Origin::signed(1), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), h, DispatchTime::At(10), )); @@ -480,7 +485,7 @@ fn kill_errors_works() { let h = set_balance_proposal_hash(1); assert_ok!(Referenda::submit( Origin::signed(1), - RawOrigin::Root.into(), + Box::new(RawOrigin::Root.into()), h, DispatchTime::At(10), )); diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs index ad52ca48f6..c8c5d5ff0e 100644 --- a/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs +++ b/substrate/frame/support/procedural/src/construct_runtime/expand/call.rs @@ -62,6 +62,42 @@ pub fn expand_outer_dispatch( pub enum Call { #variant_defs } + #[cfg(test)] + impl Call { + /// Return a list of the module names together with their size in memory. + pub const fn sizes() -> &'static [( &'static str, usize )] { + use #scrate::dispatch::Callable; + use core::mem::size_of; + &[#( + ( + stringify!(#pallet_names), + size_of::< <#pallet_names as Callable<#runtime>>::Call >(), + ), + )*] + } + + /// Panics with diagnostic information if the size is greater than the given `limit`. + pub fn assert_size_under(limit: usize) { + let size = core::mem::size_of::(); + let call_oversize = size > limit; + if call_oversize { + println!("Size of `Call` is {} bytes (provided limit is {} bytes)", size, limit); + let mut sizes = Self::sizes().to_vec(); + sizes.sort_by_key(|x| -(x.1 as isize)); + for (i, &(name, size)) in sizes.iter().enumerate().take(5) { + println!("Offender #{}: {} at {} bytes", i + 1, name, size); + } + if let Some((_, next_size)) = sizes.get(5) { + println!("{} others of size {} bytes or less", sizes.len() - 5, next_size); + } + panic!( + "Size of `Call` is more than limit; use `Box` on complex parameter types to reduce the + size of `Call`. + If the limit is too strong, maybe consider providing a higher limit." + ); + } + } + } impl #scrate::dispatch::GetDispatchInfo for Call { fn get_dispatch_info(&self) -> #scrate::dispatch::DispatchInfo { match self {