From 547fc0d0033a7bb5d9879c2c0a05a03c19d990de Mon Sep 17 00:00:00 2001 From: DavidK Date: Tue, 1 Oct 2024 16:19:37 +0300 Subject: [PATCH 01/23] Add function to clear unapproved proposals treasury from pallet --- substrate/frame/treasury/src/benchmarking.rs | 15 +++++ substrate/frame/treasury/src/lib.rs | 60 ++++++++++++++++++-- substrate/frame/treasury/src/weights.rs | 15 +++++ 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/substrate/frame/treasury/src/benchmarking.rs b/substrate/frame/treasury/src/benchmarking.rs index 650e5376fa4b..6e429f76fd7d 100644 --- a/substrate/frame/treasury/src/benchmarking.rs +++ b/substrate/frame/treasury/src/benchmarking.rs @@ -348,6 +348,21 @@ 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 edb39f230642..f7430b296e2e 100644 --- a/substrate/frame/treasury/src/lib.rs +++ b/substrate/frame/treasury/src/lib.rs @@ -86,7 +86,10 @@ extern crate alloc; use codec::{Decode, Encode, MaxEncodedLen}; use scale_info::TypeInfo; -use alloc::{boxed::Box, collections::btree_map::BTreeMap}; +use alloc::{ + boxed::Box, + collections::{btree_map::BTreeMap, btree_set::BTreeSet}, +}; use sp_runtime::{ traits::{AccountIdConversion, CheckedAdd, Saturating, StaticLookup, Zero}, Permill, RuntimeDebug, @@ -379,6 +382,12 @@ 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. @@ -407,6 +416,8 @@ pub mod pallet { NotAttempted, /// The payment has neither failed nor succeeded yet. Inconclusive, + /// No proposals were released + NoProposalsReleased, } #[pallet::hooks] @@ -777,6 +788,48 @@ 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()) + } } } @@ -796,11 +849,6 @@ 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/weights.rs b/substrate/frame/treasury/src/weights.rs index 8c9c6eb1d0fb..5fbbee1de481 100644 --- a/substrate/frame/treasury/src/weights.rs +++ b/substrate/frame/treasury/src/weights.rs @@ -58,6 +58,7 @@ 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. @@ -168,6 +169,13 @@ 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. @@ -277,4 +285,11 @@ 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) + } } From 766c3a9e5217d87a6e8b1d6c4fc311f62ac72c74 Mon Sep 17 00:00:00 2001 From: DavidK Date: Wed, 2 Oct 2024 13:11:35 +0300 Subject: [PATCH 02/23] Write migration to release bonds to proposers --- Cargo.lock | 1 + .../collectives-westend/src/lib.rs | 2 + prdoc/pr_5892.prdoc | 15 ++++ 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, 103 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 a572c37a4060..145b47a10e2f 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 a60d3ed66ac6..9ab0426557eb 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 000000000000..a67dc422ecbe --- /dev/null +++ b/prdoc/pr_5892.prdoc @@ -0,0 +1,15 @@ +# 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 55bdd4f7a498..ec735bc1ec1a 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 6e429f76fd7d..650e5376fa4b 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 f7430b296e2e..28141fd1f8e0 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 000000000000..e17c45b7573e --- /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 5fbbee1de481..8c9c6eb1d0fb 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) - } } From 41d1ad732f6b8fc1fa44db3e8be49473b7880938 Mon Sep 17 00:00:00 2001 From: DavidK Date: Wed, 2 Oct 2024 13:33:37 +0300 Subject: [PATCH 03/23] Add treasury migration to rococo runtime --- polkadot/runtime/rococo/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index e1826e4100b6..8bac7cb14f60 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1677,6 +1677,9 @@ pub mod migrations { // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, parachains_inclusion::migration::MigrateToV1, + + // Release bonds stuck in proposals + pallet_treasury::migration::MigrateV0ToV1, ); } From a78afec7e4f3c689024ac12b89e2c20d0a24c51e Mon Sep 17 00:00:00 2001 From: DavidK Date: Wed, 2 Oct 2024 13:42:50 +0300 Subject: [PATCH 04/23] Fix compilation with try-runtime feature flag --- substrate/frame/treasury/src/migration.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index e17c45b7573e..331ff462daf5 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -47,13 +47,13 @@ mod v1 { #[cfg(feature = "try-runtime")] fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { - Ok((Proposals::::count() as u32).encode()) + Ok((Proposals::::iter_values().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; + let new_count = Proposals::::iter_values().count() as u32; ensure!( old_count <= new_count, From 917bd84f2a71e98c7bb11c188f19ed7a94e151a3 Mon Sep 17 00:00:00 2001 From: DavidK Date: Thu, 3 Oct 2024 10:54:11 +0300 Subject: [PATCH 05/23] Address Muharem's comments --- .../collectives-westend/src/lib.rs | 2 +- polkadot/runtime/rococo/src/lib.rs | 2 +- prdoc/pr_5892.prdoc | 6 +- substrate/frame/treasury/src/lib.rs | 3 - substrate/frame/treasury/src/migration.rs | 131 +++++++++++------- 5 files changed, 87 insertions(+), 57 deletions(-) diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index 9ab0426557eb..266e23fe8802 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -758,7 +758,7 @@ type Migrations = ( // unreleased pallet_core_fellowship::migration::MigrateV0ToV1, // unreleased - pallet_treasury::migration::MigrateV0ToV1, + pallet_treasury::migration::ReleaseHeldProposals, ); /// Executive: handles dispatch to the various modules. diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 8bac7cb14f60..a428234b25a0 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1679,7 +1679,7 @@ pub mod migrations { parachains_inclusion::migration::MigrateToV1, // Release bonds stuck in proposals - pallet_treasury::migration::MigrateV0ToV1, + pallet_treasury::migration::ReleaseHeldProposals, ); } diff --git a/prdoc/pr_5892.prdoc b/prdoc/pr_5892.prdoc index a67dc422ecbe..96c9f1084e01 100644 --- a/prdoc/pr_5892.prdoc +++ b/prdoc/pr_5892.prdoc @@ -6,9 +6,9 @@ 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. + 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 in hold with no way to release them. + Purpose of this migration is to clear `Proposals` which are stuck and return bonds to the proposers. crates: - name: pallet-treasury diff --git a/substrate/frame/treasury/src/lib.rs b/substrate/frame/treasury/src/lib.rs index 28141fd1f8e0..ad74495ce090 100644 --- a/substrate/frame/treasury/src/lib.rs +++ b/substrate/frame/treasury/src/lib.rs @@ -200,10 +200,7 @@ 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] diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index 331ff462daf5..11c66cb1a0dc 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -1,27 +1,47 @@ +// 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; use core::marker::PhantomData; -use frame_support::traits::UncheckedOnRuntimeUpgrade; +use frame_support::{defensive, traits::OnRuntimeUpgrade}; +use pallet_balances::WeightInfo; /// The log target for this pallet. const LOG_TARGET: &str = "runtime::treasury"; -mod v1 { - use super::*; - pub struct MigrateToV1Impl(PhantomData<(T, I)>); +pub struct ReleaseHeldProposals(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); - } +impl + pallet_balances::Config, I: 'static> OnRuntimeUpgrade + for ReleaseHeldProposals +{ + 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()); + 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); + if err_amount.is_zero() { Proposals::::remove(proposal_index); log::info!( target: LOG_TARGET, @@ -29,46 +49,59 @@ mod v1 { p.bond, p.proposer, ); - proposals_released += 1; + } else { + defensive!( + "err_amount is non zero for proposal {:?}", + (proposal_index, err_amount) + ); + Proposals::::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_released += 1; } + } - log::info!( - target: LOG_TARGET, - "Storage migration v1 for pallet-treasury finished, released {} proposal bonds.", - proposals_released, - ); + log::info!( + target: LOG_TARGET, + "Migration 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) - } + // calculate and return migration weights + let approvals_read = 1; + T::DbWeight::get() + .reads_writes(proposals_released as u64 + approvals_read, proposals_released as u64) + + ::WeightInfo::force_unreserve() * proposals_released + } - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { - Ok((Proposals::::iter_values().count() as u32).encode()) - } + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + Ok((Proposals::::iter_values().count() as u32, Approvals::::get().len() 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 = Proposals::::iter_values().count() as u32; + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let (old_proposals_count, old_approvals_count) = + <(u32, u32)>::decode(&mut &state[..]).expect("Known good"); + let new_proposals_count = Proposals::::iter_values().count() as u32; + let new_approvals_count = Approvals::::get().len() as u32; - ensure!( - old_count <= new_count, - "Proposals after migration should be less or equal to old proposals" - ); - Ok(()) - } + 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(()) } } - -/// Migrate the pallet storage from `0` to `1`. -pub type MigrateV0ToV1 = frame_support::migrations::VersionedMigration< - 0, - 1, - v1::MigrateToV1Impl, - Pallet, - ::DbWeight, ->; From a3c7ad2ad9f9bcdeb36433c3e3bcaed29a8fad0b Mon Sep 17 00:00:00 2001 From: DavidK Date: Thu, 3 Oct 2024 11:04:23 +0300 Subject: [PATCH 06/23] Rename to CleanupProposals --- .../runtimes/collectives/collectives-westend/src/lib.rs | 2 +- polkadot/runtime/rococo/src/lib.rs | 2 +- prdoc/pr_5892.prdoc | 2 +- substrate/frame/treasury/src/migration.rs | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index 266e23fe8802..f937b5d94561 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -758,7 +758,7 @@ type Migrations = ( // unreleased pallet_core_fellowship::migration::MigrateV0ToV1, // unreleased - pallet_treasury::migration::ReleaseHeldProposals, + pallet_treasury::migration::CleanupProposals, ); /// Executive: handles dispatch to the various modules. diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index a428234b25a0..07da3db5cc49 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -1679,7 +1679,7 @@ pub mod migrations { parachains_inclusion::migration::MigrateToV1, // Release bonds stuck in proposals - pallet_treasury::migration::ReleaseHeldProposals, + pallet_treasury::migration::CleanupProposals, ); } diff --git a/prdoc/pr_5892.prdoc b/prdoc/pr_5892.prdoc index 96c9f1084e01..518ffa411996 100644 --- a/prdoc/pr_5892.prdoc +++ b/prdoc/pr_5892.prdoc @@ -1,7 +1,7 @@ # 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 +title: Add migration to clean unapproved proposals treasury from pallet doc: - audience: Runtime Dev diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index 11c66cb1a0dc..76dd6fa55678 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -26,10 +26,10 @@ use pallet_balances::WeightInfo; /// The log target for this pallet. const LOG_TARGET: &str = "runtime::treasury"; -pub struct ReleaseHeldProposals(PhantomData<(T, I)>); +pub struct CleanupProposals(PhantomData<(T, I)>); impl + pallet_balances::Config, I: 'static> OnRuntimeUpgrade - for ReleaseHeldProposals + for CleanupProposals { fn on_runtime_upgrade() -> frame_support::weights::Weight { let mut approval_index = BTreeSet::new(); From 4fd8531de520716efe771ac39a6432bda1056684 Mon Sep 17 00:00:00 2001 From: davidk-pt Date: Fri, 4 Oct 2024 16:01:22 +0300 Subject: [PATCH 07/23] Update prdoc/pr_5892.prdoc Co-authored-by: Muharem --- prdoc/pr_5892.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_5892.prdoc b/prdoc/pr_5892.prdoc index 518ffa411996..e20b1df84569 100644 --- a/prdoc/pr_5892.prdoc +++ b/prdoc/pr_5892.prdoc @@ -1,7 +1,7 @@ # 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 clean unapproved proposals treasury from pallet +title: Treasury: add migration to clean up unapproved deprecated proposals doc: - audience: Runtime Dev From 202fa688dacfdff8957e6af5c80c6e1d512952f9 Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 16:50:51 +0300 Subject: [PATCH 08/23] Added miration to westend --- .../collectives-westend/src/lib.rs | 2 - polkadot/runtime/rococo/src/impls.rs | 12 ++ polkadot/runtime/rococo/src/lib.rs | 6 +- polkadot/runtime/westend/src/impls.rs | 12 ++ polkadot/runtime/westend/src/lib.rs | 8 +- substrate/frame/treasury/src/migration.rs | 173 +++++++++++------- 6 files changed, 137 insertions(+), 76 deletions(-) diff --git a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs index f937b5d94561..a60d3ed66ac6 100644 --- a/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs +++ b/cumulus/parachains/runtimes/collectives/collectives-westend/src/lib.rs @@ -757,8 +757,6 @@ type Migrations = ( pallet_core_fellowship::migration::MigrateV0ToV1, // unreleased pallet_core_fellowship::migration::MigrateV0ToV1, - // unreleased - pallet_treasury::migration::CleanupProposals, ); /// Executive: handles dispatch to the various modules. diff --git a/polkadot/runtime/rococo/src/impls.rs b/polkadot/runtime/rococo/src/impls.rs index f01440ea02bc..7f11e1a7dcf7 100644 --- a/polkadot/runtime/rococo/src/impls.rs +++ b/polkadot/runtime/rococo/src/impls.rs @@ -23,6 +23,7 @@ use frame_system::RawOrigin; use polkadot_primitives::Balance; use polkadot_runtime_common::identity_migrator::{OnReapIdentity, WeightInfo}; use rococo_runtime_constants::currency::*; +use sp_core::Get; use xcm::{latest::prelude::*, VersionedLocation, VersionedXcm}; use xcm_executor::traits::TransactAsset; @@ -42,6 +43,17 @@ enum IdentityMigratorCalls { PokeDeposit(AccountId), } +/// Balance unreserve weight +pub struct BalanceUnreserveWeight(PhantomData); + +impl Get for BalanceUnreserveWeight { + fn get() -> Weight { + use pallet_balances::WeightInfo; + + T::WeightInfo::force_unreserve() + } +} + /// Type that implements `OnReapIdentity` that will send the deposit needed to store the same /// information on a parachain, sends the deposit there, and then updates it. pub struct ToParachainIdentityReaper(PhantomData<(Runtime, AccountId)>); diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 07da3db5cc49..29d98ed070ac 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -129,7 +129,7 @@ pub mod xcm_config; // Implemented types. mod impls; -use impls::ToParachainIdentityReaper; +use impls::{BalanceUnreserveWeight, ToParachainIdentityReaper}; // Governance and configurations. pub mod governance; @@ -1654,6 +1654,7 @@ pub mod migrations { pallet_elections_phragmen::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds, pallet_democracy::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds, pallet_tips::migrations::unreserve_deposits::UnreserveDeposits, + pallet_treasury::migration::cleanup_proposals::Migration>, // Delete all Gov v1 pallet storage key/values. @@ -1677,9 +1678,6 @@ pub mod migrations { // permanent pallet_xcm::migration::MigrateToLatestXcmVersion, parachains_inclusion::migration::MigrateToV1, - - // Release bonds stuck in proposals - pallet_treasury::migration::CleanupProposals, ); } diff --git a/polkadot/runtime/westend/src/impls.rs b/polkadot/runtime/westend/src/impls.rs index ac3f9e679f8d..7206acbeafa6 100644 --- a/polkadot/runtime/westend/src/impls.rs +++ b/polkadot/runtime/westend/src/impls.rs @@ -22,6 +22,7 @@ use frame_support::pallet_prelude::DispatchResult; use frame_system::RawOrigin; use polkadot_primitives::Balance; use polkadot_runtime_common::identity_migrator::{OnReapIdentity, WeightInfo}; +use sp_core::Get; use westend_runtime_constants::currency::*; use xcm::{latest::prelude::*, VersionedLocation, VersionedXcm}; use xcm_executor::traits::TransactAsset; @@ -80,6 +81,17 @@ impl ToParachainIdentityReaper { } } +/// Balance unreserve weight +pub struct BalanceUnreserveWeight(PhantomData); + +impl Get for BalanceUnreserveWeight { + fn get() -> Weight { + use pallet_balances::WeightInfo; + + T::WeightInfo::force_unreserve() + } +} + // Note / Warning: This implementation should only be used in a transactional context. If not, then // an error could result in assets being burned. impl OnReapIdentity for ToParachainIdentityReaper diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 7324485256a1..fcc29c45ed62 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -139,7 +139,7 @@ pub mod xcm_config; // Implemented types. mod impls; -use impls::ToParachainIdentityReaper; +use impls::{BalanceUnreserveWeight, ToParachainIdentityReaper}; // Governance and configurations. pub mod governance; @@ -1784,6 +1784,12 @@ pub mod migrations { Runtime, MaxAgentsToMigrate, >, + // cleanup stuck proposals + pallet_treasury::migration::cleanup_proposals::Migration< + Runtime, + (), + BalanceUnreserveWeight, + >, ); } diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index 76dd6fa55678..b9f5f3093c93 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -19,89 +19,124 @@ use super::*; use alloc::collections::BTreeSet; +#[cfg(feature = "try-runtime")] +use alloc::vec::Vec; use core::marker::PhantomData; use frame_support::{defensive, traits::OnRuntimeUpgrade}; -use pallet_balances::WeightInfo; /// The log target for this pallet. const LOG_TARGET: &str = "runtime::treasury"; -pub struct CleanupProposals(PhantomData<(T, I)>); +pub mod cleanup_proposals { + use super::*; -impl + pallet_balances::Config, I: 'static> OnRuntimeUpgrade - for CleanupProposals -{ - fn on_runtime_upgrade() -> frame_support::weights::Weight { - let mut approval_index = BTreeSet::new(); - for approval in Approvals::::get().iter() { - approval_index.insert(*approval); - } + /// Migration to cleanup unapproved proposals to return the bonds back to the proposers. + /// Proposals storage item can no longer be created and will be removed in the future. + /// + /// `` + /// + /// Example to provide weight info for migration using [`pallet_balances::Config`]: + /// ``` + /// pub struct BalanceUnreserveWeight(PhantomData); + /// + /// impl Get for BalanceUnreserveWeight { + /// fn get() -> Weight { + /// T::WeightInfo::force_unreserve() + /// } + /// } + /// ``` + pub struct Migration(PhantomData<(T, I, UnreserveWeight)>); + + impl + pallet_balances::Config, I: 'static, UnreserveWeight: Get> + OnRuntimeUpgrade for Migration + { + 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); - if err_amount.is_zero() { - Proposals::::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::::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, - ); + let mut proposals_processed = 0; + for (proposal_index, p) in Proposals::::iter() { + if !approval_index.contains(&proposal_index) { + let err_amount = T::Currency::unreserve(&p.proposer, p.bond); + if err_amount.is_zero() { + Proposals::::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::::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; } - proposals_released += 1; } - } - log::info!( - target: LOG_TARGET, - "Migration for pallet-treasury finished, released {} proposal bonds.", - proposals_released, - ); + 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_released as u64 + approvals_read, proposals_released as u64) + - ::WeightInfo::force_unreserve() * proposals_released - } + // 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, sp_runtime::TryRuntimeError> { - Ok((Proposals::::iter_values().count() as u32, Approvals::::get().len() as u32) - .encode()) - } + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result, sp_runtime::TryRuntimeError> { + let value = ( + Proposals::::iter_values().count() as u32, + Approvals::::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) -> Result<(), sp_runtime::TryRuntimeError> { - let (old_proposals_count, old_approvals_count) = - <(u32, u32)>::decode(&mut &state[..]).expect("Known good"); - let new_proposals_count = Proposals::::iter_values().count() as u32; - let new_approvals_count = Approvals::::get().len() as u32; + #[cfg(feature = "try-runtime")] + fn post_upgrade(state: Vec) -> Result<(), sp_runtime::TryRuntimeError> { + let (old_proposals_count, old_approvals_count) = + <(u32, u32)>::decode(&mut &state[..]).expect("Known good"); + let new_proposals_count = Proposals::::iter_values().count() as u32; + let new_approvals_count = Approvals::::get().len() as u32; - 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(()) + 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(()) + } } } From 7a027c321d8ca0e427b67bd0bf2020dd90ec0800 Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 16:53:48 +0300 Subject: [PATCH 09/23] Fix prdoc --- prdoc/pr_5892.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_5892.prdoc b/prdoc/pr_5892.prdoc index e20b1df84569..d85cee8b612d 100644 --- a/prdoc/pr_5892.prdoc +++ b/prdoc/pr_5892.prdoc @@ -1,7 +1,7 @@ # 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 +title: "Treasury: add migration to clean up unapproved deprecated proposals" doc: - audience: Runtime Dev From d45feab25e5130fc8465ac8759a9ac2bf7d0aaea Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 17:02:27 +0300 Subject: [PATCH 10/23] Add log/std feature propogation --- substrate/frame/treasury/Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/substrate/frame/treasury/Cargo.toml b/substrate/frame/treasury/Cargo.toml index ec735bc1ec1a..452d73f338c3 100644 --- a/substrate/frame/treasury/Cargo.toml +++ b/substrate/frame/treasury/Cargo.toml @@ -51,6 +51,7 @@ std = [ "sp-core?/std", "sp-io/std", "sp-runtime/std", + "log/std", ] runtime-benchmarks = [ "dep:sp-core", From 47bf1d31631c5bde0a1fffe69a667687a3f87fac Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 17:08:47 +0300 Subject: [PATCH 11/23] Taplo format log/std --- substrate/frame/treasury/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/treasury/Cargo.toml b/substrate/frame/treasury/Cargo.toml index 452d73f338c3..93a3d9bea93d 100644 --- a/substrate/frame/treasury/Cargo.toml +++ b/substrate/frame/treasury/Cargo.toml @@ -44,6 +44,7 @@ std = [ "frame-benchmarking?/std", "frame-support/std", "frame-system/std", + "log/std", "pallet-balances/std", "pallet-utility/std", "scale-info/std", @@ -51,7 +52,6 @@ std = [ "sp-core?/std", "sp-io/std", "sp-runtime/std", - "log/std", ] runtime-benchmarks = [ "dep:sp-core", From 438aec05880368fc040052985df773d86ef69fed Mon Sep 17 00:00:00 2001 From: davidk-pt Date: Fri, 4 Oct 2024 17:49:54 +0300 Subject: [PATCH 12/23] Update substrate/frame/treasury/src/migration.rs Co-authored-by: Muharem --- substrate/frame/treasury/src/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index b9f5f3093c93..a352dbe002fa 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -31,7 +31,7 @@ pub mod cleanup_proposals { use super::*; /// Migration to cleanup unapproved proposals to return the bonds back to the proposers. - /// Proposals storage item can no longer be created and will be removed in the future. + /// Proposals can no longer be created and the `Proposal` storage item will be removed in the future. /// /// `` /// From 6e29f9ad1df6bb34e1d5fed905b2aa623ec35b42 Mon Sep 17 00:00:00 2001 From: davidk-pt Date: Fri, 4 Oct 2024 17:50:01 +0300 Subject: [PATCH 13/23] Update substrate/frame/treasury/src/migration.rs Co-authored-by: Muharem --- substrate/frame/treasury/src/migration.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index a352dbe002fa..79e6a4334f17 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -33,8 +33,6 @@ pub mod cleanup_proposals { /// 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. /// - /// `` - /// /// Example to provide weight info for migration using [`pallet_balances::Config`]: /// ``` /// pub struct BalanceUnreserveWeight(PhantomData); From 3765bb491cebfeee0b6be7f65d0cca3d7c478362 Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 17:25:29 +0300 Subject: [PATCH 14/23] Add changed runtimes to prdoc --- prdoc/pr_5892.prdoc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/prdoc/pr_5892.prdoc b/prdoc/pr_5892.prdoc index d85cee8b612d..04d9e03273dc 100644 --- a/prdoc/pr_5892.prdoc +++ b/prdoc/pr_5892.prdoc @@ -13,3 +13,7 @@ doc: crates: - name: pallet-treasury bump: patch + - name: rococo-runtime + bump: patch + - name: westend-runtime + bump: patch From ff8ce16085572364c376090da0e2116de565f5a9 Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 17:48:10 +0300 Subject: [PATCH 15/23] Address more comments --- polkadot/runtime/rococo/src/impls.rs | 12 ------------ polkadot/runtime/rococo/src/lib.rs | 7 +++++-- polkadot/runtime/westend/src/impls.rs | 12 ------------ polkadot/runtime/westend/src/lib.rs | 7 +++++-- substrate/frame/treasury/src/migration.rs | 11 +---------- 5 files changed, 11 insertions(+), 38 deletions(-) diff --git a/polkadot/runtime/rococo/src/impls.rs b/polkadot/runtime/rococo/src/impls.rs index 7f11e1a7dcf7..f01440ea02bc 100644 --- a/polkadot/runtime/rococo/src/impls.rs +++ b/polkadot/runtime/rococo/src/impls.rs @@ -23,7 +23,6 @@ use frame_system::RawOrigin; use polkadot_primitives::Balance; use polkadot_runtime_common::identity_migrator::{OnReapIdentity, WeightInfo}; use rococo_runtime_constants::currency::*; -use sp_core::Get; use xcm::{latest::prelude::*, VersionedLocation, VersionedXcm}; use xcm_executor::traits::TransactAsset; @@ -43,17 +42,6 @@ enum IdentityMigratorCalls { PokeDeposit(AccountId), } -/// Balance unreserve weight -pub struct BalanceUnreserveWeight(PhantomData); - -impl Get for BalanceUnreserveWeight { - fn get() -> Weight { - use pallet_balances::WeightInfo; - - T::WeightInfo::force_unreserve() - } -} - /// Type that implements `OnReapIdentity` that will send the deposit needed to store the same /// information on a parachain, sends the deposit there, and then updates it. pub struct ToParachainIdentityReaper(PhantomData<(Runtime, AccountId)>); diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 29d98ed070ac..12d8213a181e 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/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, @@ -129,7 +130,7 @@ pub mod xcm_config; // Implemented types. mod impls; -use impls::{BalanceUnreserveWeight, ToParachainIdentityReaper}; +use impls::ToParachainIdentityReaper; // Governance and configurations. pub mod governance; @@ -1599,6 +1600,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::::force_unreserve(); } // Special Config for Gov V1 pallets, allowing us to run migrations for them without @@ -1654,7 +1657,7 @@ pub mod migrations { pallet_elections_phragmen::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds, pallet_democracy::migrations::unlock_and_unreserve_all_funds::UnlockAndUnreserveAllFunds, pallet_tips::migrations::unreserve_deposits::UnreserveDeposits, - pallet_treasury::migration::cleanup_proposals::Migration>, + pallet_treasury::migration::cleanup_proposals::Migration, // Delete all Gov v1 pallet storage key/values. diff --git a/polkadot/runtime/westend/src/impls.rs b/polkadot/runtime/westend/src/impls.rs index 7206acbeafa6..ac3f9e679f8d 100644 --- a/polkadot/runtime/westend/src/impls.rs +++ b/polkadot/runtime/westend/src/impls.rs @@ -22,7 +22,6 @@ use frame_support::pallet_prelude::DispatchResult; use frame_system::RawOrigin; use polkadot_primitives::Balance; use polkadot_runtime_common::identity_migrator::{OnReapIdentity, WeightInfo}; -use sp_core::Get; use westend_runtime_constants::currency::*; use xcm::{latest::prelude::*, VersionedLocation, VersionedXcm}; use xcm_executor::traits::TransactAsset; @@ -81,17 +80,6 @@ impl ToParachainIdentityReaper { } } -/// Balance unreserve weight -pub struct BalanceUnreserveWeight(PhantomData); - -impl Get for BalanceUnreserveWeight { - fn get() -> Weight { - use pallet_balances::WeightInfo; - - T::WeightInfo::force_unreserve() - } -} - // Note / Warning: This implementation should only be used in a transactional context. If not, then // an error could result in assets being burned. impl OnReapIdentity for ToParachainIdentityReaper diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index fcc29c45ed62..e592085e7755 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -123,6 +123,7 @@ pub use pallet_timestamp::Call as TimestampCall; use sp_runtime::traits::Get; #[cfg(any(feature = "std", test))] pub use sp_runtime::BuildStorage; +use pallet_balances::WeightInfo; /// Constant values used within the runtime. use westend_runtime_constants::{ @@ -139,7 +140,7 @@ pub mod xcm_config; // Implemented types. mod impls; -use impls::{BalanceUnreserveWeight, ToParachainIdentityReaper}; +use impls::ToParachainIdentityReaper; // Governance and configurations. pub mod governance; @@ -1764,6 +1765,8 @@ pub type SignedExtra = ( parameter_types! { /// Bounding number of agent pot accounts to be migrated in a single block. pub const MaxAgentsToMigrate: u32 = 300; + /// Weight for balance unreservations + pub BalanceUnreserveWeight: Weight = weights::pallet_balances::WeightInfo::::force_unreserve(); } /// All migrations that will run on the next runtime upgrade. @@ -1788,7 +1791,7 @@ pub mod migrations { pallet_treasury::migration::cleanup_proposals::Migration< Runtime, (), - BalanceUnreserveWeight, + BalanceUnreserveWeight, >, ); } diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index 79e6a4334f17..2da51742b1d3 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -33,16 +33,7 @@ pub mod cleanup_proposals { /// 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. /// - /// Example to provide weight info for migration using [`pallet_balances::Config`]: - /// ``` - /// pub struct BalanceUnreserveWeight(PhantomData); - /// - /// impl Get for BalanceUnreserveWeight { - /// fn get() -> Weight { - /// T::WeightInfo::force_unreserve() - /// } - /// } - /// ``` + /// `UnreserveWeight` returns `Weight` of `unreserve_balance` operation which is perfomed during this migration. pub struct Migration(PhantomData<(T, I, UnreserveWeight)>); impl + pallet_balances::Config, I: 'static, UnreserveWeight: Get> From 34fcbfa9bcaea3e26903a4c28da95fd0b682223b Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 18:22:27 +0300 Subject: [PATCH 16/23] Remove westend treasury pallet migration --- polkadot/runtime/rococo/src/lib.rs | 2 +- polkadot/runtime/westend/src/lib.rs | 9 --------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index 12d8213a181e..fef25527e82a 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -169,7 +169,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("rococo"), impl_name: create_runtime_str!("parity-rococo-v2.0"), authoring_version: 0, - spec_version: 1_016_001, + spec_version: 1_016_002, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 26, diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index e592085e7755..7324485256a1 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -123,7 +123,6 @@ pub use pallet_timestamp::Call as TimestampCall; use sp_runtime::traits::Get; #[cfg(any(feature = "std", test))] pub use sp_runtime::BuildStorage; -use pallet_balances::WeightInfo; /// Constant values used within the runtime. use westend_runtime_constants::{ @@ -1765,8 +1764,6 @@ pub type SignedExtra = ( parameter_types! { /// Bounding number of agent pot accounts to be migrated in a single block. pub const MaxAgentsToMigrate: u32 = 300; - /// Weight for balance unreservations - pub BalanceUnreserveWeight: Weight = weights::pallet_balances::WeightInfo::::force_unreserve(); } /// All migrations that will run on the next runtime upgrade. @@ -1787,12 +1784,6 @@ pub mod migrations { Runtime, MaxAgentsToMigrate, >, - // cleanup stuck proposals - pallet_treasury::migration::cleanup_proposals::Migration< - Runtime, - (), - BalanceUnreserveWeight, - >, ); } From ac22408d5288504f87efa9b6a40fe887e283dcdb Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 18:26:55 +0300 Subject: [PATCH 17/23] Update prdoc --- prdoc/pr_5892.prdoc | 2 -- 1 file changed, 2 deletions(-) diff --git a/prdoc/pr_5892.prdoc b/prdoc/pr_5892.prdoc index 04d9e03273dc..2b8a7c6ff4ee 100644 --- a/prdoc/pr_5892.prdoc +++ b/prdoc/pr_5892.prdoc @@ -15,5 +15,3 @@ crates: bump: patch - name: rococo-runtime bump: patch - - name: westend-runtime - bump: patch From 6e3b7b35f86c2d1b7edcbb9c83753809719fe828 Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 18:29:31 +0300 Subject: [PATCH 18/23] Decrease rococo runtime version --- polkadot/runtime/rococo/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/runtime/rococo/src/lib.rs b/polkadot/runtime/rococo/src/lib.rs index fef25527e82a..12d8213a181e 100644 --- a/polkadot/runtime/rococo/src/lib.rs +++ b/polkadot/runtime/rococo/src/lib.rs @@ -169,7 +169,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("rococo"), impl_name: create_runtime_str!("parity-rococo-v2.0"), authoring_version: 0, - spec_version: 1_016_002, + spec_version: 1_016_001, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 26, From 5110d7eee43f1bf7bce8735a3bf2e74f1b3604ec Mon Sep 17 00:00:00 2001 From: DavidK Date: Fri, 4 Oct 2024 19:07:54 +0300 Subject: [PATCH 19/23] Comment formatting fixes --- substrate/frame/treasury/src/migration.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index 2da51742b1d3..63f761ef7846 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -31,9 +31,11 @@ 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. + /// 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. + /// `UnreserveWeight` returns `Weight` of `unreserve_balance` operation which is perfomed during + /// this migration. pub struct Migration(PhantomData<(T, I, UnreserveWeight)>); impl + pallet_balances::Config, I: 'static, UnreserveWeight: Get> From b8abc12867ae1d1e114e015a1763e33969a50147 Mon Sep 17 00:00:00 2001 From: DavidK Date: Sun, 6 Oct 2024 06:28:44 +0300 Subject: [PATCH 20/23] Remove pallet_balances::Config trait bound for migration --- substrate/frame/treasury/src/migration.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index 63f761ef7846..83494bee8dc4 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -38,7 +38,7 @@ pub mod cleanup_proposals { /// this migration. pub struct Migration(PhantomData<(T, I, UnreserveWeight)>); - impl + pallet_balances::Config, I: 'static, UnreserveWeight: Get> + impl, I: 'static, UnreserveWeight: Get> OnRuntimeUpgrade for Migration { fn on_runtime_upgrade() -> frame_support::weights::Weight { From c80a3959445e41ac5fe77a5f13d5777ede7f1359 Mon Sep 17 00:00:00 2001 From: DavidK Date: Sun, 6 Oct 2024 06:34:06 +0300 Subject: [PATCH 21/23] Formatting fixes --- substrate/frame/treasury/src/migration.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/treasury/src/migration.rs b/substrate/frame/treasury/src/migration.rs index 83494bee8dc4..c0de4ce43108 100644 --- a/substrate/frame/treasury/src/migration.rs +++ b/substrate/frame/treasury/src/migration.rs @@ -38,8 +38,8 @@ pub mod cleanup_proposals { /// this migration. pub struct Migration(PhantomData<(T, I, UnreserveWeight)>); - impl, I: 'static, UnreserveWeight: Get> - OnRuntimeUpgrade for Migration + impl, I: 'static, UnreserveWeight: Get> OnRuntimeUpgrade + for Migration { fn on_runtime_upgrade() -> frame_support::weights::Weight { let mut approval_index = BTreeSet::new(); From 49e1e34b39f6d8d7f3ba7e3d103cb667f1fd0bb4 Mon Sep 17 00:00:00 2001 From: davidk-pt Date: Tue, 8 Oct 2024 11:18:51 +0300 Subject: [PATCH 22/23] Update prdoc/pr_5892.prdoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dónal Murray --- prdoc/pr_5892.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_5892.prdoc b/prdoc/pr_5892.prdoc index 2b8a7c6ff4ee..ad42cede3249 100644 --- a/prdoc/pr_5892.prdoc +++ b/prdoc/pr_5892.prdoc @@ -7,7 +7,7 @@ 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 in hold with no way to release them. + governance v1 model but there are some `Proposals` whose bonds are still on hold with no way to release them. Purpose of this migration is to clear `Proposals` which are stuck and return bonds to the proposers. crates: From dfe7e1fcf8096881b24f1683babde1d1da55f1b9 Mon Sep 17 00:00:00 2001 From: davidk-pt Date: Tue, 8 Oct 2024 11:18:58 +0300 Subject: [PATCH 23/23] Update prdoc/pr_5892.prdoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dónal Murray --- prdoc/pr_5892.prdoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prdoc/pr_5892.prdoc b/prdoc/pr_5892.prdoc index ad42cede3249..b909e443328b 100644 --- a/prdoc/pr_5892.prdoc +++ b/prdoc/pr_5892.prdoc @@ -8,7 +8,7 @@ doc: 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. - Purpose of this migration is to clear `Proposals` which are stuck and return bonds to the proposers. + The purpose of this migration is to clear `Proposals` which are stuck and return bonds to the proposers. crates: - name: pallet-treasury