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

Introduce OriginPrivilegeCmp #4166

Merged
merged 4 commits into from
Oct 29, 2021
Merged

Introduce OriginPrivilegeCmp #4166

merged 4 commits into from
Oct 29, 2021

Conversation

bkchr
Copy link
Member

@bkchr bkchr commented Oct 28, 2021

Make use of the new OriginPrivilegeCmp feature of pallet scheduler.
The idea is to make sure that a council origin with more yes votes has
higher privileges than a council origin with less yes votes. This solves
a problem that happened recently on Kusama where the council tried to
cancel a scheduled task, but that required that the same council origin
was used while the cancel motion had more yes votes than the origin
motion that scheduled this task. With this origin privilege compare it
should now be solved by checking the yes votes directly.

paritytech/substrate#10078

Make use of the new `OriginPrivilegeCmp` feature of pallet scheduler.
The idea is to make sure that a council origin with more yes votes has
higher privileges than a council origin with less yes votes. This solves
a problem that happened recently on Kusama where the council tried to
cancel a scheduled task, but that required that the same council origin
was used while the cancel motion had more yes votes than the origin
motion that scheduled this task. With this origin privilege compare it
should now be solved by checking the yes votes directly.
@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 28, 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, maybe comparing the proportion makes more sense

(
OriginCaller::Council(pallet_collective::RawOrigin::Members(l_yes_votes, _)),
OriginCaller::Council(pallet_collective::RawOrigin::Members(r_yes_votes, _)),
) => Some(l_yes_votes.cmp(r_yes_votes)),
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it would make more sense to check the proportion, consider the ScheduleOrigin is a proportion check.

We can use Perbill, u64 (as long as count is u32) or biguint

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit d3a9b5b into master Oct 29, 2021
@paritytech-processbot paritytech-processbot bot deleted the bkchr-origin-cmp branch October 29, 2021 16:53
ordian added a commit that referenced this pull request Oct 30, 2021
* master:
  remove duplicate Deposit from OnUnbalanced implementation (#4180)
  differentiate log messages (#4183)
  increase ump_service_total_weight's default value (#4127)
  companion PR to removal of light client (#4105)
  Introduce `OriginPrivilegeCmp` (#4166)
emostov pushed a commit that referenced this pull request Nov 1, 2021
* Introduce `OriginPrivilegeCmp`

Make use of the new `OriginPrivilegeCmp` feature of pallet scheduler.
The idea is to make sure that a council origin with more yes votes has
higher privileges than a council origin with less yes votes. This solves
a problem that happened recently on Kusama where the council tried to
cancel a scheduled task, but that required that the same council origin
was used while the cancel motion had more yes votes than the origin
motion that scheduled this task. With this origin privilege compare it
should now be solved by checking the yes votes directly.

* Feedback

* update lockfile for substrate

Co-authored-by: parity-processbot <>
wischli added a commit to KILTprotocol/kilt-node that referenced this pull request Jan 31, 2022
wischli added a commit to KILTprotocol/kilt-node that referenced this pull request Feb 9, 2022
* chore: bump Polkadot dependencies to v0.9.16

* refactor: use AllPalletsWithSystem hook

* chore: remove crowdloan pallet

* fix: apply no DefaultAccount check

* fix: switch to default MaxEncodedLen

paritytech/substrate#10662

* fix: apply asset quota + runtime state_version

quota: paritytech/substrate#10382
state: paritytech/substrate#9732

* fix: Preimage registrar and Scheduler integration

Scheduler: paritytech/substrate#10356
OriginPriviligeCmp: paritytech/polkadot#4166

* fix: OrderedSet

* fix: apply new fork_id to chainspecs

paritytech/substrate#10463

* fix: apply no default account for SudoConfig

* fix: apply name changes for GrandPa

paritytech/substrate#10463

* fix: EnsureOneOf

paritytech/substrate#10379

* fix: preimage weights

* fix: parachain client

* fix: clippy copied weights

* fix: bump deps

* tests: attempt staking fix

* bench: attempt to fix default accountid for dids

* fix: staking unit test pallet order execution

* fix: did unit benchmarks

* fix: fmt

* fix: revert to old hook order execution

* bench: run manually for preimage + scheduler

* fix: only import TaskManager for try-runtime feature

* fix: use correct scheduler migration + add logs

* refactor: use explicit para runtime executors

* fix: apply code review by @Diiaablo95

* chore: apply suggestion from @weichweich review

* chore: bump deps

* fix: deps
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