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

system on_runtime_upgrade should be triggered after custom on runtime upgrade, similarly to other pallet #8683

Closed
gui1117 opened this issue Apr 28, 2021 · 4 comments · Fixed by #8687
Labels
J0-enhancement An additional feature request.

Comments

@gui1117
Copy link
Contributor

gui1117 commented Apr 28, 2021

About this code:

pub fn execute_on_runtime_upgrade() -> frame_support::weights::Weight {
let mut weight = 0;
weight = weight.saturating_add(
<frame_system::Pallet<System> as OnRuntimeUpgrade>::on_runtime_upgrade(),
);
weight = weight.saturating_add(COnRuntimeUpgrade::on_runtime_upgrade());
weight = weight.saturating_add(<AllPallets as OnRuntimeUpgrade>::on_runtime_upgrade());
weight
}

I don't see why frame system pallet would take prioritised before the custom one whereas other pallet doesn't.

The custom on runtime upgrade should be able to do any kind of migration prior to pallet migration, allowing to bypass them.

cc @drewstone @al3mart

@gui1117 gui1117 added the J0-enhancement An additional feature request. label Apr 28, 2021
@al3mart
Copy link
Contributor

al3mart commented Apr 28, 2021

I'm cc'ing @apopiak here :)

@drewstone
Copy link
Contributor

@bkchr proposed removing all on_runtime_upgrade logic from the pallets and handling it directly at the runtime level, which seems like a good option imo. That way a team can check the references and order however they please.

@shawntabrizi
Copy link
Member

Was there some bigger issue that was found which pointed to this?

@drewstone
Copy link
Contributor

@shawntabrizi we're trying to run the next Edgeware upgrade and it seems the ref count migrations can't be updated on the fly (it migrates to triple count from double when we're still on single).

If there's no way to custom execute code before this, our only resolution is to fork substrate which I'd ideally try to avoid.

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J0-enhancement An additional feature request.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants