chore(contracts): Calculate public call stack item hash from Noir

circuits/cpp/src/aztec3/circuits/abis/call_context.hpp

@@ -76,7 +76,7 @@ template <typename NCT> struct CallContext {
             fr(is_static_call),    fr(is_contract_deployment),
         };
 
-        return NCT::compress(inputs, GeneratorIndex::CALL_CONTEXT);
+        return NCT::hash(inputs, GeneratorIndex::CALL_CONTEXT);
     }
 
     template <typename Builder> void assert_is_zero()

circuits/cpp/src/aztec3/circuits/abis/call_stack_item.hpp

@@ -76,7 +76,7 @@ template <typename NCT, template <class> typename PrivatePublic> struct CallStac
         };
 
         // NOLINTNEXTLINE(misc-const-correctness)
-        fr call_stack_item_hash = NCT::compress(inputs, GeneratorIndex::CALL_STACK_ITEM);
+        fr call_stack_item_hash = NCT::hash(inputs, GeneratorIndex::CALL_STACK_ITEM);
 
         return call_stack_item_hash;
     }

circuits/cpp/src/aztec3/circuits/abis/contract_deployment_data.hpp

@@ -98,7 +98,7 @@ template <typename NCT> struct ContractDeploymentData {
             function_tree_root,    contract_address_salt, portal_contract_address.to_field(),
         };
 
-        return NCT::compress(inputs, GeneratorIndex::CONTRACT_DEPLOYMENT_DATA);
+        return NCT::hash(inputs, GeneratorIndex::CONTRACT_DEPLOYMENT_DATA);
     }
 };
 

circuits/cpp/src/aztec3/circuits/abis/contract_storage_read.hpp

@@ -59,7 +59,7 @@ template <typename NCT> struct ContractStorageRead {
             current_value,
         };
 
-        return NCT::compress(inputs, GeneratorIndex::PUBLIC_DATA_READ);
+        return NCT::hash(inputs, GeneratorIndex::PUBLIC_DATA_READ);
     }
 
     void set_public()

circuits/cpp/src/aztec3/circuits/abis/contract_storage_update_request.hpp

@@ -63,7 +63,7 @@ template <typename NCT> struct ContractStorageUpdateRequest {
             new_value,
         };
 
-        return NCT::compress(inputs, GeneratorIndex::PUBLIC_DATA_UPDATE_REQUEST);
+        return NCT::hash(inputs, GeneratorIndex::PUBLIC_DATA_UPDATE_REQUEST);
     }
 
     void set_public()

circuits/cpp/src/aztec3/circuits/abis/function_data.hpp

@@ -77,7 +77,7 @@ template <typename NCT> struct FunctionData {
             fr(is_constructor),
         };
 
-        return NCT::compress(inputs, GeneratorIndex::FUNCTION_DATA);
+        return NCT::hash(inputs, GeneratorIndex::FUNCTION_DATA);
     }
 };
 

circuits/cpp/src/aztec3/circuits/abis/packers.hpp

@@ -79,7 +79,8 @@ struct ConstantsPacker {
                  PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH),
              NVP(GET_NOTES_ORACLE_RETURN_LENGTH,
                  EMPTY_NULLIFIED_COMMITMENT,
-                 CALL_PRIVATE_FUNCTION_RETURN_SIZE));  // <-- Add names of new constants here
+                 CALL_PRIVATE_FUNCTION_RETURN_SIZE,
+                 PUBLIC_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH));  // <-- Add names of new constants here
     }
 };
 

circuits/cpp/src/aztec3/circuits/abis/private_circuit_public_inputs.hpp

@@ -198,7 +198,7 @@ template <typename NCT> class PrivateCircuitPublicInputs {
         inputs.push_back(chain_id);
         inputs.push_back(version);
 
-        return NCT::compress(inputs, GeneratorIndex::PRIVATE_CIRCUIT_PUBLIC_INPUTS);
+        return NCT::hash(inputs, GeneratorIndex::PRIVATE_CIRCUIT_PUBLIC_INPUTS);
     }
 
     template <size_t SIZE> void spread_arr_into_vec(std::array<fr, SIZE> const& arr, std::vector<fr>& vec) const

circuits/cpp/src/aztec3/circuits/abis/public_circuit_public_inputs.hpp

