Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature-request] Error code system, based on Yulish style error"..." #9813

Closed
SilentCicero opened this issue Sep 15, 2020 · 8 comments
Closed
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. language design :rage4: Any changes to the language, e.g. new features stale The issue/PR was marked as stale because it has been open for too long.

Comments

@SilentCicero
Copy link

SilentCicero commented Sep 15, 2020

Abstract

Revert error messages take up a huge amount of byte space, especially for larger builds.

Motivation

You making us crazy.

Suggestion, have a simple error code system, which delivers a map of error codes at compile time. This way error code messages are not baked into the contract itself, which is wasteful.

Suggestion:

// my sol file .sol

require(someAssertion, error"my-error-code-message");

// compiled build .json

{
   "abi": [],
   "bytecode": "0x",
   "errors": {
       "0": "my-error-code-message",
   },
}

Specification

Error codes are registered in order, are unique, and properly map to the output in the build. We do this in Yul+ and it has been a huge savings for larger deployments.

Notation is similar to hex"", i.e. error"".

Backwards Compatibility

Not sure on this one.

@SilentCicero SilentCicero changed the title [feature-request] Error code system, based on Yul style error"..." [feature-request] Error code system, based on Yulish style error"..." Sep 15, 2020
@chriseth
Copy link
Contributor

chriseth commented Sep 15, 2020

I cannot dig up the issue number now, but we are planning to add custom errors to 0.8 in the way of

error MyErorrKind(/*potential parameters */)

why can then be used like

throw MyErrorKind(/*potential arguments */);
require(x, MyErrorKind())

and

try ... { ... } catch MyError(/* variables */) { ... }

The implementation would use function signatures in the same way as the ABI, so if you don't have arguments, it needs 4 bytes and a memory write.

Would that work here?

@cameel
Copy link
Member

cameel commented Sep 15, 2020

I think it was #7877.

@cameel cameel added the feature label Sep 15, 2020
@SilentCicero
Copy link
Author

@chriseth so the emit Error might be okay, but my system as you can see is very simple. Very basic. Just mainly to avoid the headache of string bloat with error messages.

I feel mine would be easier to implement, but sure, the above might do it. How are errors interpreted outside in testing environments?

The reason why I like my direction is it's clear, the strings are registered in the compiled output, and can be hooked into testing environments.

I do this with Yul+ and it works great.

@chriseth
Copy link
Contributor

Here is a new writeup that also contains some more context: https://hackmd.io/b0vr4MFrSkCLlBEacnds0Q

The advantage of typed errors is that you can properly catch them in catch statements and you can also have longer string explanations in NatSpec documentation that is exported via the ABI.

@cameel cameel added the language design :rage4: Any changes to the language, e.g. new features label Sep 25, 2020
@cameel
Copy link
Member

cameel commented Jun 9, 2021

Is this issue still relevant now that we have custom errors implemented?

@axic
Copy link
Member

axic commented Jun 9, 2021

I think the proposal is different to the error system, as the current one still uses more bytes than suggested.

Probably it is up to @SilentCicero to decide if this is pursued further or not.

@github-actions
Copy link

github-actions bot commented Mar 8, 2023

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 8, 2023
@github-actions
Copy link

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Mar 16, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. language design :rage4: Any changes to the language, e.g. new features stale The issue/PR was marked as stale because it has been open for too long.
Projects
None yet
Development

No branches or pull requests

5 participants