chore: Change private-kernel init to use BoundedVec (#2800)

Co-authored-by: sirasistant <sirasistant@gmail.com>

yarn-project/noir-private-kernel/src/Nargo.toml

@@ -2,6 +2,6 @@
 members = [
     "crates/private-kernel-lib",
     "crates/private-kernel-init",
-    "crates/private-kernel-ordering",
-    "crates/private-kernel-inner",
+    # "crates/private-kernel-ordering",
+    # "crates/private-kernel-inner",
 ]

yarn-project/noir-private-kernel/src/crates/private-kernel-init/src/main.nr

@@ -3,7 +3,7 @@ use dep::private_kernel_lib::common;
 use dep::private_kernel_lib::mocked::{Proof, AggregationObject,verify_previous_kernel_state};
 use dep::private_kernel_lib::abis::private_kernel::private_call_data::PrivateCallData;
 use dep::private_kernel_lib::transaction::request::TxRequest;
-use dep::private_kernel_lib::{KernelCircuitPublicInputs};
+use dep::private_kernel_lib::{KernelCircuitPublicInputs, KernelCircuitPublicInputsBuilder};
 use dep::private_kernel_lib::abis::combined_constant_data::CombinedConstantData;
 use dep::private_kernel_lib::utils::arrays;
 use dep::std::unsafe;
@@ -19,7 +19,7 @@ struct PrivateKernelInputsInit {
 }
 
 impl PrivateKernelInputsInit {
-    fn initialize_end_values(self, public_inputs : &mut KernelCircuitPublicInputs) {
+    fn initialize_end_values(self, public_inputs : &mut KernelCircuitPublicInputsBuilder) {
         let private_call_public_inputs = self.private_call.call_stack_item.public_inputs();
         let constants = CombinedConstantData{
             block_data : private_call_public_inputs.historical_block_data,
@@ -72,37 +72,37 @@ impl PrivateKernelInputsInit {
         assert(public_inputs.call_context.storage_contract_address.eq(call_stack_item.contract_address()), "Storage contract address must be that of the called contract");
     }
 
-    fn update_end_values(self, public_inputs : &mut KernelCircuitPublicInputs) {
+    fn update_end_values(self, public_inputs : &mut KernelCircuitPublicInputsBuilder) {
         // We only initialized constants member of public_inputs so far. Therefore, there must not be any
         // new nullifiers or logs as part of public_inputs.
 
-        let new_commitments_empty = arrays::is_empty_field(public_inputs.end.new_commitments);
+        let new_commitments_empty = arrays::is_empty_array(public_inputs.end.new_commitments.storage);
         assert(new_commitments_empty, "public_inputs.end.new_commitments must start as empty in initial kernel iteration");
 
-        let new_nullifiers_empty = arrays::is_empty_field(public_inputs.end.new_nullifiers);
+        let new_nullifiers_empty = arrays::is_empty_array(public_inputs.end.new_nullifiers.storage);
         assert(new_nullifiers_empty, "public_inputs.end.new_nullifiers must start as empty in initial kernel iteration");
 
-        let end_nullified_commitments_empty = arrays::is_empty_field(public_inputs.end.nullified_commitments);
+        let end_nullified_commitments_empty = arrays::is_empty_array(public_inputs.end.nullified_commitments.storage);
         assert(end_nullified_commitments_empty, "public_inputs.end.nullified_commitments must start as empty in initial kernel iteration");
 
-        let encrypted_logs_hash_empty = arrays::is_empty_field(public_inputs.end.encrypted_logs_hash);
+        let encrypted_logs_hash_empty = arrays::log_hashes_empty(public_inputs.end.encrypted_logs_hash);
         assert(encrypted_logs_hash_empty, "public_inputs.end.encrypted_logs_hash must start as empty in initial kernel iteration");
 
-        let unencrypted_logs_hash_empty = arrays::is_empty_field(public_inputs.end.unencrypted_logs_hash);
+        let unencrypted_logs_hash_empty = arrays::log_hashes_empty(public_inputs.end.unencrypted_logs_hash);
         assert(unencrypted_logs_hash_empty, "public_inputs.end.unencrypted_logs_hash must start as empty in initial kernel iteration");
 
-        let read_requests_empty = arrays::is_empty_field(public_inputs.end.read_requests);
+        let read_requests_empty = arrays::is_empty_array(public_inputs.end.read_requests.storage);
         assert(read_requests_empty, "public_inputs.end.read_requests must start as empty in initial kernel iteration");
 
         assert(public_inputs.end.encrypted_log_preimages_length == 0,"public_inputs.end.encrypted_log_preimages_length must start as 0 in initial kernel iteration");
         assert(public_inputs.end.unencrypted_log_preimages_length == 0,"public_inputs.end.unencrypted_log_preimages_length must start as 0 in initial kernel iteration");
 
         // Since it's the first iteration, we need to push the the tx hash nullifier into the `new_nullifiers` array
-        public_inputs.end.new_nullifiers = arrays::array_push_field(public_inputs.end.new_nullifiers, self.tx_request.hash());
+        public_inputs.end.new_nullifiers.push(self.tx_request.hash());
 
         // Push an empty nullified commitment too since each nullifier must
         // be paired with a nonzero (real or "empty") nullified commitment
-        public_inputs.end.nullified_commitments = arrays::array_push_field(public_inputs.end.nullified_commitments, constants_gen::EMPTY_NULLIFIED_COMMITMENT);
+        public_inputs.end.nullified_commitments.push(constants_gen::EMPTY_NULLIFIED_COMMITMENT);
 
         // Note that we do not need to nullify the transaction request nonce anymore.
         // Should an account want to additionally use nonces for replay protection or handling cancellations,
@@ -111,7 +111,7 @@ impl PrivateKernelInputsInit {
     }
 
     fn native_private_kernel_circuit_initial(self) -> KernelCircuitPublicInputs {
-        let mut public_inputs : KernelCircuitPublicInputs = unsafe::zeroed();
+        let mut public_inputs : KernelCircuitPublicInputsBuilder = unsafe::zeroed();
 
         // TODO(david): // Do this before any functions can modify the inputs. Why?
         self.initialize_end_values(&mut public_inputs);
@@ -124,7 +124,11 @@ impl PrivateKernelInputsInit {
 
         common::validate_call_stack(self.private_call);
 
-        common::validate_read_requests(public_inputs.constants.block_data.private_data_tree_root(),self.private_call.call_stack_item.public_inputs().read_requests,self.private_call.read_request_membership_witnesses);
+        common::validate_read_requests(
+            public_inputs.constants.block_data.private_data_tree_root(),
+            self.private_call.call_stack_item.public_inputs().read_requests,
+            self.private_call.read_request_membership_witnesses
+        );
 
         // TODO(dbanks12): feels like update_end_values should happen after contract logic
         self.update_end_values(&mut public_inputs);
@@ -141,6 +145,6 @@ impl PrivateKernelInputsInit {
 
         public_inputs.end.aggregation_object = updated_aggregation_object;
 
-        public_inputs
+        public_inputs.finish()
     }
 }

yarn-project/noir-private-kernel/src/crates/private-kernel-lib/src/abis/combined_accumulated_data.nr

@@ -16,11 +16,12 @@ use dep::aztec::constants_gen::{
     MAX_PUBLIC_DATA_READS_PER_TX
 };
 use crate::hash::NUM_FIELDS_PER_SHA256;
+use crate::utils::bounded_vec::BoundedVec;
 
 struct CombinedAccumulatedData {
     aggregation_object : AggregationObject,
 
-    read_requests : [Field;MAX_READ_REQUESTS_PER_TX],
+    read_requests : [Field; MAX_READ_REQUESTS_PER_TX],
 
     new_commitments : [Field; MAX_NEW_COMMITMENTS_PER_TX],
     new_nullifiers : [Field; MAX_NEW_NULLIFIERS_PER_TX],
@@ -44,5 +45,67 @@ struct CombinedAccumulatedData {
 
     public_data_update_requests : [PublicDataUpdateRequest; MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX],
 
-    public_data_reads : [PublicDataRead; MAX_PUBLIC_DATA_READS_PER_TX]
+    public_data_reads : [PublicDataRead; MAX_PUBLIC_DATA_READS_PER_TX],
 }
+
+struct CombinedAccumulatedDataBuilder {
+    aggregation_object : AggregationObject,
+
+    read_requests : BoundedVec<Field,MAX_READ_REQUESTS_PER_TX>,
+
+    new_commitments : BoundedVec<Field, MAX_NEW_COMMITMENTS_PER_TX>,
+    new_nullifiers : BoundedVec<Field, MAX_NEW_NULLIFIERS_PER_TX>,
+    nullified_commitments : BoundedVec<Field, MAX_NEW_NULLIFIERS_PER_TX>,
+
+    private_call_stack : BoundedVec<Field, MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX>,
+    public_call_stack : BoundedVec<Field, MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX>,
+    new_l2_to_l1_msgs : BoundedVec<Field, MAX_NEW_L2_TO_L1_MSGS_PER_TX>,
+
+    encrypted_logs_hash : [Field; NUM_FIELDS_PER_SHA256],
+    unencrypted_logs_hash : [Field; NUM_FIELDS_PER_SHA256],
+
+    // Here so that the gas cost of this request can be measured by circuits, without actually needing to feed in the
+    // variable-length data.
+    encrypted_log_preimages_length : Field,
+    unencrypted_log_preimages_length : Field,
+
+    new_contracts : BoundedVec<NewContractData, MAX_NEW_CONTRACTS_PER_TX>,
+
+    optionally_revealed_data : [OptionallyRevealedData; MAX_OPTIONALLY_REVEALED_DATA_LENGTH_PER_TX],
+
+    public_data_update_requests : BoundedVec<PublicDataUpdateRequest, MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX>,
+
+    public_data_reads : BoundedVec<PublicDataRead, MAX_PUBLIC_DATA_READS_PER_TX>,
+}
+
+impl CombinedAccumulatedDataBuilder {
+    pub fn finish(self) -> CombinedAccumulatedData {
+        CombinedAccumulatedData {
+            aggregation_object: self.aggregation_object,
+
+            read_requests: self.read_requests.storage,
+
+            new_commitments: self.new_commitments.storage,
+            new_nullifiers: self.new_nullifiers.storage,
+            nullified_commitments: self.nullified_commitments.storage,
+
+            private_call_stack: self.private_call_stack.storage,
+            public_call_stack: self.public_call_stack.storage,
+            new_l2_to_l1_msgs: self.new_l2_to_l1_msgs.storage,
+
+            encrypted_logs_hash: self.encrypted_logs_hash,
+            unencrypted_logs_hash: self.unencrypted_logs_hash,
+
+            encrypted_log_preimages_length: self.encrypted_log_preimages_length,
+            unencrypted_log_preimages_length: self.unencrypted_log_preimages_length,
+
+            new_contracts: self.new_contracts.storage,
+
+            optionally_revealed_data: self.optionally_revealed_data,
+
+            public_data_update_requests: self.public_data_update_requests.storage,
+
+            public_data_reads: self.public_data_reads.storage,
+        }
+    } 
+}

yarn-project/noir-private-kernel/src/crates/private-kernel-lib/src/abis/final_accumulated_data.nr

@@ -3,6 +3,34 @@ use crate::hash::NUM_FIELDS_PER_SHA256;
 use dep::aztec::constants_gen::{MAX_OPTIONALLY_REVEALED_DATA_LENGTH_PER_TX,MAX_NEW_CONTRACTS_PER_TX,MAX_NEW_L2_TO_L1_MSGS_PER_TX,MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX,MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX,MAX_NEW_NULLIFIERS_PER_TX,MAX_NEW_COMMITMENTS_PER_TX};
 use crate::abis::new_contract_data::NewContractData;
 use crate::abis::optionally_revealed_data::OptionallyRevealedData;
+use crate::utils::bounded_vec::BoundedVec;
+
+struct FinalAccumulatedDataBuilder {
+
+    aggregation_object : AggregationObject,
+
+    new_commitments : BoundedVec<Field, MAX_NEW_COMMITMENTS_PER_TX>,
+    new_nullifiers : BoundedVec<Field, MAX_NEW_NULLIFIERS_PER_TX>,
+    nullified_commitments : BoundedVec<Field, MAX_NEW_NULLIFIERS_PER_TX>,
+
+    // For pending nullifiers, we have:
+    // nullifiedCommitments[j] != 0 <==> newNullifiers[j] nullifies nullifiedCommitments[j]
+
+    private_call_stack : BoundedVec<Field, MAX_PRIVATE_CALL_STACK_LENGTH_PER_TX>,
+    public_call_stack : BoundedVec<Field, MAX_PUBLIC_CALL_STACK_LENGTH_PER_TX>,
+    new_l2_to_l1_msgs : BoundedVec<Field, MAX_NEW_L2_TO_L1_MSGS_PER_TX>,
+
+    encrypted_logs_hash : [Field; NUM_FIELDS_PER_SHA256],
+    unencrypted_logs_hash : [Field; NUM_FIELDS_PER_SHA256],
+
+    // Here so that the gas cost of this request can be measured by circuits, without actually needing to feed in the
+    // variable-length data.
+    encrypted_log_preimages_length : Field,
+    unencrypted_log_preimages_length : Field,
+
+    new_contracts : BoundedVec<NewContractData, MAX_NEW_CONTRACTS_PER_TX>,
+    optionally_revealed_data : BoundedVec<OptionallyRevealedData, MAX_OPTIONALLY_REVEALED_DATA_LENGTH_PER_TX>
+}
 
 struct FinalAccumulatedData{
 

yarn-project/noir-private-kernel/src/crates/private-kernel-lib/src/abis/kernel_circuit_public_inputs.nr

@@ -1,6 +1,9 @@
 
 use crate::abis::combined_constant_data::CombinedConstantData;
-use crate::abis::combined_accumulated_data::CombinedAccumulatedData;
+use crate::abis::combined_accumulated_data::{
+    CombinedAccumulatedData,
+    CombinedAccumulatedDataBuilder,
+};
 
 struct KernelCircuitPublicInputs {
 
@@ -10,3 +13,19 @@ struct KernelCircuitPublicInputs {
     is_private : bool,
 }
 
+struct KernelCircuitPublicInputsBuilder {
+    end : CombinedAccumulatedDataBuilder,
+    constants : CombinedConstantData,
+
+    is_private : bool,
+}
+
+impl KernelCircuitPublicInputsBuilder {
+    pub fn finish(self) -> KernelCircuitPublicInputs {
+        KernelCircuitPublicInputs {
+            end : self.end.finish(),
+            constants : self.constants,
+            is_private : self.is_private,
+        }
+    }
+}

yarn-project/noir-private-kernel/src/crates/private-kernel-lib/src/abis/membership_witness.nr

@@ -1,20 +1,15 @@
-use crate::utils::arrays::is_empty_field;
 use dep::aztec::constants_gen::{
     CONTRACT_TREE_HEIGHT,
     FUNCTION_TREE_HEIGHT,
 };
 
 struct MembershipWitness<N> {
     leaf_index : Field,
+    // TODO(Kev): We don't need this to be a bounded_vec but right now
+    //it makes the refactoring a bit easier
     sibling_path : [Field; N]
 }
 
-impl<N> MembershipWitness<N> {
-    fn is_empty(self) -> bool {
-        is_empty_field([self.leaf_index]) && is_empty_field(self.sibling_path)
-    }
-}
-
 // TODO(Kev): Instead of doing `MembershipWitness<FUNCTION_TREE_HEIGHT>` we are forced 
 // to  do this new struct because the typescript bindings generator
 // does not have logic to monomorphize these properly. See the file named

yarn-project/noir-private-kernel/src/crates/private-kernel-lib/src/abis/private_kernel/private_call_data.nr

@@ -12,6 +12,8 @@ use dep::aztec::constants_gen::{
 use crate::abis::call_stack_item::PrivateCallStackItem;
 use crate::abis::membership_witness::{ContractLeafMembershipWitness,FunctionLeafMembershipWitness};
 
+use crate::utils::bounded_vec::BoundedVec;
+
 type ReadRequestMembershipWitnessPrivateData = ReadRequestMembershipWitness<PRIVATE_DATA_TREE_HEIGHT>;
 
 struct PrivateCallData {