fix(avm): address bytecode hashing comments

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

@@ -118,22 +118,30 @@ void AvmBytecodeTraceBuilder::build_bytecode_hash_columns()
             bytecode_hash_trace.push_back(BytecodeHashTraceEntry{
                 .field_encoded_bytecode = field_encoded_bytecode[i],
                 .running_hash = running_hash,
-                .bytecode_length_remaining = static_cast<uint16_t>(field_encoded_bytecode.size() - i),
+                .bytecode_field_length_remaining = static_cast<uint16_t>(field_encoded_bytecode.size() - i),
             });
+            // We pair-wise hash the i-th bytecode field with the running hash (which is the output of previous i-1
+            // round). I.e.
+            // initially running_hash = 0,
+            // the first round is running_hash = hash(bytecode[0], running_hash),
+            // the second round is running_hash = hash(bytecode[1],running_hash), and so on.
             running_hash = poseidon2::hash({ field_encoded_bytecode[i], running_hash });
         }
         // Now running_hash actually contains the bytecode hash
         BytecodeHashTraceEntry last_entry;
-        last_entry.bytecode_length_remaining = 0;
+        last_entry.bytecode_field_length_remaining = 0;
         last_entry.running_hash = running_hash;
+        // Assert that the computed bytecode hash is the same as what we received as the hint
         ASSERT(running_hash == contract_bytecode.contract_class_id_preimage.public_bytecode_commitment);
 
         last_entry.class_id = compute_contract_class_id(contract_bytecode.contract_class_id_preimage.artifact_hash,
                                                         contract_bytecode.contract_class_id_preimage.private_fn_root,
                                                         running_hash);
+        // Assert that the computed class id is the same as what we received as the hint
         ASSERT(last_entry.class_id == contract_bytecode.contract_instance.contract_class_id);
 
         last_entry.contract_address = compute_address_from_instance(contract_bytecode.contract_instance);
+        // Assert that the computed contract address is the same as what we received as the hint
         ASSERT(last_entry.contract_address == contract_bytecode.contract_instance.address);
     }
 }
@@ -144,9 +152,9 @@ void AvmBytecodeTraceBuilder::finalize(std::vector<AvmFullRow<FF>>& main_trace)
         auto const& src = bytecode_hash_trace.at(i);
         auto& dest = main_trace.at(i);
         dest.bytecode_running_hash = src.running_hash;
-        dest.bytecode_length_remaining = src.bytecode_length_remaining;
+        dest.bytecode_length_remaining = src.bytecode_field_length_remaining;
         dest.bytecode_as_fields = src.field_encoded_bytecode;
-        dest.bytecode_end_latch = src.bytecode_length_remaining == 0 ? FF::one() : FF::zero();
+        dest.bytecode_end_latch = src.bytecode_field_length_remaining == 0 ? FF::one() : FF::zero();
     }
 
     // We should probably combine this step with the previous one
@@ -157,9 +165,8 @@ void AvmBytecodeTraceBuilder::finalize(std::vector<AvmFullRow<FF>>& main_trace)
         for (auto& byte : bytecode.bytecode) {
             main_trace.at(row_index).bytecode_bytes = byte;
             main_trace.at(row_index).bytecode_bytes_pc = byte_index;
+            row_index++;
         }
-        // The row index will match up with the clk
-        row_index += bytecode.bytecode.size();
     }
 }
 

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

@@ -4,6 +4,7 @@
 
 #include "barretenberg/vm/avm/trace/common.hpp"
 #include "barretenberg/vm/avm/trace/execution_hints.hpp"
+
 namespace bb::avm_trace {
 
 class AvmBytecodeTraceBuilder {
@@ -18,7 +19,7 @@ class AvmBytecodeTraceBuilder {
         // Calculate the bytecode hash
         FF running_hash{};
         // This is the length in fields, not bytes - max 3000 fields
-        uint16_t bytecode_length_remaining = 0;
+        uint16_t bytecode_field_length_remaining = 0;
 
         // Derive the class Id
         FF arifact_hash{};

yarn-project/bb-prover/src/avm_proving.test.ts

@@ -73,12 +73,6 @@ const proveAndVerifyAvmTestContract = async (
   const contractClass = makeContractClassPublic(0, publicFn);
   const contractInstance = makeContractInstanceFromClassId(contractClass.id);
 
-  const nestedCallBytecode = getAvmTestContractBytecode('public_dispatch');
-  const nestedCallFnSelector = getAvmTestContractFunctionSelector('public_dispatch');
-  const nestedCallPublicFn: PublicFunction = { bytecode: nestedCallBytecode, selector: nestedCallFnSelector };
-  const nestedCallContractClass = makeContractClassPublic(0, nestedCallPublicFn);
-  const nestedCallContractInstance = makeContractInstanceFromClassId(nestedCallContractClass.id);
-
   const instanceGet = new SerializableContractInstance({
     version: 1,
     salt: new Fr(0x123),
@@ -96,9 +90,8 @@ const proveAndVerifyAvmTestContract = async (
   worldStateDB.getContractInstance
     .mockResolvedValueOnce(contractInstance)
     .mockResolvedValueOnce(instanceGet)
-    .mockResolvedValueOnce(nestedCallContractInstance)
-    .mockResolvedValueOnce(nestedCallContractInstance);
-  worldStateDB.getContractClass.mockResolvedValueOnce(contractClass).mockResolvedValue(nestedCallContractClass);
+    .mockResolvedValue(contractInstance);
+  worldStateDB.getContractClass.mockResolvedValue(contractClass);
 
   const storageValue = new Fr(5);
   worldStateDB.storageRead.mockResolvedValue(Promise.resolve(storageValue));
@@ -113,10 +106,7 @@ const proveAndVerifyAvmTestContract = async (
   });
   const context = initContext({ env: environment, persistableState });
 
-  worldStateDB.getBytecode
-    .mockResolvedValueOnce(bytecode)
-    .mockResolvedValue(nestedCallBytecode)
-    .mockResolvedValue(nestedCallBytecode);
+  worldStateDB.getBytecode.mockResolvedValue(bytecode);
 
   const startGas = new Gas(context.machineState.gasLeft.daGas, context.machineState.gasLeft.l2Gas);
 

yarn-project/circuits.js/src/structs/avm/avm.ts

@@ -273,7 +273,7 @@ export class AvmContractInstanceHint {
 
 export class AvmContractBytecodeHints {
   /*
-   * @param bytecode currently the bytecode of the nested call function, will be changed to the contract bytecode (via the dispatch function) of the nested call
+   * @param bytecode the contract bytecode
    * @param contractInstance the contract instance of the nested call, used to derive the contract address
    * @param contractClassPreimage the contract class preimage of the nested call, used to derive the class id
    * */
@@ -321,7 +321,7 @@ export class AvmContractBytecodeHints {
    * @returns An array of fields.
    */
   static getFields(fields: FieldsOf<AvmContractBytecodeHints>) {
-    // Buffers aren't serialised the same way as they are read (lenth prefixed), so we need to do this manually.
+    // Buffers aren't serialised the same way as they are read (length prefixed), so we need to do this manually.
     const lengthPrefixedBytecode = Buffer.alloc(fields.bytecode.length + 4);
     // Add a 4-byte length prefix to the bytecode.
     lengthPrefixedBytecode.writeUInt32BE(fields.bytecode.length);

yarn-project/ivc-integration/src/avm_integration.test.ts

@@ -167,12 +167,6 @@ const proveAvmTestContract = async (
   const contractClass = makeContractClassPublic(0, publicFn);
   const contractInstance = makeContractInstanceFromClassId(contractClass.id);
 
-  const nestedCallBytecode = getAvmTestContractBytecode('public_dispatch');
-  const nestedCallFnSelector = getAvmTestContractFunctionSelector('public_dispatch');
-  const nestedCallPublicFn: PublicFunction = { bytecode: nestedCallBytecode, selector: nestedCallFnSelector };
-  const nestedCallContractClass = makeContractClassPublic(0, nestedCallPublicFn);
-  const nestedCallContractInstance = makeContractInstanceFromClassId(nestedCallContractClass.id);
-
   const instanceGet = new SerializableContractInstance({
     version: 1,
     salt: new Fr(0x123),
@@ -190,9 +184,9 @@ const proveAvmTestContract = async (
   worldStateDB.getContractInstance
     .mockResolvedValueOnce(contractInstance)
     .mockResolvedValueOnce(instanceGet)
-    .mockResolvedValueOnce(nestedCallContractInstance)
-    .mockResolvedValueOnce(nestedCallContractInstance);
-  worldStateDB.getContractClass.mockResolvedValueOnce(contractClass).mockResolvedValue(nestedCallContractClass);
+    .mockResolvedValue(contractInstance);
+
+  worldStateDB.getContractClass.mockResolvedValue(contractClass);
 
   const storageValue = new Fr(5);
   worldStateDB.storageRead.mockResolvedValue(storageValue);
@@ -207,10 +201,7 @@ const proveAvmTestContract = async (
   });
   const context = initContext({ env: environment, persistableState });
 
-  worldStateDB.getBytecode
-    .mockResolvedValueOnce(bytecode)
-    .mockResolvedValue(nestedCallBytecode)
-    .mockResolvedValue(nestedCallBytecode);
+  worldStateDB.getBytecode.mockResolvedValue(bytecode);
 
   const startGas = new Gas(context.machineState.gasLeft.daGas, context.machineState.gasLeft.l2Gas);
 

yarn-project/simulator/src/avm/avm_simulator.ts

@@ -33,10 +33,7 @@ export class AvmSimulator {
    * Fetch the bytecode and execute it in the current context.
    */
   public async execute(): Promise<AvmContractCallResult> {
-    const bytecode = await this.context.persistableState.getBytecode(
-      this.context.environment.address,
-      this.context.environment.functionSelector,
-    );
+    const bytecode = await this.context.persistableState.getBytecode(this.context.environment.address);
 
     // This assumes that we will not be able to send messages to accounts without code
     // Pending classes and instances impl details

yarn-project/simulator/src/avm/journal/journal.ts

@@ -1,6 +1,5 @@
 import {
   AztecAddress,
-  type FunctionSelector,
   type Gas,
   SerializableContractInstance,
   computePublicBytecodeCommitment,
@@ -247,7 +246,7 @@ export class AvmPersistableStateManager {
   /**
    * Get a contract's bytecode from the contracts DB, also trace the contract class and instance
    */
-  public async getBytecode(contractAddress: AztecAddress, _selector: FunctionSelector): Promise<Buffer | undefined> {
+  public async getBytecode(contractAddress: AztecAddress): Promise<Buffer | undefined> {
     let exists = true;
     let contractInstance = await this.worldStateDB.getContractInstance(contractAddress);
     // If the contract instance is not found, we assume it has not been deployed.