fix(avm)!: Pre audit public data check / squash#21266
Merged
sirasistant merged 5 commits intomerge-train/avmfrom Mar 13, 2026
Merged
fix(avm)!: Pre audit public data check / squash#21266sirasistant merged 5 commits intomerge-train/avmfrom
sirasistant merged 5 commits intomerge-train/avmfrom
Conversation
sirasistant
requested review from
IlyasRidhuan,
Maddiaa0,
fcarreiro and
jeanmon
as code owners
jeanmon
reviewed
Mar 12, 2026
| * - clk must not repeat for writes to the same slot (asserted by public_data_squash interaction) | ||
| * - low_leaf_index < 2^PUBLIC_DATA_TREE_HEIGHT (Merkle check requirement). | ||
| * - tree_size_before_write < 2^PUBLIC_DATA_TREE_HEIGHT (Merkle check requirement). | ||
| * - The tree genesis state must have an irreproducible hash for uninitialized leaves (0). |
Contributor
There was a problem hiding this comment.
"... irreproducible hash ..": What do we mean by this?
Contributor
Author
There was a problem hiding this comment.
That we can't provide a preimage to
Contributor
There was a problem hiding this comment.
You mean the tree cannot evolve to a hash of the genesis state because according to the mutation constraints it is impossible to fill the leaves with zero? Please add a little explanation.
jeanmon
reviewed
Mar 12, 2026
|
|
||
| // Next sel=1 implies current sel=1 or first_row=1 | ||
| // With this constraint, we can only have sel=1 for N rows after the first, where N could be zero. | ||
| #[START_CONDITION] |
Contributor
There was a problem hiding this comment.
Standard naming is #[TRACE_CONTINUITY]
jeanmon
reviewed
Mar 12, 2026
|
|
||
| pol commit end; // @boolean (by definition) | ||
| #[END_CONDITION] | ||
| end = sel * (1 - sel'); |
Contributor
There was a problem hiding this comment.
We save one degree if we define end like this:
end = sel - not_end
jeanmon
reviewed
Mar 12, 2026
|
|
||
| // Next sel=1 implies current sel=1 or first_row=1 | ||
| // With this constraint, we can only have sel=1 for N rows after the first, where N could be zero. | ||
| #[START_CONDITION] |
Contributor
There was a problem hiding this comment.
Please use standard naming as mentioned earlier.
jeanmon
reviewed
Mar 12, 2026
| namespace bb::avm2::simulation { | ||
|
|
||
| struct PublicDataWriteData { | ||
| PublicDataTreeLeafPreimage updated_low_leaf_preimage; |
Contributor
There was a problem hiding this comment.
Each member of type FF should be initialized to 0.
jeanmon
reviewed
Mar 12, 2026
Contributor
There was a problem hiding this comment.
Same here FF member should be explicitly initialized to zero.
jeanmon
approved these changes
Mar 12, 2026
Contributor
jeanmon
left a comment
jeanmon
left a comment
There was a problem hiding this comment.
Good work! I have only minor comments.
Merged
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Mar 13, 2026
BEGIN_COMMIT_OVERRIDE chore: contract_instance_retrieval pre-audit avm (#21220) fix(avm)!: bytecode hashing - internal audit (#21152) fix(avm)!: precomputed pre-audit (#21313) fix(avm)!: remove unused dc selector (#21314) fix(avm)!: Pre audit public data check / squash (#21266) chore(avm): Pre audit misc opcodes (#21521) END_COMMIT_OVERRIDE
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No relevant findings, just hardening, reordering and documentation.