Stored call in multisig

frame/multisig/src/benchmarking.rs

@@ -20,7 +20,7 @@
 #![cfg(feature = "runtime-benchmarks")]
 
 use super::*;
-use frame_system::{Module as System, RawOrigin, EventRecord};
+use frame_system::RawOrigin;
 use frame_benchmarking::{benchmarks, account};
 use sp_runtime::traits::{Bounded, Saturating};
 use core::convert::TryInto;
@@ -29,14 +29,6 @@ use crate::Module as Multisig;
 
 const SEED: u32 = 0;
 
-fn assert_last_event<T: Trait>(generic_event: <T as Trait>::Event) {
-	let events = System::<T>::events();
-	let system_event: <T as frame_system::Trait>::Event = generic_event.into();
-	// compare to the last event record
-	let EventRecord { event, .. } = &events[events.len() - 1];
-	assert_eq!(event, &system_event);
-}
-
 fn setup_multi<T: Trait>(s: u32, z: u32)
 	-> Result<(Vec<T::AccountId>, Vec<u8>), &'static str>
 {
@@ -70,10 +62,7 @@ benchmarks! {
 		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
 	}: _(RawOrigin::Signed(caller.clone()), signatories, Box::new(call))
 	verify {
-		let timepoint = Multisig::<T>::timepoint();
-		assert_last_event::<T>(
-			RawEvent::MultisigExecuted(caller, timepoint, multi_account_id, call_hash, Ok(())).into()
-		);
+		// If the benchmark resolves, then the call was dispatched successfully.
 	}
 
 	as_multi_create {

frame/multisig/src/lib.rs

@@ -50,11 +50,11 @@ 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, constants::{WEIGHT_PER_MICROS, WEIGHT_PER_NANOS}},
+use frame_support::{traits::{Get, ReservableCurrency, Currency},
+	weights::{Weight, GetDispatchInfo, constants::{WEIGHT_PER_NANOS, WEIGHT_PER_MICROS}},
 	dispatch::{DispatchResultWithPostInfo, DispatchErrorWithPostInfo, PostDispatchInfo},
 };
-use frame_system::{self as system, ensure_signed};
+use frame_system::{self as system, ensure_signed, RawOrigin};
 use sp_runtime::{DispatchError, DispatchResult, traits::Dispatchable};
 
 mod tests;
@@ -128,8 +128,8 @@ decl_storage! {
 
 decl_error! {
 	pub enum Error for Module<T: Trait> {
-		/// Threshold is too low (zero).
-		ZeroThreshold,
+		/// Threshold must be 2 or greater.
+		MinimumThreshold,
 		/// Call is already approved by this signatory.
 		AlreadyApproved,
 		/// Call doesn't need any (more) approvals.
@@ -152,9 +152,7 @@ decl_error! {
 		WrongTimepoint,
 		/// A timepoint was given, yet no multisig operation is underway.
 		UnexpectedTimepoint,
-		/// A call with a `false` `IsCallable` filter was attempted.
-		Uncallable,
-		/// The maximum weight information was provided was too low.
+		/// The maximum weight information provided was too low.
 		WeightTooLow,
 	}
 }
@@ -257,7 +255,7 @@ decl_module! {
 		/// Result is equivalent to the dispatched result.
 		///
 		/// # <weight>
-		/// O(Z) where Z is the length of the call.
+		/// O(Z + C) where Z is the length of the call and C its execution weight.
 		/// -------------------------------
 		/// - Base Weight: 33.72 + 0.002 * Z µs
 		/// - DB Weight: None
@@ -283,17 +281,8 @@ decl_module! {
 
 			let id = Self::multi_account_id(&signatories, 1);
 
-			// We're now executing as a freshly authenticated new account, so the previous call
-			// restrictions no longer apply.
-			let _guard = ClearFilterGuard::<T::IsCallable, <T as Trait>::Call>::new();
-			ensure!(T::IsCallable::filter(&call), Error::<T>::Uncallable);
-
-			let (call_len, call_hash) = call.using_encoded(|c| (c.len(), blake2_256(&c)));
-			let result = call.dispatch(frame_system::RawOrigin::Signed(id.clone()).into());
-
-			Self::deposit_event(RawEvent::MultisigExecuted(
-				who, Self::timepoint(), id, call_hash, result.map(|_| ()).map_err(|e| e.error)
-			));
+			let call_len = call.using_encoded(|c| c.len());
+			let result = call.dispatch(RawOrigin::Signed(id.clone()).into());
 
 			result.map(|post_dispatch_info| post_dispatch_info.actual_weight
 				.map(|actual_weight| weight_of::as_multi_threshold_1::<T>(
@@ -370,8 +359,8 @@ decl_module! {
 			other_signatories.len(),
 			call.len(),
 			*max_weight,
-			true,
-			true,
+			true, // assume worst case: calls write
+			true, // assume worst case: refunded
 		)]
 		fn as_multi(origin,
 			threshold: u16,
@@ -428,8 +417,8 @@ decl_module! {
 			other_signatories.len(),
 			0, // call_len is zero in this case
 			*max_weight,
-			true,
-			true,
+			true, // assume worst case: calls write
+			true, // assume worst case: refunded
 		)]
 		fn approve_as_multi(origin,
 			threshold: u16,
@@ -480,7 +469,7 @@ decl_module! {
 			call_hash: [u8; 32],
 		) -> DispatchResult {
 			let who = ensure_signed(origin)?;
-			ensure!(threshold >= 2, Error::<T>::ZeroThreshold);
+			ensure!(threshold >= 2, Error::<T>::MinimumThreshold);
 			let max_sigs = T::MaxSignatories::get() as usize;
 			ensure!(!other_signatories.is_empty(), Error::<T>::TooFewSignatories);
 			ensure!(other_signatories.len() < max_sigs, Error::<T>::TooManySignatories);
@@ -521,7 +510,7 @@ impl<T: Trait> Module<T> {
 		call_or_hash: CallOrHash,
 		max_weight: Weight,
 	) -> DispatchResultWithPostInfo {
-		ensure!(threshold >= 2, Error::<T>::ZeroThreshold);
+		ensure!(threshold >= 2, Error::<T>::MinimumThreshold);
 		let max_sigs = T::MaxSignatories::get() as usize;
 		ensure!(!other_signatories.is_empty(), Error::<T>::TooFewSignatories);
 		let other_signatories_len = other_signatories.len();
@@ -561,12 +550,8 @@ impl<T: Trait> Module<T> {
 			if let Some(call) = maybe_approved_call {
 				// verify weight
 				ensure!(call.get_dispatch_info().weight <= max_weight, Error::<T>::WeightTooLow);
-				// We're now executing as a freshly authenticated new account, so the previous call
-				// restrictions no longer apply.
-				let _guard = ClearFilterGuard::<T::IsCallable, <T as Trait>::Call>::new();
-				ensure!(T::IsCallable::filter(&call), Error::<T>::Uncallable);
 
-				let result = call.dispatch(frame_system::RawOrigin::Signed(id.clone()).into());
+				let result = call.dispatch(RawOrigin::Signed(id.clone()).into());
 				let _ = T::Currency::unreserve(&m.depositor, m.deposit);
 				<Multisigs<T>>::remove(&id, call_hash);
 				Self::clear_call(&call_hash);

frame/multisig/src/tests.rs

@@ -450,12 +450,16 @@ fn multisig_2_of_3_cannot_reissue_same_call() {
 }
 
 #[test]
-fn zero_threshold_fails() {
+fn minimum_threshold_check_works() {
 	new_test_ext().execute_with(|| {
 		let call = Call::Balances(BalancesCall::transfer(6, 15)).encode();
 		assert_noop!(
-			Multisig::as_multi(Origin::signed(1), 0, vec![2], None, call, false, 0),
-			Error::<Test>::ZeroThreshold,
+			Multisig::as_multi(Origin::signed(1), 0, vec![2], None, call.clone(), false, 0),
+			Error::<Test>::MinimumThreshold,
+		);
+		assert_noop!(
+			Multisig::as_multi(Origin::signed(1), 1, vec![2], None, call.clone(), false, 0),
+			Error::<Test>::MinimumThreshold,
 		);
 	});
 }
@@ -501,11 +505,11 @@ fn multisig_1_of_3_works() {
 		let hash = blake2_256(&call);
 		assert_noop!(
 			Multisig::approve_as_multi(Origin::signed(1), 1, vec![2, 3], None, hash.clone(), 0),
-			Error::<Test>::ZeroThreshold,
+			Error::<Test>::MinimumThreshold,
 		);
 		assert_noop!(
-			Multisig::as_multi(Origin::signed(4), 1, vec![2, 3], None, call.clone(), false, 0),
-			Error::<Test>::ZeroThreshold,
+			Multisig::as_multi(Origin::signed(1), 1, vec![2, 3], None, call.clone(), false, 0),
+			Error::<Test>::MinimumThreshold,
 		);
 		let boxed_call = Box::new(Call::Balances(BalancesCall::transfer(6, 15)));
 		assert_ok!(Multisig::as_multi_threshold_1(Origin::signed(1), vec![2, 3], boxed_call));
@@ -519,8 +523,8 @@ 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_threshold_1(Origin::signed(1), vec![2], call),
-			Error::<Test>::Uncallable,
+			Multisig::as_multi_threshold_1(Origin::signed(1), vec![2], call.clone()),
+			DispatchError::BadOrigin,
 		);
 	});
 }