From c27a3d7bdb359263adad2f1cfea88aefedf9ca7f Mon Sep 17 00:00:00 2001 From: DavidK Date: Wed, 2 Oct 2024 13:11:35 +0300 Subject: [PATCH] Write migration to release bonds to proposers --- Cargo.lock | 1 + .../collectives-westend/src/lib.rs | 2 + prdoc/pr_5892.prdoc | 16 ++++ substrate/frame/treasury/Cargo.toml | 1 + substrate/frame/treasury/src/benchmarking.rs | 15 ---- substrate/frame/treasury/src/lib.rs | 64 +++------------- substrate/frame/treasury/src/migration.rs | 74 +++++++++++++++++++ substrate/frame/treasury/src/weights.rs | 15 ---- 8 files changed, 104 insertions(+), 84 deletions(-) create mode 100644 prdoc/pr_5892.prdoc create mode 100644 substrate/frame/treasury/src/migration.rs diff --git a/Cargo.lock b/Cargo.lock index a572c37a40602..145b47a10e2fd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12854,6 +12854,7 @@ dependencies = [ "frame-support", "frame-system", "impl-trait-for-tuples", + "log", "pallet-balances", "pallet-utility", "parity-scale-codec", diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index a60d3ed66ac67..9ab0426557eba 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -757,6 +757,8 @@ type Migrations = ( pallet_core_fellowship::migration::MigrateV0ToV1, // unreleased pallet_core_fellowship::migration::MigrateV0ToV1, + // unreleased + pallet_treasury::migration::MigrateV0ToV1, ); /// Executive: handles dispatch to the various modules. diff --git a/prdoc/pr_5892.prdoc b/prdoc/pr_5892.prdoc new file mode 100644 index 0000000000000..1b39e7fdeb225 --- /dev/null +++ b/prdoc/pr_5892.prdoc @@ -0,0 +1,16 @@ +# 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: Add migration to clear unapproved proposals treasury from pallet + +doc: + - audience: + - Runtime Dev + description: | + It is no longer possible to create Proposals in this pallet but there + are some Proposals whose bonds are stuck. Purpose of this migration is to + clear those and return bonds to the proposers. + +crates: + - name: pallet-treasury + bump: patch diff --git a/substrate/frame/treasury/Cargo.toml b/substrate/frame/treasury/Cargo.toml index 55bdd4f7a4986..ec735bc1ec1a5 100644 --- a/substrate/frame/treasury/Cargo.toml +++ b/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 } diff --git a/substrate/frame/treasury/src/benchmarking.rs b/substrate/frame/treasury/src/benchmarking.rs index 6e429f76fd7dd..650e5376fa4b2 100644 --- a/substrate/frame/treasury/src/benchmarking.rs +++ b/substrate/frame/treasury/src/benchmarking.rs @@ -348,21 +348,6 @@ mod benchmarks { Ok(()) } - #[benchmark] - fn release_proposal_bonds() -> Result<(), BenchmarkError> { - let (_, _value, _beneficiary_lookup) = setup_proposal::(SEED); - let origin = - T::RejectOrigin::try_successful_origin().map_err(|_| BenchmarkError::Weightless)?; - - #[block] - { - let _ = Treasury::::release_proposal_bonds(origin as T::RuntimeOrigin); - } - - assert!(Spends::::get(0).is_none()); - Ok(()) - } - impl_benchmark_test_suite!( Treasury, crate::tests::ExtBuilder::default().build(), diff --git a/substrate/frame/treasury/src/lib.rs b/substrate/frame/treasury/src/lib.rs index f7430b296e2e4..28141fd1f8e0f 100644 --- a/substrate/frame/treasury/src/lib.rs +++ b/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; @@ -86,10 +87,7 @@ extern crate alloc; use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; -use alloc::{ - boxed::Box, - collections::{btree_map::BTreeMap, btree_set::BTreeSet}, -}; +use alloc::{boxed::Box, collections::btree_map::BTreeMap}; use sp_runtime::{ traits::{AccountIdConversion, CheckedAdd, Saturating, StaticLookup, Zero}, Permill, RuntimeDebug, @@ -202,7 +200,10 @@ pub mod pallet { }; use frame_system::pallet_prelude::*; + const STORAGE_VERSION: StorageVersion = StorageVersion::new(1); + #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] pub struct Pallet(PhantomData<(T, I)>); #[pallet::config] @@ -382,12 +383,6 @@ pub mod pallet { /// A spend was processed and removed from the storage. It might have been successfully /// paid or it may have expired. SpendProcessed { index: SpendIndex }, - /// An approved spend was voided. - ProposalBondReleased { - proposal_index: ProposalIndex, - amount: BalanceOf, - account: T::AccountId, - }, } /// Error for the treasury pallet. @@ -416,8 +411,6 @@ pub mod pallet { NotAttempted, /// The payment has neither failed nor succeeded yet. Inconclusive, - /// No proposals were released - NoProposalsReleased, } #[pallet::hooks] @@ -788,48 +781,6 @@ pub mod pallet { Self::deposit_event(Event::::AssetSpendVoided { index }); Ok(()) } - - /// ## Dispatch Origin - /// - /// Any user with account - /// - /// ## Details - /// - /// Releases any proposals that didn't make it into Approval yet - /// - /// ## Events - /// - /// Emits [`Event::ProposalBondReleased`] if successful. - #[pallet::call_index(9)] - #[pallet::weight(T::WeightInfo::release_proposal_bonds())] - pub fn release_proposal_bonds(origin: OriginFor) -> DispatchResultWithPostInfo { - let _who = ensure_signed(origin)?; - - let mut approval_index = BTreeSet::new(); - for approval in Approvals::::get().iter() { - approval_index.insert(*approval); - } - - let mut proposals_released = 0; - for (proposal_index, p) in Proposals::::iter() { - if !approval_index.contains(&proposal_index) { - let err_amount = T::Currency::unreserve(&p.proposer, p.bond); - debug_assert!(err_amount.is_zero()); - Proposals::::remove(proposal_index); - proposals_released += 1; - Self::deposit_event(Event::::ProposalBondReleased { - proposal_index, - amount: p.bond, - account: p.proposer, - }); - } - } - - // we don't want people spamming this function if it doesn't do any work - ensure!(proposals_released > 0, Error::::NoProposalsReleased); - - Ok(frame_support::dispatch::Pays::No.into()) - } } } @@ -849,6 +800,11 @@ impl, I: 'static> Pallet { ProposalCount::::get() } + /// Public function to proposals storage. + pub fn proposals(index: ProposalIndex) -> Option>> { + Proposals::::get(index) + } + /// Public function to approvals storage. pub fn approvals() -> BoundedVec { Approvals::::get() diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs new file mode 100644 index 0000000000000..e17c45b7573ef --- /dev/null +++ b/substrate/frame/treasury/src/migration.rs @@ -0,0 +1,74 @@ +use super::*; +use alloc::collections::BTreeSet; +use core::marker::PhantomData; +use frame_support::traits::UncheckedOnRuntimeUpgrade; + +/// The log target for this pallet. +const LOG_TARGET: &str = "runtime::treasury"; + +mod v1 { + use super::*; + pub struct MigrateToV1Impl(PhantomData<(T, I)>); + + impl, I: 'static> UncheckedOnRuntimeUpgrade for MigrateToV1Impl { + fn on_runtime_upgrade() -> frame_support::weights::Weight { + let mut approval_index = BTreeSet::new(); + for approval in Approvals::::get().iter() { + approval_index.insert(*approval); + } + + let mut proposals_released = 0; + for (proposal_index, p) in Proposals::::iter() { + if !approval_index.contains(&proposal_index) { + let err_amount = T::Currency::unreserve(&p.proposer, p.bond); + debug_assert!(err_amount.is_zero()); + Proposals::::remove(proposal_index); + log::info!( + target: LOG_TARGET, + "Released bond amount of {:?} to proposer {:?}", + p.bond, + p.proposer, + ); + proposals_released += 1; + } + } + + log::info!( + target: LOG_TARGET, + "Storage migration v1 for pallet-treasury finished, released {} proposal bonds.", + proposals_released, + ); + + // calculate and return migration weights + let approvals_read = 1; + T::DbWeight::get() + .reads_writes(proposals_released as u64 + approvals_read, proposals_released as u64) + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + Ok((Proposals::::count() as u32).encode()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let old_count = u32::decode(&mut &state[..]).expect("Known good"); + let new_count = Regions::::iter_values().count() as u32; + + ensure!( + old_count <= new_count, + "Proposals after migration should be less or equal to old proposals" + ); + Ok(()) + } + } +} + +/// Migrate the pallet storage from `0` to `1`. +pub type MigrateV0ToV1 = frame_support::migrations::VersionedMigration< + 0, + 1, + v1::MigrateToV1Impl, + Pallet, + ::DbWeight, +>; diff --git a/substrate/frame/treasury/src/weights.rs b/substrate/frame/treasury/src/weights.rs index 5fbbee1de481b..8c9c6eb1d0fbb 100644 --- a/substrate/frame/treasury/src/weights.rs +++ b/substrate/frame/treasury/src/weights.rs @@ -58,7 +58,6 @@ pub trait WeightInfo { fn payout() -> Weight; fn check_status() -> Weight; fn void_spend() -> Weight; - fn release_proposal_bonds() -> Weight; } /// Weights for `pallet_treasury` using the Substrate node and recommended hardware. @@ -169,13 +168,6 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(1_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } - fn release_proposal_bonds() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 1_000_000 picoseconds. - Weight::from_parts(2_000_000, 0) - } } // For backwards compatibility and tests. @@ -285,11 +277,4 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(1_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } - fn release_proposal_bonds() -> Weight { - // Proof Size summary in bytes: - // Measured: `0` - // Estimated: `0` - // Minimum execution time: 1_000_000 picoseconds. - Weight::from_parts(2_000_000, 0) - } }