fix: Enforce recipients of l2l1msgs having 20 bytes at most #19898
+329
−87
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sirasistant
requested review from
IlyasRidhuan,
LeilaWang,
Maddiaa0,
dbanks12,
fcarreiro and
jeanmon
as code owners
sirasistant
removed request for
IlyasRidhuan,
LeilaWang,
Maddiaa0,
dbanks12,
fcarreiro and
jeanmon
sirasistant
changed the title
fcarreiro
reviewed
Jan 23, 2026
fcarreiro
reviewed
Jan 23, 2026
|
|
||
| namespace { | ||
|
|
||
| EthAddress ff_to_eth_address(FF ff) |
LeilaWang
approved these changes
Jan 23, 2026
Contributor
LeilaWang
left a comment
LeilaWang
left a comment
There was a problem hiding this comment.
Only a small nit. Circuit side LGTM 😄
noir-projects/noir-protocol-circuits/crates/types/src/constants.nr
Outdated
Show resolved
Hide resolved
fcarreiro
reviewed
Jan 23, 2026
Collaborator
Flakey Tests🤖 says: This CI run detected 2 tests that failed, but were tolerated due to a .test_patterns.yml entry. |
sirasistant
changed the title
jeanmon
reviewed
Jan 26, 2026
Contributor
jeanmon
left a comment
jeanmon
left a comment
There was a problem hiding this comment.
Looks good. Please consider my comments.
| sel_execute_send_l2_to_l1_msg * (REMAINING_L2_TO_L1_MSG_WRITES * (sel_l2_to_l1_msg_limit_error * (1 - remaining_l2_to_l1_msgs_inv) + remaining_l2_to_l1_msgs_inv) - 1 + sel_l2_to_l1_msg_limit_error) = 0; | ||
|
|
||
| // The opcode errors if we have reached the limit or if we are in a static context | ||
| // TODO: We need this temporarily while we do not allow for aliases in the lookup tuple |
Contributor
There was a problem hiding this comment.
During pre-audit, I am trying to remove any TODO. Can you please replace the TODO, by "Lookup constant support: " or something similar?
|
|
||
| get_gas_tracker().consume_gas(); | ||
|
|
||
| if (greater_than.gt(recipient, MemoryValue::from(FF(MAX_ETH_ADDRESS_VALUE)))) { |
Contributor
There was a problem hiding this comment.
Can you write a comment that it is crucial to have this check before the 2 other error checks (static and MAX number of messages), because in circuit, we always (in opcode logic) check this and needs the gt event to always be emitted.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves https://linear.app/aztec-labs/issue/AVM-209/l2l1msgs-possible-completeness-issue-with-recipient-20-bytes