Skip to content

Commit 769d979

Browse files
IlyasRidhuanIlyasRidhuan
committed
refactor bytecode trace builder
1 parent 29cd4cc commit 769d979

File tree

10 files changed

+71
-56
lines changed

10 files changed

+71
-56
lines changed

‎barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,12 @@ class AvmExecutionTests : public ::testing::Test {
3636
ExecutionHints execution_hints,
3737
uint32_t side_effect_counter,
3838
std::vector<FF> calldata,
39-
std::vector<uint8_t> contract_bytecode) {
39+
const std::vector<std::vector<uint8_t>>& all_contracts_bytecode) {
4040
return AvmTraceBuilder(std::move(public_inputs),
4141
std::move(execution_hints),
4242
side_effect_counter,
4343
std::move(calldata),
44-
contract_bytecode)
44+
all_contracts_bytecode)
4545
.set_full_precomputed_tables(false)
4646
.set_range_check_required(false);
4747
});

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

+24-23
Original file line numberDiff line numberDiff line change
@@ -4,41 +4,42 @@
44

55
namespace bb::avm_trace {
66
using poseidon2 = crypto::Poseidon2<crypto::Poseidon2Bn254ScalarFieldParams>;
7-
AvmBytecodeTraceBuilder::AvmBytecodeTraceBuilder(const std::vector<uint8_t>& contract_bytecode,
8-
const ExecutionHints& hints)
7+
AvmBytecodeTraceBuilder::AvmBytecodeTraceBuilder(const std::vector<std::vector<uint8_t>>& all_contracts_bytecode)
8+
: all_contracts_bytecode(all_contracts_bytecode)
9+
{}
10+
11+
std::vector<FF> AvmBytecodeTraceBuilder::pack_bytecode(const std::vector<uint8_t>& contract_bytes)
912
{
10-
// Do we need to pad contract_bytecode to be 31bytes?
11-
std::vector<std::vector<uint8_t>> all_contracts_bytecode{ contract_bytecode };
12-
for (const auto& hint : hints.externalcall_hints) {
13-
all_contracts_bytecode.push_back(hint.bytecode);
14-
}
15-
for (auto& contract : all_contracts_bytecode) {
16-
// To make from_buffer<uint256_t> work properly, we need to make sure the contract is a multiple of 31 bytes
17-
// Otherwise we will end up over-reading the buffer
18-
size_t padding = 31 - (contract.size() % 31);
19-
contract.resize(contract.size() + padding, 0);
20-
std::vector<FF> contract_bytecode_fields;
21-
for (size_t i = 0; i < contract.size(); i += 31) {
22-
uint256_t u256_elem = from_buffer<uint256_t>(contract, i);
23-
// Drop the last byte
24-
contract_bytecode_fields.emplace_back(u256_elem >> 8);
25-
}
26-
this->all_contracts_bytecode.push_back(contract_bytecode_fields);
13+
// To make from_buffer<uint256_t> work properly, we need to make sure the contract is a multiple of 31 bytes
14+
// Otherwise we will end up over-reading the buffer
15+
size_t padding = 31 - (contract_bytes.size() % 31);
16+
// We dont want to mutate the original contract bytes, since we will (probably) need them later in the trace
17+
// unpadded
18+
std::vector<uint8_t> contract_bytes_padded = contract_bytes;
19+
contract_bytes_padded.resize(contract_bytes_padded.size() + padding, 0);
20+
std::vector<FF> contract_bytecode_fields;
21+
for (size_t i = 0; i < contract_bytes_padded.size(); i += 31) {
22+
uint256_t u256_elem = from_buffer<uint256_t>(contract_bytes_padded, i);
23+
// Drop the last byte
24+
contract_bytecode_fields.emplace_back(u256_elem >> 8);
2725
}
26+
return contract_bytecode_fields;
2827
}
2928

3029
void AvmBytecodeTraceBuilder::build_bytecode_columns()
3130
{
31+
// This is the main loop that will generate the bytecode trace
3232
for (auto& contract_bytecode : all_contracts_bytecode) {
3333
FF running_hash = FF::zero();
34+
auto packed_bytecode = pack_bytecode(contract_bytecode);
3435
// This size is already based on the number of fields
35-
for (size_t i = 0; i < contract_bytecode.size(); ++i) {
36+
for (size_t i = 0; i < packed_bytecode.size(); ++i) {
3637
bytecode_trace.push_back(BytecodeTraceEntry{
37-
.packed_bytecode = contract_bytecode[i],
38+
.packed_bytecode = packed_bytecode[i],
3839
.running_hash = running_hash,
39-
.bytecode_length_remaining = static_cast<uint16_t>(contract_bytecode.size() - i),
40+
.bytecode_length_remaining = static_cast<uint16_t>(packed_bytecode.size() - i),
4041
});
41-
running_hash = poseidon2::hash({ contract_bytecode[i], running_hash });
42+
running_hash = poseidon2::hash({ packed_bytecode[i], running_hash });
4243
}
4344
// Now running_hash actually contains the bytecode hash
4445
BytecodeTraceEntry last_entry;

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

+7-3
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ class AvmBytecodeTraceBuilder {
2222
// Derive the contract address
2323
FF contract_address{};
2424
};
25+
AvmBytecodeTraceBuilder() = default;
2526
// These interfaces will change when we start feeding in more inputs and hints
26-
AvmBytecodeTraceBuilder(const std::vector<uint8_t>& contract_bytecode, const ExecutionHints& hints);
27+
AvmBytecodeTraceBuilder(const std::vector<std::vector<uint8_t>>& all_contracts_bytecode);
2728

2829
size_t size() const { return bytecode_trace.size(); }
2930
void reset();
@@ -32,9 +33,12 @@ class AvmBytecodeTraceBuilder {
3233
void build_bytecode_columns();
3334

3435
private:
36+
// Converts the bytecode into field elements (chunks of 31 bytes)
37+
static std::vector<FF> pack_bytecode(const std::vector<uint8_t>& contract_bytes);
38+
3539
std::vector<BytecodeTraceEntry> bytecode_trace;
36-
// This will contain the bytecode as field elements
37-
std::vector<std::vector<FF>> all_contracts_bytecode;
40+
// The first element is the main top-level contract, the rest are external calls
41+
std::vector<std::vector<uint8_t>> all_contracts_bytecode;
3842
// TODO: Come back to this
3943
// VmPublicInputs public_inputs;
4044
// ExecutionHints hints;

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

+22-15
Original file line numberDiff line numberDiff line change
@@ -146,17 +146,18 @@ void show_trace_info(const auto& trace)
146146
} // namespace
147147

148148
// Needed for dependency injection in tests.
149-
Execution::TraceBuilderConstructor Execution::trace_builder_constructor = [](VmPublicInputs public_inputs,
150-
ExecutionHints execution_hints,
151-
uint32_t side_effect_counter,
152-
std::vector<FF> calldata,
153-
std::vector<uint8_t> contract_bytecode) {
154-
return AvmTraceBuilder(std::move(public_inputs),
155-
std::move(execution_hints),
156-
side_effect_counter,
157-
std::move(calldata),
158-
contract_bytecode);
159-
};
149+
Execution::TraceBuilderConstructor Execution::trace_builder_constructor =
150+
[](VmPublicInputs public_inputs,
151+
ExecutionHints execution_hints,
152+
uint32_t side_effect_counter,
153+
std::vector<FF> calldata,
154+
std::vector<std::vector<uint8_t>> all_contract_bytecode) {
155+
return AvmTraceBuilder(std::move(public_inputs),
156+
std::move(execution_hints),
157+
side_effect_counter,
158+
std::move(calldata),
159+
all_contract_bytecode);
160+
};
160161

161162
/**
162163
* @brief Temporary routine to generate default public inputs (gas values) until we get
@@ -253,7 +254,7 @@ bool Execution::verify(AvmFlavor::VerificationKey vk, HonkProof const& proof)
253254
std::copy(returndata_offset, raw_proof_offset, std::back_inserter(returndata));
254255
std::copy(raw_proof_offset, proof.end(), std::back_inserter(raw_proof));
255256

256-
VmPublicInputs public_inputs = convert_public_inputs(public_inputs_vec);
257+
VmPublicInputs public_inputs = avm_trace::convert_public_inputs(public_inputs_vec);
257258
std::vector<std::vector<FF>> public_inputs_columns =
258259
copy_public_inputs_columns(public_inputs, calldata, returndata);
259260
return verifier.verify_proof(raw_proof, public_inputs_columns);
@@ -279,13 +280,19 @@ std::vector<Row> Execution::gen_trace(std::vector<uint8_t> const& bytecode,
279280
vinfo("------- GENERATING TRACE -------");
280281
// TODO(https://github.com/AztecProtocol/aztec-packages/issues/6718): construction of the public input columns
281282
// should be done in the kernel - this is stubbed and underconstrained
282-
VmPublicInputs public_inputs = convert_public_inputs(public_inputs_vec);
283+
VmPublicInputs public_inputs = avm_trace::convert_public_inputs(public_inputs_vec);
283284
uint32_t start_side_effect_counter =
284285
!public_inputs_vec.empty() ? static_cast<uint32_t>(public_inputs_vec[PCPI_START_SIDE_EFFECT_COUNTER_OFFSET])
285286
: 0;
286-
287+
std::vector<std::vector<uint8_t>> all_contract_bytecode;
288+
all_contract_bytecode.reserve(execution_hints.externalcall_hints.size() + 1);
289+
// Start with the main, top-level contract bytecode
290+
all_contract_bytecode.push_back(bytecode);
291+
for (const auto& externalcall_hint : execution_hints.externalcall_hints) {
292+
all_contract_bytecode.emplace_back(externalcall_hint.bytecode);
293+
}
287294
AvmTraceBuilder trace_builder = Execution::trace_builder_constructor(
288-
public_inputs, execution_hints, start_side_effect_counter, calldata, bytecode);
295+
public_inputs, execution_hints, start_side_effect_counter, calldata, all_contract_bytecode);
289296

290297
// Copied version of pc maintained in trace builder. The value of pc is evolving based
291298
// on opcode logic and therefore is not maintained here. However, the next opcode in the execution

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

+6-5
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@ namespace bb::avm_trace {
1515
class Execution {
1616
public:
1717
static constexpr size_t SRS_SIZE = 1 << 22;
18-
using TraceBuilderConstructor = std::function<AvmTraceBuilder(VmPublicInputs public_inputs,
19-
ExecutionHints execution_hints,
20-
uint32_t side_effect_counter,
21-
std::vector<FF> calldata,
22-
std::vector<uint8_t> contract_bytecode)>;
18+
using TraceBuilderConstructor =
19+
std::function<AvmTraceBuilder(VmPublicInputs public_inputs,
20+
ExecutionHints execution_hints,
21+
uint32_t side_effect_counter,
22+
std::vector<FF> calldata,
23+
const std::vector<std::vector<uint8_t>>& all_contract_bytecode)>;
2324

2425
Execution() = default;
2526

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

+3-3
Original file line numberDiff line numberDiff line change
@@ -278,16 +278,16 @@ void AvmTraceBuilder::finalise_mem_trace_lookup_counts()
278278
* underlying traces and initialize gas values.
279279
*/
280280
AvmTraceBuilder::AvmTraceBuilder(VmPublicInputs public_inputs,
281-
const ExecutionHints& execution_hints_,
281+
ExecutionHints execution_hints_,
282282
uint32_t side_effect_counter,
283283
std::vector<FF> calldata,
284-
const std::vector<uint8_t>& contract_bytecode)
284+
const std::vector<std::vector<uint8_t>>& all_contract_bytecode)
285285
// NOTE: we initialise the environment builder here as it requires public inputs
286286
: calldata(std::move(calldata))
287287
, side_effect_counter(side_effect_counter)
288288
, execution_hints(std::move(execution_hints_))
289289
, kernel_trace_builder(side_effect_counter, public_inputs, execution_hints)
290-
, bytecode_trace_builder(contract_bytecode, execution_hints)
290+
, bytecode_trace_builder(all_contract_bytecode)
291291
{
292292
// TODO: think about cast
293293
gas_trace_builder.set_initial_gas(

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ class AvmTraceBuilder {
5454

5555
public:
5656
AvmTraceBuilder(VmPublicInputs public_inputs = {},
57-
const ExecutionHints& execution_hints = {},
57+
ExecutionHints execution_hints = {},
5858
uint32_t side_effect_counter = 0,
5959
std::vector<FF> calldata = {},
60-
const std::vector<uint8_t>& contract_bytecode = {});
60+
const std::vector<std::vector<uint8_t>>& all_contract_bytecode = {});
6161

6262
uint32_t getPc() const { return pc; }
6363

‎noir-projects/noir-contracts/contracts/contract_class_registerer_contract/src/main.nr

+3
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,9 @@ contract ContractClassRegisterer {
5454
if (i < bytecode_length_in_fields) {
5555
// We skip the first element when hashing since it is the length
5656
computed_public_bytecode_commitment = std::hash::poseidon2::Poseidon2::hash([packed_public_bytecode[i + 1],computed_public_bytecode_commitment],2);
57+
} else {
58+
// Any bytes after the bytecode length must be 0
59+
assert_eq(packed_public_bytecode[i + 1], 0);
5760
}
5861
}
5962
assert_eq(computed_public_bytecode_commitment, public_bytecode_commitment);

‎yarn-project/circuits.js/src/contract/contract_class_id.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('ContractClass', () => {
2525
};
2626

2727
expect(computeContractClassId(contractClass).toString()).toMatchInlineSnapshot(
28-
`"0x174bf0daff21f2b8b88f7d2b7ef7ed6a7b083c979a2996a4e78963ad4b84ff8d"`,
28+
`"0x2d5c712c483891d42e5bca539e8516fc52b5b024568ac71e4fe47c0c0157f851"`,
2929
);
3030
});
3131
});

‎yarn-project/circuits.js/src/contract/events/contract_class_registered_event.test.ts

+1-2
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,8 @@ describe('ContractClassRegisteredEvent', () => {
1111
);
1212
expect(event.artifactHash.toString()).toEqual('0x072dce903b1a299d6820eeed695480fe9ec46658b1101885816aed6dd86037f0');
1313
expect(event.packedPublicBytecode.length).toEqual(27090);
14-
// TODO: #5860
1514
expect(computePublicBytecodeCommitment(event.packedPublicBytecode).toString()).toEqual(
16-
'0x0000000000000000000000000000000000000000000000000000000000000005',
15+
'0x0378491b34825cd67d1e13e140bbc80f2cd3a9b52171ea577f8f11620d4198ba',
1716
);
1817
});
1918
});

0 commit comments

Comments
 (0)