Merge branch 'master' into feature/improve-erc6372-behavior-test

.changeset/four-chairs-help.md

@@ -0,0 +1,5 @@
+---
+"openzeppelin-solidity": minor
+---
+
+`Clones`: Add `cloneWithImmutableArgs` and `cloneDeterministicWithImmutableArgs` variants that create clones with per-instance immutable arguments. The immutable arguments can be retrieved using `fetchCloneArgs`. The corresponding `predictDeterministicWithImmutableArgs` function is also included.

GUIDELINES.md

@@ -131,6 +131,13 @@ In addition to the official Solidity Style Guide we have a number of other conve
   abstract contract AccessControl is ..., {
   ```
 
+* Return values are generally not named, unless they are not immediately clear or there are multiple return values.
+
+  ```solidity
+  function expiration() public view returns (uint256) { // Good
+  function hasRole() public view returns (bool isMember, uint32 currentDelay) { // Good
+  ```
+
 * Unchecked arithmetic blocks should contain comments explaining why overflow is guaranteed not to happen. If the reason is immediately apparent from the line above the unchecked block, the comment may be omitted.
 
 * Custom errors should be declared following the [EIP-6093](https://eips.ethereum.org/EIPS/eip-6093) rationale whenever reasonable. Also, consider the following:

contracts/access/manager/AccessManager.sol

@@ -97,7 +97,15 @@ contract AccessManager is Context, Multicall, IAccessManager {
         uint32 nonce;
     }
 
+    /**
+     * @dev The identifier of the admin role. Required to perform most configuration operations including
+     * other roles' management and target restrictions.
+     */
     uint64 public constant ADMIN_ROLE = type(uint64).min; // 0
+
+    /**
+     * @dev The identifier of the public role. Automatically granted to all addresses with no delay.
+     */
     uint64 public constant PUBLIC_ROLE = type(uint64).max; // 2**64-1
 
     mapping(address target => TargetConfig mode) private _targets;
@@ -687,7 +695,7 @@ contract AccessManager is Context, Multicall, IAccessManager {
 
         (bool adminRestricted, uint64 roleId, uint32 operationDelay) = _getAdminRestrictions(data);
 
-        // isTragetClosed apply to non-admin-restricted function
+        // isTargetClosed apply to non-admin-restricted function
         if (!adminRestricted && isTargetClosed(address(this))) {
             return (false, 0);
         }

contracts/access/manager/IAccessManager.sol

@@ -3,7 +3,6 @@
 
 pragma solidity ^0.8.20;
 
-import {IAccessManaged} from "./IAccessManaged.sol";
 import {Time} from "../../utils/types/Time.sol";
 
 interface IAccessManager {

contracts/governance/Governor.sol

@@ -769,7 +769,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
 
         // Extract what would be the `#proposer=0x` marker beginning the suffix
         bytes12 marker;
-        assembly {
+        assembly ("memory-safe") {
             // - Start of the string contents in memory = description + 32
             // - First character of the marker = len - 52
             //   - Length of "#proposer=0x0000000000000000000000000000000000000000" = 52

contracts/governance/IGovernor.sol

@@ -8,6 +8,9 @@ import {IERC6372} from "../interfaces/IERC6372.sol";
 
 /**
  * @dev Interface of the {Governor} core.
+ *
+ * NOTE: Event parameters lack the `indexed` keyword for compatibility with GovernorBravo events.
+ * Making event parameters `indexed` affects how events are decoded, potentially breaking existing indexers.
  */
 interface IGovernor is IERC165, IERC6372 {
     enum ProposalState {

contracts/interfaces/IERC1363Receiver.sol

@@ -18,7 +18,7 @@ interface IERC1363Receiver {
      * (i.e. 0x88a7ca5c, or its own function selector).
      *
      * @param operator The address which called `transferAndCall` or `transferFromAndCall` function.
-     * @param from The address which are tokens transferred from.
+     * @param from The address which the tokens are transferred from.
      * @param value The amount of tokens transferred.
      * @param data Additional data with no specified format.
      * @return `bytes4(keccak256("onTransferReceived(address,address,uint256,bytes)"))` if transfer is allowed unless throwing.

contracts/interfaces/IERC2981.sol

@@ -15,6 +15,9 @@ interface IERC2981 is IERC165 {
     /**
      * @dev Returns how much royalty is owed and to whom, based on a sale price that may be denominated in any unit of
      * exchange. The royalty amount is denominated and should be paid in that same unit of exchange.
+     *
+     * NOTE: ERC-2981 allows setting the royalty to 100% of the price. In that case all the price would be sent to the
+     * royalty receiver and 0 tokens to the seller. Contracts dealing with royalty should consider empty transfers.
      */
     function royaltyInfo(
         uint256 tokenId,

contracts/metatx/ERC2771Forwarder.sol

@@ -286,7 +286,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
 
             uint256 gasLeft;
 
-            assembly {
+            assembly ("memory-safe") {
                 success := call(reqGas, to, value, add(data, 0x20), mload(data), 0, 0)
                 gasLeft := gas()
             }
@@ -309,8 +309,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
         bool success;
         uint256 returnSize;
         uint256 returnValue;
-        /// @solidity memory-safe-assembly
-        assembly {
+        assembly ("memory-safe") {
             // Perform the staticcall and save the result in the scratch space.
             // | Location  | Content  | Content (Hex)                                                      |
             // |-----------|----------|--------------------------------------------------------------------|
@@ -362,8 +361,7 @@ contract ERC2771Forwarder is EIP712, Nonces {
             // 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.20
             // https://docs.soliditylang.org/en/v0.8.20/control-structures.html#panic-via-assert-and-error-via-require
-            /// @solidity memory-safe-assembly
-            assembly {
+            assembly ("memory-safe") {
                 invalid()
             }
         }

contracts/proxy/Clones.sol

@@ -3,6 +3,7 @@
 
 pragma solidity ^0.8.20;
 
+import {Create2} from "../utils/Create2.sol";
 import {Errors} from "../utils/Errors.sol";
 
 /**
@@ -17,6 +18,8 @@ import {Errors} from "../utils/Errors.sol";
  * deterministic method.
  */
 library Clones {
+    error CloneArgumentsTooLong();
+
     /**
      * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation`.
      *
@@ -37,8 +40,7 @@ library Clones {
         if (address(this).balance < value) {
             revert Errors.InsufficientBalance(address(this).balance, value);
         }
-        /// @solidity memory-safe-assembly
-        assembly {
+        assembly ("memory-safe") {
             // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes
             // of the `implementation` address with the bytecode before the address.
             mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
@@ -77,8 +79,7 @@ library Clones {
         if (address(this).balance < value) {
             revert Errors.InsufficientBalance(address(this).balance, value);
         }
-        /// @solidity memory-safe-assembly
-        assembly {
+        assembly ("memory-safe") {
             // Cleans the upper 96 bits of the `implementation` word, then packs the first 3 bytes
             // of the `implementation` address with the bytecode before the address.
             mstore(0x00, or(shr(0xe8, shl(0x60, implementation)), 0x3d602d80600a3d3981f3363d3d373d3d3d363d73000000))
@@ -99,8 +100,7 @@ library Clones {
         bytes32 salt,
         address deployer
     ) internal pure returns (address predicted) {
-        /// @solidity memory-safe-assembly
-        assembly {
+        assembly ("memory-safe") {
             let ptr := mload(0x40)
             mstore(add(ptr, 0x38), deployer)
             mstore(add(ptr, 0x24), 0x5af43d82803e903d91602b57fd5bf3ff)
@@ -121,4 +121,142 @@ library Clones {
     ) internal view returns (address predicted) {
         return predictDeterministicAddress(implementation, salt, address(this));
     }
+
+    /**
+     * @dev Deploys and returns the address of a clone that mimics the behavior of `implementation` with custom
+     * immutable arguments. These are provided through `args` and cannot be changed after deployment. To
+     * access the arguments within the implementation, use {fetchCloneArgs}.
+     *
+     * This function uses the create opcode, which should never revert.
+     */
+    function cloneWithImmutableArgs(address implementation, bytes memory args) internal returns (address instance) {
+        return cloneWithImmutableArgs(implementation, args, 0);
+    }
+
+    /**
+     * @dev Same as {xref-Clones-cloneWithImmutableArgs-address-bytes-}[cloneWithImmutableArgs], but with a `value`
+     * parameter to send native currency to the new contract.
+     *
+     * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory)
+     * to always have enough balance for new deployments. Consider exposing this function under a payable method.
+     */
+    function cloneWithImmutableArgs(
+        address implementation,
+        bytes memory args,
+        uint256 value
+    ) internal returns (address instance) {
+        if (address(this).balance < value) {
+            revert Errors.InsufficientBalance(address(this).balance, value);
+        }
+        bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args);
+        assembly ("memory-safe") {
+            instance := create(value, add(bytecode, 0x20), mload(bytecode))
+        }
+        if (instance == address(0)) {
+            revert Errors.FailedDeployment();
+        }
+    }
+
+    /**
+     * @dev Deploys and returns the address of a clone that mimics the behaviour of `implementation` with custom
+     * immutable arguments. These are provided through `args` and cannot be changed after deployment. To
+     * access the arguments within the implementation, use {fetchCloneArgs}.
+     *
+     * This function uses the create2 opcode and a `salt` to deterministically deploy the clone. Using the same
+     * `implementation` and `salt` multiple time will revert, since the clones cannot be deployed twice at the same
+     * address.
+     */
+    function cloneDeterministicWithImmutableArgs(
+        address implementation,
+        bytes memory args,
+        bytes32 salt
+    ) internal returns (address instance) {
+        return cloneDeterministicWithImmutableArgs(implementation, args, salt, 0);
+    }
+
+    /**
+     * @dev Same as {xref-Clones-cloneDeterministicWithImmutableArgs-address-bytes-bytes32-}[cloneDeterministicWithImmutableArgs],
+     * but with a `value` parameter to send native currency to the new contract.
+     *
+     * NOTE: Using a non-zero value at creation will require the contract using this function (e.g. a factory)
+     * to always have enough balance for new deployments. Consider exposing this function under a payable method.
+     */
+    function cloneDeterministicWithImmutableArgs(
+        address implementation,
+        bytes memory args,
+        bytes32 salt,
+        uint256 value
+    ) internal returns (address instance) {
+        bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args);
+        return Create2.deploy(value, salt, bytecode);
+    }
+
+    /**
+     * @dev Computes the address of a clone deployed using {Clones-cloneDeterministicWithImmutableArgs}.
+     */
+    function predictDeterministicAddressWithImmutableArgs(
+        address implementation,
+        bytes memory args,
+        bytes32 salt,
+        address deployer
+    ) internal pure returns (address predicted) {
+        bytes memory bytecode = _cloneCodeWithImmutableArgs(implementation, args);
+        return Create2.computeAddress(salt, keccak256(bytecode), deployer);
+    }
+
+    /**
+     * @dev Computes the address of a clone deployed using {Clones-cloneDeterministicWithImmutableArgs}.
+     */
+    function predictDeterministicAddressWithImmutableArgs(
+        address implementation,
+        bytes memory args,
+        bytes32 salt
+    ) internal view returns (address predicted) {
+        return predictDeterministicAddressWithImmutableArgs(implementation, args, salt, address(this));
+    }
+
+    /**
+     * @dev Get the immutable args attached to a clone.
+     *
+     * - If `instance` is a clone that was deployed using `clone` or `cloneDeterministic`, this
+     *   function will return an empty array.
+     * - If `instance` is a clone that was deployed using `cloneWithImmutableArgs` or
+     *   `cloneDeterministicWithImmutableArgs`, this function will return the args array used at
+     *   creation.
+     * - If `instance` is NOT a clone deployed using this library, the behavior is undefined. This
+     *   function should only be used to check addresses that are known to be clones.
+     */
+    function fetchCloneArgs(address instance) internal view returns (bytes memory) {
+        bytes memory result = new bytes(instance.code.length - 0x2d); // revert if length is too short
+        assembly ("memory-safe") {
+            extcodecopy(instance, add(result, 0x20), 0x2d, mload(result))
+        }
+        return result;
+    }
+
+    /**
+     * @dev Helper that prepares the initcode of the proxy with immutable args.
+     *
+     * An assembly variant of this function requires copying the `args` array, which can be efficiently done using
+     * `mcopy`. Unfortunately, that opcode is not available before cancun. A pure solidity implementation using
+     * abi.encodePacked is more expensive but also more portable and easier to review.
+     *
+     * NOTE: https://eips.ethereum.org/EIPS/eip-170[EIP-170] limits the length of the contract code to 24576 bytes.
+     * With the proxy code taking 45 bytes, that limits the length of the immutable args to 24531 bytes.
+     */
+    function _cloneCodeWithImmutableArgs(
+        address implementation,
+        bytes memory args
+    ) private pure returns (bytes memory) {
+        if (args.length > 0x5fd3) revert CloneArgumentsTooLong();
+        return
+            abi.encodePacked(
+                hex"61",
+                uint16(args.length + 0x2d),
+                hex"3d81600a3d39f3363d3d373d3d3d363d73",
+                implementation,
+                hex"5af43d82803e903d91602b57fd5bf3",
+                args
+            );
+    }
 }

contracts/proxy/ERC1967/ERC1967Utils.sol

@@ -9,7 +9,7 @@ import {Address} from "../../utils/Address.sol";
 import {StorageSlot} from "../../utils/StorageSlot.sol";
 
 /**
- * @dev This abstract contract provides getters and event emitting update functions for
+ * @dev This library provides getters and event emitting update functions for
  * https://eips.ethereum.org/EIPS/eip-1967[ERC-1967] slots.
  */
 library ERC1967Utils {

contracts/proxy/transparent/TransparentUpgradeableProxy.sol

@@ -15,7 +15,13 @@ import {ProxyAdmin} from "./ProxyAdmin.sol";
  * include them in the ABI so this interface must be used to interact with it.
  */
 interface ITransparentUpgradeableProxy is IERC1967 {
-    function upgradeToAndCall(address, bytes calldata) external payable;
+    /**
+     * @dev Upgrade the implementation of the proxy to `newImplementation`, and subsequently execute the function call
+     * encoded in `data`.
+     *
+     * See {UUPSUpgradeable-upgradeToAndCall}
+     */
+    function upgradeToAndCall(address newImplementation, bytes calldata data) external payable;
 }
 
 /**

contracts/token/ERC1155/ERC1155.sol

@@ -381,8 +381,7 @@ abstract contract ERC1155 is Context, ERC165, IERC1155, IERC1155MetadataURI, IER
         uint256 element1,
         uint256 element2
     ) private pure returns (uint256[] memory array1, uint256[] memory array2) {
-        /// @solidity memory-safe-assembly
-        assembly {
+        assembly ("memory-safe") {
             // Load the free memory pointer
             array1 := mload(0x40)
             // Set array length to 1

contracts/token/ERC1155/utils/ERC1155Utils.sol

@@ -15,7 +15,7 @@ library ERC1155Utils {
      * @dev Performs an acceptance check for the provided `operator` by calling {IERC1155-onERC1155Received}
      * on the `to` address. The `operator` is generally the address that initiated the token transfer (i.e. `msg.sender`).
      *
-     * The acceptance call is not executed and treated as a no-op if the target address is doesn't contain code (i.e. an EOA).
+     * The acceptance call is not executed and treated as a no-op if the target address doesn't contain code (i.e. an EOA).
      * Otherwise, the recipient must implement {IERC1155Receiver-onERC1155Received} and return the acceptance magic value to accept
      * the transfer.
      */
@@ -38,8 +38,7 @@ library ERC1155Utils {
                     // non-IERC1155Receiver implementer
                     revert IERC1155Errors.ERC1155InvalidReceiver(to);
                 } else {
-                    /// @solidity memory-safe-assembly
-                    assembly {
+                    assembly ("memory-safe") {
                         revert(add(32, reason), mload(reason))
                     }
                 }
@@ -51,7 +50,7 @@ library ERC1155Utils {
      * @dev Performs a batch acceptance check for the provided `operator` by calling {IERC1155-onERC1155BatchReceived}
      * on the `to` address. The `operator` is generally the address that initiated the token transfer (i.e. `msg.sender`).
      *
-     * The acceptance call is not executed and treated as a no-op if the target address is doesn't contain code (i.e. an EOA).
+     * The acceptance call is not executed and treated as a no-op if the target address doesn't contain code (i.e. an EOA).
      * Otherwise, the recipient must implement {IERC1155Receiver-onERC1155Received} and return the acceptance magic value to accept
      * the transfer.
      */
@@ -76,8 +75,7 @@ library ERC1155Utils {
                     // non-IERC1155Receiver implementer
                     revert IERC1155Errors.ERC1155InvalidReceiver(to);
                 } else {
-                    /// @solidity memory-safe-assembly
-                    assembly {
+                    assembly ("memory-safe") {
                         revert(add(32, reason), mload(reason))
                     }
                 }

contracts/token/ERC20/extensions/draft-ERC20TemporaryApproval.sol

@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: MIT
 
-pragma solidity ^0.8.20;
+pragma solidity ^0.8.24;
 
 import {IERC20, ERC20} from "../ERC20.sol";
 import {IERC7674} from "../../../interfaces/draft-IERC7674.sol";