-
Notifications
You must be signed in to change notification settings - Fork 245
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
feat(env): Replace panic with panic_str and add abort method #492
Conversation
} | ||
|
||
/// Aborts the current contract execution without a custom message. | ||
/// To include a message, use [`panic_str`]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you said this is to really minimize overhead. Should we put something in the comments for folks to know when they might want to use this? This might be wrong, but I was imagining something like, "This can be a useful approach for smart contracts that avoid using UTF-8 for optimization reasons, something something…"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has nothing to do with optimizing, as this has the same internal type as bytes, this just makes sure at compile time that the "bytes" are utf8 and is a more ergonomic type (easier to get to &str
for logging messages and bytes gives no benefit since non-utf8 disallowed).
TLDR just makes the API harder to misuse as the type is correct now
I wanted to avoid doing this to increase the number of changes necessary, but since it doesn't break anything I think it might be okay. This makes
panic
inline withlog
where it correctly takes a&str
because the bytes must be utf8, symmetric with #362. It seems weird to update one and not the other before the stable release.Also adds
abort
which just panics without a custom message, as this could be useful for some. This second part is optional as likely if someone is trying to optimize this much for contract size, it might be fine to require going through unsafe sys interface directly, to discourage this usage by default.