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

Commit

Permalink
Fix migrations (#7340)
Browse files Browse the repository at this point in the history
Okay this was stupid 🤦

Signed-off-by: Oliver Tale-Yazdi <[email protected]>
  • Loading branch information
ggwpez authored and coderobe committed Jun 12, 2023
1 parent 8b2d97f commit ba42b9c
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 66 deletions.
73 changes: 41 additions & 32 deletions runtime/parachains/src/configuration/migration/v5.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,27 +127,35 @@ pub struct V5HostConfiguration<BlockNumber> {
pub minimum_validation_upgrade_delay: BlockNumber,
}

#[frame_support::storage_alias]
pub(crate) type V4ActiveConfig<T: Config> =
StorageValue<Pallet<T>, V4HostConfiguration<BlockNumberFor<T>>, OptionQuery>;

#[frame_support::storage_alias]
pub(crate) type V4PendingConfigs<T: Config> = StorageValue<
Pallet<T>,
Vec<(SessionIndex, V4HostConfiguration<BlockNumberFor<T>>)>,
OptionQuery,
>;

#[frame_support::storage_alias]
pub(crate) type V5ActiveConfig<T: Config> =
StorageValue<Pallet<T>, V5HostConfiguration<BlockNumberFor<T>>, OptionQuery>;

#[frame_support::storage_alias]
pub(crate) type V5PendingConfigs<T: Config> = StorageValue<
Pallet<T>,
Vec<(SessionIndex, V5HostConfiguration<BlockNumberFor<T>>)>,
OptionQuery,
>;
mod v4 {
use super::*;

#[frame_support::storage_alias]
pub(crate) type ActiveConfig<T: Config> =
StorageValue<Pallet<T>, V4HostConfiguration<BlockNumberFor<T>>, OptionQuery>;

#[frame_support::storage_alias]
pub(crate) type PendingConfigs<T: Config> = StorageValue<
Pallet<T>,
Vec<(SessionIndex, V4HostConfiguration<BlockNumberFor<T>>)>,
OptionQuery,
>;
}

mod v5 {
use super::*;

#[frame_support::storage_alias]
pub(crate) type ActiveConfig<T: Config> =
StorageValue<Pallet<T>, V5HostConfiguration<BlockNumberFor<T>>, OptionQuery>;

#[frame_support::storage_alias]
pub(crate) type PendingConfigs<T: Config> = StorageValue<
Pallet<T>,
Vec<(SessionIndex, V5HostConfiguration<BlockNumberFor<T>>)>,
OptionQuery,
>;
}

impl<BlockNumber: Default + From<u32>> Default for V4HostConfiguration<BlockNumber> {
fn default() -> Self {
Expand Down Expand Up @@ -266,6 +274,7 @@ impl<T: Config> OnRuntimeUpgrade for MigrateToV5<T> {
}

fn on_runtime_upgrade() -> Weight {
log::info!(target: configuration::LOG_TARGET, "MigrateToV5 started");
if StorageVersion::get::<Pallet<T>>() == 4 {
let weight_consumed = migrate_to_v5::<T>();

Expand Down Expand Up @@ -352,13 +361,13 @@ executor_params : Default::default(),
}
};

let v4 = V4ActiveConfig::<T>::get()
let v4 = v4::ActiveConfig::<T>::get()
.defensive_proof("Could not decode old config")
.unwrap_or_default();
let v5 = translate(v4);
V5ActiveConfig::<T>::set(Some(v5));
v5::ActiveConfig::<T>::set(Some(v5));

let pending_v4 = V4PendingConfigs::<T>::get()
let pending_v4 = v4::PendingConfigs::<T>::get()
.defensive_proof("Could not decode old pending")
.unwrap_or_default();
let mut pending_v5 = Vec::new();
Expand All @@ -367,7 +376,7 @@ executor_params : Default::default(),
let v5 = translate(v4);
pending_v5.push((session, v5));
}
V5PendingConfigs::<T>::set(Some(pending_v5.clone()));
v5::PendingConfigs::<T>::set(Some(pending_v5.clone()));

let num_configs = (pending_v5.len() + 1) as u64;
T::DbWeight::get().reads_writes(num_configs, num_configs)
Expand Down Expand Up @@ -397,8 +406,8 @@ mod tests {
// doesn't need to be read and also leaving it as one line allows to easily copy it.
let raw_config = hex_literal::hex!["0000a000005000000a00000000c8000000c800000a0000000a000000100e0000580200000000500000c800000700e8764817020040011e00000000000000005039278c0400000000000000000000005039278c0400000000000000000000e8030000009001001e00000000000000009001008070000000000000000000000a0000000a0000000a00000001000000010500000001c80000000600000058020000580200000200000059000000000000001e000000280000000700c817a80402004001010200000014000000"];

let v4 = v5::V4HostConfiguration::<primitives::BlockNumber>::decode(&mut &raw_config[..])
.unwrap();
let v4 =
V4HostConfiguration::<primitives::BlockNumber>::decode(&mut &raw_config[..]).unwrap();

// We check only a sample of the values here. If we missed any fields or messed up data types
// that would skew all the fields coming after.
Expand All @@ -421,7 +430,7 @@ mod tests {
// We specify only the picked fields and the rest should be provided by the `Default`
// implementation. That implementation is copied over between the two types and should work
// fine.
let v4 = v5::V4HostConfiguration::<primitives::BlockNumber> {
let v4 = V4HostConfiguration::<primitives::BlockNumber> {
ump_max_individual_weight: Weight::from_parts(0x71616e6f6e0au64, 0x71616e6f6e0au64),
needed_approvals: 69,
thread_availability_period: 55,
Expand All @@ -438,13 +447,13 @@ mod tests {

new_test_ext(Default::default()).execute_with(|| {
// Implant the v4 version in the state.
V4ActiveConfig::<Test>::set(Some(v4));
V4PendingConfigs::<Test>::set(Some(pending_configs));
v4::ActiveConfig::<Test>::set(Some(v4));
v4::PendingConfigs::<Test>::set(Some(pending_configs));

migrate_to_v5::<Test>();

let v5 = V5ActiveConfig::<Test>::get().unwrap();
let mut configs_to_check = V5PendingConfigs::<Test>::get().unwrap();
let v5 = v5::ActiveConfig::<Test>::get().unwrap();
let mut configs_to_check = v5::PendingConfigs::<Test>::get().unwrap();
configs_to_check.push((0, v5.clone()));

for (_, v4) in configs_to_check {
Expand Down
76 changes: 43 additions & 33 deletions runtime/parachains/src/configuration/migration/v6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,27 +34,35 @@ use super::v5::V5HostConfiguration;
// Change this once there is V7.
type V6HostConfiguration<BlockNumber> = configuration::HostConfiguration<BlockNumber>;

#[frame_support::storage_alias]
pub(crate) type V5ActiveConfig<T: Config> =
StorageValue<Pallet<T>, V5HostConfiguration<BlockNumberFor<T>>, OptionQuery>;

#[frame_support::storage_alias]
pub(crate) type V5PendingConfigs<T: Config> = StorageValue<
Pallet<T>,
Vec<(SessionIndex, V5HostConfiguration<BlockNumberFor<T>>)>,
OptionQuery,
>;

#[frame_support::storage_alias]
pub(crate) type V6ActiveConfig<T: Config> =
StorageValue<Pallet<T>, V6HostConfiguration<BlockNumberFor<T>>, OptionQuery>;

#[frame_support::storage_alias]
pub(crate) type V6PendingConfigs<T: Config> = StorageValue<
Pallet<T>,
Vec<(SessionIndex, V6HostConfiguration<BlockNumberFor<T>>)>,
OptionQuery,
>;
mod v5 {
use super::*;

#[frame_support::storage_alias]
pub(crate) type ActiveConfig<T: Config> =
StorageValue<Pallet<T>, V5HostConfiguration<BlockNumberFor<T>>, OptionQuery>;

#[frame_support::storage_alias]
pub(crate) type PendingConfigs<T: Config> = StorageValue<
Pallet<T>,
Vec<(SessionIndex, V5HostConfiguration<BlockNumberFor<T>>)>,
OptionQuery,
>;
}

mod v6 {
use super::*;

#[frame_support::storage_alias]
pub(crate) type ActiveConfig<T: Config> =
StorageValue<Pallet<T>, V6HostConfiguration<BlockNumberFor<T>>, OptionQuery>;

#[frame_support::storage_alias]
pub(crate) type PendingConfigs<T: Config> = StorageValue<
Pallet<T>,
Vec<(SessionIndex, V6HostConfiguration<BlockNumberFor<T>>)>,
OptionQuery,
>;
}

pub struct MigrateToV6<T>(sp_std::marker::PhantomData<T>);
impl<T: Config> OnRuntimeUpgrade for MigrateToV6<T> {
Expand All @@ -65,6 +73,7 @@ impl<T: Config> OnRuntimeUpgrade for MigrateToV6<T> {
}

fn on_runtime_upgrade() -> Weight {
log::info!(target: configuration::LOG_TARGET, "MigrateToV6 started");
if StorageVersion::get::<Pallet<T>>() == 5 {
let weight_consumed = migrate_to_v6::<T>();

Expand Down Expand Up @@ -145,24 +154,24 @@ executor_params : pre.executor_params,
}
};

let v5 = V5ActiveConfig::<T>::get()
let v5 = v5::ActiveConfig::<T>::get()
.defensive_proof("Could not decode old config")
.unwrap_or_default();
let v6 = translate(v5);
V6ActiveConfig::<T>::set(Some(v6));
v6::ActiveConfig::<T>::set(Some(v6));

let pending_v4 = V5PendingConfigs::<T>::get()
let pending_v5 = v5::PendingConfigs::<T>::get()
.defensive_proof("Could not decode old pending")
.unwrap_or_default();
let mut pending_v5 = Vec::new();
let mut pending_v6 = Vec::new();

for (session, v5) in pending_v4.into_iter() {
for (session, v5) in pending_v5.into_iter() {
let v6 = translate(v5);
pending_v5.push((session, v6));
pending_v6.push((session, v6));
}
V6PendingConfigs::<T>::set(Some(pending_v5.clone()));
v6::PendingConfigs::<T>::set(Some(pending_v6.clone()));

let num_configs = (pending_v5.len() + 1) as u64;
let num_configs = (pending_v6.len() + 1) as u64;
T::DbWeight::get().reads_writes(num_configs, num_configs)
}

Expand Down Expand Up @@ -201,6 +210,7 @@ mod tests {
assert_eq!(v5.n_delay_tranches, 40);
assert_eq!(v5.ump_max_individual_weight, Weight::from_parts(20_000_000_000, 5_242_880));
assert_eq!(v5.minimum_validation_upgrade_delay, 15); // This is the last field in the struct.
assert_eq!(v5.group_rotation_frequency, 10);
}

#[test]
Expand Down Expand Up @@ -230,13 +240,13 @@ mod tests {

new_test_ext(Default::default()).execute_with(|| {
// Implant the v5 version in the state.
V5ActiveConfig::<Test>::set(Some(v5));
V5PendingConfigs::<Test>::set(Some(pending_configs));
v5::ActiveConfig::<Test>::set(Some(v5));
v5::PendingConfigs::<Test>::set(Some(pending_configs));

migrate_to_v6::<Test>();

let v6 = V6ActiveConfig::<Test>::get().unwrap();
let mut configs_to_check = V6PendingConfigs::<Test>::get().unwrap();
let v6 = v6::ActiveConfig::<Test>::get().unwrap();
let mut configs_to_check = v6::PendingConfigs::<Test>::get().unwrap();
configs_to_check.push((0, v6.clone()));

for (_, v5) in configs_to_check {
Expand Down
21 changes: 20 additions & 1 deletion runtime/parachains/src/configuration/migration_ump.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,29 @@ pub mod latest {
"Last pending HostConfig upgrade:\n\n{:#?}\n",
pending.last()
);
let Some(last) = pending.last() else {
return Err("There must be a new pending upgrade enqueued".into());
};
ensure!(
pending.len() == old_pending as usize + 1,
"There must be a new pending upgrade enqueued"
"There must be exactly one new pending upgrade enqueued"
);
if let Err(err) = last.1.check_consistency() {
log::error!(
target: LOG_TARGET,
"Last PendingConfig is invalidity {:?}", err,
);

return Err("Pending upgrade must be sane but was not".into())
}
if let Err(err) = ActiveConfig::<T>::get().check_consistency() {
log::error!(
target: LOG_TARGET,
"ActiveConfig is invalid: {:?}", err,
);

return Err("Active upgrade must be sane but was not".into())
}

Ok(())
}
Expand Down

0 comments on commit ba42b9c

Please sign in to comment.