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

Commit

Permalink
Migrate Weights properly to v2 (#1722)
Browse files Browse the repository at this point in the history
* Migrate Weights properly to v2

* Add missing on_runtime_upgrade implementation

* Fix benchmarks

* Apply suggestions from code review

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* cargo fmt

Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  • Loading branch information
KiChjang and ggwpez committed Oct 11, 2022
1 parent 92956a3 commit 882a892
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 99 deletions.
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
55 changes: 24 additions & 31 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,10 @@ 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 +83,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 +127,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,17 +151,18 @@ 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[..])
.map_err(|_| Error::<T>::OverLimit)?;
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 });
Ok(Some(weight_used.saturating_add(Weight::from_ref_time(1_000_000))).into())
Expand Down Expand Up @@ -744,49 +755,31 @@ mod tests {
assert_eq!(overweights(), vec![0]);

assert_noop!(
DmpQueue::service_overweight(
RuntimeOrigin::signed(1),
0,
Weight::from_ref_time(20000)
),
DmpQueue::service_overweight(RuntimeOrigin::signed(1), 0, 20000),
BadOrigin
);
assert_noop!(
DmpQueue::service_overweight(
RuntimeOrigin::root(),
1,
Weight::from_ref_time(20000)
),
DmpQueue::service_overweight(RuntimeOrigin::root(), 1, 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() }
.get_dispatch_info()
.weight;
let base_weight = 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),
)
.unwrap();
let info = DmpQueue::service_overweight(RuntimeOrigin::root(), 0, 20000).unwrap();
let actual_weight = info.actual_weight.unwrap();
assert_eq!(actual_weight, base_weight + Weight::from_ref_time(10000));
assert_eq!(take_trace(), vec![msg_complete(10000)]);
assert!(overweights().is_empty());

assert_noop!(
DmpQueue::service_overweight(
RuntimeOrigin::root(),
0,
Weight::from_ref_time(20000)
),
DmpQueue::service_overweight(RuntimeOrigin::root(), 0, 20000),
Error::<Test>::Unknown
);
});
Expand Down
101 changes: 101 additions & 0 deletions pallets/dmp-queue/src/migration.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// 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 = T::DbWeight::get().reads(1);

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`.
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);
33 changes: 22 additions & 11 deletions pallets/xcmp-queue/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@ 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 +133,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,8 +148,9 @@ pub mod pallet {
&mut data.as_slice(),
)
.map_err(|_| Error::<T>::BadXcm)?;
let used = Self::handle_xcm_message(sender, sent_at, xcm, weight_limit)
.map_err(|_| Error::<T>::WeightOverLimit)?;
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 });
Ok(Some(used.saturating_add(Weight::from_ref_time(1_000_000))).into())
Expand Down Expand Up @@ -222,9 +226,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 +239,14 @@ 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 +259,12 @@ 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

0 comments on commit 882a892

Please sign in to comment.