refactor(circuits): - use msgpack for cbind routines of native private kernel circuits

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

@@ -66,69 +66,53 @@ TEST_F(private_kernel_tests, circuit_basic)
     EXPECT_TRUE(private_kernel_builder.check_circuit());
 }
 
+// TODO(1998): Lack of support for msgpack deserialization for Circuitresult type for this test
+// to be able to call private_kernel__sim_init.
 /**
  * @brief Some private circuit simulation checked against its results via cbinds
  */
+/*
 TEST_F(private_kernel_tests, circuit_cbinds)
 {
-    NT::fr const& arg0 = 5;
-    NT::fr const& arg1 = 1;
-    NT::fr const& arg2 = 999;
-    std::array<NT::fr, NUM_FIELDS_PER_SHA256> const& encrypted_logs_hash = { NT::fr(16), NT::fr(69) };
-    NT::fr const& encrypted_log_preimages_length = NT::fr(100);
-    std::array<NT::fr, NUM_FIELDS_PER_SHA256> const& unencrypted_logs_hash = { NT::fr(26), NT::fr(47) };
-    NT::fr const& unencrypted_log_preimages_length = NT::fr(50);
-
-    // first run actual simulation to get public inputs
-    auto const& private_inputs = do_private_call_get_kernel_inputs_init(true,
-                                                                        constructor,
-                                                                        { arg0, arg1, arg2 },
-                                                                        encrypted_logs_hash,
-                                                                        unencrypted_logs_hash,
-                                                                        encrypted_log_preimages_length,
-                                                                        unencrypted_log_preimages_length,
-                                                                        true);
-    DummyBuilder builder = DummyBuilder("private_kernel_tests__circuit_create_proof_cbinds");
-    auto const& public_inputs = native_private_kernel_circuit_initial(builder, private_inputs);
-
-    // serialize expected public inputs for later comparison
-    std::vector<uint8_t> expected_public_inputs_vec;
-    serialize::write(expected_public_inputs_vec, public_inputs);
-
-    //***************************************************************************
-    // Now run the simulate/prove cbinds to make sure their outputs match
-    //***************************************************************************
-    // TODO(david): might be able to get rid of proving key buffer
-    uint8_t const* pk_buf = nullptr;
-    private_kernel__init_proving_key(&pk_buf);
-    // info("Proving key size: ", pk_size);
-
-    // TODO(david): might be able to get rid of verification key buffer
-    // uint8_t const* vk_buf;
-    // size_t vk_size = private_kernel__init_verification_key(pk_buf, &vk_buf);
-    // info("Verification key size: ", vk_size);
-
-    std::vector<uint8_t> signed_constructor_tx_request_vec;
-    serialize::write(signed_constructor_tx_request_vec, private_inputs.tx_request);
-
-    std::vector<uint8_t> private_constructor_call_vec;
-    serialize::write(private_constructor_call_vec, private_inputs.private_call);
-
-    // uint8_t const* proof_data_buf = nullptr;
-    uint8_t const* public_inputs_buf = nullptr;
-    size_t public_inputs_size = 0;
-    // info("Simulating to generate public inputs...");
-    uint8_t* const circuit_failure_ptr = private_kernel__sim_init(signed_constructor_tx_request_vec.data(),
-                                                                  private_constructor_call_vec.data(),
-                                                                  &public_inputs_size,
-                                                                  &public_inputs_buf);
-    ASSERT_TRUE(circuit_failure_ptr == nullptr);
-
-    // TODO(david): better equality check
-    // for (size_t i = 0; i < public_inputs_size; i++)
-    for (size_t i = 0; i < 10; i++) {
-        ASSERT_EQ(public_inputs_buf[i], expected_public_inputs_vec[i]);
-    }
+   NT::fr const& arg0 = 5;
+   NT::fr const& arg1 = 1;
+   NT::fr const& arg2 = 999;
+   std::array<NT::fr, NUM_FIELDS_PER_SHA256> const& encrypted_logs_hash = { NT::fr(16), NT::fr(69) };
+   NT::fr const& encrypted_log_preimages_length = NT::fr(100);
+   std::array<NT::fr, NUM_FIELDS_PER_SHA256> const& unencrypted_logs_hash = { NT::fr(26), NT::fr(47) };
+   NT::fr const& unencrypted_log_preimages_length = NT::fr(50);
+
+   // first run actual simulation to get public inputs
+   auto const& private_inputs = do_private_call_get_kernel_inputs_init(true,
+                                                                       constructor,
+                                                                       { arg0, arg1, arg2 },
+                                                                       encrypted_logs_hash,
+                                                                       unencrypted_logs_hash,
+                                                                       encrypted_log_preimages_length,
+                                                                       unencrypted_log_preimages_length,
+                                                                       true);
+   DummyBuilder builder = DummyBuilder("private_kernel_tests__circuit_create_proof_cbinds");
+   auto const& public_inputs = native_private_kernel_circuit_initial(builder, private_inputs);
+
+   // ***************************************************************************
+   //  Now run the simulate/prove cbinds to make sure their outputs match
+   // ***************************************************************************
+   // TODO(david): might be able to get rid of proving key buffer
+   uint8_t const* pk_buf = nullptr;
+   private_kernel__init_proving_key(&pk_buf);
+   // info("Proving key size: ", pk_size);
+
+   // TODO(david): might be able to get rid of verification key buffer
+   // uint8_t const* vk_buf;
+   // size_t vk_size = private_kernel__init_verification_key(pk_buf, &vk_buf);
+   // info("Verification key size: ", vk_size);
+
+   auto exp_result = builder.result_or_error(public_inputs);
+   // Does not compile. See https://github.com/AztecProtocol/aztec-packages/issues/1998
+   auto res = call_msgpack_cbind<decltype(exp_result)>(private_kernel__sim_init, private_inputs);
+
+   ASSERT_TRUE(exp_result.result == res.result);
 }
+*/
 
 }  // namespace aztec3::circuits::kernel::private_kernel

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

