Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add migration to clear unapproved proposals from treasury pallet #5892

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion polkadot/runtime/rococo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -168,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,
davidk-pt marked this conversation as resolved.
Show resolved Hide resolved
impl_version: 0,
apis: RUNTIME_API_VERSIONS,
transaction_version: 26,
Expand Down Expand Up @@ -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::<Runtime>::force_unreserve();
}

// Special Config for Gov V1 pallets, allowing us to run migrations for them without
Expand Down Expand Up @@ -1654,6 +1657,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.

Expand Down
19 changes: 19 additions & 0 deletions prdoc/pr_5892.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# 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 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
bump: patch
- name: rococo-runtime
bump: patch
- name: westend-runtime
bump: patch
2 changes: 2 additions & 0 deletions substrate/frame/treasury/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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",
Expand Down
1 change: 1 addition & 0 deletions substrate/frame/treasury/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
#![cfg_attr(not(feature = "std"), no_std)]

mod benchmarking;
pub mod migration;
#[cfg(test)]
mod tests;
pub mod weights;
Expand Down
131 changes: 131 additions & 0 deletions substrate/frame/treasury/src/migration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// 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::*;
davidk-pt marked this conversation as resolved.
Show resolved Hide resolved
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> + pallet_balances::Config, I: 'static, UnreserveWeight: Get<Weight>>
davidk-pt marked this conversation as resolved.
Show resolved Hide resolved
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(())
}
}
}
Loading