Stored call in multisig (#6319)

* Stored call in multisig * Docs. * Benchmarks. * Fix * Update frame/multisig/src/lib.rs Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> * patch benchmarks * Minor grumbles. * Update as_multi weight * Fixes and refactoring. * Split out threshold=1 and opaquify Call. * Compiles, tests pass, weights are broken * Update benchmarks, add working tests * Add benchmark to threshold 1, add event too * suppress warning for now * @xlc improvment nit * Update weight and tests * Test for weight check * Fix line width * one more line width error * Apply suggestions from code review Co-authored-by: Alexander Popiak <alexander.popiak@parity.io> * fix merge * more @apopiak feedback * Multisig handles no preimage * Optimize return weight after dispatch * Error on failed deposit. Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com> Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com> Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
2026-06-15 10:21:05 +00:00 · 2020-06-17 12:24:32 +02:00
parent 5f991ae9b8
commit 67c9ac9393
3 changed files with 711 additions and 241 deletions
@@ -22,14 +22,15 @@
 use super::*;
 use frame_system::RawOrigin;
 use frame_benchmarking::{benchmarks, account};
-use sp_runtime::traits::Saturating;
+use sp_runtime::traits::{Bounded, Saturating};
+use core::convert::TryInto;

 use crate::Module as Multisig;

 const SEED: u32 = 0;

 fn setup_multi<T: Trait>(s: u32, z: u32)
-	-> Result<(Vec<T::AccountId>, Box<<T as Trait>::Call>), &'static str>
+	-> Result<(Vec<T::AccountId>, Vec<u8>), &'static str>
 {
 	let mut signatories: Vec<T::AccountId> = Vec::new();
 	for i in 0 .. s {
@@ -41,36 +42,79 @@ fn setup_multi<T: Trait>(s: u32, z: u32)
 		signatories.push(signatory);
 	}
 	signatories.sort();
-	let call: Box<<T as Trait>::Call> = Box::new(frame_system::Call::remark(vec![0; z as usize]).into());
-	return Ok((signatories, call))
+	// Must first convert to outer call type.
+	let call: <T as Trait>::Call = frame_system::Call::<T>::remark(vec![0; z as usize]).into();
+	let call_data = call.encode();
+	return Ok((signatories, call_data))
 }

 benchmarks! {
 	_ { }

+	as_multi_threshold_1 {
+		// Transaction Length
+		let z in 0 .. 10_000;
+		let max_signatories = T::MaxSignatories::get().into();
+		let (mut signatories, _) = setup_multi::<T>(max_signatories, z)?;
+		let call: <T as Trait>::Call = frame_system::Call::<T>::remark(vec![0; z as usize]).into();
+		let call_hash = call.using_encoded(blake2_256);
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, 1);
+		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
+	}: _(RawOrigin::Signed(caller.clone()), signatories, Box::new(call))
+	verify {
+		// If the benchmark resolves, then the call was dispatched successfully.
+	}
+
 	as_multi_create {
 		// 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::<T>(s, z)?;
+		let call_hash = blake2_256(&call);
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, s.try_into().unwrap());
 		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, 0)
+	verify {
+		assert!(Multisigs::<T>::contains_key(multi_account_id, call_hash));
+	}

-	as_multi_approve {
-		// Signatories, need at least 2 people
+	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::<T>(s, z)?;
+		let call_hash = blake2_256(&call);
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, s.try_into().unwrap());
+		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
+		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
+	}: as_multi(RawOrigin::Signed(caller), s as u16, signatories, None, call, true, 0)
+	verify {
+		assert!(Multisigs::<T>::contains_key(multi_account_id, call_hash));
+		assert!(Calls::<T>::contains_key(call_hash));
+	}
+
+	as_multi_approve {
+		// Signatories, need at least 3 people (so we don't complete the multisig)
+		let s in 3 .. T::MaxSignatories::get() as u32;
+		// Transaction Length
+		let z in 0 .. 10_000;
+		let (mut signatories, call) = setup_multi::<T>(s, z)?;
+		let call_hash = blake2_256(&call);
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, s.try_into().unwrap());
 		let mut signatories2 = signatories.clone();
 		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
 		// before the call, get the timepoint
 		let timepoint = Multisig::<T>::timepoint();
