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

Migrate Weights properly to v2 #1722

Merged
merged 5 commits into from
Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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.

2 changes: 2 additions & 0 deletions pallets/dmp-queue/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ edition = "2021"

[dependencies]
codec = { package = "parity-scale-codec", version = "3.0.0", features = [ "derive" ], default-features = false }
log = { version = "0.4.17", default-features = false }
scale-info = { version = "2.2.0", default-features = false, features = ["derive"] }

# Substrate
Expand All @@ -32,6 +33,7 @@ std = [
"scale-info/std",
"frame-support/std",
"frame-system/std",
"log/std",
"sp-io/std",
"sp-runtime/std",
"sp-std/std",
Expand Down
27 changes: 17 additions & 10 deletions pallets/dmp-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

#![cfg_attr(not(feature = "std"), no_std)]

pub mod migration;

use codec::{Decode, DecodeLimit, Encode};
use cumulus_primitives_core::{relay_chain::BlockNumber as RelayBlockNumber, DmpMessageHandler};
use frame_support::{
Expand All @@ -31,7 +33,7 @@ pub use pallet::*;
use scale_info::TypeInfo;
use sp_runtime::RuntimeDebug;
use sp_std::{convert::TryFrom, prelude::*};
use xcm::{latest::prelude::*, VersionedXcm, MAX_XCM_DECODE_DEPTH};
use xcm::{latest::{prelude::*, Weight as XcmWeight}, VersionedXcm, MAX_XCM_DECODE_DEPTH};

#[derive(Copy, Clone, Eq, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)]
pub struct ConfigData {
Expand Down Expand Up @@ -78,6 +80,7 @@ pub mod pallet {

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

Expand Down Expand Up @@ -121,6 +124,10 @@ pub mod pallet {

#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_runtime_upgrade() -> Weight {
migration::migrate_to_latest::<T>()
}

fn on_idle(_now: T::BlockNumber, max_weight: Weight) -> Weight {
// on_idle processes additional messages with any remaining block weight.
Self::service_queue(max_weight)
Expand All @@ -141,16 +148,16 @@ pub mod pallet {
///
/// Events:
/// - `OverweightServiced`: On success.
#[pallet::weight(weight_limit.saturating_add(Weight::from_ref_time(1_000_000)))]
#[pallet::weight(Weight::from_ref_time(weight_limit.saturating_add(1_000_000)))]
pub fn service_overweight(
origin: OriginFor<T>,
index: OverweightIndex,
weight_limit: Weight,
weight_limit: XcmWeight,
) -> DispatchResultWithPostInfo {
T::ExecuteOverweightOrigin::ensure_origin(origin)?;

let (sent_at, data) = Overweight::<T>::get(index).ok_or(Error::<T>::Unknown)?;
let weight_used = Self::try_service_message(weight_limit, sent_at, &data[..])
let weight_used = Self::try_service_message(Weight::from_ref_time(weight_limit), sent_at, &data[..])
.map_err(|_| Error::<T>::OverLimit)?;
Overweight::<T>::remove(index);
Self::deposit_event(Event::OverweightServiced { overweight_index: index, weight_used });
Expand Down Expand Up @@ -747,33 +754,33 @@ mod tests {
DmpQueue::service_overweight(
RuntimeOrigin::signed(1),
0,
Weight::from_ref_time(20000)
20000
),
BadOrigin
);
assert_noop!(
DmpQueue::service_overweight(
RuntimeOrigin::root(),
1,
Weight::from_ref_time(20000)
20000
),
Error::<Test>::Unknown
);
assert_noop!(
DmpQueue::service_overweight(RuntimeOrigin::root(), 0, Weight::from_ref_time(9999)),
DmpQueue::service_overweight(RuntimeOrigin::root(), 0, 9999),
Error::<Test>::OverLimit
);
assert_eq!(take_trace(), vec![msg_limit_reached(10000)]);

let base_weight =
super::Call::<Test>::service_overweight { index: 0, weight_limit: Weight::zero() }
super::Call::<Test>::service_overweight { index: 0, weight_limit: 0 }
.get_dispatch_info()
.weight;
use frame_support::dispatch::GetDispatchInfo;
let info = DmpQueue::service_overweight(
RuntimeOrigin::root(),
0,
Weight::from_ref_time(20000),
20000,
)
.unwrap();
let actual_weight = info.actual_weight.unwrap();
Expand All @@ -785,7 +792,7 @@ mod tests {
DmpQueue::service_overweight(
RuntimeOrigin::root(),
0,
Weight::from_ref_time(20000)
20000
),
Error::<Test>::Unknown
);
Expand Down
103 changes: 103 additions & 0 deletions pallets/dmp-queue/src/migration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Copyright 2022 Parity Technologies (UK) Ltd.
// This file is part of Polkadot.

// Polkadot is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.

// Polkadot is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.

// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.

//! A module that is responsible for migration of storage.

use crate::{Config, Pallet, Store};
use frame_support::{pallet_prelude::*, traits::StorageVersion, weights::{constants::WEIGHT_PER_MILLIS, Weight}};
use xcm::latest::Weight as XcmWeight;

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

/// Migrates the pallet storage to the most recent version, checking and setting the
/// `StorageVersion`.
pub fn migrate_to_latest<T: Config>() -> Weight {
let mut weight = Weight::zero();
KiChjang marked this conversation as resolved.
Show resolved Hide resolved

if StorageVersion::get::<Pallet<T>>() == 0 {
weight += migrate_to_v1::<T>();
StorageVersion::new(1).put::<Pallet<T>>();
}

weight
}

mod v0 {
use super::*;
use codec::{Decode, Encode};

#[derive(Decode, Encode, Debug)]
pub struct ConfigData {
pub max_individual: XcmWeight,
}

impl Default for ConfigData {
fn default() -> Self {
ConfigData {
max_individual: 10u64 * WEIGHT_PER_MILLIS.ref_time(),
}
}
}
}

/// Migrates `QueueConfigData` from v1 (using only reference time weights) to v2 (with
/// 2D weights).
///
/// NOTE: Only use this function if you know what you're doing. Default to using
/// `migrate_to_latest`.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe an additional sentence about the scenario where one would want to call this just as a sanity check for people who know what they're doing ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

In particular maybe also a note that this is "called by migrate_to_latest when applicable" or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Honestly, the situation where I think it would make sense to call migrate_to_v1 directly is during unit testing, otherwise I really don't see why anyone would call this method on its own.

pub fn migrate_to_v1<T: Config>() -> Weight {
let translate = |pre: v0::ConfigData| -> super::ConfigData {
super::ConfigData {
max_individual: Weight::from_ref_time(pre.max_individual),
}
};

if let Err(_) = <Pallet<T> as Store>::Configuration::translate(|pre| pre.map(translate)) {
log::error!(
target: "dmp_queue",
"unexpected error when performing translation of the QueueConfig type during storage upgrade to v2"
);
}

T::DbWeight::get().reads_writes(1, 1)
}

#[cfg(test)]
mod tests {
use super::*;
use crate::tests::{new_test_ext, Test};

#[test]
fn test_migration_to_v1() {
let v0 = v0::ConfigData {
max_individual: 30_000_000_000,
};

new_test_ext().execute_with(|| {
frame_support::storage::unhashed::put_raw(
&crate::Configuration::<Test>::hashed_key(),
&v0.encode(),
);

migrate_to_v1::<Test>();

let v1 = crate::Configuration::<Test>::get();

assert_eq!(v0.max_individual, v1.max_individual.ref_time());
});
}
}
2 changes: 1 addition & 1 deletion pallets/xcmp-queue/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use frame_system::RawOrigin;

benchmarks! {
set_config_with_u32 {}: update_resume_threshold(RawOrigin::Root, 100)
set_config_with_weight {}: update_weight_restrict_decay(RawOrigin::Root, Weight::from_ref_time(3_000_000))
set_config_with_weight {}: update_weight_restrict_decay(RawOrigin::Root, 3_000_000)
}

impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test);
20 changes: 10 additions & 10 deletions pallets/xcmp-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use rand_chacha::{
use scale_info::TypeInfo;
use sp_runtime::{traits::Hash, RuntimeDebug};
use sp_std::{convert::TryFrom, prelude::*};
use xcm::{latest::prelude::*, VersionedXcm, WrapVersion, MAX_XCM_DECODE_DEPTH};
use xcm::{latest::{prelude::*, Weight as XcmWeight}, VersionedXcm, WrapVersion, MAX_XCM_DECODE_DEPTH};
use xcm_executor::traits::ConvertOrigin;

pub use pallet::*;
Expand Down Expand Up @@ -130,11 +130,11 @@ pub mod pallet {
///
/// Events:
/// - `OverweightServiced`: On success.
#[pallet::weight((weight_limit.saturating_add(Weight::from_ref_time(1_000_000)), DispatchClass::Operational,))]
#[pallet::weight((Weight::from_ref_time(weight_limit.saturating_add(1_000_000)), DispatchClass::Operational,))]
pub fn service_overweight(
origin: OriginFor<T>,
index: OverweightIndex,
weight_limit: Weight,
weight_limit: XcmWeight,
) -> DispatchResultWithPostInfo {
T::ExecuteOverweightOrigin::ensure_origin(origin)?;

Expand All @@ -145,7 +145,7 @@ pub mod pallet {
&mut data.as_slice(),
)
.map_err(|_| Error::<T>::BadXcm)?;
let used = Self::handle_xcm_message(sender, sent_at, xcm, weight_limit)
let used = Self::handle_xcm_message(sender, sent_at, xcm, Weight::from_ref_time(weight_limit))
.map_err(|_| Error::<T>::WeightOverLimit)?;
Overweight::<T>::remove(index);
Self::deposit_event(Event::OverweightServiced { index, used });
Expand Down Expand Up @@ -222,9 +222,9 @@ pub mod pallet {
/// - `origin`: Must pass `Root`.
/// - `new`: Desired value for `QueueConfigData.threshold_weight`
#[pallet::weight((T::WeightInfo::set_config_with_weight(), DispatchClass::Operational,))]
pub fn update_threshold_weight(origin: OriginFor<T>, new: Weight) -> DispatchResult {
pub fn update_threshold_weight(origin: OriginFor<T>, new: XcmWeight) -> DispatchResult {
ensure_root(origin)?;
QueueConfig::<T>::mutate(|data| data.threshold_weight = new);
QueueConfig::<T>::mutate(|data| data.threshold_weight = Weight::from_ref_time(new));

Ok(())
}
Expand All @@ -235,9 +235,9 @@ pub mod pallet {
/// - `origin`: Must pass `Root`.
/// - `new`: Desired value for `QueueConfigData.weight_restrict_decay`.
#[pallet::weight((T::WeightInfo::set_config_with_weight(), DispatchClass::Operational,))]
pub fn update_weight_restrict_decay(origin: OriginFor<T>, new: Weight) -> DispatchResult {
pub fn update_weight_restrict_decay(origin: OriginFor<T>, new: XcmWeight) -> DispatchResult {
ensure_root(origin)?;
QueueConfig::<T>::mutate(|data| data.weight_restrict_decay = new);
QueueConfig::<T>::mutate(|data| data.weight_restrict_decay = Weight::from_ref_time(new));

Ok(())
}
Expand All @@ -250,10 +250,10 @@ pub mod pallet {
#[pallet::weight((T::WeightInfo::set_config_with_weight(), DispatchClass::Operational,))]
pub fn update_xcmp_max_individual_weight(
origin: OriginFor<T>,
new: Weight,
new: XcmWeight,
) -> DispatchResult {
ensure_root(origin)?;
QueueConfig::<T>::mutate(|data| data.xcmp_max_individual_weight = new);
QueueConfig::<T>::mutate(|data| data.xcmp_max_individual_weight = Weight::from_ref_time(new));

Ok(())
}
Expand Down
Loading