Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storage Version Migration #3220

Merged
merged 33 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
b3f5509
feat: add paseo migration
wangminqi Dec 26, 2024
ff541fd
Merge branch 'dev' into p-1245-manul-migration-based-on-storage-version
wangminqi Dec 26, 2024
c40b447
chore: fix
wangminqi Dec 26, 2024
2967bf7
Merge branch 'dev' into p-1245-manul-migration-based-on-storage-version
wangminqi Dec 26, 2024
d70657a
chore: fix
wangminqi Dec 26, 2024
9834af8
Merge branch 'p-1245-manul-migration-based-on-storage-version' of htt…
wangminqi Dec 26, 2024
a89868d
chore: fix
wangminqi Dec 27, 2024
569742f
chore: fix
wangminqi Dec 27, 2024
17208cb
chore: fix
wangminqi Dec 27, 2024
43ac264
chore: fix
wangminqi Dec 28, 2024
d91689b
chore: fix clippy
wangminqi Dec 28, 2024
32a57b3
chore: fix
wangminqi Dec 28, 2024
963a2a4
chore: fix
wangminqi Dec 28, 2024
b0f8e73
feat: add all migrations
wangminqi Dec 29, 2024
d316122
chore: fix
wangminqi Dec 29, 2024
9655922
chore: fix
wangminqi Dec 29, 2024
8be0f13
chore: fix
wangminqi Dec 29, 2024
a7230bb
chore: fix
wangminqi Dec 29, 2024
7776751
chore: fix
wangminqi Jan 2, 2025
95ea03a
Merge branch 'dev' into p-1245-manul-migration-based-on-storage-version
wangminqi Jan 2, 2025
5a12c3a
chore: fix
wangminqi Jan 2, 2025
a8933f1
Merge branch 'p-1245-manul-migration-based-on-storage-version' of htt…
wangminqi Jan 2, 2025
13fd353
chore: fix
wangminqi Jan 2, 2025
a0c104f
chore: clippy
wangminqi Jan 2, 2025
093c82e
chore: add preimage clean
wangminqi Jan 3, 2025
8fd12b0
chore: fix
wangminqi Jan 3, 2025
8983ba4
chore: fix
wangminqi Jan 3, 2025
db6b60e
chore: fix
wangminqi Jan 3, 2025
11fb7d1
chore: fix
wangminqi Jan 3, 2025
6ed3089
chore: fix clippy
wangminqi Jan 3, 2025
a975e7d
chore: fix
wangminqi Jan 3, 2025
68a4299
chore: fix
wangminqi Jan 3, 2025
f69079a
Merge branch 'dev' into p-1245-manul-migration-based-on-storage-version
BillyWooo Jan 7, 2025
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
2 changes: 2 additions & 0 deletions parachain/runtime/litentry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ pub mod asset_config;
pub mod constants;
pub mod precompiles;

pub mod migration;
pub mod weights;
pub mod xcm_config;

Expand Down Expand Up @@ -156,6 +157,7 @@ pub type Executive = frame_executive::Executive<
// It was reverse order before.
// See the comment before collation related pallets too.
AllPalletsWithSystem,
migration::Migrations<Runtime>,
>;

