Skip to content

Commit

Permalink
fix(avm): address bytecode hashing comments
Browse files Browse the repository at this point in the history
  • Loading branch information
IlyasRidhuan committed Oct 25, 2024
1 parent 5ff45f6 commit 2608730
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 41 deletions.
19 changes: 13 additions & 6 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/bytecode_trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -106,22 +106,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);
}
}
Expand All @@ -132,9 +140,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
Expand All @@ -145,9 +153,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();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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{};
Expand Down
16 changes: 3 additions & 13 deletions yarn-project/bb-prover/src/avm_proving.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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));
Expand All @@ -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);

Expand Down
4 changes: 2 additions & 2 deletions yarn-project/circuits.js/src/structs/avm/avm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
* */
Expand Down Expand Up @@ -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);
Expand Down
17 changes: 4 additions & 13 deletions yarn-project/ivc-integration/src/avm_integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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);
Expand All @@ -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);

Expand Down
5 changes: 1 addition & 4 deletions yarn-project/simulator/src/avm/avm_simulator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions yarn-project/simulator/src/avm/journal/journal.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import {
AztecAddress,
type FunctionSelector,
type Gas,
SerializableContractInstance,
computePublicBytecodeCommitment,
Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 2608730

Please sign in to comment.