fix(kernels): add array overflow composer error

circuits/cpp/src/aztec3/circuits/kernel/private/.test.cpp

@@ -715,6 +715,7 @@ TEST(private_kernel_tests, circuit_create_proof_cbinds)
     free((void*)public_inputs_buf);
 }
 
+
 /**
  * @brief Test this dummy cbind
  */
@@ -730,4 +731,28 @@ TEST(private_kernel_tests, cbind_private_kernel__dummy_previous_kernel)
     EXPECT_EQ(actual_ss.str(), expected_ss.str());
 }
 
+TEST(private_kernel_tests, private_kernel_should_fail_if_aggregating_too_many_commitments)
+{
+    // Negative test to check if push_array_to_array fails if two many commitments are merged together
+    DummyComposer composer = DummyComposer("should_fail_if_aggregating_too_many_commitments");
+
+    NT::fr const& amount = 5;
+    NT::fr const& asset_id = 1;
+    NT::fr const& memo = 999;
+
+    PrivateKernelInputsInner<NT> private_inputs =
+        do_private_call_get_kernel_inputs_inner(false, deposit, { amount, asset_id, memo });
+
+    // Mock the previous new commitments to be full, therefore no need commitments can be added
+    std::array<fr, KERNEL_NEW_COMMITMENTS_LENGTH> full_new_commitments{};
+    for (size_t i = 0; i < KERNEL_NEW_COMMITMENTS_LENGTH; ++i) {
+        full_new_commitments[i] = i + 1;
+    }
+    private_inputs.previous_kernel.public_inputs.end.new_commitments = full_new_commitments;
+    auto const& public_inputs = native_private_kernel_circuit_inner(composer, private_inputs);
+
+    ASSERT_TRUE(composer.failed());
+    ASSERT_EQ(composer.get_first_failure().code, CircuitErrorCode::ARRAY_OVERFLOW);
+}
+
 }  // namespace aztec3::circuits::kernel::private_kernel

circuits/cpp/src/aztec3/circuits/kernel/private/common.cpp

@@ -81,16 +81,16 @@ void common_update_end_values(DummyComposer& composer,
                 new_nullifiers[i] == 0 ? 0 : silo_nullifier<NT>(storage_contract_address, new_nullifiers[i]);
         }
 
-        push_array_to_array(siloed_new_commitments, public_inputs.end.new_commitments);
-        push_array_to_array(siloed_new_nullifiers, public_inputs.end.new_nullifiers);
+        push_array_to_array(composer, siloed_new_commitments, public_inputs.end.new_commitments);
+        push_array_to_array(composer, siloed_new_nullifiers, public_inputs.end.new_nullifiers);
     }
 
     {  // call stacks
         const auto& this_private_call_stack = private_call_public_inputs.private_call_stack;
-        push_array_to_array(this_private_call_stack, public_inputs.end.private_call_stack);
+        push_array_to_array(composer, this_private_call_stack, public_inputs.end.private_call_stack);
 
         const auto& this_public_call_stack = private_call_public_inputs.public_call_stack;
-        push_array_to_array(this_public_call_stack, public_inputs.end.public_call_stack);
+        push_array_to_array(composer, this_public_call_stack, public_inputs.end.public_call_stack);
     }
 
     {  // new l2 to l1 messages
@@ -109,7 +109,7 @@ void common_update_end_values(DummyComposer& composer,
                                                                            new_l2_to_l1_msgs[i]);
             }
         }
-        push_array_to_array(new_l2_to_l1_msgs_to_insert, public_inputs.end.new_l2_to_l1_msgs);
+        push_array_to_array(composer, new_l2_to_l1_msgs_to_insert, public_inputs.end.new_l2_to_l1_msgs);
     }
 
     {  // logs hashes
@@ -169,8 +169,7 @@ void common_contract_logic(DummyComposer& composer,
                                                             portal_contract_address,
                                                             contract_dep_data.function_tree_root };
 
-        array_push<NewContractData<NT>, KERNEL_NEW_CONTRACTS_LENGTH>(public_inputs.end.new_contracts,
-                                                                     native_new_contract_data);
+        array_push(composer, public_inputs.end.new_contracts, native_new_contract_data);
         composer.do_assert(contract_dep_data.constructor_vk_hash == private_call_vk_hash,
                            "constructor_vk_hash doesn't match private_call_vk_hash",
                            CircuitErrorCode::PRIVATE_KERNEL__INVALID_CONSTRUCTOR_VK_HASH);
@@ -185,7 +184,7 @@ void common_contract_logic(DummyComposer& composer,
         auto const new_contract_address_nullifier = NT::fr::serialize_from_buffer(NT::blake3s(blake_input).data());
 
         // push the contract address nullifier to nullifier vector
-        array_push(public_inputs.end.new_nullifiers, new_contract_address_nullifier);
+        array_push(composer, public_inputs.end.new_nullifiers, new_contract_address_nullifier);
     } else {
         // non-contract deployments must specify contract address being interacted with
         composer.do_assert(storage_contract_address != 0,

circuits/cpp/src/aztec3/circuits/kernel/private/native_private_kernel_circuit_init.cpp

@@ -123,7 +123,9 @@ void validate_inputs(DummyComposer& composer, PrivateKernelInputsInit<NT> const&
     // hard-coded into the circuit and assert that that is the one which has been used in the base case).
 }
 
-void update_end_values(PrivateKernelInputsInit<NT> const& private_inputs, KernelCircuitPublicInputs<NT>& public_inputs)
+void update_end_values(DummyComposer& composer,
+                       PrivateKernelInputsInit<NT> const& private_inputs,
+                       KernelCircuitPublicInputs<NT>& public_inputs)
 {
     // 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.
@@ -136,13 +138,13 @@ void update_end_values(PrivateKernelInputsInit<NT> const& private_inputs, Kernel
     ASSERT(public_inputs.end.unencrypted_log_preimages_length == fr(0));
 
     // Since it's the first iteration, we need to push the the tx hash nullifier into the `new_nullifiers` array
-    array_push(public_inputs.end.new_nullifiers, private_inputs.signed_tx_request.hash());
+    array_push(composer, public_inputs.end.new_nullifiers, private_inputs.signed_tx_request.hash());
 
     // Nonce nullifier
     // DANGER: This is terrible. This should not be part of the protocol. This is an intentional bodge to reach a
     // milestone. This must not be the way we derive nonce nullifiers in production. It can be front-run by other
     // users. It is not domain separated. Naughty.
-    array_push(public_inputs.end.new_nullifiers, private_inputs.signed_tx_request.tx_request.nonce);
+    array_push(composer, public_inputs.end.new_nullifiers, private_inputs.signed_tx_request.tx_request.nonce);
 }
 
 // NOTE: THIS IS A VERY UNFINISHED WORK IN PROGRESS.
@@ -167,7 +169,7 @@ KernelCircuitPublicInputs<NT> native_private_kernel_circuit_initial(DummyCompose
     // TODO(jeanmon) FIXME - https://github.com/AztecProtocol/aztec-packages/issues/671
     // common_validate_call_stack(composer, private_inputs.private_call);
 
-    update_end_values(private_inputs, public_inputs);
+    update_end_values(composer, private_inputs, public_inputs);
 
     common_update_end_values(composer, private_inputs.private_call, public_inputs);
 

circuits/cpp/src/aztec3/circuits/kernel/public/.test.cpp

@@ -1116,4 +1116,5 @@ TEST(public_kernel_tests, previous_public_kernel_fails_if_incorrect_storage_cont
     ASSERT_EQ(dummyComposer.get_first_failure().code,
               CircuitErrorCode::PUBLIC_KERNEL__CALL_CONTEXT_INVALID_STORAGE_ADDRESS_FOR_DELEGATE_CALL);
 }
+
 }  // namespace aztec3::circuits::kernel::public_kernel

circuits/cpp/src/aztec3/circuits/kernel/public/common.hpp

@@ -202,8 +202,9 @@ void common_validate_inputs(DummyComposer& composer, KernelInput const& public_k
  * @param public_kernel_inputs The inputs to this iteration of the kernel circuit
  * @param circuit_outputs The circuit outputs to be populated
  */
-template <typename KernelInput>
-void propagate_valid_public_data_update_requests(KernelInput const& public_kernel_inputs,
+template <typename KernelInput, typename Composer>
+void propagate_valid_public_data_update_requests(Composer& composer,
+                                                 KernelInput const& public_kernel_inputs,
                                                  KernelCircuitPublicInputs<NT>& circuit_outputs)
 {
     const auto& contract_address = public_kernel_inputs.public_call.call_stack_item.contract_address;
@@ -219,7 +220,7 @@ void propagate_valid_public_data_update_requests(KernelInput const& public_kerne
             .old_value = compute_public_data_tree_value<NT>(update_request.old_value),
             .new_value = compute_public_data_tree_value<NT>(update_request.new_value),
         };
-        array_push(circuit_outputs.end.public_data_update_requests, new_write);
+        array_push(composer, circuit_outputs.end.public_data_update_requests, new_write);
     }
 }
 
@@ -229,8 +230,10 @@ void propagate_valid_public_data_update_requests(KernelInput const& public_kerne
  * @param public_kernel_inputs The inputs to this iteration of the kernel circuit
  * @param circuit_outputs The circuit outputs to be populated
  */
-template <typename KernelInput> void propagate_valid_public_data_reads(KernelInput const& public_kernel_inputs,
-                                                                       KernelCircuitPublicInputs<NT>& circuit_outputs)
+template <typename KernelInput, typename Composer>
+void propagate_valid_public_data_reads(Composer& composer,
+                                       KernelInput const& public_kernel_inputs,
+                                       KernelCircuitPublicInputs<NT>& circuit_outputs)
 {
     const auto& contract_address = public_kernel_inputs.public_call.call_stack_item.contract_address;
     const auto& reads = public_kernel_inputs.public_call.call_stack_item.public_inputs.contract_storage_reads;
@@ -243,28 +246,31 @@ template <typename KernelInput> void propagate_valid_public_data_reads(KernelInp
             .leaf_index = compute_public_data_tree_index<NT>(contract_address, contract_storage_read.storage_slot),
             .value = compute_public_data_tree_value<NT>(contract_storage_read.current_value),
         };
-        array_push(circuit_outputs.end.public_data_reads, new_read);
+        array_push(composer, circuit_outputs.end.public_data_reads, new_read);
     }
 }
 
 /**
  * @brief Propagates valid (i.e. non-empty) public data reads from this iteration to the circuit output
  * @tparam The type of kernel input
+ * @tparam The current composer
  * @param public_kernel_inputs The inputs to this iteration of the kernel circuit
  * @param circuit_outputs The circuit outputs to be populated
  */
-template <typename KernelInput> void common_update_public_end_values(KernelInput const& public_kernel_inputs,
-                                                                     KernelCircuitPublicInputs<NT>& circuit_outputs)
+template <typename KernelInput, typename Composer>
+void common_update_public_end_values(Composer& composer,
+                                     KernelInput const& public_kernel_inputs,
+                                     KernelCircuitPublicInputs<NT>& circuit_outputs)
 {
     // Updates the circuit outputs with new state changes, call stack etc
     circuit_outputs.is_private = false;
 
     const auto& stack = public_kernel_inputs.public_call.call_stack_item.public_inputs.public_call_stack;
-    push_array_to_array(stack, circuit_outputs.end.public_call_stack);
+    push_array_to_array(composer, stack, circuit_outputs.end.public_call_stack);
 
-    propagate_valid_public_data_update_requests(public_kernel_inputs, circuit_outputs);
+    propagate_valid_public_data_update_requests(composer, public_kernel_inputs, circuit_outputs);
 
-    propagate_valid_public_data_reads(public_kernel_inputs, circuit_outputs);
+    propagate_valid_public_data_reads(composer, public_kernel_inputs, circuit_outputs);
 }
 
 /**

circuits/cpp/src/aztec3/circuits/kernel/public/native_public_kernel_circuit_no_previous_kernel.cpp

@@ -66,9 +66,9 @@ void update_public_end_values(DummyComposer& composer,
                        "new_nullifiers array must be empty in a first iteration of public kernel",
                        CircuitErrorCode::PUBLIC_KERNEL__NEW_NULLIFIERS_NOT_EMPTY_IN_FIRST_ITERATION);
 
-    array_push(circuit_outputs.end.new_nullifiers, public_kernel_inputs.signed_tx_request.hash());
+    array_push(composer, circuit_outputs.end.new_nullifiers, public_kernel_inputs.signed_tx_request.hash());
 
-    common_update_public_end_values(public_kernel_inputs, circuit_outputs);
+    common_update_public_end_values(composer, public_kernel_inputs, circuit_outputs);
 }
 }  // namespace
 

circuits/cpp/src/aztec3/circuits/kernel/public/native_public_kernel_circuit_private_previous_kernel.cpp

@@ -65,7 +65,7 @@ KernelCircuitPublicInputs<NT> native_public_kernel_circuit_private_previous_kern
     validate_this_public_call_hash(composer, public_kernel_inputs, public_inputs);
 
     // update the public end state of the circuit
-    common_update_public_end_values(public_kernel_inputs, public_inputs);
+    common_update_public_end_values(composer, public_kernel_inputs, public_inputs);
 
     return public_inputs;
 };

circuits/cpp/src/aztec3/circuits/kernel/public/native_public_kernel_circuit_public_previous_kernel.cpp

@@ -60,7 +60,7 @@ KernelCircuitPublicInputs<NT> native_public_kernel_circuit_public_previous_kerne
     validate_this_public_call_hash(composer, public_kernel_inputs, public_inputs);
 
     // update the public end state of the circuit
-    common_update_public_end_values(public_kernel_inputs, public_inputs);
+    common_update_public_end_values(composer, public_kernel_inputs, public_inputs);
 
     return public_inputs;
 };

circuits/cpp/src/aztec3/utils/array.hpp

@@ -1,6 +1,9 @@
 #pragma once
 #include "./types/native_types.hpp"
 
+#include "aztec3/utils/circuit_errors.hpp"
+
+#include "barretenberg/proof_system/types/composer_type.hpp"
 #include <barretenberg/barretenberg.hpp>
 
 /**
@@ -97,18 +100,20 @@ template <typename T, size_t SIZE> T array_pop(std::array<T, SIZE>& arr)
 /**
  * @brief Helper method to push an item into the first empty slot in an array
  * @tparam The type of the value stored in the array
+ * @tparam The composer type
  * @tparam The size of the array
  * @param The array into which we want to store the value
  */
-template <typename T, size_t SIZE> void array_push(std::array<T, SIZE>& arr, T const& value)
+template <typename T, typename Composer, size_t SIZE>
+void array_push(Composer& composer, std::array<T, SIZE>& arr, T const& value)
 {
     for (size_t i = 0; i < arr.size(); ++i) {
         if (is_empty(arr[i])) {
             arr[i] = value;
             return;
         }
     }
-    throw_or_abort("array_push cannot push to a full array");
+    composer.do_assert(false, "array_push cannot push to a full array", CircuitErrorCode::ARRAY_OVERFLOW);
 };
 
 /**
@@ -137,17 +142,22 @@ template <typename T, size_t SIZE> NT::boolean is_array_empty(std::array<T, SIZE
  * @param The `source` array
  * @param The `target` array
  */
-template <size_t size_1, size_t size_2, typename T>
-void push_array_to_array(std::array<T, size_1> const& source, std::array<T, size_2>& target)
+template <size_t size_1, size_t size_2, typename T, typename Composer>
+void push_array_to_array(Composer& composer, std::array<T, size_1> const& source, std::array<T, size_2>& target)
 {
     // Check if the `source` array is too large vs the remaining capacity of the `target` array
     size_t const source_size = static_cast<size_t>(uint256_t(array_length(source)));
     size_t const target_size = static_cast<size_t>(uint256_t(array_length(target)));
-    ASSERT(source_size <= size_2 - target_size);
+
+    composer.do_assert(source_size <= size_2 - target_size,
+                       "push_array_to_array cannot overflow the target",
+                       CircuitErrorCode::ARRAY_OVERFLOW);
 
     // Ensure that there are no non-zero values in the `target` array after the first zero-valued index
     for (size_t i = target_size; i < size_2; i++) {
-        ASSERT(is_empty(target[i]));
+        composer.do_assert(is_empty(target[i]),
+                           "push_array_to_array inserting new array into a non empty space",
+                           CircuitErrorCode::ARRAY_OVERFLOW);
     }
     // Copy the non-zero elements of the `source` array to the `target` array at the first zero-valued index
     auto zero_index = target_size;

circuits/cpp/src/aztec3/utils/circuit_errors.hpp

@@ -75,8 +75,10 @@ enum CircuitErrorCode : uint16_t {
     CONTRACT_TREE_SNAPSHOT_MISMATCH = 7006,
     PUBLIC_DATA_TREE_ROOT_MISMATCH = 7007,
     MEMBERSHIP_CHECK_FAILED = 7008,
+    ARRAY_OVERFLOW = 7009,
 
     ROOT_CIRCUIT_FAILED = 8000,
+
 };
 
 struct CircuitError {

yarn-project/circuits.js/src/kernel/public_kernel.test.ts

@@ -1,5 +1,15 @@
-import { simulatePublicKernelCircuit, simulatePublicKernelCircuitNoPreviousKernel } from '../index.js';
-import { makePublicKernelInputsNoKernelInput, makePublicKernelInputsWithEmptyOutput } from '../tests/factories.js';
+import {
+  CircuitError,
+  KERNEL_PUBLIC_DATA_READS_LENGTH,
+  makeTuple,
+  simulatePublicKernelCircuit,
+  simulatePublicKernelCircuitNoPreviousKernel,
+} from '../index.js';
+import {
+  makePublicDataRead,
+  makePublicKernelInputsNoKernelInput,
+  makePublicKernelInputsWithEmptyOutput,
+} from '../tests/factories.js';
 
 describe('kernel/public_kernel', () => {
   it('simulates public kernel circuit with previous public kernel', async function () {
@@ -24,4 +34,20 @@ describe('kernel/public_kernel', () => {
     const result = await simulatePublicKernelCircuitNoPreviousKernel(input);
     expect(result).toBeDefined();
   });
+
+  it('simulating public kernel circuit fails when aggregating proofs will overflow', async function () {
+    const input = await makePublicKernelInputsWithEmptyOutput();
+    // Fix validity
+    input.publicCall.callStackItem.functionData.isConstructor = false;
+    input.publicCall.callStackItem.functionData.isPrivate = false;
+    input.previousKernel.publicInputs.isPrivate = false;
+
+    // Cause array overflow
+    const fullStateReadsObject = makeTuple(KERNEL_PUBLIC_DATA_READS_LENGTH, makePublicDataRead, 0x01);
+    input.previousKernel.publicInputs.end.publicDataReads = fullStateReadsObject;
+
+    await expect(simulatePublicKernelCircuit(input)).rejects.toThrow(
+      new CircuitError(7009, 'array_push cannot push to a full array'),
+    );
+  });
 });