improve kernel and rollup errors (#1104)

* improve kernel and rollup errors

* rename errors with consistency + per david's PR comments

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

@@ -179,10 +179,10 @@ void common_update_end_values(DummyBuilder& builder,
         // No state changes are allowed for static calls:
         builder.do_assert(is_array_empty(new_commitments) == true,
                           "new_commitments must be empty for static calls",
-                          CircuitErrorCode::PRIVATE_KERNEL__NEW_COMMITMENTS_NOT_EMPTY_FOR_STATIC_CALL);
+                          CircuitErrorCode::PRIVATE_KERNEL__NEW_COMMITMENTS_PROHIBITED_IN_STATIC_CALL);
         builder.do_assert(is_array_empty(new_nullifiers) == true,
                           "new_nullifiers must be empty for static calls",
-                          CircuitErrorCode::PRIVATE_KERNEL__NEW_NULLIFIERS_NOT_EMPTY_FOR_STATIC_CALL);
+                          CircuitErrorCode::PRIVATE_KERNEL__NEW_NULLIFIERS_PROHIBITED_IN_STATIC_CALL);
     }
 
     const auto& storage_contract_address = private_call_public_inputs.call_context.storage_contract_address;
@@ -196,8 +196,16 @@ void common_update_end_values(DummyBuilder& builder,
             if (witness.is_transient) {  // only forward transient to public inputs
                 const auto siloed_read_request =
                     read_request == 0 ? 0 : silo_commitment<NT>(storage_contract_address, read_request);
-                array_push(builder, public_inputs.end.read_requests, siloed_read_request);
-                array_push(builder, public_inputs.end.read_request_membership_witnesses, witness);
+                array_push(builder,
+                           public_inputs.end.read_requests,
+                           siloed_read_request,
+                           format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING,
+                                  "too many transient read requests in one tx"));
+                array_push(builder,
+                           public_inputs.end.read_request_membership_witnesses,
+                           witness,
+                           format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING,
+                                  "too many transient read request membership witnesses in one tx"));
             }
         }
     }
@@ -210,7 +218,11 @@ void common_update_end_values(DummyBuilder& builder,
             siloed_new_nullifiers[i] =
                 new_nullifiers[i] == 0 ? 0 : silo_nullifier<NT>(storage_contract_address, new_nullifiers[i]);
         }
-        push_array_to_array(builder, siloed_new_nullifiers, public_inputs.end.new_nullifiers);
+        push_array_to_array(
+            builder,
+            siloed_new_nullifiers,
+            public_inputs.end.new_nullifiers,
+            format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many new nullifiers in one tx"));
 
         // commitments
         std::array<NT::fr, MAX_NEW_COMMITMENTS_PER_CALL> siloed_new_commitments{};
@@ -222,15 +234,27 @@ void common_update_end_values(DummyBuilder& builder,
             siloed_new_commitments[i] =
                 new_commitments[i] == 0 ? 0 : silo_commitment<NT>(storage_contract_address, unique_commitment);
         }
-        push_array_to_array(builder, siloed_new_commitments, public_inputs.end.new_commitments);
+        push_array_to_array(
+            builder,
+            siloed_new_commitments,
+            public_inputs.end.new_commitments,
+            format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many new commitments in one tx"));
     }
 
     {  // call stacks
         const auto& this_private_call_stack = private_call_public_inputs.private_call_stack;
-        push_array_to_array(builder, this_private_call_stack, public_inputs.end.private_call_stack);
+        push_array_to_array(
+            builder,
+            this_private_call_stack,
+            public_inputs.end.private_call_stack,
+            format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many private call stacks in one tx"));
 
         const auto& this_public_call_stack = private_call_public_inputs.public_call_stack;
-        push_array_to_array(builder, this_public_call_stack, public_inputs.end.public_call_stack);
+        push_array_to_array(
+            builder,
+            this_public_call_stack,
+            public_inputs.end.public_call_stack,
+            format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many public call stacks in one tx"));
     }
 
     {  // new l2 to l1 messages
@@ -246,7 +270,11 @@ void common_update_end_values(DummyBuilder& builder,
                                                                            new_l2_to_l1_msgs[i]);
             }
         }
-        push_array_to_array(builder, new_l2_to_l1_msgs_to_insert, public_inputs.end.new_l2_to_l1_msgs);
+        push_array_to_array(
+            builder,
+            new_l2_to_l1_msgs_to_insert,
+            public_inputs.end.new_l2_to_l1_msgs,
+            format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many new l2 to l1 messages in one tx"));
     }
 
     {  // logs hashes
@@ -305,7 +333,11 @@ void common_contract_logic(DummyBuilder& builder,
                                                             portal_contract_address,
                                                             contract_dep_data.function_tree_root };
 
-        array_push(builder, public_inputs.end.new_contracts, native_new_contract_data);
+        array_push(builder,
+                   public_inputs.end.new_contracts,
+                   native_new_contract_data,
+                   format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many contracts created in one tx"));
+
         builder.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);
@@ -320,7 +352,11 @@ void common_contract_logic(DummyBuilder& builder,
         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(builder, public_inputs.end.new_nullifiers, new_contract_address_nullifier);
+        array_push(builder,
+                   public_inputs.end.new_nullifiers,
+                   new_contract_address_nullifier,
+                   format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING,
+                          "too many nullifiers in one tx to add the new contract address"));
     } else {
         // non-contract deployments must specify contract address being interacted with
         builder.do_assert(storage_contract_address != 0,

circuits/cpp/src/aztec3/circuits/kernel/private/init.hpp

@@ -24,4 +24,8 @@ using OracleWrapper = aztec3::circuits::apps::OracleWrapperInterface<Builder>;
 
 using FunctionExecutionContext = aztec3::circuits::apps::FunctionExecutionContext<Builder>;
 
+// Used when calling library functions like `psuh_array` which have their own generic error code.
+// So we pad this in front of the error message to identify where the error originally came from.
+const std::string PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING = "private_kernel_circuit: ";
+
 }  // namespace aztec3::circuits::kernel::private_kernel

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

@@ -78,19 +78,20 @@ void validate_this_private_call_against_tx_request(DummyBuilder& builder,
     const auto& call_stack_item = private_inputs.private_call.call_stack_item;
 
     builder.do_assert(tx_request.origin == call_stack_item.contract_address,
-                      "user's intent does not match initial private call (tx_request.origin must match "
-                      "call_stack_item.contract_address)",
+                      "user's intent does not match initial private call (origin address of tx_request must match "
+                      "call_stack_item's contract_address)",
                       CircuitErrorCode::PRIVATE_KERNEL__USER_INTENT_MISMATCH_BETWEEN_TX_REQUEST_AND_CALL_STACK_ITEM);
 
     builder.do_assert(tx_request.function_data.hash() == call_stack_item.function_data.hash(),
                       "user's intent does not match initial private call (tx_request.function_data must match "
                       "call_stack_item.function_data)",
                       CircuitErrorCode::PRIVATE_KERNEL__USER_INTENT_MISMATCH_BETWEEN_TX_REQUEST_AND_CALL_STACK_ITEM);
 
-    builder.do_assert(tx_request.args_hash == call_stack_item.public_inputs.args_hash,
-                      "user's intent does not match initial private call (tx_request.args must match "
-                      "call_stack_item.public_inputs.args)",
-                      CircuitErrorCode::PRIVATE_KERNEL__USER_INTENT_MISMATCH_BETWEEN_TX_REQUEST_AND_CALL_STACK_ITEM);
+    builder.do_assert(
+        tx_request.args_hash == call_stack_item.public_inputs.args_hash,
+        "user's intent does not match initial private call (noir function args passed to tx_request must match "
+        "args in the call_stack_item)",
+        CircuitErrorCode::PRIVATE_KERNEL__USER_INTENT_MISMATCH_BETWEEN_TX_REQUEST_AND_CALL_STACK_ITEM);
 };
 
 void validate_inputs(DummyBuilder& builder, PrivateKernelInputsInit<NT> const& private_inputs)
@@ -153,7 +154,11 @@ void update_end_values(DummyBuilder& builder,
                       CircuitErrorCode::PRIVATE_KERNEL__UNSUPPORTED_OP);
 
     // Since it's the first iteration, we need to push the the tx hash nullifier into the `new_nullifiers` array
-    array_push(builder, public_inputs.end.new_nullifiers, private_inputs.tx_request.hash());
+    array_push(builder,
+               public_inputs.end.new_nullifiers,
+               private_inputs.tx_request.hash(),
+               format(PRIVATE_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING,
+                      "could not push tx hash nullifier into new_nullifiers array. Too many new nullifiers in one tx"));
 
     // 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,

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

@@ -223,8 +223,8 @@ TEST_F(native_private_kernel_init_tests, contract_deployment_args_hash_mismatch_
     EXPECT_EQ(builder.get_first_failure().code,
               CircuitErrorCode::PRIVATE_KERNEL__USER_INTENT_MISMATCH_BETWEEN_TX_REQUEST_AND_CALL_STACK_ITEM);
     EXPECT_EQ(builder.get_first_failure().message,
-              "user's intent does not match initial private call (tx_request.args must match "
-              "call_stack_item.public_inputs.args)");
+              "user's intent does not match initial private call (noir function args passed to tx_request must match "
+              "args in the call_stack_item)");
 }
 
 TEST_F(native_private_kernel_init_tests, private_function_is_private_false_fails)

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

@@ -111,14 +111,14 @@ void common_validate_call_stack(DummyBuilder& builder, KernelInput const& public
                                  preimage_portal_address,
                                  " expected ",
                                  expected_portal_address,
-                                 "; does not reconcile"),
+                                 "; does not reconcile for a delagate call"),
                           CircuitErrorCode::PUBLIC_KERNEL__PUBLIC_CALL_STACK_INVALID_PORTAL_ADDRESS);
 
         const auto num_contract_storage_update_requests =
             array_length(preimage.public_inputs.contract_storage_update_requests);
         builder.do_assert(
             !is_static_call || num_contract_storage_update_requests == 0,
-            format("contract_storage_update_requests[", i, "] should be empty"),
+            format("contract_storage_update_requests[", i, "] should be empty for a static call"),
             CircuitErrorCode::PUBLIC_KERNEL__PUBLIC_CALL_STACK_CONTRACT_STORAGE_UPDATES_PROHIBITED_FOR_STATIC_CALL);
     }
 };
@@ -141,12 +141,12 @@ void common_validate_call_context(DummyBuilder& builder, KernelInput const& publ
         array_length(call_stack_item.public_inputs.contract_storage_update_requests);
 
     builder.do_assert(!is_delegate_call || contract_address != storage_contract_address,
-                      std::string("call_context contract_address == storage_contract_address on delegate_call"),
+                      std::string("curent contract address must not match storage contract address for delegate calls"),
                       CircuitErrorCode::PUBLIC_KERNEL__CALL_CONTEXT_INVALID_STORAGE_ADDRESS_FOR_DELEGATE_CALL);
 
     builder.do_assert(
         !is_static_call || contract_storage_update_requests_length == 0,
-        std::string("call_context contract storage update requests found on static call"),
+        std::string("No contract storage update requests are allowed for static calls"),
         CircuitErrorCode::PUBLIC_KERNEL__CALL_CONTEXT_CONTRACT_STORAGE_UPDATE_REQUESTS_PROHIBITED_FOR_STATIC_CALL);
 };
 
@@ -178,10 +178,10 @@ void common_validate_inputs(DummyBuilder& builder, KernelInput const& public_ker
                       "Contract deployment can't be a public function",
                       CircuitErrorCode::PUBLIC_KERNEL__CONTRACT_DEPLOYMENT_NOT_ALLOWED);
     builder.do_assert(this_call_stack_item.contract_address != 0,
-                      "Contract address must be valid",
+                      "Contract address must be non-zero",
                       CircuitErrorCode::PUBLIC_KERNEL__CONTRACT_ADDRESS_INVALID);
     builder.do_assert(this_call_stack_item.function_data.function_selector != 0,
-                      "Function signature must be valid",
+                      "Function signature must be non-zero",
                       CircuitErrorCode::PUBLIC_KERNEL__FUNCTION_SIGNATURE_INVALID);
     builder.do_assert(this_call_stack_item.function_data.is_constructor == false,
                       "Constructors can't be public functions",
@@ -190,7 +190,7 @@ void common_validate_inputs(DummyBuilder& builder, KernelInput const& public_ker
                       "Cannot execute a private function with the public kernel circuit",
                       CircuitErrorCode::PUBLIC_KERNEL__PRIVATE_FUNCTION_NOT_ALLOWED);
     builder.do_assert(public_kernel_inputs.public_call.bytecode_hash != 0,
-                      "Bytecode hash must be valid",
+                      "Bytecode hash must be non-zero",
                       CircuitErrorCode::PUBLIC_KERNEL__BYTECODE_HASH_INVALID);
 }
 
@@ -206,10 +206,10 @@ void perform_static_call_checks(Builder& builder, KernelInput const& public_kern
 
     if (is_static_call) {
         builder.do_assert(utils::is_array_empty(new_commitments) == true,
-                          "perform_static_call_checks in static call new commitments must be empty",
+                          "no new commitments must be created for static calls",
                           CircuitErrorCode::PUBLIC_KERNEL__NEW_COMMITMENTS_PROHIBITED_IN_STATIC_CALL);
         builder.do_assert(utils::is_array_empty(new_nullifiers) == true,
-                          "perform_static_call_checks in static call new nullifiers must be empty",
+                          "no new nullifiers must be created for static calls",
                           CircuitErrorCode::PUBLIC_KERNEL__NEW_NULLIFIERS_PROHIBITED_IN_STATIC_CALL);
     }
 }
