feat(avm): class id derivation #12263
Merged
Conversation
This file contains 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
IlyasRidhuan
force-pushed
the
ir/02-24-feat_avm_class_id_derivation
branch
from
8cfc56a to
44a43eb
Compare
IlyasRidhuan
commented
Feb 25, 2025
| @@ -156,6 +156,18 @@ const PIL_CONSTANTS = [ | |||
| 'MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS', | |||
| ]; | |||
|
|
|||
| const PIL_GENERATORS: string[] = [ | |||
There was a problem hiding this comment.
Generators weren't included in the PIL in the previous PR - followed the same approach as the cpp gen
IlyasRidhuan
commented
Feb 25, 2025
| for (size_t i = 0; i < num_perm_events; i++) { | ||
| std::array<FF, 3> perm_input = { 0, 0, 0 }; |
There was a problem hiding this comment.
This was actually a little bug in poseidon2_hash.pil 😱. We weren't cleaning out the input array between permutation runs.
IlyasRidhuan
commented
Feb 25, 2025
| @@ -67,21 +67,20 @@ namespace poseidon2_hash; | |||
| // The permutation output values are represented by b_0, b_1, b_2, b_3 | |||
| // This most definitely could be simplified to a lower degree check | |||
| // The next perm input is constrained to be the previous perm output + the new values to be hashed if we | |||
| // are not at the start or the end of hashing | |||
| pol NEXT_INPUT_IS_PREV_OUTPUT_SEL = (1 - LATCH_CONDITION) * (1 - start); | |||
There was a problem hiding this comment.
This was a bug 😨 . It was too relaxed, it's changed to only be the (1 - LATCH_CONDITION)
IlyasRidhuan
commented
Feb 25, 2025
| @@ -43,4 +43,13 @@ namespace constants; | |||
| pol START_EMIT_NULLIFIER_WRITE_OFFSET = 207; | |||
| pol START_EMIT_L2_TO_L1_MSG_WRITE_OFFSET = 223; | |||
| pol START_EMIT_UNENCRYPTED_LOG_WRITE_OFFSET = 225; | |||
| pol GENERATOR_INDEX__NOTE_HASH_NONCE = 2; | |||
There was a problem hiding this comment.
Not used but part of constants gen right now
IlyasRidhuan
commented
Feb 25, 2025
| } | ||
|
|
||
| // TODO: This should probably be refined and moved to bc_retrieval test file once that exists | ||
| TEST(ClassIdDerivationConstrainingTest, WithRetrievalInteraction) |
There was a problem hiding this comment.
When bc_retrieval is being properly done, we should move the test to there
IlyasRidhuan
requested review from
jeanmon,
fcarreiro and
Maddiaa0
as code owners
fcarreiro
approved these changes
Feb 26, 2025
| // H(GENERATOR_INDEX__CONTRACT_LEAF, artifact_hash, private_function_root, public_bytecode_commitment) | ||
| pol commit class_id; | ||
|
|
||
| pol commit temp_constant_for_lookup; |
There was a problem hiding this comment.
Can you add a todo comment about the constants?
| // Since the inputs to poseidon2 have to be chunks of 3, we need two lookups if we want to do this in a single row | ||
| #[CLASS_ID_POSEIDON2_0] | ||
| sel { temp_constant_for_lookup, artifact_hash, private_function_root, class_id } | ||
| in poseidon2_hash.sel { poseidon2_hash.input_0, poseidon2_hash.input_1, poseidon2_hash.input_2, poseidon2_hash.output }; |
There was a problem hiding this comment.
Is it possible to use a more fine grained selector like poseidon2_hash.end? Saves us inverses etc! (same below).
IlyasRidhuan
force-pushed
the
ir/02-24-feat_avm_class_id_derivation
branch
from
a02e2f7 to
dd671ad
Compare
TomAFrench
added a commit
that referenced
this pull request
Feb 26, 2025
* master: (92 commits) chore: Log prover publisher address on creation (#12267) feat: https for bootnode in devnet (#12161) feat(avm): support shifts in lookups (#12280) feat(docs): Add flamegraph tool to counter contract tutorial (#12202) feat(spartan): 192 node 1 tps - additional validator service (#12238) feat(avm): class id derivation (#12263) docs: Fees doc snippets and code snippets (#12229) refactor: proving cost in fee header (#12048) fix: prometheus scrapes itself in the cluster (#12277) feat: metrics (#12256) chore: cleanup stdlib internal imports (#12274) git subrepo push --branch=master noir-projects/aztec-nr git_subrepo.sh: Fix parent in .gitrepo file. [skip ci] chore: replace relative paths to noir-protocol-circuits git subrepo push --branch=master barretenberg fix: Enforce no import side effects (#12268) refactor!: note interfaces (#12106) yolo undenoise tests feat: new transcript functionality to hash elements without including in proof (#12233) chore: remove gcloud metrics (#12265) ...
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.
Please read contributing guidelines and remove this line.