-
Notifications
You must be signed in to change notification settings - Fork 599
feat: hash contract class logs with poseidon + add preimage to blob #12061
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
Changes from all commits
2031c58
2160e38
a00ab47
03adfc7
6057a35
cb739a2
f8fafa0
b6d797a
ccb7be7
6d2349d
56b8cd1
c2ebf15
7cd0932
2dcc661
edf15af
36edf5a
fee44dc
5bc9634
9685104
9764a0e
6a4a0da
7593562
81920fd
e26bb79
530f013
34a43bf
799d842
a9f8424
3e70e8a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,12 +7,15 @@ pub contract ContractClassRegisterer { | |
| use dep::aztec::protocol_types::{ | ||
| abis::log_hash::LogHash, | ||
| constants::{ | ||
| ARTIFACT_FUNCTION_TREE_MAX_HEIGHT, FUNCTION_TREE_HEIGHT, | ||
| MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS, | ||
| ARTIFACT_FUNCTION_TREE_MAX_HEIGHT, CONTRACT_CLASS_LOG_DATA_SIZE_IN_FIELDS, | ||
| FUNCTION_TREE_HEIGHT, MAX_PACKED_BYTECODE_SIZE_PER_PRIVATE_FUNCTION_IN_FIELDS, | ||
| MAX_PACKED_BYTECODE_SIZE_PER_UNCONSTRAINED_FUNCTION_IN_FIELDS, | ||
| MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS, REGISTERER_CONTRACT_BYTECODE_CAPSULE_SLOT, | ||
| }, | ||
| contract_class_id::ContractClassId, | ||
| hash::poseidon2_hash_subarray_variable, | ||
| traits::is_empty, | ||
| utils::arrays::{array_concat, find_index_hint_from_end}, | ||
| }; | ||
|
|
||
| use dep::aztec::{ | ||
|
|
@@ -217,13 +220,19 @@ pub contract ContractClassRegisterer { | |
| // cannot prove non-registration. Therefore, it is possible that a malicious oracle might prevent sequencers | ||
| // from including transactions with calls to certain badly-broadcasted contracts. | ||
| // TODO(#8978): review correctness | ||
| let log_hash = | ||
| unsafe { emit_contract_class_unencrypted_log_private(contract_address, log, counter) }; | ||
| let log_to_emit: [Field; CONTRACT_CLASS_LOG_DATA_SIZE_IN_FIELDS] = | ||
| array_concat(log, [0; CONTRACT_CLASS_LOG_DATA_SIZE_IN_FIELDS - N]); | ||
| // Safety: The below length is constrained in the base rollup. | ||
| let index = unsafe { find_index_hint_from_end(log_to_emit, |elem| !is_empty(elem)) }; | ||
| let length = if index == N { 0 } else { index + 1 }; | ||
| // TODO: We will eventually remove this hash. | ||
| let log_hash = poseidon2_hash_subarray_variable(log_to_emit, length); | ||
| // Safety: the below only exists to broadcast the raw log, so we can provide it to the base rollup later to be constrained. | ||
| unsafe { | ||
| emit_contract_class_unencrypted_log_private(contract_address, log_to_emit, counter); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Forewarning, I still don't really know my way around the codebase or our naming conventions.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've not really touched the oracle that much, but since we actually add state in ts which is different to the state tracked here (raw log vs hashed log), maybe emit still stands. I think the notify is different but unsure? |
||
| } | ||
|
|
||
| // 40 = addr (32) + raw log len (4) + processed log len (4) | ||
| context.contract_class_logs_hashes.push( | ||
| LogHash { value: log_hash, counter, length: 40 + (N as Field) * 32 }, | ||
| ); | ||
| context.contract_class_logs_hashes.push(LogHash { value: log_hash, counter, length }); | ||
| } | ||
|
|
||
| #[private] | ||
|
|
||
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.
I think some of this comment is out of date now, since your special poseidon2_hash_subarray_variable function is able to hash the whole thing in-circuit, so the purpose of the oracle is now just to inform typescript, rather than its previous purpose as a hack to compute the log_hash.
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.
True, I was hoping the reviewer would give me some guidance on this. If we move the hash to the oracle / unconstrained as planned this comment is still correct isn't it? Shall I change it then change it back?