@@ -238,7 +238,11 @@ void propagate_valid_public_data_update_requests(Builder& builder,
             .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(builder, circuit_outputs.end.public_data_update_requests, new_write);
+        array_push(
+            builder,
+            circuit_outputs.end.public_data_update_requests,
+            new_write,
+            format(PUBLIC_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many public data update requests in one tx"));
     }
 }
 
@@ -264,7 +268,10 @@ void propagate_valid_public_data_reads(Builder& builder,
             .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(builder, circuit_outputs.end.public_data_reads, new_read);
+        array_push(builder,
+                   circuit_outputs.end.public_data_reads,
+                   new_read,
+                   format(PUBLIC_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many public data reads in one tx"));
     }
 }
 
@@ -294,7 +301,10 @@ void propagate_new_commitments(Builder& builder,
         }
     }
 
-    push_array_to_array(builder, siloed_new_commitments, circuit_outputs.end.new_commitments);
+    push_array_to_array(builder,
+                        siloed_new_commitments,
+                        circuit_outputs.end.new_commitments,
+                        format(PUBLIC_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many new commitments in one tx"));
 }
 
 /**
@@ -323,7 +333,10 @@ void propagate_new_nullifiers(Builder& builder,
         }
     }
 
-    push_array_to_array(builder, siloed_new_nullifiers, circuit_outputs.end.new_nullifiers);
+    push_array_to_array(builder,
+                        siloed_new_nullifiers,
+                        circuit_outputs.end.new_nullifiers,
+                        format(PUBLIC_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many new nullifiers in one tx"));
 }
 
 /**
@@ -355,7 +368,11 @@ void propagate_new_l2_to_l1_messages(Builder& builder,
                 storage_contract_address, version, portal_contract_address, chain_id, new_l2_to_l1_msgs[i]);
         }
     }
-    push_array_to_array(builder, new_l2_to_l1_msgs_to_insert, circuit_outputs.end.new_l2_to_l1_msgs);
+    push_array_to_array(
+        builder,
+        new_l2_to_l1_msgs_to_insert,
+        circuit_outputs.end.new_l2_to_l1_msgs,
+        format(PUBLIC_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many new l2 to l1 messages in one tx"));
 }
 
 /**
@@ -406,7 +423,11 @@ void common_update_public_end_values(Builder& builder,
     perform_static_call_checks(builder, public_kernel_inputs);
 
     const auto& stack = public_kernel_inputs.public_call.call_stack_item.public_inputs.public_call_stack;
-    push_array_to_array(builder, stack, circuit_outputs.end.public_call_stack);
+    push_array_to_array(
+        builder,
+        stack,
+        circuit_outputs.end.public_call_stack,
+        format(PUBLIC_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING, "too many public call stack items in one tx"));
 
     propagate_new_commitments(builder, public_kernel_inputs, circuit_outputs);
     propagate_new_nullifiers(builder, public_kernel_inputs, circuit_outputs);

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

@@ -24,4 +24,8 @@ using DB = oracle::FakeDB;
 using oracle::NativeOracle;
 using OracleWrapper = aztec3::circuits::apps::OracleWrapperInterface<Builder>;
 
+// Used when calling library functions like `psuh_array` which have their own generic error code.
+// So we pad this in front of the error message to identify where the error originally came from.
+const std::string PUBLIC_KERNEL_CIRCUIT_ERROR_MESSAGE_BEGINNING = "public_kernel_circuit: ";
+
 }  // namespace aztec3::circuits::kernel::public_kernel

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

@@ -20,10 +20,11 @@ using CircuitErrorCode = aztec3::utils::CircuitErrorCode;
 void validate_inputs(DummyBuilder& builder, PublicKernelInputs<NT> const& public_kernel_inputs)
 {
     builder.do_assert(array_length(public_kernel_inputs.previous_kernel.public_inputs.end.private_call_stack) == 0,
-                      "Private call stack must be empty",
+                      "Private call stack must be empty when executing in the public kernel (i.e. all private calls "
+                      "must have been completed)",
                       CircuitErrorCode::PUBLIC_KERNEL__NON_EMPTY_PRIVATE_CALL_STACK);
     builder.do_assert(public_kernel_inputs.previous_kernel.public_inputs.is_private == true,
-                      "Previous kernel must be private",
+                      "Previous kernel must be private when in this public kernel version",
                       CircuitErrorCode::PUBLIC_KERNEL__PREVIOUS_KERNEL_NOT_PRIVATE);
 }
 }  // namespace