Release v5.3 cherrypick #1 (#5509)

Signed-off-by: Hadrien Croubois <[email protected]>
Co-authored-by: Francisco Giordano <[email protected]>
Co-authored-by: Joseph Delong <[email protected]>
Co-authored-by: Arr00 <[email protected]>
Co-authored-by: Renan Souza <[email protected]>

Author: 5 people

.changeset/gorgeous-apes-jam.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`TimelockController`: Receive function is now virtual.

contracts/access/manager/AuthorityUtils.sol

@@ -17,16 +17,16 @@ library AuthorityUtils {
         address target,
         bytes4 selector
     ) internal view returns (bool immediate, uint32 delay) {
-        (bool success, bytes memory data) = authority.staticcall(
-            abi.encodeCall(IAuthority.canCall, (caller, target, selector))
-        );
-        if (success) {
-            if (data.length >= 0x40) {
-                (immediate, delay) = abi.decode(data, (bool, uint32));
-            } else if (data.length >= 0x20) {
-                immediate = abi.decode(data, (bool));
+        bytes memory data = abi.encodeCall(IAuthority.canCall, (caller, target, selector));
+
+        assembly ("memory-safe") {
+            mstore(0x00, 0x00)
+            mstore(0x20, 0x00)
+
+            if staticcall(gas(), authority, add(data, 0x20), mload(data), 0x00, 0x40) {
+                immediate := mload(0x00)
+                delay := mload(0x20)
             }
         }
-        return (immediate, delay);
     }
 }

contracts/governance/TimelockController.sol

@@ -152,7 +152,7 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
     /**
      * @dev Contract might receive/hold ETH as part of the maintenance process.
      */
-    receive() external payable {}
+    receive() external payable virtual {}
 
     /**
      * @dev See {IERC165-supportsInterface}.

hardhat.config.js

@@ -90,7 +90,6 @@ module.exports = {
       'initcode-size': 'off',
     },
     '*': {
-      'code-size': true,
       'unused-param': !argv.coverage, // coverage causes unused-param warnings
       'transient-storage': false,
       default: 'error',

test/helpers/precompiles.js

@@ -0,0 +1,12 @@
+module.exports = {
+  ecRecover: '0x0000000000000000000000000000000000000001',
+  SHA2_256: '0x0000000000000000000000000000000000000002',
+  RIPEMD_160: '0x0000000000000000000000000000000000000003',
+  identity: '0x0000000000000000000000000000000000000004',
+  modexp: '0x0000000000000000000000000000000000000005',
+  ecAdd: '0x0000000000000000000000000000000000000006',
+  ecMul: '0x0000000000000000000000000000000000000007',
+  ecPairing: '0x0000000000000000000000000000000000000008',
+  blake2f: '0x0000000000000000000000000000000000000009',
+  pointEvaluation: '0x000000000000000000000000000000000000000a',
+};

test/utils/cryptography/SignatureChecker.test.js

@@ -2,6 +2,8 @@ const { ethers } = require('hardhat');
 const { expect } = require('chai');
 const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers');
 
+const precompile = require('../../helpers/precompiles');
+
 const TEST_MESSAGE = ethers.id('OpenZeppelin');
 const TEST_MESSAGE_HASH = ethers.hashMessage(TEST_MESSAGE);
 
@@ -25,35 +27,47 @@ describe('SignatureChecker (ERC1271)', function () {
 
   describe('EOA account', function () {
     it('with matching signer and signature', async function () {
-      expect(await this.mock.$isValidSignatureNow(this.signer, TEST_MESSAGE_HASH, this.signature)).to.be.true;
+      await expect(this.mock.$isValidSignatureNow(this.signer, TEST_MESSAGE_HASH, this.signature)).to.eventually.be
+        .true;
     });
 
     it('with invalid signer', async function () {
-      expect(await this.mock.$isValidSignatureNow(this.other, TEST_MESSAGE_HASH, this.signature)).to.be.false;
+      await expect(this.mock.$isValidSignatureNow(this.other, TEST_MESSAGE_HASH, this.signature)).to.eventually.be
+        .false;
     });
 
     it('with invalid signature', async function () {
-      expect(await this.mock.$isValidSignatureNow(this.signer, WRONG_MESSAGE_HASH, this.signature)).to.be.false;
+      await expect(this.mock.$isValidSignatureNow(this.signer, WRONG_MESSAGE_HASH, this.signature)).to.eventually.be
+        .false;
     });
   });
 
   describe('ERC1271 wallet', function () {
     for (const fn of ['isValidERC1271SignatureNow', 'isValidSignatureNow']) {
       describe(fn, function () {
         it('with matching signer and signature', async function () {
-          expect(await this.mock.getFunction(`$${fn}`)(this.wallet, TEST_MESSAGE_HASH, this.signature)).to.be.true;
+          await expect(this.mock.getFunction(`$${fn}`)(this.wallet, TEST_MESSAGE_HASH, this.signature)).to.eventually.be
+            .true;
         });
 
         it('with invalid signer', async function () {
-          expect(await this.mock.getFunction(`$${fn}`)(this.mock, TEST_MESSAGE_HASH, this.signature)).to.be.false;
+          await expect(this.mock.getFunction(`$${fn}`)(this.mock, TEST_MESSAGE_HASH, this.signature)).to.eventually.be
+            .false;
+        });
+
+        it('with identity precompile', async function () {
+          await expect(this.mock.getFunction(`$${fn}`)(precompile.identity, TEST_MESSAGE_HASH, this.signature)).to
+            .eventually.be.false;
         });
 
         it('with invalid signature', async function () {
-          expect(await this.mock.getFunction(`$${fn}`)(this.wallet, WRONG_MESSAGE_HASH, this.signature)).to.be.false;
+          await expect(this.mock.getFunction(`$${fn}`)(this.wallet, WRONG_MESSAGE_HASH, this.signature)).to.eventually
+            .be.false;
         });
 
         it('with malicious wallet', async function () {
-          expect(await this.mock.getFunction(`$${fn}`)(this.malicious, TEST_MESSAGE_HASH, this.signature)).to.be.false;
+          await expect(this.mock.getFunction(`$${fn}`)(this.malicious, TEST_MESSAGE_HASH, this.signature)).to.eventually
+            .be.false;
         });
       });
     }

test/utils/math/Math.t.sol

@@ -204,6 +204,7 @@ contract MathTest is Test {
         assertEq(xyLo, qdRemLo);
     }
 
+    /// forge-config: default.allow_internal_expect_revert = true
     function testMulDivDomain(uint256 x, uint256 y, uint256 d) public {
         (uint256 xyHi, ) = _mulHighLow(x, y);
 
@@ -216,6 +217,7 @@ contract MathTest is Test {
     }
 
     // MOD EXP
+    /// forge-config: default.allow_internal_expect_revert = true
     function testModExp(uint256 b, uint256 e, uint256 m) public {
         if (m == 0) {
             vm.expectRevert(stdError.divisionError);
@@ -236,6 +238,7 @@ contract MathTest is Test {
         }
     }
 
+    /// forge-config: default.allow_internal_expect_revert = true
     function testModExpMemory(uint256 b, uint256 e, uint256 m) public {
         if (m == 0) {
             vm.expectRevert(stdError.divisionError);