Skip to content
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.

Sign up for GitHub

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

chore: Use slices for computing args hash in class registerer [WIP] #5410

Closed
wants to merge 3 commits into from
Closed

chore: Use slices for computing args hash in class registerer [WIP] #5410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76bbd0b
chore: Use slices for computing args hash in class registerer
spalladino Mar 22, 2024
b21c275
Remove hash_args from the equation
spalladino Mar 25, 2024
9fbbef4
Add hash_args_slice
spalladino Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions noir-projects/noir-contracts/contracts/contract_class_registerer_contract/src/main.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ mod events;
mod capsule;

contract ContractClassRegisterer {
use dep::aztec::context::PrivateContext;
use dep::aztec::prelude::{AztecAddress, EthAddress, FunctionSelector, emit_unencrypted_log};
use dep::aztec::protocol_types::{
contract_class_id::ContractClassId,
hash::hash_args_slice, contract_class_id::ContractClassId,
constants::{
ARTIFACT_FUNCTION_TREE_MAX_HEIGHT, FUNCTION_TREE_HEIGHT,
MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS, REGISTERER_CONTRACT_CLASS_REGISTERED_MAGIC_VALUE
Expand All @@ -23,12 +24,24 @@ contract ContractClassRegisterer {
use crate::capsule::pop_capsule;

#[aztec(private)]
fn register(artifact_hash: Field, private_functions_root: Field, public_bytecode_commitment: Field) {
#[aztec(nocontextcreation)]
fn register(
artifact_hash: Field,
private_functions_root: Field,
public_bytecode_commitment: Field,
packed_public_bytecode: [Field; MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS]
) {
// We manually create the context so we can manually hash the args using the bytecode array as slice
// See https://github.com/noir-lang/noir/issues/4395#issuecomment-2007694169 for context
let mut hasher_slice = packed_public_bytecode.as_slice();
hasher_slice = hasher_slice.push_front(public_bytecode_commitment);
hasher_slice = hasher_slice.push_front(private_functions_root);
hasher_slice = hasher_slice.push_front(artifact_hash);
let mut context = PrivateContext::new(inputs, hash_args_slice(hasher_slice));

// TODO: Validate public_bytecode_commitment is the correct commitment of packed_public_bytecode
// TODO: Validate packed_public_bytecode is legit public bytecode

let packed_public_bytecode: [Field; MAX_PACKED_PUBLIC_BYTECODE_SIZE_IN_FIELDS] = pop_capsule();

// Compute contract class id from preimage
let contract_class_id = ContractClassId::compute(
artifact_hash,
Expand Down
24 changes: 24 additions & 0 deletions noir-projects/noir-protocol-circuits/crates/types/src/hash.nr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,30 @@ pub fn hash_args_array<N>(args: [Field; N]) -> Field {
hash_args(args_vec)
}

pub fn hash_args_slice(args: [Field]) -> Field {
if args.len() == 0 {
0
} else {
let mut chunks_hashes = [0; ARGS_HASH_CHUNK_COUNT];
for i in 0..ARGS_HASH_CHUNK_COUNT {
let mut chunk_hash = 0;
let start_chunk_index = i * ARGS_HASH_CHUNK_LENGTH;
if start_chunk_index < args.len() {
let mut chunk_args = [0; ARGS_HASH_CHUNK_LENGTH];
for j in 0..ARGS_HASH_CHUNK_LENGTH {
let item_index = i * ARGS_HASH_CHUNK_LENGTH + j;
if item_index < args.len() {
chunk_args[j] = args[item_index];
}
}
chunk_hash = pedersen_hash(chunk_args, GENERATOR_INDEX__FUNCTION_ARGS);
}
chunks_hashes[i] = chunk_hash;
}
pedersen_hash(chunks_hashes, GENERATOR_INDEX__FUNCTION_ARGS)
}
}

pub fn hash_args(args: BoundedVec<Field, MAX_ARGS_LENGTH>) -> Field {
if args.len() == 0 {
0
Expand Down
4 changes: 4 additions & 0 deletions noir/noir-repo/aztec_macros/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ fn transform_module(module: &mut SortedModule) -> Result<bool, AztecMacroError>
let mut is_initializer = false;
let mut is_internal = false;
let mut insert_init_check = has_initializer;
let mut insert_context_creation = true;

for secondary_attribute in func.def.attributes.secondary.clone() {
if is_custom_attribute(&secondary_attribute, "aztec(private)") {
Expand All @@ -134,6 +135,8 @@ fn transform_module(module: &mut SortedModule) -> Result<bool, AztecMacroError>
is_public = true;
} else if is_custom_attribute(&secondary_attribute, "aztec(public-vm)") {
is_public_vm = true;
} else if is_custom_attribute(&secondary_attribute, "aztec(nocontextcreation)") {
insert_context_creation = false;
}
}

Expand All @@ -146,6 +149,7 @@ fn transform_module(module: &mut SortedModule) -> Result<bool, AztecMacroError>
is_initializer,
insert_init_check,
is_internal,
insert_context_creation,
)?;
has_transformed_module = true;
} else if is_public_vm {
Expand Down
7 changes: 5 additions & 2 deletions noir/noir-repo/aztec_macros/src/transforms/functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub fn transform_function(
is_initializer: bool,
insert_init_check: bool,
is_internal: bool,
insert_context_creation: bool,
Comment on lines 31 to +34
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated, but I really dislike passing multiple bools as args. What's the rustacean way of properly doing this? Using an "options" struct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An options struct (which potentially implements Default) and/or you could have another version of the function with the most common values for those bools as defaults. There's also the argument that stronger types should be preferred over booleans, e.g:

transform_function(
    "foo",
    StorageOpt::Defined,
    InitializerOpt::None,
    InitCheckOpt::None,
    IsInternal::Yes,
    ContextOpt::InsertContextCreation,
);

I find this a bit verbose though.

) -> Result<(), AztecMacroError> {
let context_name = format!("{}Context", ty);
let inputs_name = format!("{}ContextInputs", ty);
Expand Down Expand Up @@ -60,8 +61,10 @@ pub fn transform_function(
}

// Insert the context creation as the first action
let create_context = create_context(&context_name, &func.def.parameters)?;
func.def.body.statements.splice(0..0, (create_context).iter().cloned());
if insert_context_creation {
let create_context = create_context(&context_name, &func.def.parameters)?;
func.def.body.statements.splice(0..0, (create_context).iter().cloned());
}

// Add the inputs to the params
let input = create_inputs(&inputs_name);
Expand Down
Loading