Change Governor.cancel to receive all parameters (#4056)

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(

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

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
      */

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(

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(

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),