@@ -3,8 +3,7 @@
 #include "index.hpp"
 #include "utils.hpp"
 
-#include "aztec3/circuits/abis/kernel_circuit_public_inputs.hpp"
-#include "aztec3/circuits/abis/previous_kernel_data.hpp"
+#include "aztec3/circuits/abis/private_kernel/private_kernel_inputs_init.hpp"
 #include "aztec3/circuits/abis/private_kernel/private_kernel_inputs_inner.hpp"
 #include "aztec3/circuits/abis/private_kernel/private_kernel_inputs_ordering.hpp"
 #include "aztec3/constants.hpp"
@@ -17,7 +16,6 @@ namespace {
 using Builder = UltraCircuitBuilder;
 using NT = aztec3::utils::types::NativeTypes;
 using DummyCircuitBuilder = aztec3::utils::DummyCircuitBuilder;
-using aztec3::circuits::abis::PreviousKernelData;
 using aztec3::circuits::abis::TxRequest;
 using aztec3::circuits::abis::private_kernel::PrivateCallData;
 using aztec3::circuits::abis::private_kernel::PrivateKernelInputsInit;
@@ -63,68 +61,17 @@ WASM_EXPORT size_t private_kernel__init_verification_key(uint8_t const* pk_buf,
 
 CBIND(private_kernel__dummy_previous_kernel, []() { return dummy_previous_kernel(); });
 
-// TODO(dbanks12): comment about how public_inputs is a confusing name
-// returns size of public inputs
-WASM_EXPORT uint8_t* private_kernel__sim_init(uint8_t const* tx_request_buf,
-                                              uint8_t const* private_call_buf,
-                                              size_t* private_kernel_public_inputs_size_out,
-                                              uint8_t const** private_kernel_public_inputs_buf)
-{
+CBIND(private_kernel__sim_init, [](PrivateKernelInputsInit<NT> private_inputs) {
     DummyCircuitBuilder builder = DummyCircuitBuilder("private_kernel__sim_init");
+    auto const& public_inputs = native_private_kernel_circuit_initial(builder, private_inputs);
+    return builder.result_or_error(public_inputs);
+});
 
-    PrivateCallData<NT> private_call_data;
-    serialize::read(private_call_buf, private_call_data);
-
-    TxRequest<NT> tx_request;
-    serialize::read(tx_request_buf, tx_request);
-
-    PrivateKernelInputsInit<NT> const private_inputs = PrivateKernelInputsInit<NT>{
-        .tx_request = tx_request,
-        .private_call = private_call_data,
-    };
-
-    auto public_inputs = native_private_kernel_circuit_initial(builder, private_inputs);
-
-    // serialize public inputs to bytes vec
-    std::vector<uint8_t> public_inputs_vec;
-    serialize::write(public_inputs_vec, public_inputs);
-    // copy public inputs to output buffer
-    auto* raw_public_inputs_buf = (uint8_t*)malloc(public_inputs_vec.size());
-    memcpy(raw_public_inputs_buf, (void*)public_inputs_vec.data(), public_inputs_vec.size());
-    *private_kernel_public_inputs_buf = raw_public_inputs_buf;
-    *private_kernel_public_inputs_size_out = public_inputs_vec.size();
-    return builder.alloc_and_serialize_first_failure();
-}
-
-WASM_EXPORT uint8_t* private_kernel__sim_inner(uint8_t const* previous_kernel_buf,
-                                               uint8_t const* private_call_buf,
-                                               size_t* private_kernel_public_inputs_size_out,
-                                               uint8_t const** private_kernel_public_inputs_buf)
-{
+CBIND(private_kernel__sim_inner, [](PrivateKernelInputsInner<NT> private_inputs) {
     DummyCircuitBuilder builder = DummyCircuitBuilder("private_kernel__sim_inner");
-    PrivateCallData<NT> private_call_data;
-    serialize::read(private_call_buf, private_call_data);
-
-    PreviousKernelData<NT> previous_kernel;
-    serialize::read(previous_kernel_buf, previous_kernel);
-
-    PrivateKernelInputsInner<NT> const private_inputs = PrivateKernelInputsInner<NT>{
-        .previous_kernel = previous_kernel,
-        .private_call = private_call_data,
-    };
-
-    auto public_inputs = native_private_kernel_circuit_inner(builder, private_inputs);
-
-    // serialize public inputs to bytes vec
-    std::vector<uint8_t> public_inputs_vec;
-    serialize::write(public_inputs_vec, public_inputs);
-    // copy public inputs to output buffer
-    auto* raw_public_inputs_buf = (uint8_t*)malloc(public_inputs_vec.size());
-    memcpy(raw_public_inputs_buf, (void*)public_inputs_vec.data(), public_inputs_vec.size());
-    *private_kernel_public_inputs_buf = raw_public_inputs_buf;
-    *private_kernel_public_inputs_size_out = public_inputs_vec.size();
-    return builder.alloc_and_serialize_first_failure();
-}
+    auto const& public_inputs = native_private_kernel_circuit_inner(builder, private_inputs);
+    return builder.result_or_error(public_inputs);
+});
 
 CBIND(private_kernel__sim_ordering, [](PrivateKernelInputsOrdering<NT> private_inputs) {
     DummyCircuitBuilder builder = DummyCircuitBuilder("private_kernel__sim_ordering");

circuits/cpp/src/aztec3/circuits/kernel/private/c_bind.h

@@ -7,14 +7,8 @@
 WASM_EXPORT size_t private_kernel__init_proving_key(uint8_t const** pk_buf);
 WASM_EXPORT size_t private_kernel__init_verification_key(uint8_t const* pk_buf, uint8_t const** vk_buf);
 CBIND_DECL(private_kernel__dummy_previous_kernel);
-WASM_EXPORT uint8_t* private_kernel__sim_init(uint8_t const* tx_request_buf,
-                                              uint8_t const* private_call_buf,
-                                              size_t* private_kernel_public_inputs_size_out,
-                                              uint8_t const** private_kernel_public_inputs_buf);
-WASM_EXPORT uint8_t* private_kernel__sim_inner(uint8_t const* previous_kernel_buf,
-                                               uint8_t const* private_call_buf,
-                                               size_t* private_kernel_public_inputs_size_out,
-                                               uint8_t const** private_kernel_public_inputs_buf);
+CBIND_DECL(private_kernel__sim_init);
+CBIND_DECL(private_kernel__sim_inner);
 CBIND_DECL(private_kernel__sim_ordering);
 WASM_EXPORT size_t private_kernel__prove(uint8_t const* tx_request_buf,
                                          uint8_t const* previous_kernel_buf,

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

@@ -1,7 +1,7 @@
 #include "testing_harness.hpp"
 
-#include "aztec3/circuits/abis/private_kernel/private_kernel_inputs_inner.hpp"
 #include "aztec3/circuits/apps/test_apps/escrow/deposit.hpp"
+#include "aztec3/circuits/kernel/private/common.hpp"
 #include "aztec3/constants.hpp"
 #include "aztec3/utils/array.hpp"
 #include "aztec3/utils/circuit_errors.hpp"
@@ -31,6 +31,52 @@ class native_private_kernel_ordering_tests : public ::testing::Test {
     static void SetUpTestSuite() { barretenberg::srs::init_crs_factory("../barretenberg/cpp/srs_db/ignition"); }
 };
 
+// TODO(1998): testing cbind calls private_kernel__sim_ordering, private_kernel__sim_init, private_kernel__sim_inner
+//       in their respective test suites once msgpack capabilities allow it. One current limitation is due to
+//       the lack of support to deserialize std::variant in particular CircuitResult type.
+//       See https://github.com/AztecProtocol/aztec-packages/issues/1998
+/**
+ * @brief Test cbind
+ */
+// TEST_F(native_private_kernel_ordering_tests, cbind_private_kernel__sim_ordering)
+// {
+//     auto func = [](PrivateKernelInputsOrdering<NT> private_inputs) {
+//         DummyCircuitBuilder builder = DummyCircuitBuilder("private_kernel__sim_ordering");
+//         auto const& public_inputs = native_private_kernel_circuit_ordering(builder, private_inputs);
+//         return builder.result_or_error(public_inputs);
+//     };
+
+//     NT::fr const& amount = 5;
+//     NT::fr const& asset_id = 1;
+//     NT::fr const& memo = 999;
+//     std::array<NT::fr, NUM_FIELDS_PER_SHA256> const& empty_logs_hash = { NT::fr(16), NT::fr(69) };
+//     NT::fr const& empty_log_preimages_length = NT::fr(100);
+
+//     // Generate private inputs including proofs and vkeys for app circuit and previous kernel
+//     auto const& private_inputs_inner = do_private_call_get_kernel_inputs_inner(false,
+//                                                                                deposit,
+//                                                                                { amount, asset_id, memo },
+//                                                                                empty_logs_hash,
+//                                                                                empty_logs_hash,
+//                                                                                empty_log_preimages_length,
+//                                                                                empty_log_preimages_length,
+//                                                                                empty_logs_hash,
+//                                                                                empty_logs_hash,
+//                                                                                empty_log_preimages_length,
+//                                                                                empty_log_preimages_length,
+//                                                                                true);
+//     PrivateKernelInputsOrdering<NT> private_inputs{ private_inputs_inner.previous_kernel,
+//                                                     std::array<fr, MAX_READ_REQUESTS_PER_TX>{ fr(123), fr(89) } };
+
+//     auto [actual, expected] = call_func_and_wrapper(func, private_kernel__sim_ordering, private_inputs);
+
+//     std::stringstream actual_ss;
+//     std::stringstream expected_ss;
+//     actual_ss << actual;
+//     expected_ss << expected;
+//     EXPECT_EQ(actual_ss.str(), expected_ss.str());
+// }
+
 TEST_F(native_private_kernel_ordering_tests, native_matching_one_read_request_to_commitment_works)
 {
     auto private_inputs_inner = do_private_call_get_kernel_inputs_inner(false, deposit, standard_test_args());

yarn-project/acir-simulator/src/client/private_execution.ts

@@ -218,7 +218,7 @@ export class PrivateFunctionExecution {
     publicInputs.unencryptedLogsHash = to2Fields(unencryptedLogs.hash());
     publicInputs.unencryptedLogPreimagesLength = new Fr(unencryptedLogs.getSerializedLength());
 
-    const callStackItem = new PrivateCallStackItem(this.contractAddress, this.functionData, publicInputs);
+    const callStackItem = new PrivateCallStackItem(this.contractAddress, this.functionData, publicInputs, false);
     const returnValues = decodeReturnValues(this.abi, publicInputs.returnValues);
 
     this.log(`Returning from call to ${this.contractAddress.toString()}:${selector}`);

yarn-project/aztec-rpc/src/kernel_prover/kernel_prover.test.ts

@@ -1,6 +1,7 @@
 import { ExecutionResult, NewNoteData } from '@aztec/acir-simulator';
 import {
   KernelCircuitPublicInputs,
+  MAX_NEW_COMMITMENTS_PER_CALL,
   MAX_NEW_COMMITMENTS_PER_TX,
   MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL,
   MAX_READ_REQUESTS_PER_CALL,
@@ -12,6 +13,7 @@ import {
   VK_TREE_HEIGHT,
   VerificationKey,
   makeEmptyProof,
+  makeTuple,
 } from '@aztec/circuits.js';
 import { makeTxRequest } from '@aztec/circuits.js/factories';
 import { AztecAddress } from '@aztec/foundation/aztec-address';
@@ -46,10 +48,14 @@ describe('Kernel Prover', () => {
 
   const createExecutionResult = (fnName: string, newNoteIndices: number[] = []): ExecutionResult => {
     const publicInputs = PrivateCircuitPublicInputs.empty();
-    publicInputs.newCommitments = newNoteIndices.map(idx => generateFakeCommitment(notes[idx]));
+    publicInputs.newCommitments = makeTuple(
+      MAX_NEW_COMMITMENTS_PER_CALL,
+      i => (i < newNoteIndices.length ? generateFakeCommitment(notes[newNoteIndices[i]]) : Fr.ZERO),
+      0,
+    );
     return {
       // Replace `FunctionData` with `string` for easier testing.
-      callStackItem: new PrivateCallStackItem(AztecAddress.ZERO, fnName as any, publicInputs),
+      callStackItem: new PrivateCallStackItem(AztecAddress.ZERO, fnName as any, publicInputs, false),
       nestedExecutions: (dependencies[fnName] || []).map(name => createExecutionResult(name)),
       vk: VerificationKey.makeFake().toBuffer(),
       preimages: { newNotes: newNoteIndices.map(idx => notes[idx]), nullifiedNotes: [] },
@@ -79,9 +85,11 @@ describe('Kernel Prover', () => {
   };
 
   const expectExecution = (fns: string[]) => {
-    const callStackItemsInit = proofCreator.createProofInit.mock.calls.map(args => args[1].callStackItem.functionData);
+    const callStackItemsInit = proofCreator.createProofInit.mock.calls.map(
+      args => args[0].privateCall.callStackItem.functionData,
+    );
     const callStackItemsInner = proofCreator.createProofInner.mock.calls.map(
-      args => args[1].callStackItem.functionData,
+      args => args[0].privateCall.callStackItem.functionData,
     );
 
     expect(proofCreator.createProofInit).toHaveBeenCalledTimes(Math.min(1, fns.length));