@@ -104,9 +104,7 @@ template <typename NCT> struct PublicCircuitPublicInputs {
 
         std::vector<fr> inputs;
 
-        // NOTE: we omit the call_context from this hash function, and instead hash it within CallStackItem, for
-        // efficiency, so that fewer hashes are needed to 'unwrap' the call_context in the kernel circuit.
-        // inputs.push_back(call_context.hash());
+        inputs.push_back(call_context.hash());
 
         inputs.push_back(args_hash);
         spread_arr_into_vec(return_values, inputs);
@@ -120,12 +118,12 @@ template <typename NCT> struct PublicCircuitPublicInputs {
         spread_arr_into_vec(new_l2_to_l1_msgs, inputs);
 
         spread_arr_into_vec(unencrypted_logs_hash, inputs);
-
         inputs.push_back(unencrypted_log_preimages_length);
 
         inputs.push_back(historic_public_data_tree_root);
+        inputs.push_back(prover_address);
 
-        return NCT::compress(inputs, GeneratorIndex::PUBLIC_CIRCUIT_PUBLIC_INPUTS);
+        return NCT::hash(inputs, GeneratorIndex::PUBLIC_CIRCUIT_PUBLIC_INPUTS);
     }
 
     template <size_t SIZE> void spread_arr_into_vec(std::array<fr, SIZE> const& arr, std::vector<fr>& vec) const

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

@@ -666,6 +666,9 @@ TEST(public_kernel_tests, incorrect_storage_contract_address_fails_for_regular_c
             NT::fr(inputs.public_call.public_call_stack_preimages[i].contract_address) + 1;
         inputs.public_call.public_call_stack_preimages[i].public_inputs.call_context.storage_contract_address =
             new_contract_address;
+        // update the call stack item hash after the change in the preimage
+        inputs.public_call.call_stack_item.public_inputs.public_call_stack[i] =
+            inputs.public_call.public_call_stack_preimages[i].hash();
         auto public_inputs = native_public_kernel_circuit_private_previous_kernel(dummyBuilder, inputs);
         ASSERT_TRUE(dummyBuilder.failed());
         ASSERT_EQ(dummyBuilder.get_first_failure().code,
@@ -683,6 +686,9 @@ TEST(public_kernel_tests, incorrect_msg_sender_fails_for_regular_calls)
         const auto new_msg_sender = inputs.public_call.public_call_stack_preimages[i].contract_address;
         // change the storage contract address so it does not equal the contract address
         inputs.public_call.public_call_stack_preimages[i].public_inputs.call_context.msg_sender = new_msg_sender;
+        // update the call stack item hash after the change in the preimage
+        inputs.public_call.call_stack_item.public_inputs.public_call_stack[i] =
+            inputs.public_call.public_call_stack_preimages[i].hash();
         auto public_inputs = native_public_kernel_circuit_private_previous_kernel(dummyBuilder, inputs);
         ASSERT_TRUE(dummyBuilder.failed());
         ASSERT_EQ(dummyBuilder.get_first_failure().code,
@@ -701,31 +707,34 @@ TEST(public_kernel_tests, public_kernel_circuit_succeeds_for_mixture_of_regular_
     const auto contract_portal_address = NT::fr(inputs.public_call.portal_contract_address);
 
     // redefine the child calls/stacks to use some delegate calls
-    std::array<PublicCallStackItem, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL> child_call_stacks;
     NT::uint32 const seed = 1000;
     NT::fr child_contract_address = 100000;
     NT::fr child_portal_contract_address = 200000;
     NT::boolean is_delegate_call = false;
-    std::array<NT::fr, MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL> call_stack_hashes{};
     for (size_t i = 0; i < MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL; i++) {
-        child_call_stacks[i] =
+        inputs.public_call.public_call_stack_preimages[i] =
             // NOLINTNEXTLINE(readability-suspicious-call-argument)
             generate_call_stack_item(child_contract_address,
                                      is_delegate_call ? origin_msg_sender : contract_address,
                                      is_delegate_call ? contract_address : child_contract_address,
                                      is_delegate_call ? contract_portal_address : child_portal_contract_address,
                                      is_delegate_call,
                                      seed);
-        call_stack_hashes[i] = child_call_stacks[i].hash();
+        inputs.public_call.call_stack_item.public_inputs.public_call_stack[i] =
+            inputs.public_call.public_call_stack_preimages[i].hash();
 
         // change the next call type
         is_delegate_call = !is_delegate_call;
         child_contract_address++;
         child_portal_contract_address++;
     }
-    inputs.public_call.call_stack_item.public_inputs.public_call_stack = call_stack_hashes;
-    inputs.public_call.public_call_stack_preimages = child_call_stacks;
+
+    // we update the hash of the current call stack item in the previous kernel,
+    // since we modified the hash of the nested calls, which changes the hash of the parent item
+    inputs.previous_kernel.public_inputs.end.public_call_stack[0] = inputs.public_call.call_stack_item.hash();
+
     auto public_inputs = native_public_kernel_circuit_private_previous_kernel(dummyBuilder, inputs);
+    ASSERT_EQ(dummyBuilder.get_first_failure(), utils::CircuitError::no_error());
     ASSERT_FALSE(dummyBuilder.failed());
 }
 

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

@@ -66,6 +66,6 @@ void validate_this_public_call_hash(DummyBuilder& builder,
                ") does not match provided public_call_hash (",
                popped_public_call_hash,
                ") at the top of the call stack"),
-        CircuitErrorCode::PUBLIC_KERNEL__CALCULATED_PRIVATE_CALL_HASH_AND_PROVIDED_PRIVATE_CALL_HASH_MISMATCH);
+        CircuitErrorCode::PUBLIC_KERNEL__CALCULATED_PUBLIC_CALL_HASH_AND_PROVIDED_PUBLIC_CALL_HASH_MISMATCH);
 };
 }  // namespace aztec3::circuits::kernel::public_kernel

circuits/cpp/src/aztec3/constants.hpp

@@ -232,6 +232,12 @@ constexpr size_t PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH =
     NUM_FIELDS_PER_SHA256 + 1 +         // + 1 for unencrypted logs preimage length
     COMMITMENT_TREES_ROOTS_LENGTH + 2;  // + 2 for chain_id and version
 
+constexpr size_t PUBLIC_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH =
+    2 + RETURN_VALUES_LENGTH +  // + 1 for args_hash + 1 call_context.hash
+    MAX_PUBLIC_DATA_UPDATE_REQUESTS_PER_CALL + MAX_PUBLIC_DATA_READS_PER_CALL + MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL +
+    MAX_NEW_COMMITMENTS_PER_CALL + MAX_NEW_NULLIFIERS_PER_CALL + MAX_NEW_L2_TO_L1_MSGS_PER_CALL + 5;
+
+
 // Size of the return value of a private function call,
 constexpr size_t CALL_PRIVATE_FUNCTION_RETURN_SIZE =
     1 + FUNCTION_DATA_LENGTH + PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH + 1;

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

@@ -42,7 +42,7 @@ enum CircuitErrorCode : uint16_t {
     PUBLIC_KERNEL__NON_EMPTY_PRIVATE_CALL_STACK = 3005,
     PUBLIC_KERNEL__PREVIOUS_KERNEL_NOT_PRIVATE = 3009,
     PUBLIC_KERNEL__PREVIOUS_KERNEL_NOT_PUBLIC = 3010,
-    PUBLIC_KERNEL__CALCULATED_PRIVATE_CALL_HASH_AND_PROVIDED_PRIVATE_CALL_HASH_MISMATCH = 3011,
+    PUBLIC_KERNEL__CALCULATED_PUBLIC_CALL_HASH_AND_PROVIDED_PUBLIC_CALL_HASH_MISMATCH = 3011,
     PUBLIC_KERNEL__PUBLIC_CALL_STACK_MISMATCH = 3012,
     PUBLIC_KERNEL__CONTRACT_DEPLOYMENT_NOT_ALLOWED = 3013,
     PUBLIC_KERNEL__CONSTRUCTOR_NOT_ALLOWED = 3014,

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

@@ -12,6 +12,7 @@ import {
   TxContext,
 } from '@aztec/circuits.js';
 import {
+  computeCallStackItemHash,
   computeCommitmentNonce,
   computeContractAddressFromPartial,
   computeSecretMessageHash,
@@ -516,22 +517,28 @@ describe('Private Execution test suite', () => {
         args,
       });
 
-      expect(result.enqueuedPublicFunctionCalls).toHaveLength(1);
-      expect(result.enqueuedPublicFunctionCalls[0]).toEqual(
-        PublicCallRequest.from({
-          contractAddress: childAddress,
-          functionData: new FunctionData(childSelector, false, false),
-          args: [new Fr(42n)],
-          callContext: CallContext.from({
-            msgSender: parentAddress,
-            storageContractAddress: childAddress,
-            portalContractAddress: childPortalContractAddress,
-            isContractDeployment: false,
-            isDelegateCall: false,
-            isStaticCall: false,
-          }),
+      const publicCallRequest = PublicCallRequest.from({
+        contractAddress: childAddress,
+        functionData: new FunctionData(childSelector, false, false),
+        args: [new Fr(42n)],
+        callContext: CallContext.from({
+          msgSender: parentAddress,
+          storageContractAddress: childAddress,
+          portalContractAddress: childPortalContractAddress,
+          isContractDeployment: false,
+          isDelegateCall: false,
+          isStaticCall: false,
         }),
+      });
+
+      const publicCallRequestHash = computeCallStackItemHash(
+        await CircuitsWasm.get(),
+        await publicCallRequest.toPublicCallStackItem(),
       );
+
+      expect(result.enqueuedPublicFunctionCalls).toHaveLength(1);
+      expect(result.enqueuedPublicFunctionCalls[0]).toEqual(publicCallRequest);
+      expect(result.callStackItem.publicInputs.publicCallStack[0]).toEqual(publicCallRequestHash);
     });
   });
 

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

@@ -1,17 +1,13 @@
 import {
   CallContext,
-  CircuitsWasm,
   ContractDeploymentData,
   FunctionData,
-  MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL,
   PrivateCallStackItem,
   PublicCallRequest,
 } from '@aztec/circuits.js';
-import { computeCallStackItemHash } from '@aztec/circuits.js/abis';
 import { Curve } from '@aztec/circuits.js/barretenberg';
 import { FunctionAbi, decodeReturnValues } from '@aztec/foundation/abi';
 import { AztecAddress } from '@aztec/foundation/aztec-address';
-import { padArrayEnd } from '@aztec/foundation/collection';
 import { Fr, Point } from '@aztec/foundation/fields';
 import { createDebugLogger } from '@aztec/foundation/log';
 import { to2Fields } from '@aztec/foundation/serialize';
@@ -177,8 +173,6 @@ export class PrivateFunctionExecution {
 
     const publicInputs = extractPublicInputs(partialWitness, acir);
 
-    const wasm = await CircuitsWasm.get();
-
     // TODO(https://github.com/AztecProtocol/aztec-packages/issues/1165) --> set this in Noir
     publicInputs.encryptedLogsHash = to2Fields(encryptedLogs.hash());
     publicInputs.encryptedLogPreimagesLength = new Fr(encryptedLogs.getSerializedLength());
@@ -188,15 +182,6 @@ export class PrivateFunctionExecution {
     const callStackItem = new PrivateCallStackItem(this.contractAddress, this.functionData, publicInputs);
     const returnValues = decodeReturnValues(this.abi, publicInputs.returnValues);
 
-    // TODO(#499): Noir fails to compute the enqueued calls preimages properly, since it cannot use pedersen generators, so we patch those values here.
-    const publicCallStackItems = await Promise.all(enqueuedPublicFunctionCalls.map(c => c.toPublicCallStackItem()));
-    const publicStack = await Promise.all(publicCallStackItems.map(c => computeCallStackItemHash(wasm, c)));
-    callStackItem.publicInputs.publicCallStack = padArrayEnd(
-      publicStack,
-      Fr.ZERO,
-      MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL,
-    );
-
     this.log(`Returning from call to ${this.contractAddress.toString()}:${selector}`);
 
     const readRequestPartialWitnesses = this.context.getReadRequestPartialWitnesses();

yarn-project/circuits.js/src/abis/__snapshots__/abis.test.ts.snap

@@ -99,46 +99,46 @@ Fr {
 exports[`abis wasm bindings hashes a tx request 1`] = `
 {
   "data": [
-    12,
-    232,
+    48,
+    57,
+    170,
+    107,
+    100,
+    47,
+    7,
     162,
-    197,
-    24,
-    169,
-    220,
-    190,
+    88,
+    236,
+    91,
+    140,
+    100,
+    126,
     249,
-    181,
-    143,
-    125,
-    42,
-    148,
+    245,
+    88,
+    16,
+    57,
+    72,
+    61,
+    110,
+    171,
+    162,
+    119,
     122,
-    184,
-    9,
-    76,
-    249,
-    203,
-    86,
-    123,
-    134,
-    233,
-    199,
-    172,
-    142,
-    3,
-    153,
-    58,
-    146,
-    26,
+    158,
+    95,
+    157,
+    42,
+    141,
+    232,
   ],
   "type": "Buffer",
 }
 `;
 
 exports[`abis wasm bindings hashes constructor info 1`] = `
 Fr {
-  "value": 74771810193401619436460949258064408227201554785021679654928411451496318664n,
+  "value": 3768952371995154608073783464050856449274677454782425541065822710687690385468n,
 }
 `;
 

yarn-project/circuits.js/src/cbind/constants.gen.ts

@@ -63,6 +63,7 @@ export const PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH = 54;
 export const GET_NOTES_ORACLE_RETURN_LENGTH = 86;
 export const EMPTY_NULLIFIED_COMMITMENT = 1000000;
 export const CALL_PRIVATE_FUNCTION_RETURN_SIZE = 60;
+export const PUBLIC_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH = 33;
 export enum GeneratorIndex {
   COMMITMENT = 1,
   COMMITMENT_NONCE = 2,