use _validateCancel instead of overriding cancel

contracts/governance/Governor.sol

@@ -484,13 +484,21 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         // changes it. The `getProposalId` duplication has a cost that is limited, and that we accept.
         uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
 
+        if (!_validateCancel(proposalId)) revert GovernorUnableToCancel(proposalId, _msgSender());
+
+        return _cancel(targets, values, calldatas, descriptionHash);
+    }
+
+    function _validateCancel(uint256 proposalId) internal view virtual returns (bool) {
         // public cancel restrictions (on top of existing _cancel restrictions).
-        _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Pending));
+        if (_encodeStateBitmap(state(proposalId)) & _encodeStateBitmap(ProposalState.Pending) == bytes32(0)) {
+            return false;
+        }
         if (_msgSender() != proposalProposer(proposalId)) {
-            revert GovernorOnlyProposer(_msgSender());
+            return false;
         }
 
-        return _cancel(targets, values, calldatas, descriptionHash);
+        return true;
     }
 
     /**

contracts/governance/IGovernor.sol

@@ -39,11 +39,6 @@ interface IGovernor is IERC165, IERC6372 {
      */
     error GovernorDisabledDeposit();
 
-    /**
-     * @dev The `account` is not a proposer.
-     */
-    error GovernorOnlyProposer(address account);
-
     /**
      * @dev The `account` is not the governance executor.
      */
