Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Make Multisig Pallet Bounded #12457

Merged
merged 3 commits into from
Oct 25, 2022
Merged

Make Multisig Pallet Bounded #12457

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8fd1b67
Bounded multisig
shawntabrizi Oct 9, 2022
f23eb96
just use u32
shawntabrizi Oct 20, 2022
dc0dfe9
Merge remote-tracking branch 'origin/master' into shawntabrizi-bounde…
Oct 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion frame/multisig/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -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",
Expand Down
31 changes: 21 additions & 10 deletions frame/multisig/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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},
Expand Down Expand Up @@ -94,7 +95,9 @@ type BalanceOf<T> =
/// 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<BlockNumber> {
/// The height of the chain at the point in time.
height: BlockNumber,
Expand All @@ -103,16 +106,20 @@ pub struct Timepoint<BlockNumber> {
}

/// An open multisig operation.
#[derive(Clone, Eq, PartialEq, Encode, Decode, Default, RuntimeDebug, TypeInfo)]
pub struct Multisig<BlockNumber, Balance, AccountId> {
#[derive(Clone, Eq, PartialEq, Encode, Decode, Default, RuntimeDebug, TypeInfo, MaxEncodedLen)]
#[scale_info(skip_type_params(MaxApprovals))]
pub struct Multisig<BlockNumber, Balance, AccountId, MaxApprovals>
where
MaxApprovals: Get<u32>,
{
/// The extrinsic when the multisig operation was opened.
when: Timepoint<BlockNumber>,
/// The amount held in reserve of the `depositor`, to be returned once the operation ends.
deposit: Balance,
/// 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<AccountId>,
approvals: BoundedVec<AccountId, MaxApprovals>,
}

type CallHash = [u8; 32];
Expand Down Expand Up @@ -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<T>(_);

Expand All @@ -182,7 +188,7 @@ pub mod pallet {
T::AccountId,
Blake2_128Concat,
[u8; 32],
Multisig<T::BlockNumber, BalanceOf<T>, T::AccountId>,
Multisig<T::BlockNumber, BalanceOf<T>, T::AccountId, GetU16ToU32<T::MaxSignatories>>,
>;

#[pallet::error]
Expand Down Expand Up @@ -601,7 +607,9 @@ impl<T: Config> Pallet<T> {

if let Some(pos) = maybe_pos {
// Record approval.
m.approvals.insert(pos, who.clone());
m.approvals
.try_insert(pos, who.clone())
.map_err(|_| Error::<T>::TooManySignatories)?;
<Multisigs<T>>::insert(&id, call_hash, m);
Self::deposit_event(Event::MultisigApproval {
approving: who,
Expand Down Expand Up @@ -629,14 +637,17 @@ impl<T: Config> Pallet<T> {

T::Currency::reserve(&who, deposit)?;

let initial_approvals =
vec![who.clone()].try_into().map_err(|_| Error::<T>::TooManySignatories)?;

<Multisigs<T>>::insert(
&id,
call_hash,
Multisig {
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 });
Expand Down
8 changes: 8 additions & 0 deletions primitives/core/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,14 @@ impl<T: Default> Get<T> for GetDefault {
}
}

/// A simple wrapper to turn a `Get<u16>` to `Get<u32>`.
pub struct GetU16ToU32<T>(core::marker::PhantomData<T>);
impl<T: Get<u16>> Get<u32> for GetU16ToU32<T> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why MaxSignatories is u16 instead of u32?
Since we have the MEL bound now, i think it can be changed.

Copy link
Contributor

@kianenigma kianenigma Oct 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

impl<T: Into<u32>, G: Get<T>> Get<u32> for G {
  fn get() -> u32 { G::get().into() }
}

and you won't need the struct.

Although I agree about keeping all the bounds u32 preferably.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesnt work. will just make it u32

fn get() -> u32 {
T::get().into()
}
}

macro_rules! impl_const_get {
($name:ident, $t:ty) => {
#[doc = "Const getter for a basic type."]
Expand Down