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

Companion for substrate#8724 #2994

Merged
15 commits merged into from
May 26, 2021
Merged

Companion for substrate#8724 #2994

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
a28bfe9
runtime: create migrations for grandpa storage prefix
octol May 6, 2021
9c73867
runtime: address review comments
octol May 12, 2021
e778278
Merge remote-tracking branch 'upstream/master' into jon/migrate-grand…
octol May 12, 2021
e2dcfa9
Merge remote-tracking branch 'upstream/master' into jon/migrate-grand…
octol May 13, 2021
77e477f
runtime: update spec_version
octol May 13, 2021
bda222e
Merge remote-tracking branch 'upstream/master' into jon/migrate-grand…
octol May 20, 2021
57e2491
westend: fix incorrect merge
octol May 20, 2021
6da0632
runtime: change to v3.1 from v4 for grandpa migrations
octol May 20, 2021
25ba8b8
Merge remote-tracking branch 'upstream/master' into jon/migrate-grand…
octol May 20, 2021
c4aabf3
cargo.lock: manually specify pallet-grandpa in lock file
octol May 21, 2021
850d6f6
cargo.lock: fix typo
octol May 21, 2021
5a5b655
Merge branch 'master' into jon/migrate-grandpa-pallet-framev2
andresilva May 26, 2021
0ccd4b0
update substrate
andresilva May 26, 2021
e422708
Merge branch 'master' into jon/migrate-grandpa-pallet-framev2
andresilva May 26, 2021
650edbe
update substrate
andresilva May 26, 2021
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
32 changes: 31 additions & 1 deletion runtime/kusama/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1365,6 +1365,36 @@ impl frame_support::traits::OnRuntimeUpgrade for ParachainHostConfigurationMigra
}
}

pub struct GrandpaStoragePrefixMigration;
impl frame_support::traits::OnRuntimeUpgrade for GrandpaStoragePrefixMigration {
fn on_runtime_upgrade() -> frame_support::weights::Weight {
use frame_support::traits::PalletInfo;
if let Some(name) = <Runtime as frame_system::Config>::PalletInfo::name::<Grandpa>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could use expect here as Grandpa is part of pallets in construct_runtime, thus it has been given a name.

Copy link
Contributor

@aurexav aurexav Oct 18, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think we could use expect here as Grandpa is part of pallets in construct_runtime, thus it has been given a name.

But this will break the chain if there is something wrong. IMO panic in on_initialize is unrecoverable. Very dangerous code.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the proof given is correct, GrandPa is declared in construct_runtime, construct_runtime will implement the PalletInfo and give a name to every pallet, so GrandPa must have a name.
This code path should also have been tested at least one time before going to production.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that code is correct.
But why not if let Some. Any specific reason?

This is a runtime level custom migration instead #[pallet::hook]. And many other projects will follow this code.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know, for me the proof makes sense so both are fine.

pallet_grandpa::migrations::v4::migrate::<Runtime, Grandpa, _>(name)
} else {
log::warn!("Grandpa storage prefix migration skipped: unable to fetch name");
0
}
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<(), &'static str> {
use frame_support::traits::PalletInfo;
if let Some(name) = <Runtime as frame_system::Config>::PalletInfo::name::<Grandpa>() {
pallet_grandpa::migrations::v4::pre_migration::<Grandpa, _>(name);
} else {
log::warn!("Grandpa storage prefix migration pre-upgrade skipped: unable to fetch name");
}
Ok(())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade() -> Result<(), &'static str> {
pallet_grandpa::migrations::v4::post_migration::<Grandpa>();
Ok(())
}
}

/// The address format for describing accounts.
pub type Address = sp_runtime::MultiAddress<AccountId, ()>;
/// Block header type as expected by this runtime.
Expand Down Expand Up @@ -1394,7 +1424,7 @@ pub type Executive = frame_executive::Executive<
frame_system::ChainContext<Runtime>,
Runtime,
AllPallets,
ParachainHostConfigurationMigration,
(ParachainHostConfigurationMigration, GrandpaStoragePrefixMigration),
>;
/// The payload being signed in the transactions.
pub type SignedPayload = generic::SignedPayload<Call, SignedExtra>;
Expand Down
32 changes: 31 additions & 1 deletion runtime/polkadot/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1027,6 +1027,36 @@ construct_runtime! {
}
}

