From 8f822f1ce3bdf5269d369eeafe52f0f2f5ce26c5 Mon Sep 17 00:00:00 2001 From: Arr00 <13561405+arr00@users.noreply.github.com> Date: Fri, 25 Oct 2024 15:53:33 -0400 Subject: [PATCH] chore: allow proposer to cancel while pending, active, succeeded, and queued --- contracts/governance/Governor.sol | 9 +++++--- test/governance/Governor.test.js | 34 ++++++++++--------------------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index 390411556b0..94396b7d274 100644 --- 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 index 3e48ccfee7f..87c5d4f4955 100644 --- a/test/governance/Governor.test.js +++ b/test/governance/Governor.test.js @@ -632,13 +632,8 @@ 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 () { @@ -646,13 +641,8 @@ describe('Governor', function () { 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 }, + ), ); }); });