impl fp_self_contained::SelfContainedCall for RuntimeCall {
Expand Down
363 changes: 362 additions & 1 deletion parachain/runtime/litentry/src/migration/mod.rs
Original file line number Diff line number Diff line change
@@ -1 +1,362 @@

// Copyright 2020-2024 Trust Computing GmbH.
// This file is part of Litentry.
//
// Litentry 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.
//
// Litentry 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 Litentry. If not, see <https://www.gnu.org/licenses/>.

// By the time of this migration on Litentry 9220 (polkadot stable2407)
// The current storage version: pallet version:
// scheduler: - 0
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm I think we only have StorageVersion now, no PalletVersion?

As I saw there was once a migration from PalletVersion to the new StorageVersion: https://paritytech.github.io/polkadot-sdk/master/src/frame_support/migrations.rs.html#255-261

Copy link
Member Author

@wangminqi wangminqi Jan 2, 2025

Choose a reason for hiding this comment

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

Yes, apply this migration first then try-runtime feature.

pallet version is indeed storage version. And truth is we are missing/wrong on both.

There are few pallets (weird) using a special "storageVersion" StorageMap holding the info. But palletVersion query onchain is not bonding to this storage at all. (see my comment on transactionPayment, vesting pallets)

The PalletVersion migration you providing is mostly likely not going to work for us. It just clean the Pallet version storage (if any) and assign the latest storage version without doing any specific migration.

// multisig: - 0
// preimage: - 0
// balances: - 0
// democracy: - 0
// bounties: - 0
// xcmpQueue: - 3
// polkadotXcm: - 0
// developerCommittee - 0
// developerCommitteeMembership - 0
// transactionPayment: V1Ancient 0
// vesting: V1 0

// Our target storage version: pallet version: (stable2407)
// scheduler: - 0 => 4
// multisig: - 0 => 1
// preimage: - 0 => 1
// balances: - 0 => 1
// democracy: - 0 => 1
// bounties: - 0 => 4
// xcmpQueue: - 3 => 5
// polkadotXcm: - 0 => 1
// developerCommittee - 0 => 4
// developerCommitteeMembership - 0 => 4
// transactionPayment: V1Ancient => V2 0 (it is built by genesis, so maybe no fix is fine)
// vesting: V1 0

// In try-runtime, current implementation, the storage version is not checked,
// Pallet version is used instead.
use frame_support::traits::{
Get, GetStorageVersion, OnRuntimeUpgrade, PalletInfoAccess, StorageVersion,
};
use frame_system::pallet_prelude::BlockNumberFor;
use pallet_balances::InactiveIssuance;
use pallet_scheduler::Agenda;
use sp_std::marker::PhantomData;
#[cfg(feature = "try-runtime")]
use sp_std::vec::Vec;

pub type Migrations<Runtime> = (
// Scheduler V0 => V4
// The official pallet does not provide any available migration
// We, Litentry Storage have two old unexecuted expired root tasks.
// This storage should be clean up and update storage version to V4 directly.
// PS: Looks like two old tasks fits V2/V3 structure
RemoveSchedulerOldStorage<Runtime>,
// Multsig V0 => V1
// Remove old unsettled call storage and do the refund
// Does not matter if we do have old storage or not
// Should simply works
pallet_multisig::migrations::v1::MigrateToV1<Runtime>,
// Preimage V0 => V1
// Migrate old StatusFor and PreimageFor Storage into new Storage
pallet_preimage::migration::v1::Migration<Runtime>,
// Balances V0 => V1
// The official pallet migration is not need since we do not have any XCM deactive accounts
// But our onchain inactiveIssuance storage of pallet_balance is non-negative
// TODO: Where does this number come from?
BalancesUpdateStorageVersionResetInactive<Runtime>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe it's just calculated from total_issuance - active_issuance.

I googled a bit:

How Does It Emerge?
inactive issuance can accumulate in the following scenarios:

Locks Applied by Other Pallets: Pallets like pallet-staking or pallet-democracy can introduce locks on tokens, marking them as inactive until certain conditions are met (e.g., unlocking after a referendum).
Misconfiguration or Bugs: An incorrect runtime implementation or upgrade could inadvertently increase inactive issuance.
Custom Pallets or Extensions: Developers may implement additional features that lock balances in ways not accounted for during runtime upgrades or storage migrations.
When tokens are locked via these mechanisms, they are no longer counted as "active issuance" but still exist in the chain's total issuance.

// Democracy V0 => V1
// This migration only effects onging proposal/referedum, NextExternal
// The referedum info's proposal hash is migrated if the hash is in old form (In our case, even for an onging one it will do nothing)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does it mean it's better to wait until there's no on-going ref/proposals to do this migration?

Copy link
Member Author

Choose a reason for hiding this comment

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

Technically it should be the same whether or not there is any onging ones.

see extra comment
// The referedum info's proposal hash is migrated if the hash is in old form (In our case, even for an onging one it will do nothing)

new ongoing ref/proposal should already have new form, so it will do nothing.

But from a ghost story perspective, of course it will be better if no onging ones.

pallet_democracy::migrations::v1::v1::Migration<Runtime>,
// Bounties V0 => V4
// The official migration does nothing but change pallet name and bump version
// So we just bump version storage instead
BountiesUpdateStorageVersion<Runtime>,
// V3 to V4
// XCMP QueueConfig has different default value
// Migration targeting at changing storage value to new default value if old value matched
// Our current Paseo setting has already hard-coded
// This migration should have no effect except bumping storage version
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mean Litentry?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, updated.

cumulus_pallet_xcmp_queue::migration::v4::MigrationToV4<Runtime>,
// V4 to V5
// Did nothing to storage
// Just checking MaxActiveOutboundChannels is not exceeded
// Our current Paseo Storage is 0
// This migration should have no effect except bumping storage version
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mean Litentry?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, updated.

cumulus_pallet_xcmp_queue::migration::v5::MigrateV4ToV5<Runtime>,
// PolkadotXcm V0 => V1
// Our storage is already correct
// This migration is for can old weightInfo into new weightInfo form
// Should do nothing but bump version storage for us
pallet_xcm::migration::v1::MigrateToV1<Runtime>,
// DeveloperCommittee V0 => V4
// The official migration does nothing but change pallet name and bump version
// So we just bump version storage instead
DeveloperCommitteeUpdateStorageVersion<Runtime>,
// DeveloperCommitteeMembership V0 => V4
// The official migration does nothing but change pallet name and bump version
// So we just bump version storage instead
DeveloperCommitteeMembershipUpdateStorageVersion<Runtime>,
);

pub struct RemoveSchedulerOldStorage<T>(PhantomData<T>);
impl<T> OnRuntimeUpgrade for RemoveSchedulerOldStorage<T>
where
T: frame_system::Config + pallet_scheduler::Config,
BlockNumberFor<T>: From<u32>,
{
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
log::info!("Pre check pallet scheduler storage only has two precise tasks leftover");
let one: BlockNumberFor<T> = 3067200u32.into();
let two: BlockNumberFor<T> = 2995200u32.into();
for (when, vec_schedule) in <Agenda<T>>::iter() {
assert!(when == one || when == two, "Extra schedule exists");
}
Ok(Vec::<u8>::new())
}

fn on_runtime_upgrade() -> frame_support::weights::Weight {
// Remove Scheduler Storage precisely of according block agenda only
// TODO: Very Weak safety
let one: BlockNumberFor<T> = 3067200u32.into();
let two: BlockNumberFor<T> = 2995200u32.into();
Agenda::<T>::remove(one);
Agenda::<T>::remove(two);

#[allow(deprecated)]
frame_support::storage::migration::remove_storage_prefix(
b"Scheduler",
b"StorageVersion",
&[],
);
StorageVersion::new(4).put::<pallet_scheduler::Pallet<T>>();
<T as frame_system::Config>::DbWeight::get().reads_writes(2, 4)
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
let one: BlockNumberFor<T> = 3067200u32.into();
let two: BlockNumberFor<T> = 2995200u32.into();
for (when, vec_schedule) in <Agenda<T>>::iter() {
assert!(when != one && when != two, "Old schedule still exists");
}

ensure!(StorageVersion::get::<pallet_scheduler::Pallet<T>>() == 4, "Must upgrade");

Ok(())
}
}

const BALANCES_LOG_TARGET: &str = "runtime::balances";
pub struct BalancesUpdateStorageVersionResetInactive<T>(PhantomData<T>);
impl<T> OnRuntimeUpgrade for BalancesUpdateStorageVersionResetInactive<T>
where
T: frame_system::Config + pallet_balances::Config,
{
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
ensure!(
StorageVersion::get::<pallet_balances::Pallet<T>>() == 0,
"Already upgrade to some non-zero version"
);
Ok(Vec::<u8>::new())
}

fn on_runtime_upgrade() -> frame_support::weights::Weight {
let on_chain_version = pallet_balances::Pallet::<T>::on_chain_storage_version();

if on_chain_version == 0 {
// Remove the old `StorageVersion` type.
frame_support::storage::unhashed::kill(&frame_support::storage::storage_prefix(
pallet_balances::Pallet::<T>::name().as_bytes(),
"StorageVersion".as_bytes(),
));

InactiveIssuance::<T>::kill();

Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO we should not touch it - just upgrading the storage version is enough

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, I will remove this part of code for security purpose.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you remove it? Then we are good to go I guess

// Set storage version to `1`.
StorageVersion::new(1).put::<pallet_balances::Pallet<T>>();

log::info!(target: BALANCES_LOG_TARGET, "Storage to version 1");
T::DbWeight::get().reads_writes(1, 3)
} else {
log::info!(
target: BALANCES_LOG_TARGET,
"Migration did not execute. This probably should be removed"
);
T::DbWeight::get().reads(1)
}
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
ensure!(StorageVersion::get::<pallet_balances::Pallet<T>>() == 1, "Must upgrade");
Ok(())
}
}

const BOUNTIES_LOG_TARGET: &str = "runtime::bounties";
pub struct BountiesUpdateStorageVersion<T>(PhantomData<T>);
impl<T> OnRuntimeUpgrade for BountiesUpdateStorageVersion<T>
where
T: frame_system::Config + pallet_bounties::Config,
{
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
ensure!(
StorageVersion::get::<pallet_bounties::Pallet<T>>() == 0,
"Already upgrade to some non-zero version"
);
Ok(Vec::<u8>::new())
}

fn on_runtime_upgrade() -> frame_support::weights::Weight {
let on_chain_version = pallet_bounties::Pallet::<T>::on_chain_storage_version();

if on_chain_version == 0 {
// Remove the old `StorageVersion` type.
frame_support::storage::unhashed::kill(&frame_support::storage::storage_prefix(
pallet_bounties::Pallet::<T>::name().as_bytes(),
"StorageVersion".as_bytes(),
));

// Set storage version to `4`.
StorageVersion::new(4).put::<pallet_bounties::Pallet<T>>();

log::info!(target: BOUNTIES_LOG_TARGET, "Storage to version 4");
T::DbWeight::get().reads_writes(1, 3)
} else {
log::info!(
target: BOUNTIES_LOG_TARGET,
"Migration did not execute. This probably should be removed"
);
T::DbWeight::get().reads(1)
}
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
ensure!(StorageVersion::get::<pallet_bounties::Pallet<T>>() == 4, "Must upgrade");
Ok(())
}
}

