chore: allow proposer to cancel while pending, active, succeeded, and…

… queued
OpenZeppelin · Oct 25, 2024 · 17a1c23 · 17a1c23
1 parent bcdfa84
commit 17a1c23
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 28 deletions.
diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol
@@ -471,9 +471,12 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         // changes it. The `hashProposal` duplication has a cost that is limited, and that we accept.
         uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
 
-        // public cancel restrictions (on top of existing _cancel restrictions).
-        _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Pending));
-        if (_msgSender() != proposalProposer(proposalId)) {
+        address proposer = proposalProposer(proposalId);
+        if (proposer == address(0)) {
+            revert GovernorNonexistentProposal(proposalId);
+        }
+
+        if (_msgSender() != proposer) {
             revert GovernorOnlyProposer(_msgSender());
         }
 

diff --git a/test/governance/Governor.test.js b/test/governance/Governor.test.js
@@ -527,7 +527,7 @@ describe('Governor', function () {
         });
       });
 
-      describe('cancel', function () {
+      describe.only('cancel', function () {
         describe('internal', function () {
           it('before proposal', async function () {
             await expect(this.helper.cancel('internal'))
@@ -607,7 +607,7 @@ describe('Governor', function () {
           });
         });
 
-        describe('public', function () {
+        describe.only('public', function () {
           it('before proposal', async function () {
             await expect(this.helper.cancel('external'))
               .to.be.revertedWithCustomError(this.mock, 'GovernorNonexistentProposal')
@@ -632,27 +632,17 @@ describe('Governor', 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]),
-              );
+            await this.helper.cancel('external');
+            expect(await this.mock.state(this.proposal.id)).to.equal(ProposalState.Canceled);
           });
 
           it('after vote', async function () {
             await this.helper.propose();
             await this.helper.waitForSnapshot();
             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]),
-              );
+            await this.helper.connect(this.owner).cancel('external');
+            expect(await this.mock.state(this.proposal.id)).to.equal(ProposalState.Canceled);
           });
 
           it('after deadline', async function () {
@@ -661,13 +651,8 @@ describe('Governor', function () {
             await this.helper.connect(this.voter1).vote({ support: VoteType.For });
             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]),
-              );
+            await this.helper.connect(this.owner).cancel('external');
+            expect(await this.mock.state(this.proposal.id)).to.equal(ProposalState.Canceled);
           });
 
           it('after execution', async function () {
@@ -677,12 +662,15 @@ describe('Governor', function () {
             await this.helper.waitForDeadline();
             await this.helper.execute();
 
-            await expect(this.helper.cancel('external'))
+            await expect(this.helper.connect(this.owner).cancel('external'))
               .to.be.revertedWithCustomError(this.mock, 'GovernorUnexpectedProposalState')
               .withArgs(
                 this.proposal.id,
                 ProposalState.Executed,
-                GovernorHelper.proposalStatesToBitMap([ProposalState.Pending]),
+                GovernorHelper.proposalStatesToBitMap(
+                  [ProposalState.Canceled, ProposalState.Expired, ProposalState.Executed],
+                  { inverted: true },
+                ),
               );
           });
         });