-
Notifications
You must be signed in to change notification settings - Fork 2
fix: check for uninitialized voting modules #446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
0xOneTony
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm but lets merge #448 first
5ef7688 to
1e23489
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Proposal Validation Fails for Zero Voting Module
The validation for an invalid voting module in submitUpgradeProposal, submitCouncilMemberElectionsProposal, and submitFundingProposal incorrectly checks bytes(proposalTypeConfig.name).length == 0. This allows proposals with a non-empty name but an invalid (zero) module address to pass validation. The check should instead directly validate if the module address is zero (votingModule == address(0)), which aligns with the ProposalValidator_InvalidVotingModule error's intent.
packages/contracts-bedrock/src/governance/ProposalValidator.sol#L342-L346
optimism/packages/contracts-bedrock/src/governance/ProposalValidator.sol
Lines 342 to 346 in 05eda74
| // Validate voting module exists | |
| if (bytes(proposalTypeConfig.name).length == 0) { | |
| revert ProposalValidator_InvalidVotingModule(); | |
| } |
Was this report helpful? Give feedback by reacting with 👍 or 👎
* fix: check for uninitialized voting modules * fix: pre-pr * fix: pre-pr
* fix: check for uninitialized voting modules * fix: pre-pr * fix: pre-pr
Closes OPT-937
Fixes [L-0] Voting Module is not validated during
submissionRegarding the voting module changed validation. It is not required given that the move to vote function retrieves the
proposalHashbased on currentProposalTypeConfiguratorconfig. In case thevotingModuleAddresshas been changed for a given ProposalType, the computed hash won't be the same and the move to vote function will revert due to an unexisting proposal.In case we anyways want to add this validation for better dev experience, it would involve a design modification, given that proposal data in move to vote functions is retrieved from the computed hash based on the proposal data, including the proposal types configurator voting module