feat(avm): don't hash bytecode in fast simulation

barretenberg/cpp/src/barretenberg/vm2/simulation/standalone/pure_bytecode_manager.cpp

@@ -69,16 +69,12 @@ BytecodeId PureTxBytecodeManager::get_bytecode(const AztecAddress& address)
     auto& klass = maybe_klass.value();
     debug("Bytecode for ", address, " successfully retrieved!");
 
-    // TODO(dbanks12): in TS, the PublicContractsDB will hash the bytecode if it has never been hashed there before.
-    // After that, it caches it. It should only happen once per contract class, but when we are making a callback
-    // to the TS cache to hash the bytecode there, it might be unnecessarily slow, in which case we could do the same
-    // hashing and caching here in C++ and avoid callbacks to TS.
-    std::optional<FF> maybe_bytecode_commitment = contract_db.get_bytecode_commitment(current_class_id);
-    // If we reach this point, class ID and instance both exist which means bytecode commitment must exist.
-    assert(maybe_bytecode_commitment.has_value());
-    BytecodeId bytecode_id = maybe_bytecode_commitment.value();
-
-    // Check if we've already processed this bytecode.
+    // For fast simulation, we use the class_id as the bytecode_id instead of computing the
+    // expensive bytecode commitment hash. This is safe because class_id uniquely identifies
+    // the bytecode. The actual commitment is only needed for trace generation / witgen.
+    BytecodeId bytecode_id = current_class_id;
+
+    // Check if we've already processed this bytecode (different class can have same bytecode).
     if (bytecodes.contains(bytecode_id)) {
         return bytecode_id;
     }

barretenberg/cpp/src/barretenberg/vm2/simulation_helper.cpp

@@ -12,6 +12,7 @@
 #include "barretenberg/vm2/simulation/gadgets/emit_unencrypted_log.hpp"
 #include "barretenberg/vm2/simulation/interfaces/db.hpp"
 #include "barretenberg/vm2/simulation/interfaces/debug_log.hpp"
+#include "barretenberg/vm2/simulation/interfaces/update_check.hpp"
 #include "barretenberg/vm2/simulation/lib/call_stack_metadata_collector.hpp"
 #include "barretenberg/vm2/simulation/lib/db_types.hpp"
 #include "barretenberg/vm2/simulation/lib/execution_id_manager.hpp"
@@ -386,24 +387,57 @@ TxSimulationResult AvmSimulationHelper::simulate_fast(ContractDBInterface& raw_c
 
         tx.non_revertible_accumulated_data.nullifiers.at(0), base_merkle_db, side_effect_tracker);
 
-    // NoopUpdateCheck update_check;
-    // TODO(#18161): Note that if we need to gather hints here, we can't use the NoopUpdateCheck as it will skip
-    // collecting a required hint for a storage_read. Optionally use Noop if we don't need hints:
-
-    UpdateCheck update_check(poseidon2, range_check, greater_than, merkle_db, update_check_emitter, global_variables);
+    std::unique_ptr<UpdateCheckInterface> update_check = [&]() -> std::unique_ptr<UpdateCheckInterface> {
+        if (config.collect_hints) {
+            // When collecting hints, we need to use UpdateCheck to collect required hints for storage_read.
+            return std::make_unique<UpdateCheck>(
+                poseidon2, range_check, greater_than, merkle_db, update_check_emitter, global_variables);
+        } else {
+            return std::make_unique<NoopUpdateCheck>();
+        }
+    }();
 
     InstructionInfoDB instruction_info_db;
 
     ContractInstanceManager contract_instance_manager(
-        contract_db, merkle_db, update_check, field_gt, protocol_contracts, contract_instance_retrieval_emitter);
+        contract_db, merkle_db, *update_check, field_gt, protocol_contracts, contract_instance_retrieval_emitter);
+
+    // These are needed for the TxBytecodeManager, but not for the PureTxBytecodeManager.
+    std::unique_ptr<NoopEventEmitter<BytecodeHashingEvent>> bytecode_hashing_emitter;
+    std::unique_ptr<BytecodeHasher> bytecode_hasher;
+    std::unique_ptr<NoopEventEmitter<BytecodeRetrievalEvent>> bytecode_retrieval_emitter;
+    std::unique_ptr<NoopEventEmitter<BytecodeDecompositionEvent>> bytecode_decomposition_emitter;
+    std::unique_ptr<NoopEventEmitter<InstructionFetchingEvent>> instruction_fetching_emitter;
+    std::unique_ptr<TxBytecodeManagerInterface> tx_bytecode_manager =
+        [&]() -> std::unique_ptr<TxBytecodeManagerInterface> {
+        if (config.collect_hints) {
+            // When collecting hints, we need to call the contract DB to get the bytecode commitment.
+            // The pure bytecode manager doesn't do this, so we use the gadget version.
+            bytecode_hashing_emitter = std::make_unique<NoopEventEmitter<BytecodeHashingEvent>>();
+            bytecode_hasher = std::make_unique<BytecodeHasher>(poseidon2, *bytecode_hashing_emitter);
+            bytecode_retrieval_emitter = std::make_unique<NoopEventEmitter<BytecodeRetrievalEvent>>();
+            bytecode_decomposition_emitter = std::make_unique<NoopEventEmitter<BytecodeDecompositionEvent>>();
+            instruction_fetching_emitter = std::make_unique<NoopEventEmitter<InstructionFetchingEvent>>();
+            return std::make_unique<TxBytecodeManager>(contract_db,
+                                                       merkle_db,
+                                                       *bytecode_hasher,
+                                                       range_check,
+                                                       contract_instance_manager,
+                                                       retrieved_bytecodes_tree_check,
+                                                       *bytecode_retrieval_emitter,
+                                                       *bytecode_decomposition_emitter,
+                                                       *instruction_fetching_emitter);
+        } else {
+            return std::make_unique<PureTxBytecodeManager>(contract_db, contract_instance_manager);
+        }
+    }();
 
-    PureTxBytecodeManager bytecode_manager(contract_db, contract_instance_manager);
     PureExecutionComponentsProvider execution_components(greater_than, instruction_info_db);
 
     PureMemoryProvider memory_provider;
     CalldataHashingProvider calldata_hashing_provider(poseidon2, calldata_emitter);
     InternalCallStackManagerProvider internal_call_stack_manager_provider(internal_call_stack_emitter);
-    ContextProvider context_provider(bytecode_manager,
+    ContextProvider context_provider(*tx_bytecode_manager,
                                      memory_provider,
                                      calldata_hashing_provider,
                                      internal_call_stack_manager_provider,

yarn-project/bb-prover/src/avm_proving_tests/avm_proving_tester.ts

@@ -94,7 +94,7 @@ const provingConfig: PublicSimulatorConfig = PublicSimulatorConfig.from({
   collectCallMetadata: true,
   collectDebugLogs: false,
   collectHints: true, // Required for proving!
-  collectPublicInputs: true,
+  collectPublicInputs: true, // Required for proving!
   collectStatistics: false,
 });
 

yarn-project/simulator/src/public/public_tx_simulator/cpp_vs_ts_public_tx_simulator.ts

@@ -1,5 +1,4 @@
 import { type Logger, createLogger, logLevel } from '@aztec/foundation/log';
-import { writeTestData } from '@aztec/foundation/testing/files';
 import { avmSimulate } from '@aztec/native';
 import { ProtocolContractsList } from '@aztec/protocol-contracts';
 import {
@@ -126,12 +125,6 @@ export class CppVsTsPublicTxSimulator extends PublicTxSimulator implements Publi
     // Deserialize the msgpack result
     this.log.debug(`Deserializing C++ from buffer (size: ${resultBuffer.length})...`);
     const cppResultJSON: object = deserializeFromMessagePack(resultBuffer);
-    // Write testdata if AZTEC_WRITE_TESTDATA=1.
-    writeTestData(
-      `barretenberg/cpp/src/barretenberg/vm2/testing/tx_result_${txHash}.testdata.bin`,
-      resultBuffer,
-      /*raw=*/ true,
-    );
     this.log.debug(`Deserializing C++ result to PublicTxResult...`);
     const cppResult = PublicTxResult.fromPlainObject(cppResultJSON);
     this.log.debug(`Done.`);