diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp index 80a49dafcec..d4359ec0b1c 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp @@ -9,6 +9,7 @@ #include "barretenberg/vm/avm/generated/verifier.hpp" #include "barretenberg/vm/avm/trace/common.hpp" #include "barretenberg/vm/avm/trace/deserialization.hpp" +#include "barretenberg/vm/avm/trace/gadgets/merkle_tree.hpp" #include "barretenberg/vm/avm/trace/helper.hpp" #include "barretenberg/vm/avm/trace/instructions.hpp" #include "barretenberg/vm/avm/trace/kernel_trace.hpp" @@ -336,23 +337,33 @@ std::vector Execution::gen_trace(AvmPublicInputs const& public_inputs, if (phase == TxExecutionPhase::SETUP) { vinfo("Inserting non-revertible side effects from private before SETUP phase. Checkpointing trees."); // Temporary spot for private non-revertible insertion - std::vector siloed_nullifiers; - siloed_nullifiers.insert( - siloed_nullifiers.end(), - public_inputs.previous_non_revertible_accumulated_data.nullifiers.begin(), - public_inputs.previous_non_revertible_accumulated_data.nullifiers.begin() + - public_inputs.previous_non_revertible_accumulated_data_array_lengths.nullifiers); - trace_builder.insert_private_state(siloed_nullifiers, {}); + auto siloed_nullifiers = + std::vector(public_inputs.previous_non_revertible_accumulated_data.nullifiers.begin(), + public_inputs.previous_non_revertible_accumulated_data.nullifiers.begin() + + public_inputs.previous_non_revertible_accumulated_data_array_lengths.nullifiers); + + auto unique_note_hashes = + std::vector(public_inputs.previous_non_revertible_accumulated_data.note_hashes.begin(), + public_inputs.previous_non_revertible_accumulated_data.note_hashes.begin() + + public_inputs.previous_non_revertible_accumulated_data_array_lengths.note_hashes); + + trace_builder.insert_private_state(siloed_nullifiers, unique_note_hashes); + trace_builder.checkpoint_non_revertible_state(); } else if (phase == TxExecutionPhase::APP_LOGIC) { vinfo("Inserting revertible side effects from private before APP_LOGIC phase"); // Temporary spot for private revertible insertion - std::vector siloed_nullifiers; - siloed_nullifiers.insert(siloed_nullifiers.end(), - public_inputs.previous_revertible_accumulated_data.nullifiers.begin(), - public_inputs.previous_revertible_accumulated_data.nullifiers.begin() + - public_inputs.previous_revertible_accumulated_data_array_lengths.nullifiers); - trace_builder.insert_private_state(siloed_nullifiers, {}); + auto siloed_nullifiers = + std::vector(public_inputs.previous_revertible_accumulated_data.nullifiers.begin(), + public_inputs.previous_revertible_accumulated_data.nullifiers.begin() + + public_inputs.previous_revertible_accumulated_data_array_lengths.nullifiers); + + auto siloed_note_hashes = + std::vector(public_inputs.previous_revertible_accumulated_data.note_hashes.begin(), + public_inputs.previous_revertible_accumulated_data.note_hashes.begin() + + public_inputs.previous_revertible_accumulated_data_array_lengths.note_hashes); + + trace_builder.insert_private_revertible_state(siloed_nullifiers, siloed_note_hashes); } vinfo("Beginning execution of phase ", to_name(phase), " (", public_call_requests.size(), " enqueued calls)."); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.cpp index cff31389fc1..f3332c79a69 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.cpp @@ -34,6 +34,16 @@ FF AvmMerkleTreeTraceBuilder::unconstrained_silo_note_hash(FF contract_address, return Poseidon2::hash({ GENERATOR_INDEX__SILOED_NOTE_HASH, contract_address, note_hash }); } +FF AvmMerkleTreeTraceBuilder::unconstrained_compute_note_hash_nonce(FF tx_hash, FF note_index_in_tx) +{ + return Poseidon2::hash({ GENERATOR_INDEX__NOTE_HASH_NONCE, tx_hash, note_index_in_tx }); +} + +FF AvmMerkleTreeTraceBuilder::unconstrained_compute_unique_note_hash(FF nonce, FF siloed_note_hash) +{ + return Poseidon2::hash({ GENERATOR_INDEX__UNIQUE_NOTE_HASH, nonce, siloed_note_hash }); +} + FF AvmMerkleTreeTraceBuilder::unconstrained_silo_nullifier(FF contract_address, FF nullifier) { return Poseidon2::hash({ GENERATOR_INDEX__OUTER_NULLIFIER, contract_address, nullifier }); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.hpp index 354ef169df4..581ee0660a5 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/gadgets/merkle_tree.hpp @@ -99,6 +99,8 @@ class AvmMerkleTreeTraceBuilder { static FF unconstrained_hash_public_data_preimage(const PublicDataTreeLeafPreimage& preimage); static FF unconstrained_silo_note_hash(FF contract_address, FF note_hash); + static FF unconstrained_compute_note_hash_nonce(FF tx_hash, FF note_index_in_tx); + static FF unconstrained_compute_unique_note_hash(FF nonce, FF siloed_note_hash); static FF unconstrained_silo_nullifier(FF contract_address, FF nullifier); static FF unconstrained_compute_public_tree_leaf_slot(FF contract_address, FF leaf_index); // Could probably template these diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp index ad890c865b1..747d79d722f 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp @@ -213,8 +213,14 @@ std::vector AvmTraceBuilder::get_bytecode(const FF contract_address, bo throw std::runtime_error("Bytecode not found"); } +uint32_t AvmTraceBuilder::get_inserted_note_hashes_count() +{ + return merkle_tree_trace_builder.get_tree_snapshots().note_hash_tree.size - + public_inputs.start_tree_snapshots.note_hash_tree.size; +} + void AvmTraceBuilder::insert_private_state(const std::vector& siloed_nullifiers, - [[maybe_unused]] const std::vector& siloed_note_hashes) + const std::vector& unique_note_hashes) { for (const auto& siloed_nullifier : siloed_nullifiers) { auto hint = execution_hints.nullifier_write_hints[nullifier_write_counter++]; @@ -225,6 +231,28 @@ void AvmTraceBuilder::insert_private_state(const std::vector& siloed_nullifi siloed_nullifier, hint.insertion_path); } + + for (const auto& unique_note_hash : unique_note_hashes) { + auto hint = execution_hints.note_hash_write_hints[note_hash_write_counter++]; + merkle_tree_trace_builder.perform_note_hash_append(0, unique_note_hash, hint.sibling_path); + } +} + +void AvmTraceBuilder::insert_private_revertible_state(const std::vector& siloed_nullifiers, + const std::vector& siloed_note_hashes) +{ + // Revertibles come only siloed from private, so we need to make them unique here + std::vector unique_note_hashes; + unique_note_hashes.reserve(siloed_note_hashes.size()); + + for (size_t i = 0; i < siloed_note_hashes.size(); i++) { + size_t note_index_in_tx = i + get_inserted_note_hashes_count(); + FF nonce = AvmMerkleTreeTraceBuilder::unconstrained_compute_note_hash_nonce(get_tx_hash(), note_index_in_tx); + unique_note_hashes.push_back( + AvmMerkleTreeTraceBuilder::unconstrained_compute_unique_note_hash(nonce, siloed_note_hashes.at(i))); + } + + insert_private_state(siloed_nullifiers, unique_note_hashes); } void AvmTraceBuilder::pay_fee() @@ -2776,8 +2804,8 @@ AvmError AvmTraceBuilder::op_note_hash_exists(uint8_t indirect, AvmError AvmTraceBuilder::op_emit_note_hash(uint8_t indirect, uint32_t note_hash_offset) { auto const clk = static_cast(main_trace.size()) + 1; - - if (note_hash_write_counter >= MAX_NOTE_HASHES_PER_TX) { + uint32_t inserted_note_hashes_count = get_inserted_note_hashes_count(); + if (inserted_note_hashes_count >= MAX_NOTE_HASHES_PER_TX) { AvmError error = AvmError::SIDE_EFFECT_LIMIT_REACHED; auto row = Row{ .main_clk = clk, @@ -2797,16 +2825,20 @@ AvmError AvmTraceBuilder::op_emit_note_hash(uint8_t indirect, uint32_t note_hash row.main_op_err = FF(static_cast(!is_ok(error))); AppendTreeHint note_hash_write_hint = execution_hints.note_hash_write_hints.at(note_hash_write_counter++); - auto siloed_note_hash = AvmMerkleTreeTraceBuilder::unconstrained_silo_note_hash( + FF siloed_note_hash = AvmMerkleTreeTraceBuilder::unconstrained_silo_note_hash( current_public_call_request.contract_address, row.main_ia); - ASSERT(row.main_ia == note_hash_write_hint.leaf_value); + FF nonce = + AvmMerkleTreeTraceBuilder::unconstrained_compute_note_hash_nonce(get_tx_hash(), inserted_note_hashes_count); + FF unique_note_hash = AvmMerkleTreeTraceBuilder::unconstrained_compute_unique_note_hash(nonce, siloed_note_hash); + + ASSERT(unique_note_hash == note_hash_write_hint.leaf_value); // We first check that the index is currently empty bool insert_index_is_empty = merkle_tree_trace_builder.perform_note_hash_read( clk, FF::zero(), note_hash_write_hint.leaf_index, note_hash_write_hint.sibling_path); ASSERT(insert_index_is_empty); // Update the root with the new leaf that is appended - merkle_tree_trace_builder.perform_note_hash_append(clk, siloed_note_hash, note_hash_write_hint.sibling_path); + merkle_tree_trace_builder.perform_note_hash_append(clk, unique_note_hash, note_hash_write_hint.sibling_path); // Constrain gas cost gas_trace_builder.constrain_gas(clk, OpCode::EMITNOTEHASH); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp index 5e6af27c2a9..fa0d1f89833 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp @@ -234,7 +234,9 @@ class AvmTraceBuilder { void rollback_to_non_revertible_checkpoint(); std::vector get_bytecode(const FF contract_address, bool check_membership = false); std::unordered_set bytecode_membership_cache; - void insert_private_state(const std::vector& siloed_nullifiers, const std::vector& siloed_note_hashes); + void insert_private_state(const std::vector& siloed_nullifiers, const std::vector& unique_note_hashes); + void insert_private_revertible_state(const std::vector& siloed_nullifiers, + const std::vector& siloed_note_hashes); void pay_fee(); // These are used for testing only. @@ -359,6 +361,8 @@ class AvmTraceBuilder { AvmMemoryTag write_tag, IntermRegister reg, AvmMemTraceBuilder::MemOpOwner mem_op_owner = AvmMemTraceBuilder::MAIN); + uint32_t get_inserted_note_hashes_count(); + FF get_tx_hash() const { return public_inputs.previous_non_revertible_accumulated_data.nullifiers[0]; } // TODO: remove these once everything is constrained. AvmMemoryTag unconstrained_get_memory_tag(AddressWithMode addr); diff --git a/yarn-project/circuits.js/src/hash/hash.ts b/yarn-project/circuits.js/src/hash/hash.ts index a6ca04d5227..79fd9d4d98e 100644 --- a/yarn-project/circuits.js/src/hash/hash.ts +++ b/yarn-project/circuits.js/src/hash/hash.ts @@ -39,11 +39,11 @@ export function siloNoteHash(contract: AztecAddress, uniqueNoteHash: Fr): Fr { * Computes a unique note hash. * @dev Includes a nonce which contains data that guarantees the resulting note hash will be unique. * @param nonce - A nonce (typically derived from tx hash and note hash index in the tx). - * @param noteHash - A note hash. + * @param siloedNoteHash - A note hash. * @returns A unique note hash. */ -export function computeUniqueNoteHash(nonce: Fr, noteHash: Fr): Fr { - return poseidon2HashWithSeparator([nonce, noteHash], GeneratorIndex.UNIQUE_NOTE_HASH); +export function computeUniqueNoteHash(nonce: Fr, siloedNoteHash: Fr): Fr { + return poseidon2HashWithSeparator([nonce, siloedNoteHash], GeneratorIndex.UNIQUE_NOTE_HASH); } /** diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index e35b4d2519f..8b6f89fcca5 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -8,7 +8,14 @@ import { SerializableContractInstance, } from '@aztec/circuits.js'; import { Grumpkin } from '@aztec/circuits.js/barretenberg'; -import { computePublicDataTreeLeafSlot, computeVarArgsHash, siloNullifier } from '@aztec/circuits.js/hash'; +import { + computeNoteHashNonce, + computePublicDataTreeLeafSlot, + computeUniqueNoteHash, + computeVarArgsHash, + siloNoteHash, + siloNullifier, +} from '@aztec/circuits.js/hash'; import { makeContractClassPublic, makeContractInstanceFromClassId } from '@aztec/circuits.js/testing'; import { FunctionSelector } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; @@ -66,6 +73,7 @@ import { mockGetContractClass, mockGetContractInstance, mockL1ToL2MessageExists, + mockNoteHashCount, mockNoteHashExists, mockNullifierExists, mockStorageRead, @@ -155,6 +163,7 @@ describe('AVM simulator: transpiled Noir contracts', () => { const trace = mock(); const nestedTrace = mock(); + mockNoteHashCount(trace, 0); mockTraceFork(trace, nestedTrace); const ephemeralTrees = await AvmEphemeralForest.create(worldStateDB.getMerkleInterface()); const persistableState = initPersistableStateManager({ worldStateDB, trace, merkleTrees: ephemeralTrees }); @@ -621,13 +630,17 @@ describe('AVM simulator: transpiled Noir contracts', () => { const calldata = [value0]; const context = createContext(calldata); const bytecode = getAvmTestContractBytecode('new_note_hash'); + mockNoteHashCount(trace, 0); const results = await new AvmSimulator(context).executeBytecode(bytecode); expect(results.reverted).toBe(false); expect(results.output).toEqual([]); expect(trace.traceNewNoteHash).toHaveBeenCalledTimes(1); - expect(trace.traceNewNoteHash).toHaveBeenCalledWith(expect.objectContaining(address), /*noteHash=*/ value0); + const siloedNotehash = siloNoteHash(address, value0); + const nonce = computeNoteHashNonce(Fr.fromBuffer(context.persistableState.txHash.toBuffer()), 0); + const uniqueNoteHash = computeUniqueNoteHash(nonce, siloedNotehash); + expect(trace.traceNewNoteHash).toHaveBeenCalledWith(uniqueNoteHash); }); it('Should append a new nullifier correctly', async () => { diff --git a/yarn-project/simulator/src/avm/fixtures/index.ts b/yarn-project/simulator/src/avm/fixtures/index.ts index ac84ef7d702..53a259419a8 100644 --- a/yarn-project/simulator/src/avm/fixtures/index.ts +++ b/yarn-project/simulator/src/avm/fixtures/index.ts @@ -1,4 +1,4 @@ -import { isNoirCallStackUnresolved } from '@aztec/circuit-types'; +import { TxHash, isNoirCallStackUnresolved } from '@aztec/circuit-types'; import { GasFees, GlobalVariables, MAX_L2_GAS_PER_TX_PUBLIC_PORTION } from '@aztec/circuits.js'; import { type FunctionArtifact, FunctionSelector } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; @@ -45,6 +45,7 @@ export function initPersistableStateManager(overrides?: { nullifiers?: NullifierManager; doMerkleOperations?: boolean; merkleTrees?: AvmEphemeralForest; + txHash?: TxHash; }): AvmPersistableStateManager { const worldStateDB = overrides?.worldStateDB || mock(); return new AvmPersistableStateManager( @@ -54,6 +55,7 @@ export function initPersistableStateManager(overrides?: { overrides?.nullifiers || new NullifierManager(worldStateDB), overrides?.doMerkleOperations || false, overrides?.merkleTrees || mock(), + overrides?.txHash || new TxHash(new Fr(27).toBuffer()), ); } diff --git a/yarn-project/simulator/src/avm/journal/journal.test.ts b/yarn-project/simulator/src/avm/journal/journal.test.ts index dd50228ab55..9aae7f88889 100644 --- a/yarn-project/simulator/src/avm/journal/journal.test.ts +++ b/yarn-project/simulator/src/avm/journal/journal.test.ts @@ -1,5 +1,5 @@ import { AztecAddress, SerializableContractInstance, computePublicBytecodeCommitment } from '@aztec/circuits.js'; -import { siloNullifier } from '@aztec/circuits.js/hash'; +import { computeNoteHashNonce, computeUniqueNoteHash, siloNoteHash, siloNullifier } from '@aztec/circuits.js/hash'; import { makeContractClassPublic } from '@aztec/circuits.js/testing'; import { Fr } from '@aztec/foundation/fields'; @@ -13,6 +13,7 @@ import { mockGetContractClass, mockGetContractInstance, mockL1ToL2MessageExists, + mockNoteHashCount, mockNoteHashExists, mockNullifierExists, mockStorageRead, @@ -80,9 +81,13 @@ describe('journal', () => { }); it('writeNoteHash works', () => { + mockNoteHashCount(trace, 1); persistableState.writeNoteHash(address, utxo); expect(trace.traceNewNoteHash).toHaveBeenCalledTimes(1); - expect(trace.traceNewNoteHash).toHaveBeenCalledWith(expect.objectContaining(address), /*noteHash=*/ utxo); + const siloedNotehash = siloNoteHash(address, utxo); + const nonce = computeNoteHashNonce(Fr.fromBuffer(persistableState.txHash.toBuffer()), 1); + const uniqueNoteHash = computeUniqueNoteHash(nonce, siloedNotehash); + expect(trace.traceNewNoteHash).toHaveBeenCalledWith(uniqueNoteHash); }); it('checkNullifierExists works for missing nullifiers', async () => { diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts index 2d0ab2f7a5f..35d349add3b 100644 --- a/yarn-project/simulator/src/avm/journal/journal.ts +++ b/yarn-project/simulator/src/avm/journal/journal.ts @@ -1,4 +1,4 @@ -import { MerkleTreeId } from '@aztec/circuit-types'; +import { MerkleTreeId, type TxHash } from '@aztec/circuit-types'; import { AztecAddress, CANONICAL_AUTH_REGISTRY_ADDRESS, @@ -12,7 +12,13 @@ import { ROUTER_ADDRESS, SerializableContractInstance, } from '@aztec/circuits.js'; -import { computePublicDataTreeLeafSlot, siloNoteHash, siloNullifier } from '@aztec/circuits.js/hash'; +import { + computeNoteHashNonce, + computePublicDataTreeLeafSlot, + computeUniqueNoteHash, + siloNoteHash, + siloNullifier, +} from '@aztec/circuits.js/hash'; import { Fr } from '@aztec/foundation/fields'; import { jsonStringify } from '@aztec/foundation/json-rpc'; import { createLogger } from '@aztec/foundation/log'; @@ -55,6 +61,7 @@ export class AvmPersistableStateManager { private readonly doMerkleOperations: boolean = false, /** Ephmeral forest for merkle tree operations */ public merkleTrees: AvmEphemeralForest, + public readonly txHash: TxHash, ) {} /** @@ -65,6 +72,7 @@ export class AvmPersistableStateManager { trace: PublicSideEffectTraceInterface, pendingSiloedNullifiers: Fr[], doMerkleOperations: boolean = false, + txHash: TxHash, ) { const parentNullifiers = NullifierManager.newWithPendingSiloedNullifiers(worldStateDB, pendingSiloedNullifiers); const ephemeralForest = await AvmEphemeralForest.create(worldStateDB.getMerkleInterface()); @@ -75,6 +83,7 @@ export class AvmPersistableStateManager { /*nullifiers=*/ parentNullifiers.fork(), doMerkleOperations, ephemeralForest, + txHash, ); } @@ -85,6 +94,7 @@ export class AvmPersistableStateManager { worldStateDB: WorldStateDB, trace: PublicSideEffectTraceInterface, doMerkleOperations: boolean = false, + txHash: TxHash, ) { const ephemeralForest = await AvmEphemeralForest.create(worldStateDB.getMerkleInterface()); return new AvmPersistableStateManager( @@ -94,6 +104,7 @@ export class AvmPersistableStateManager { /*nullifiers=*/ new NullifierManager(worldStateDB), /*doMerkleOperations=*/ doMerkleOperations, ephemeralForest, + txHash, ); } @@ -108,6 +119,7 @@ export class AvmPersistableStateManager { this.nullifiers.fork(), this.doMerkleOperations, this.merkleTrees.fork(), + this.txHash, ); } @@ -290,20 +302,41 @@ export class AvmPersistableStateManager { } /** - * Write a note hash, trace the write. + * Write a raw note hash, silo it and make it unique, then trace the write. * @param noteHash - the unsiloed note hash to write */ public writeNoteHash(contractAddress: AztecAddress, noteHash: Fr): void { - this.log.debug(`noteHashes(${contractAddress}) += @${noteHash}.`); + const siloedNoteHash = siloNoteHash(contractAddress, noteHash); + + this.writeSiloedNoteHash(siloedNoteHash); + } + + /** + * Write a note hash, make it unique, trace the write. + * @param noteHash - the non unique note hash to write + */ + public writeSiloedNoteHash(noteHash: Fr): void { + const txHash = Fr.fromBuffer(this.txHash.toBuffer()); + const nonce = computeNoteHashNonce(txHash, this.trace.getNoteHashCount()); + const uniqueNoteHash = computeUniqueNoteHash(nonce, noteHash); + + this.writeUniqueNoteHash(uniqueNoteHash); + } + + /** + * Write a note hash, trace the write. + * @param noteHash - the siloed unique hash to write + */ + public writeUniqueNoteHash(noteHash: Fr): void { + this.log.debug(`noteHashes += @${noteHash}.`); if (this.doMerkleOperations) { // Should write a helper for this const leafIndex = new Fr(this.merkleTrees.treeMap.get(MerkleTreeId.NOTE_HASH_TREE)!.leafCount); - const siloedNoteHash = siloNoteHash(contractAddress, noteHash); - const insertionPath = this.merkleTrees.appendNoteHash(siloedNoteHash); - this.trace.traceNewNoteHash(contractAddress, noteHash, leafIndex, insertionPath); + const insertionPath = this.merkleTrees.appendNoteHash(noteHash); + this.trace.traceNewNoteHash(noteHash, leafIndex, insertionPath); } else { - this.trace.traceNewNoteHash(contractAddress, noteHash); + this.trace.traceNewNoteHash(noteHash); } } diff --git a/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts b/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts index f38a335ebd5..af4fd095786 100644 --- a/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/accrued_substate.test.ts @@ -1,5 +1,5 @@ import { AztecAddress, Fr } from '@aztec/circuits.js'; -import { siloNullifier } from '@aztec/circuits.js/hash'; +import { computeNoteHashNonce, computeUniqueNoteHash, siloNoteHash, siloNullifier } from '@aztec/circuits.js/hash'; import { mock } from 'jest-mock-extended'; @@ -10,7 +10,7 @@ import { Field, Uint8, Uint32 } from '../avm_memory_types.js'; import { InstructionExecutionError, StaticCallAlterationError } from '../errors.js'; import { initContext, initExecutionEnvironment, initPersistableStateManager } from '../fixtures/index.js'; import { type AvmPersistableStateManager } from '../journal/journal.js'; -import { mockL1ToL2MessageExists, mockNoteHashExists, mockNullifierExists } from '../test_utils.js'; +import { mockL1ToL2MessageExists, mockNoteHashCount, mockNoteHashExists, mockNullifierExists } from '../test_utils.js'; import { EmitNoteHash, EmitNullifier, @@ -120,10 +120,14 @@ describe('Accrued Substate', () => { }); it('Should append a new note hash correctly', async () => { + mockNoteHashCount(trace, 0); context.machineState.memory.set(value0Offset, new Field(value0)); await new EmitNoteHash(/*indirect=*/ 0, /*offset=*/ value0Offset).execute(context); expect(trace.traceNewNoteHash).toHaveBeenCalledTimes(1); - expect(trace.traceNewNoteHash).toHaveBeenCalledWith(expect.objectContaining(address), /*noteHash=*/ value0); + const siloedNotehash = siloNoteHash(address, value0); + const nonce = computeNoteHashNonce(Fr.fromBuffer(context.persistableState.txHash.toBuffer()), 0); + const uniqueNoteHash = computeUniqueNoteHash(nonce, siloedNotehash); + expect(trace.traceNewNoteHash).toHaveBeenCalledWith(uniqueNoteHash); }); }); diff --git a/yarn-project/simulator/src/avm/test_utils.ts b/yarn-project/simulator/src/avm/test_utils.ts index 48afe3c9e32..27785930567 100644 --- a/yarn-project/simulator/src/avm/test_utils.ts +++ b/yarn-project/simulator/src/avm/test_utils.ts @@ -28,6 +28,10 @@ export function mockStorageRead(worldStateDB: WorldStateDB, value: Fr) { (worldStateDB as jest.Mocked).storageRead.mockResolvedValue(value); } +export function mockNoteHashCount(mockedTrace: PublicSideEffectTraceInterface, count: number) { + (mockedTrace as jest.Mocked).getNoteHashCount.mockReturnValue(count); +} + export function mockStorageReadWithMap(worldStateDB: WorldStateDB, mockedStorage: Map) { (worldStateDB as jest.Mocked).storageRead.mockImplementation((_address, slot) => Promise.resolve(mockedStorage.get(slot.toBigInt()) ?? Fr.ZERO), diff --git a/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.test.ts b/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.test.ts index 7ede684befe..a6b34e58e13 100644 --- a/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.test.ts +++ b/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.test.ts @@ -99,10 +99,10 @@ describe('Enqueued-call Side Effect Trace', () => { }); it('Should trace note hashes', () => { - trace.traceNewNoteHash(address, utxo, leafIndex, siblingPath); + trace.traceNewNoteHash(utxo, leafIndex, siblingPath); expect(trace.getCounter()).toBe(startCounterPlus1); - const expected = [new NoteHash(utxo, startCounter).scope(address)]; + const expected = [new NoteHash(utxo, startCounter)]; expect(trace.getSideEffects().noteHashes).toEqual(expected); const expectedHint = new AvmAppendTreeHint(leafIndex, utxo, siblingPath); @@ -285,11 +285,9 @@ describe('Enqueued-call Side Effect Trace', () => { it('Should enforce maximum number of new note hashes', () => { for (let i = 0; i < MAX_NOTE_HASHES_PER_TX; i++) { - trace.traceNewNoteHash(AztecAddress.fromNumber(i), new Fr(i), Fr.ZERO, []); + trace.traceNewNoteHash(new Fr(i), Fr.ZERO, []); } - expect(() => trace.traceNewNoteHash(AztecAddress.fromNumber(42), new Fr(42), Fr.ZERO, [])).toThrow( - SideEffectLimitReachedError, - ); + expect(() => trace.traceNewNoteHash(new Fr(42), Fr.ZERO, [])).toThrow(SideEffectLimitReachedError); }); it('Should enforce maximum number of new nullifiers', () => { @@ -339,9 +337,7 @@ describe('Enqueued-call Side Effect Trace', () => { expect(() => trace.tracePublicStorageWrite(AztecAddress.fromNumber(42), new Fr(42), new Fr(42), true)).toThrow( SideEffectLimitReachedError, ); - expect(() => trace.traceNewNoteHash(AztecAddress.fromNumber(42), new Fr(42), new Fr(42))).toThrow( - SideEffectLimitReachedError, - ); + expect(() => trace.traceNewNoteHash(new Fr(42), new Fr(42))).toThrow(SideEffectLimitReachedError); expect(() => trace.traceNewNullifier(new Fr(42))).toThrow(SideEffectLimitReachedError); expect(() => trace.traceNewL2ToL1Message(AztecAddress.fromNumber(42), new Fr(42), new Fr(42))).toThrow( SideEffectLimitReachedError, @@ -366,7 +362,7 @@ describe('Enqueued-call Side Effect Trace', () => { testCounter++; nestedTrace.traceNoteHashCheck(address, utxo, leafIndex, existsDefault, []); // counter does not increment for note hash checks - nestedTrace.traceNewNoteHash(address, utxo, Fr.ZERO, []); + nestedTrace.traceNewNoteHash(utxo, Fr.ZERO, []); testCounter++; nestedTrace.traceNullifierCheck(utxo, true, lowLeafPreimage, Fr.ZERO, []); testCounter++; diff --git a/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts b/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts index 5ff5e7427c1..040ebbaaa66 100644 --- a/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts +++ b/yarn-project/simulator/src/public/enqueued_call_side_effect_trace.ts @@ -42,7 +42,6 @@ import { PublicDataWrite, ScopedL2ToL1Message, ScopedLogHash, - type ScopedNoteHash, SerializableContractInstance, type TreeSnapshots, } from '@aztec/circuits.js'; @@ -74,7 +73,7 @@ export type SideEffects = { enqueuedCalls: PublicCallRequest[]; publicDataWrites: PublicDataUpdateRequest[]; - noteHashes: ScopedNoteHash[]; + noteHashes: NoteHash[]; nullifiers: Nullifier[]; l2ToL1Msgs: ScopedL2ToL1Message[]; @@ -111,7 +110,7 @@ export class PublicEnqueuedCallSideEffectTrace implements PublicSideEffectTraceI private publicDataWrites: PublicDataUpdateRequest[] = []; private protocolPublicDataWritesLength: number = 0; private userPublicDataWritesLength: number = 0; - private noteHashes: ScopedNoteHash[] = []; + private noteHashes: NoteHash[] = []; private nullifiers: Nullifier[] = []; private l2ToL1Messages: ScopedL2ToL1Message[] = []; private unencryptedLogs: UnencryptedL2Log[] = []; @@ -194,6 +193,10 @@ export class PublicEnqueuedCallSideEffectTrace implements PublicSideEffectTraceI this.sideEffectCounter++; } + public getNoteHashCount() { + return this.previousSideEffectArrayLengths.noteHashes + this.noteHashes.length; + } + public tracePublicStorageRead( contractAddress: AztecAddress, slot: Fr, @@ -272,19 +275,12 @@ export class PublicEnqueuedCallSideEffectTrace implements PublicSideEffectTraceI // NOTE: counter does not increment for note hash checks (because it doesn't rely on pending note hashes) } - public traceNewNoteHash( - contractAddress: AztecAddress, - noteHash: Fr, - leafIndex: Fr = Fr.zero(), - path: Fr[] = emptyNoteHashPath(), - ) { + public traceNewNoteHash(noteHash: Fr, leafIndex: Fr = Fr.zero(), path: Fr[] = emptyNoteHashPath()) { if (this.noteHashes.length + this.previousSideEffectArrayLengths.noteHashes >= MAX_NOTE_HASHES_PER_TX) { throw new SideEffectLimitReachedError('note hash', MAX_NOTE_HASHES_PER_TX); } - // TODO(dbanks12): make unique and silo instead of scoping - //const siloedNoteHash = siloNoteHash(contractAddress, noteHash); - this.noteHashes.push(new NoteHash(noteHash, this.sideEffectCounter).scope(contractAddress)); + this.noteHashes.push(new NoteHash(noteHash, this.sideEffectCounter)); this.log.debug(`NEW_NOTE_HASH cnt: ${this.sideEffectCounter}`); this.avmCircuitHints.noteHashWrites.items.push(new AvmAppendTreeHint(leafIndex, noteHash, path)); this.incrementSideEffectCounter(); diff --git a/yarn-project/simulator/src/public/execution.ts b/yarn-project/simulator/src/public/execution.ts index e6fe8c17457..cf9832201a2 100644 --- a/yarn-project/simulator/src/public/execution.ts +++ b/yarn-project/simulator/src/public/execution.ts @@ -20,7 +20,6 @@ import { RevertCode, type ScopedL2ToL1Message, type ScopedLogHash, - type ScopedNoteHash, type TreeLeafReadRequest, } from '@aztec/circuits.js'; import { computeVarArgsHash } from '@aztec/circuits.js/hash'; @@ -29,7 +28,7 @@ export interface PublicSideEffects { /** The contract storage update requests performed. */ publicDataWrites: PublicDataUpdateRequest[]; /** The new note hashes to be inserted into the note hashes tree. */ - noteHashes: ScopedNoteHash[]; + noteHashes: NoteHash[]; /** The new nullifiers to be inserted into the nullifier tree. */ nullifiers: Nullifier[]; /** The new l2 to l1 messages generated to be inserted into the messages tree. */ diff --git a/yarn-project/simulator/src/public/public_tx_context.ts b/yarn-project/simulator/src/public/public_tx_context.ts index dc4807dcb2f..185327d5a87 100644 --- a/yarn-project/simulator/src/public/public_tx_context.ts +++ b/yarn-project/simulator/src/public/public_tx_context.ts @@ -91,7 +91,7 @@ export class PublicTxContext { const previousAccumulatedDataArrayLengths = new SideEffectArrayLengths( /*publicDataWrites*/ 0, /*protocolPublicDataWrites*/ 0, - countAccumulatedItems(nonRevertibleAccumulatedDataFromPrivate.noteHashes), + /*noteHashes*/ 0, /*nullifiers=*/ 0, countAccumulatedItems(nonRevertibleAccumulatedDataFromPrivate.l2ToL1Msgs), /*unencryptedLogsHashes*/ 0, @@ -102,7 +102,12 @@ export class PublicTxContext { ); // Transaction level state manager that will be forked for revertible phases. - const txStateManager = await AvmPersistableStateManager.create(worldStateDB, enqueuedCallTrace, doMerkleOperations); + const txStateManager = await AvmPersistableStateManager.create( + worldStateDB, + enqueuedCallTrace, + doMerkleOperations, + fetchTxHash(nonRevertibleAccumulatedDataFromPrivate), + ); const gasSettings = tx.data.constants.txContext.gasSettings; const gasUsedByPrivate = tx.data.gasUsed; @@ -186,12 +191,7 @@ export class PublicTxContext { * @returns The transaction's hash. */ getTxHash(): TxHash { - // Private kernel functions are executed client side and for this reason tx hash is already set as first nullifier - const firstNullifier = this.nonRevertibleAccumulatedDataFromPrivate.nullifiers[0]; - if (!firstNullifier || firstNullifier.isZero()) { - throw new Error(`Cannot get tx hash since first nullifier is missing`); - } - return new TxHash(firstNullifier.toBuffer()); + return fetchTxHash(this.nonRevertibleAccumulatedDataFromPrivate); } /** @@ -425,3 +425,12 @@ function applyMaxToAvailableGas(availableGas: Gas) { /*l2Gas=*/ Math.min(availableGas.l2Gas, MAX_L2_GAS_PER_TX_PUBLIC_PORTION), ); } + +function fetchTxHash(nonRevertibleAccumulatedData: PrivateToPublicAccumulatedData): TxHash { + // Private kernel functions are executed client side and for this reason tx hash is already set as first nullifier + const firstNullifier = nonRevertibleAccumulatedData.nullifiers[0]; + if (!firstNullifier || firstNullifier.isZero()) { + throw new Error(`Cannot get tx hash since first nullifier is missing`); + } + return new TxHash(firstNullifier.toBuffer()); +} diff --git a/yarn-project/simulator/src/public/public_tx_simulator.ts b/yarn-project/simulator/src/public/public_tx_simulator.ts index 37921773c48..24bc8089bdc 100644 --- a/yarn-project/simulator/src/public/public_tx_simulator.ts +++ b/yarn-project/simulator/src/public/public_tx_simulator.ts @@ -378,6 +378,11 @@ export class PublicTxSimulator { ); } } + for (const noteHash of context.nonRevertibleAccumulatedDataFromPrivate.noteHashes) { + if (!noteHash.isEmpty()) { + stateManager.writeUniqueNoteHash(noteHash); + } + } } /** @@ -397,6 +402,12 @@ export class PublicTxSimulator { ); } } + for (const noteHash of context.revertibleAccumulatedDataFromPrivate.noteHashes) { + if (!noteHash.isEmpty()) { + // Revertible note hashes from private are not hashed with nonce, since private can't know their final position, only we can. + stateManager.writeSiloedNoteHash(noteHash); + } + } } private async payFee(context: PublicTxContext) { diff --git a/yarn-project/simulator/src/public/side_effect_trace_interface.ts b/yarn-project/simulator/src/public/side_effect_trace_interface.ts index d422f5fdb82..a08aa391d13 100644 --- a/yarn-project/simulator/src/public/side_effect_trace_interface.ts +++ b/yarn-project/simulator/src/public/side_effect_trace_interface.ts @@ -39,7 +39,8 @@ export interface PublicSideEffectTraceInterface { insertionPath?: Fr[], ): void; traceNoteHashCheck(contractAddress: AztecAddress, noteHash: Fr, leafIndex: Fr, exists: boolean, path?: Fr[]): void; - traceNewNoteHash(contractAddress: AztecAddress, noteHash: Fr, leafIndex?: Fr, path?: Fr[]): void; + traceNewNoteHash(uniqueNoteHash: Fr, leafIndex?: Fr, path?: Fr[]): void; + getNoteHashCount(): number; traceNullifierCheck( siloedNullifier: Fr, exists: boolean, diff --git a/yarn-project/simulator/src/public/transitional_adapters.ts b/yarn-project/simulator/src/public/transitional_adapters.ts index 36f700166b4..d6fcfea0c66 100644 --- a/yarn-project/simulator/src/public/transitional_adapters.ts +++ b/yarn-project/simulator/src/public/transitional_adapters.ts @@ -6,7 +6,6 @@ import { type GasSettings, type GlobalVariables, MAX_L2_TO_L1_MSGS_PER_TX, - MAX_NOTE_HASHES_PER_TX, MAX_TOTAL_PUBLIC_DATA_UPDATE_REQUESTS_PER_TX, PrivateToAvmAccumulatedData, PrivateToAvmAccumulatedDataArrayLengths, @@ -19,7 +18,6 @@ import { countAccumulatedItems, mergeAccumulatedData, } from '@aztec/circuits.js'; -import { computeNoteHashNonce, computeUniqueNoteHash, siloNoteHash } from '@aztec/circuits.js/hash'; import { padArrayEnd } from '@aztec/foundation/collection'; import { assertLength } from '@aztec/foundation/serialize'; @@ -86,55 +84,6 @@ export function generateAvmCircuitPublicInputs( revertibleAccumulatedDataFromPrivate, ); - const txHash = avmCircuitPublicInputs.previousNonRevertibleAccumulatedData.nullifiers[0]; - - // Add nonces to revertible note hashes from private. These don't have nonces since we don't know - // the final position in the tx until the AVM has executed. - // TODO: Use the final position in the tx - for ( - let revertibleIndex = 0; - revertibleIndex < avmCircuitPublicInputs.previousRevertibleAccumulatedData.noteHashes.length; - revertibleIndex++ - ) { - const noteHash = avmCircuitPublicInputs.previousRevertibleAccumulatedData.noteHashes[revertibleIndex]; - if (noteHash.isZero()) { - continue; - } - const indexInTx = - revertibleIndex + avmCircuitPublicInputs.previousNonRevertibleAccumulatedDataArrayLengths.noteHashes; - - const nonce = computeNoteHashNonce(txHash, indexInTx); - const uniqueNoteHash = computeUniqueNoteHash(nonce, noteHash); - avmCircuitPublicInputs.previousRevertibleAccumulatedData.noteHashes[revertibleIndex] = uniqueNoteHash; - } - - // merge all revertible & non-revertible side effects into output accumulated data - const noteHashesFromPrivate = revertCode.isOK() - ? mergeAccumulatedData( - avmCircuitPublicInputs.previousNonRevertibleAccumulatedData.noteHashes, - avmCircuitPublicInputs.previousRevertibleAccumulatedData.noteHashes, - ) - : avmCircuitPublicInputs.previousNonRevertibleAccumulatedData.noteHashes; - avmCircuitPublicInputs.accumulatedData.noteHashes = assertLength( - mergeAccumulatedData(noteHashesFromPrivate, avmCircuitPublicInputs.accumulatedData.noteHashes), - MAX_NOTE_HASHES_PER_TX, - ); - - // Silo and add nonces for note hashes emitted by the AVM - const scopedNoteHashesFromPublic = trace.getSideEffects().noteHashes; - for (let i = 0; i < scopedNoteHashesFromPublic.length; i++) { - const scopedNoteHash = scopedNoteHashesFromPublic[i]; - const noteHash = scopedNoteHash.value; - if (!noteHash.isZero()) { - const noteHashIndexInTx = i + countAccumulatedItems(noteHashesFromPrivate); - const nonce = computeNoteHashNonce(txHash, noteHashIndexInTx); - const siloedNoteHash = siloNoteHash(scopedNoteHash.contractAddress, noteHash); - const uniqueNoteHash = computeUniqueNoteHash(nonce, siloedNoteHash); - - avmCircuitPublicInputs.accumulatedData.noteHashes[noteHashIndexInTx] = uniqueNoteHash; - } - } - const msgsFromPrivate = revertCode.isOK() ? mergeAccumulatedData( avmCircuitPublicInputs.previousNonRevertibleAccumulatedData.l2ToL1Msgs,