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

Collective pallet: max proposal weight #13771

Merged
merged 4 commits into from
Apr 19, 2023

Conversation

muharem
Copy link
Contributor

@muharem muharem commented Mar 30, 2023

Introducing a limit for the weight of the proposed call.

polkadot companion: paritytech/polkadot#6983
cumulus companion: paritytech/cumulus#2410

@muharem muharem added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit F3-breaks_API This PR changes public API; next release should be major. labels Mar 30, 2023
@muharem muharem changed the title Collective Pallet: max proposal weight Collective pallet: max proposal weight Mar 30, 2023
@muharem
Copy link
Contributor Author

muharem commented Mar 31, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

@muharem muharem requested review from ggwpez and bkchr and removed request for ggwpez March 31, 2023 14:01
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Personally I would probably favor a disapprove_proposal that can be initiated by a simple majority of collective members to cancel a call.

Comment on lines +717 to +721
let proposal_weight = proposal.get_dispatch_info().weight;
ensure!(
proposal_weight.all_lte(T::MaxProposalWeight::get()),
Error::<T, I>::WrongProposalWeight
);
Copy link
Member

Choose a reason for hiding this comment

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

The proposal weight is already added to the weight of propose if threshold < 2. So, we would not even add this tx to a block when the weight is too big. TLDR: This should not be required.

Copy link
Member

Choose a reason for hiding this comment

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

But yeah, to have the properties as do_propose_proposed it makes sense.

@kianenigma kianenigma added D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited and removed D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit labels Apr 13, 2023
@jakoblell jakoblell added D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Apr 17, 2023
@muharem
Copy link
Contributor Author

muharem commented Apr 19, 2023

bot merge

@paritytech-processbot
Copy link

Error: Github API says paritytech/cumulus#2410 is not mergeable

@muharem
Copy link
Contributor Author

muharem commented Apr 19, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

@muharem
Copy link
Contributor Author

muharem commented Apr 19, 2023

bot merge

@paritytech-processbot
Copy link

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

@ggwpez
Copy link
Member

ggwpez commented Apr 19, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 50de15d into master Apr 19, 2023
@paritytech-processbot paritytech-processbot bot deleted the muharem-collectives-max-proposal-weight branch April 19, 2023 11:00
AurevoirXavier added a commit to darwinia-network/darwinia that referenced this pull request Jul 11, 2023
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* collective: max proposal weight

* fix test

---------

Co-authored-by: parity-processbot <>
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 D1-audited 👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited F3-breaks_API This PR changes public API; next release should be major.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants