-
Notifications
You must be signed in to change notification settings - Fork 578
feat(avm): defensively assert cd hashes #19346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(avm): defensively assert cd hashes #19346
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
e1ca69a to
8325579
Compare
97782de to
36ee031
Compare
yarn-project/simulator/src/public/fuzzing/avm_fuzzer_simulator.ts
Outdated
Show resolved
Hide resolved
36ee031 to
86093ec
Compare
8325579 to
5ab4f28
Compare
fcarreiro
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note that this does make C++ simulation check the cd hashes and fail catastrophically if sth goes wrong. Assuming that the hash is already validated in TX validation, this should be ok (a noop change).
Btw, the sequencer does not generate hints and give them to the prover. The prover itself generates the hints from the TXs and gives them to the agents (all of it under its control).
Once CI passes, lmk and I'll approve.
barretenberg/cpp/src/barretenberg/vm2/simulation/interfaces/context_provider.hpp
Outdated
Show resolved
Hide resolved
barretenberg/cpp/src/barretenberg/vm2/simulation/interfaces/calldata_hashing.hpp
Outdated
Show resolved
Hide resolved
barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/calldata_hashing.hpp
Outdated
Show resolved
Hide resolved
barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/calldata_hashing.cpp
Show resolved
Hide resolved
barretenberg/cpp/src/barretenberg/vm2/simulation/gadgets/calldata_hashing.cpp
Outdated
Show resolved
Hide resolved
86093ec to
619ee29
Compare
5ab4f28 to
998b6c7
Compare
619ee29 to
0ffaa16
Compare
BEGIN_COMMIT_OVERRIDE feat(avm)!: optionally use TS logger in C++ simulation (#19305) chore(avm): bytecode caching comments chore(avm): disable VK hash checking in tests fix(avm)!: instr_fetching soundness bug (#19381) fix(avm): dont catch wide exceptions (#19388) refactor(avm): Refactor get contract instance fuzzer backfill (#19387) feat(avm): mutate enqueued calls (#19315) chore(avm): migrate to BB asserts (#19395) fix!: more missing boolean constraints in calldata, calldata hashing, sha256 mem PILs (#19367) feat(avm): defensively assert cd hashes (#19346) chore: annotate booleans in PIL, and add some missing boolean constraints (#19371) fix!: missing boolean constraints on zero checks targets (#19401) fix!: context did not constrain returndata size to 0 at start, and had a misnamed relation (#19404) END_COMMIT_OVERRIDE

This PR moves us from
compute_calldata_hash->assert_calldata_hashwhich essentially adds a degree of validation to the calldata hash in the AVM cpp code.Why tho?
assert_address_derivation.H(calldata) != calldata_hash)Sharp Edges
In TS, cd hash validation is done at the tx validation level. I don't think it is worthwhile to implement the cd hash validation in the TS simulator - so instead I've implemented a validation in the TS fuzzer entrypoint