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

Disarm OnRuntimeUpgrade::pre/post_upgrade Tuple footgun #14759

Conversation

liamaharon
Copy link
Contributor

@liamaharon liamaharon commented Aug 14, 2023

Old versions of the Executive pallet calls pre_upgrade and post_upgrade on a tuple of pallets which was made a silent noop in #12537.

This can lead to confusion, see #13681.

This PR changes the noop behavior into a noisy error to inform devs of the incorrect usage and improves documentation so it's clear how to fix it.


also has insubstantial fix I forgot to push before I merged this PR: #14779 (comment)

@liamaharon liamaharon requested review from kianenigma, ggwpez and a team August 14, 2023 08:12
@liamaharon liamaharon added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”. labels Aug 14, 2023
liamaharon and others added 2 commits August 15, 2023 15:53
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
@liamaharon liamaharon marked this pull request as draft August 15, 2023 12:14
@liamaharon liamaharon changed the title Fail loudly when OnRuntimeUpgrade::pre/post_upgrade is called on a tuple of pallets Implement OnRuntimeUpgrade::pre/post_upgrade for tuples Aug 15, 2023
@liamaharon liamaharon changed the title Implement OnRuntimeUpgrade::pre/post_upgrade for tuples Remove OnRuntimeUpgrade::pre/post_upgrade tuple footgun Aug 16, 2023
@liamaharon liamaharon changed the title Remove OnRuntimeUpgrade::pre/post_upgrade tuple footgun Remove OnRuntimeUpgrade::pre/post_upgrade Tuple footgun Aug 16, 2023
@liamaharon liamaharon marked this pull request as ready for review August 16, 2023 04:39
@KiChjang
Copy link
Contributor

CI is complaining about the [`Tuple`] syntax, which needs to be fixed.

liamaharon and others added 3 commits August 16, 2023 16:24
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
frame/support/src/traits/hooks.rs Outdated Show resolved Hide resolved
frame/support/src/traits/hooks.rs Outdated Show resolved Hide resolved
liamaharon and others added 2 commits August 16, 2023 23:20
Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>
Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>
@liamaharon liamaharon requested review from a team and removed request for kianenigma, ggwpez and KiChjang August 16, 2023 16:03
@liamaharon liamaharon changed the title Remove OnRuntimeUpgrade::pre/post_upgrade Tuple footgun Disarm OnRuntimeUpgrade::pre/post_upgrade Tuple footgun Aug 16, 2023
@liamaharon
Copy link
Contributor Author

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 0c60003 into master Aug 16, 2023
53 checks passed
@paritytech-processbot paritytech-processbot bot deleted the liam-return-error-on-incorrect-pre_upgrade-post_upgrade-tuple-usage branch August 16, 2023 17:04
Ank4n pushed a commit that referenced this pull request Aug 20, 2023
* return error on incorrect tuple usage of pre_upgrade and post_upgrade

* add test

* comment lint

* Update frame/support/src/traits/hooks.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Update frame/support/src/traits/hooks.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* address feedback

* Update frame/support/src/traits/hooks.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Update frame/support/src/traits/hooks.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Update frame/support/src/traits/hooks.rs

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* muharem comments

* Update frame/support/src/traits/hooks.rs

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

* Update frame/support/src/traits/hooks.rs

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

* remove useless type

---------

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants