Skip to content

Commit

Permalink
fix(contracts): Calculate hash of private call stack items in Noir (#…
Browse files Browse the repository at this point in the history
…1209)

Fix hashing of private call stack items to be consistent across Noir and
cpp.

Note that this does not solve #499 in the kernel circuit (cc @jeanmon)
since uncommenting the call stack hash validation breaks most of the
kernel tests since they modify call stack items without updating the
corresponding hashes. I tried fixing them but after a few hours of
repeated failures decided to cut my losses and submit what I got working
on the noir and ts side.
  • Loading branch information
spalladino authored Jul 31, 2023
1 parent 7688457 commit 5bf5674
Show file tree
Hide file tree
Showing 23 changed files with 139 additions and 45 deletions.
10 changes: 9 additions & 1 deletion circuits/cpp/src/aztec3/circuits/abis/c_bind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ using aztec3::circuits::abis::StorageSlotGeneratorIndexPacker;
using aztec3::circuits::abis::TxContext;
using aztec3::circuits::abis::TxRequest;
using NT = aztec3::utils::types::NativeTypes;
using aztec3::circuits::abis::PrivateTypes;
using aztec3::circuits::abis::PublicTypes;

// Cbind helper functions
Expand Down Expand Up @@ -459,7 +460,14 @@ WASM_EXPORT void abis__compute_transaction_hash(uint8_t const* tx_request_buf, u
NT::fr::serialize_to_buffer(to_write, output);
}

WASM_EXPORT void abis__compute_call_stack_item_hash(uint8_t const* call_stack_item_buf, uint8_t* output)
WASM_EXPORT void abis__compute_private_call_stack_item_hash(uint8_t const* call_stack_item_buf, uint8_t* output)
{
CallStackItem<NT, PrivateTypes> call_stack_item;
serialize::read(call_stack_item_buf, call_stack_item);
NT::fr::serialize_to_buffer(call_stack_item.hash(), output);
}

WASM_EXPORT void abis__compute_public_call_stack_item_hash(uint8_t const* call_stack_item_buf, uint8_t* output)
{
CallStackItem<NT, PublicTypes> call_stack_item;
serialize::read(call_stack_item_buf, call_stack_item);
Expand Down
3 changes: 2 additions & 1 deletion circuits/cpp/src/aztec3/circuits/abis/packers.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ struct ConstantsPacker {
GET_NOTES_ORACLE_RETURN_LENGTH,
EMPTY_NULLIFIED_COMMITMENT,
CALL_PRIVATE_FUNCTION_RETURN_SIZE,
PUBLIC_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH)); // <-- Add names of new constants here
PUBLIC_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH,
PRIVATE_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH)); // <-- Add names of new constants here
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ template <typename NCT> class PrivateCircuitPublicInputs {
inputs.push_back(chain_id);
inputs.push_back(version);

if (inputs.size() != PRIVATE_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH) {
throw_or_abort("Incorrect number of input fields when hashing PrivateCircuitPublicInputs");
}
return NCT::hash(inputs, GeneratorIndex::PRIVATE_CIRCUIT_PUBLIC_INPUTS);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "aztec3/utils/types/circuit_types.hpp"
#include "aztec3/utils/types/native_types.hpp"

#include "barretenberg/common/throw_or_abort.hpp"
#include <barretenberg/barretenberg.hpp>

namespace aztec3::circuits::abis {
Expand Down Expand Up @@ -123,6 +124,9 @@ template <typename NCT> struct PublicCircuitPublicInputs {
inputs.push_back(historic_public_data_tree_root);
inputs.push_back(prover_address);

if (inputs.size() != PUBLIC_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH) {
throw_or_abort("Incorrect number of input fields when hashing PublicCircuitPublicInputs");
}
return NCT::hash(inputs, GeneratorIndex::PUBLIC_CIRCUIT_PUBLIC_INPUTS);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@ void validate_this_private_call_hash(DummyBuilder& builder,

builder.do_assert(
popped_private_call_hash == calculated_this_private_call_hash,
"calculated private_call_hash does not match provided private_call_hash at the top of the call stack",
format("calculated private_call_hash (",
calculated_this_private_call_hash,
") does not match provided private_call_hash (",
popped_private_call_hash,
") at the top of the call stack"),
CircuitErrorCode::PRIVATE_KERNEL__CALCULATED_PRIVATE_CALL_HASH_AND_PROVIDED_PRIVATE_CALL_HASH_MISMATCH);
};

Expand Down
10 changes: 9 additions & 1 deletion circuits/cpp/src/aztec3/constants.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,12 @@ constexpr size_t PRIVATE_CIRCUIT_PUBLIC_INPUTS_LENGTH =
MAX_NEW_L2_TO_L1_MSGS_PER_CALL + NUM_FIELDS_PER_SHA256 + NUM_FIELDS_PER_SHA256 + 2 // + 2 for logs preimage lengths
+ COMMITMENT_TREES_ROOTS_LENGTH + CONTRACT_DEPLOYMENT_DATA_LENGTH + 2; // + 2 for chain_id and version

constexpr size_t PRIVATE_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH =
1 + 1 // call_context_hash + args_hash
+ RETURN_VALUES_LENGTH + MAX_READ_REQUESTS_PER_CALL + MAX_NEW_COMMITMENTS_PER_CALL +
2 * MAX_NEW_NULLIFIERS_PER_CALL + MAX_PRIVATE_CALL_STACK_LENGTH_PER_CALL + MAX_PUBLIC_CALL_STACK_LENGTH_PER_CALL +
MAX_NEW_L2_TO_L1_MSGS_PER_CALL + NUM_FIELDS_PER_SHA256 + NUM_FIELDS_PER_SHA256 + 2 // + 2 for logs preimage lengths
+ COMMITMENT_TREES_ROOTS_LENGTH + 3; // + 3 for contract_deployment_data.hash(), chain_id, version

constexpr size_t CONTRACT_STORAGE_UPDATE_REQUEST_LENGTH = 3;
constexpr size_t CONTRACT_STORAGE_READ_LENGTH = 2;
Expand All @@ -236,7 +242,9 @@ constexpr size_t PUBLIC_CIRCUIT_PUBLIC_INPUTS_LENGTH =
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;
MAX_NEW_COMMITMENTS_PER_CALL + MAX_NEW_NULLIFIERS_PER_CALL + MAX_NEW_L2_TO_L1_MSGS_PER_CALL +
NUM_FIELDS_PER_SHA256 + // unencrypted_logs_hash
3; // unencrypted_log_preimages_length + historic_public_data_tree_root + prover_address


// Size of the return value of a private function call,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ describe('Private Execution test suite', () => {

describe('nested calls', () => {
const privateIncrement = txContextFields.chainId.value + txContextFields.version.value;

it('child function should be callable', async () => {
const initialValue = 100n;
const abi = ChildContractAbi.functions.find(f => f.name === 'value')!;
Expand Down Expand Up @@ -413,6 +414,11 @@ describe('Private Execution test suite', () => {
expect(oracle.getPortalContractAddress.mock.calls[0]).toEqual([childAddress]);
expect(result.nestedExecutions).toHaveLength(1);
expect(result.nestedExecutions[0].callStackItem.publicInputs.returnValues[0]).toEqual(new Fr(privateIncrement));

// check that Noir calculated the call stack item hash like cpp does
const wasm = await CircuitsWasm.get();
const expectedCallStackItemHash = computeCallStackItemHash(wasm, result.nestedExecutions[0].callStackItem);
expect(result.callStackItem.publicInputs.privateCallStack[0]).toEqual(expectedCallStackItemHash);
});
});

Expand Down
36 changes: 34 additions & 2 deletions yarn-project/circuits.js/src/abis/abis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
FunctionData,
FunctionLeafPreimage,
NewContractData,
PrivateCallStackItem,
PublicCallStackItem,
PublicKey,
TxRequest,
Expand Down Expand Up @@ -359,9 +360,40 @@ export function computeTxHash(wasm: IWasmModule, txRequest: TxRequest): Fr {
* @param callStackItem - The call stack item.
* @returns The call stack item hash.
*/
export function computeCallStackItemHash(wasm: IWasmModule, callStackItem: PublicCallStackItem): Fr {
export function computeCallStackItemHash(
wasm: IWasmModule,
callStackItem: PrivateCallStackItem | PublicCallStackItem,
): Fr {
if (callStackItem instanceof PrivateCallStackItem) {
return computePrivateCallStackItemHash(wasm, callStackItem);
} else if (callStackItem instanceof PublicCallStackItem) {
return computePublicCallStackItemHash(wasm, callStackItem);
} else {
throw new Error(`Unexpected call stack item type`);
}
}

/**
* Computes a call stack item hash.
* @param wasm - Relevant WASM wrapper.
* @param callStackItem - The call stack item.
* @returns The call stack item hash.
*/
export function computePrivateCallStackItemHash(wasm: IWasmModule, callStackItem: PrivateCallStackItem): Fr {
wasm.call('pedersen__init');
const value = wasmSyncCall(wasm, 'abis__compute_private_call_stack_item_hash', callStackItem, 32);
return Fr.fromBuffer(value);
}

/**
* Computes a call stack item hash.
* @param wasm - Relevant WASM wrapper.
* @param callStackItem - The call stack item.
* @returns The call stack item hash.
*/
export function computePublicCallStackItemHash(wasm: IWasmModule, callStackItem: PublicCallStackItem): Fr {
wasm.call('pedersen__init');
const value = wasmSyncCall(wasm, 'abis__compute_call_stack_item_hash', callStackItem, 32);
const value = wasmSyncCall(wasm, 'abis__compute_public_call_stack_item_hash', callStackItem, 32);
return Fr.fromBuffer(value);
}

Expand Down
1 change: 1 addition & 0 deletions yarn-project/circuits.js/src/cbind/constants.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export const GET_NOTES_ORACLE_RETURN_LENGTH = 86;
export const EMPTY_NULLIFIED_COMMITMENT = 1000000;
export const CALL_PRIVATE_FUNCTION_RETURN_SIZE = 62;
export const PUBLIC_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH = 33;
export const PRIVATE_CIRCUIT_PUBLIC_INPUTS_HASH_INPUT_LENGTH = 46;
export enum GeneratorIndex {
COMMITMENT = 1,
COMMITMENT_NONCE = 2,
Expand Down
Loading

0 comments on commit 5bf5674

Please sign in to comment.