From 77a459e98734a3effe8fb9834033f56a8b460c4a Mon Sep 17 00:00:00 2001 From: Hadrien Croubois Date: Thu, 20 Feb 2025 09:23:15 +0100 Subject: [PATCH] Release v5.3 cherrypick #1 (#5509) Signed-off-by: Hadrien Croubois Co-authored-by: Francisco Giordano Co-authored-by: Joseph Delong Co-authored-by: Arr00 <13561405+arr00@users.noreply.github.com> Co-authored-by: Renan Souza --- .changeset/gorgeous-apes-jam.md | 5 ++++ contracts/access/manager/AuthorityUtils.sol | 18 ++++++------ contracts/governance/TimelockController.sol | 2 +- hardhat.config.js | 1 - test/helpers/precompiles.js | 12 ++++++++ .../cryptography/SignatureChecker.test.js | 28 ++++++++++++++----- test/utils/math/Math.t.sol | 3 ++ 7 files changed, 51 insertions(+), 18 deletions(-) create mode 100644 .changeset/gorgeous-apes-jam.md create mode 100644 test/helpers/precompiles.js diff --git a/.changeset/gorgeous-apes-jam.md b/.changeset/gorgeous-apes-jam.md new file mode 100644 index 000000000..14ca3522e --- /dev/null +++ b/.changeset/gorgeous-apes-jam.md @@ -0,0 +1,5 @@ +--- +'openzeppelin-solidity': minor +--- + +`TimelockController`: Receive function is now virtual. diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol index fb3018ca8..61cf0407c 100644 --- a/contracts/access/manager/AuthorityUtils.sol +++ b/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); } } diff --git a/contracts/governance/TimelockController.sol b/contracts/governance/TimelockController.sol index 349d940fd..d2ba17016 100644 --- a/contracts/governance/TimelockController.sol +++ b/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}. diff --git a/hardhat.config.js b/hardhat.config.js index d39d3d073..b4b8d630f 100644 --- a/hardhat.config.js +++ b/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', diff --git a/test/helpers/precompiles.js b/test/helpers/precompiles.js new file mode 100644 index 000000000..fb6b7132a --- /dev/null +++ b/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', +}; diff --git a/test/utils/cryptography/SignatureChecker.test.js b/test/utils/cryptography/SignatureChecker.test.js index e6a08491a..2b14f2f4c 100644 --- a/test/utils/cryptography/SignatureChecker.test.js +++ b/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,15 +27,18 @@ 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; }); }); @@ -41,19 +46,28 @@ describe('SignatureChecker (ERC1271)', 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; }); }); } diff --git a/test/utils/math/Math.t.sol b/test/utils/math/Math.t.sol index 6c122d66c..c41ec9e3b 100644 --- a/test/utils/math/Math.t.sol +++ b/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);