Merge branch 'master' into lib-618-accesscontrol-admin-rules

.changeset/small-cars-appear.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`ECDSA`: Add a function `toDataWithIntendedValidatorHash` that encodes data with version 0x00 following EIP-191.

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,37 +77,63 @@ 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(
             _msgSender() == proposer || getVotes(proposer, clock() - 1) < proposalThreshold(),
             "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(

contracts/utils/cryptography/ECDSA.sol

@@ -204,4 +204,14 @@ library ECDSA {
             data := keccak256(ptr, 0x42)
         }
     }
+
+    /**
+     * @dev Returns an Ethereum Signed Data with intended validator, created from a
+     * `validator` and `data` according to the version 0 of EIP-191.
+     *
+     * See {recover}.
+     */
+    function toDataWithIntendedValidatorHash(address validator, bytes memory data) internal pure returns (bytes32) {
+        return keccak256(abi.encodePacked("\x19\x00", validator, data));
+    }
 }

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

test/helpers/sign.js

@@ -4,6 +4,21 @@ function toEthSignedMessageHash(messageHex) {
   return web3.utils.sha3(Buffer.concat([prefix, messageBuffer]));
 }
 
+/**
+ * Create a signed data with intended validator according to the version 0 of EIP-191
+ * @param validatorAddress The address of the validator
+ * @param dataHex The data to be concatenated with the prefix and signed
+ */
+function toDataWithIntendedValidatorHash(validatorAddress, dataHex) {
+  const validatorBuffer = Buffer.from(web3.utils.hexToBytes(validatorAddress));
+  const dataBuffer = Buffer.from(web3.utils.hexToBytes(dataHex));
+  const preambleBuffer = Buffer.from('\x19');
+  const versionBuffer = Buffer.from('\x00');
+  const ethMessage = Buffer.concat([preambleBuffer, versionBuffer, validatorBuffer, dataBuffer]);
+
+  return web3.utils.sha3(ethMessage);
+}
+
 /**
  * Create a signer between a contract and a signer for a voucher of method, args, and redeemer
  * Note that `method` is the web3 method, not the truffle-contract method
@@ -43,5 +58,6 @@ const getSignFor =
 
 module.exports = {
   toEthSignedMessageHash,
+  toDataWithIntendedValidatorHash,
   getSignFor,
 };

test/token/ERC20/extensions/ERC4626.t.sol

@@ -12,7 +12,7 @@ contract ERC4626StdTest is ERC4626Test {
         _underlying_ = address(new ERC20Mock());
         _vault_ = address(new ERC4626Mock(_underlying_));
         _delta_ = 0;
-        _vaultMayBeEmpty = false;
+        _vaultMayBeEmpty = true;
         _unlimitedAmount = true;
     }
 }

test/utils/cryptography/ECDSA.test.js

@@ -1,5 +1,5 @@
 const { expectRevert } = require('@openzeppelin/test-helpers');
-const { toEthSignedMessageHash } = require('../../helpers/sign');
+const { toEthSignedMessageHash, toDataWithIntendedValidatorHash } = require('../../helpers/sign');
 
 const { expect } = require('chai');
 
@@ -8,6 +8,7 @@ const ECDSA = artifacts.require('$ECDSA');
 const TEST_MESSAGE = web3.utils.sha3('OpenZeppelin');
 const WRONG_MESSAGE = web3.utils.sha3('Nope');
 const NON_HASH_MESSAGE = '0x' + Buffer.from('abcd').toString('hex');
+const RANDOM_ADDRESS = web3.utils.toChecksumAddress(web3.utils.randomHex(20));
 
 function to2098Format(signature) {
   const long = web3.utils.hexToBytes(signature);
@@ -248,4 +249,12 @@ contract('ECDSA', function (accounts) {
       );
     });
   });
+
+  context('toDataWithIntendedValidatorHash', function () {
+    it('returns the hash correctly', async function () {
+      expect(
+        await this.ecdsa.methods['$toDataWithIntendedValidatorHash(address,bytes)'](RANDOM_ADDRESS, NON_HASH_MESSAGE),
+      ).to.equal(toDataWithIntendedValidatorHash(RANDOM_ADDRESS, NON_HASH_MESSAGE));
+    });
+  });
 });