-		// Create the multi
-		Multisig::<T>::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone())?;
+		// Create the multi, storing for worst case
+		Multisig::<T>::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone(), true, 0)?;
 		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, 0)
+	verify {
+		let multisig = Multisigs::<T>::get(multi_account_id, call_hash).ok_or("multisig not created")?;
+		assert_eq!(multisig.approvals.len(), 2);
+	}

 	as_multi_complete {
 		// Signatories, need at least 2 people
@@ -78,21 +122,27 @@ benchmarks! {
 		// Transaction Length
 		let z in 0 .. 10_000;
 		let (mut signatories, call) = setup_multi::<T>(s, z)?;
+		let call_hash = blake2_256(&call);
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, s.try_into().unwrap());
 		let mut signatories2 = signatories.clone();
 		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
 		// before the call, get the timepoint
 		let timepoint = Multisig::<T>::timepoint();
-		// Create the multi
-		Multisig::<T>::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone())?;
+		// Create the multi, storing it for worst case
+		Multisig::<T>::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone(), true, 0)?;
 		// 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::<T>::as_multi(o, s as u16, signatories_loop, Some(timepoint), call.clone())?;
+			Multisig::<T>::as_multi(o, s as u16, signatories_loop, Some(timepoint), call.clone(), false, 0)?;
 		}
 		let caller2 = signatories2.remove(0);
-	}: as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call)
+		assert!(Multisigs::<T>::contains_key(&multi_account_id, call_hash));
+	}: as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call, false, Weight::max_value())
+	verify {
+		assert!(!Multisigs::<T>::contains_key(&multi_account_id, call_hash));
+	}

 	approve_as_multi_create {
 		// Signatories, need at least 2 people
@@ -100,10 +150,14 @@ benchmarks! {
 		// Transaction Length
 		let z in 0 .. 10_000;
 		let (mut signatories, call) = setup_multi::<T>(s, z)?;
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, s.try_into().unwrap());
 		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
-		let call_hash = call.using_encoded(blake2_256);
+		let call_hash = blake2_256(&call);
 		// Create the multi
-	}: approve_as_multi(RawOrigin::Signed(caller), s as u16, signatories, None, call_hash)
+	}: approve_as_multi(RawOrigin::Signed(caller), s as u16, signatories, None, call_hash, 0)
+	verify {
+		assert!(Multisigs::<T>::contains_key(multi_account_id, call_hash));
+	}

 	approve_as_multi_approve {
 		// Signatories, need at least 2 people
@@ -112,14 +166,63 @@ benchmarks! {
 		let z in 0 .. 10_000;
 		let (mut signatories, call) = setup_multi::<T>(s, z)?;
 		let mut signatories2 = signatories.clone();
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, s.try_into().unwrap());
 		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
-		let call_hash = call.using_encoded(blake2_256);
+		let call_hash = blake2_256(&call);
 		// before the call, get the timepoint
 		let timepoint = Multisig::<T>::timepoint();
 		// Create the multi
-		Multisig::<T>::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone())?;
+		Multisig::<T>::as_multi(
+			RawOrigin::Signed(caller.clone()).into(),
+			s as u16,
+			signatories,
+			None,
+			call.clone(),
+			false,
+			0
+		)?;
 		let caller2 = signatories2.remove(0);
-	}: approve_as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call_hash)
+	}: approve_as_multi(RawOrigin::Signed(caller2), s as u16, signatories2, Some(timepoint), call_hash, 0)
+	verify {
+		let multisig = Multisigs::<T>::get(multi_account_id, call_hash).ok_or("multisig not created")?;
+		assert_eq!(multisig.approvals.len(), 2);
+	}
+
+	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::<T>(s, z)?;
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, s.try_into().unwrap());
+		let mut signatories2 = signatories.clone();
+		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
+		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
+		let call_hash = blake2_256(&call);
+		// before the call, get the timepoint
+		let timepoint = Multisig::<T>::timepoint();
+		// Create the multi
+		Multisig::<T>::as_multi(RawOrigin::Signed(caller).into(), s as u16, signatories, None, call.clone(), true, 0)?;
+		// 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::<T>::as_multi(o, s as u16, signatories_loop, Some(timepoint), call.clone(), false, 0)?;
+		}
+		let caller2 = signatories2.remove(0);
+		assert!(Multisigs::<T>::contains_key(&multi_account_id, call_hash));
+	}: approve_as_multi(
+		RawOrigin::Signed(caller2),
+		s as u16,
+		signatories2,
+		Some(timepoint),
+		call_hash,
+		Weight::max_value()
+	)
+	verify {
+		assert!(!Multisigs::<T>::contains_key(multi_account_id, call_hash));
+	}

 	cancel_as_multi {
 		// Signatories, need at least 2 people
@@ -127,13 +230,40 @@ benchmarks! {
 		// Transaction Length
 		let z in 0 .. 10_000;
 		let (mut signatories, call) = setup_multi::<T>(s, z)?;
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, s.try_into().unwrap());
 		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
-		let call_hash = call.using_encoded(blake2_256);
+		let call_hash = blake2_256(&call);
 		let timepoint = Multisig::<T>::timepoint();
 		// Create the multi
 		let o = RawOrigin::Signed(caller.clone()).into();
-		Multisig::<T>::as_multi(o, s as u16, signatories.clone(), None, call.clone())?;
+		Multisig::<T>::as_multi(o, s as u16, signatories.clone(), None, call.clone(), true, 0)?;
+		assert!(Multisigs::<T>::contains_key(&multi_account_id, call_hash));
 	}: _(RawOrigin::Signed(caller), s as u16, signatories, timepoint, call_hash)
+	verify {
+		assert!(!Multisigs::<T>::contains_key(multi_account_id, 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::<T>(s, z)?;
+		let multi_account_id = Multisig::<T>::multi_account_id(&signatories, s.try_into().unwrap());
+		let caller = signatories.pop().ok_or("signatories should have len 2 or more")?;
+		T::Currency::make_free_balance_be(&caller, BalanceOf::<T>::max_value());
+		let call_hash = blake2_256(&call);
+		let timepoint = Multisig::<T>::timepoint();
+		// Create the multi
+		let o = RawOrigin::Signed(caller.clone()).into();
+		Multisig::<T>::as_multi(o, s as u16, signatories.clone(), None, call.clone(), true, 0)?;
+		assert!(Multisigs::<T>::contains_key(&multi_account_id, call_hash));
+		assert!(Calls::<T>::contains_key(call_hash));
+	}: cancel_as_multi(RawOrigin::Signed(caller), s as u16, signatories, timepoint, call_hash)
+	verify {
+		assert!(!Multisigs::<T>::contains_key(&multi_account_id, call_hash));
+		assert!(!Calls::<T>::contains_key(call_hash));
+	}
 }

 #[cfg(test)]
@@ -145,12 +275,16 @@ mod tests {
 	#[test]
 	fn test_benchmarks() {
 		new_test_ext().execute_with(|| {
+			assert_ok!(test_benchmark_as_multi_threshold_1::<Test>());
 			assert_ok!(test_benchmark_as_multi_create::<Test>());
+			assert_ok!(test_benchmark_as_multi_create_store::<Test>());
 			assert_ok!(test_benchmark_as_multi_approve::<Test>());
 			assert_ok!(test_benchmark_as_multi_complete::<Test>());
 			assert_ok!(test_benchmark_approve_as_multi_create::<Test>());
 			assert_ok!(test_benchmark_approve_as_multi_approve::<Test>());
+			assert_ok!(test_benchmark_approve_as_multi_complete::<Test>());
 			assert_ok!(test_benchmark_cancel_as_multi::<Test>());
+			assert_ok!(test_benchmark_cancel_as_multi_store::<Test>());
 		});
 	}
 }