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

Panic error codes are hard to use #10432

Closed
drortirosh opened this issue Nov 28, 2020 · 4 comments
Closed

Panic error codes are hard to use #10432

drortirosh opened this issue Nov 28, 2020 · 4 comments
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

@drortirosh
Copy link

Current 0.8 suggestion for Panic revert is to use enum values.

Instead of enum values, it is better to provide string32 (string encoded in base32 value)
that is, change the encoding from Panic(uint256) to Panic(bytes32)

Motivation

Numeric constants are notorious and error-prone to use, and have no benefit over string.

Specification

the internal type-checking should revert with signature Panic(bytes32)
The parameter value would be one of the following:

ASSERT, OVER/UNDER-FLOW, DIV-BY-ZERO, POP-EMPTY-ARRAY, OUT-OF-BOUNDS, ARRAY-TOO-LARGE

Backwards Compatibility

(none: its a new feature)

@cameel
Copy link
Member

cameel commented Nov 28, 2020

Even though the signature is Panic(uint256), not all 32 bytes are used. All existing codes fit in 1 byte which minimizes the impact on the size of the resulting bytecode (see the discussion in #9824 (comment)).

But I agree that something like Panic(0x12) is not very readable and requires looking up the code or memorizing it. I think there should be some syntactic sugar for that. E.g. Panic("DivisionByZero") - a special overloaded constructor that accepts a string literal and gets translated in the code generator to one accepting the corresponding numeric code. Or even Panic(DivisionByZero) or PanicDivisionByZero() but that has the disadvantage of polluting the global namespace with more stuff.

Alternatively, global constants representing the codes (#9671) might be something to define once and put in the standard library (#10282).

@cameel cameel added feature language design :rage4: Any changes to the language, e.g. new features labels Nov 28, 2020
@chriseth
Copy link
Contributor

@drortirosh when you say "difficult to use" , which kind of "use" do you have in mind? I don't expect user-supplied Solidity code to create such panic error codes directly - after all, they are internal errors. On the other hand, code that consumes panic errors probably does either not care about the code itself (Solidity code) or directly translates it into a human-readable string (debugging tools).

@github-actions
Copy link

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 12, 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 20, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 20, 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

4 participants