From a9ea794a79e1852b7b5d847b44b694cd5a99270e Mon Sep 17 00:00:00 2001 From: Daniel Gretzke Date: Fri, 4 Oct 2024 00:22:49 +0200 Subject: [PATCH] Update ERC-7751: Update custom error and add security considerations Merged by EIP-Bot. --- ERCS/erc-7751.md | 51 +++++++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/ERCS/erc-7751.md b/ERCS/erc-7751.md index ac91a16651..d12ad7670e 100644 --- a/ERCS/erc-7751.md +++ b/ERCS/erc-7751.md @@ -12,7 +12,7 @@ created: 2024-08-06 ## Abstract -This ERC proposes a standard for handling bubbled up reverts in Ethereum smart contracts using custom errors. This standard aims to improve the clarity and usability of revert reasons by allowing additional context to be passed alongside the raw bytes of the bubbled up revert. The custom errors should follow the naming structure `Wrap__` followed by a descriptive name and allow an arbitrary number of parameters, with the first being the address of the called contract and the second being the raw bytes of the bubbled up revert. +This ERC proposes a standard for handling bubbled up reverts in Ethereum smart contracts using a dedicated custom error. This standard aims to improve the clarity and usability of revert reasons by allowing additional context to be passed alongside the raw bytes of the bubbled up revert. The `WrappedError` custom error should wrap reverts from called contracts and provide a consistent interface for parsing and handling reverts in tools like Etherscan or Tenderly. ## Motivation @@ -22,20 +22,22 @@ Currently, when a smart contract calls another and the called contract reverts, The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and "OPTIONAL" in this document are to be interpreted as described in RFC 2119 and RFC 8174. -1. **Custom Error Naming Convention:** - - Custom errors that bubble up reverts MUST be prefixed with `Wrap__`. - - Example: `Wrap__ERC20TransferFailed`. -2. **Parameters:** - - The first parameter MUST be the address of the called contract that reverted. - - The second parameter MUST be the raw bytes of the bubbled up revert. - - Additional parameters MAY be included to provide context. - - Example: `Wrap__ERC20TransferFailed(address token, bytes reason, address recipient)`. +In order to wrap a revert, a contract MUST revert with the following error that corresponds to the following signature `0x90bfb865`: -If no additional parameters are defined and a generic call reverts, the custom error `Wrap__SubcontextReverted(address, bytes)` SHOULD be thrown to ensure a consistent signature. +```solidity +error WrappedError(address target, bytes4 selector, bytes reason, bytes details); +``` + +Where: + +- `target` is the address of the called contract that reverted. +- `selector` is the selector of the called function that reverted. If the call was an ETH transfer without any data, the selector MUST be `bytes4(0)` +- `reason` is the raw bytes of the revert reason. +- `details` is optional additional context about the revert. In cases where no additional context is needed, the `details` bytes can be empty. In cases with additional context, the `details` bytes MUST be an ABI encoded custom error declared on the contract that emits the `WrappedError` error. ## Rationale -By including the called contract, raw revert bytes and additional context, developers can provide more detailed information about the failure. Additionally, by standardizing the way reverts are bubbled up, it also enables nested bubbled up reverts where multiple reverts thrown by different contracts can be followed recursively. The reverts can also be parsed and handled by tools like Etherscan and Foundry to further enhance the readability and debuggability of smart contract interactions, as well as facilitating better error handling practices in general. +By including the called contract and function, raw revert bytes and additional context, developers can provide more detailed information about the failure. Additionally, by standardizing the way reverts are bubbled up, it also enables nested bubbled up reverts where multiple reverts thrown by different contracts can be followed recursively. The reverts can also be parsed and handled by tools like Etherscan and Foundry to further enhance the readability and debuggability of smart contract interactions, as well as facilitating better error handling practices in general. ## Backwards Compatibility @@ -64,7 +66,9 @@ contract Token { contract Vault { Token token; - error Wrap__ERC20TransferFailed(address token, bytes reason, address recipient); + error WrappedError(address target, bytes4 selector, bytes reason, bytes details); + error ERC20TransferFailed(address recipient); + constructor(Token token_) { token = token_; @@ -73,7 +77,7 @@ contract Vault { function withdraw(address to, uint256 amount) external { // logic try token.transfer(to, amount) {} catch (bytes memory error) { - revert Wrap__ERC20TransferFailed(address(token), error, to); + revert WrappedError(address(token), token.transfer.selector, error, abi.encodeWithSelector(ERC20TransferFailed.selector, to)); } } } @@ -81,7 +85,7 @@ contract Vault { contract Router { Vault vault; - error Wrap__SubcontextReverted(address target, bytes reason); + error WrappedError(address target, bytes4 selector, bytes reason, bytes details); constructor(Vault vault_) { vault = vault_; @@ -90,7 +94,7 @@ contract Router { function withdraw(uint256 amount) external { // logic try vault.withdraw(msg.sender, amount) {} catch (bytes memory error) { - revert Wrap__SubcontextReverted(address(vault), error); + revert WrappedError(address(vault), vault.withdraw.selector, error, ""); } } } @@ -103,12 +107,13 @@ contract Test { try router.withdraw(amount) {} catch (bytes memory thrownError) { bytes memory expectedError = abi.encodeWithSelector( - Router.Wrap__SubcontextReverted.selector, address(vault), abi.encodeWithSelector( - Vault.Wrap__ERC20TransferFailed.selector, + Router.WrappedError.selector, address(vault), vault.withdraw.selector, abi.encodeWithSelector( + Vault.WrappedError.selector, address(token), + token.transfer.selector, abi.encodeWithSignature("Error(string)", "insufficient balance"), - address(this) - ) + abi.encodeWithSelector(Vault.ERC20TransferFailed.selector, address(this)) + ), "" ); assert(keccak256(thrownError) == keccak256(expectedError)); } @@ -122,13 +127,15 @@ When catching a revert from a called contract, the calling contract should rever ```solidity contract Foo { - error Wrap__SubcontextReverted(address target, bytes reason); + + error WrappedError(address target, bytes4 selector, bytes reason, bytes details); + error MyCustomError(uint256 x); function foo(address to, bytes memory data) external { // logic (bool success, bytes memory returnData) = to.call(data); if (!success) { - revert Wrap__SubcontextReverted(to, returnData); + revert WrappedError(to, bytes4(data), returnData, abi.encodeWithSelector(MyCustomError.selector, 42)); } } } @@ -136,7 +143,7 @@ contract Foo { ## Security Considerations -This EIP does not introduce new security risks. +Smart contracts could either drop or purposefully suppress the bubbled up reverts along the revert chain. Additionally, smart contracts may also lie or incorrectly report the wrapped reverts, so the information is not guaranteed to be accurate. ## Copyright