Fix: Enforce proposal status when calling Governor.execute depending on proposalNeedsQueuing

[Amxx]

Fixes H-03

PR Checklist

• Tests
• Documentation
• Changeset entry (run npx changeset add)

[changeset-bot]

🦋 Changeset detected

Latest commit: 73ee8d5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
openzeppelin-solidity	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Amxx]

This is no longer necessary because Governor.execute enforces status = Queued

[Amxx]

Before this change, this would not have reverted

[coderabbitai]

Walkthrough

The changes introduce dynamic queuing logic to the Governor contract. The proposalNeedsQueuing function now determines whether a proposal must be in the Queued state during execution. When proposalNeedsQueuing returns true, only Queued state is permitted; otherwise, only Succeeded state is permitted. The GovernorNotQueuedProposal error is removed from the interface. The GovernorTimelockCompound._executeOperations method no longer enforces a queued-state guard. Tests are updated across multiple governance extensions to reflect the new conditional state validation logic and use GovernorUnexpectedProposalState errors with corresponding state bitmaps.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: enforcing proposal status in Governor.execute based on the proposalNeedsQueuing condition, which aligns with the core security fix addressed across multiple files.
Description check	✅ Passed	The description relates to the changeset by referencing the specific audit issue (H-03) being fixed and describing the enforcement of proposal status based on proposalNeedsQueuing, which matches the implementation changes throughout the PR.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/governor-enforce-queue

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.changeset/silver-falcons-lay.md:
- Line 5: Fix the typo in the changeset note by replacing the word "Scrict" with
"Strict" in the backtick-quoted entry for `Governor` so the line reads:
"`Governor`: Strict enforcement of the expected proposal state depending on
`proposalNeedsQueuing` when calling `execute`."

In `@test/governance/extensions/GovernorTimelockAccess.test.js`:
- Around line 265-266: Fix the typo in the comment ("queueud" → "queued") and
make the revert assertion specific by expecting the
GovernorUnexpectedProposalState error when calling this.helper.execute();
replace the generic await expect(this.helper.execute()).to.be.reverted with an
assertion that the call is reverted with the GovernorUnexpectedProposalState
(include the queued-only allowed states message/args if applicable) so the test
verifies the exact failure mode for this.helper.execute().

In `@test/governance/extensions/GovernorTimelockCompound.test.js`:
- Around line 159-164: The revert assertion is checking the wrong proposal id:
update the .withArgs(...) call used with
.to.be.revertedWithCustomError(this.mock, 'GovernorUnexpectedProposalState') to
pass the local duplicate proposal identifier id (the one passed to
this.helper.execute()) instead of this.proposal.id; keep the other args
(ProposalState.Succeeded and
GovernorHelper.proposalStatesToBitMap([ProposalState.Queued])) unchanged so the
assertion targets the duplicate proposal created in this test.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ca6f5fa and ca2d1cc.

📒 Files selected for processing (9)

• .changeset/silver-falcons-lay.md
• contracts/governance/Governor.sol
• contracts/governance/IGovernor.sol
• contracts/governance/extensions/GovernorTimelockCompound.sol
• test/governance/Governor.test.js
• test/governance/extensions/GovernorTimelockAccess.test.js
• test/governance/extensions/GovernorTimelockCompound.test.js
• test/governance/extensions/GovernorTimelockControl.test.js
• test/governance/extensions/GovernorVotesQuorumFraction.test.js

💤 Files with no reviewable changes (2)

• contracts/governance/extensions/GovernorTimelockCompound.sol
• contracts/governance/IGovernor.sol