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

pallet-scheduler: Introduce OriginPrivilegeCmp #10078

Merged
merged 5 commits into from
Oct 29, 2021

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Oct 21, 2021

When a scheduled task should be canceled, the origin that tries to
cancel the task is compared to the origin the task should be executed
with. Before this pr this check only allowed that both origins are
equal. However, this is problematic as this means that for example a
council origin it needs to be have the same amount of yes votes to
cancel the scheduled task. While a council origin with more yes votes
should be able to cancel this task. This happened recently on Kusama and
lead to a failed cancelation of a scheduled task. With this pr the two
origins are compared and the cancelling origin needs to have greater or
equal privileges as the origin that scheduled the task. What a greater,
equal or less privilege is, can be configured in the runtime.

For simplicity, a EqualPrivilegeOnly implementation is provided that
only checks if two origins are equal. So, this mimics the old behaviour.

polkadot companion: paritytech/polkadot#4166

skip check-dependent-cumulus

When a scheduled task should be canceled, the origin that tries to
cancel the task is compared to the origin the task should be executed
with. Before this pr this check only allowed that both origins are
equal. However, this is problematic as this means that for example a
council origin it needs to be have the same amount of yes votes to
cancel the scheduled task. While a council origin with more yes votes
should be able to cancel this task. This happened recently on Kusama and
lead to a failed cancelation of a scheduled task. With this pr the two
origins are compared and the cancelling origin needs to have greater or
equal privileges as the origin that scheduled the task. What a greater,
equal or less privilege is, can be configured in the runtime.

For simplicity, a `EqualPrivilegeOnly` implementation is provided that
only checks if two origins are equal. So, this mimics the old behaviour.
@bkchr bkchr 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 labels Oct 21, 2021
Copy link
Contributor

@gui1117 gui1117 left a comment

Choose a reason for hiding this comment

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

looks good to me.

@gui1117
Copy link
Contributor

gui1117 commented Oct 27, 2021

needs another review and then merge master and a companion

@bkchr
Copy link
Member Author

bkchr commented Oct 29, 2021

bot merge

@paritytech-processbot
Copy link

Error: Github API says paritytech/polkadot#4166 is not mergeable

@bkchr
Copy link
Member Author

bkchr commented Oct 29, 2021

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 945377f into master Oct 29, 2021
@paritytech-processbot paritytech-processbot bot deleted the bckr-pallet-schedule branch October 29, 2021 16:00
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
* pallet-scheduler: Introduce `OriginPrivilegeCmp`

When a scheduled task should be canceled, the origin that tries to
cancel the task is compared to the origin the task should be executed
with. Before this pr this check only allowed that both origins are
equal. However, this is problematic as this means that for example a
council origin it needs to be have the same amount of yes votes to
cancel the scheduled task. While a council origin with more yes votes
should be able to cancel this task. This happened recently on Kusama and
lead to a failed cancelation of a scheduled task. With this pr the two
origins are compared and the cancelling origin needs to have greater or
equal privileges as the origin that scheduled the task. What a greater,
equal or less privilege is, can be configured in the runtime.

For simplicity, a `EqualPrivilegeOnly` implementation is provided that
only checks if two origins are equal. So, this mimics the old behaviour.

* FMT

* fix import

* Small optimizations

Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
AurevoirXavier added a commit to darwinia-network/darwinia-common that referenced this pull request Aug 4, 2022
AurevoirXavier added a commit to darwinia-network/darwinia that referenced this pull request Aug 16, 2022
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* pallet-scheduler: Introduce `OriginPrivilegeCmp`

When a scheduled task should be canceled, the origin that tries to
cancel the task is compared to the origin the task should be executed
with. Before this pr this check only allowed that both origins are
equal. However, this is problematic as this means that for example a
council origin it needs to be have the same amount of yes votes to
cancel the scheduled task. While a council origin with more yes votes
should be able to cancel this task. This happened recently on Kusama and
lead to a failed cancelation of a scheduled task. With this pr the two
origins are compared and the cancelling origin needs to have greater or
equal privileges as the origin that scheduled the task. What a greater,
equal or less privilege is, can be configured in the runtime.

For simplicity, a `EqualPrivilegeOnly` implementation is provided that
only checks if two origins are equal. So, this mimics the old behaviour.

* FMT

* fix import

* Small optimizations

Co-authored-by: Shawn Tabrizi <shawntabrizi@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants