chore(avm)!: Bytecode Decomposition pre-audit docs - WIP#20120
Draft
MirandaWood wants to merge 3 commits intomerge-train/avmfrom
Draft
chore(avm)!: Bytecode Decomposition pre-audit docs - WIP#20120MirandaWood wants to merge 3 commits intomerge-train/avmfrom
MirandaWood wants to merge 3 commits intomerge-train/avmfrom
Conversation
MirandaWood
commented
Feb 3, 2026
Comment on lines
141
to
+143
| // Remark: Depending on how bytecode hashing constraints work, we might remove this relation. | ||
| #[BC_DEC_SEL_BYTES_REM_NON_ZERO] | ||
| // MW note: We do NOT constrain this in bytecode hashing, but we could move it there. One downside of doing that is there are no lookups | ||
| // into this trace at /each row/ (all conditional e.g. sel_not_start) so we would need something gated by sel. |
Contributor
Author
There was a problem hiding this comment.
If everyone agrees, I could remove this Remark and keep #[BYTES_REM_NON_ZERO] in this trace?
MirandaWood
commented
Feb 3, 2026
Comment on lines
358
to
+364
| // NOTE: The bytecode hashing trace constrains that every 31st pc is packed, however | ||
| // we must duplicate this check here as we do not constrain uniqueness of bytecode ids: | ||
| // MW note: Is this true now that we use a permutation into this trace? The perm. looks up pc (constrained to be a multiple of 31), | ||
| // bytecode id (as above, not constrained to be unique, but is constrained to be consistent), and packed_field. | ||
| // We should ensure that packed_field is non zero when we have bytes to pack. IIUC this is enforced by hashing trace: | ||
| // hashing permutations are on as long as we have not finished the bytecode => sel_packed_read[i] must be on => | ||
| // sel_packed must be on every 31st byte => #[BC_DECOMPOSITION_REPACKING] must be satisfied. |
Contributor
Author
There was a problem hiding this comment.
Maybe we could remove this check - would appreciate a logic check on this comment!
Since the hashing trace is the 'source', if it is 'off' then decomp could easily just skip any packing. But this may be ok since a) we seem to assume a decomp event is always accompanied by a hashing event and b) currently only the hashing trace makes use of packed_field and its correctness.
MirandaWood
commented
Feb 3, 2026
Comment on lines
379
to
+380
| // TODO: Once we manually optimize the generated files, this subrelation can benefit from short-circuit multiplication. | ||
| // MW note: Can we do this ^ now? |
Contributor
Author
There was a problem hiding this comment.
As above - I'm not sure what this old comment was referring to!
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
1 participant
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.
WIP!
This branch is only low priority changes (no bug fixes or big logic changes) including:
Applies to AVM-51
TODOs/Notes: See comments!