const DEVELOPER_COMMITTEE_LOG_TARGET: &str = "runtime::collective3";
pub struct DeveloperCommitteeUpdateStorageVersion<T>(PhantomData<T>);
impl<T> OnRuntimeUpgrade for DeveloperCommitteeUpdateStorageVersion<T>
where
T: frame_system::Config + pallet_collective::Config<pallet_collective::Instance3>,
{
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
ensure!(
StorageVersion::get::<pallet_collective::Pallet<T, pallet_collective::Instance3>>()
== 0,
"Already upgrade to some non-zero version"
);
Ok(Vec::<u8>::new())
}

fn on_runtime_upgrade() -> frame_support::weights::Weight {
let on_chain_version =
pallet_collective::Pallet::<T, pallet_collective::Instance3>::on_chain_storage_version(
);

if on_chain_version == 0 {
// Remove the old `StorageVersion` type.
frame_support::storage::unhashed::kill(&frame_support::storage::storage_prefix(
pallet_collective::Pallet::<T, pallet_collective::Instance3>::name().as_bytes(),
"StorageVersion".as_bytes(),
));

// Set storage version to `4`.
StorageVersion::new(4)
.put::<pallet_collective::Pallet<T, pallet_collective::Instance3>>();

log::info!(target: DEVELOPER_COMMITTEE_LOG_TARGET, "Storage to version 4");
T::DbWeight::get().reads_writes(1, 3)
} else {
log::info!(
target: DEVELOPER_COMMITTEE_LOG_TARGET,
"Migration did not execute. This probably should be removed"
);
T::DbWeight::get().reads(1)
}
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
ensure!(
StorageVersion::get::<pallet_collective::Pallet<T, pallet_collective::Instance3>>()
== 4,
"Must upgrade"
);
Ok(())
}
}

const DEVELOPER_COMMITTEE_MEMBERSHIP_LOG_TARGET: &str = "runtime::membership3";
pub struct DeveloperCommitteeMembershipUpdateStorageVersion<T>(PhantomData<T>);
impl<T> OnRuntimeUpgrade for DeveloperCommitteeMembershipUpdateStorageVersion<T>
where
T: frame_system::Config + pallet_membership::Config<pallet_membership::Instance3>,
{
#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<Vec<u8>, &'static str> {
ensure!(
StorageVersion::get::<pallet_membership::Pallet<T, pallet_membership::Instance3>>()
== 0,
"Already upgrade to some non-zero version"
);
Ok(Vec::<u8>::new())
}

fn on_runtime_upgrade() -> frame_support::weights::Weight {
let on_chain_version =
pallet_membership::Pallet::<T, pallet_membership::Instance3>::on_chain_storage_version(
);

if on_chain_version == 0 {
// Remove the old `StorageVersion` type.
frame_support::storage::unhashed::kill(&frame_support::storage::storage_prefix(
pallet_membership::Pallet::<T, pallet_membership::Instance3>::name().as_bytes(),
"StorageVersion".as_bytes(),
));

// Set storage version to `4`.
StorageVersion::new(4)
.put::<pallet_membership::Pallet<T, pallet_membership::Instance3>>();

log::info!(target: DEVELOPER_COMMITTEE_MEMBERSHIP_LOG_TARGET, "Storage to version 4");
T::DbWeight::get().reads_writes(1, 3)
} else {
log::info!(
target: DEVELOPER_COMMITTEE_MEMBERSHIP_LOG_TARGET,
"Migration did not execute. This probably should be removed"
);
T::DbWeight::get().reads(1)
}
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(_state: Vec<u8>) -> Result<(), &'static str> {
ensure!(
StorageVersion::get::<pallet_membership::Pallet<T, pallet_membership::Instance3>>()
== 4,
"Must upgrade"
);
Ok(())
}
}
6 changes: 6 additions & 0 deletions parachain/runtime/litentry/src/xcm_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,3 +244,9 @@ impl cumulus_pallet_xcmp_queue::Config for Runtime {
type WeightInfo = cumulus_pallet_xcmp_queue::weights::SubstrateWeight<Runtime>;
type PriceForSiblingDelivery = NoPriceForMessageDelivery<ParaId>;
}

// TODO:: remove after migration of storage version finished
impl cumulus_pallet_xcmp_queue::migration::v5::V5Config for Runtime {
// This must be the same as the `ChannelInfo` from the `Config`:
type ChannelList = ParachainSystem;
}
Loading
Loading