Merge branch 'master' into EIP712-getters

Author: RenanSouza2

.changeset/lovely-geckos-hide.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': major
+---
+
+Replace revert strings and require statements with custom errors.

GUIDELINES.md

@@ -115,3 +115,17 @@ In addition to the official Solidity Style Guide we have a number of other conve
   ```
 
 * 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:
+  
+  * The domain prefix should be picked in the following order:
+    1. Use `ERC<number>` if the error is a violation of an ERC specification.
+    2. Use the name of the underlying component where it belongs (eg. `Governor`, `ECDSA`, or `Timelock`).
+
+  * The location of custom errors should be decided in the following order:
+    1. Take the errors from their underlying ERCs if they're already defined.
+    2. Declare the errors in the underlying interface/library if the error makes sense in its context.
+    3. Declare the error in the implementation if the underlying interface/library is not suitable to do so (eg. interface/library already specified in an ERC).
+    4. Declare the error in an extension if the error only happens in such extension or child contracts.
+
+  * Custom error names should not be declared twice along the library to avoid duplicated identifier declarations when inheriting from multiple contracts.

README.md

@@ -70,7 +70,9 @@ Finally, you may want to take a look at the [guides on our blog](https://blog.op
 
 This project is maintained by [OpenZeppelin](https://openzeppelin.com) with the goal of providing a secure and reliable library of smart contract components for the ecosystem. We address security through risk management in various areas such as engineering and open source best practices, scoping and API design, multi-layered review processes, and incident response preparedness.
 
-The security policy is detailed in [`SECURITY.md`](./SECURITY.md), and specifies how you can report security vulnerabilities, which versions will receive security patches, and how to stay informed about them. We run a [bug bounty program on Immunefi](https://immunefi.com/bounty/openzeppelin) to reward the responsible disclosure of vulnerabilities.
+The [OpenZeppelin Contracts Security Center](https://contracts.openzeppelin.com/security) contains more details about the secure development process.
+
+The security policy is detailed in [`SECURITY.md`](./SECURITY.md) as well, and specifies how you can report security vulnerabilities, which versions will receive security patches, and how to stay informed about them. We run a [bug bounty program on Immunefi](https://immunefi.com/bounty/openzeppelin) to reward the responsible disclosure of vulnerabilities.
 
 The engineering guidelines we follow to promote project quality can be found in [`GUIDELINES.md`](./GUIDELINES.md).
 

contracts/access/AccessControl.sol

@@ -107,16 +107,7 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
      */
     function _checkRole(bytes32 role, address account) internal view virtual {
         if (!hasRole(role, account)) {
-            revert(
-                string(
-                    abi.encodePacked(
-                        "AccessControl: account ",
-                        Strings.toHexString(account),
-                        " is missing role ",
-                        Strings.toHexString(uint256(role), 32)
-                    )
-                )
-            );
+            revert AccessControlUnauthorizedAccount(account, role);
         }
     }
 
@@ -173,14 +164,16 @@ abstract contract AccessControl is Context, IAccessControl, ERC165 {
      *
      * Requirements:
      *
-     * - the caller must be `account`.
+     * - the caller must be `callerConfirmation`.
      *
      * May emit a {RoleRevoked} event.
      */
-    function renounceRole(bytes32 role, address account) public virtual {
-        require(account == _msgSender(), "AccessControl: can only renounce roles for self");
+    function renounceRole(bytes32 role, address callerConfirmation) public virtual {
+        if (callerConfirmation != _msgSender()) {
+            revert AccessControlBadConfirmation();
+        }
 
-        _revokeRole(role, account);
+        _revokeRole(role, callerConfirmation);
     }
 
     /**

contracts/access/AccessControlDefaultAdminRules.sol

@@ -53,7 +53,9 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
      * @dev Sets the initial values for {defaultAdminDelay} and {defaultAdmin} address.
      */
     constructor(uint48 initialDelay, address initialDefaultAdmin) {
-        require(initialDefaultAdmin != address(0), "AccessControl: 0 default admin");
+        if (initialDefaultAdmin == address(0)) {
+            revert AccessControlInvalidDefaultAdmin(address(0));
+        }
         _currentDelay = initialDelay;
         _grantRole(DEFAULT_ADMIN_ROLE, initialDefaultAdmin);
     }
@@ -80,15 +82,19 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
      * @dev See {AccessControl-grantRole}. Reverts for `DEFAULT_ADMIN_ROLE`.
      */
     function grantRole(bytes32 role, address account) public virtual override(AccessControl, IAccessControl) {
-        require(role != DEFAULT_ADMIN_ROLE, "AccessControl: can't directly grant default admin role");
+        if (role == DEFAULT_ADMIN_ROLE) {
+            revert AccessControlEnforcedDefaultAdminRules();
+        }
         super.grantRole(role, account);
     }
 
     /**
      * @dev See {AccessControl-revokeRole}. Reverts for `DEFAULT_ADMIN_ROLE`.
      */
     function revokeRole(bytes32 role, address account) public virtual override(AccessControl, IAccessControl) {
-        require(role != DEFAULT_ADMIN_ROLE, "AccessControl: can't directly revoke default admin role");
+        if (role == DEFAULT_ADMIN_ROLE) {
+            revert AccessControlEnforcedDefaultAdminRules();
+        }
         super.revokeRole(role, account);
     }
 
@@ -108,10 +114,9 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
     function renounceRole(bytes32 role, address account) public virtual override(AccessControl, IAccessControl) {
         if (role == DEFAULT_ADMIN_ROLE && account == defaultAdmin()) {
             (address newDefaultAdmin, uint48 schedule) = pendingDefaultAdmin();
-            require(
-                newDefaultAdmin == address(0) && _isScheduleSet(schedule) && _hasSchedulePassed(schedule),
-                "AccessControl: only can renounce in two delayed steps"
-            );
+            if (newDefaultAdmin != address(0) || !_isScheduleSet(schedule) || !_hasSchedulePassed(schedule)) {
+                revert AccessControlEnforcedDefaultAdminDelay(schedule);
+            }
             delete _pendingDefaultAdminSchedule;
         }
         super.renounceRole(role, account);
@@ -128,7 +133,9 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
      */
     function _grantRole(bytes32 role, address account) internal virtual override {
         if (role == DEFAULT_ADMIN_ROLE) {
-            require(defaultAdmin() == address(0), "AccessControl: default admin already granted");
+            if (defaultAdmin() != address(0)) {
+                revert AccessControlEnforcedDefaultAdminRules();
+            }
             _currentDefaultAdmin = account;
         }
         super._grantRole(role, account);
@@ -148,7 +155,9 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
      * @dev See {AccessControl-_setRoleAdmin}. Reverts for `DEFAULT_ADMIN_ROLE`.
      */
     function _setRoleAdmin(bytes32 role, bytes32 adminRole) internal virtual override {
-        require(role != DEFAULT_ADMIN_ROLE, "AccessControl: can't violate default admin rules");
+        if (role == DEFAULT_ADMIN_ROLE) {
+            revert AccessControlEnforcedDefaultAdminRules();
+        }
         super._setRoleAdmin(role, adminRole);
     }
 
@@ -236,7 +245,10 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
      */
     function acceptDefaultAdminTransfer() public virtual {
         (address newDefaultAdmin, ) = pendingDefaultAdmin();
-        require(_msgSender() == newDefaultAdmin, "AccessControl: pending admin must accept");
+        if (_msgSender() != newDefaultAdmin) {
+            // Enforce newDefaultAdmin explicit acceptance.
+            revert AccessControlInvalidDefaultAdmin(_msgSender());
+        }
         _acceptDefaultAdminTransfer();
     }
 
@@ -247,7 +259,9 @@ abstract contract AccessControlDefaultAdminRules is IAccessControlDefaultAdminRu
      */
     function _acceptDefaultAdminTransfer() internal virtual {
         (address newAdmin, uint48 schedule) = pendingDefaultAdmin();
-        require(_isScheduleSet(schedule) && _hasSchedulePassed(schedule), "AccessControl: transfer delay not passed");
+        if (!_isScheduleSet(schedule) || !_hasSchedulePassed(schedule)) {
+            revert AccessControlEnforcedDefaultAdminDelay(schedule);
+        }
         _revokeRole(DEFAULT_ADMIN_ROLE, defaultAdmin());
         _grantRole(DEFAULT_ADMIN_ROLE, newAdmin);
         delete _pendingDefaultAdmin;

contracts/access/IAccessControl.sol

@@ -7,6 +7,18 @@ pragma solidity ^0.8.19;
  * @dev External interface of AccessControl declared to support ERC165 detection.
  */
 interface IAccessControl {
+    /**
+     * @dev The `account` is missing a role.
+     */
+    error AccessControlUnauthorizedAccount(address account, bytes32 neededRole);
+
+    /**
+     * @dev The caller of a function is not the expected one.
+     *
+     * NOTE: Don't confuse with {AccessControlUnauthorizedAccount}.
+     */
+    error AccessControlBadConfirmation();
+
     /**
      * @dev Emitted when `newAdminRole` is set as ``role``'s admin role, replacing `previousAdminRole`
      *
@@ -82,7 +94,7 @@ interface IAccessControl {
      *
      * Requirements:
      *
-     * - the caller must be `account`.
+     * - the caller must be `callerConfirmation`.
      */
-    function renounceRole(bytes32 role, address account) external;
+    function renounceRole(bytes32 role, address callerConfirmation) external;
 }

contracts/access/IAccessControlDefaultAdminRules.sol

@@ -11,6 +11,28 @@ import "./IAccessControl.sol";
  * _Available since v4.9._
  */
 interface IAccessControlDefaultAdminRules is IAccessControl {
+    /**
+     * @dev The new default admin is not a valid default admin.
+     */
+    error AccessControlInvalidDefaultAdmin(address defaultAdmin);
+
+    /**
+     * @dev At least one of the following rules was violated:
+     *
+     * - The `DEFAULT_ADMIN_ROLE` must only be managed by itself.
+     * - The `DEFAULT_ADMIN_ROLE` must only be held by one account at the time.
+     * - Any `DEFAULT_ADMIN_ROLE` transfer must be in two delayed steps.
+     */
+    error AccessControlEnforcedDefaultAdminRules();
+
+    /**
+     * @dev The delay for transferring the default admin delay is enforced and
+     * the operation must wait until `schedule`.
+     *
+     * NOTE: `schedule` can be 0 indicating there's no transfer scheduled.
+     */
+    error AccessControlEnforcedDefaultAdminDelay(uint48 schedule);
+
     /**
      * @dev Emitted when a {defaultAdmin} transfer is started, setting `newAdmin` as the next
      * address to become the {defaultAdmin} by calling {acceptDefaultAdminTransfer} only after `acceptSchedule`

contracts/access/Ownable.sol

@@ -20,6 +20,16 @@ import "../utils/Context.sol";
 abstract contract Ownable is Context {
     address private _owner;
 
+    /**
+     * @dev The caller account is not authorized to perform an operation.
+     */
+    error OwnableUnauthorizedAccount(address account);
+
+    /**
+     * @dev The owner is not a valid owner account. (eg. `address(0)`)
+     */
+    error OwnableInvalidOwner(address owner);
+
     event OwnershipTransferred(address indexed previousOwner, address indexed newOwner);
 
     /**
@@ -48,7 +58,9 @@ abstract contract Ownable is Context {
      * @dev Throws if the sender is not the owner.
      */
     function _checkOwner() internal view virtual {
-        require(owner() == _msgSender(), "Ownable: caller is not the owner");
+        if (owner() != _msgSender()) {
+            revert OwnableUnauthorizedAccount(_msgSender());
+        }
     }
 
     /**
@@ -67,7 +79,9 @@ abstract contract Ownable is Context {
      * Can only be called by the current owner.
      */
     function transferOwnership(address newOwner) public virtual onlyOwner {
-        require(newOwner != address(0), "Ownable: new owner is the zero address");
+        if (newOwner == address(0)) {
+            revert OwnableInvalidOwner(address(0));
+        }
         _transferOwnership(newOwner);
     }
 

contracts/access/Ownable2Step.sol

@@ -51,7 +51,9 @@ abstract contract Ownable2Step is Ownable {
      */
     function acceptOwnership() public virtual {
         address sender = _msgSender();
-        require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner");
+        if (pendingOwner() != sender) {
+            revert OwnableUnauthorizedAccount(sender);
+        }
         _transferOwnership(sender);
     }
 }

contracts/finance/VestingWallet.sol

@@ -23,6 +23,11 @@ contract VestingWallet is Context {
     event EtherReleased(uint256 amount);
     event ERC20Released(address indexed token, uint256 amount);
 
+    /**
+     * @dev The `beneficiary` is not a valid account.
+     */
+    error VestingWalletInvalidBeneficiary(address beneficiary);
+
     uint256 private _released;
     mapping(address => uint256) private _erc20Released;
     address private immutable _beneficiary;
@@ -33,7 +38,9 @@ contract VestingWallet is Context {
      * @dev Set the beneficiary, start timestamp and vesting duration of the vesting wallet.
      */
     constructor(address beneficiaryAddress, uint64 startTimestamp, uint64 durationSeconds) payable {
-        require(beneficiaryAddress != address(0), "VestingWallet: beneficiary is zero address");
+        if (beneficiaryAddress == address(0)) {
+            revert VestingWalletInvalidBeneficiary(address(0));
+        }
         _beneficiary = beneficiaryAddress;
         _start = startTimestamp;
         _duration = durationSeconds;