Merge branch 'master' into feature/Governor-storage

.changeset/afraid-walls-smell.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`ERC1155Receiver`: Removed in favor of `ERC1155Holder`.

.changeset/eight-peaches-guess.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`Proxy`: Removed redundant `receive` function.

.changeset/empty-taxis-kiss.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`UUPSUpgradeable`, `TransparentUpgradeableProxy` and `ProxyAdmin`: Removed `upgradeTo` and `upgrade` functions, and made `upgradeToAndCall` and `upgradeAndCall` ignore the data argument if it is empty. It is no longer possible to invoke the receive function (or send value with empty data) along with an upgrade.

.changeset/fluffy-countries-buy.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`Arrays`: Optimize `findUpperBound` by removing redundant SLOAD.

.changeset/hot-dingos-kiss.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`MessageHashUtils`: Add a new library for creating message digest to be used along with signing or recovery such as  ECDSA or ERC-1271. These functions are moved from the `ECDSA` library.

.changeset/proud-seals-complain.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`BeaconProxy`: Use an immutable variable to store the address of the beacon. It is no longer possible for a `BeaconProxy` to upgrade by changing to another beacon.

.changeset/purple-cats-cheer.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`GovernorTimelockControl`: Add the Governor instance address as part of the TimelockController operation `salt` to avoid operation id collisions between governors using the same TimelockController.

.changeset/spicy-sheep-eat.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`access`: Move `AccessControl` extensions to a dedicated directory.

.changeset/tender-shirts-turn.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`BeaconProxy`: Reject value in initialization unless a payable function is explicitly invoked.

.changeset/wild-rockets-rush.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+`Math`: Renamed members of `Rounding` enum, and added a new rounding mode for "away from zero".

.github/workflows/checks.yml

@@ -100,7 +100,6 @@ jobs:
       - uses: crytic/[email protected]
         with:
           node-version: 18.15
-          slither-version: 0.9.3
 
   codespell:
     runs-on: ubuntu-latest

README.md

