Skip to content

Add assert_msg(cond, "message") emitting Solidity Error(string)#1399

Merged
g-r-a-n-t merged 2 commits intoargotorg:masterfrom
cburgdorf:better_assert
Apr 30, 2026
Merged

Add assert_msg(cond, "message") emitting Solidity Error(string)#1399
g-r-a-n-t merged 2 commits intoargotorg:masterfrom
cburgdorf:better_assert

Conversation

@cburgdorf
Copy link
Copy Markdown
Collaborator

@cburgdorf cburgdorf commented Apr 16, 2026

Introduces assert_msg<const N: usize>(b: bool, message: String<N>) in
the std prelude. On b == false it reverts with the 4-byte selector
0x08c379a0 (keccak256("Error(string)")[..4]) followed by the
ABI-encoded message, matching Solidity's require(cond, "message") /
revert("message").

Also improves const-generic inference so a string literal can satisfy a
generic String<N> parameter without an annotation at the call site:
when a string literal is passed to a parameter of shape String<N> and
the inference key for N appears only in that one parameter, the
checker pins N to the literal's min length. Keys shared across
multiple parameters are left alone so the other arguments (e.g.
[u8; N]) still drive inference for N.

Implements #1348

@cburgdorf
Copy link
Copy Markdown
Collaborator Author

@codex review

Copy link
Copy Markdown

@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: fb5af0b111

ℹ️ 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".

Comment thread crates/hir/src/analysis/ty/unify.rs Outdated
@cburgdorf cburgdorf force-pushed the better_assert branch 2 times, most recently from 0feac7a to 9151f04 Compare April 17, 2026 19:14
@cburgdorf
Copy link
Copy Markdown
Collaborator Author

@codex review

Copy link
Copy Markdown

@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: 9151f04914

ℹ️ 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".

Comment thread crates/hir/src/analysis/ty/ty_check/callable.rs Outdated
@cburgdorf
Copy link
Copy Markdown
Collaborator Author

@codex review

Copy link
Copy Markdown

@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: 404023ca5e

ℹ️ 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".

Comment thread crates/hir/src/analysis/ty/ty_check/callable.rs Outdated
@cburgdorf
Copy link
Copy Markdown
Collaborator Author

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. You're on a roll.

ℹ️ 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 marked this pull request as ready for review April 17, 2026 21:37
@cburgdorf cburgdorf requested a review from sbillig April 17, 2026 21:38
@sbillig
Copy link
Copy Markdown
Collaborator

sbillig commented Apr 22, 2026

Proposal:

Define assert_msg in terms of Text aka DynString:

     use super::abi::Text

     pub fn assert_msg(_ b: own bool, _ message: own Text) {
         if !b {
             let (ptr, len) = encode_calldata(0x08c379a0, message)
             ops::revert(offset: ptr, len: len)
         }
     }

(this might increase gas usage)

and remove the hir changes. There's an issue in hir to be solved, but the current solution isn't the right one. I think we need to modify the type unifier so that it imposes an N >= literal_len constraint for String<N>, which probably isn't a huge change, but could be deferred.

@cburgdorf
Copy link
Copy Markdown
Collaborator Author

@sbillig ok, made the requested change.

@sbillig
Copy link
Copy Markdown
Collaborator

sbillig commented Apr 29, 2026

I think the abi-encoded payload is wrong. I'm rebasing and investigating.

@sbillig
Copy link
Copy Markdown
Collaborator

sbillig commented Apr 29, 2026

Here's what the agent says:

Change assert_msg from:

let (ptr, len) = encode_calldata(0x08c379a0, message)

to:

let (ptr, len) = encode_calldata(0x08c379a0, (message,))

with a short comment explaining why.

Why: encode_calldata expects its second argument to represent the full ABI payload after
the selector. Generated msg/custom-error structs already do that. A bare Text/DynString
encodes as its dynamic payload:

len || bytes

but Error(string) has one ABI argument, so it needs the single-argument head first:

offset(0x20) || len || bytes

So for "boom", the correct full revert data is:

08c379a0
0000000000000000000000000000000000000000000000000000000000000020
0000000000000000000000000000000000000000000000000000000000000004
626f6f6d00000000000000000000000000000000000000000000000000000000

@cburgdorf cburgdorf force-pushed the better_assert branch 2 times, most recently from 25e5450 to 9c90b02 Compare April 29, 2026 19:18
@cburgdorf
Copy link
Copy Markdown
Collaborator Author

Ok, I did what the agent said 🫡

Introduces `assert_msg(b: bool, message: Text)` in the std prelude.
On `b == false` it reverts with the 4-byte selector `0x08c379a0`
(keccak256("Error(string)")[..4]) followed by the ABI-encoded
 message, matching Solidity's `require(cond, "message")` /
`revert("message")`.
@cburgdorf
Copy link
Copy Markdown
Collaborator Author

...and added another test that covers this.

@g-r-a-n-t g-r-a-n-t self-requested a review April 30, 2026 01:22
@g-r-a-n-t g-r-a-n-t merged commit 7769fe4 into argotorg:master Apr 30, 2026
7 checks passed
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.

3 participants