Introduce Preimage in Pallet Collective

substrate/frame/collective/Cargo.toml

@@ -26,9 +26,11 @@ scale-info = { features = ["derive"], workspace = true }
 sp-core = { workspace = true }
 sp-io = { workspace = true }
 sp-runtime = { workspace = true }
+pallet-migrations = { workspace = true }
 
 [dev-dependencies]
 pallet-balances = { workspace = true, default-features = false }
+pallet-preimage = { workspace = true, default-features = false}
 
 [features]
 default = ["std"]
@@ -43,6 +45,7 @@ std = [
 	"sp-core/std",
 	"sp-io/std",
 	"sp-runtime/std",
+	"pallet-migrations/std"
 ]
 runtime-benchmarks = [
 	"frame-benchmarking/runtime-benchmarks",

substrate/frame/collective/src/lib.rs

@@ -60,7 +60,8 @@ use frame_support::{
 	ensure,
 	traits::{
 		Backing, ChangeMembers, Consideration, EnsureOrigin, EnsureOriginWithArg, Get, GetBacking,
-		InitializeMembers, MaybeConsideration, OriginTrait, StorageVersion,
+		InitializeMembers, MaybeConsideration, OriginTrait, QueryPreimage, StorageVersion,
+		StorePreimage,
 	},
 	weights::Weight,
 };
@@ -398,6 +399,8 @@ pub mod pallet {
 		/// consider using a constant cost (e.g., [`crate::deposit::Constant`]) equal to the minimum
 		/// balance under the `runtime-benchmarks` feature.
 		type Consideration: MaybeConsideration<Self::AccountId, u32>;
+
+		type Preimages: QueryPreimage<H = Self::Hashing> + StorePreimage;
 	}
 
 	#[pallet::genesis_config]
@@ -436,11 +439,6 @@ pub mod pallet {
 	pub type Proposals<T: Config<I>, I: 'static = ()> =
 		StorageValue<_, BoundedVec<T::Hash, T::MaxProposals>, ValueQuery>;
 
-	/// Actual proposal for a given hash, if it's current.
-	#[pallet::storage]
-	pub type ProposalOf<T: Config<I>, I: 'static = ()> =
-		StorageMap<_, Identity, T::Hash, <T as Config<I>>::Proposal, OptionQuery>;
-
 	/// Consideration cost created for publishing and storing a proposal.
 	///
 	/// Determined by [Config::Consideration] and may be not present for certain proposals (e.g. if
@@ -854,7 +852,7 @@ pub mod pallet {
 		pub fn kill(origin: OriginFor<T>, proposal_hash: T::Hash) -> DispatchResultWithPostInfo {
 			T::KillOrigin::ensure_origin(origin)?;
 			ensure!(
-				ProposalOf::<T, I>::get(&proposal_hash).is_some(),
+				Voting::<T, I>::contains_key(&proposal_hash),
 				Error::<T, I>::ProposalMissing
 			);
 			let burned = if let Some((who, cost)) = <CostOf<T, I>>::take(proposal_hash) {
@@ -888,7 +886,7 @@ pub mod pallet {
 		) -> DispatchResult {
 			ensure_signed_or_root(origin)?;
 			ensure!(
-				ProposalOf::<T, I>::get(&proposal_hash).is_none(),
+				!Voting::<T, I>::contains_key(&proposal_hash),
 				Error::<T, I>::ProposalActive
 			);
 			if let Some((who, cost)) = <CostOf<T, I>>::take(proposal_hash) {
@@ -933,7 +931,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		);
 
 		let proposal_hash = T::Hashing::hash_of(&proposal);
-		ensure!(!<ProposalOf<T, I>>::contains_key(proposal_hash), Error::<T, I>::DuplicateProposal);
+		ensure!(!Voting::<T, I>::contains_key(proposal_hash), Error::<T, I>::DuplicateProposal);
 
 		let seats = Members::<T, I>::get().len() as MemberCount;
 		let result = proposal.dispatch(RawOrigin::Members(1, seats).into());
@@ -960,7 +958,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		);
 
 		let proposal_hash = T::Hashing::hash_of(&proposal);
-		ensure!(!<ProposalOf<T, I>>::contains_key(proposal_hash), Error::<T, I>::DuplicateProposal);
+		ensure!(!Voting::<T, I>::contains_key(proposal_hash), Error::<T, I>::DuplicateProposal);
 
 		let active_proposals =
 			<Proposals<T, I>>::try_mutate(|proposals| -> Result<usize, DispatchError> {
@@ -976,7 +974,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		let index = ProposalCount::<T, I>::get();
 
 		<ProposalCount<T, I>>::mutate(|i| *i += 1);
-		<ProposalOf<T, I>>::insert(proposal_hash, proposal);
+		T::Preimages::note(proposal.encode().into())?;
+		let votes = {
+			let end = frame_system::Pallet::<T>::block_number() + T::MotionDuration::get();
+			Votes { index, threshold, ayes: vec![], nays: vec![], end }
+		};
+		<Voting<T, I>>::insert(proposal_hash, votes);
 		let votes = {
 			let end = frame_system::Pallet::<T>::block_number() + T::MotionDuration::get();
 			Votes { index, threshold, ayes: vec![], nays: vec![], end }
@@ -1135,12 +1138,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		length_bound: u32,
 		weight_bound: Weight,
 	) -> Result<(<T as Config<I>>::Proposal, usize), DispatchError> {
-		let key = ProposalOf::<T, I>::hashed_key_for(hash);
-		// read the length of the proposal storage entry directly
-		let proposal_len =
-			storage::read(&key, &mut [0; 0], 0).ok_or(Error::<T, I>::ProposalMissing)?;
+		let proposal_len = T::Preimages::len(hash).ok_or(Error::<T, I>::ProposalMissing)?;
 		ensure!(proposal_len <= length_bound, Error::<T, I>::WrongProposalLength);
-		let proposal = ProposalOf::<T, I>::get(hash).ok_or(Error::<T, I>::ProposalMissing)?;
+
+		let bytes = T::Preimages::fetch(hash, Some(proposal_len))
+			.map_err(|_| Error::<T, I>::ProposalMissing)?;
+		let proposal = <T as Config<I>>::Proposal::decode(&mut &bytes[..])
+			.map_err(|_| Error::<T, I>::ProposalMissing)?;
 		let proposal_weight = proposal.get_dispatch_info().call_weight;
 		ensure!(proposal_weight.all_lte(weight_bound), Error::<T, I>::WrongProposalWeight);
 		Ok((proposal, proposal_len as usize))
@@ -1192,7 +1196,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	// Removes a proposal from the pallet, cleaning up votes and the vector of proposals.
 	fn remove_proposal(proposal_hash: T::Hash) -> u32 {
 		// remove proposal and vote
-		ProposalOf::<T, I>::remove(&proposal_hash);
+		T::Preimages::unnote(&proposal_hash);
 		Voting::<T, I>::remove(&proposal_hash);
 		let num_proposals = Proposals::<T, I>::mutate(|proposals| {
 			proposals.retain(|h| h != &proposal_hash);
@@ -1210,12 +1214,12 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 	/// Looking at proposals:
 	///
 	/// * Each hash of a proposal that is stored inside `Proposals` must have a
-	/// call mapped to it inside the `ProposalOf` storage map.
+	/// call mapped to it inside the `Voting` storage map.
 	/// * `ProposalCount` must always be more or equal to the number of
 	/// proposals inside the `Proposals` storage value. The reason why
 	/// `ProposalCount` can be more is because when a proposal is removed the
 	/// count is not deducted.
-	/// * Count of `ProposalOf` should match the count of `Proposals`
+	/// * Count of `Voting` should match the count of `Proposals`
 	///
 	/// Looking at votes:
 	/// * The sum of aye and nay votes for a proposal can never exceed
@@ -1234,8 +1238,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 		Proposals::<T, I>::get().into_iter().try_for_each(
 			|proposal| -> Result<(), TryRuntimeError> {
 				ensure!(
-					ProposalOf::<T, I>::get(proposal).is_some(),
-					"Proposal hash from `Proposals` is not found inside the `ProposalOf` mapping."
+					Voting::<T, I>::contains_key(proposal),
+					"Proposal hash from `Proposals` is not found inside the `Voting` mapping."
 				);
 				Ok(())
 			},
@@ -1246,8 +1250,8 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
 			"The actual number of proposals is greater than `ProposalCount`"
 		);
 		ensure!(
-			Proposals::<T, I>::get().into_iter().count() == <ProposalOf<T, I>>::iter_keys().count(),
-			"Proposal count inside `Proposals` is not equal to the proposal count in `ProposalOf`"
+			Proposals::<T, I>::get().into_iter().count() == <Voting<T, I>>::iter_keys().count(),
+			"Proposal count inside `Proposals` is not equal to the proposal count in `Voting`"
 		);
 
 		Proposals::<T, I>::get().into_iter().try_for_each(

substrate/frame/collective/src/migrations/mod.rs

@@ -17,3 +17,4 @@
 
 /// Version 4.
 pub mod v4;
+pub mod v5;

substrate/frame/collective/src/migrations/v5.rs

@@ -0,0 +1,77 @@
+use super::super::*;
+use codec::{Decode, Encode, MaxEncodedLen};
+use frame_support::{
+    migrations::{SteppedMigration, SteppedMigrationError},
+    pallet_prelude::*,
+    traits::{StorePreimage},
+    weights::WeightMeter,
+};
+use scale_info::TypeInfo;
+
+#[frame_support::storage_alias]
+pub type OldProposalOf<T: Config<I>, I: 'static> =
+StorageMap<Pallet<T, I>, Identity, <T as frame_system::Config>::Hash, <T as Config<I>>::Proposal>;
+
+#[derive(Encode, Decode, MaxEncodedLen, TypeInfo, Clone, Copy, PartialEq, Eq, RuntimeDebug)]
+pub enum Cursor {
+    MigrateProposals(u32),
+    ClearStorage,
+}
+
+pub struct MigrateToV5<T, I>(PhantomData<(T, I)>);
+
+impl<T: Config<I>, I: 'static> SteppedMigration for MigrateToV5<T, I> {
+    type Cursor = Cursor;
+    type Identifier = [u8; 32];
+
+    fn id() -> Self::Identifier {
+        *b"CollectiveMigrationV5___________"
+    }
+
+    fn step(
+        cursor: Option<Self::Cursor>,
+        meter: &mut WeightMeter,
+    ) -> Result<Option<Self::Cursor>, SteppedMigrationError> {
+        let cursor = cursor.unwrap_or(Cursor::MigrateProposals(0));
+
+        match cursor {
+            Cursor::MigrateProposals(index) => {
+                let proposals = Proposals::<T, I>::get();
+                let count = proposals.len() as u32;
+
+                let mut current_index = index;
+
+                let weight_per_item = T::DbWeight::get().reads_writes(2, 2);
+
+                while current_index < count {
+                    if meter.try_consume(weight_per_item).is_err() {
+                        return Ok(Some(Cursor::MigrateProposals(current_index)));
+                    }
+
+                    let hash = proposals[current_index as usize];
+                    if let Some(proposal) = OldProposalOf::<T, I>::get(hash) {
+                        let _ = T::Preimages::note(proposal.encode().into());
+                        OldProposalOf::<T, I>::remove(hash);
+                    }
+
+                    current_index += 1;
+                }
+
+                Ok(Some(Cursor::ClearStorage))
+            },
+            Cursor::ClearStorage => {
+                let limit = 100u32;
+                let result = OldProposalOf::<T, I>::clear(limit, None);
+
+                let weight = T::DbWeight::get().reads_writes(1, 1).saturating_mul(result.loops as u64);
+                meter.consume(weight);
+
+                if result.maybe_cursor.is_some() {
+                    Ok(Some(Cursor::ClearStorage))
+                } else {
+                    Ok(None)
+                }
+            }
+        }
+    }
+}