diff --git a/frame/multisig/src/benchmarking.rs b/frame/multisig/src/benchmarking.rs index fa2ec52e6b2c5..55bb880fb5582 100644 --- a/frame/multisig/src/benchmarking.rs +++ b/frame/multisig/src/benchmarking.rs @@ -55,7 +55,16 @@ benchmarks! { let z in 0 .. 10_000; let (mut signatories, call) = setup_multi::(s, z)?; let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; - }: as_multi(RawOrigin::Signed(caller), s as u16, signatories, None, call) + }: as_multi(RawOrigin::Signed(caller), s as u16, signatories, None, call, false) + + as_multi_create_store { + // Signatories, need at least 2 total people + let s in 2 .. T::MaxSignatories::get() as u32; + // Transaction Length + let z in 0 .. 10_000; + let (mut signatories, call) = setup_multi::(s, z)?; + let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; + }: as_multi(RawOrigin::Signed(caller), s as u16, signatories, None, call, true) as_multi_approve { // Signatories, need at least 2 people @@ -68,9 +77,9 @@ benchmarks! { // before the call, get the timepoint let timepoint = Multisig::::timepoint(); // Create the multi - Multisig::::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone())?; + Multisig::::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone(), false)?; let caller2 = signatories2.remove(0); - }: as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call) + }: as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call, false) as_multi_complete { // Signatories, need at least 2 people @@ -83,16 +92,16 @@ benchmarks! { // before the call, get the timepoint let timepoint = Multisig::::timepoint(); // Create the multi - Multisig::::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone())?; + Multisig::::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone(), false)?; // Everyone except the first person approves for i in 1 .. s - 1 { let mut signatories_loop = signatories2.clone(); let caller_loop = signatories_loop.remove(i as usize); let o = RawOrigin::Signed(caller_loop).into(); - Multisig::::as_multi(o, s as u16, signatories_loop, Some(timepoint), call.clone())?; + Multisig::::as_multi(o, s as u16, signatories_loop, Some(timepoint), call.clone(), false)?; } let caller2 = signatories2.remove(0); - }: as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call) + }: as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call, false) approve_as_multi_create { // Signatories, need at least 2 people @@ -117,7 +126,30 @@ benchmarks! { // before the call, get the timepoint let timepoint = Multisig::::timepoint(); // Create the multi - Multisig::::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone())?; + Multisig::::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone(), false)?; + let caller2 = signatories2.remove(0); + }: approve_as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call_hash) + + approve_as_multi_complete { + // Signatories, need at least 2 people + let s in 2 .. T::MaxSignatories::get() as u32; + // Transaction Length + let z in 0 .. 10_000; + let (mut signatories, call) = setup_multi::(s, z)?; + let mut signatories2 = signatories.clone(); + let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; + let call_hash = call.using_encoded(blake2_256); + // before the call, get the timepoint + let timepoint = Multisig::::timepoint(); + // Create the multi + Multisig::::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone(), true)?; + // Everyone except the first person approves + for i in 1 .. s - 1 { + let mut signatories_loop = signatories2.clone(); + let caller_loop = signatories_loop.remove(i as usize); + let o = RawOrigin::Signed(caller_loop).into(); + Multisig::::as_multi(o, s as u16, signatories_loop, Some(timepoint), call.clone(), false)?; + } let caller2 = signatories2.remove(0); }: approve_as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call_hash) @@ -132,8 +164,22 @@ benchmarks! { let timepoint = Multisig::::timepoint(); // Create the multi let o = RawOrigin::Signed(caller.clone()).into(); - Multisig::::as_multi(o, s as u16, signatories.clone(), None, call.clone())?; + Multisig::::as_multi(o, s as u16, signatories.clone(), None, call.clone(), false)?; }: _(RawOrigin::Signed(caller), s as u16, signatories, timepoint, call_hash) + + cancel_as_multi_store { + // Signatories, need at least 2 people + let s in 2 .. T::MaxSignatories::get() as u32; + // Transaction Length + let z in 0 .. 10_000; + let (mut signatories, call) = setup_multi::(s, z)?; + let caller = signatories.pop().ok_or("signatories should have len 2 or more")?; + let call_hash = call.using_encoded(blake2_256); + let timepoint = Multisig::::timepoint(); + // Create the multi + let o = RawOrigin::Signed(caller.clone()).into(); + Multisig::::as_multi(o, s as u16, signatories.clone(), None, call.clone(), true)?; + }: cancel_as_multi(RawOrigin::Signed(caller), s as u16, signatories, timepoint, call_hash) } #[cfg(test)] diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index bde0a06de60fb..4ca215b6c2a14 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -48,14 +48,16 @@ use sp_std::prelude::*; use codec::{Encode, Decode}; -use sp_io::hashing::blake2_256; use frame_support::{decl_module, decl_event, decl_error, decl_storage, Parameter, ensure, RuntimeDebug}; use frame_support::{traits::{Get, ReservableCurrency, Currency, Filter, FilterStack, ClearFilterGuard}, weights::{Weight, GetDispatchInfo, DispatchClass, Pays}, - dispatch::{DispatchResultWithPostInfo, DispatchErrorWithPostInfo, PostDispatchInfo}, + dispatch::{DispatchResultWithPostInfo, PostDispatchInfo}, }; use frame_system::{self as system, ensure_signed}; -use sp_runtime::{DispatchError, DispatchResult, traits::Dispatchable}; +use sp_runtime::{ + DispatchError, DispatchResult, + traits::{Dispatchable, Hash}, +}; mod tests; mod benchmarking; @@ -120,8 +122,10 @@ decl_storage! { trait Store for Module as Multisig { /// The set of open multisig operations. pub Multisigs: double_map - hasher(twox_64_concat) T::AccountId, hasher(blake2_128_concat) [u8; 32] + hasher(twox_64_concat) T::AccountId, hasher(blake2_128_concat) T::Hash => Option, T::AccountId>>; + + pub Calls: map hasher(identity) T::Hash => Option<(Vec, T::AccountId, BalanceOf)>; } } @@ -145,14 +149,14 @@ decl_error! { NotFound, /// Only the account that originally created the multisig is able to cancel it. NotOwner, - /// No timepoint was given, yet the multisig operation is already underway. - NoTimepoint, /// A different timepoint was given to the multisig operation that is underway. WrongTimepoint, - /// A timepoint was given, yet no multisig operation is underway. - UnexpectedTimepoint, /// A call with a `false` `IsCallable` filter was attempted. Uncallable, + /// Call needs more approvals. + NotApproved, + /// Call information is missing. + MissingCall, } } @@ -161,7 +165,7 @@ decl_event! { pub enum Event where AccountId = ::AccountId, BlockNumber = ::BlockNumber, - CallHash = [u8; 32] + CallHash = ::Hash { /// A new multisig operation has begun. First param is the account that is approving, /// second is the multisig account, third is hash of the call. @@ -180,25 +184,6 @@ decl_event! { } } -mod weight_of { - use super::*; - - /// - Base Weight: - /// - Create: 46.55 + 0.089 * S µs - /// - Approve: 34.03 + .112 * S µs - /// - Complete: 40.36 + .225 * S µs - /// - DB Weight: - /// - Reads: Multisig Storage, [Caller Account] - /// - Writes: Multisig Storage, [Caller Account] - /// - Plus Call Weight - pub fn as_multi(other_sig_len: usize, call_weight: Weight) -> Weight { - call_weight - .saturating_add(45_000_000) - .saturating_add((other_sig_len as Weight).saturating_mul(250_000)) - .saturating_add(T::DbWeight::get().reads_writes(1, 1)) - } -} - decl_module! { pub struct Module for enum Call where origin: T::Origin { type Error = Error; @@ -268,95 +253,43 @@ decl_module! { /// - Writes: Multisig Storage, [Caller Account] /// - Plus Call Weight /// # - #[weight = ( - weight_of::as_multi::(other_signatories.len(), call.get_dispatch_info().weight), - call.get_dispatch_info().class, - Pays::Yes, - )] - fn as_multi(origin, + #[weight = 0] + fn create(origin, threshold: u16, other_signatories: Vec, - maybe_timepoint: Option>, + call_hash: T::Hash, + ) -> DispatchResult { + let who = ensure_signed(origin)?; + Self::ensure_valid_input(threshold, &other_signatories)?; + let signatories = Self::ensure_sorted_and_insert(other_signatories, who.clone())?; + + let id = Self::multi_account_id(&signatories, threshold); + + let deposit = T::DepositBase::get() + + T::DepositFactor::get() * threshold.into(); + T::Currency::reserve(&who, deposit)?; + >::insert(&id, call_hash, Multisig { + when: Self::timepoint(), + deposit, + depositor: who.clone(), + approvals: vec![who.clone()], + }); + Self::deposit_event(RawEvent::NewMultisig(who, id, call_hash)); + Ok(()) + } + + #[weight = 0] + fn note_preimage(origin, call: Box<::Call>, - ) -> DispatchResultWithPostInfo { + ) { let who = ensure_signed(origin)?; // We're now executing as a freshly authenticated new account, so the previous call // restrictions no longer apply. let _guard = ClearFilterGuard::::Call>::new(); ensure!(T::IsCallable::filter(call.as_ref()), Error::::Uncallable); - ensure!(threshold >= 1, Error::::ZeroThreshold); - let max_sigs = T::MaxSignatories::get() as usize; - ensure!(!other_signatories.is_empty(), Error::::TooFewSignatories); - let other_signatories_len = other_signatories.len(); - ensure!(other_signatories_len < max_sigs, Error::::TooManySignatories); - let signatories = Self::ensure_sorted_and_insert(other_signatories, who.clone())?; - - let id = Self::multi_account_id(&signatories, threshold); - let call_hash = call.using_encoded(blake2_256); - - if let Some(mut m) = >::get(&id, call_hash) { - let timepoint = maybe_timepoint.ok_or(Error::::NoTimepoint)?; - ensure!(m.when == timepoint, Error::::WrongTimepoint); - if let Err(pos) = m.approvals.binary_search(&who) { - // we know threshold is greater than zero from the above ensure. - if (m.approvals.len() as u16) < threshold - 1 { - m.approvals.insert(pos, who.clone()); - >::insert(&id, call_hash, m); - Self::deposit_event(RawEvent::MultisigApproval(who, timepoint, id, call_hash)); - // Call is not made, so the actual weight does not include call - return Ok(Some(weight_of::as_multi::(other_signatories_len, 0)).into()) - } - } else { - if (m.approvals.len() as u16) < threshold { - Err(Error::::AlreadyApproved)? - } - } - - let result = call.dispatch(frame_system::RawOrigin::Signed(id.clone()).into()); - let _ = T::Currency::unreserve(&m.depositor, m.deposit); - >::remove(&id, call_hash); - Self::deposit_event(RawEvent::MultisigExecuted( - who, timepoint, id, call_hash, result.map(|_| ()).map_err(|e| e.error) - )); - return Ok(None.into()) - } else { - ensure!(maybe_timepoint.is_none(), Error::::UnexpectedTimepoint); - if threshold > 1 { - let deposit = T::DepositBase::get() - + T::DepositFactor::get() * threshold.into(); - T::Currency::reserve(&who, deposit)?; - >::insert(&id, call_hash, Multisig { - when: Self::timepoint(), - deposit, - depositor: who.clone(), - approvals: vec![who.clone()], - }); - Self::deposit_event(RawEvent::NewMultisig(who, id, call_hash)); - // Call is not made, so we can return that weight - return Ok(Some(weight_of::as_multi::(other_signatories_len, 0)).into()) - } else { - let result = call.dispatch(frame_system::RawOrigin::Signed(id).into()); - match result { - Ok(post_dispatch_info) => { - match post_dispatch_info.actual_weight { - Some(actual_weight) => return Ok(Some(weight_of::as_multi::(other_signatories_len, actual_weight)).into()), - None => return Ok(None.into()), - } - }, - Err(err) => { - match err.post_info.actual_weight { - Some(actual_weight) => { - let weight_used = weight_of::as_multi::(other_signatories_len, actual_weight); - return Err(DispatchErrorWithPostInfo { post_info: Some(weight_used).into(), error: err.error.into() }) - }, - None => { - return Err(err) - } - } - } - } - } - } + let encoded_call = call.encode(); + let call_hash = T::Hashing::hash(&encoded_call); + Self::store_call(who.clone(), &call_hash, encoded_call)?; } /// Register approval for a dispatch to be made from a deterministic composite account if @@ -405,52 +338,61 @@ decl_module! { DispatchClass::Normal, Pays::Yes, )] - fn approve_as_multi(origin, + fn approve(origin, threshold: u16, other_signatories: Vec, - maybe_timepoint: Option>, - call_hash: [u8; 32], + timepoint: Timepoint, + call_hash: T::Hash, ) -> DispatchResult { let who = ensure_signed(origin)?; - ensure!(threshold >= 1, Error::::ZeroThreshold); - let max_sigs = T::MaxSignatories::get() as usize; - ensure!(!other_signatories.is_empty(), Error::::TooFewSignatories); - ensure!(other_signatories.len() < max_sigs, Error::::TooManySignatories); + Self::ensure_valid_input(threshold, &other_signatories)?; let signatories = Self::ensure_sorted_and_insert(other_signatories, who.clone())?; let id = Self::multi_account_id(&signatories, threshold); - if let Some(mut m) = >::get(&id, call_hash) { - let timepoint = maybe_timepoint.ok_or(Error::::NoTimepoint)?; - ensure!(m.when == timepoint, Error::::WrongTimepoint); - ensure!(m.approvals.len() < threshold as usize, Error::::NoApprovalsNeeded); - if let Err(pos) = m.approvals.binary_search(&who) { - m.approvals.insert(pos, who.clone()); - >::insert(&id, call_hash, m); - Self::deposit_event(RawEvent::MultisigApproval(who, timepoint, id, call_hash)); - } else { - Err(Error::::AlreadyApproved)? - } + let mut m = >::get(&id, call_hash).ok_or(Error::::NotFound)?; + ensure!(m.when == timepoint, Error::::WrongTimepoint); + ensure!(m.approvals.len() < threshold as usize, Error::::NoApprovalsNeeded); + + if let Err(pos) = m.approvals.binary_search(&who) { + m.approvals.insert(pos, who.clone()); + >::insert(&id, call_hash, m); + Self::deposit_event(RawEvent::MultisigApproval(who, timepoint, id, call_hash)); } else { - if threshold > 1 { - ensure!(maybe_timepoint.is_none(), Error::::UnexpectedTimepoint); - let deposit = T::DepositBase::get() - + T::DepositFactor::get() * threshold.into(); - T::Currency::reserve(&who, deposit)?; - >::insert(&id, call_hash, Multisig { - when: Self::timepoint(), - deposit, - depositor: who.clone(), - approvals: vec![who.clone()], - }); - Self::deposit_event(RawEvent::NewMultisig(who, id, call_hash)); - } else { - Err(Error::::NoApprovalsNeeded)? - } + Err(Error::::AlreadyApproved)? } Ok(()) } + #[weight = 0] + fn complete(origin, + threshold: u16, + other_signatories: Vec, + timepoint: Timepoint, + call_hash: T::Hash, + ) -> DispatchResultWithPostInfo { + let who = ensure_signed(origin)?; + Self::ensure_valid_input(threshold, &other_signatories)?; + let signatories = Self::ensure_sorted_and_insert(other_signatories, who.clone())?; + + let id = Self::multi_account_id(&signatories, threshold); + + let m = >::get(&id, call_hash).ok_or(Error::::NotFound)?; + ensure!(m.when == timepoint, Error::::WrongTimepoint); + ensure!(m.approvals.len() as u16 >= threshold, Error::::NotApproved); + + // TODO: Do we need to revalidate call is callable? + let call = Self::take_call(&call_hash).ok_or(Error::::MissingCall)?; + + let result = call.dispatch(frame_system::RawOrigin::Signed(id.clone()).into()); + let _ = T::Currency::unreserve(&m.depositor, m.deposit); + >::remove(&id, call_hash); + Self::deposit_event(RawEvent::MultisigExecuted( + who, timepoint, id, call_hash, result.map(|_| ()).map_err(|e| e.error) + )); + return Ok(None.into()) + } + /// Cancel a pre-existing, on-going multisig transaction. Any deposit reserved previously /// for this operation will be unreserved on success. /// @@ -485,17 +427,14 @@ decl_module! { DispatchClass::Normal, Pays::Yes, )] - fn cancel_as_multi(origin, + fn cancel(origin, threshold: u16, other_signatories: Vec, timepoint: Timepoint, - call_hash: [u8; 32], + call_hash: T::Hash, ) -> DispatchResult { let who = ensure_signed(origin)?; - ensure!(threshold >= 1, Error::::ZeroThreshold); - let max_sigs = T::MaxSignatories::get() as usize; - ensure!(!other_signatories.is_empty(), Error::::TooFewSignatories); - ensure!(other_signatories.len() < max_sigs, Error::::TooManySignatories); + Self::ensure_valid_input(threshold, &other_signatories)?; let signatories = Self::ensure_sorted_and_insert(other_signatories, who.clone())?; let id = Self::multi_account_id(&signatories, threshold); @@ -506,7 +445,8 @@ decl_module! { ensure!(m.depositor == who, Error::::NotOwner); let _ = T::Currency::unreserve(&m.depositor, m.deposit); - >::remove(&id, call_hash); + >::remove(&id, &call_hash); + Self::take_call(&call_hash); Self::deposit_event(RawEvent::MultisigCancelled(who, timepoint, id, call_hash)); Ok(()) @@ -520,8 +460,30 @@ impl Module { /// /// NOTE: `who` must be sorted. If it is not, then you'll get the wrong answer. pub fn multi_account_id(who: &[T::AccountId], threshold: u16) -> T::AccountId { - let entropy = (b"modlpy/utilisuba", who, threshold).using_encoded(blake2_256); - T::AccountId::decode(&mut &entropy[..]).unwrap_or_default() + let entropy = (b"modlpy/utilisuba", who, threshold).using_encoded(T::Hashing::hash); + T::AccountId::decode(&mut &entropy.as_ref()[..]).unwrap_or_default() + } + + /// Place a call's encoded data in storage, reserving funds as appropriate. + fn store_call(who: T::AccountId, hash: &T::Hash, data: Vec) -> DispatchResult { + if !Calls::::contains_key(hash) { + let deposit = T::DepositBase::get() + + T::DepositFactor::get() * BalanceOf::::from(((data.len() + 31) / 32) as u32); + T::Currency::reserve(&who, deposit)?; + // we store `data` here because storing `call` would result in needing another `.encode`. + Calls::::insert(&hash, (data, who, deposit)); + } + Ok(()) + } + + /// Attempt to remove a call from storage, returning it and any deposit on it to the owner. + fn take_call(hash: &T::Hash) -> Option<::Call> { + if let Some((data, who, deposit)) = Calls::::take(hash) { + T::Currency::unreserve(&who, deposit); + Decode::decode(&mut &data[..]).ok() + } else { + None + } } /// The current `Timepoint`. @@ -532,6 +494,15 @@ impl Module { } } + /// Validate the user's input is sane before doing heavy runtime operations. + fn ensure_valid_input(threshold: u16, other_signatories: &[T::AccountId]) -> DispatchResult { + ensure!(threshold >= 1, Error::::ZeroThreshold); + let max_sigs = T::MaxSignatories::get() as usize; + ensure!(!other_signatories.is_empty(), Error::::TooFewSignatories); + ensure!(other_signatories.len() < max_sigs, Error::::TooManySignatories); + Ok(()) + } + /// Check that signatories is sorted and doesn't contain sender, then insert sender. fn ensure_sorted_and_insert(other_signatories: Vec, who: T::AccountId) -> Result, DispatchError> diff --git a/frame/multisig/src/tests.rs b/frame/multisig/src/tests.rs index 067ef4cf98e68..a72ed87401c7d 100644 --- a/frame/multisig/src/tests.rs +++ b/frame/multisig/src/tests.rs @@ -156,7 +156,7 @@ fn now() -> Timepoint { } #[test] -fn multisig_deposit_is_taken_and_returned() { +fn multisig_deposit_is_taken_and_returned_when_approved() { new_test_ext().execute_with(|| { let multi = Multisig::multi_account_id(&[1, 2, 3][..], 2); assert_ok!(Balances::transfer(Origin::signed(1), multi, 5)); @@ -164,35 +164,41 @@ fn multisig_deposit_is_taken_and_returned() { assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call.clone())); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); assert_eq!(Balances::free_balance(1), 2); assert_eq!(Balances::reserved_balance(1), 3); - assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call)); + assert_ok!(Multisig::approve(Origin::signed(2), 2, vec![1, 3], now(), hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(2), call)); + assert_ok!(Multisig::complete(Origin::signed(2), 2, vec![1, 3], now(), hash)); assert_eq!(Balances::free_balance(1), 5); assert_eq!(Balances::reserved_balance(1), 0); }); } #[test] -fn cancel_multisig_returns_deposit() { +fn multisig_deposit_is_taken_and_returned_when_canceled() { new_test_ext().execute_with(|| { + let multi = Multisig::multi_account_id(&[1, 2, 3][..], 2); + assert_ok!(Balances::transfer(Origin::signed(1), multi, 5)); + assert_ok!(Balances::transfer(Origin::signed(2), multi, 5)); + assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); + let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); - let hash = call.using_encoded(blake2_256); - assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 3, vec![2, 3], None, hash.clone())); - assert_ok!(Multisig::approve_as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), hash.clone())); - assert_eq!(Balances::free_balance(1), 6); - assert_eq!(Balances::reserved_balance(1), 4); - assert_ok!( - Multisig::cancel_as_multi(Origin::signed(1), 3, vec![2, 3], now(), hash.clone()), - ); - assert_eq!(Balances::free_balance(1), 10); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); + assert_eq!(Balances::free_balance(1), 2); + assert_eq!(Balances::reserved_balance(1), 3); + + assert_ok!(Multisig::cancel(Origin::signed(1), 2, vec![2, 3], now(), hash)); + assert_eq!(Balances::free_balance(1), 5); assert_eq!(Balances::reserved_balance(1), 0); }); } #[test] -fn timepoint_checking_works() { +fn multisig_deposit_is_taken_and_returned_when_approved_with_call_storage() { new_test_ext().execute_with(|| { let multi = Multisig::multi_account_id(&[1, 2, 3][..], 2); assert_ok!(Balances::transfer(Origin::signed(1), multi, 5)); @@ -200,27 +206,114 @@ fn timepoint_checking_works() { assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); - let hash = call.using_encoded(blake2_256); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(1), call)); + assert_eq!(Balances::free_balance(1), 0); + assert_eq!(Balances::reserved_balance(1), 5); + + assert_ok!(Multisig::approve(Origin::signed(2), 2, vec![1, 3], now(), hash)); + assert_ok!(Multisig::complete(Origin::signed(2), 2, vec![1, 3], now(), hash)); + assert_eq!(Balances::free_balance(1), 5); + assert_eq!(Balances::reserved_balance(1), 0); + }); +} - assert_noop!( - Multisig::approve_as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), hash.clone()), - Error::::UnexpectedTimepoint, - ); +#[test] +fn multisig_deposit_is_taken_and_returned_when_canceled_with_call_storage() { + new_test_ext().execute_with(|| { + let multi = Multisig::multi_account_id(&[1, 2, 3][..], 2); + assert_ok!(Balances::transfer(Origin::signed(1), multi, 5)); + assert_ok!(Balances::transfer(Origin::signed(2), multi, 5)); + assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); - assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 2, vec![2, 3], None, hash)); + let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(1), call)); + assert_eq!(Balances::free_balance(1), 0); + assert_eq!(Balances::reserved_balance(1), 5); + + assert_ok!(Multisig::cancel(Origin::signed(1), 2, vec![2, 3], now(), hash)); + assert_eq!(Balances::free_balance(1), 5); + assert_eq!(Balances::reserved_balance(1), 0); + }); +} + +// TODO: This test may not make sense with the new logic +// #[test] +// fn multisig_deposit_is_taken_and_returned_with_alt_call_storage() { +// new_test_ext().execute_with(|| { +// let multi = Multisig::multi_account_id(&[1, 2, 3][..], 3); +// assert_ok!(Balances::transfer(Origin::signed(1), multi, 5)); +// assert_ok!(Balances::transfer(Origin::signed(2), multi, 5)); +// assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); + +// let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); +// let hash = call.using_encoded(::Hashing::hash); + +// assert_ok!(Multisig::create(Origin::signed(1), 3, vec![2, 3], None, hash)); +// assert_eq!(Balances::free_balance(1), 1); +// assert_eq!(Balances::reserved_balance(1), 4); + +// assert_ok!(Multisig::approve(Origin::signed(2), 3, vec![1, 3], now(), call, true)); +// assert_noop!(Multisig::complete(Origin::signed(2), 3, vec![1, 3], now(), call), Error::::NotApproved); +// assert_eq!(Balances::free_balance(2), 3); +// assert_eq!(Balances::reserved_balance(2), 2); +// assert_eq!(Balances::free_balance(1), 1); +// assert_eq!(Balances::reserved_balance(1), 4); + +// assert_ok!(Multisig::approve(Origin::signed(3), 3, vec![1, 2], now(), hash)); +// assert_ok!(Multisig::complete(Origin::signed(3), 3, vec![1, 2], now(), call)); +// assert_eq!(Balances::free_balance(1), 5); +// assert_eq!(Balances::reserved_balance(1), 0); +// assert_eq!(Balances::free_balance(2), 5); +// assert_eq!(Balances::reserved_balance(2), 0); +// }); +// } + +#[test] +fn timepoint_checking_works() { + new_test_ext().execute_with(|| { + let multi = Multisig::multi_account_id(&[1, 2, 3][..], 2); + assert_ok!(Balances::transfer(Origin::signed(1), multi, 5)); + assert_ok!(Balances::transfer(Origin::signed(2), multi, 5)); + assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); + + let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); + let hash = call.using_encoded(::Hashing::hash); + + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(1), call)); - assert_noop!( - Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], None, call.clone()), - Error::::NoTimepoint, - ); let later = Timepoint { index: 1, .. now() }; assert_noop!( - Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(later), call.clone()), + Multisig::approve(Origin::signed(2), 2, vec![1, 3], later, hash), Error::::WrongTimepoint, ); }); } +#[test] +fn multisig_2_of_3_works_with_call_storing() { + new_test_ext().execute_with(|| { + let multi = Multisig::multi_account_id(&[1, 2, 3][..], 2); + assert_ok!(Balances::transfer(Origin::signed(1), multi, 5)); + assert_ok!(Balances::transfer(Origin::signed(2), multi, 5)); + assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); + + let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(1), call)); + assert_eq!(Balances::free_balance(6), 0); + + assert_ok!(Multisig::approve(Origin::signed(2), 2, vec![1, 3], now(), hash)); + assert_ok!(Multisig::complete(Origin::signed(2), 2, vec![1, 3], now(), hash)); + assert_eq!(Balances::free_balance(6), 15); + }); +} + #[test] fn multisig_2_of_3_works() { new_test_ext().execute_with(|| { @@ -230,11 +323,13 @@ fn multisig_2_of_3_works() { assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); - let hash = call.using_encoded(blake2_256); - assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 2, vec![2, 3], None, hash)); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(1), call)); assert_eq!(Balances::free_balance(6), 0); - assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call)); + assert_ok!(Multisig::approve(Origin::signed(2), 2, vec![1, 3], now(), hash)); + assert_ok!(Multisig::complete(Origin::signed(2), 2, vec![1, 3], now(), hash)); assert_eq!(Balances::free_balance(6), 15); }); } @@ -248,12 +343,14 @@ fn multisig_3_of_3_works() { assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); - let hash = call.using_encoded(blake2_256); - assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 3, vec![2, 3], None, hash.clone())); - assert_ok!(Multisig::approve_as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), hash.clone())); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 3, vec![2, 3], hash)); + assert_ok!(Multisig::approve(Origin::signed(2), 3, vec![1, 3], now(), hash)); assert_eq!(Balances::free_balance(6), 0); - assert_ok!(Multisig::as_multi(Origin::signed(3), 3, vec![1, 2], Some(now()), call)); + assert_ok!(Multisig::approve(Origin::signed(3), 3, vec![1, 2], now(), hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(3), call)); + assert_ok!(Multisig::complete(Origin::signed(3), 3, vec![1, 2], now(), hash)); assert_eq!(Balances::free_balance(6), 15); }); } @@ -262,33 +359,52 @@ fn multisig_3_of_3_works() { fn cancel_multisig_works() { new_test_ext().execute_with(|| { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); - let hash = call.using_encoded(blake2_256); - assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 3, vec![2, 3], None, hash.clone())); - assert_ok!(Multisig::approve_as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), hash.clone())); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 3, vec![2, 3], hash)); + assert_ok!(Multisig::approve(Origin::signed(2), 3, vec![1, 3], now(), hash)); assert_noop!( - Multisig::cancel_as_multi(Origin::signed(2), 3, vec![1, 3], now(), hash.clone()), + Multisig::cancel(Origin::signed(2), 3, vec![1, 3], now(), hash), Error::::NotOwner, ); assert_ok!( - Multisig::cancel_as_multi(Origin::signed(1), 3, vec![2, 3], now(), hash.clone()), + Multisig::cancel(Origin::signed(1), 3, vec![2, 3], now(), hash), ); }); } #[test] -fn multisig_2_of_3_as_multi_works() { +fn cancel_multisig_with_call_storage_works() { new_test_ext().execute_with(|| { - let multi = Multisig::multi_account_id(&[1, 2, 3][..], 2); - assert_ok!(Balances::transfer(Origin::signed(1), multi, 5)); - assert_ok!(Balances::transfer(Origin::signed(2), multi, 5)); - assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); - let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call.clone())); - assert_eq!(Balances::free_balance(6), 0); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 3, vec![2, 3], hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(1), call)); + assert_eq!(Balances::free_balance(1), 4); + assert_ok!(Multisig::approve(Origin::signed(2), 3, vec![1, 3], now(), hash)); + assert_noop!( + Multisig::cancel(Origin::signed(2), 3, vec![1, 3], now(), hash), + Error::::NotOwner, + ); + assert_ok!( + Multisig::cancel(Origin::signed(1), 3, vec![2, 3], now(), hash), + ); + assert_eq!(Balances::free_balance(1), 10); + }); +} - assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call)); - assert_eq!(Balances::free_balance(6), 15); +#[test] +fn cancel_multisig_with_alt_call_storage_works() { + new_test_ext().execute_with(|| { + let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 3, vec![2, 3], hash)); + assert_eq!(Balances::free_balance(1), 6); + assert_ok!(Multisig::approve(Origin::signed(2), 3, vec![1, 3], now(), hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(2), call)); + assert_eq!(Balances::free_balance(2), 8); + assert_ok!(Multisig::cancel(Origin::signed(1), 3, vec![2, 3], now(), hash)); + assert_eq!(Balances::free_balance(1), 10); + assert_eq!(Balances::free_balance(2), 10); }); } @@ -301,18 +417,26 @@ fn multisig_2_of_3_as_multi_with_many_calls_works() { assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); let call1 = Box::new(Call::Balances(BalancesCall::transfer(6, 10))); + let hash1 = call1.using_encoded(::Hashing::hash); + let call2 = Box::new(Call::Balances(BalancesCall::transfer(7, 5))); + let hash2 = call2.using_encoded(::Hashing::hash); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call1.clone())); - assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], None, call2.clone())); - assert_ok!(Multisig::as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), call2)); - assert_ok!(Multisig::as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), call1)); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash1)); + assert_ok!(Multisig::create(Origin::signed(2), 2, vec![1, 3], hash2)); + assert_ok!(Multisig::approve(Origin::signed(3), 2, vec![1, 2], now(), hash1)); + assert_ok!(Multisig::approve(Origin::signed(3), 2, vec![1, 2], now(), hash2)); + assert_ok!(Multisig::note_preimage(Origin::signed(3), call1)); + assert_ok!(Multisig::note_preimage(Origin::signed(3), call2)); + assert_ok!(Multisig::complete(Origin::signed(1), 2, vec![2, 3], now(), hash1)); + assert_ok!(Multisig::complete(Origin::signed(2), 2, vec![1, 3], now(), hash2)); assert_eq!(Balances::free_balance(6), 10); assert_eq!(Balances::free_balance(7), 5); }); } +// TODO: This test is weird? #[test] fn multisig_2_of_3_cannot_reissue_same_call() { new_test_ext().execute_with(|| { @@ -322,15 +446,20 @@ fn multisig_2_of_3_cannot_reissue_same_call() { assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); let call = Box::new(Call::Balances(BalancesCall::transfer(6, 10))); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call.clone())); - assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call.clone())); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); + assert_ok!(Multisig::approve(Origin::signed(2), 2, vec![1, 3], now(), hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(2), call.clone())); + assert_ok!(Multisig::complete(Origin::signed(2), 2, vec![1, 3], now(), hash)); assert_eq!(Balances::free_balance(multi), 5); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call.clone())); - assert_ok!(Multisig::as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), call.clone())); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); + assert_ok!(Multisig::approve(Origin::signed(3), 2, vec![1, 2], now(), hash)); + assert_ok!(Multisig::note_preimage(Origin::signed(3), call)); + assert_ok!(Multisig::complete(Origin::signed(3), 2, vec![1, 2], now(), hash)); let err = DispatchError::from(BalancesError::::InsufficientBalance).stripped(); - expect_event(RawEvent::MultisigExecuted(3, now(), multi, call.using_encoded(blake2_256), Err(err))); + expect_event(RawEvent::MultisigExecuted(3, now(), multi, hash, Err(err))); }); } @@ -338,8 +467,9 @@ fn multisig_2_of_3_cannot_reissue_same_call() { fn zero_threshold_fails() { new_test_ext().execute_with(|| { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); + let hash = call.using_encoded(::Hashing::hash); assert_noop!( - Multisig::as_multi(Origin::signed(1), 0, vec![2], None, call), + Multisig::create(Origin::signed(1), 0, vec![2], hash), Error::::ZeroThreshold, ); }); @@ -349,8 +479,9 @@ fn zero_threshold_fails() { fn too_many_signatories_fails() { new_test_ext().execute_with(|| { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); + let hash = call.using_encoded(::Hashing::hash); assert_noop!( - Multisig::as_multi(Origin::signed(1), 2, vec![2, 3, 4], None, call.clone()), + Multisig::create(Origin::signed(1), 2, vec![2, 3, 4], hash), Error::::TooManySignatories, ); }); @@ -360,15 +491,15 @@ fn too_many_signatories_fails() { fn duplicate_approvals_are_ignored() { new_test_ext().execute_with(|| { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); - let hash = call.using_encoded(blake2_256); - assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 2, vec![2, 3], None, hash.clone())); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); assert_noop!( - Multisig::approve_as_multi(Origin::signed(1), 2, vec![2, 3], Some(now()), hash.clone()), + Multisig::approve(Origin::signed(1), 2, vec![2, 3], now(), hash), Error::::AlreadyApproved, ); - assert_ok!(Multisig::approve_as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), hash.clone())); + assert_ok!(Multisig::approve(Origin::signed(2), 2, vec![1, 3], now(), hash)); assert_noop!( - Multisig::approve_as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), hash.clone()), + Multisig::approve(Origin::signed(3), 2, vec![1, 2], now(), hash), Error::::NoApprovalsNeeded, ); }); @@ -383,16 +514,19 @@ fn multisig_1_of_3_works() { assert_ok!(Balances::transfer(Origin::signed(3), multi, 5)); let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); - let hash = call.using_encoded(blake2_256); + let hash = call.using_encoded(::Hashing::hash); + // TODO: What is this? + // assert_noop!( + // Multisig::create(Origin::signed(4), 1, vec![2, 3], hash), + // BalancesError::::InsufficientBalance, + // ); + assert_ok!(Multisig::create(Origin::signed(1), 1, vec![2, 3], hash)); assert_noop!( - Multisig::approve_as_multi(Origin::signed(1), 1, vec![2, 3], None, hash.clone()), + Multisig::approve(Origin::signed(1), 1, vec![2, 3], now(), hash), Error::::NoApprovalsNeeded, ); - assert_noop!( - Multisig::as_multi(Origin::signed(4), 1, vec![2, 3], None, call.clone()), - BalancesError::::InsufficientBalance, - ); - assert_ok!(Multisig::as_multi(Origin::signed(1), 1, vec![2, 3], None, call)); + assert_ok!(Multisig::note_preimage(Origin::signed(1), call)); + assert_ok!(Multisig::complete(Origin::signed(1), 1, vec![2, 3], now(), hash)); assert_eq!(Balances::free_balance(6), 15); }); @@ -402,8 +536,10 @@ fn multisig_1_of_3_works() { fn multisig_filters() { new_test_ext().execute_with(|| { let call = Box::new(Call::System(frame_system::Call::set_code(vec![]))); + let hash = call.using_encoded(::Hashing::hash); + assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); assert_noop!( - Multisig::as_multi(Origin::signed(1), 1, vec![], None, call.clone()), + Multisig::note_preimage(Origin::signed(1), call), Error::::Uncallable, ); });