feat: Restore hashing args via slice for performance (#5539)

I replaced the ArgsHasher struct with a BoundedVec because a noir version completely separated slices and generic arrays in the frontend, so we couldn't pass it to the hash_args fn that needed an array. This makes unconstrained fns slower since they have to allocate in memory the full max length that the arguments could have. In this PR I instead move to a full slice approach, where hash_args takes a slice and hash_args_array just casts it to a slice. This avoids allocating memory unnecessarily in public functions.
AztecProtocol · Apr 3, 2024 · eb3acdf · eb3acdf
1 parent e928c33
commit eb3acdf
Show file tree

Hide file tree

Showing 9 changed files with 173 additions and 136 deletions.
diff --git a/noir-projects/aztec-nr/authwit/src/auth.nr b/noir-projects/aztec-nr/authwit/src/auth.nr
@@ -1,9 +1,8 @@
 use dep::aztec::protocol_types::{
     abis::function_selector::FunctionSelector, address::AztecAddress,
-    constants::{GENERATOR_INDEX__AUTHWIT_INNER, GENERATOR_INDEX__AUTHWIT_OUTER},
-    hash::{hash_args_array, pedersen_hash}
+    constants::{GENERATOR_INDEX__AUTHWIT_INNER, GENERATOR_INDEX__AUTHWIT_OUTER}, hash::pedersen_hash
 };
-use dep::aztec::context::{PrivateContext, PublicContext, Context};
+use dep::aztec::{context::{PrivateContext, PublicContext, Context}, hash::hash_args_array};
 
 global IS_VALID_SELECTOR = 0xabf64ad4; // 4 first bytes of keccak256("IS_VALID()")
 

diff --git a/noir-projects/aztec-nr/aztec/src/context/private_context.nr b/noir-projects/aztec-nr/aztec/src/context/private_context.nr
@@ -1,6 +1,7 @@
 use crate::{
     context::{inputs::PrivateContextInputs, interface::ContextInterface},
     key::nullifier_key::validate_nullifier_key_against_address, messaging::process_l1_to_l2_message,
+    hash::hash_args_array,
     oracle::{
     arguments, call_private_function::call_private_function_internal,
     enqueue_public_function_call::enqueue_public_function_call_internal, context::get_portal_address,
@@ -27,7 +28,7 @@ use dep::protocol_types::{
     RETURN_VALUES_LENGTH
 },
     contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
-    grumpkin_private_key::GrumpkinPrivateKey, hash::hash_args_array, header::Header,
+    grumpkin_private_key::GrumpkinPrivateKey, header::Header,
     messaging::l2_to_l1_message::L2ToL1Message, utils::reader::Reader, traits::is_empty
 };
 

diff --git a/noir-projects/aztec-nr/aztec/src/context/public_context.nr b/noir-projects/aztec-nr/aztec/src/context/public_context.nr
@@ -1,6 +1,7 @@
 use crate::{
     context::{inputs::PublicContextInputs, interface::ContextInterface, interface::PublicContextInterface},
-    messaging::process_l1_to_l2_message, oracle::{arguments, public_call::call_public_function_internal}
+    messaging::process_l1_to_l2_message,
+    oracle::{arguments, public_call::call_public_function_internal}, hash::hash_args_array
 };
 use dep::protocol_types::{
     abis::{
@@ -18,9 +19,8 @@ use dep::protocol_types::{
     MAX_NULLIFIER_READ_REQUESTS_PER_CALL, MAX_NULLIFIER_NON_EXISTENT_READ_REQUESTS_PER_CALL,
     RETURN_VALUES_LENGTH
 },
-    contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest},
-    hash::hash_args_array, header::Header, messaging::l2_to_l1_message::L2ToL1Message,
-    utils::reader::Reader
+    contrakt::{storage_read::StorageRead, storage_update_request::StorageUpdateRequest}, header::Header,
+    messaging::l2_to_l1_message::L2ToL1Message, utils::reader::Reader
 };
 
 struct PublicContext {

diff --git a/noir-projects/aztec-nr/aztec/src/hash.nr b/noir-projects/aztec-nr/aztec/src/hash.nr
@@ -1,10 +1,13 @@
 use dep::protocol_types::{
     address::{AztecAddress, EthAddress},
-    constants::{GENERATOR_INDEX__L1_TO_L2_MESSAGE_SECRET, GENERATOR_INDEX__NULLIFIER},
-    hash::{pedersen_hash, silo_nullifier}
+    constants::{
+    GENERATOR_INDEX__L1_TO_L2_MESSAGE_SECRET, GENERATOR_INDEX__NULLIFIER, ARGS_HASH_CHUNK_COUNT,
+    GENERATOR_INDEX__FUNCTION_ARGS, ARGS_HASH_CHUNK_LENGTH
+},
+    traits::Hash, hash::{pedersen_hash, silo_nullifier}
 };
 
-use dep::protocol_types::hash::{hash_args, hash_args_array, sha256_to_field};
+use dep::protocol_types::hash::sha256_to_field;
 
 pub fn compute_secret_hash(secret: Field) -> Field {
     // TODO(#1205) This is probably not the right index to use
@@ -51,3 +54,67 @@ pub fn compute_message_nullifier(message_hash: Field, secret: Field, leaf_index:
 pub fn compute_siloed_nullifier(address: AztecAddress, nullifier: Field) -> Field {
     silo_nullifier(address, nullifier)
 }
+
+struct ArgsHasher {
+    fields: [Field],
+}
+
+impl Hash for ArgsHasher {
+    fn hash(self) -> Field {
+        hash_args(self.fields)
+    }
+}
+
+impl ArgsHasher {
+    pub fn new() -> Self {
+        Self { fields: [] }
+    }
+
+    pub fn add(&mut self, field: Field) {
+        self.fields = self.fields.push_back(field);
+    }
+
+    pub fn add_multiple<N>(&mut self, fields: [Field; N]) {
+        for i in 0..N {
+            self.fields = self.fields.push_back(fields[i]);
+        }
+    }
+}
+
+pub fn hash_args_array<N>(args: [Field; N]) -> Field {
+    hash_args(args.as_slice())
+}
+
+pub fn hash_args(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)
+    }
+}
+
+#[test]
+fn compute_var_args_hash() {
+    let mut input = ArgsHasher::new();
+    for i in 0..800 {
+        input.add(i as Field);
+    }
+    let hash = input.hash();
+    assert(hash == 0x05a1023fef839ac88731f49ae983e172c1b600a3c8f3393ad0ac25d819ac0f0f);
+}
diff --git a/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr b/noir-projects/noir-contracts/contracts/docs_example_contract/src/main.nr
@@ -281,14 +281,14 @@ contract DocsExample {
         // ************************************************************
         // The hasher is a structure used to generate a hash of the circuits inputs. 
         // docs:start:context-example-hasher
-        let mut serialized_args = BoundedVec::new();
-        serialized_args.push(a);
-        serialized_args.push(b);
+        let mut args_hasher = dep::aztec::hash::ArgsHasher::new();
+        args_hasher.add(a);
+        args_hasher.add(b);
         // docs:end:context-example-hasher
 
         // The context object is created with the inputs and the hash of the inputs
         // docs:start:context-example-context
-        let mut context = PrivateContext::new(inputs, hash_args(serialized_args));
+        let mut context = PrivateContext::new(inputs, args_hasher.hash());
         // docs:end:context-example-context
 
         // docs:start:storage-example-context

diff --git a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr
@@ -7,15 +7,15 @@ contract Test {
 
     use dep::aztec::protocol_types::{
         abis::private_circuit_public_inputs::PrivateCircuitPublicInputs,
-        constants::{MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, MAX_NOTES_PER_PAGE}, hash::hash_args
+        constants::{MAX_NOTE_HASH_READ_REQUESTS_PER_CALL, MAX_NOTES_PER_PAGE}
     };
     // docs:start:unencrypted_import
     use dep::aztec::prelude::emit_unencrypted_log;
     // docs:end:unencrypted_import
 
     use dep::aztec::{
         context::{Context, inputs::private_context_inputs::PrivateContextInputs},
-        hash::{pedersen_hash, compute_secret_hash},
+        hash::{pedersen_hash, compute_secret_hash, ArgsHasher},
         note::{
         lifecycle::{create_note, destroy_note}, note_getter::{get_notes, view_notes},
         note_getter_options::NoteStatus
@@ -192,22 +192,22 @@ contract Test {
         a_struct: DummyNote,
         a_deep_struct: DeepStruct
     ) -> distinct pub PrivateCircuitPublicInputs {
-        let mut args = BoundedVec::new();
-        args.push(a_field);
-        args.push(a_bool as Field);
-        args.push(a_number as Field);
-        args.extend_from_array(an_array);
-        args.push(a_struct.amount);
-        args.push(a_struct.secret_hash);
-        args.push(a_deep_struct.a_field);
-        args.push(a_deep_struct.a_bool as Field);
-        args.push(a_deep_struct.a_note.amount);
-        args.push(a_deep_struct.a_note.secret_hash);
+        let mut args = ArgsHasher::new();
+        args.add(a_field);
+        args.add(a_bool as Field);
+        args.add(a_number as Field);
+        args.add_multiple(an_array);
+        args.add(a_struct.amount);
+        args.add(a_struct.secret_hash);
+        args.add(a_deep_struct.a_field);
+        args.add(a_deep_struct.a_bool as Field);
+        args.add(a_deep_struct.a_note.amount);
+        args.add(a_deep_struct.a_note.secret_hash);
         for note in a_deep_struct.many_notes {
-            args.push(note.amount);
-            args.push(note.secret_hash);
+            args.add(note.amount);
+            args.add(note.secret_hash);
         }
-        let args_hash = hash_args(args);
+        let args_hash = args.hash();
         let mut context = PrivateContext::new(inputs, args_hash);
         context.return_values.push(args_hash);
         context.finish()

diff --git a/noir-projects/noir-protocol-circuits/crates/types/src/hash.nr b/noir-projects/noir-protocol-circuits/crates/types/src/hash.nr
@@ -6,14 +6,14 @@ use crate::contract_class_id::ContractClassId;
 use crate::abis::side_effect::SideEffect;
 use crate::utils::{uint256::U256, field::field_from_bytes_32_trunc};
 use crate::constants::{
-    ARGS_HASH_CHUNK_COUNT, ARGS_HASH_CHUNK_LENGTH, MAX_ARGS_LENGTH, FUNCTION_TREE_HEIGHT,
-    GENERATOR_INDEX__SILOED_NOTE_HASH, GENERATOR_INDEX__OUTER_NULLIFIER, GENERATOR_INDEX__VK,
-    GENERATOR_INDEX__CONSTRUCTOR, GENERATOR_INDEX__PARTIAL_ADDRESS, GENERATOR_INDEX__CONTRACT_ADDRESS,
-    GENERATOR_INDEX__NOTE_HASH_NONCE, GENERATOR_INDEX__UNIQUE_NOTE_HASH, GENERATOR_INDEX__FUNCTION_ARGS
+    FUNCTION_TREE_HEIGHT, GENERATOR_INDEX__SILOED_NOTE_HASH, GENERATOR_INDEX__OUTER_NULLIFIER,
+    GENERATOR_INDEX__VK, GENERATOR_INDEX__CONSTRUCTOR, GENERATOR_INDEX__PARTIAL_ADDRESS,
+    GENERATOR_INDEX__CONTRACT_ADDRESS, GENERATOR_INDEX__NOTE_HASH_NONCE,
+    GENERATOR_INDEX__UNIQUE_NOTE_HASH
 };
+use crate::traits::Hash;
 use crate::messaging::l2_to_l1_message::L2ToL1Message;
 use crate::merkle_tree::root::root_from_sibling_path;
-
 use dep::std::hash::{pedersen_hash_with_separator, sha256};
 
 pub fn sha256_to_field<N>(bytes_to_hash: [u8; N]) -> Field {
@@ -23,36 +23,6 @@ pub fn sha256_to_field<N>(bytes_to_hash: [u8; N]) -> Field {
     hash_in_a_field
 }
 
-pub fn hash_args_array<N>(args: [Field; N]) -> Field {
-    let mut args_vec = BoundedVec::new();
-    args_vec.extend_from_array(args);
-    hash_args(args_vec)
-}
-
-pub fn hash_args(args: BoundedVec<Field, MAX_ARGS_LENGTH>) -> 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.get_unchecked(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 private_functions_root_from_siblings(
     selector: FunctionSelector,
     vk_hash: Field,
@@ -224,16 +194,6 @@ fn smoke_sha256_to_field() {
     assert(mod_res == 0x448ebbc9e1a31220a2f3830c18eef61b9bd070e5084b7fa2a359fe729184e0);
 }
 
-#[test]
-fn compute_var_args_hash() {
-    let mut input = BoundedVec::new();
-    for i in 0..800 {
-        input.push(i as Field);
-    }
-    let hash = hash_args(input);
-    assert(hash == 0x05a1023fef839ac88731f49ae983e172c1b600a3c8f3393ad0ac25d819ac0f0f);
-}
-
 #[test]
 fn compute_l2_l1_hash() {
     // All zeroes

diff --git a/...ts/noir-protocol-circuits/crates/types/src/tests/private_circuit_public_inputs_builder.nr b/...ts/noir-protocol-circuits/crates/types/src/tests/private_circuit_public_inputs_builder.nr
@@ -5,7 +5,7 @@ use crate::{
     private_circuit_public_inputs::PrivateCircuitPublicInputs, read_request::ReadRequest,
     side_effect::{SideEffect, SideEffectLinkedToNoteHash}
 },
-    address::{AztecAddress, compute_initialization_hash}, hash::hash_args_array, header::Header,
+    address::{AztecAddress, compute_initialization_hash}, header::Header,
     messaging::l2_to_l1_message::L2ToL1Message, tests::fixtures
 };
 use crate::constants::{
@@ -52,7 +52,7 @@ impl PrivateCircuitPublicInputsBuilder {
     pub fn new() -> Self {
         let mut public_inputs: PrivateCircuitPublicInputsBuilder = dep::std::unsafe::zeroed();
 
-        let args_hash = hash_args_array([]);
+        let args_hash = 0;
 
         let contract_data = fixtures::contracts::default_contract;
         let portal_contract_address = contract_data.portal_contract_address;