use _validateCancel instead of overriding cancel

contracts/governance/Governor.sol

@@ -484,11 +484,8 @@ 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);
 
-        // public cancel restrictions (on top of existing _cancel restrictions).
-        _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Pending));
-        if (_msgSender() != proposalProposer(proposalId)) {
-            revert GovernorOnlyProposer(_msgSender());
-        }
+        address caller = _msgSender();
+        if (!_validateCancel(proposalId, caller)) revert GovernorUnableToCancel(proposalId, caller);
 
         return _cancel(targets, values, calldatas, descriptionHash);
     }
@@ -805,6 +802,15 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         }
     }
 
+    /**
+     * @dev Check if the `caller` can cancel the proposal with the given `proposalId`.
+     *
+     * The default implementation allows the proposal proposer to cancel the proposal during the pending state.
+     */
+    function _validateCancel(uint256 proposalId, address caller) internal view virtual returns (bool) {
+        return (state(proposalId) == ProposalState.Pending) && caller == proposalProposer(proposalId);
+    }
+
     /**
      * @inheritdoc IERC6372
      */

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

@@ -13,41 +13,6 @@ abstract contract GovernorProposalGuardian is Governor {
 
     event ProposalGuardianSet(address oldProposalGuardian, address newProposalGuardian);
 
-    /**
-     * @dev Override {IGovernor-cancel} 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`).
-     */
-    function cancel(
-        address[] memory targets,
-        uint256[] memory values,
-        bytes[] memory calldatas,
-        bytes32 descriptionHash
-    ) public virtual override returns (uint256) {
-        address caller = _msgSender();
-        address guardian = proposalGuardian();
-
-        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);
-        } 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);
-        }
-    }
-
     /**
      * @dev Getter that returns the address of the proposal guardian.
      */
@@ -73,4 +38,30 @@ abstract contract GovernorProposalGuardian is Governor {
         emit ProposalGuardianSet(_proposalGuardian, newProposalGuardian);
         _proposalGuardian = newProposalGuardian;
     }
+
+    /**
+     * @dev Override `_validateCancel` that implements the extended cancellation logic.
+     *
+     * * The {proposalGuardian} can cancel any proposal at any point.
+     * * If no proposal guardian is set, the {IGovernor-proposalProposer} can cancel their proposals at any point.
+     * * All other conditions are forwarded to super for default cancellation validation (as defined in {Governor-_validateCancel}).
+     */
+    function _validateCancel(uint256 proposalId, address caller) internal view virtual override returns (bool) {
+        address guardian = proposalGuardian();
+
+        if (guardian == address(0)) {
+            // if there is no proposal guardian
+            // ... no restriction on when the proposer can cancel
+            if (caller == proposalProposer(proposalId)) return true;
+        } else if (guardian == caller) {
+            // if there is a proposal guardian, and the caller is the proposal guardian
+            // ... just cancel
+            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, caller);
+    }
 }

contracts/mocks/governance/GovernorProposalGuardianMock.sol

@@ -18,12 +18,10 @@ 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,
+        address caller
+    ) internal view override(Governor, GovernorProposalGuardian) returns (bool) {
+        return super._validateCancel(proposalId, caller);
     }
 }

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,16 +1,15 @@
 const { ethers } = require('hardhat');
 const { expect } = require('chai');
 const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
-const { impersonate } = require('../../helpers/account');
 
+const { impersonate } = require('../../helpers/account');
 const { GovernorHelper } = require('../../helpers/governance');
 const { ProposalState } = require('../../helpers/enums');
 
 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 () {