From c7584c064f0c02b0a06a267a4b04b1e669fe8f51 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 10 Jun 2020 13:48:45 +0200 Subject: [PATCH 1/7] Stored call in multisig --- frame/multisig/src/lib.rs | 46 +++++++++++- frame/multisig/src/tests.rs | 143 ++++++++++++++++++++++++++++++------ 2 files changed, 166 insertions(+), 23 deletions(-) diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index bde0a06de60fb..4e11d6b04a786 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -122,6 +122,8 @@ decl_storage! { pub Multisigs: double_map hasher(twox_64_concat) T::AccountId, hasher(blake2_128_concat) [u8; 32] => Option, T::AccountId>>; + + pub Calls: map hasher(identity) [u8; 32] => Option<(Vec, T::AccountId, BalanceOf)>; } } @@ -278,6 +280,7 @@ decl_module! { other_signatories: Vec, maybe_timepoint: Option>, call: Box<::Call>, + store_call: bool, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; // We're now executing as a freshly authenticated new account, so the previous call @@ -292,7 +295,12 @@ decl_module! { 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); + + let encoded_call = call.encode(); + let call_hash = blake2_256(&encoded_call); + if store_call { + Self::store_call(who.clone(), &call_hash, encoded_call)?; + } if let Some(mut m) = >::get(&id, call_hash) { let timepoint = maybe_timepoint.ok_or(Error::::NoTimepoint)?; @@ -315,6 +323,7 @@ decl_module! { let result = call.dispatch(frame_system::RawOrigin::Signed(id.clone()).into()); let _ = T::Currency::unreserve(&m.depositor, m.deposit); >::remove(&id, call_hash); + Self::take_call(&call_hash); Self::deposit_event(RawEvent::MultisigExecuted( who, timepoint, id, call_hash, result.map(|_| ()).map_err(|e| e.error) )); @@ -425,6 +434,17 @@ decl_module! { ensure!(m.when == timepoint, Error::::WrongTimepoint); ensure!(m.approvals.len() < threshold as usize, Error::::NoApprovalsNeeded); if let Err(pos) = m.approvals.binary_search(&who) { + if m.approvals.len() + 1 == threshold as usize { + if let Some(call) = Self::take_call(&call_hash) { + 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(()); + } + } m.approvals.insert(pos, who.clone()); >::insert(&id, call_hash, m); Self::deposit_event(RawEvent::MultisigApproval(who, timepoint, id, call_hash)); @@ -506,7 +526,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(()) @@ -524,6 +545,27 @@ impl Module { T::AccountId::decode(&mut &entropy[..]).unwrap_or_default() } + /// Please a call in storage, reserving funds as appropriate. + fn store_call(who: T::AccountId, hash: &[u8; 32], 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(()) + } + + fn take_call(hash: &[u8; 32]) -> 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`. pub fn timepoint() -> Timepoint { Timepoint { diff --git a/frame/multisig/src/tests.rs b/frame/multisig/src/tests.rs index 067ef4cf98e68..c33cdabae5ad6 100644 --- a/frame/multisig/src/tests.rs +++ b/frame/multisig/src/tests.rs @@ -164,16 +164,65 @@ 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())); + assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call.clone(), false)); 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::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call, false)); assert_eq!(Balances::free_balance(1), 5); assert_eq!(Balances::reserved_balance(1), 0); }); } +#[test] +fn multisig_deposit_is_taken_and_returned_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)); + + let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); + let hash = call.using_encoded(blake2_256); + assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call, true)); + assert_eq!(Balances::free_balance(1), 0); + assert_eq!(Balances::reserved_balance(1), 5); + + assert_ok!(Multisig::approve_as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), hash)); + assert_eq!(Balances::free_balance(1), 5); + assert_eq!(Balances::reserved_balance(1), 0); + }); +} + +#[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(blake2_256); + + assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 3, vec![2, 3], None, hash.clone())); + assert_eq!(Balances::free_balance(1), 1); + assert_eq!(Balances::reserved_balance(1), 4); + + assert_ok!(Multisig::as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), call, true)); + 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_as_multi(Origin::signed(3), 3, vec![1, 2], Some(now()), hash)); + 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 cancel_multisig_returns_deposit() { new_test_ext().execute_with(|| { @@ -210,17 +259,35 @@ fn timepoint_checking_works() { assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 2, vec![2, 3], None, hash)); assert_noop!( - Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], None, call.clone()), + Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], None, call.clone(), false), 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::as_multi(Origin::signed(2), 2, vec![1, 3], Some(later), call.clone(), false), 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(blake2_256); + assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call, true)); + assert_eq!(Balances::free_balance(6), 0); + + assert_ok!(Multisig::approve_as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), hash)); + assert_eq!(Balances::free_balance(6), 15); + }); +} + #[test] fn multisig_2_of_3_works() { new_test_ext().execute_with(|| { @@ -234,7 +301,7 @@ fn multisig_2_of_3_works() { assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 2, vec![2, 3], None, hash)); 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::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call, false)); assert_eq!(Balances::free_balance(6), 15); }); } @@ -253,7 +320,7 @@ fn multisig_3_of_3_works() { assert_ok!(Multisig::approve_as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), hash.clone())); 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::as_multi(Origin::signed(3), 3, vec![1, 2], Some(now()), call, false)); assert_eq!(Balances::free_balance(6), 15); }); } @@ -275,6 +342,40 @@ fn cancel_multisig_works() { }); } +#[test] +fn cancel_multisig_with_call_storage_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::as_multi(Origin::signed(1), 3, vec![2, 3], None, call, true)); + assert_eq!(Balances::free_balance(1), 4); + assert_ok!(Multisig::approve_as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), hash.clone())); + assert_noop!( + Multisig::cancel_as_multi(Origin::signed(2), 3, vec![1, 3], now(), hash.clone()), + Error::::NotOwner, + ); + assert_ok!( + Multisig::cancel_as_multi(Origin::signed(1), 3, vec![2, 3], now(), hash.clone()), + ); + assert_eq!(Balances::free_balance(1), 10); + }); +} + +#[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(blake2_256); + assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 3, vec![2, 3], None, hash.clone())); + assert_eq!(Balances::free_balance(1), 6); + assert_ok!(Multisig::as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), call, true)); + assert_eq!(Balances::free_balance(2), 8); + assert_ok!(Multisig::cancel_as_multi(Origin::signed(1), 3, vec![2, 3], now(), hash)); + assert_eq!(Balances::free_balance(1), 10); + assert_eq!(Balances::free_balance(2), 10); + }); +} + #[test] fn multisig_2_of_3_as_multi_works() { new_test_ext().execute_with(|| { @@ -284,10 +385,10 @@ fn multisig_2_of_3_as_multi_works() { 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_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call.clone(), false)); 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::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call, false)); assert_eq!(Balances::free_balance(6), 15); }); } @@ -303,10 +404,10 @@ fn multisig_2_of_3_as_multi_with_many_calls_works() { let call1 = Box::new(Call::Balances(BalancesCall::transfer(6, 10))); let call2 = Box::new(Call::Balances(BalancesCall::transfer(7, 5))); - 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::as_multi(Origin::signed(1), 2, vec![2, 3], None, call1.clone(), false)); + assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], None, call2.clone(), false)); + assert_ok!(Multisig::as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), call2, false)); + assert_ok!(Multisig::as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), call1, false)); assert_eq!(Balances::free_balance(6), 10); assert_eq!(Balances::free_balance(7), 5); @@ -322,12 +423,12 @@ 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())); + assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call.clone(), false)); + assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call.clone(), false)); 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::as_multi(Origin::signed(1), 2, vec![2, 3], None, call.clone(), false)); + assert_ok!(Multisig::as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), call.clone(), false)); let err = DispatchError::from(BalancesError::::InsufficientBalance).stripped(); expect_event(RawEvent::MultisigExecuted(3, now(), multi, call.using_encoded(blake2_256), Err(err))); @@ -339,7 +440,7 @@ fn zero_threshold_fails() { new_test_ext().execute_with(|| { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); assert_noop!( - Multisig::as_multi(Origin::signed(1), 0, vec![2], None, call), + Multisig::as_multi(Origin::signed(1), 0, vec![2], None, call, false), Error::::ZeroThreshold, ); }); @@ -350,7 +451,7 @@ fn too_many_signatories_fails() { new_test_ext().execute_with(|| { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); assert_noop!( - Multisig::as_multi(Origin::signed(1), 2, vec![2, 3, 4], None, call.clone()), + Multisig::as_multi(Origin::signed(1), 2, vec![2, 3, 4], None, call.clone(), false), Error::::TooManySignatories, ); }); @@ -389,10 +490,10 @@ fn multisig_1_of_3_works() { Error::::NoApprovalsNeeded, ); assert_noop!( - Multisig::as_multi(Origin::signed(4), 1, vec![2, 3], None, call.clone()), + Multisig::as_multi(Origin::signed(4), 1, vec![2, 3], None, call.clone(), false), BalancesError::::InsufficientBalance, ); - assert_ok!(Multisig::as_multi(Origin::signed(1), 1, vec![2, 3], None, call)); + assert_ok!(Multisig::as_multi(Origin::signed(1), 1, vec![2, 3], None, call, false)); assert_eq!(Balances::free_balance(6), 15); }); @@ -403,7 +504,7 @@ fn multisig_filters() { new_test_ext().execute_with(|| { let call = Box::new(Call::System(frame_system::Call::set_code(vec![]))); assert_noop!( - Multisig::as_multi(Origin::signed(1), 1, vec![], None, call.clone()), + Multisig::as_multi(Origin::signed(1), 1, vec![], None, call.clone(), false), Error::::Uncallable, ); }); From 0ddd158b007bcf0fde49fb8e955b439ef6d5d6e8 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 10 Jun 2020 13:58:15 +0200 Subject: [PATCH 2/7] Docs. --- frame/multisig/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index 4e11d6b04a786..e32137ae3a4d4 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -545,7 +545,7 @@ impl Module { T::AccountId::decode(&mut &entropy[..]).unwrap_or_default() } - /// Please a call in storage, reserving funds as appropriate. + /// Place a call's encoded data in storage, reserving funds as appropriate. fn store_call(who: T::AccountId, hash: &[u8; 32], data: Vec) -> DispatchResult { if !Calls::::contains_key(hash) { let deposit = T::DepositBase::get() @@ -557,6 +557,7 @@ impl Module { Ok(()) } + /// Attempt to remove a call from storage, returning it and any deposit on it to the owner. fn take_call(hash: &[u8; 32]) -> Option<::Call> { if let Some((data, who, deposit)) = Calls::::take(hash) { T::Currency::unreserve(&who, deposit); From c30f54f8cc292a7aac82f5af498516f6f2ae4f83 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 10 Jun 2020 15:03:34 +0200 Subject: [PATCH 3/7] Benchmarks. --- frame/multisig/src/benchmarking.rs | 62 ++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 8 deletions(-) diff --git a/frame/multisig/src/benchmarking.rs b/frame/multisig/src/benchmarking.rs index fa2ec52e6b2c5..5a12f66ad6303 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,7 +164,21 @@ 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)?; }: _(RawOrigin::Signed(caller), s as u16, signatories, timepoint, call_hash) } From dbac9eb6ba5731edec5eb8b9fb96298296aa8856 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Wed, 10 Jun 2020 16:18:42 +0200 Subject: [PATCH 4/7] Fix --- frame/multisig/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/multisig/src/benchmarking.rs b/frame/multisig/src/benchmarking.rs index 5a12f66ad6303..55bb880fb5582 100644 --- a/frame/multisig/src/benchmarking.rs +++ b/frame/multisig/src/benchmarking.rs @@ -179,7 +179,7 @@ benchmarks! { // Create the multi let o = RawOrigin::Signed(caller.clone()).into(); Multisig::::as_multi(o, s as u16, signatories.clone(), None, call.clone(), true)?; - }: _(RawOrigin::Signed(caller), s as u16, signatories, timepoint, call_hash) + }: cancel_as_multi(RawOrigin::Signed(caller), s as u16, signatories, timepoint, call_hash) } #[cfg(test)] From 50d70f572841a189c8a6435557ebff99322c34bd Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 11 Jun 2020 01:43:18 +0200 Subject: [PATCH 5/7] rough refactor --- frame/multisig/src/lib.rs | 201 ++++++++++++------------------ frame/multisig/src/tests.rs | 235 +++++++++++++++++++++--------------- 2 files changed, 211 insertions(+), 225 deletions(-) diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index e32137ae3a4d4..d957078001bd9 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -147,14 +147,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, } } @@ -270,23 +270,13 @@ 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: Box<::Call>, - store_call: bool, + call_hash: [u8; 32], ) -> 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); @@ -296,76 +286,32 @@ decl_module! { 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)); + // Call is not made, so we can return that weight + return Ok(Some(weight_of::as_multi::(other_signatories_len, 0)).into()) + } + + #[weight = 0] + fn note_preimage(origin, + call: Box<::Call>, + ) { + 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); let encoded_call = call.encode(); let call_hash = blake2_256(&encoded_call); - if store_call { - Self::store_call(who.clone(), &call_hash, encoded_call)?; - } - - 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::take_call(&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) - } - } - } - } - } - } + Self::store_call(who.clone(), &call_hash, encoded_call)?; } /// Register approval for a dispatch to be made from a deterministic composite account if @@ -414,10 +360,10 @@ decl_module! { DispatchClass::Normal, Pays::Yes, )] - fn approve_as_multi(origin, + fn approve(origin, threshold: u16, other_signatories: Vec, - maybe_timepoint: Option>, + timepoint: Timepoint, call_hash: [u8; 32], ) -> DispatchResult { let who = ensure_signed(origin)?; @@ -429,48 +375,53 @@ decl_module! { 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) { - if m.approvals.len() + 1 == threshold as usize { - if let Some(call) = Self::take_call(&call_hash) { - 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(()); - } - } - 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: [u8; 32], + ) -> DispatchResultWithPostInfo { + 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); + 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 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. /// @@ -505,7 +456,7 @@ decl_module! { DispatchClass::Normal, Pays::Yes, )] - fn cancel_as_multi(origin, + fn cancel(origin, threshold: u16, other_signatories: Vec, timepoint: Timepoint, diff --git a/frame/multisig/src/tests.rs b/frame/multisig/src/tests.rs index c33cdabae5ad6..ab623fa8da8e3 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,18 +164,21 @@ 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(), false)); + let hash = call.using_encoded(blake2_256); + 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, false)); + 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 multisig_deposit_is_taken_and_returned_with_call_storage() { +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)); @@ -184,62 +187,91 @@ fn multisig_deposit_is_taken_and_returned_with_call_storage() { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); let hash = call.using_encoded(blake2_256); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call, true)); - assert_eq!(Balances::free_balance(1), 0); - assert_eq!(Balances::reserved_balance(1), 5); + 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::approve_as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), hash)); + 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 multisig_deposit_is_taken_and_returned_with_alt_call_storage() { +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][..], 3); + 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::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_as_multi(Origin::signed(1), 3, vec![2, 3], None, hash.clone())); - assert_eq!(Balances::free_balance(1), 1); - assert_eq!(Balances::reserved_balance(1), 4); - - assert_ok!(Multisig::as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), call, true)); - 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_as_multi(Origin::signed(3), 3, vec![1, 2], Some(now()), hash)); + 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_eq!(Balances::free_balance(2), 5); - assert_eq!(Balances::reserved_balance(2), 0); }); } #[test] -fn cancel_multisig_returns_deposit() { +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)); + 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); + 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(blake2_256); + +// 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(|| { @@ -251,20 +283,12 @@ fn timepoint_checking_works() { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); let hash = call.using_encoded(blake2_256); - assert_noop!( - Multisig::approve_as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), hash.clone()), - Error::::UnexpectedTimepoint, - ); - - assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 2, vec![2, 3], None, 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(), false), - Error::::NoTimepoint, - ); let later = Timepoint { index: 1, .. now() }; assert_noop!( - Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(later), call.clone(), false), + Multisig::approve(Origin::signed(2), 2, vec![1, 3], later, hash), Error::::WrongTimepoint, ); }); @@ -280,10 +304,12 @@ fn multisig_2_of_3_works_with_call_storing() { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); let hash = call.using_encoded(blake2_256); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call, true)); + 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_as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), hash)); + 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); }); } @@ -298,10 +324,12 @@ fn multisig_2_of_3_works() { 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)); + 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, false)); + 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); }); } @@ -316,11 +344,13 @@ fn multisig_3_of_3_works() { 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_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, false)); + 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); }); } @@ -330,14 +360,14 @@ 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())); + 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), ); }); } @@ -347,15 +377,16 @@ fn cancel_multisig_with_call_storage_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::as_multi(Origin::signed(1), 3, vec![2, 3], None, call, true)); + 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_as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), hash.clone())); + 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), ); assert_eq!(Balances::free_balance(1), 10); }); @@ -366,33 +397,17 @@ 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(blake2_256); - assert_ok!(Multisig::approve_as_multi(Origin::signed(1), 3, vec![2, 3], None, hash.clone())); + assert_ok!(Multisig::create(Origin::signed(1), 3, vec![2, 3], hash)); assert_eq!(Balances::free_balance(1), 6); - assert_ok!(Multisig::as_multi(Origin::signed(2), 3, vec![1, 3], Some(now()), call, true)); + 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_as_multi(Origin::signed(1), 3, vec![2, 3], now(), hash)); + 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); }); } -#[test] -fn multisig_2_of_3_as_multi_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(), false)); - assert_eq!(Balances::free_balance(6), 0); - - assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call, false)); - assert_eq!(Balances::free_balance(6), 15); - }); -} - #[test] fn multisig_2_of_3_as_multi_with_many_calls_works() { new_test_ext().execute_with(|| { @@ -402,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(blake2_256); + let call2 = Box::new(Call::Balances(BalancesCall::transfer(7, 5))); + let hash2 = call2.using_encoded(blake2_256); - assert_ok!(Multisig::as_multi(Origin::signed(1), 2, vec![2, 3], None, call1.clone(), false)); - assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], None, call2.clone(), false)); - assert_ok!(Multisig::as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), call2, false)); - assert_ok!(Multisig::as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), call1, false)); + 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(|| { @@ -423,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(), false)); - assert_ok!(Multisig::as_multi(Origin::signed(2), 2, vec![1, 3], Some(now()), call.clone(), false)); + let hash = call.using_encoded(blake2_256); + 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(), false)); - assert_ok!(Multisig::as_multi(Origin::signed(3), 2, vec![1, 2], Some(now()), call.clone(), false)); + 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))); }); } @@ -439,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(blake2_256); assert_noop!( - Multisig::as_multi(Origin::signed(1), 0, vec![2], None, call, false), + Multisig::create(Origin::signed(1), 0, vec![2], hash), Error::::ZeroThreshold, ); }); @@ -450,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(blake2_256); assert_noop!( - Multisig::as_multi(Origin::signed(1), 2, vec![2, 3, 4], None, call.clone(), false), + Multisig::create(Origin::signed(1), 2, vec![2, 3, 4], hash), Error::::TooManySignatories, ); }); @@ -462,14 +492,14 @@ 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())); + 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, ); }); @@ -485,15 +515,18 @@ fn multisig_1_of_3_works() { let call = Box::new(Call::Balances(BalancesCall::transfer(6, 15))); let hash = call.using_encoded(blake2_256); + // 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(), false), - BalancesError::::InsufficientBalance, - ); - assert_ok!(Multisig::as_multi(Origin::signed(1), 1, vec![2, 3], None, call, false)); + 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); }); @@ -503,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(blake2_256); + 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(), false), + Multisig::note_preimage(Origin::signed(1), call), Error::::Uncallable, ); }); From ccc69947e38df8ccb1025fa27b5855b058eac199 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 11 Jun 2020 05:43:57 +0200 Subject: [PATCH 6/7] consolidate input verification --- frame/multisig/src/lib.rs | 57 +++++++++++---------------------------- 1 file changed, 16 insertions(+), 41 deletions(-) diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index d957078001bd9..0d39ce7d818d9 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -52,7 +52,7 @@ 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}; @@ -182,25 +182,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; @@ -275,13 +256,9 @@ decl_module! { threshold: u16, other_signatories: Vec, call_hash: [u8; 32], - ) -> DispatchResultWithPostInfo { + ) -> 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); - let other_signatories_len = other_signatories.len(); - 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); @@ -296,8 +273,7 @@ decl_module! { 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()) + Ok(()) } #[weight = 0] @@ -367,10 +343,7 @@ decl_module! { call_hash: [u8; 32], ) -> 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); @@ -397,11 +370,7 @@ decl_module! { call_hash: [u8; 32], ) -> DispatchResultWithPostInfo { 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); - let other_signatories_len = other_signatories.len(); - 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); @@ -463,10 +432,7 @@ decl_module! { call_hash: [u8; 32], ) -> 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); @@ -526,6 +492,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> From 15b949e07c99507212139ac9d3f4935ff3d75938 Mon Sep 17 00:00:00 2001 From: Shawn Tabrizi Date: Thu, 11 Jun 2020 05:48:36 +0200 Subject: [PATCH 7/7] Use generic hasher --- frame/multisig/src/lib.rs | 30 +++++++++++++++------------- frame/multisig/src/tests.rs | 40 ++++++++++++++++++------------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index 0d39ce7d818d9..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, 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,10 +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) [u8; 32] => Option<(Vec, T::AccountId, BalanceOf)>; + pub Calls: map hasher(identity) T::Hash => Option<(Vec, T::AccountId, BalanceOf)>; } } @@ -163,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. @@ -255,7 +257,7 @@ decl_module! { fn create(origin, threshold: u16, other_signatories: Vec, - call_hash: [u8; 32], + call_hash: T::Hash, ) -> DispatchResult { let who = ensure_signed(origin)?; Self::ensure_valid_input(threshold, &other_signatories)?; @@ -286,7 +288,7 @@ decl_module! { let _guard = ClearFilterGuard::::Call>::new(); ensure!(T::IsCallable::filter(call.as_ref()), Error::::Uncallable); let encoded_call = call.encode(); - let call_hash = blake2_256(&encoded_call); + let call_hash = T::Hashing::hash(&encoded_call); Self::store_call(who.clone(), &call_hash, encoded_call)?; } @@ -340,7 +342,7 @@ decl_module! { threshold: u16, other_signatories: Vec, timepoint: Timepoint, - call_hash: [u8; 32], + call_hash: T::Hash, ) -> DispatchResult { let who = ensure_signed(origin)?; Self::ensure_valid_input(threshold, &other_signatories)?; @@ -367,7 +369,7 @@ decl_module! { threshold: u16, other_signatories: Vec, timepoint: Timepoint, - call_hash: [u8; 32], + call_hash: T::Hash, ) -> DispatchResultWithPostInfo { let who = ensure_signed(origin)?; Self::ensure_valid_input(threshold, &other_signatories)?; @@ -429,7 +431,7 @@ decl_module! { threshold: u16, other_signatories: Vec, timepoint: Timepoint, - call_hash: [u8; 32], + call_hash: T::Hash, ) -> DispatchResult { let who = ensure_signed(origin)?; Self::ensure_valid_input(threshold, &other_signatories)?; @@ -458,12 +460,12 @@ 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: &[u8; 32], data: Vec) -> DispatchResult { + 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); @@ -475,7 +477,7 @@ impl Module { } /// Attempt to remove a call from storage, returning it and any deposit on it to the owner. - fn take_call(hash: &[u8; 32]) -> Option<::Call> { + 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() diff --git a/frame/multisig/src/tests.rs b/frame/multisig/src/tests.rs index ab623fa8da8e3..a72ed87401c7d 100644 --- a/frame/multisig/src/tests.rs +++ b/frame/multisig/src/tests.rs @@ -164,7 +164,7 @@ fn multisig_deposit_is_taken_and_returned_when_approved() { 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_eq!(Balances::free_balance(1), 2); assert_eq!(Balances::reserved_balance(1), 3); @@ -186,7 +186,7 @@ fn multisig_deposit_is_taken_and_returned_when_canceled() { 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_eq!(Balances::free_balance(1), 2); assert_eq!(Balances::reserved_balance(1), 3); @@ -206,7 +206,7 @@ fn multisig_deposit_is_taken_and_returned_when_approved_with_call_storage() { 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); @@ -228,7 +228,7 @@ fn multisig_deposit_is_taken_and_returned_when_canceled_with_call_storage() { 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); @@ -250,7 +250,7 @@ fn multisig_deposit_is_taken_and_returned_when_canceled_with_call_storage() { // 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), 3, vec![2, 3], None, hash)); // assert_eq!(Balances::free_balance(1), 1); @@ -281,7 +281,7 @@ 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)); @@ -303,7 +303,7 @@ fn multisig_2_of_3_works_with_call_storing() { 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(6), 0); @@ -323,7 +323,7 @@ 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); + 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); @@ -343,7 +343,7 @@ 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); + 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); @@ -359,7 +359,7 @@ 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); + 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!( @@ -376,7 +376,7 @@ fn cancel_multisig_works() { fn cancel_multisig_with_call_storage_works() { new_test_ext().execute_with(|| { 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), 3, vec![2, 3], hash)); assert_ok!(Multisig::note_preimage(Origin::signed(1), call)); assert_eq!(Balances::free_balance(1), 4); @@ -396,7 +396,7 @@ fn cancel_multisig_with_call_storage_works() { 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(blake2_256); + 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)); @@ -417,10 +417,10 @@ 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(blake2_256); + let hash1 = call1.using_encoded(::Hashing::hash); let call2 = Box::new(Call::Balances(BalancesCall::transfer(7, 5))); - let hash2 = call2.using_encoded(blake2_256); + let hash2 = call2.using_encoded(::Hashing::hash); assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash1)); assert_ok!(Multisig::create(Origin::signed(2), 2, vec![1, 3], hash2)); @@ -446,7 +446,7 @@ 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))); - 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::approve(Origin::signed(2), 2, vec![1, 3], now(), hash)); assert_ok!(Multisig::note_preimage(Origin::signed(2), call.clone())); @@ -467,7 +467,7 @@ 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(blake2_256); + let hash = call.using_encoded(::Hashing::hash); assert_noop!( Multisig::create(Origin::signed(1), 0, vec![2], hash), Error::::ZeroThreshold, @@ -479,7 +479,7 @@ 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(blake2_256); + let hash = call.using_encoded(::Hashing::hash); assert_noop!( Multisig::create(Origin::signed(1), 2, vec![2, 3, 4], hash), Error::::TooManySignatories, @@ -491,7 +491,7 @@ 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); + let hash = call.using_encoded(::Hashing::hash); assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); assert_noop!( Multisig::approve(Origin::signed(1), 2, vec![2, 3], now(), hash), @@ -514,7 +514,7 @@ 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), @@ -536,7 +536,7 @@ 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(blake2_256); + let hash = call.using_encoded(::Hashing::hash); assert_ok!(Multisig::create(Origin::signed(1), 2, vec![2, 3], hash)); assert_noop!( Multisig::note_preimage(Origin::signed(1), call),