Companion for substrate#8724 #2994
Conversation
octol
added
B7-runtimenoteworthy
C1-low
octol
added
E1-database_migration
runtime/kusama/src/lib.rs
Outdated
| 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>() { |
There was a problem hiding this comment.
I think we could use expect here as Grandpa is part of pallets in construct_runtime, thus it has been given a name.
There was a problem hiding this comment.
I think we could use
expecthere 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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I don't know, for me the proof makes sense so both are fine.
…pa-pallet-framev2
…pa-pallet-framev2
…pa-pallet-framev2
octol
added
the
D5-nicetohaveaudit ⚠️
Cargo.lock
Outdated
| version = "3.1.0" | ||
| source = "git+https://github.com/paritytech/substrate?branch=jon/migrate-grandpa-pallet-framev2#32ba95b6eac24b79de07bc0e2b64e1f5786c028c" |
There was a problem hiding this comment.
Added this to make the substrate PR companion check go green. I hoping here that the cargo update that processbot does when merging will reset this. Otherwise I'll have to revert this before merging the companion
|
Error: Companion update failed: Command 'Command { std: "git" "merge" "origin/master" "--no-ff" "--no-edit", kill_on_drop: false }' failed with status Some(1); output: no output |
|
bot merge |
|
Waiting for commit status. |
|
Merge aborted: Checks failed for 0ccd4b0 |
|
bot merge |
|
Trying merge. |
|
bot merge |
|
Trying merge. |
ghost
merged commit 
Companion for paritytech/substrate#8724
Migrate grandpa pallet to the new storage prefix to support converting the pallet to the new macros