From 8fd1b670db55acddd097f0cf12fa69c5d7d8ea1c Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Sun, 9 Oct 2022 19:30:04 -0400 Subject: [PATCH 1/2] Bounded multisig --- frame/multisig/Cargo.toml | 3 ++- frame/multisig/src/lib.rs | 31 +++++++++++++++++++++---------- primitives/core/src/lib.rs | 8 ++++++++ 3 files changed, 31 insertions(+), 11 deletions(-) diff --git a/frame/multisig/Cargo.toml b/frame/multisig/Cargo.toml index bfd0870d30c22..b92b1f8aafed1 100644 --- a/frame/multisig/Cargo.toml +++ b/frame/multisig/Cargo.toml @@ -21,13 +21,13 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } +sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" } # third party log = { version = "0.4.17", default-features = false } [dev-dependencies] pallet-balances = { version = "4.0.0-dev", path = "../balances" } -sp-core = { version = "6.0.0", path = "../../primitives/core" } [features] default = ["std"] @@ -37,6 +37,7 @@ std = [ "frame-support/std", "frame-system/std", "scale-info/std", + "sp-core/std", "sp-io/std", "sp-runtime/std", "sp-std/std", diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index e3031cc830209..7d55615cc6c42 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -51,7 +51,7 @@ pub mod migrations; mod tests; pub mod weights; -use codec::{Decode, Encode}; +use codec::{Decode, Encode, MaxEncodedLen}; use frame_support::{ dispatch::{ DispatchErrorWithPostInfo, DispatchResult, DispatchResultWithPostInfo, GetDispatchInfo, @@ -60,10 +60,11 @@ use frame_support::{ ensure, traits::{Currency, Get, ReservableCurrency}, weights::Weight, - RuntimeDebug, + BoundedVec, RuntimeDebug, }; use frame_system::{self as system, RawOrigin}; use scale_info::TypeInfo; +use sp_core::GetU16ToU32; use sp_io::hashing::blake2_256; use sp_runtime::{ traits::{Dispatchable, TrailingZeroInput, Zero}, @@ -94,7 +95,9 @@ type BalanceOf = /// 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 /// composite was created to be uniquely identified. -#[derive(Copy, Clone, Eq, PartialEq, Encode, Decode, Default, RuntimeDebug, TypeInfo)] +#[derive( + Copy, Clone, Eq, PartialEq, Encode, Decode, Default, RuntimeDebug, TypeInfo, MaxEncodedLen, +)] pub struct Timepoint { /// The height of the chain at the point in time. height: BlockNumber, @@ -103,8 +106,12 @@ pub struct Timepoint { } /// An open multisig operation. -#[derive(Clone, Eq, PartialEq, Encode, Decode, Default, RuntimeDebug, TypeInfo)] -pub struct Multisig { +#[derive(Clone, Eq, PartialEq, Encode, Decode, Default, RuntimeDebug, TypeInfo, MaxEncodedLen)] +#[scale_info(skip_type_params(MaxApprovals))] +pub struct Multisig +where + MaxApprovals: Get, +{ /// The extrinsic when the multisig operation was opened. when: Timepoint, /// The amount held in reserve of the `depositor`, to be returned once the operation ends. @@ -112,7 +119,7 @@ pub struct Multisig { /// The account who opened it (i.e. the first to approve it). depositor: AccountId, /// The approvals achieved so far, including the depositor. Always sorted. - approvals: Vec, + approvals: BoundedVec, } type CallHash = [u8; 32]; @@ -170,7 +177,6 @@ pub mod pallet { #[pallet::pallet] #[pallet::generate_store(pub(super) trait Store)] - #[pallet::without_storage_info] #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(_); @@ -182,7 +188,7 @@ pub mod pallet { T::AccountId, Blake2_128Concat, [u8; 32], - Multisig, T::AccountId>, + Multisig, T::AccountId, GetU16ToU32>, >; #[pallet::error] @@ -601,7 +607,9 @@ impl Pallet { if let Some(pos) = maybe_pos { // Record approval. - m.approvals.insert(pos, who.clone()); + m.approvals + .try_insert(pos, who.clone()) + .map_err(|_| Error::::TooManySignatories)?; >::insert(&id, call_hash, m); Self::deposit_event(Event::MultisigApproval { approving: who, @@ -629,6 +637,9 @@ impl Pallet { T::Currency::reserve(&who, deposit)?; + let initial_approvals = + vec![who.clone()].try_into().map_err(|_| Error::::TooManySignatories)?; + >::insert( &id, call_hash, @@ -636,7 +647,7 @@ impl Pallet { when: Self::timepoint(), deposit, depositor: who.clone(), - approvals: vec![who.clone()], + approvals: initial_approvals, }, ); Self::deposit_event(Event::NewMultisig { approving: who, multisig: id, call_hash }); diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index fda7604d5337f..795fde924bcb3 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -419,6 +419,14 @@ impl Get for GetDefault { } } +/// A simple wrapper to turn a `Get` to `Get`. +pub struct GetU16ToU32(core::marker::PhantomData); +impl> Get for GetU16ToU32 { + fn get() -> u32 { + T::get().into() + } +} + macro_rules! impl_const_get { ($name:ident, $t:ty) => { #[doc = "Const getter for a basic type."] From f23eb969285cdaadb3b97054a329e2b33d36e30c Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 20 Oct 2022 17:35:47 -0400 Subject: [PATCH 2/2] just use u32 --- bin/node/runtime/src/lib.rs | 2 +- frame/multisig/Cargo.toml | 3 +-- frame/multisig/src/benchmarking.rs | 12 ++++++------ frame/multisig/src/lib.rs | 5 ++--- frame/multisig/src/tests.rs | 4 ++-- frame/utility/src/tests.rs | 2 +- primitives/core/src/lib.rs | 8 -------- 7 files changed, 13 insertions(+), 23 deletions(-) diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 142173621036d..ed70a21e23507 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -249,7 +249,7 @@ impl pallet_multisig::Config for Runtime { type Currency = Balances; type DepositBase = DepositBase; type DepositFactor = DepositFactor; - type MaxSignatories = ConstU16<100>; + type MaxSignatories = ConstU32<100>; type WeightInfo = pallet_multisig::weights::SubstrateWeight; } diff --git a/frame/multisig/Cargo.toml b/frame/multisig/Cargo.toml index b92b1f8aafed1..bfd0870d30c22 100644 --- a/frame/multisig/Cargo.toml +++ b/frame/multisig/Cargo.toml @@ -21,13 +21,13 @@ frame-system = { version = "4.0.0-dev", default-features = false, path = "../sys sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/io" } sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" } sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" } -sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" } # third party log = { version = "0.4.17", default-features = false } [dev-dependencies] pallet-balances = { version = "4.0.0-dev", path = "../balances" } +sp-core = { version = "6.0.0", path = "../../primitives/core" } [features] default = ["std"] @@ -37,7 +37,6 @@ std = [ "frame-support/std", "frame-system/std", "scale-info/std", - "sp-core/std", "sp-io/std", "sp-runtime/std", "sp-std/std", diff --git a/frame/multisig/src/benchmarking.rs b/frame/multisig/src/benchmarking.rs index d949414e31cb3..d5faf9ae8ac1a 100644 --- a/frame/multisig/src/benchmarking.rs +++ b/frame/multisig/src/benchmarking.rs @@ -69,7 +69,7 @@ benchmarks! { as_multi_create { // Signatories, need at least 2 total people - let s in 2 .. T::MaxSignatories::get() as u32; + let s in 2 .. T::MaxSignatories::get(); // Transaction Length let z in 0 .. 10_000; let (mut signatories, call) = setup_multi::(s, z)?; @@ -86,7 +86,7 @@ benchmarks! { as_multi_approve { // Signatories, need at least 3 people (so we don't complete the multisig) - let s in 3 .. T::MaxSignatories::get() as u32; + let s in 3 .. T::MaxSignatories::get(); // Transaction Length let z in 0 .. 10_000; let (mut signatories, call) = setup_multi::(s, z)?; @@ -110,7 +110,7 @@ benchmarks! { as_multi_complete { // Signatories, need at least 2 people - let s in 2 .. T::MaxSignatories::get() as u32; + let s in 2 .. T::MaxSignatories::get(); // Transaction Length let z in 0 .. 10_000; let (mut signatories, call) = setup_multi::(s, z)?; @@ -141,7 +141,7 @@ benchmarks! { approve_as_multi_create { // Signatories, need at least 2 people - let s in 2 .. T::MaxSignatories::get() as u32; + let s in 2 .. T::MaxSignatories::get(); // Transaction Length, not a component let z = 10_000; let (mut signatories, call) = setup_multi::(s, z)?; @@ -159,7 +159,7 @@ benchmarks! { approve_as_multi_approve { // Signatories, need at least 2 people - let s in 2 .. T::MaxSignatories::get() as u32; + let s in 2 .. T::MaxSignatories::get(); // Transaction Length, not a component let z = 10_000; let (mut signatories, call) = setup_multi::(s, z)?; @@ -190,7 +190,7 @@ benchmarks! { cancel_as_multi { // Signatories, need at least 2 people - let s in 2 .. T::MaxSignatories::get() as u32; + let s in 2 .. T::MaxSignatories::get(); // Transaction Length, not a component let z = 10_000; let (mut signatories, call) = setup_multi::(s, z)?; diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index 7d55615cc6c42..34d9124f9b69e 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -64,7 +64,6 @@ use frame_support::{ }; use frame_system::{self as system, RawOrigin}; use scale_info::TypeInfo; -use sp_core::GetU16ToU32; use sp_io::hashing::blake2_256; use sp_runtime::{ traits::{Dispatchable, TrailingZeroInput, Zero}, @@ -166,7 +165,7 @@ pub mod pallet { /// The maximum amount of signatories allowed in the multisig. #[pallet::constant] - type MaxSignatories: Get; + type MaxSignatories: Get; /// Weight information for extrinsics in this pallet. type WeightInfo: WeightInfo; @@ -188,7 +187,7 @@ pub mod pallet { T::AccountId, Blake2_128Concat, [u8; 32], - Multisig, T::AccountId, GetU16ToU32>, + Multisig, T::AccountId, T::MaxSignatories>, >; #[pallet::error] diff --git a/frame/multisig/src/tests.rs b/frame/multisig/src/tests.rs index f753b6f386c56..206e566cf4cb6 100644 --- a/frame/multisig/src/tests.rs +++ b/frame/multisig/src/tests.rs @@ -24,7 +24,7 @@ use super::*; use crate as pallet_multisig; use frame_support::{ assert_noop, assert_ok, parameter_types, - traits::{ConstU16, ConstU32, ConstU64, Contains}, + traits::{ConstU32, ConstU64, Contains}, }; use sp_core::H256; use sp_runtime::{ @@ -107,7 +107,7 @@ impl Config for Test { type Currency = Balances; type DepositBase = ConstU64<1>; type DepositFactor = ConstU64<1>; - type MaxSignatories = ConstU16<3>; + type MaxSignatories = ConstU32<3>; type WeightInfo = (); } diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index ebd8dda81adbc..81cec1c295c30 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -143,7 +143,7 @@ impl pallet_balances::Config for Test { parameter_types! { pub const MultisigDepositBase: u64 = 1; pub const MultisigDepositFactor: u64 = 1; - pub const MaxSignatories: u16 = 3; + pub const MaxSignatories: u32 = 3; } impl example::Config for Test {} diff --git a/primitives/core/src/lib.rs b/primitives/core/src/lib.rs index 795fde924bcb3..fda7604d5337f 100644 --- a/primitives/core/src/lib.rs +++ b/primitives/core/src/lib.rs @@ -419,14 +419,6 @@ impl Get for GetDefault { } } -/// A simple wrapper to turn a `Get` to `Get`. -pub struct GetU16ToU32(core::marker::PhantomData); -impl> Get for GetU16ToU32 { - fn get() -> u32 { - T::get().into() - } -} - macro_rules! impl_const_get { ($name:ident, $t:ty) => { #[doc = "Const getter for a basic type."]