Discussion for Add ERC 7751: Wrapping of bubbled up reverts by gretzke · Pull Request #578 · ethereum/ERCs · GitHub
3 Likes
very cool & sufficient gas trade offs for improved UX! i feel it needs be emphasised that following this at usual entry point contracts like SCA, routers etc is important to not lose the stack trace.
I’d like to propose a profound change to the ERC.
Motivation
Currently the ERC relies on thwo things to detect and precess wrapped custom errors:
- Being able to detect the custom error (human readable) name from its “selector” (and argument format)
- Being able to identify the relevant custom errors by looking at a prefix in the custom error name
Saying that the first step is resolved by the presence of the ABI is big oversimplification. In practice it doesn’t work that well.
-
If the contract is not verified, you don’t have an ABI to check. In particular, some networks may not have an obvious verification workflow.
-
If the contract is a proxy, you need to know which ABI to look for. Simple ERC-1167 clones are usually well suported, but other proxy pattern are not that easy to work with. In particular, when dealing with a diamond proxy (ERC-1538 / ERC-2535), it is not obvious how to know which facet was used and get the correct ABI from the facet’s verified source code
-
Last bu not least, when If a contract just “bubbles” the custom error without using this wrapper syntax, the ABI used to decode the wrapper will be hidden in the trace.
- User calls contract A
- Contract A calls Contract B
- Contract B calls Contract C
- Contract C revert with some custom error
- Contract B catches the error, and wrap it in a Wrapping custom error
- Contract A gets the wrapped error from B, and bubble’s it without any wrapping
Here, You have A that throws a wrapped custom error that you cannot decode unless you know that it comes from B.
The big strenght of require(boolean, string) is that it emits a standardized format Error(string) that everyone should be able to decode without having to know any ABI. If that gets bubbled from contract to contract, you may lose the information about who triggered it (you need a trace for that) but you don’t lose the ability to decode the reason (and display it to users).
I think this EIP should target the same thing. A wrapped custom error should be (at least partly) decodable without any information about the ABI of the contract that triggered it.
Specification
When a contract performs a (static)-call, and when that operation fails with some bytes encoding the reason (that can be a custom error, a panic, a revert reason, …), then the contract that received that error can emit the following custom error
error WrappedError(address target, bytes4 selector, bytes reason, bytes details);
With
target: the address that was called, and that returned an errorselector: the first 4 bytes of the call that reverted. If the call was an eth transfer without any data, putbytes4(0)herereason: The error message that was receivedparams: an optional buffer that contains details about the operation that fails. It should correspond to a custom error declared on the contract that emits theWrappedError,- it should be formated using a 4 bytes selector, similar to how function data and custom errors are encoded
- it should be possible to decode it using the ABI of the contract that wrapped the error.
The benefit of this approach is that without the ABI, the only thing you miss is the ability to decode the optional params. You still have full ability to determine that “this function on this contract failled, and this is what we got”.
Note that this allows to rebuild revert traces from nested WrappedError without having to decode the params.
Example and pseudocode
// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
import {IERC20} from "@openzeppelin/contracts/interfaces/IERC20.sol";
interface IERC7751 {
error WrappedError(address target, bytes4 selector, bytes reason, bytes details);
}
contract Vault {
error WithdrawError(address to, uint256 amount);
function withdraw(IERC20 token, address to, uint256 amount) external {
// logic
try token.transfer(to, amount) {} catch (bytes memory error) {
revert IERC7751.WrappedError(address(token), token.transfer.selector, error, abi.encodeWithSignature("WithdrawError(address,uint256)", to, amount));
//NOTE: if solidity was to add support for https://github.com/ethereum/solidity/pull/14974, we could do
//revert IERC7751.WrappedError(address(token), token.transfer.selector, error, abi.encodeError(WithdrawError, to, amount));
}
}
}
contract Router {
function withdraw(IERC20 token, Vault vault, uint256 amount) external {
// logic
try vault.withdraw(token, msg.sender, amount) {} catch (bytes memory error) {
revert IERC7751.WrappedError(address(vault), vault.withdraw.selector, error, new bytes(0));
}
}
}
For decoding a custom error that was received (by calling contract context), one can do
const {ethers} = require('ethers');
function* indentEach(indent, lines) {
for (const line of lines) {
if (Array.isArray(line)) {
yield* indentEach(indent + 1, line);
} else {
const padding = ' '.repeat(indent);
yield* line.split('\n').map(subline => (subline === '' ? '' : padding + subline));
}
}
}
function formatLines(...lines) {
return [...indentEach(0, lines)].join('\n') + '\n';
}
function tryDecode(fragment, data) {
const error = ethers.ErrorFragment.from(fragment);
const interface = ethers.Interface.from([ error ]);
try {
return interface.decodeErrorResult(error, data);
} catch {
return undefined;
}
}
function parseError(error, context) {
if (details = tryDecode('Error(string)', error))
{
const [ reason ] = details;
return [
'[ Revert with reason ]',
`- reason: ${reason}`,
];
}
else if (details = tryDecode('WrappedError(address,bytes4,bytes,bytes)', error))
{
const [ target, selector, reason, params ] = details;
const withParams = ethers.getBytes(params).length > 0;
return [
'[ Wrapped Error ]',
`- calling function ${selector} on contract ${target}`,
`- with underlying error:`,
parseError(reason, target),
withParams && `- and params:`,
withParams && parseError(params, context),
].filter(Boolean);
}
else
{
return [
'[ Unknown error ]',
`- raw data: "${error}"`,
`- try decoding it using ABI at ${context}`,
];
}
}
const IERC7751 = ethers.Interface.from([ 'error WrappedError(address,bytes4,bytes,bytes)' ]);
const StringInterface = ethers.Interface.from([ 'error Error(string)' ]);
const ParamsInterface = ethers.Interface.from([ 'error SomeParams(uint256, uint256)' ]);
const error = IERC7751.encodeErrorResult(
'WrappedError',
[
'0x33da045DC129a97807FCb13bf30baa2Fb2DcC29F',
'0x321f2612',
IERC7751.encodeErrorResult(
'WrappedError',
[
'0xd6B94a1b01c0e79AF91178A8eF0dcc0F7B191708',
'0xa9059cbb',
StringInterface.encodeErrorResult(
'Error',
[
'big badaboom'
]
),
'0x'
]
),
ParamsInterface.encodeErrorResult(
'SomeParams',
[
17,
42
]
)
]
);
console.log(formatLines(parseError(error, '0x239F4A46A9b348A4DE4008ba2DaC4b8be26daDba')));
That returns
[ Wrapped Error ]
- calling function 0x321f2612 on contract 0x33da045DC129a97807FCb13bf30baa2Fb2DcC29F
- with underlying error:
[ Wrapped Error ]
- calling function 0xa9059cbb on contract 0xd6B94a1b01c0e79AF91178A8eF0dcc0F7B191708
- with underlying error:
[ Revert with reason ]
- reason: big badaboom
- and params:
[ Unknown error ]
- raw data: "0xe55cbd440000000000000000000000000000000000000000000000000000000000000011000000000000000000000000000000000000000000000000000000000000002a"
- try decoding it using ABI at 0x239F4A46A9b348A4DE4008ba2DaC4b8be26daDba
Ideally, the address of the contract that produces the error and the calldata it was processing should be retrievable from the execution layer without smart contract code.
Currently one could look at the execution trace to try to do that, but unreliably heuristics are needed to differentiate original errors from errors that are bubbled up.
In that sense, I do think this ERC gets right the fact that the smart contract needs to explicitly mark an error as bubbled. But it would be better if we could omit the address and the calldata (or function arguments), since those should technically be retrievable in some other way.
There’s also the issue that traces are too heavyweight. We would need nodes to provide error data in receipts.
Overall I agree with @Amxx motivations for changing the EIP. However, it’s not clear to me how the new proposal fixes the error bubbling for proxies (e.g. ERC-1167 clones, ERC1967 proxies), for those cases the ABI is still hidden one execution context deeper than expected.
Some off-chain tooling I’ve worked with checks whether the contract has a non-zero address at the ERC1967 implementation slot and then it retrieves the implementation ABI (if available) but it’s not 100% reliable. For example, one can purposely deploy a proxy pointing to another proxy just to obfuscate the lower level information.
I tend to agree with @frangio that the address of the smart contract (in general, the execution context trace) should be provided by other mechanisms, but, many wallets don’t have access to execution traces when they show an error in a UI as they only check for the gas estimation to determine whether the transaction reverts or not. That makes me think that it’s just easier to add the address to the WrappedError @Amxx is proposing.
I thinbk we all agree that should be available. But the fact is, it is not available today.
Changing the client to include more information is a long and tedious process. The goal of this ERC is to propose a solution that is available right now, in the app/user space, without any client modifications.