Add migration to clear unapproved proposals from treasury pallet (#5892)

Resolves polkadot-fellows/runtimes#459

Tested with rococo/westend/kusama/polkadot runtimes

---------

Co-authored-by: DavidK <[email protected]>
Co-authored-by: Muharem <[email protected]>
Co-authored-by: Shawn Tabrizi <[email protected]>
Co-authored-by: Dónal Murray <[email protected]>

polkadot/runtime/rococo/src/lib.rs

@@ -33,6 +33,7 @@ use frame_support::{
 	dynamic_params::{dynamic_pallet_params, dynamic_params},
 	traits::FromContains,
 };
+use pallet_balances::WeightInfo;
 use pallet_nis::WithMaximumOf;
 use polkadot_primitives::{
 	slashing,
@@ -1601,6 +1602,8 @@ pub mod migrations {
 		pub const TechnicalMembershipPalletName: &'static str = "TechnicalMembership";
 		pub const TipsPalletName: &'static str = "Tips";
 		pub const PhragmenElectionPalletId: LockIdentifier = *b"phrelect";
+		/// Weight for balance unreservations
+		pub BalanceUnreserveWeight: Weight = weights::pallet_balances_balances::WeightInfo::<Runtime>::force_unreserve();
 	}
 
 	// Special Config for Gov V1 pallets, allowing us to run migrations for them without
@@ -1656,6 +1659,7 @@ pub mod migrations {
         pallet_elections_phragmen::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds<UnlockConfig>,
         pallet_democracy::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds<UnlockConfig>,
         pallet_tips::migrations::unreserve_deposits::UnreserveDeposits<UnlockConfig, ()>,
+        pallet_treasury::migration::cleanup_proposals::Migration<Runtime, (), BalanceUnreserveWeight>,
 
         // Delete all Gov v1 pallet storage key/values.
 

prdoc/pr_5892.prdoc

@@ -0,0 +1,17 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "Treasury: add migration to clean up unapproved deprecated proposals"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      It is no longer possible to create `Proposals` storage item in `pallet-treasury` due to migration from
+      governance v1 model but there are some `Proposals` whose bonds are still on hold with no way to release them.
+      The purpose of this migration is to clear `Proposals` which are stuck and return bonds to the proposers.
+
+crates:
+    - name: pallet-treasury
+      bump: patch
+    - name: rococo-runtime
+      bump: patch

substrate/frame/treasury/Cargo.toml

@@ -30,6 +30,7 @@ frame-system = { workspace = true }
 pallet-balances = { workspace = true }
 sp-runtime = { workspace = true }
 sp-core = { optional = true, workspace = true }
+log = { workspace = true }
 
 [dev-dependencies]
 sp-io = { workspace = true, default-features = true }
@@ -43,6 +44,7 @@ std = [
 	"frame-benchmarking?/std",
 	"frame-support/std",
 	"frame-system/std",
+	"log/std",
 	"pallet-balances/std",
 	"pallet-utility/std",
 	"scale-info/std",

substrate/frame/treasury/src/lib.rs

@@ -73,6 +73,7 @@
 #![cfg_attr(not(feature = "std"), no_std)]
 
 mod benchmarking;
+pub mod migration;
 #[cfg(test)]
 mod tests;
 pub mod weights;

substrate/frame/treasury/src/migration.rs

@@ -0,0 +1,133 @@
+// This file is part of Substrate.
+
+// Copyright (C) Parity Technologies (UK) Ltd.
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// 	http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//! Treasury pallet migrations.
+
+use super::*;
+use alloc::collections::BTreeSet;
+#[cfg(feature = "try-runtime")]
+use alloc::vec::Vec;
+use core::marker::PhantomData;
+use frame_support::{defensive, traits::OnRuntimeUpgrade};
+
+/// The log target for this pallet.
+const LOG_TARGET: &str = "runtime::treasury";
+
+pub mod cleanup_proposals {
+	use super::*;
+
+	/// Migration to cleanup unapproved proposals to return the bonds back to the proposers.
+	/// Proposals can no longer be created and the `Proposal` storage item will be removed in the
+	/// future.
+	///
+	/// `UnreserveWeight` returns `Weight` of `unreserve_balance` operation which is perfomed during
+	/// this migration.
+	pub struct Migration<T, I, UnreserveWeight>(PhantomData<(T, I, UnreserveWeight)>);
+
+	impl<T: Config<I>, I: 'static, UnreserveWeight: Get<Weight>> OnRuntimeUpgrade
+		for Migration<T, I, UnreserveWeight>
+	{
+		fn on_runtime_upgrade() -> frame_support::weights::Weight {
+			let mut approval_index = BTreeSet::new();
+			for approval in Approvals::<T, I>::get().iter() {
+				approval_index.insert(*approval);
+			}
+
+			let mut proposals_processed = 0;
+			for (proposal_index, p) in Proposals::<T, I>::iter() {
+				if !approval_index.contains(&proposal_index) {
+					let err_amount = T::Currency::unreserve(&p.proposer, p.bond);
+					if err_amount.is_zero() {
+						Proposals::<T, I>::remove(proposal_index);
+						log::info!(
+							target: LOG_TARGET,
+							"Released bond amount of {:?} to proposer {:?}",
+							p.bond,
+							p.proposer,
+						);
+					} else {
+						defensive!(
+							"err_amount is non zero for proposal {:?}",
+							(proposal_index, err_amount)
+						);
+						Proposals::<T, I>::mutate_extant(proposal_index, |proposal| {
+							proposal.value = err_amount;
+						});
+						log::info!(
+							target: LOG_TARGET,
+							"Released partial bond amount of {:?} to proposer {:?}",
+							p.bond - err_amount,
+							p.proposer,
+						);
+					}
+					proposals_processed += 1;
+				}
+			}
+
+			log::info!(
+				target: LOG_TARGET,
+				"Migration for pallet-treasury finished, released {} proposal bonds.",
+				proposals_processed,
+			);
+
+			// calculate and return migration weights
+			let approvals_read = 1;
+			T::DbWeight::get().reads_writes(
+				proposals_processed as u64 + approvals_read,
+				proposals_processed as u64,
+			) + UnreserveWeight::get() * proposals_processed
+		}
+
+		#[cfg(feature = "try-runtime")]
+		fn pre_upgrade() -> Result<Vec<u8>, sp_runtime::TryRuntimeError> {
+			let value = (
+				Proposals::<T, I>::iter_values().count() as u32,
+				Approvals::<T, I>::get().len() as u32,
+			);
+			log::info!(
+				target: LOG_TARGET,
+				"Proposals and Approvals count {:?}",
+				value,
+			);
+			Ok(value.encode())
+		}
+
+		#[cfg(feature = "try-runtime")]
+		fn post_upgrade(state: Vec<u8>) -> Result<(), sp_runtime::TryRuntimeError> {
+			let (old_proposals_count, old_approvals_count) =
+				<(u32, u32)>::decode(&mut &state[..]).expect("Known good");
+			let new_proposals_count = Proposals::<T, I>::iter_values().count() as u32;
+			let new_approvals_count = Approvals::<T, I>::get().len() as u32;
+
+			log::info!(
+				target: LOG_TARGET,
+				"Proposals and Approvals count {:?}",
+				(new_proposals_count, new_approvals_count),
+			);
+
+			ensure!(
+				new_proposals_count <= old_proposals_count,
+				"Proposals after migration should be less or equal to old proposals"
+			);
+			ensure!(
+				new_approvals_count == old_approvals_count,
+				"Approvals after migration should remain the same"
+			);
+			Ok(())
+		}
+	}
+}