Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[Feature] Add a migration that drains and refunds stored calls #12313

Merged
merged 11 commits into from
Sep 29, 2022
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.

3 changes: 3 additions & 0 deletions frame/multisig/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/
sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" }
sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" }

# third party
log = { version = "0.4.17", default-features = false }

[dev-dependencies]
pallet-balances = { version = "4.0.0-dev", path = "../balances" }
sp-core = { version = "6.0.0", path = "../../primitives/core" }
Expand Down
25 changes: 22 additions & 3 deletions frame/multisig/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#![cfg_attr(not(feature = "std"), no_std)]

mod benchmarking;
pub mod migrations;
mod tests;
pub mod weights;

Expand All @@ -58,7 +59,7 @@ use frame_support::{
},
ensure,
traits::{Currency, Get, ReservableCurrency},
weights::{GetDispatchInfo, Weight},
weights::Weight,
RuntimeDebug,
};
use frame_system::{self as system, RawOrigin};
Expand All @@ -73,6 +74,20 @@ pub use weights::WeightInfo;

pub use pallet::*;

/// The log target of this pallet.
pub const LOG_TARGET: &'static str = "runtime::multisig";

// syntactic sugar for logging.
#[macro_export]
macro_rules! log {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this in here if you only use it in the migration?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I'd rather define logging macro for the whole package and have the opportunity to use it when/if needed. It's also consistent with other pallets which define logging.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ruseinov's point is fair to me, I had to do the same in the past.

@ggwpez what I challenge you about is a way so we won't need to define this log macro per-pallet? wdyt?

($level:tt, $patter:expr $(, $values:expr)* $(,)?) => {
log::$level!(
target: crate::LOG_TARGET,
concat!("[{:?}] ✍️ ", $patter), <frame_system::Pallet<T>>::block_number() $(, $values)*
)
};
}

type BalanceOf<T> =
<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;

Expand Down Expand Up @@ -103,7 +118,7 @@ pub struct Multisig<BlockNumber, Balance, AccountId> {
type CallHash = [u8; 32];

enum CallOrHash<T: Config> {
Call(<T as Config>::Call),
Call(<T as Config>::RuntimeCall),
Hash([u8; 32]),
}

Expand Down Expand Up @@ -150,9 +165,13 @@ pub mod pallet {
type WeightInfo: WeightInfo;
}

/// The current storage version.
const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

#[pallet::pallet]
#[pallet::generate_store(pub(super) trait Store)]
#[pallet::without_storage_info]
#[pallet::storage_version(STORAGE_VERSION)]
pub struct Pallet<T>(_);

/// The set of open multisig operations.
Expand Down Expand Up @@ -356,7 +375,7 @@ pub mod pallet {
threshold: u16,
other_signatories: Vec<T::AccountId>,
maybe_timepoint: Option<Timepoint<T::BlockNumber>>,
call: Box<<T as Config>::Call>,
call: Box<<T as Config>::RuntimeCall>,
max_weight: Weight,
) -> DispatchResultWithPostInfo {
let who = ensure_signed(origin)?;
Expand Down
86 changes: 86 additions & 0 deletions frame/multisig/src/migrations.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
// This file is part of Substrate.

// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Copyright (C) 2019-2022 Parity Technologies (UK) Ltd.
// Copyright (C) 2022 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.

// Migrations for Multisig Pallet

use super::*;
ggwpez marked this conversation as resolved.
Show resolved Hide resolved
use frame_support::{
dispatch::GetStorageVersion,
traits::{OnRuntimeUpgrade, WrapperKeepOpaque},
Identity,
};

#[cfg(feature = "try-runtime")]
use frame_support::ensure;

pub mod v1 {
use super::*;

type OpaqueCall<T> = WrapperKeepOpaque<<T as Config>::RuntimeCall>;

#[frame_support::storage_alias]
type Calls<T: Config> = StorageMap<
Pallet<T>,
Identity,
[u8; 32],
(OpaqueCall<T>, <T as frame_system::Config>::AccountId, BalanceOf<T>),
>;

pub struct MigrateToV1<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for MigrateToV1<T> {
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
let onchain = Pallet::<T>::on_chain_storage_version();

ensure!(onchain < 1, "this migration can be deleted");

log!(info, "Number of calls to refund and delete: {}", Calls::<T>::iter().count());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be pedantic, we could add some math here that: if the number of calls is so much that this exhausts block weight, abort or log a warning.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or just have a limit configurable limit (eg 100) which can then be tested with try-runtime in advance to respect the limits.


Ok(Vec::new())
}

fn on_runtime_upgrade() -> Weight {
let current = Pallet::<T>::current_storage_version();
let onchain = Pallet::<T>::on_chain_storage_version();

if onchain > 0 {
log!(info, "MigrateToV1 should be removed");
return T::DbWeight::get().reads(1)
}

Calls::<T>::drain().for_each(|(_call_hash, (_data, caller, deposit))| {
T::Currency::unreserve(&caller, deposit);
});

current.put::<Pallet<T>>();

<T as frame_system::Config>::BlockWeights::get().max_block
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
let onchain = Pallet::<T>::on_chain_storage_version();
ensure!(onchain < 2, "this migration needs to be removed");
ensure!(onchain == 1, "this migration needs to be run");
ensure!(
Calls::<T>::iter().count() == 0,
"there are some dangling calls that need to be destroyed and refunded"
);
Ok(())
}
}
}