feat: Note hash management in the AVM (#10666)

- Removes old siloing and nonce application from transitional adapters
- Handling of note hashes from private: nonrevertibles are inserted in
setup and revertibles at the start of app logic both in simulator and
witgen
- Makes unique emitted note hashes from public inside the AVM itself,
both in simulator and witgen

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<Row> 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<FF> 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<FF>(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<FF>(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<FF> 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<FF>(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<FF>(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).");

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 });

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

barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp

@@ -213,8 +213,14 @@ std::vector<uint8_t> 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<FF>& siloed_nullifiers,
-                                           [[maybe_unused]] const std::vector<FF>& siloed_note_hashes)
+                                           const std::vector<FF>& 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<FF>& 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<FF>& siloed_nullifiers,
+                                                      const std::vector<FF>& siloed_note_hashes)
+{
+    // Revertibles come only siloed from private, so we need to make them unique here
+    std::vector<FF> 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<uint32_t>(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<uint32_t>(!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);

barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp

@@ -234,7 +234,9 @@ class AvmTraceBuilder {
     void rollback_to_non_revertible_checkpoint();
     std::vector<uint8_t> get_bytecode(const FF contract_address, bool check_membership = false);
     std::unordered_set<FF> bytecode_membership_cache;
-    void insert_private_state(const std::vector<FF>& siloed_nullifiers, const std::vector<FF>& siloed_note_hashes);
+    void insert_private_state(const std::vector<FF>& siloed_nullifiers, const std::vector<FF>& unique_note_hashes);
+    void insert_private_revertible_state(const std::vector<FF>& siloed_nullifiers,
+                                         const std::vector<FF>& 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);

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);
 }
 
 /**

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<PublicSideEffectTraceInterface>();
     const nestedTrace = mock<PublicSideEffectTraceInterface>();
+    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 () => {

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<WorldStateDB>();
   return new AvmPersistableStateManager(
@@ -54,6 +55,7 @@ export function initPersistableStateManager(overrides?: {
     overrides?.nullifiers || new NullifierManager(worldStateDB),
     overrides?.doMerkleOperations || false,
     overrides?.merkleTrees || mock<AvmEphemeralForest>(),
+    overrides?.txHash || new TxHash(new Fr(27).toBuffer()),
   );
 }
 

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 () => {