From 60b59daca5957d0b1b0ec4b727f1328247d37d4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81lvaro=20Rodr=C3=ADguez?= Date: Wed, 10 Jan 2024 17:13:46 +0100 Subject: [PATCH] refactor: Cleanup duplicated methods for structs after traits (#3912) After adding the Empty and Eq trait, we can remove duplicated methods for fields and structs. --- .../crates/private-kernel-lib/src/common.nr | 48 +++----- .../src/private_kernel_init.nr | 20 ++-- .../src/private_kernel_inner.nr | 19 ++- .../src/private_kernel_ordering.nr | 100 +++------------- .../crates/public-kernel-lib/src/common.nr | 35 +++--- .../src/public_kernel_public_previous.nr | 20 +--- .../src/crates/public-kernel-lib/src/utils.nr | 38 +----- .../src/crates/types/src/traits.nr | 26 +++++ .../src/crates/types/src/utils/arrays.nr | 110 ++---------------- 9 files changed, 107 insertions(+), 309 deletions(-) diff --git a/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/common.nr b/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/common.nr index 069a232fd53..82b33510291 100644 --- a/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/common.nr +++ b/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/common.nr @@ -37,14 +37,11 @@ use dep::types::{ arrays::{ array_length, array_to_bounded_vec, - is_empty_array, - is_empty_struct_array, - struct_array_to_bounded_vec, validate_array, - validate_struct_array, }, bounded_vec::BoundedVec, }, + traits::is_empty_array, }; pub fn validate_arrays(app_public_inputs: PrivateCircuitPublicInputs) { @@ -53,15 +50,9 @@ pub fn validate_arrays(app_public_inputs: PrivateCircuitPublicInputs) { // to push_array_to_array() routines which rely on the passed arrays to be well-formed. validate_array(app_public_inputs.return_values); - validate_struct_array(app_public_inputs.read_requests, |r: SideEffect| r.is_empty()); - validate_struct_array( - app_public_inputs.new_commitments, - |c: SideEffect| c.is_empty() - ); - validate_struct_array( - app_public_inputs.new_nullifiers, - |n: SideEffectLinkedToNoteHash| n.is_empty() - ); + validate_array(app_public_inputs.read_requests); + validate_array(app_public_inputs.new_commitments); + validate_array(app_public_inputs.new_nullifiers); validate_array(app_public_inputs.private_call_stack_hashes); validate_array(app_public_inputs.public_call_stack_hashes); validate_array(app_public_inputs.new_l2_to_l1_msgs); @@ -114,13 +105,13 @@ pub fn initialize_end_values( // functions within this circuit: let start = previous_kernel.public_inputs.end; - public_inputs.end.read_requests = struct_array_to_bounded_vec(start.read_requests, |c: SideEffect| c.is_empty(), SideEffect::empty()); + public_inputs.end.read_requests = array_to_bounded_vec(start.read_requests); - public_inputs.end.new_commitments = struct_array_to_bounded_vec(start.new_commitments, |c: SideEffect| c.is_empty(), SideEffect::empty()); - public_inputs.end.new_nullifiers = struct_array_to_bounded_vec(start.new_nullifiers, |c: SideEffectLinkedToNoteHash| c.is_empty(), SideEffectLinkedToNoteHash::empty()); + public_inputs.end.new_commitments = array_to_bounded_vec(start.new_commitments); + public_inputs.end.new_nullifiers = array_to_bounded_vec(start.new_nullifiers); - public_inputs.end.private_call_stack = struct_array_to_bounded_vec(start.private_call_stack, |c: CallRequest| c.is_empty(), CallRequest::empty()); - public_inputs.end.public_call_stack = struct_array_to_bounded_vec(start.public_call_stack, |c: CallRequest| c.is_empty(), CallRequest::empty()); + public_inputs.end.private_call_stack = array_to_bounded_vec(start.private_call_stack); + public_inputs.end.public_call_stack = array_to_bounded_vec(start.public_call_stack); public_inputs.end.new_l2_to_l1_msgs = array_to_bounded_vec(start.new_l2_to_l1_msgs); public_inputs.end.encrypted_logs_hash = start.encrypted_logs_hash; @@ -130,7 +121,7 @@ pub fn initialize_end_values( public_inputs.end.unencrypted_log_preimages_length = start.unencrypted_log_preimages_length; public_inputs.end.optionally_revealed_data = start.optionally_revealed_data; - public_inputs.end.new_contracts = struct_array_to_bounded_vec(start.new_contracts, |ncd: NewContractData| ncd.is_empty(), NewContractData::empty()); + public_inputs.end.new_contracts = array_to_bounded_vec(start.new_contracts); } fn perform_static_call_checks(private_call: PrivateCallData) { @@ -139,13 +130,10 @@ fn perform_static_call_checks(private_call: PrivateCallData) { if is_static_call { // No state changes are allowed for static calls: assert( - is_empty_struct_array(public_inputs.new_commitments, |c: SideEffect| c.is_empty()), "new_commitments must be empty for static calls" + is_empty_array(public_inputs.new_commitments), "new_commitments must be empty for static calls" ); assert( - is_empty_struct_array( - public_inputs.new_nullifiers, - |n: SideEffectLinkedToNoteHash| n.is_empty() - ), "new_nullifiers must be empty for static calls" + is_empty_array(public_inputs.new_nullifiers), "new_nullifiers must be empty for static calls" ); } } @@ -241,11 +229,7 @@ pub fn update_end_values(private_call: PrivateCallData, public_inputs: &mut Kern // Call stacks // Private call stack. - let private_call_stack = struct_array_to_bounded_vec( - private_call.private_call_stack, - |c: CallRequest| c.is_empty(), - CallRequest::empty() - ); + let private_call_stack = array_to_bounded_vec(private_call.private_call_stack); validate_call_requests( private_call_stack, private_call_public_inputs.private_call_stack_hashes, @@ -253,11 +237,7 @@ pub fn update_end_values(private_call: PrivateCallData, public_inputs: &mut Kern ); public_inputs.end.private_call_stack.push_vec(private_call_stack); // Public call stack. - let public_call_stack = struct_array_to_bounded_vec( - private_call.public_call_stack, - |c: CallRequest| c.is_empty(), - CallRequest::empty() - ); + let public_call_stack = array_to_bounded_vec(private_call.public_call_stack); validate_call_requests( public_call_stack, private_call_public_inputs.public_call_stack_hashes, diff --git a/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_init.nr b/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_init.nr index 2a7d6decb5e..5b215d82971 100644 --- a/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_init.nr +++ b/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_init.nr @@ -9,7 +9,7 @@ use dep::types::{ }, mocked::{Proof, verify_previous_kernel_state}, transaction::request::TxRequest, - utils::arrays::{is_empty_array, is_empty_struct_array}, + traits::is_empty_array, }; // Initialization struct for private inputs to the private kernel @@ -135,7 +135,7 @@ mod tests { }, tests::private_call_data_builder::PrivateCallDataBuilder, transaction::request::TxRequest, - utils::arrays::{array_length, struct_array_length}, + utils::arrays::array_length, }; struct PrivateKernelInitInputsBuilder { @@ -520,7 +520,7 @@ mod tests { assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash()); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 0); + assert_eq(array_length(public_inputs.end.read_requests), 0); } #[test] @@ -536,7 +536,7 @@ mod tests { assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash()); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 0); + assert_eq(array_length(public_inputs.end.read_requests), 0); } #[test] @@ -552,7 +552,7 @@ mod tests { assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash()); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 0); + assert_eq(array_length(public_inputs.end.read_requests), 0); } #[test] @@ -568,7 +568,7 @@ mod tests { assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash()); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 0); + assert_eq(array_length(public_inputs.end.read_requests), 0); } #[test] @@ -584,7 +584,7 @@ mod tests { assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash()); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 1); + assert_eq(array_length(public_inputs.end.read_requests), 1); } #[test] @@ -601,7 +601,7 @@ mod tests { assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash()); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 1); + assert_eq(array_length(public_inputs.end.read_requests), 1); } #[test] @@ -617,8 +617,6 @@ mod tests { assert_eq(public_inputs.end.new_nullifiers[0].value, builder.tx_request.hash()); // non-transient read requests are NOT forwarded - assert_eq( - struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), MAX_READ_REQUESTS_PER_CALL - ); + assert_eq(array_length(public_inputs.end.read_requests), MAX_READ_REQUESTS_PER_CALL); } } diff --git a/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_inner.nr b/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_inner.nr index de3b83f9534..ed1fbbb411e 100644 --- a/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_inner.nr +++ b/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_inner.nr @@ -8,7 +8,6 @@ use dep::types::{ side_effect::{SideEffect, SideEffectLinkedToNoteHash}, }, mocked::verify_previous_kernel_state, - utils::arrays::{array_length, struct_array_length}, }; struct PrivateKernelInputsInner { @@ -97,7 +96,7 @@ mod tests { address::AztecAddress, hash::compute_logs_hash, utils::{ - arrays::{array_length, struct_array_length}, + arrays::array_length, bounded_vec::BoundedVec, }, }; @@ -626,7 +625,7 @@ mod tests { let public_inputs = builder.execute(); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 0); + assert_eq(array_length(public_inputs.end.read_requests), 0); } #[test] @@ -638,7 +637,7 @@ mod tests { let public_inputs = builder.execute(); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 0); + assert_eq(array_length(public_inputs.end.read_requests), 0); } #[test] @@ -650,7 +649,7 @@ mod tests { let public_inputs = builder.execute(); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 0); + assert_eq(array_length(public_inputs.end.read_requests), 0); } #[test] @@ -662,7 +661,7 @@ mod tests { let public_inputs = builder.execute(); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 0); + assert_eq(array_length(public_inputs.end.read_requests), 0); } #[test] @@ -674,7 +673,7 @@ mod tests { let public_inputs = builder.execute(); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 1); + assert_eq(array_length(public_inputs.end.read_requests), 1); } #[test] @@ -688,7 +687,7 @@ mod tests { let public_inputs = builder.execute(); // non-transient read requests are NOT forwarded - assert_eq(struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), 1); + assert_eq(array_length(public_inputs.end.read_requests), 1); } #[test] @@ -700,9 +699,7 @@ mod tests { let public_inputs = builder.execute(); // non-transient read requests are NOT forwarded - assert_eq( - struct_array_length(public_inputs.end.read_requests, |r: SideEffect| r.is_empty()), MAX_READ_REQUESTS_PER_CALL - ); + assert_eq(array_length(public_inputs.end.read_requests), MAX_READ_REQUESTS_PER_CALL); } #[test] diff --git a/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_ordering.nr b/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_ordering.nr index db8a5144533..ebaca1fd70c 100644 --- a/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_ordering.nr +++ b/yarn-project/noir-protocol-circuits/src/crates/private-kernel-lib/src/private_kernel_ordering.nr @@ -20,9 +20,10 @@ use dep::types::{ compute_unique_siloed_commitment, }, utils::{ - arrays::{array_length, struct_array_length, struct_array_eq, is_empty_struct_array}, + arrays::{array_length, array_eq}, bounded_vec::BoundedVec, }, + traits::is_empty_array, }; struct PrivateKernelInputsOrdering { @@ -33,7 +34,7 @@ struct PrivateKernelInputsOrdering { impl PrivateKernelInputsOrdering { fn validate_inputs(self) { - assert_eq(struct_array_length(self.previous_kernel.public_inputs.end.private_call_stack, |c: CallRequest| c.is_empty()), 0, + assert_eq(array_length(self.previous_kernel.public_inputs.end.private_call_stack), 0, "Private call stack must be empty when executing the ordering circuit"); } @@ -173,9 +174,10 @@ mod tests { hash::compute_unique_siloed_commitments, tests::previous_kernel_data_builder::PreviousKernelDataBuilder, utils::{ - arrays::{array_eq, struct_array_eq, array_length, struct_array_length, is_empty_array, is_empty_struct_array}, + arrays::{array_eq, array_length}, bounded_vec::BoundedVec, }, + traits::is_empty_array, }; struct PrivateKernelOrderingInputsBuilder { @@ -255,13 +257,7 @@ mod tests { let unique_siloed_commitments = builder.get_unique_siloed_commitments(); let public_inputs = builder.execute(); - assert( - struct_array_length( - public_inputs.end.new_commitments, - |r: SideEffect| r.is_empty() - ) - == 1 - ); + assert(array_length(public_inputs.end.new_commitments) == 1); assert(public_inputs.end.new_commitments[0].eq(unique_siloed_commitments[0])); } @@ -278,12 +274,7 @@ mod tests { let unique_siloed_commitments = builder.get_unique_siloed_commitments(); let public_inputs = builder.execute(); - assert_eq( - struct_array_length( - public_inputs.end.new_commitments, - |r: SideEffect| r.is_empty() - ), MAX_NEW_COMMITMENTS_PER_TX - ); + assert_eq(array_length(public_inputs.end.new_commitments), MAX_NEW_COMMITMENTS_PER_TX); for i in 0..MAX_NEW_COMMITMENTS_PER_TX { assert(public_inputs.end.new_commitments[i].eq(unique_siloed_commitments[i])); } @@ -313,23 +304,11 @@ mod tests { let new_nullifiers = builder.get_new_nullifiers(); let public_inputs = builder.execute(); - assert( - is_empty_struct_array( - public_inputs.end.new_commitments, - |c: SideEffect| c.is_empty() - ) - ); + assert(is_empty_array(public_inputs.end.new_commitments)); // The nullifier at index 1 is chopped. let expected_new_nullifiers = [new_nullifiers[0], new_nullifiers[2]]; - assert( - struct_array_eq( - public_inputs.end.new_nullifiers, - expected_new_nullifiers, - |a: SideEffectLinkedToNoteHash, b: SideEffectLinkedToNoteHash| a.eq(b), - |a: SideEffectLinkedToNoteHash| a.is_empty() - ) - ); + assert(array_eq(public_inputs.end.new_nullifiers, expected_new_nullifiers)); } #[test] @@ -349,24 +328,15 @@ mod tests { let public_inputs = builder.execute(); assert( - struct_array_eq( + array_eq( public_inputs.end.new_commitments, - [unique_siloed_commitments[0]], - |a: SideEffect, b: SideEffect| a.eq(b), - |a: SideEffect| a.is_empty() + [unique_siloed_commitments[0]] ) ); // The nullifier at index 1 is chopped. let expected_new_nullifiers = [new_nullifiers[0], new_nullifiers[2]]; - assert( - struct_array_eq( - public_inputs.end.new_nullifiers, - expected_new_nullifiers, - |a: SideEffectLinkedToNoteHash, b: SideEffectLinkedToNoteHash| a.eq(b), - |a: SideEffectLinkedToNoteHash| a.is_empty() - ) - ); + assert(array_eq(public_inputs.end.new_nullifiers, expected_new_nullifiers)); } #[test] @@ -383,20 +353,8 @@ mod tests { let new_nullifiers = builder.get_new_nullifiers(); let public_inputs = builder.execute(); - assert( - is_empty_struct_array( - public_inputs.end.new_commitments, - |c: SideEffect| c.is_empty() - ) - ); - assert( - struct_array_eq( - public_inputs.end.new_nullifiers, - [new_nullifiers[0]], - |a: SideEffectLinkedToNoteHash, b: SideEffectLinkedToNoteHash| a.eq(b), - |a: SideEffectLinkedToNoteHash| a.is_empty() - ) - ); + assert(is_empty_array(public_inputs.end.new_commitments)); + assert(array_eq(public_inputs.end.new_nullifiers, [new_nullifiers[0]])); } #[test] @@ -407,18 +365,8 @@ mod tests { builder.append_nullifiers(2); let public_inputs = builder.execute(); - assert_eq( - struct_array_length( - public_inputs.end.new_commitments, - |r: SideEffect| r.is_empty() - ), 2 - ); - assert_eq( - struct_array_length( - public_inputs.end.new_nullifiers, - |r: SideEffectLinkedToNoteHash| r.is_empty() - ), 3 - ); + assert_eq(array_length(public_inputs.end.new_commitments), 2); + assert_eq(array_length(public_inputs.end.new_nullifiers), 3); } // same as previous test, but this time there are 0 commitments! @@ -430,20 +378,8 @@ mod tests { builder.append_nullifiers(2); let public_inputs = builder.execute(); - assert( - struct_array_length( - public_inputs.end.new_commitments, - |r: SideEffect| r.is_empty() - ) - == 0 - ); - assert( - struct_array_length( - public_inputs.end.new_nullifiers, - |r: SideEffectLinkedToNoteHash| r.is_empty() - ) - == 3 - ); + assert(array_length(public_inputs.end.new_commitments) == 0); + assert(array_length(public_inputs.end.new_nullifiers) == 3); } #[test(should_fail_with="New nullifier is transient but hint is invalid")] diff --git a/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/common.nr b/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/common.nr index 115079acd9a..e38c5f5ae67 100644 --- a/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/common.nr +++ b/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/common.nr @@ -28,10 +28,11 @@ use dep::types::{ }, hash::{silo_commitment, silo_nullifier, compute_l2_to_l1_hash, accumulate_sha256}, utils::{ - arrays::{array_length, array_to_bounded_vec, is_empty_array, struct_array_length, struct_array_to_bounded_vec}, + arrays::{array_length, array_to_bounded_vec}, bounded_vec::BoundedVec, uint128::U128, }, + traits::is_empty_array }; use crate::hash::{compute_public_data_tree_index, compute_public_data_tree_value}; @@ -66,42 +67,36 @@ pub fn initialize_end_values( // functions within this circuit: let start = previous_kernel.public_inputs.end; - circuit_outputs.end.new_commitments = struct_array_to_bounded_vec(start.new_commitments, |c: SideEffect| c.is_empty(), SideEffect::empty()); - circuit_outputs.end.new_nullifiers = struct_array_to_bounded_vec(start.new_nullifiers, |c: SideEffectLinkedToNoteHash| c.is_empty(), SideEffectLinkedToNoteHash::empty()); + circuit_outputs.end.new_commitments = array_to_bounded_vec(start.new_commitments); + circuit_outputs.end.new_nullifiers = array_to_bounded_vec(start.new_nullifiers); - circuit_outputs.end.private_call_stack = struct_array_to_bounded_vec(start.private_call_stack, |c: CallRequest| c.is_empty(), CallRequest::empty()); - circuit_outputs.end.public_call_stack = struct_array_to_bounded_vec(start.public_call_stack, |c: CallRequest| c.is_empty(), CallRequest::empty()); + circuit_outputs.end.private_call_stack = array_to_bounded_vec(start.private_call_stack); + circuit_outputs.end.public_call_stack = array_to_bounded_vec(start.public_call_stack); circuit_outputs.end.new_l2_to_l1_msgs = array_to_bounded_vec(start.new_l2_to_l1_msgs); circuit_outputs.end.optionally_revealed_data = start.optionally_revealed_data; - circuit_outputs.end.public_data_update_requests = struct_array_to_bounded_vec(start.public_data_update_requests, |pdu: PublicDataUpdateRequest| pdu.is_empty(), PublicDataUpdateRequest::empty()); - circuit_outputs.end.public_data_reads = struct_array_to_bounded_vec(start.public_data_reads, |pdr: PublicDataRead| pdr.is_empty(), PublicDataRead::empty()); + circuit_outputs.end.public_data_update_requests = array_to_bounded_vec(start.public_data_update_requests); + circuit_outputs.end.public_data_reads = array_to_bounded_vec(start.public_data_reads); // Public kernel does not modify encrypted logs values --> we just copy them to output circuit_outputs.end.encrypted_logs_hash = start.encrypted_logs_hash; circuit_outputs.end.encrypted_log_preimages_length = start.encrypted_log_preimages_length; - circuit_outputs.end.new_contracts = struct_array_to_bounded_vec(previous_kernel.public_inputs.end.new_contracts, |ncd: NewContractData| ncd.is_empty(), NewContractData::empty()); + circuit_outputs.end.new_contracts = array_to_bounded_vec(previous_kernel.public_inputs.end.new_contracts); } fn perform_static_call_checks(public_call: PublicCallData) { let public_inputs = public_call.call_stack_item.public_inputs; if public_inputs.call_context.is_static_call { // No state changes are allowed for static calls: - let new_commitments_length = struct_array_length(public_inputs.new_commitments, |r: SideEffect| r.is_empty()); + let new_commitments_length = array_length(public_inputs.new_commitments); assert(new_commitments_length == 0, "new_commitments must be empty for static calls"); - let new_nullifiers_length = struct_array_length( - public_inputs.new_nullifiers, - |r: SideEffectLinkedToNoteHash| r.is_empty() - ); + let new_nullifiers_length = array_length(public_inputs.new_nullifiers); assert(new_nullifiers_length == 0, "new_nullifiers must be empty for static calls"); - let update_requests_length = struct_array_length( - public_inputs.contract_storage_update_requests, - |r: StorageUpdateRequest| r.is_empty() - ); + let update_requests_length = array_length(public_inputs.contract_storage_update_requests); assert( update_requests_length == 0, "No contract storage update requests are allowed for static calls" ); @@ -142,11 +137,7 @@ pub fn update_public_end_values(public_call: PublicCallData, circuit_outputs: &m perform_static_call_checks(public_call); // Update public call stack. - let public_call_requests = struct_array_to_bounded_vec( - public_call.public_call_stack, - |c: CallRequest| c.is_empty(), - CallRequest::empty() - ); + let public_call_requests = array_to_bounded_vec(public_call.public_call_stack); let hashes = public_call.call_stack_item.public_inputs.public_call_stack_hashes; validate_call_requests(public_call_requests, hashes, public_call); circuit_outputs.end.public_call_stack.push_vec(public_call_requests); diff --git a/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/public_kernel_public_previous.nr b/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/public_kernel_public_previous.nr index 5e5fc3724a1..985690d46b9 100644 --- a/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/public_kernel_public_previous.nr +++ b/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/public_kernel_public_previous.nr @@ -73,7 +73,7 @@ mod tests { public_call_data_builder::PublicCallDataBuilder, }, utils::{ - arrays::{array_eq, struct_array_eq}, + arrays::array_eq, }, }; use dep::types::constants::{ @@ -188,14 +188,7 @@ mod tests { let public_inputs = builder.execute(); - assert( - struct_array_eq( - public_inputs.end.new_commitments, - new_commitments, - |a: SideEffect, b: SideEffect| a.eq(b), - |a: SideEffect| a.is_empty() - ) - ); + assert(array_eq(public_inputs.end.new_commitments, new_commitments)); } #[test] @@ -260,14 +253,7 @@ mod tests { let public_inputs = builder.execute(); - assert( - struct_array_eq( - public_inputs.end.new_nullifiers, - new_nullifiers, - |a: SideEffectLinkedToNoteHash, b: SideEffectLinkedToNoteHash| a.eq(b), - |a: SideEffectLinkedToNoteHash| a.is_empty() - ) - ); + assert(array_eq(public_inputs.end.new_nullifiers, new_nullifiers)); } #[test] diff --git a/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/utils.nr b/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/utils.nr index 2cdd041eb6f..7c4483d46ca 100644 --- a/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/utils.nr +++ b/yarn-project/noir-protocol-circuits/src/crates/public-kernel-lib/src/utils.nr @@ -12,7 +12,7 @@ use dep::types::{ storage_update_request::StorageUpdateRequest, }, utils::{ - arrays::struct_array_eq, + arrays::array_eq, }, }; @@ -34,14 +34,7 @@ pub fn compute_public_data_reads( } pub fn assert_eq_call_requests(call_requests: [CallRequest; N], expected: [CallRequest; S]) { - assert( - struct_array_eq( - call_requests, - expected, - |c1: CallRequest, c2: CallRequest| c1.eq(c2), - |c: CallRequest| c.is_empty() - ) - ); + assert(array_eq(call_requests, expected)); } pub fn compute_public_data_update_requests( @@ -66,40 +59,19 @@ pub fn assert_eq_public_data_reads( public_data_reads: [PublicDataRead; N], expected: [PublicDataRead; S] ) { - assert( - struct_array_eq( - public_data_reads, - expected, - |pdr1: PublicDataRead, pdr2: PublicDataRead| pdr1.eq(pdr2), - |pdr: PublicDataRead| pdr.is_empty() - ) - ); + assert(array_eq(public_data_reads, expected)); } pub fn assert_eq_public_data_update_requests( public_data_update_requests: [PublicDataUpdateRequest; N], expected: [PublicDataUpdateRequest; S] ) { - assert( - struct_array_eq( - public_data_update_requests, - expected, - |r1: PublicDataUpdateRequest, r2: PublicDataUpdateRequest| r1.eq(r2), - |r: PublicDataUpdateRequest| r.is_empty() - ) - ); + assert(array_eq(public_data_update_requests, expected)); } pub fn assert_eq_new_contracts( new_contracts: [NewContractData; N], expected: [NewContractData; S] ) { - assert( - struct_array_eq( - new_contracts, - expected, - |c1: NewContractData, c2: NewContractData| c1.eq(c2), - |c: NewContractData| c.is_empty() - ) - ); + assert(array_eq(new_contracts, expected)); } diff --git a/yarn-project/noir-protocol-circuits/src/crates/types/src/traits.nr b/yarn-project/noir-protocol-circuits/src/crates/types/src/traits.nr index 03ddb8d9bb8..a5ad3cff772 100644 --- a/yarn-project/noir-protocol-circuits/src/crates/types/src/traits.nr +++ b/yarn-project/noir-protocol-circuits/src/crates/types/src/traits.nr @@ -1,8 +1,34 @@ +// Trait: is_empty +// +// The general is_empty trait checks if a data type is is empty, +// and it defines empty for the basic data types as 0. +// +// If a Field is equal to zero, then it is regarded as zero. +// We will go with this definition for now, however it can be problematic +// if a value can actually be zero. In a future refactor, we can +// use the optional type for safety. Doing it now would lead to a worse devex +// and would make it harder to sync up with the cpp code. // Preferred over Default trait to convey intent, as default doesn't necessarily mean empty. trait Empty { fn empty() -> Self; } +impl Empty for Field { fn empty() -> Self {0} } + +impl Empty for u1 { fn empty() -> Self {0} } +impl Empty for u8 { fn empty() -> Self {0} } +impl Empty for u16 { fn empty() -> Self {0} } +impl Empty for u32 { fn empty() -> Self {0} } +impl Empty for u64 { fn empty() -> Self {0} } + +pub fn is_empty(item: T) -> bool where T: Empty + Eq { + item.eq(T::empty()) +} + +pub fn is_empty_array(array: [T; N]) -> bool where T: Empty + Eq { + array.all(|elem| is_empty(elem)) +} + trait Hash { fn hash(self) -> Field; } diff --git a/yarn-project/noir-protocol-circuits/src/crates/types/src/utils/arrays.nr b/yarn-project/noir-protocol-circuits/src/crates/types/src/utils/arrays.nr index 526f444baf7..239867b05e0 100644 --- a/yarn-project/noir-protocol-circuits/src/crates/types/src/utils/arrays.nr +++ b/yarn-project/noir-protocol-circuits/src/crates/types/src/utils/arrays.nr @@ -1,49 +1,9 @@ -use crate::abis::new_contract_data::NewContractData; use dep::std::array; +use dep::std::cmp::Eq; use crate::utils::bounded_vec::BoundedVec; +use crate::traits::{Empty, is_empty}; -// Trait: is_empty -// -// We manually monomorphize the is_empty trait as its a bit simpler -// and the complexity is localized to this utils package. -// -// The general is_empty trait checks if a data type is is empty, -// and it defines empty for the basic data types as 0. -// -// If a Field is equal to zero, then it is regarded as zero. -// We will go with this definition for now, however it can be problematic -// if a value can actually be zero. In a future refactor, we can -// use the optional type for safety. Doing it now would lead to a worse devex -// and would make it harder to sync up with the cpp code. -pub fn is_empty_array(array: [Field; T]) -> bool { - array.all(|elem| is_empty(elem)) -} - -fn is_empty(value: Field) -> bool { - value == 0 -} - -pub fn is_empty_struct_array(array: [T; N], is_empty: fn(T) -> bool) -> bool { - array.all(|elem| is_empty(elem)) -} - -// Replace it with `array_to_bounded_vec` when trait is available. -pub fn struct_array_to_bounded_vec( - array: [T; N], - is_empty_item: fn(T) -> bool, - empty_item: T -) -> BoundedVec { - let mut len = 0; - for elem in array { - if !is_empty_item(elem) { - len += 1; - } - } - - BoundedVec { storage: array, len, empty_value: empty_item } -} - -pub fn array_to_bounded_vec(array: [Field; N]) -> BoundedVec { +pub fn array_to_bounded_vec(array: [T; N]) -> BoundedVec where T: Empty + Eq { let mut len = 0; for elem in array { if !is_empty(elem) { @@ -51,42 +11,20 @@ pub fn array_to_bounded_vec(array: [Field; N]) -> BoundedVec { } } - BoundedVec { storage: array, len, empty_value: 0 } + BoundedVec { storage: array, len, empty_value: T::empty() } } // Routine which validates that all zero values of an array form a contiguous region at the end, i.e., // of the form: [*,*,*...,0,0,0,0] where any * is non-zero. Note that a full array of non-zero values is // valid. -// -// TODO(David): A bit of a nit, we use is_empty, but we are actually doing is_zero. -// TODO: Concretely, are we validating empty or zero regions? -// -// TODO: We can possibly optimize this by taking advantage of the fact that adding 0 does not -// TODO: change a running sum. -pub fn validate_array(array: [Field; T]) { +pub fn validate_array(array: [T; N]) where T: Empty + Eq { let array_length = array.len(); let mut first_zero_pos = array_length; let mut last_non_zero_pos = 0; - // TODO: using is_empty here is a bit wasteful - for i in 0..array_length { - let is_empty = is_empty(array[i]); - if !is_empty { - last_non_zero_pos = i; - } else if is_empty & (first_zero_pos == array_length) { - first_zero_pos = i; - } - } - assert((last_non_zero_pos as u64) <= (first_zero_pos as u64), "invalid array"); -} -// TODO: Replace it with `validate_array` -pub fn validate_struct_array(array: [T; N], is_empty_item: fn(T) -> bool) { - let array_length = array.len(); - let mut first_zero_pos = array_length; - let mut last_non_zero_pos = 0; for i in 0..array_length { - let is_empty = is_empty_item(array[i]); + let is_empty = is_empty(array[i]); if !is_empty { last_non_zero_pos = i; } else if is_empty & (first_zero_pos == array_length) { @@ -98,7 +36,7 @@ pub fn validate_struct_array(array: [T; N], is_empty_item: fn(T) -> bool) // Helper method to determine the number of non-zero/empty elements in a validated array (ie, validate_array(array) // should be true). -pub fn array_length(array: [Field; N]) -> Field { +pub fn array_length(array: [T; N]) -> Field where T: Empty + Eq { let mut length = 0; let mut end = false; for elem in array { @@ -110,40 +48,14 @@ pub fn array_length(array: [Field; N]) -> Field { length } -// TODO: Replace it with `array_length`. -pub fn struct_array_length(array: [T; N], is_empty_item: fn(T) -> bool) -> Field { - let mut length = 0; - let mut end = false; - for elem in array { - end |= is_empty_item(elem); - if !end { - length += 1; - } - } - length -} - -pub fn array_eq(array: [Field; N], expected: [Field; S]) -> bool { +pub fn array_eq(array: [T; N], expected: [T; S]) -> bool where T: Empty + Eq { let mut eq = array_length(array) == S; - for i in 0..S { - eq = eq & (array[i] == expected[i]); - } - eq -} -// TODO: Replace it with `array_eq`. Trait: eq, is_empty -pub fn struct_array_eq( - array: [T; N], - expected: [T; S], - eq: fn(T, T) -> bool, - is_empty_item: fn(T) -> bool -) -> bool { - let length = struct_array_length(array, is_empty_item); - let mut ret = length == S; for i in 0..S { - ret &= eq(array[i], expected[i]); + eq &= array[i].eq(expected[i]); } - ret + + eq } #[test]