pub struct GrandpaStoragePrefixMigration;
impl frame_support::traits::OnRuntimeUpgrade for GrandpaStoragePrefixMigration {
fn on_runtime_upgrade() -> frame_support::weights::Weight {
use frame_support::traits::PalletInfo;
if let Some(name) = <Runtime as frame_system::Config>::PalletInfo::name::<Grandpa>() {
pallet_grandpa::migrations::v4::migrate::<Runtime, Grandpa, _>(name)
} else {
log::warn!("Grandpa storage prefix migration skipped: unable to fetch name");
0
}
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<(), &'static str> {
use frame_support::traits::PalletInfo;
if let Some(name) = <Runtime as frame_system::Config>::PalletInfo::name::<Grandpa>() {
pallet_grandpa::migrations::v4::pre_migration::<Grandpa, _>(name);
} else {
log::warn!("Grandpa storage prefix migration pre-upgrade skipped: unable to fetch name");
}
Ok(())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade() -> Result<(), &'static str> {
pallet_grandpa::migrations::v4::post_migration::<Grandpa>();
Ok(())
}
}

/// The address format for describing accounts.
pub type Address = sp_runtime::MultiAddress<AccountId, ()>;
/// Block header type as expected by this runtime.
Expand Down Expand Up @@ -1057,7 +1087,7 @@ pub type Executive = frame_executive::Executive<
frame_system::ChainContext<Runtime>,
Runtime,
AllPallets,
(),
GrandpaStoragePrefixMigration,
>;
/// The payload being signed in transactions.
pub type SignedPayload = generic::SignedPayload<Call, SignedExtra>;
Expand Down
32 changes: 31 additions & 1 deletion runtime/rococo/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ pub type Executive = frame_executive::Executive<
frame_system::ChainContext<Runtime>,
Runtime,
AllPallets,
(),
GrandpaStoragePrefixMigration,
>;
/// The payload being signed in transactions.
pub type SignedPayload = generic::SignedPayload<Call, SignedExtra>;
Expand Down Expand Up @@ -251,6 +251,36 @@ construct_runtime! {
}
}

pub struct GrandpaStoragePrefixMigration;
impl frame_support::traits::OnRuntimeUpgrade for GrandpaStoragePrefixMigration {
fn on_runtime_upgrade() -> frame_support::weights::Weight {
use frame_support::traits::PalletInfo;
if let Some(name) = <Runtime as frame_system::Config>::PalletInfo::name::<Grandpa>() {
pallet_grandpa::migrations::v4::migrate::<Runtime, Grandpa, _>(name)
} else {
log::warn!("Grandpa storage prefix migration skipped: unable to fetch name");
0
}
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<(), &'static str> {
use frame_support::traits::PalletInfo;
if let Some(name) = <Runtime as frame_system::Config>::PalletInfo::name::<Grandpa>() {
pallet_grandpa::migrations::v4::pre_migration::<Grandpa, _>(name);
} else {
log::warn!("Grandpa storage prefix migration pre-upgrade skipped: unable to fetch name");
}
Ok(())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade() -> Result<(), &'static str> {
pallet_grandpa::migrations::v4::post_migration::<Grandpa>();
Ok(())
}
}

pub struct BaseFilter;
impl Filter<Call> for BaseFilter {
fn filter(_call: &Call) -> bool {
Expand Down
32 changes: 31 additions & 1 deletion runtime/westend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1054,6 +1054,36 @@ impl frame_support::traits::OnRuntimeUpgrade for ParachainHostConfigurationMigra
}
}

pub struct GrandpaStoragePrefixMigration;
impl frame_support::traits::OnRuntimeUpgrade for GrandpaStoragePrefixMigration {
fn on_runtime_upgrade() -> frame_support::weights::Weight {
use frame_support::traits::PalletInfo;
if let Some(name) = <Runtime as frame_system::Config>::PalletInfo::name::<Grandpa>() {
pallet_grandpa::migrations::v4::migrate::<Runtime, Grandpa, _>(name)
} else {
log::warn!("Grandpa storage prefix migration skipped: unable to fetch name");
0
}
}

#[cfg(feature = "try-runtime")]
fn pre_upgrade() -> Result<(), &'static str> {
use frame_support::traits::PalletInfo;
if let Some(name) = <Runtime as frame_system::Config>::PalletInfo::name::<Grandpa>() {
pallet_grandpa::migrations::v4::pre_migration::<Grandpa, _>(name);
} else {
log::warn!("Grandpa storage prefix migration pre-upgrade skipped: unable to fetch name");
}
Ok(())
}

#[cfg(feature = "try-runtime")]
fn post_upgrade() -> Result<(), &'static str> {
pallet_grandpa::migrations::v4::post_migration::<Grandpa>();
Ok(())
}
}

/// The address format for describing accounts.
pub type Address = sp_runtime::MultiAddress<AccountId, ()>;
/// Block header type as expected by this runtime.
Expand Down Expand Up @@ -1083,7 +1113,7 @@ pub type Executive = frame_executive::Executive<
frame_system::ChainContext<Runtime>,
Runtime,
AllPallets,
ParachainHostConfigurationMigration,
(ParachainHostConfigurationMigration, GrandpaStoragePrefixMigration),
>;
/// The payload being signed in transactions.
pub type SignedPayload = generic::SignedPayload<Call, SignedExtra>;
Expand Down