8000 Make `unwrap()` ABI-encode errors on revert by cburgdorf · Pull Request #1342 · argotorg/fe · GitHub
[go: up one dir, main page]

Skip to content

Make unwrap() ABI-encode errors on revert#1342

Merged
sbillig merged 2 commits intoargotorg:masterfrom
cburgdorf:unwrap_revert
Mar 19, 2026
Merged

Make unwrap() ABI-encode errors on revert#1342
sbillig merged 2 commits intoargotorg:masterfrom
cburgdorf:unwrap_revert

Conversation

@cburgdorf
Copy link
Collaborator
@cburgdorf cburgdorf commented Mar 19, 2026

Alternative to #1326

Make unwrap() ABI-encode errors on revert

Result::unwrap() now automatically ABI-encodes the error value when
reverting, instead of emitting an empty revert(0, 0).

A new panic_with_value<T> intrinsic carries the error value from
unwrap() through MIR as AbortWithValue. The monomorphizer, where
types are concrete, checks whether T implements Encode<Sol> + AbiSize.
If satisfied, the call is rewritten to std::evm::effects::revert<T>(),
producing ABI-encoded revert data. If not, a compile error is emitted
requiring the error type to implement the necessary traits.

abi_static_size_bytes() only handled primitives and structs/tuples,
causing encoded_size<T>() to fail at codegen when T is an enum.
The extern intrinsic has no body, so when the compile-time constant
folding failed, the call fell through to a missing function.

Add enum support: discriminant word (32 bytes) + max variant payload,
mirroring the pattern in layout::ty_size_bytes.
@cburgdorf
Copy link
Collaborator Author

@codex review

Copy link
@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 96d8b38267

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@cburgdorf cburgdorf force-pushed the unwrap_revert branch 2 times, most recently from 436ecd9 to 9007ea3 Compare March 19, 2026 10:25
@cburgdorf
Copy link
Collaborator Author

@codex review

@cburgdorf cburgdorf marked this pull request as ready for review March 19, 2026 10:41
Copy link
@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9007ea3491

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9007ea3491

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@cburgdorf
Copy link
Collaborator Author

@codex review

Copy link
@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3f06209dc4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

`Result::unwrap()` now automatically ABI-encodes the error value when
reverting, instead of emitting an empty `revert(0, 0)`.

A new `panic_with_value<T>` intrinsic carries the error value from
`unwrap()` through MIR as `AbortWithValue`. The monomorphizer, where
types are concrete, checks whether `T` implements `Encode<Sol> + AbiSize`.
If satisfied, the call is rewritten to `std::evm::effects::revert<T>()`,
producing ABI-encoded revert data. If not, a compile error is emitted
requiring the error type to implement the necessary traits.
@cburgdorf
Copy link
Collaborator Author

@sbillig That's an alternative to #1326 where unwrap itself will cause the revert with the ABI encoded revert data.

@sbillig sbillig merged commit 5799b54 into argotorg:master Mar 19, 2026
6 checks passed
@cburgdorf cburgdorf mentioned this pull request Mar 20, 2026
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

0