Bound uses of Call (#11649)

* Introduce preimages module in traits

* Multisize Preimages

* Len not actually necessary

* Tweaks to the preimage API

* Fixes

* Get Scheduler building with new API

* Scheduler tests pass

* Bounded Scheduler 🎉

* Use Agenda holes and introduce IncompleteSince to avoid need to reschedule

* Tests pass with new weight system

* New benchmarks

* Add missing file

* Drop preimage when permenantly overeight

* Drop preimage when permenantly overeight

* Referenda uses latest preimage API

* Testing ok

* Adding tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* fmt

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add preimage migration

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Docs

* Remove dbg

* Refactor Democracy

* Refactor Democracy

* Add final MEL

* Remove silly maps

* Fixes

* Minor refactor

* Formatting

* Fixes

* Fixes

* Fixes

* Update frame/preimage/src/lib.rs

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>

* Add migrations to Democracy

* WIP

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Resolve conflicts

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Revert "Resolve conflicts"

This reverts commit 734d66d69e54553471ffa54fa52e3e304dc8f106.

* Undo wrong resolves...

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* WIP

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Make compile

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* massage clippy

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* More clippy

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* clippy annoyance

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* clippy annoyance

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix benchmarks

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* add missing file

* Test <Preimage as QueryPreimage>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* More tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Clippy harassment

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* clippy

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fixup tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Remove old stuff

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* fmt

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Test <Scheduler as Anon> trait functions

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update pallet-ui tests

Why is this needed? Should not be the case unless master is broken...

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* More scheduler trait test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* More tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Apply review suggestion

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Beauty fixes

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add Scheduler test migration_v3_to_v4_works

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Merge fixup

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Keep referenda benchmarks instantiatable

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update weights

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Use new scheduler weight functions

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Use new democracy weight functions

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Use weight compare functions

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update pallet-ui tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* More renaming…

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* More renaming…

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add comment

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Implement OnRuntimeUpgrade for scheduler::v3_to_v4 migration

Put the migration into a proper `MigrateToV4` struct and implement
the OnRuntimeUpgrade hooks for it. Also move the test to use that
instead.

This should make it easier for adding it to Polkadot.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Clippy

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Handle undecodable Agendas

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Remove trash

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Use new OnRuntimeUpgrade functions

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* fix test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix BoundedSlice::truncate_from

Co-authored-by: jakoblell

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix pre_upgrade hook return values

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add more error logging

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Find too large preimages in the pre_upgrade hook

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Test that too large Calls in agendas are ignored

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Use new OnRuntimeUpgrade hooks

Why did the CI not catch this?!

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* works fine - just more logs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix staking migration

Causing issues on Kusama...

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix UI tests

No idea why this is needed. This is actually undoing an earlier change.
Maybe the CI has different rustc versions!?

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Remove multisig's Calls (#12072)

* Remove multisig's Calls

* Multisig: Fix tests and re-introduce reserve logic (#12241)

* Fix tests and re-introduce reserve logic

* fix benches

* add todo

* remove irrelevant bench

* [Feature] Add a migration that drains and refunds stored calls (#12313)

* [Feature] Add a migration that drains and refunds stored calls

* migration fixes

* fixes

* address review comments

* consume the whole block weight

* fix assertions

* license header

* fix interface

Co-authored-by: parity-processbot <>

Co-authored-by: parity-processbot <>
Co-authored-by: Roman Useinov <roman.useinov@gmail.com>

* Fix test

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Fix multisig benchmarks

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* ".git/.scripts/bench-bot.sh" pallet dev pallet_democracy

* ".git/.scripts/bench-bot.sh" pallet dev pallet_scheduler

* ".git/.scripts/bench-bot.sh" pallet dev pallet_preimage

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Co-authored-by: parity-processbot <>
Co-authored-by: Roman Useinov <roman.useinov@gmail.com>

substrate/frame/multisig/src/lib.rs

@@ -47,6 +47,7 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
 mod benchmarking;
+pub mod migrations;
 mod tests;
 pub mod weights;
 
@@ -57,7 +58,7 @@ use frame_support::{
 		PostDispatchInfo,
 	},
 	ensure,
-	traits::{Currency, Get, ReservableCurrency, WrapperKeepOpaque},
+	traits::{Currency, Get, ReservableCurrency},
 	weights::Weight,
 	RuntimeDebug,
 };
@@ -73,6 +74,20 @@ pub use weights::WeightInfo;
 
 pub use pallet::*;
 
+/// The log target of this pallet.
+pub const LOG_TARGET: &'static str = "runtime::multisig";
+
+// syntactic sugar for logging.
+#[macro_export]
+macro_rules! log {
+	($level:tt, $patter:expr $(, $values:expr)* $(,)?) => {
+		log::$level!(
+			target: crate::LOG_TARGET,
+			concat!("[{:?}] ✍️ ", $patter), <frame_system::Pallet<T>>::block_number() $(, $values)*
+		)
+	};
+}
+
 type BalanceOf<T> =
 	<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
 
@@ -100,12 +115,10 @@ pub struct Multisig<BlockNumber, Balance, AccountId> {
 	approvals: Vec<AccountId>,
 }
 
-type OpaqueCall<T> = WrapperKeepOpaque<<T as Config>::RuntimeCall>;
-
 type CallHash = [u8; 32];
 
 enum CallOrHash<T: Config> {
-	Call(OpaqueCall<T>, bool),
+	Call(<T as Config>::RuntimeCall),
 	Hash([u8; 32]),
 }
 
@@ -152,9 +165,13 @@ pub mod pallet {
 		type WeightInfo: WeightInfo;
 	}
 
+	/// The current storage version.
+	const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);
+
 	#[pallet::pallet]
 	#[pallet::generate_store(pub(super) trait Store)]
 	#[pallet::without_storage_info]
+	#[pallet::storage_version(STORAGE_VERSION)]
 	pub struct Pallet<T>(_);
 
 	/// The set of open multisig operations.
@@ -168,10 +185,6 @@ pub mod pallet {
 		Multisig<T::BlockNumber, BalanceOf<T>, T::AccountId>,
 	>;
 
-	#[pallet::storage]
-	pub type Calls<T: Config> =
-		StorageMap<_, Identity, [u8; 32], (OpaqueCall<T>, T::AccountId, BalanceOf<T>)>;
-
 	#[pallet::error]
 	pub enum Error<T> {
 		/// Threshold must be 2 or greater.
@@ -343,13 +356,13 @@ pub mod pallet {
 		///   taken for its lifetime of `DepositBase + threshold * DepositFactor`.
 		/// -------------------------------
 		/// - DB Weight:
-		///     - Reads: Multisig Storage, [Caller Account], Calls (if `store_call`)
-		///     - Writes: Multisig Storage, [Caller Account], Calls (if `store_call`)
+		///     - Reads: Multisig Storage, [Caller Account]
+		///     - Writes: Multisig Storage, [Caller Account]
 		/// - Plus Call Weight
 		/// # </weight>
 		#[pallet::weight({
 			let s = other_signatories.len() as u32;
-			let z = call.encoded_len() as u32;
+			let z = call.using_encoded(|d| d.len()) as u32;
 
 			T::WeightInfo::as_multi_create(s, z)
 			.max(T::WeightInfo::as_multi_create_store(s, z))
@@ -362,8 +375,7 @@ pub mod pallet {
 			threshold: u16,
 			other_signatories: Vec<T::AccountId>,
 			maybe_timepoint: Option<Timepoint<T::BlockNumber>>,
-			call: OpaqueCall<T>,
-			store_call: bool,
+			call: Box<<T as Config>::RuntimeCall>,
 			max_weight: Weight,
 		) -> DispatchResultWithPostInfo {
 			let who = ensure_signed(origin)?;
@@ -372,7 +384,7 @@ pub mod pallet {
 				threshold,
 				other_signatories,
 				maybe_timepoint,
-				CallOrHash::Call(call, store_call),
+				CallOrHash::Call(*call),
 				max_weight,
 			)
 		}
@@ -462,8 +474,8 @@ pub mod pallet {
 		/// - Storage: removes one item.
 		/// ----------------------------------
 		/// - DB Weight:
-		///     - Read: Multisig Storage, [Caller Account], Refund Account, Calls
-		///     - Write: Multisig Storage, [Caller Account], Refund Account, Calls
+		///     - Read: Multisig Storage, [Caller Account], Refund Account
+		///     - Write: Multisig Storage, [Caller Account], Refund Account
 		/// # </weight>
 		#[pallet::weight(T::WeightInfo::cancel_as_multi(other_signatories.len() as u32))]
 		pub fn cancel_as_multi(
@@ -489,7 +501,6 @@ pub mod pallet {
 			let err_amount = T::Currency::unreserve(&m.depositor, m.deposit);
 			debug_assert!(err_amount.is_zero());
 			<Multisigs<T>>::remove(&id, &call_hash);
-			Self::clear_call(&call_hash);
 
 			Self::deposit_event(Event::MultisigCancelled {
 				cancelling: who,
@@ -531,13 +542,12 @@ impl<T: Config> Pallet<T> {
 		let id = Self::multi_account_id(&signatories, threshold);
 
 		// Threshold > 1; this means it's a multi-step operation. We extract the `call_hash`.
-		let (call_hash, call_len, maybe_call, store) = match call_or_hash {
-			CallOrHash::Call(call, should_store) => {
-				let call_hash = blake2_256(call.encoded());
-				let call_len = call.encoded_len();
-				(call_hash, call_len, Some(call), should_store)
+		let (call_hash, call_len, maybe_call) = match call_or_hash {
+			CallOrHash::Call(call) => {
+				let (call_hash, call_len) = call.using_encoded(|d| (blake2_256(d), d.len()));
+				(call_hash, call_len, Some(call))
 			},
-			CallOrHash::Hash(h) => (h, 0, None, false),
+			CallOrHash::Hash(h) => (h, 0, None),
 		};
 
 		// Branch on whether the operation has already started or not.
@@ -556,13 +566,7 @@ impl<T: Config> Pallet<T> {
 			}
 
 			// We only bother fetching/decoding call if we know that we're ready to execute.
-			let maybe_approved_call = if approvals >= threshold {
-				Self::get_call(&call_hash, maybe_call.as_ref())
-			} else {
-				None
-			};
-
-			if let Some((call, call_len)) = maybe_approved_call {
+			if let Some(call) = maybe_call.filter(|_| approvals >= threshold) {
 				// verify weight
 				ensure!(
 					call.get_dispatch_info().weight.all_lte(max_weight),
@@ -572,7 +576,6 @@ impl<T: Config> Pallet<T> {
 				// Clean up storage before executing call to avoid an possibility of reentrancy
 				// attack.
 				<Multisigs<T>>::remove(&id, call_hash);
-				Self::clear_call(&call_hash);
 				T::Currency::unreserve(&m.depositor, m.deposit);
 
 				let result = call.dispatch(RawOrigin::Signed(id.clone()).into());
@@ -596,19 +599,6 @@ impl<T: Config> Pallet<T> {
 				// We cannot dispatch the call now; either it isn't available, or it is, but we
 				// don't have threshold approvals even with our signature.
 
-				// Store the call if desired.
-				let stored = if let Some(data) = maybe_call.filter(|_| store) {
-					Self::store_call_and_reserve(
-						who.clone(),
-						&call_hash,
-						data,
-						BalanceOf::<T>::zero(),
-					)?;
-					true
-				} else {
-					false
-				};
-
 				if let Some(pos) = maybe_pos {
 					// Record approval.
 					m.approvals.insert(pos, who.clone());
@@ -622,17 +612,11 @@ impl<T: Config> Pallet<T> {
 				} else {
 					// If we already approved and didn't store the Call, then this was useless and
 					// we report an error.
-					ensure!(stored, Error::<T>::AlreadyApproved);
+					Err(Error::<T>::AlreadyApproved)?
 				}
 
-				let final_weight = if stored {
-					T::WeightInfo::as_multi_approve_store(
-						other_signatories_len as u32,
-						call_len as u32,
-					)
-				} else {
-					T::WeightInfo::as_multi_approve(other_signatories_len as u32, call_len as u32)
-				};
+				let final_weight =
+					T::WeightInfo::as_multi_approve(other_signatories_len as u32, call_len as u32);
 				// Call is not made, so the actual weight does not include call
 				Ok(Some(final_weight).into())
 			}
@@ -643,14 +627,7 @@ impl<T: Config> Pallet<T> {
 			// Just start the operation by recording it in storage.
 			let deposit = T::DepositBase::get() + T::DepositFactor::get() * threshold.into();
 
-			// Store the call if desired.
-			let stored = if let Some(data) = maybe_call.filter(|_| store) {
-				Self::store_call_and_reserve(who.clone(), &call_hash, data, deposit)?;
-				true
-			} else {
-				T::Currency::reserve(&who, deposit)?;
-				false
-			};
+			T::Currency::reserve(&who, deposit)?;
 
 			<Multisigs<T>>::insert(
 				&id,
@@ -664,58 +641,13 @@ impl<T: Config> Pallet<T> {
 			);
 			Self::deposit_event(Event::NewMultisig { approving: who, multisig: id, call_hash });
 
-			let final_weight = if stored {
-				T::WeightInfo::as_multi_create_store(other_signatories_len as u32, call_len as u32)
-			} else {
-				T::WeightInfo::as_multi_create(other_signatories_len as u32, call_len as u32)
-			};
+			let final_weight =
+				T::WeightInfo::as_multi_create(other_signatories_len as u32, call_len as u32);
 			// Call is not made, so the actual weight does not include call
 			Ok(Some(final_weight).into())
 		}
 	}
 
-	/// Place a call's encoded data in storage, reserving funds as appropriate.
-	///
-	/// We store `data` here because storing `call` would result in needing another `.encode`.
-	///
-	/// Returns a `bool` indicating whether the data did end up being stored.
-	fn store_call_and_reserve(
-		who: T::AccountId,
-		hash: &[u8; 32],
-		data: OpaqueCall<T>,
-		other_deposit: BalanceOf<T>,
-	) -> DispatchResult {
-		ensure!(!Calls::<T>::contains_key(hash), Error::<T>::AlreadyStored);
-		let deposit = other_deposit +
-			T::DepositBase::get() +
-			T::DepositFactor::get() *
-				BalanceOf::<T>::from(((data.encoded_len() + 31) / 32) as u32);
-		T::Currency::reserve(&who, deposit)?;
-		Calls::<T>::insert(&hash, (data, who, deposit));
-		Ok(())
-	}
-
-	/// Attempt to decode and return the call, provided by the user or from storage.
-	fn get_call(
-		hash: &[u8; 32],
-		maybe_known: Option<&OpaqueCall<T>>,
-	) -> Option<(<T as Config>::RuntimeCall, usize)> {
-		maybe_known.map_or_else(
-			|| {
-				Calls::<T>::get(hash)
-					.and_then(|(data, ..)| Some((data.try_decode()?, data.encoded_len())))
-			},
-			|data| Some((data.try_decode()?, data.encoded_len())),
-		)
-	}
-
-	/// Attempt to remove a call from storage, returning any deposit on it to the owner.
-	fn clear_call(hash: &[u8; 32]) {
-		if let Some((_, who, deposit)) = Calls::<T>::take(hash) {
-			T::Currency::unreserve(&who, deposit);
-		}
-	}
-
 	/// The current `Timepoint`.
 	pub fn timepoint() -> Timepoint<T::BlockNumber> {
 		Timepoint {