Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Next Next commit
[Feature] Add a migration that drains and refunds stored calls
  • Loading branch information
ruseinov committed Sep 20, 2022
commit 7bb5e1360b16e87f3e07be3b2c7fbccd27c3a152
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
56 changes: 56 additions & 0 deletions frame/multisig/src/migrations.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
use super::*;
use frame_support::{
traits::{OnRuntimeUpgrade, WrapperKeepOpaque},
Blake2_256,
};

#[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<
crate::Pallet<T>,
Blake2_256,
[u8; 32],
(OpaqueCall<T>, T::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<(), &'static str> {
let current = Pallet::<T>::current_storage_version();
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>::count());

Ok(())
}

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

ensure!(onchain > 0, "this migration can be deleted");

let calls = Calls::<T>::drain().for_each(|call_hash, (_data, caller, deposit)| {
T::Currency::unreserve(&caller, deposit)?;
});
}

#[cfg(feature = "try-runtime")]
fn post_upgrade() -> Result<(), &'static str> {
let onchain = Pallet::<T>::on_chain_storage_version();

Copy link
Member

Choose a reason for hiding this comment

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

Could you add an assertion that the new version is exactly 1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the assertion we have now is wrong, about to fix it.

ensure!(onchain > 0, "this migration needs to be run");
ensure!(Calls<T>::count() == 0,
"there are some dangling calls that need to be destroyed and refunded");
}
}
}