diff --git a/contracts/governance/Governor.sol b/contracts/governance/Governor.sol index e3691a3ed3a..1483da6d2b3 100644 --- a/contracts/governance/Governor.sol +++ b/contracts/governance/Governor.sol @@ -336,10 +336,16 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive /** * @dev See {IGovernor-cancel}. */ - function cancel(uint256 proposalId) public virtual override { + function cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) public virtual override returns (uint256) { + uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash); require(state(proposalId) == ProposalState.Pending, "Governor: too late to cancel"); require(_msgSender() == _proposals[proposalId].proposer, "Governor: only proposer can cancel"); - _cancel(proposalId); + return _cancel(targets, values, calldatas, descriptionHash); } /** @@ -407,16 +413,8 @@ abstract contract Governor is Context, ERC165, EIP712, IGovernor, IERC721Receive bytes[] memory calldatas, bytes32 descriptionHash ) internal virtual returns (uint256) { - return _cancel(hashProposal(targets, values, calldatas, descriptionHash)); - } + uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash); - /** - * @dev Internal cancel mechanism: locks up the proposal timer, preventing it from being re-submitted. Marks it as - * canceled to allow distinguishing it from executed proposals. - * - * Emits a {IGovernor-ProposalCanceled} event. - */ - function _cancel(uint256 proposalId) internal virtual returns (uint256) { ProposalState status = state(proposalId); require( diff --git a/contracts/governance/IGovernor.sol b/contracts/governance/IGovernor.sol index b12d2f63227..70f81efe815 100644 --- a/contracts/governance/IGovernor.sol +++ b/contracts/governance/IGovernor.sol @@ -235,12 +235,17 @@ abstract contract IGovernor is IERC165, IERC6372 { ) public payable virtual returns (uint256 proposalId); /** - * @dev Cancel a proposal. This is restricted to Pending proposal (before the vote starts) and is restricted to - * the proposal's proposer. + * @dev Cancel a proposal. A proposal is cancellable by the proposer, but only while it is Pending state, i.e. + * before the vote starts. * * Emits a {ProposalCanceled} event. */ - function cancel(uint256 proposalId) public virtual; + function cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) public virtual returns (uint256 proposalId); /** * @dev Cast a vote diff --git a/contracts/governance/compatibility/GovernorCompatibilityBravo.sol b/contracts/governance/compatibility/GovernorCompatibilityBravo.sol index 1b8c0936817..39c895bf092 100644 --- a/contracts/governance/compatibility/GovernorCompatibilityBravo.sol +++ b/contracts/governance/compatibility/GovernorCompatibilityBravo.sol @@ -77,29 +77,55 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp * @dev See {IGovernorCompatibilityBravo-queue}. */ function queue(uint256 proposalId) public virtual override { - ProposalDetails storage details = _proposalDetails[proposalId]; - queue( - details.targets, - details.values, - _encodeCalldata(details.signatures, details.calldatas), - details.descriptionHash - ); + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) = _getProposalParameters(proposalId); + + queue(targets, values, calldatas, descriptionHash); } /** * @dev See {IGovernorCompatibilityBravo-execute}. */ function execute(uint256 proposalId) public payable virtual override { - ProposalDetails storage details = _proposalDetails[proposalId]; - execute( - details.targets, - details.values, - _encodeCalldata(details.signatures, details.calldatas), - details.descriptionHash - ); + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) = _getProposalParameters(proposalId); + + execute(targets, values, calldatas, descriptionHash); } - function cancel(uint256 proposalId) public virtual override(IGovernor, Governor) { + /** + * @dev Cancel a proposal with GovernorBravo logic. + */ + function cancel(uint256 proposalId) public virtual { + ( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) = _getProposalParameters(proposalId); + + cancel(targets, values, calldatas, descriptionHash); + } + + /** + * @dev Cancel a proposal with GovernorBravo logic. At any moment a proposal can be cancelled, either by the + * proposer, or by third parties if the proposer's voting power has dropped below the proposal threshold. + */ + function cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) public virtual override(IGovernor, Governor) returns (uint256) { + uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash); address proposer = _proposalDetails[proposalId].proposer; require( @@ -107,7 +133,7 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp "GovernorBravo: proposer above threshold" ); - _cancel(proposalId); + return _cancel(targets, values, calldatas, descriptionHash); } /** @@ -128,6 +154,25 @@ abstract contract GovernorCompatibilityBravo is IGovernorTimelock, IGovernorComp return fullcalldatas; } + /** + * @dev Retrieve proposal parameters by id, with fully encoded calldatas. + */ + function _getProposalParameters( + uint256 proposalId + ) + private + view + returns (address[] memory targets, uint256[] memory values, bytes[] memory calldatas, bytes32 descriptionHash) + { + ProposalDetails storage details = _proposalDetails[proposalId]; + return ( + details.targets, + details.values, + _encodeCalldata(details.signatures, details.calldatas), + details.descriptionHash + ); + } + /** * @dev Store proposal metadata for later lookup */ diff --git a/contracts/mocks/governance/GovernorCompatibilityBravoMock.sol b/contracts/mocks/governance/GovernorCompatibilityBravoMock.sol index 2727794f62c..1b87d143390 100644 --- a/contracts/mocks/governance/GovernorCompatibilityBravoMock.sol +++ b/contracts/mocks/governance/GovernorCompatibilityBravoMock.sol @@ -66,8 +66,13 @@ abstract contract GovernorCompatibilityBravoMock is return super.execute(targets, values, calldatas, salt); } - function cancel(uint256 proposalId) public override(Governor, GovernorCompatibilityBravo, IGovernor) { - super.cancel(proposalId); + function cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) public override(Governor, GovernorCompatibilityBravo, IGovernor) returns (uint256) { + return super.cancel(targets, values, calldatas, descriptionHash); } function _execute( diff --git a/contracts/mocks/wizard/MyGovernor3.sol b/contracts/mocks/wizard/MyGovernor3.sol index 4192cae9442..f4d29515627 100644 --- a/contracts/mocks/wizard/MyGovernor3.sol +++ b/contracts/mocks/wizard/MyGovernor3.sol @@ -54,8 +54,13 @@ contract MyGovernor is return super.propose(targets, values, calldatas, description); } - function cancel(uint256 proposalId) public override(Governor, GovernorCompatibilityBravo, IGovernor) { - super.cancel(proposalId); + function cancel( + address[] memory targets, + uint256[] memory values, + bytes[] memory calldatas, + bytes32 descriptionHash + ) public override(Governor, GovernorCompatibilityBravo, IGovernor) returns (uint256) { + return super.cancel(targets, values, calldatas, descriptionHash); } function _execute( diff --git a/test/helpers/governance.js b/test/helpers/governance.js index 1ffa086cbd8..4b38b7588e4 100644 --- a/test/helpers/governance.js +++ b/test/helpers/governance.js @@ -68,7 +68,13 @@ class GovernorHelper { switch (visibility) { case 'external': - return this.governor.methods['cancel(uint256)'](...concatOpts([proposal.id], opts)); + if (proposal.useCompatibilityInterface) { + return this.governor.methods['cancel(uint256)'](...concatOpts([proposal.id], opts)); + } else { + return this.governor.methods['cancel(address[],uint256[],bytes[],bytes32)']( + ...concatOpts(proposal.shortProposal, opts), + ); + } case 'internal': return this.governor.methods['$_cancel(address[],uint256[],bytes[],bytes32)']( ...concatOpts(proposal.shortProposal, opts),