Revert for nonexistent proposals in proposalExecutionPlan

[Azzurra88]

Add existence check to GovernorTimelockAccess.proposalExecutionPlan(), matching the error behavior of GovernorStorage view functions.

[changeset-bot]

⚠️ No Changeset found

Latest commit: a3d8fae

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[coderabbitai]

Walkthrough

The pull request adds input validation to the proposalExecutionPlan function in GovernorTimelockAccess.sol by introducing a guard that checks if a proposal snapshot equals zero. When this condition is met, the function reverts with a GovernorNonexistentProposal error, catching non-existent proposals early. A corresponding test case is added to verify this validation behavior with a non-existent proposal ID. No public function signatures or data structures are modified.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding a revert mechanism for nonexistent proposals in the proposalExecutionPlan function.
Description check	✅ Passed	The description is directly related to the changeset, explaining the purpose of the existence check being added to match error behavior of GovernorStorage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 308c491 and a3d8fae.

📒 Files selected for processing (2)

• contracts/governance/extensions/GovernorTimelockAccess.sol
• test/governance/extensions/GovernorTimelockAccess.test.js

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)

• GitHub Check: Redirect rules - solidity-contracts
• GitHub Check: Header rules - solidity-contracts
• GitHub Check: Pages changed - solidity-contracts
• GitHub Check: halmos
• GitHub Check: tests-foundry
• GitHub Check: slither
• GitHub Check: tests
• GitHub Check: tests-upgradeable
• GitHub Check: coverage

🔇 Additional comments (2)

test/governance/extensions/GovernorTimelockAccess.test.js (1)

354-359: LGTM! Test correctly verifies the validation behavior.

The test case appropriately verifies that proposalExecutionPlan reverts with GovernorNonexistentProposal when called with a non-existent proposal ID, and confirms the error includes the correct proposal ID argument.

contracts/governance/extensions/GovernorTimelockAccess.sol (1)

161-163: LGTM! Defensive validation aligns with Governor base contract.

The existence check correctly uses proposalSnapshot(proposalId) == 0 to detect non-existent proposals and reverts early with a clear error. This pattern aligns with the base Governor contract and improves robustness. Note that GovernorStorage extensions intentionally use descriptionHash == 0 for validation since they maintain separate proposal detail storage, so the validation method is context-dependent.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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