@@ -41,6 +41,8 @@ OpenZeppelin Contracts features a [stable API](https://docs.openzeppelin.com/con
 $ forge install OpenZeppelin/openzeppelin-contracts
 ```
 
+Add `@openzeppelin/contracts/=lib/openzeppelin-contracts/contracts/` in `remappings.txt.` 
+
 ### Usage
 
 Once installed, you can use the contracts in the library by importing them:

contracts/access/AccessControl.sol

@@ -5,7 +5,6 @@ pragma solidity ^0.8.19;
 
 import {IAccessControl} from "./IAccessControl.sol";
 import {Context} from "../utils/Context.sol";
-import {Strings} from "../utils/Strings.sol";
 import {ERC165} from "../utils/introspection/ERC165.sol";
 
 /**

contracts/access/README.adoc

@@ -8,7 +8,7 @@ This directory provides ways to restrict who can access the functions of a contr
 - {AccessControl} provides a general role based access control mechanism. Multiple hierarchical roles can be created and assigned each to multiple accounts.
 - {Ownable} is a simpler mechanism with a single owner "role" that can be assigned to a single account. This simpler mechanism can be useful for quick tests but projects with production concerns are likely to outgrow it.
 
-== Authorization
+== Core
 
 {{Ownable}}
 
@@ -18,8 +18,12 @@ This directory provides ways to restrict who can access the functions of a contr
 
 {{AccessControl}}
 
+== Extensions
+
 {{IAccessControlEnumerable}}
 
 {{AccessControlEnumerable}}
 
+{{IAccessControlDefaultAdminRules}}
+
 {{AccessControlDefaultAdminRules}}

contracts/access/AccessControlDefaultAdminRules.sol → contracts/access/extensions/AccessControlDefaultAdminRules.sol

@@ -3,11 +3,11 @@
 
 pragma solidity ^0.8.19;
 
-import {AccessControl, IAccessControl} from "./AccessControl.sol";
 import {IAccessControlDefaultAdminRules} from "./IAccessControlDefaultAdminRules.sol";
-import {SafeCast} from "../utils/math/SafeCast.sol";
-import {Math} from "../utils/math/Math.sol";
-import {IERC5313} from "../interfaces/IERC5313.sol";
+import {AccessControl, IAccessControl} from "../AccessControl.sol";
+import {SafeCast} from "../../utils/math/SafeCast.sol";
+import {Math} from "../../utils/math/Math.sol";
+import {IERC5313} from "../../interfaces/IERC5313.sol";
 
 /**
  * @dev Extension of {AccessControl} that allows specifying special rules to manage

contracts/access/AccessControlEnumerable.sol → contracts/access/extensions/AccessControlEnumerable.sol

@@ -4,8 +4,8 @@
 pragma solidity ^0.8.19;
 
 import {IAccessControlEnumerable} from "./IAccessControlEnumerable.sol";
-import {AccessControl} from "./AccessControl.sol";
-import {EnumerableSet} from "../utils/structs/EnumerableSet.sol";
+import {AccessControl} from "../AccessControl.sol";
+import {EnumerableSet} from "../../utils/structs/EnumerableSet.sol";
 
 /**
  * @dev Extension of {AccessControl} that allows enumerating the members of each role.

contracts/access/IAccessControlDefaultAdminRules.sol → contracts/access/extensions/IAccessControlDefaultAdminRules.sol

@@ -3,7 +3,7 @@
 
 pragma solidity ^0.8.19;
 
-import {IAccessControl} from "./IAccessControl.sol";
+import {IAccessControl} from "../IAccessControl.sol";
 
 /**
  * @dev External interface of AccessControlDefaultAdminRules declared to support ERC165 detection.

contracts/access/IAccessControlEnumerable.sol → contracts/access/extensions/IAccessControlEnumerable.sol

@@ -3,7 +3,7 @@
 
 pragma solidity ^0.8.19;
 
-import {IAccessControl} from "./IAccessControl.sol";
+import {IAccessControl} from "../IAccessControl.sol";
 
 /**
  * @dev External interface of AccessControlEnumerable declared to support ERC165 detection.

contracts/governance/TimelockController.sol

@@ -6,7 +6,6 @@ pragma solidity ^0.8.19;
 import {AccessControl} from "../access/AccessControl.sol";
 import {ERC721Holder} from "../token/ERC721/utils/ERC721Holder.sol";
 import {ERC1155Holder} from "../token/ERC1155/utils/ERC1155Holder.sol";
-import {ERC1155Receiver} from "../token/ERC1155/utils/ERC1155Receiver.sol";
 import {Address} from "../utils/Address.sol";
 
 /**
@@ -103,7 +102,7 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
     /**
      * @dev Initializes the contract with the following parameters:
      *
-     * - `minDelay`: initial minimum delay for operations
+     * - `minDelay`: initial minimum delay in seconds for operations
      * - `proposers`: accounts to be granted proposer and canceller roles
      * - `executors`: accounts to be granted executor role
      * - `admin`: optional account to be granted admin role; disable with zero address
@@ -160,7 +159,7 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
      */
     function supportsInterface(
         bytes4 interfaceId
-    ) public view virtual override(AccessControl, ERC1155Receiver) returns (bool) {
+    ) public view virtual override(AccessControl, ERC1155Holder) returns (bool) {
         return super.supportsInterface(interfaceId);
     }
 
@@ -219,7 +218,7 @@ contract TimelockController is AccessControl, ERC721Holder, ERC1155Holder {
     }
 
     /**
-     * @dev Returns the minimum delay for an operation to become valid.
+     * @dev Returns the minimum delay in seconds for an operation to become valid.
      *
      * This value can be changed by executing an operation that calls `updateDelay`.
      */

contracts/governance/extensions/GovernorTimelockCompound.sol

@@ -4,7 +4,6 @@
 pragma solidity ^0.8.19;
 
 import {IGovernor, Governor} from "../Governor.sol";
-import {SafeCast} from "../../utils/math/SafeCast.sol";
 import {ICompoundTimelock} from "../../vendor/compound/ICompoundTimelock.sol";
 import {IERC165} from "../../interfaces/IERC165.sol";
 import {Address} from "../../utils/Address.sol";

contracts/governance/extensions/GovernorTimelockControl.sol

@@ -79,8 +79,9 @@ abstract contract GovernorTimelockControl is Governor {
     ) internal virtual override returns (bool, uint48) {
         uint256 delay = _timelock.getMinDelay();
 
-        _timelockIds[proposalId] = _timelock.hashOperationBatch(targets, values, calldatas, 0, descriptionHash);
-        _timelock.scheduleBatch(targets, values, calldatas, 0, descriptionHash, delay);
+        bytes32 salt = _timelockSalt(descriptionHash);
+        _timelockIds[proposalId] = _timelock.hashOperationBatch(targets, values, calldatas, 0, salt);
+        _timelock.scheduleBatch(targets, values, calldatas, 0, salt, delay);
 
         return (true, SafeCast.toUint48(block.timestamp + delay));
     }
@@ -97,7 +98,7 @@ abstract contract GovernorTimelockControl is Governor {
         bytes32 descriptionHash
     ) internal virtual override {
         // execute
-        _timelock.executeBatch{value: msg.value}(targets, values, calldatas, 0, descriptionHash);
+        _timelock.executeBatch{value: msg.value}(targets, values, calldatas, 0, _timelockSalt(descriptionHash));
         // cleanup for refund
         delete _timelockIds[proposalId];
     }
@@ -148,4 +149,14 @@ abstract contract GovernorTimelockControl is Governor {
         emit TimelockChange(address(_timelock), address(newTimelock));
         _timelock = newTimelock;
     }
+
+    /**
+     * @dev Computes the {TimelockController} operation salt.
+     *
+     * It is computed with the governor address itself to avoid collisions across governor instances using the
+     * same timelock.
+     */
+    function _timelockSalt(bytes32 descriptionHash) private view returns (bytes32) {
+        return bytes20(address(this)) ^ descriptionHash;
+    }
 }

contracts/metatx/ERC2771Forwarder.sol

@@ -53,7 +53,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
         bytes signature;
     }
 
-    bytes32 private constant _FORWARD_REQUEST_TYPEHASH =
+    bytes32 internal constant _FORWARD_REQUEST_TYPEHASH =
         keccak256(
             "ForwardRequest(address from,address to,uint256 value,uint256 gas,uint256 nonce,uint48 deadline,bytes data)"
         );
@@ -255,7 +255,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
                 abi.encodePacked(request.data, request.from)
             );
 
-            _checkForwardedGas(request);
+            _checkForwardedGas(gasleft(), request);
 
             emit ExecutedForwardRequest(signer, currentNonce, success);
         }
@@ -270,10 +270,10 @@ contract ERC2771Forwarder is EIP712, Nonces {
      *
      * It reverts consuming all the available gas if the forwarded gas is not the requested gas.
      *
-     * IMPORTANT: This function should be called exactly the end of the forwarded call. Any gas consumed
-     * in between will make room for bypassing this check.
+     * IMPORTANT: The `gasLeft` parameter should be measured exactly at the end of the forwarded call.
+     * Any gas consumed in between will make room for bypassing this check.
      */
-    function _checkForwardedGas(ForwardRequestData calldata request) private view {
+    function _checkForwardedGas(uint256 gasLeft, ForwardRequestData calldata request) private pure {
         // To avoid insufficient gas griefing attacks, as referenced in https://ronan.eth.limo/blog/ethereum-gas-dangers/
         //
         // A malicious relayer can attempt to shrink the gas forwarded so that the underlying call reverts out-of-gas
@@ -295,7 +295,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
         // -    req.gas >= X * 63 / 64
         // In other words if req.gas < X * 63 / 64 then req.gas / 63 <= gasleft(), thus if the relayer behaves honestly
         // the forwarding does not revert.
-        if (gasleft() < request.gas / 63) {
+        if (gasLeft < request.gas / 63) {
             // We explicitly trigger invalid opcode to consume all gas and bubble-up the effects, since
             // neither revert or assert consume all gas since Solidity 0.8.0
             // https://docs.soliditylang.org/en/v0.8.0/control-structures.html#panic-via-assert-and-error-via-require

contracts/mocks/UpgreadeableBeaconMock.sol

@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.19;
+
+import {IBeacon} from "../proxy/beacon/IBeacon.sol";
+
+contract UpgradeableBeaconMock is IBeacon {
+    address public implementation;
+
+    constructor(address impl) {
+        implementation = impl;
+    }
+}

contracts/mocks/docs/ERC4626Fees.sol

@@ -81,10 +81,10 @@ abstract contract ERC4626Fees is ERC4626 {
     }
 
     function _feeOnRaw(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) {
-        return assets.mulDiv(feeBasePoint, 1e5, Math.Rounding.Up);
+        return assets.mulDiv(feeBasePoint, 1e5, Math.Rounding.Ceil);
     }
 
     function _feeOnTotal(uint256 assets, uint256 feeBasePoint) private pure returns (uint256) {
-        return assets.mulDiv(feeBasePoint, feeBasePoint + 1e5, Math.Rounding.Up);
+        return assets.mulDiv(feeBasePoint, feeBasePoint + 1e5, Math.Rounding.Ceil);
     }
 }

contracts/mocks/proxy/ClashingImplementation.sol

@@ -9,7 +9,7 @@ pragma solidity ^0.8.19;
 contract ClashingImplementation {
     event ClashingImplementationCall();
 
-    function upgradeTo(address) external payable {
+    function upgradeToAndCall(address, bytes calldata) external payable {
         emit ClashingImplementationCall();
     }
 

contracts/mocks/proxy/UUPSUpgradeableMock.sol

@@ -23,12 +23,8 @@ contract UUPSUpgradeableMock is NonUpgradeableMock, UUPSUpgradeable {
 }
 
 contract UUPSUpgradeableUnsafeMock is UUPSUpgradeableMock {
-    function upgradeTo(address newImplementation) public override {
-        ERC1967Utils.upgradeToAndCall(newImplementation, bytes(""), false);
-    }
-
     function upgradeToAndCall(address newImplementation, bytes memory data) public payable override {
-        ERC1967Utils.upgradeToAndCall(newImplementation, data, false);
+        ERC1967Utils.upgradeToAndCall(newImplementation, data);
     }
 }
 

contracts/mocks/token/ERC20PermitNoRevertMock.sol

@@ -2,7 +2,6 @@
 
 pragma solidity ^0.8.19;
 
-import {ERC20} from "../../token/ERC20/ERC20.sol";
 import {ERC20Permit} from "../../token/ERC20/extensions/ERC20Permit.sol";
 
 abstract contract ERC20PermitNoRevertMock is ERC20Permit {

contracts/proxy/ERC1967/ERC1967Proxy.sol

@@ -18,9 +18,13 @@ contract ERC1967Proxy is Proxy {
      *
      * If `_data` is nonempty, it's used as data in a delegate call to `_logic`. This will typically be an encoded
      * function call, and allows initializing the storage of the proxy like a Solidity constructor.
+     *
+     * Requirements:
+     *
+     * - If `data` is empty, `msg.value` must be zero.
      */
     constructor(address _logic, bytes memory _data) payable {
-        ERC1967Utils.upgradeToAndCall(_logic, _data, false);
+        ERC1967Utils.upgradeToAndCall(_logic, _data);
     }
 
     /**