Skip to content
This repository has been archived by the owner on Jul 13, 2023. It is now read-only.

Blog post about custom errors #135

Merged
merged 1 commit into from
Apr 21, 2021
Merged

Blog post about custom errors #135

merged 1 commit into from
Apr 21, 2021

Conversation

hrkrshnn
Copy link
Member

No description provided.


[Solidity v0.8.4](https://github.com/ethereum/solidity/releases/tag/v0.8.4) provides a convenient and gas-efficient way to explain to user why an operation failed. They can be defined inside and outside of contracts (including interfaces and libraries).

The syntax of Errors are similar to that of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rather put this after the example? It could be confusing to mention events here, because only the syntax is similar, not the semantics.

}
```

In the above example, `revert Unauthorized();` is equivalent to the following Yul code:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this going a bit too deep?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should rather highlight the cost difference compared to revert("Unauthorized")?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to include something that makes it clear that solidity isn't randomly assigning numbers to errors. Perhaps I can rephrase this slightly better, but still include the Yul code.

I'll also highlight the cost difference.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not too complicated, maybe we should really just show how to decode it using ethers.js - in that way the encoding should be clear, shouldn't it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People will stop reading when they see "depth" and inline assembly. We can use this, but then it should be put at the very end of the blog post.

@chriseth
Copy link
Contributor

Maybe we should add

  • that there is no convenient way to catch errors in Solidity, but that is planned.
  • you should not rely on error data (because inner calls' error data bubbles up), only use it as indication about the reason for failure
  • a way to retrieve the error data via the RPC and state that we hope to get support by the frameworks soon?

@chriseth
Copy link
Contributor

Maybe we could even add a snippet for ethers.js about how to decode the error in the frontend manually?

@chriseth
Copy link
Contributor

@hrkrshnn hrkrshnn force-pushed the error-blog branch 5 times, most recently from 5ca8713 to 6d1a46d Compare April 20, 2021 15:38
@hrkrshnn
Copy link
Member Author

@chriseth Fixed the review comments.

I did not include anything about RPC calls. Not sure what I should write about it.

Copy link
Member

@franzihei franzihei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of comments. :)

_posts/2021-04-20-custom-errors.md Outdated Show resolved Hide resolved
_posts/2021-04-20-custom-errors.md Outdated Show resolved Hide resolved
_posts/2021-04-20-custom-errors.md Outdated Show resolved Hide resolved
_posts/2021-04-20-custom-errors.md Outdated Show resolved Hide resolved
_posts/2021-04-20-custom-errors.md Outdated Show resolved Hide resolved
_posts/2021-04-20-custom-errors.md Outdated Show resolved Hide resolved
_posts/2021-04-20-custom-errors.md Outdated Show resolved Hide resolved
contracts it calls directly. Furthermore, any contract can fake any error by returning data that
matches an error signature, even if the error is not defined anywhere.

Currently, there is no convenient way to catch errors in Solidity, but this is planned.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have an issue or anything that we can link to or can we make the "this is planned" more detailed or sound a bit nicer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any issue. I'll make a new one.

@hrkrshnn hrkrshnn force-pushed the error-blog branch 2 times, most recently from f8b7dbc to 653e4c0 Compare April 21, 2021 08:15

contract VendingMachine {
address payable owner = payable(msg.sender);
error Unauthorized();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should move it outside of the contract so that it stands out more?

together with the [revert
statement](https://docs.soliditylang.org/en/latest/control-structures.html#revert-statement) which
causes all changes in the current call to be reverted and passes the error data back to the caller.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add: Using errors together with require is not yet supported (see below)

and then further down explain that require(c, "abc") should be translated into if (!c) revert Abc();

}
```

The error data would be encoded identically as the ABI encoding for function calls, i.e.,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use this instead of inline assembly above?

Comment on lines 110 to 126
const abi = [{
"inputs": [{
"internalType": "uint256",
"name": "available",
"type": "uint256"
},
{
"internalType": "uint256",
"name": "required",
"type": "uint256"
}
],
"name": "InsufficientBalance",
"outputs": [],
"stateMutability": "nonpayable",
"type": "function"
}];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One reason why ethers is far superior to web3.js:

Suggested change
const abi = [{
"inputs": [{
"internalType": "uint256",
"name": "available",
"type": "uint256"
},
{
"internalType": "uint256",
"name": "required",
"type": "uint256"
}
],
"name": "InsufficientBalance",
"outputs": [],
"stateMutability": "nonpayable",
"type": "function"
}];
const abi = ["function InsufficientBalance(uint256, uint256)"];

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would only return

[
  BigNumber { _hex: '0x0100', _isBigNumber: true },
  BigNumber { _hex: '0x0100000000', _isBigNumber: true }
]

With the full abi-json, it's nice to see the parameters available and required mapped to values.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it help if you use function InsufficientBalance(uint256 available, uint256 required)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow. That actually works. I'll change the abi.

@chriseth
Copy link
Contributor

@ricmoo I'm wondering how for the support for custom errors is in ethers.js - maybe you can help?

@ricmoo
Copy link

ricmoo commented Apr 21, 2021

Heya, just catching up now on this issue.

I've been waiting for EIP-838 to add to the Human-Readable ABI. I've been planning to add something similar to (for example):

const ABI = [
    "event Transfer(address from, address to, uint amount",
    "function transfer(address to, uint amount)"
    "error InsufficientBalance(address owner, uint balance)"
];

// or something similar I hope Solidity dumps out:, similar to Event

const ABI_JSON = [
  {
    type: "error",
    name: "InsufficientBalance",
    inputs: [
      { name: "owner", type: "address" },
      { name: "balance", type: "uint256" },
    ]
  }
];

Then if the contract throws that selector (i.e. the length of the result is congruent to 4 mod 32, with he first 4 bytes matching keccak256("InsufficientFunds(address,uint256)")) the error reason would be included in the thrown CALL_EXCEPTION error.

As an example for a blog on manual performing this, it would be something similar to (pseudocode):

if (result.length % 32 == 4 && result.slice(0, 4) === SELECTOR) {
    const result = abiDecode([ "address", "uint" ], result.slice(4))
    console.log(result[0], result[1]);
}

Is this available yet? Or is this something else, and I'm just confused. :)

Either way, I'll help out any way I can.

@chriseth
Copy link
Contributor

Thanks for the quick response, @ricmoo! The errors will be very similar to events with type 'error' in the ABI. Can we use decodeFunctionData as in https://github.com/ethereum/solidity-blog/pull/135/files#diff-c78d8b6f4ffa6dfa8f5555b5d9a0be5d043268c4b83eedf56f3b81051b7f4b5eR133 ? Also, do you know a more or less reliable way to get the error data? Then we could include a full example starting with initiating the transaction and ending with a 'console.log' of the decoded error - maybe even including the natspec.

// BigNumber { _hex: '0x0100000000', _isBigNumber: true },
// available: BigNumber { _hex: '0x0100', _isBigNumber: true },
// required: BigNumber { _hex: '0x0100000000', _isBigNumber: true }
// ]
Copy link
Member Author

@hrkrshnn hrkrshnn Apr 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ricmoo This is currently how we suggested dealing with decoding errors. Do you have anything to add here?

@ricmoo
Copy link

ricmoo commented Apr 21, 2021

LOL! Awesome and insane.

That solution will work, making ethers think the Error is a function and let it compute the selector as is, but I would add a new type, an ErrorFragment, and add support to Interface.decodeFunctionResult so that parsing a revert reason throws an error with all the parameters parsed out along with the error name and parsed values.

Then it would look like:

// Pure/view will just work
try {
    const balance = amount contract.balance(constants.ZeroAddress);
    console.log("BALANCE", balance);
} catch (error) {
    console.log(error);
    // error: {
    //.  code: "CALL_EXCEPTION",
    //   errorSignature: "CannotGetBalanceOfZero(address)",
    //   errorArgs: [ "0x0000...0000" ]
    // }
}

For non-constant methods though, the return data isn't really around anywhere. Maybe an example would be something akin to using the staticCall?

try {
    const check = await contract.staticCall.transfer(other, amount);
    console.log(check);
} catch (error) {
    console.log(error);
    // {
    //.  errorSignature: "InsufficientBalance(address)"
    //.  errorArgs: other
    // }
}

When are you thinking of releasing this? Is the support for EIP-838 ready today? I could start adding first-class support tomorrow.

@chriseth
Copy link
Contributor

@ricmoo we plan to release it today and it would be great to have something that shows how it can be used right now instead of maybe tomorrow :)

Thing thing about transactions is really annoying. Is there no way to retrieve the error data? This here - https://gist.github.com/gluk64/fdea559472d957f1138ed93bcbc6f78a - seems to just throw eth_call after a failing transaction to see why it went wrong. Do you think that will work?

@ricmoo
Copy link

ricmoo commented Apr 21, 2021

So the code you have will work fine for something today. And I'll start on adding first-class support tomorrow.

About using eth_call, it can work and might even work in many cases, but certainly isn't guaranteed to work all the time. Simple counter example:

  function randomlyFail() {
    if (block.timestamp % 2) { revert("So long sucka!"); }
  }

The eth_call might succeed just fine before and after (depending on what the node uses as its timestamp during evaluation) but the transaction failed. This is obviously contrived, but this is effectively why many flash bot and the ilk transactions fail. So there is no good way to programatically get this data.

It would be nice if transaction receipts included return data for the most recent X blocks, but it obviously starts becoming very expensive quick, and would basically require recent blocks to basically have archive data available. I understand the reasons they don't do it. But it would be nice. :)

Maybe when interacting with an archive node we could offer a more automatic injection...

@chriseth
Copy link
Contributor

You obviously mean

error SoLongSucka();
...
function randomlyFail() { if (block.timestamp % 2) { revert SoLongSuca(); }

;)

So passing the transaction's block number as in the example would work (unless there are some other transactions that mess with the state in the same block), but you are mainly saying that it will not work on most rpc endpoints because they need an archive node for anything but the most recent blocks, right?

@ricmoo
Copy link

ricmoo commented Apr 21, 2021

Ah yes. I'm not up to date on the latest (or possibly any?) syntax. :)

But yes, exactly. A transaction may fail while the eth_call passes. Or possibly worse, the transaction may fail for one reason and the eth_call fails with a different error. Only an archive node can replay the transaction to find out what actually happened.


const interface = new ethers.utils.Interface(abi);
const error_data = "0xcf47918100000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000100000000";
interface.decodeFunctionData(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.log(...?

Comment on lines 98 to 105
interface.decodeFunctionData(
interface.functions['InsufficientBalance(uint256,uint256)'],
error_data
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe

Suggested change
interface.decodeFunctionData(
interface.functions['InsufficientBalance(uint256,uint256)'],
error_data
);
const decoded = interface.decodeFunctionData(
interface.functions['InsufficientBalance(uint256,uint256)'],
error_data
);
console.log(`Insufficient balance for transfer. Needed ${decoded.required}` but only ${decoded.available} available.`)

(plus formatting of the big numbers)

@hrkrshnn hrkrshnn force-pushed the error-blog branch 2 times, most recently from 108ae7d to fd4b55f Compare April 21, 2021 14:35
@chriseth
Copy link
Contributor

@ricmoo now that I have you here: I'm assuming this is an error from ethers.js just seeing the version "5.0.7": creation of Storage errored: invalid fragment object (argument="value", value={"inputs":[],"name":"X","type":"error"}, code=INVALID_ARGUMENT, version=abi/5.0.7)

Can you either fully implement it or create a quick fix by ignoring types in the ABI you don't know about? This error prevents contracts using errors from being deployed by remix.

@hrkrshnn hrkrshnn merged commit da841dd into master Apr 21, 2021
@hrkrshnn hrkrshnn deleted the error-blog branch April 21, 2021 16:22
@ricmoo
Copy link

ricmoo commented Apr 22, 2021

@chriseth Yupp, it's being tracked in issue 1497 to remove the error in a patch and issue 1498 for a minor bump to add support. :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants