Skip to content

Commit

Permalink
chore(avm): Gas alignments with simulator (#6873)
Browse files Browse the repository at this point in the history
Resolves #6860
  • Loading branch information
jeanmon authored Jun 4, 2024
1 parent 504fea2 commit 54339d4
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 16 deletions.
3 changes: 2 additions & 1 deletion barretenberg/cpp/pil/avm/avm_main.pil
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,7 @@ namespace avm_main(256);

//===== CONTROL_FLOW_CONSISTENCY ============================================
pol INTERNAL_CALL_STACK_SELECTORS = (first + sel_internal_call + sel_internal_return + sel_halt);
pol ALL_CTRL_FLOW_SEL = sel_jump + sel_jumpi + sel_internal_call + sel_internal_return + sel_external_call;
pol ALL_CTRL_FLOW_SEL = sel_jump + sel_jumpi + sel_internal_call + sel_internal_return;

pol ALL_BINARY_SEL = sel_op_and + sel_op_or + sel_op_xor;
pol ALL_GADGET_SEL = sel_op_radix_le + sel_op_sha256 + sel_op_poseidon2 + sel_op_keccak + sel_op_pedersen;
Expand All @@ -439,6 +439,7 @@ namespace avm_main(256);
// setting a newly introduced boolean mem_op_activate_gas which is set in witness generation.
// We should remove this shortcut and constrain this activation through bytecode decomposition.
// Alternatively, we introduce a boolean selector for the three opcodes mentioned above.
// Note: External call gas cost is not constrained
pol commit gas_cost_active;
pol commit mem_op_activate_gas; // TODO: remove this one
gas_cost_active - OPCODE_SELECTORS - ALL_CTRL_FLOW_SEL - mem_op_activate_gas = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1060,9 +1060,8 @@ template <typename FF_> class avm_mainImpl {
avm_main_sel_op_emit_l2_to_l1_msg) +
avm_main_sel_op_sload) +
avm_main_sel_op_sstore))) -
((((avm_main_sel_jump + avm_main_sel_jumpi) + avm_main_sel_internal_call) +
avm_main_sel_internal_return) +
avm_main_sel_external_call)) -
(((avm_main_sel_jump + avm_main_sel_jumpi) + avm_main_sel_internal_call) +
avm_main_sel_internal_return)) -
avm_main_mem_op_activate_gas);
tmp *= scaling_factor;
std::get<89>(evals) += tmp;
Expand Down
26 changes: 26 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_gas_trace.cpp
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include "barretenberg/vm/avm_trace/avm_gas_trace.hpp"
#include "barretenberg/vm/avm_trace/avm_opcode.hpp"

namespace bb::avm_trace {

Expand Down Expand Up @@ -50,4 +51,29 @@ void AvmGasTraceBuilder::constrain_gas_lookup(uint32_t clk, OpCode opcode)
gas_trace.push_back(entry);
}

void AvmGasTraceBuilder::constrain_gas_for_external_call(uint32_t clk)
{
// Arbitrary constants for the moment
uint32_t l2_gas_cost = 6; // This will be hinted
uint32_t da_gas_cost = 23; // This will be hinted

remaining_l2_gas -= l2_gas_cost;
remaining_da_gas -= da_gas_cost;

// Decrease the gas left
// Create a gas trace entry
GasTraceEntry entry = {
.clk = clk,
.opcode = OpCode::CALL,
.l2_gas_cost = 0, // We need 0 in this case because we do not activate the gas_cost_active selector to satisfy
// #[L2_GAS_INACTIVE].
.da_gas_cost = 0, // We need 0 in this case because we do not activate the gas_cost_active selector to satisfy
// #[DA_GAS_INACTIVE].
.remaining_l2_gas = remaining_l2_gas,
.remaining_da_gas = remaining_da_gas,
};

gas_trace.push_back(entry);
}

} // namespace bb::avm_trace
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "barretenberg/vm/avm_trace/avm_common.hpp"
#include "barretenberg/vm/avm_trace/avm_opcode.hpp"
#include <cstdint>

namespace bb::avm_trace {

Expand Down Expand Up @@ -129,6 +130,7 @@ class AvmGasTraceBuilder {
std::vector<GasTraceEntry> finalize();

void constrain_gas_lookup(uint32_t clk, OpCode opcode);
void constrain_gas_for_external_call(uint32_t clk);
void set_initial_gas(uint32_t l2_gas, uint32_t da_gas);

std::vector<GasTraceEntry> gas_trace;
Expand Down
37 changes: 27 additions & 10 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@ void AvmTraceBuilder::reset()
alu_trace_builder.reset();
bin_trace_builder.reset();
kernel_trace_builder.reset();
gas_trace_builder.reset();
conversion_trace_builder.reset();
sha256_trace_builder.reset();
poseidon2_trace_builder.reset();
keccak_trace_builder.reset();
pedersen_trace_builder.reset();

return_data_counter = 0;
}

Expand Down Expand Up @@ -1822,8 +1829,10 @@ void AvmTraceBuilder::calldata_copy(
call_ptr, clk, IntermRegister::IC, mem_idx_c, ic, AvmMemoryTag::U0, AvmMemoryTag::FF);
}

// Constrain gas cost
gas_trace_builder.constrain_gas_lookup(clk, OpCode::CALLDATACOPY);
// Constrain gas cost on the first row
if (pos == 0) {
gas_trace_builder.constrain_gas_lookup(clk, OpCode::CALLDATACOPY);
}

main_trace.push_back(Row{
.avm_main_clk = clk,
Expand All @@ -1838,7 +1847,8 @@ void AvmTraceBuilder::calldata_copy(
.avm_main_mem_idx_b = FF(mem_idx_b),
.avm_main_mem_idx_c = FF(mem_idx_c),
.avm_main_mem_op_a = FF(mem_op_a),
.avm_main_mem_op_activate_gas = FF(1), // TODO: remove in the long term
.avm_main_mem_op_activate_gas = FF(static_cast<uint32_t>(
pos == 0)), // TODO: remove in the long term. This activate gas only for the first row.
.avm_main_mem_op_b = FF(mem_op_b),
.avm_main_mem_op_c = FF(mem_op_c),
.avm_main_pc = FF(pc++),
Expand Down Expand Up @@ -1946,8 +1956,10 @@ std::vector<FF> AvmTraceBuilder::return_op(uint8_t indirect, uint32_t ret_offset
returnMem.push_back(ic);
}

// Constrain gas cost
gas_trace_builder.constrain_gas_lookup(clk, OpCode::RETURN);
// Constrain gas cost on the first row
if (pos == 0) {
gas_trace_builder.constrain_gas_lookup(clk, OpCode::RETURN);
}

main_trace.push_back(Row{
.avm_main_clk = clk,
Expand All @@ -1962,7 +1974,8 @@ std::vector<FF> AvmTraceBuilder::return_op(uint8_t indirect, uint32_t ret_offset
.avm_main_mem_idx_b = FF(mem_idx_b),
.avm_main_mem_idx_c = FF(mem_idx_c),
.avm_main_mem_op_a = FF(mem_op_a),
.avm_main_mem_op_activate_gas = FF(1), // TODO: remove in the long term
.avm_main_mem_op_activate_gas = FF(static_cast<uint32_t>(
pos == 0)), // TODO: remove in the long term. This activate gas only for the first row.
.avm_main_mem_op_b = FF(mem_op_b),
.avm_main_mem_op_c = FF(mem_op_c),
.avm_main_pc = FF(pc),
Expand Down Expand Up @@ -2339,7 +2352,7 @@ void AvmTraceBuilder::op_call([[maybe_unused]] uint8_t indirect,
// We can load up to 4 things per row
auto register_order = std::array{ IntermRegister::IA, IntermRegister::IB, IntermRegister::IC, IntermRegister::ID };
// Constrain gas cost
gas_trace_builder.constrain_gas_lookup(clk, OpCode::CALL);
gas_trace_builder.constrain_gas_for_external_call(clk);
// Indirect is ZEROTH, SECOND and FOURTH bit COME BACK TO MAKING THIS ALL SUPPORTED
auto read_ind_gas_offset =
mem_trace_builder.indirect_read_and_load_from_memory(call_ptr, clk, IndirectRegister::IND_A, gas_offset);
Expand Down Expand Up @@ -3921,15 +3934,19 @@ std::vector<Row> AvmTraceBuilder::finalize(uint32_t min_trace_size, bool range_c

auto& dest = main_trace.at(gas_entry.clk);
auto& next = main_trace.at(gas_entry.clk + 1);
dest.avm_main_gas_cost_active = FF(1);

// TODO: gas is not constrained for external call at this time
if (gas_entry.opcode != OpCode::CALL) {
dest.avm_main_gas_cost_active = FF(1);
}

// Write each of the relevant gas accounting values
dest.avm_main_opcode_val = static_cast<uint8_t>(gas_entry.opcode);
dest.avm_main_l2_gas_op = gas_entry.l2_gas_cost;
dest.avm_main_da_gas_op = gas_entry.da_gas_cost;

current_l2_gas_remaining -= gas_entry.l2_gas_cost;
current_da_gas_remaining -= gas_entry.da_gas_cost;
current_l2_gas_remaining = gas_entry.remaining_l2_gas;
current_da_gas_remaining = gas_entry.remaining_da_gas;
next.avm_main_l2_gas_remaining = current_l2_gas_remaining;
next.avm_main_da_gas_remaining = current_da_gas_remaining;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "avm_common.test.hpp"
#include "barretenberg/numeric/uint128/uint128.hpp"
#include "barretenberg/vm/avm_trace/avm_common.hpp"
#include "barretenberg/vm/avm_trace/avm_helper.hpp"
#include "barretenberg/vm/avm_trace/avm_trace.hpp"
#include "barretenberg/vm/tests/helpers.test.hpp"
#include <array>
Expand Down Expand Up @@ -1893,7 +1894,7 @@ TEST_F(AvmArithmeticNegativeTestsFF, operationWithErrorFlag)

EXPECT_THROW_WITH_MESSAGE(validate_trace_check_circuit(std::move(trace)), "SUBOP_ERROR_RELEVANT_OP");

trace_builder.reset();
trace_builder = AvmTraceBuilder(public_inputs);

trace_builder.calldata_copy(0, 0, 3, 0, std::vector<FF>{ 8, 4, 17 });

Expand All @@ -1910,7 +1911,7 @@ TEST_F(AvmArithmeticNegativeTestsFF, operationWithErrorFlag)

EXPECT_THROW_WITH_MESSAGE(validate_trace_check_circuit(std::move(trace)), "SUBOP_ERROR_RELEVANT_OP");

trace_builder.reset();
trace_builder = AvmTraceBuilder(public_inputs);

trace_builder.calldata_copy(0, 0, 3, 0, std::vector<FF>{ 5, 0, 20 });

Expand Down

0 comments on commit 54339d4

Please sign in to comment.