@@ -112,6 +107,11 @@ interface IGovernor is IERC165, IERC6372 {
      */
     error GovernorInvalidSignature(address voter);
 
+    /**
+     * @dev The given `account` is unable to cancel the proposal with given `proposalId`.
+     */
+    error GovernorUnableToCancel(uint256 proposalId, address account);
+
     /**
      * @dev Emitted when a proposal is created.
      */

contracts/governance/extensions/GovernorProposalGuardian.sol

@@ -14,38 +14,32 @@ abstract contract GovernorProposalGuardian is Governor {
     event ProposalGuardianSet(address oldProposalGuardian, address newProposalGuardian);
 
     /**
-     * @dev Override {IGovernor-cancel} that implements the extended cancellation logic.
+     * @dev Override `_validateCancel` that implements the extended cancellation logic.
      *
      * * The {proposalGuardian} can cancel any proposal at any point in the lifecycle.
      * * if no proposal guardian is set, the {proposalProposer} can cancel their proposals at any point in the lifecycle.
-     * * if the proposal guardian is set, the {proposalProposer} keeps their default rights defined in {IGovernor-cancel} (calling `super`).
+     * * if the proposal guardian is set, the {proposalProposer} keeps their default rights defined in {IGovernor-_validateCancel} (calling `super`).
      */
-    function cancel(
-        address[] memory targets,
-        uint256[] memory values,
-        bytes[] memory calldatas,
-        bytes32 descriptionHash
-    ) public virtual override returns (uint256) {
-        address caller = _msgSender();
+    function _validateCancel(uint256 proposalId) internal view virtual override returns (bool) {
         address guardian = proposalGuardian();
+        address caller = _msgSender();
 
         if (guardian == address(0)) {
             // if there is no proposal guardian
             // ... only the proposer can cancel
             // ... no restriction on when the proposer can cancel
-            uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
             address proposer = proposalProposer(proposalId);
-            if (caller != proposer) revert GovernorOnlyProposer(caller);
-            return _cancel(targets, values, calldatas, descriptionHash);
+            if (caller == proposer) return true;
         } else if (guardian == caller) {
             // if there is a proposal guardian, and the caller is the proposal guardian
             // ... just cancel
-            return _cancel(targets, values, calldatas, descriptionHash);
-        } else {
-            // if there is a proposal guardian, and the caller is not the proposal guardian
-            // ... apply default behavior
-            return super.cancel(targets, values, calldatas, descriptionHash);
+            return true;
         }
+
+        // if there is no guardian and the caller isn't the proposer or
+        // there is a proposal guardian, and the caller is not the proposal guardian
+        // ... apply default behavior
+        return super._validateCancel(proposalId);
     }
 
     /**

contracts/mocks/governance/GovernorProposalGuardianMock.sol

@@ -18,12 +18,9 @@ abstract contract GovernorProposalGuardianMock is
         return super.proposalThreshold();
     }
 
-    function cancel(
-        address[] memory targets,
-        uint256[] memory values,
-        bytes[] memory calldatas,
-        bytes32 descriptionHash
-    ) public override(Governor, GovernorProposalGuardian) returns (uint256) {
-        return super.cancel(targets, values, calldatas, descriptionHash);
+    function _validateCancel(
+        uint256 proposalId
+    ) internal view override(Governor, GovernorProposalGuardian) returns (bool) {
+        return super._validateCancel(proposalId);
     }
 }

test/governance/Governor.test.js

@@ -624,21 +624,17 @@ describe('Governor', function () {
             await this.helper.connect(this.proposer).propose();
 
             await expect(this.helper.connect(this.owner).cancel('external'))
-              .to.be.revertedWithCustomError(this.mock, 'GovernorOnlyProposer')
-              .withArgs(this.owner);
+              .to.be.revertedWithCustomError(this.mock, 'GovernorUnableToCancel')
+              .withArgs(this.proposal.id, this.owner);
           });
 
           it('after vote started', async function () {
             await this.helper.propose();
             await this.helper.waitForSnapshot(1n); // snapshot + 1 block
 
             await expect(this.helper.cancel('external'))
-              .to.be.revertedWithCustomError(this.mock, 'GovernorUnexpectedProposalState')
-              .withArgs(
-                this.proposal.id,
-                ProposalState.Active,
-                GovernorHelper.proposalStatesToBitMap([ProposalState.Pending]),
-              );
+              .to.be.revertedWithCustomError(this.mock, 'GovernorUnableToCancel')
+              .withArgs(this.proposal.id, this.owner);
           });
 
           it('after vote', async function () {
@@ -647,12 +643,8 @@ describe('Governor', function () {
             await this.helper.connect(this.voter1).vote({ support: VoteType.For });
 
             await expect(this.helper.cancel('external'))
-              .to.be.revertedWithCustomError(this.mock, 'GovernorUnexpectedProposalState')
-              .withArgs(
-                this.proposal.id,
-                ProposalState.Active,
-                GovernorHelper.proposalStatesToBitMap([ProposalState.Pending]),
-              );
+              .to.be.revertedWithCustomError(this.mock, 'GovernorUnableToCancel')
+              .withArgs(this.proposal.id, this.voter1);
           });
 
           it('after deadline', async function () {
@@ -662,12 +654,8 @@ describe('Governor', function () {
             await this.helper.waitForDeadline();
 
             await expect(this.helper.cancel('external'))
-              .to.be.revertedWithCustomError(this.mock, 'GovernorUnexpectedProposalState')
-              .withArgs(
-                this.proposal.id,
-                ProposalState.Succeeded,
-                GovernorHelper.proposalStatesToBitMap([ProposalState.Pending]),
-              );
+              .to.be.revertedWithCustomError(this.mock, 'GovernorUnableToCancel')
+              .withArgs(this.proposal.id, this.voter1);
           });
 
           it('after execution', async function () {
@@ -678,12 +666,8 @@ describe('Governor', function () {
             await this.helper.execute();
 
             await expect(this.helper.cancel('external'))
-              .to.be.revertedWithCustomError(this.mock, 'GovernorUnexpectedProposalState')
-              .withArgs(
-                this.proposal.id,
-                ProposalState.Executed,
-                GovernorHelper.proposalStatesToBitMap([ProposalState.Pending]),
-              );
+              .to.be.revertedWithCustomError(this.mock, 'GovernorUnableToCancel')
+              .withArgs(this.proposal.id, this.voter1);
           });
         });
       });

test/governance/extensions/GovernorProposalGuardian.test.js

@@ -1,7 +1,7 @@
 const { ethers } = require('hardhat');
 const { expect } = require('chai');
-const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 const { impersonate } = require('../../helpers/account');
+const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 
 const { GovernorHelper } = require('../../helpers/governance');
 const { ProposalState } = require('../../helpers/enums');
@@ -10,7 +10,6 @@ const TOKENS = [
   { Token: '$ERC20Votes', mode: 'blocknumber' },
   { Token: '$ERC20VotesTimestampMock', mode: 'timestamp' },
 ];
-
 const name = 'Proposal Guardian Governor';
 const version = '1';
 const tokenName = 'MockToken';
@@ -105,12 +104,8 @@ describe('GovernorProposalGuardian', function () {
 
         it('from proposer when proposal guardian is non-zero', async function () {
           await expect(this.helper.connect(this.proposer).cancel())
-            .to.be.revertedWithCustomError(this.mock, 'GovernorUnexpectedProposalState')
-            .withArgs(
-              this.proposal.id,
-              ProposalState.Active,
-              GovernorHelper.proposalStatesToBitMap([ProposalState.Pending]),
-            );
+            .to.be.revertedWithCustomError(this.mock, 'GovernorUnableToCancel')
+            .withArgs(this.proposal.id, this.proposer);
         });
 
         it('from proposer when proposal guardian is zero', async function () {