From 539afe4d6e20d4e10b87e11a025a2efff325da5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Tue, 19 Oct 2021 16:15:10 +0200 Subject: [PATCH 1/4] pallet-multisig: Improve opaque call handling Before the opaque call was just a type redefinition of `Vec`. With metadata v14 that was breaking external tools, as they stopped looking at the type name. To improve the situation the `WrapperKeepOpaque` type is introduced that communicates to the outside the correct type info. --- Cargo.lock | 1 + client/db/Cargo.toml | 1 + frame/multisig/src/lib.rs | 48 ++++++------ frame/multisig/src/tests.rs | 129 ++++++++++++++++++++++++------- frame/support/src/traits.rs | 2 +- frame/support/src/traits/misc.rs | 103 +++++++++++++++++++++++- 6 files changed, 229 insertions(+), 55 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f15e363bfd6ae..78c1c11cc1292 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7571,6 +7571,7 @@ dependencies = [ name = "sc-client-db" version = "0.10.0-dev" dependencies = [ + "criterion", "hash-db", "kvdb", "kvdb-memorydb", diff --git a/client/db/Cargo.toml b/client/db/Cargo.toml index 53af082d3b91d..29e91493edc9b 100644 --- a/client/db/Cargo.toml +++ b/client/db/Cargo.toml @@ -41,6 +41,7 @@ substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/ru quickcheck = "1.0.3" kvdb-rocksdb = "0.14.0" tempfile = "3" +criterion = "0.3.5" [features] default = [] diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index 43040ada45a98..c3aa5cfc0e471 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -56,7 +56,7 @@ use frame_support::{ DispatchErrorWithPostInfo, DispatchResult, DispatchResultWithPostInfo, PostDispatchInfo, }, ensure, - traits::{Currency, Get, ReservableCurrency}, + traits::{Currency, Get, ReservableCurrency, WrapperKeepOpaque}, weights::{GetDispatchInfo, Weight}, RuntimeDebug, }; @@ -74,8 +74,6 @@ pub use pallet::*; type BalanceOf = <::Currency as Currency<::AccountId>>::Balance; -/// Just a bunch of bytes, but they should decode to a valid `Call`. -pub type OpaqueCall = Vec; /// A global extrinsic index, formed as the extrinsic index within a block, together with that /// block's height. This allows a transaction in which a multisig operation of a particular @@ -101,10 +99,12 @@ pub struct Multisig { approvals: Vec, } +type OpaqueCall = WrapperKeepOpaque<::Call>; + type CallHash = [u8; 32]; -enum CallOrHash { - Call(OpaqueCall, bool), +enum CallOrHash { + Call(OpaqueCall, bool), Hash([u8; 32]), } @@ -168,7 +168,7 @@ pub mod pallet { #[pallet::storage] pub type Calls = - StorageMap<_, Identity, [u8; 32], (OpaqueCall, T::AccountId, BalanceOf)>; + StorageMap<_, Identity, [u8; 32], (OpaqueCall, T::AccountId, BalanceOf)>; #[pallet::error] pub enum Error { @@ -339,7 +339,7 @@ pub mod pallet { /// # #[pallet::weight({ let s = other_signatories.len() as u32; - let z = call.len() as u32; + let z = call.encoded_len() as u32; T::WeightInfo::as_multi_create(s, z) .max(T::WeightInfo::as_multi_create_store(s, z)) @@ -352,7 +352,7 @@ pub mod pallet { threshold: u16, other_signatories: Vec, maybe_timepoint: Option>, - call: OpaqueCall, + call: OpaqueCall, store_call: bool, max_weight: Weight, ) -> DispatchResultWithPostInfo { @@ -370,9 +370,8 @@ pub mod pallet { /// Register approval for a dispatch to be made from a deterministic composite account if /// approved by a total of `threshold - 1` of `other_signatories`. /// - /// Payment: `DepositBase` will be reserved if this is the first approval, plus - /// `threshold` times `DepositFactor`. It is returned once this dispatch happens or - /// is cancelled. + /// Payment: `DepositBase` will be reserved if this is the first appromes `DepositFactor`. + /// It is returned once this dispatch happens or is cancelled. /// /// The dispatch origin for this call must be _Signed_. /// @@ -406,9 +405,9 @@ pub mod pallet { let s = other_signatories.len() as u32; T::WeightInfo::approve_as_multi_create(s) - .max(T::WeightInfo::approve_as_multi_approve(s)) - .max(T::WeightInfo::approve_as_multi_complete(s)) - .saturating_add(*max_weight) + .max(T::WeightInfo::approve_as_multi_approve(s)) + .max(T::WeightInfo::approve_as_multi_complete(s)) + .saturating_add(*max_weight) })] pub fn approve_as_multi( origin: OriginFor, @@ -502,7 +501,7 @@ impl Pallet { threshold: u16, other_signatories: Vec, maybe_timepoint: Option>, - call_or_hash: CallOrHash, + call_or_hash: CallOrHash, max_weight: Weight, ) -> DispatchResultWithPostInfo { ensure!(threshold >= 2, Error::::MinimumThreshold); @@ -517,8 +516,8 @@ impl Pallet { // Threshold > 1; this means it's a multi-step operation. We extract the `call_hash`. let (call_hash, call_len, maybe_call, store) = match call_or_hash { CallOrHash::Call(call, should_store) => { - let call_hash = blake2_256(&call); - let call_len = call.len(); + let call_hash = blake2_256(call.encoded()); + let call_len = call.encoded_len(); (call_hash, call_len, Some(call), should_store) }, CallOrHash::Hash(h) => (h, 0, None, false), @@ -541,7 +540,7 @@ impl Pallet { // We only bother fetching/decoding call if we know that we're ready to execute. let maybe_approved_call = if approvals >= threshold { - Self::get_call(&call_hash, maybe_call.as_ref().map(|c| c.as_ref())) + Self::get_call(&call_hash, maybe_call.as_ref()) } else { None }; @@ -658,13 +657,13 @@ impl Pallet { fn store_call_and_reserve( who: T::AccountId, hash: &[u8; 32], - data: OpaqueCall, + data: OpaqueCall, other_deposit: BalanceOf, ) -> DispatchResult { ensure!(!Calls::::contains_key(hash), Error::::AlreadyStored); let deposit = other_deposit + T::DepositBase::get() + - T::DepositFactor::get() * BalanceOf::::from(((data.len() + 31) / 32) as u32); + T::DepositFactor::get() * BalanceOf::::from(((data.encoded_len() + 31) / 32) as u32); T::Currency::reserve(&who, deposit)?; Calls::::insert(&hash, (data, who, deposit)); Ok(()) @@ -673,15 +672,14 @@ impl Pallet { /// Attempt to decode and return the call, provided by the user or from storage. fn get_call( hash: &[u8; 32], - maybe_known: Option<&[u8]>, + maybe_known: Option<&OpaqueCall>, ) -> Option<(::Call, usize)> { maybe_known.map_or_else( || { - Calls::::get(hash).and_then(|(data, ..)| { - Decode::decode(&mut &data[..]).ok().map(|d| (d, data.len())) - }) + Calls::::get(hash) + .and_then(|(data, ..)| Some((data.try_decode()?, data.encoded_len()))) }, - |data| Decode::decode(&mut &data[..]).ok().map(|d| (d, data.len())), + |data| Some((data.try_decode()?, data.encoded_len())), ) } diff --git a/frame/multisig/src/tests.rs b/frame/multisig/src/tests.rs index 3d311cf5d3dc8..d46c22ec73d09 100644 --- a/frame/multisig/src/tests.rs +++ b/frame/multisig/src/tests.rs @@ -31,6 +31,7 @@ use sp_runtime::{ type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic; type Block = frame_system::mocking::MockBlock; +type OpaqueCall = super::OpaqueCall; frame_support::construct_runtime!( pub enum Test where @@ -152,7 +153,7 @@ fn multisig_deposit_is_taken_and_returned() { 2, vec![2, 3], None, - data.clone(), + OpaqueCall::from_encoded(data.clone()), false, 0 )); @@ -164,7 +165,7 @@ fn multisig_deposit_is_taken_and_returned() { 2, vec![1, 3], Some(now()), - data, + OpaqueCall::from_encoded(data), false, call_weight )); @@ -185,7 +186,15 @@ fn multisig_deposit_is_taken_and_returned_with_call_storage() { let call_weight = call.get_dispatch_info().weight; let data = call.encode(); let hash = blake2_256(&data); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, data, true, 0)); + assert_ok!(Multisig::as_multi( + Origin::signed(1), + 2, + vec![2, 3], + None, + OpaqueCall::from_encoded(data), + true, + 0 + )); assert_eq!(Balances::free_balance(1), 0); assert_eq!(Balances::reserved_balance(1), 5); @@ -231,7 +240,7 @@ fn multisig_deposit_is_taken_and_returned_with_alt_call_storage() { 3, vec![1, 3], Some(now()), - data, + OpaqueCall::from_encoded(data), true, 0 )); @@ -316,7 +325,15 @@ fn timepoint_checking_works() { assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 2, vec![2, 3], None, hash, 0)); assert_noop!( - Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], None, call.clone(), false, 0), + Multisig::as_multi( + Origin::signed(2), + 2, + vec![1, 3], + None, + OpaqueCall::from_encoded(call.clone()), + false, + 0 + ), Error::::NoTimepoint, ); let later = Timepoint { index: 1, ..now() }; @@ -326,7 +343,7 @@ fn timepoint_checking_works() { 2, vec![1, 3], Some(later), - call.clone(), + OpaqueCall::from_encoded(call), false, 0 ), @@ -347,7 +364,15 @@ fn multisig_2_of_3_works_with_call_storing() { let call_weight = call.get_dispatch_info().weight; let data = call.encode(); let hash = blake2_256(&data); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, data, true, 0)); + assert_ok!(Multisig::as_multi( + Origin::signed(1), + 2, + vec![2, 3], + None, + OpaqueCall::from_encoded(data), + true, + 0 + )); assert_eq!(Balances::free_balance(6), 0); assert_ok!(Multisig::approve_as_multi( @@ -382,7 +407,7 @@ fn multisig_2_of_3_works() { 2, vec![1, 3], Some(now()), - data, + OpaqueCall::from_encoded(data), false, call_weight )); @@ -425,7 +450,7 @@ fn multisig_3_of_3_works() { 3, vec![1, 2], Some(now()), - data, + OpaqueCall::from_encoded(data), false, call_weight )); @@ -473,7 +498,15 @@ fn cancel_multisig_with_call_storage_works() { new_test_ext().execute_with(|| { let call = call_transfer(6, 15).encode(); let hash = blake2_256(&call); - assert_ok!(Multisig::as_multi(Origin::signed(1), 3, vec![2, 3], None, call, true, 0)); + assert_ok!(Multisig::as_multi( + Origin::signed(1), + 3, + vec![2, 3], + None, + OpaqueCall::from_encoded(call), + true, + 0 + )); assert_eq!(Balances::free_balance(1), 4); assert_ok!(Multisig::approve_as_multi( Origin::signed(2), @@ -517,7 +550,7 @@ fn cancel_multisig_with_alt_call_storage_works() { 3, vec![1, 3], Some(now()), - call, + OpaqueCall::from_encoded(call), true, 0 )); @@ -544,7 +577,7 @@ fn multisig_2_of_3_as_multi_works() { 2, vec![2, 3], None, - data.clone(), + OpaqueCall::from_encoded(data.clone()), false, 0 )); @@ -555,7 +588,7 @@ fn multisig_2_of_3_as_multi_works() { 2, vec![1, 3], Some(now()), - data, + OpaqueCall::from_encoded(data), false, call_weight )); @@ -583,7 +616,7 @@ fn multisig_2_of_3_as_multi_with_many_calls_works() { 2, vec![2, 3], None, - data1.clone(), + OpaqueCall::from_encoded(data1.clone()), false, 0 )); @@ -592,7 +625,7 @@ fn multisig_2_of_3_as_multi_with_many_calls_works() { 2, vec![1, 3], None, - data2.clone(), + OpaqueCall::from_encoded(data2.clone()), false, 0 )); @@ -601,7 +634,7 @@ fn multisig_2_of_3_as_multi_with_many_calls_works() { 2, vec![1, 2], Some(now()), - data1, + OpaqueCall::from_encoded(data1), false, call1_weight )); @@ -610,7 +643,7 @@ fn multisig_2_of_3_as_multi_with_many_calls_works() { 2, vec![1, 2], Some(now()), - data2, + OpaqueCall::from_encoded(data2), false, call2_weight )); @@ -637,7 +670,7 @@ fn multisig_2_of_3_cannot_reissue_same_call() { 2, vec![2, 3], None, - data.clone(), + OpaqueCall::from_encoded(data.clone()), false, 0 )); @@ -646,7 +679,7 @@ fn multisig_2_of_3_cannot_reissue_same_call() { 2, vec![1, 3], Some(now()), - data.clone(), + OpaqueCall::from_encoded(data.clone()), false, call_weight )); @@ -657,7 +690,7 @@ fn multisig_2_of_3_cannot_reissue_same_call() { 2, vec![2, 3], None, - data.clone(), + OpaqueCall::from_encoded(data.clone()), false, 0 )); @@ -666,7 +699,7 @@ fn multisig_2_of_3_cannot_reissue_same_call() { 2, vec![1, 2], Some(now()), - data.clone(), + OpaqueCall::from_encoded(data), false, call_weight )); @@ -683,11 +716,27 @@ fn minimum_threshold_check_works() { new_test_ext().execute_with(|| { let call = call_transfer(6, 15).encode(); assert_noop!( - Multisig::as_multi(Origin::signed(1), 0, vec![2], None, call.clone(), false, 0), + Multisig::as_multi( + Origin::signed(1), + 0, + vec![2], + None, + OpaqueCall::from_encoded(call.clone()), + false, + 0 + ), Error::::MinimumThreshold, ); assert_noop!( - Multisig::as_multi(Origin::signed(1), 1, vec![2], None, call.clone(), false, 0), + Multisig::as_multi( + Origin::signed(1), + 1, + vec![2], + None, + OpaqueCall::from_encoded(call.clone()), + false, + 0 + ), Error::::MinimumThreshold, ); }); @@ -698,7 +747,15 @@ fn too_many_signatories_fails() { new_test_ext().execute_with(|| { let call = call_transfer(6, 15).encode(); assert_noop!( - Multisig::as_multi(Origin::signed(1), 2, vec![2, 3, 4], None, call.clone(), false, 0), + Multisig::as_multi( + Origin::signed(1), + 2, + vec![2, 3, 4], + None, + OpaqueCall::from_encoded(call), + false, + 0 + ), Error::::TooManySignatories, ); }); @@ -765,7 +822,15 @@ fn multisig_1_of_3_works() { Error::::MinimumThreshold, ); assert_noop!( - Multisig::as_multi(Origin::signed(1), 1, vec![2, 3], None, call.clone(), false, 0), + Multisig::as_multi( + Origin::signed(1), + 1, + vec![2, 3], + None, + OpaqueCall::from_encoded(call), + false, + 0 + ), Error::::MinimumThreshold, ); let boxed_call = Box::new(call_transfer(6, 15)); @@ -801,14 +866,22 @@ fn weight_check_works() { 2, vec![2, 3], None, - data.clone(), + OpaqueCall::from_encoded(data.clone()), false, 0 )); assert_eq!(Balances::free_balance(6), 0); assert_noop!( - Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), data, false, 0), + Multisig::as_multi( + Origin::signed(2), + 2, + vec![1, 3], + Some(now()), + OpaqueCall::from_encoded(data), + false, + 0 + ), Error::::MaxWeightTooLow, ); }); @@ -860,7 +933,7 @@ fn multisig_handles_no_preimage_after_all_approve() { 3, vec![1, 2], Some(now()), - data, + OpaqueCall::from_encoded(data), false, call_weight )); diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 5ac0208dc2033..30260e06f5e24 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -52,7 +52,7 @@ mod misc; pub use misc::{ Backing, ConstU32, EnsureInherentsAreFirst, EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime, IsSubType, IsType, Len, OffchainWorker, - OnKilledAccount, OnNewAccount, SameOrOther, Time, TryDrop, UnixTime, WrapperOpaque, + OnKilledAccount, OnNewAccount, SameOrOther, Time, TryDrop, UnixTime, WrapperOpaque, WrapperKeepOpaque, }; mod stored_map; diff --git a/frame/support/src/traits/misc.rs b/frame/support/src/traits/misc.rs index 9109bfeeae722..d7123f8fbc40b 100644 --- a/frame/support/src/traits/misc.rs +++ b/frame/support/src/traits/misc.rs @@ -18,7 +18,7 @@ //! Smaller traits used in FRAME which don't need their own file. use crate::dispatch::Parameter; -use codec::{Decode, Encode, EncodeLike, Input, MaxEncodedLen}; +use codec::{CompactLen, Decode, Encode, EncodeLike, Input, MaxEncodedLen, DecodeAll}; use scale_info::{build::Fields, meta_type, Path, Type, TypeInfo, TypeParameter}; use sp_runtime::{traits::Block as BlockT, DispatchError}; use sp_std::prelude::*; @@ -390,6 +390,7 @@ impl, const T: u32> EstimateCallFee for pub struct WrapperOpaque(pub T); impl EncodeLike for WrapperOpaque {} +impl EncodeLike> for WrapperOpaque {} impl Encode for WrapperOpaque { fn size_hint(&self) -> usize { @@ -456,6 +457,93 @@ impl TypeInfo for WrapperOpaque { } } +/// A wrapper for any type `T` which implement encode/decode in a way compatible with `Vec`. +/// +/// This type is similar to [`WrapperOpaque`], but it differs in the way it stores the type `T`. +/// While [`WrapperOpaque`] stores the decoded type, the [`WrapperKeepOpaque`] stores the type only +/// in its opaque format, aka as a `Vec`. To access the real type `T` [`Self::try_decode`] needs +/// to be used. +#[derive(Debug, Eq, PartialEq, Default, Clone)] +pub struct WrapperKeepOpaque { + data: Vec, + _phantom: sp_std::marker::PhantomData, +} + +impl WrapperKeepOpaque { + /// Try to decode the wrapped type from the inner `data`. + /// + /// Returns `None` if the decoding failed. + pub fn try_decode(&self) -> Option { + T::decode_all(&mut &self.data[..]).ok() + } + + /// Returns the length of the encoded `T`. + pub fn encoded_len(&self) -> usize { + self.data.len() + } + + /// Returns the encoded data. + pub fn encoded(&self) -> &[u8] { + &self.data + } + + /// Create from the given encoded `data`. + pub fn from_encoded(data: Vec) -> Self { + Self { data, _phantom: sp_std::marker::PhantomData } + } +} + +impl EncodeLike for WrapperKeepOpaque {} +impl EncodeLike> for WrapperKeepOpaque {} + +impl Encode for WrapperKeepOpaque { + fn size_hint(&self) -> usize { + self.data.len() + codec::Compact::::compact_len(&(self.data.len() as u32)) + } + + fn encode_to(&self, dest: &mut O) { + self.data.encode_to(dest); + } + + fn encode(&self) -> Vec { + self.data.encode() + } + + fn using_encoded R>(&self, f: F) -> R { + self.data.using_encoded(f) + } +} + +impl Decode for WrapperKeepOpaque { + fn decode(input: &mut I) -> Result { + Ok(Self { data: Vec::::decode(input)?, _phantom: sp_std::marker::PhantomData }) + } + + fn skip(input: &mut I) -> Result<(), codec::Error> { + >::skip(input) + } +} + +impl MaxEncodedLen for WrapperKeepOpaque { + fn max_encoded_len() -> usize { + WrapperOpaque::::max_encoded_len() + } +} + +impl TypeInfo for WrapperKeepOpaque { + type Identity = Self; + fn type_info() -> Type { + Type::builder() + .path(Path::new("WrapperKeepOpaque", module_path!())) + .type_params(vec![TypeParameter::new("T", Some(meta_type::()))]) + .composite( + Fields::unnamed() + .field(|f| f.compact::()) + .field(|f| f.ty::().type_name("T")), + ) + } +} + #[cfg(test)] mod test { use super::*; @@ -488,4 +576,17 @@ mod test { ); assert_eq!(>::max_encoded_len(), 2usize.pow(14) + 4); } + + #[test] + fn test_keep_opaque_wrapper() { + let data = 3u32.encode().encode(); + + let keep_opaque = WrapperKeepOpaque::::decode(&mut &data[..]).unwrap(); + keep_opaque.try_decode().unwrap(); + + let data = WrapperOpaque(50u32).encode(); + let decoded = WrapperKeepOpaque::::decode(&mut &data[..]).unwrap(); + let data = decoded.encode(); + WrapperOpaque::::decode(&mut &data[..]).unwrap(); + } } From 7affeea730780e1fb52cc160493ebd677587b4d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 22 Oct 2021 20:16:51 +0200 Subject: [PATCH 2/4] Cleanup --- Cargo.lock | 1 - client/db/Cargo.toml | 1 - frame/multisig/src/lib.rs | 8 +++++--- frame/support/src/traits.rs | 3 ++- frame/support/src/traits/misc.rs | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 78c1c11cc1292..f15e363bfd6ae 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7571,7 +7571,6 @@ dependencies = [ name = "sc-client-db" version = "0.10.0-dev" dependencies = [ - "criterion", "hash-db", "kvdb", "kvdb-memorydb", diff --git a/client/db/Cargo.toml b/client/db/Cargo.toml index 29e91493edc9b..53af082d3b91d 100644 --- a/client/db/Cargo.toml +++ b/client/db/Cargo.toml @@ -41,7 +41,6 @@ substrate-test-runtime-client = { version = "2.0.0", path = "../../test-utils/ru quickcheck = "1.0.3" kvdb-rocksdb = "0.14.0" tempfile = "3" -criterion = "0.3.5" [features] default = [] diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index c3aa5cfc0e471..53567cc212afd 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -370,8 +370,9 @@ pub mod pallet { /// Register approval for a dispatch to be made from a deterministic composite account if /// approved by a total of `threshold - 1` of `other_signatories`. /// - /// Payment: `DepositBase` will be reserved if this is the first appromes `DepositFactor`. - /// It is returned once this dispatch happens or is cancelled. + /// Payment: `DepositBase` will be reserved if this is the first approval, plus + /// `threshold` times `DepositFactor`. It is returned once this dispatch happens or + /// is cancelled. /// /// The dispatch origin for this call must be _Signed_. /// @@ -663,7 +664,8 @@ impl Pallet { ensure!(!Calls::::contains_key(hash), Error::::AlreadyStored); let deposit = other_deposit + T::DepositBase::get() + - T::DepositFactor::get() * BalanceOf::::from(((data.encoded_len() + 31) / 32) as u32); + T::DepositFactor::get() * + BalanceOf::::from(((data.encoded_len() + 31) / 32) as u32); T::Currency::reserve(&who, deposit)?; Calls::::insert(&hash, (data, who, deposit)); Ok(()) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 30260e06f5e24..ee1dec59d0202 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -52,7 +52,8 @@ mod misc; pub use misc::{ Backing, ConstU32, EnsureInherentsAreFirst, EstimateCallFee, ExecuteBlock, ExtrinsicCall, Get, GetBacking, GetDefault, HandleLifetime, IsSubType, IsType, Len, OffchainWorker, - OnKilledAccount, OnNewAccount, SameOrOther, Time, TryDrop, UnixTime, WrapperOpaque, WrapperKeepOpaque, + OnKilledAccount, OnNewAccount, SameOrOther, Time, TryDrop, UnixTime, WrapperKeepOpaque, + WrapperOpaque, }; mod stored_map; diff --git a/frame/support/src/traits/misc.rs b/frame/support/src/traits/misc.rs index d7123f8fbc40b..6587945604d0b 100644 --- a/frame/support/src/traits/misc.rs +++ b/frame/support/src/traits/misc.rs @@ -18,7 +18,7 @@ //! Smaller traits used in FRAME which don't need their own file. use crate::dispatch::Parameter; -use codec::{CompactLen, Decode, Encode, EncodeLike, Input, MaxEncodedLen, DecodeAll}; +use codec::{CompactLen, Decode, DecodeAll, Encode, EncodeLike, Input, MaxEncodedLen}; use scale_info::{build::Fields, meta_type, Path, Type, TypeInfo, TypeParameter}; use sp_runtime::{traits::Block as BlockT, DispatchError}; use sp_std::prelude::*; From 34d704a9ff394048a17cd16aec8d4232612a42d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Fri, 22 Oct 2021 21:12:04 +0200 Subject: [PATCH 3/4] Fix benchmarks --- frame/multisig/src/benchmarking.rs | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/frame/multisig/src/benchmarking.rs b/frame/multisig/src/benchmarking.rs index edfeba253e5f0..50e089d4d1db8 100644 --- a/frame/multisig/src/benchmarking.rs +++ b/frame/multisig/src/benchmarking.rs @@ -29,7 +29,7 @@ use crate::Pallet as Multisig; const SEED: u32 = 0; -fn setup_multi(s: u32, z: u32) -> Result<(Vec, Vec), &'static str> { +fn setup_multi(s: u32, z: u32) -> Result<(Vec, OpaqueCall), &'static str> { let mut signatories: Vec = Vec::new(); for i in 0..s { let signatory = account("signatory", i, SEED); @@ -42,7 +42,7 @@ fn setup_multi(s: u32, z: u32) -> Result<(Vec, Vec) // Must first convert to outer call type. let call: ::Call = frame_system::Call::::remark { remark: vec![0; z as usize] }.into(); - let call_data = call.encode(); + let call_data = OpaqueCall::::from_encoded(call.encode()); return Ok((signatories, call_data)) } @@ -72,7 +72,7 @@ benchmarks! { // Transaction Length let z in 0 .. 10_000; let (mut signatories, call) = setup_multi::(s, z)?; - let call_hash = blake2_256(&call); + let call_hash = blake2_256(&call.encoded()); let multi_account_id = Multisig::::multi_account_id(&signatories, s.try_into().unwrap()); let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; // Whitelist caller account from further DB operations. @@ -90,7 +90,7 @@ benchmarks! { // Transaction Length let z in 0 .. 10_000; let (mut signatories, call) = setup_multi::(s, z)?; - let call_hash = blake2_256(&call); + let call_hash = blake2_256(&call.encoded()); let multi_account_id = Multisig::::multi_account_id(&signatories, s.try_into().unwrap()); let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); @@ -109,7 +109,7 @@ benchmarks! { // Transaction Length let z in 0 .. 10_000; let (mut signatories, call) = setup_multi::(s, z)?; - let call_hash = blake2_256(&call); + let call_hash = blake2_256(&call.encoded()); let multi_account_id = Multisig::::multi_account_id(&signatories, s.try_into().unwrap()); let mut signatories2 = signatories.clone(); let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; @@ -134,7 +134,7 @@ benchmarks! { // Transaction Length let z in 0 .. 10_000; let (mut signatories, call) = setup_multi::(s, z)?; - let call_hash = blake2_256(&call); + let call_hash = blake2_256(&call.encoded()); let multi_account_id = Multisig::::multi_account_id(&signatories, s.try_into().unwrap()); let mut signatories2 = signatories.clone(); let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; @@ -160,7 +160,7 @@ benchmarks! { // Transaction Length let z in 0 .. 10_000; let (mut signatories, call) = setup_multi::(s, z)?; - let call_hash = blake2_256(&call); + let call_hash = blake2_256(&call.encoded()); let multi_account_id = Multisig::::multi_account_id(&signatories, s.try_into().unwrap()); let mut signatories2 = signatories.clone(); let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; @@ -193,7 +193,7 @@ benchmarks! { let (mut signatories, call) = setup_multi::(s, z)?; let multi_account_id = Multisig::::multi_account_id(&signatories, s.try_into().unwrap()); let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; - let call_hash = blake2_256(&call); + let call_hash = blake2_256(&call.encoded()); // Whitelist caller account from further DB operations. let caller_key = frame_system::Account::::hashed_key_for(&caller); frame_benchmarking::benchmarking::add_to_whitelist(caller_key.into()); @@ -212,7 +212,7 @@ benchmarks! { let mut signatories2 = signatories.clone(); let multi_account_id = Multisig::::multi_account_id(&signatories, s.try_into().unwrap()); let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; - let call_hash = blake2_256(&call); + let call_hash = blake2_256(&call.encoded()); // before the call, get the timepoint let timepoint = Multisig::::timepoint(); // Create the multi @@ -245,7 +245,7 @@ benchmarks! { let mut signatories2 = signatories.clone(); let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; T::Currency::make_free_balance_be(&caller, BalanceOf::::max_value()); - let call_hash = blake2_256(&call); + let call_hash = blake2_256(&call.encoded()); // before the call, get the timepoint let timepoint = Multisig::::timepoint(); // Create the multi @@ -282,7 +282,7 @@ benchmarks! { let (mut signatories, call) = setup_multi::(s, z)?; let multi_account_id = Multisig::::multi_account_id(&signatories, s.try_into().unwrap()); let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; - let call_hash = blake2_256(&call); + let call_hash = blake2_256(&call.encoded()); let timepoint = Multisig::::timepoint(); // Create the multi let o = RawOrigin::Signed(caller.clone()).into(); From e0abf959e1ba294f9c6c1428ef443280fd2833ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= Date: Sun, 24 Oct 2021 23:48:55 +0200 Subject: [PATCH 4/4] FMT --- frame/multisig/src/benchmarking.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/multisig/src/benchmarking.rs b/frame/multisig/src/benchmarking.rs index 50e089d4d1db8..1390b6eebbe34 100644 --- a/frame/multisig/src/benchmarking.rs +++ b/frame/multisig/src/benchmarking.rs @@ -29,7 +29,10 @@ use crate::Pallet as Multisig; const SEED: u32 = 0; -fn setup_multi(s: u32, z: u32) -> Result<(Vec, OpaqueCall), &'static str> { +fn setup_multi( + s: u32, + z: u32, +) -> Result<(Vec, OpaqueCall), &'static str> { let mut signatories: Vec = Vec::new(); for i in 0..s { let signatory = account("signatory", i, SEED);