fix!: misc avm - fixes constraints, ts sim, cpp sim, tracegen#16664
Merged
fix!: misc avm - fixes constraints, ts sim, cpp sim, tracegen#16664
Conversation
Contributor
Author
This was referenced Aug 29, 2025
dbanks12
changed the title
dbanks12
changed the title
dbanks12
force-pushed
the
db/misc-fixes-sideeffects
branch
from
78f4786 to
3c874e4
Compare
dbanks12
requested review from
IlyasRidhuan,
Maddiaa0,
fcarreiro,
jeanmon and
sirasistant
as code owners
dbanks12
commented
Aug 29, 2025
dbanks12
requested review from
MirandaWood
and removed request for
Maddiaa0,
fcarreiro,
jeanmon and
sirasistant
This was referenced Aug 29, 2025
IlyasRidhuan
approved these changes
Aug 31, 2025
Contributor
IlyasRidhuan
left a comment
IlyasRidhuan
left a comment
There was a problem hiding this comment.
LGTM! The comments are minor and more nit-ty
| `Contract instance for address ${contractAddress} in DB: ${exists} != nullifier tree: ${nullifierExistsInTree}. This is a bug!`, | ||
| ); | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
Feels like instead of this coud be re-organised to use the early return instead of this extra if statement. Is the order of checks the following?
isCanonical ? return instance- Nullifier (non)member check
- Return instance (already checked to undefined if !exists)
We probably don't even need the if(exists) branch (the log can be made more generic)
Contributor
Author
There was a problem hiding this comment.
I think it can be simplified to this, but we need an if exists or if !exists somewhere, at least to gate the update check.
fcarreiro
reviewed
Sep 1, 2025
…tion and tx tracegen. Trace side-effect operations before they fail in TS state manager. Always perform nullifier non-membership check on non-existence in AVM TS.
dbanks12
force-pushed
the
db/misc-fixes-sideeffects
branch
from
3c874e4 to
953f475
Compare
ludamad
pushed a commit
that referenced
this pull request
Dec 16, 2025
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
4 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.
Fixes to alu, context, execution, tx pil. Fixes to tx execution and tx tracegen. Trace side-effect operations before they fail in TS state manager. Always perform nullifier non-membership check on non-existence in AVM TS. Side-effects rollbacks in tx execution.
sel_err(any error) rather thansel_tag_err. It at least needs to be disabled on div-by-0 as well.sel_instruction_fetching_successshould be forced to 0 on bytecode retrieval faliure.sel_failureto capture "error or revert opcode".