Skip to content

Commit

Permalink
chore!: remove coinbase and unimplemented block gas limit opcodes fro…
Browse files Browse the repository at this point in the history
…m AVM (#8408)
  • Loading branch information
dbanks12 authored Sep 6, 2024
1 parent e51d157 commit dd09b76
Show file tree
Hide file tree
Showing 25 changed files with 69 additions and 373 deletions.
6 changes: 0 additions & 6 deletions avm-transpiler/src/opcodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,8 @@ pub enum AvmOpcode {
VERSION,
BLOCKNUMBER,
TIMESTAMP,
COINBASE,
FEEPERL2GAS,
FEEPERDAGAS,
BLOCKL2GASLIMIT,
BLOCKDAGASLIMIT,
CALLDATACOPY,
// Gas
L2GASLEFT,
Expand Down Expand Up @@ -116,11 +113,8 @@ impl AvmOpcode {
AvmOpcode::VERSION => "VERSION",
AvmOpcode::BLOCKNUMBER => "BLOCKNUMBER",
AvmOpcode::TIMESTAMP => "TIMESTAMP",
AvmOpcode::COINBASE => "COINBASE",
AvmOpcode::FEEPERL2GAS => "FEEPERL2GAS",
AvmOpcode::FEEPERDAGAS => "FEEPERDAGAS",
AvmOpcode::BLOCKL2GASLIMIT => "BLOCKL2GASLIMIT",
AvmOpcode::BLOCKDAGASLIMIT => "BLOCKDAGASLIMIT",
// Execution Environment - Calldata
AvmOpcode::CALLDATACOPY => "CALLDATACOPY",

Expand Down
1 change: 0 additions & 1 deletion barretenberg/cpp/pil/avm/constants_gen.pil
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ namespace constants(256);
pol VERSION_SELECTOR = 30;
pol BLOCK_NUMBER_SELECTOR = 31;
pol TIMESTAMP_SELECTOR = 33;
pol COINBASE_SELECTOR = 34;
pol FEE_PER_DA_GAS_SELECTOR = 36;
pol FEE_PER_L2_GAS_SELECTOR = 37;
pol END_GLOBAL_VARIABLES = 38;
Expand Down
5 changes: 1 addition & 4 deletions barretenberg/cpp/pil/avm/kernel.pil
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,6 @@ namespace main(256);
#[TIMESTAMP_KERNEL]
sel_op_timestamp * (kernel_in_offset - constants.TIMESTAMP_SELECTOR) = 0;

#[COINBASE_KERNEL]
sel_op_coinbase * (kernel_in_offset - constants.COINBASE_SELECTOR) = 0;

// CONTEXT - ENVIRONMENT - GLOBALS - FEES
#[FEE_DA_GAS_KERNEL]
sel_op_fee_per_da_gas * (kernel_in_offset - constants.FEE_PER_DA_GAS_SELECTOR) = 0;
Expand Down Expand Up @@ -170,7 +167,7 @@ namespace main(256);
//===== LOOKUPS INTO THE PUBLIC INPUTS ===========================================
pol KERNEL_INPUT_SELECTORS = sel_op_address + sel_op_storage_address + sel_op_sender
+ sel_op_function_selector + sel_op_transaction_fee + sel_op_chain_id
+ sel_op_version + sel_op_block_number + sel_op_coinbase + sel_op_timestamp
+ sel_op_version + sel_op_block_number + sel_op_timestamp
+ sel_op_fee_per_l2_gas + sel_op_fee_per_da_gas;
// Ensure that only one kernel lookup is active when the kernel_in_offset is active
#[KERNEL_INPUT_ACTIVE_CHECK]
Expand Down
2 changes: 0 additions & 2 deletions barretenberg/cpp/pil/avm/main.pil
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ namespace main(256);
pol commit sel_op_chain_id;
pol commit sel_op_version;
pol commit sel_op_block_number;
pol commit sel_op_coinbase;
pol commit sel_op_timestamp;
// CONTEXT - ENVIRONMENT - GLOBALS - FEES
pol commit sel_op_fee_per_l2_gas;
Expand Down Expand Up @@ -224,7 +223,6 @@ namespace main(256);
sel_op_chain_id * (1 - sel_op_chain_id) = 0;
sel_op_version * (1 - sel_op_version) = 0;
sel_op_block_number * (1 - sel_op_block_number) = 0;
sel_op_coinbase * (1 - sel_op_coinbase) = 0;
sel_op_timestamp * (1 - sel_op_timestamp) = 0;
sel_op_fee_per_l2_gas * (1 - sel_op_fee_per_l2_gas) = 0;
sel_op_fee_per_da_gas * (1 - sel_op_fee_per_da_gas) = 0;
Expand Down
44 changes: 12 additions & 32 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1418,20 +1418,16 @@ TEST_F(AvmExecutionTests, kernelInputOpcodes)
+ to_hex(OpCode::TIMESTAMP) + // opcode TIMESTAMP
"00" // Indirect flag
"00000009" // dst_offset
// Not in simulator
// + to_hex(OpCode::COINBASE) + // opcode COINBASE
// "00" // Indirect flag
// "00000009" // dst_offset
+ to_hex(OpCode::FEEPERL2GAS) + // opcode FEEPERL2GAS
"00" // Indirect flag
"0000000a" // dst_offset
+ to_hex(OpCode::FEEPERDAGAS) + // opcode FEEPERDAGAS
"00" // Indirect flag
"0000000b" // dst_offset
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"00000001" // ret offset 1
"0000000b"; // ret size 11
+ to_hex(OpCode::FEEPERL2GAS) + // opcode FEEPERL2GAS
"00" // Indirect flag
"0000000a" // dst_offset
+ to_hex(OpCode::FEEPERDAGAS) + // opcode FEEPERDAGAS
"00" // Indirect flag
"0000000b" // dst_offset
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"00000001" // ret offset 1
"0000000b"; // ret size 11

auto bytecode = hex_to_bytes(bytecode_hex);
auto instructions = Deserialization::parse(bytecode);
Expand Down Expand Up @@ -1483,13 +1479,6 @@ TEST_F(AvmExecutionTests, kernelInputOpcodes)
AllOf(Field(&Instruction::op_code, OpCode::TIMESTAMP),
Field(&Instruction::operands, ElementsAre(VariantWith<uint8_t>(0), VariantWith<uint32_t>(9)))));

// COINBASE
// Not in simulator
// EXPECT_THAT(instructions.at(8),
// AllOf(Field(&Instruction::op_code, OpCode::COINBASE),
// Field(&Instruction::operands, ElementsAre(VariantWith<uint8_t>(0),
// VariantWith<uint32_t>(10)))));

// FEEPERL2GAS
EXPECT_THAT(instructions.at(9),
AllOf(Field(&Instruction::op_code, OpCode::FEEPERL2GAS),
Expand All @@ -1514,15 +1503,14 @@ TEST_F(AvmExecutionTests, kernelInputOpcodes)
FF version = 7;
FF blocknumber = 8;
FF timestamp = 9;
// FF coinbase = 10; // Not in simulator
FF feeperl2gas = 10;
FF feeperdagas = 11;

// The return data for this test should be a the opcodes in sequence, as the opcodes dst address lines up with
// this array The returndata call above will then return this array
std::vector<FF> const expected_returndata = {
address, storage_address, sender, function_selector, transaction_fee, chainid, version,
blocknumber, /*coinbase,*/ timestamp, feeperl2gas, feeperdagas,
address, storage_address, sender, function_selector, transaction_fee, chainid,
version, blocknumber, timestamp, feeperl2gas, feeperdagas,
};

// Set up public inputs to contain the above values
Expand All @@ -1540,8 +1528,6 @@ TEST_F(AvmExecutionTests, kernelInputOpcodes)
public_inputs_vec[VERSION_OFFSET] = version;
public_inputs_vec[BLOCK_NUMBER_OFFSET] = blocknumber;
public_inputs_vec[TIMESTAMP_OFFSET] = timestamp;
// Not in the simulator yet
// public_inputs_vec[COINBASE_OFFSET] = coinbase;
// Global variables - Gas
public_inputs_vec[FEE_PER_DA_GAS_OFFSET] = feeperdagas;
public_inputs_vec[FEE_PER_L2_GAS_OFFSET] = feeperl2gas;
Expand Down Expand Up @@ -1597,12 +1583,6 @@ TEST_F(AvmExecutionTests, kernelInputOpcodes)
std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_timestamp == 1; });
EXPECT_EQ(timestamp_row->main_ia, timestamp);

// // Check coinbase
// Not in simulator
// auto coinbase_row =
// std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_coinbase == 1; });
// EXPECT_EQ(coinbase_row->main_ia, coinbase);

// Check feeperdagas
auto feeperdagas_row =
std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_fee_per_da_gas == 1; });
Expand Down
60 changes: 0 additions & 60 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/kernel.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -525,40 +525,6 @@ TEST_F(AvmKernelPositiveTests, kernelBlockNumber)
test_kernel_lookup(true, indirect_apply_opcodes, checks);
}

TEST_F(AvmKernelPositiveTests, kernelCoinbase)
{
uint32_t dst_offset = 42;
uint32_t indirect_dst_offset = 69;
auto direct_apply_opcodes = [=](AvmTraceBuilder& trace_builder) {
trace_builder.op_coinbase(/*indirect*/ false, dst_offset);
};
auto indirect_apply_opcodes = [=](AvmTraceBuilder& trace_builder) {
trace_builder.op_set(
/*indirect*/ false,
/*value*/ dst_offset,
/*dst_offset*/ indirect_dst_offset,
AvmMemoryTag::U32);
trace_builder.op_coinbase(/*indirect*/ true, indirect_dst_offset);
};

auto checks = [=](bool indirect, const std::vector<Row>& trace) {
auto fee_row =
std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_coinbase == FF(1); });
EXPECT_TRUE(fee_row != trace.end());

expect_row(fee_row,
/*kernel_in_offset=*/COINBASE_SELECTOR,
/*ia=*/COINBASE_SELECTOR +
1, // Note the value generated above for public inputs is the same as the index read + 1
/*ind_a*/ indirect ? indirect_dst_offset : 0,
/*mem_addr_a*/ dst_offset,
/*w_in_tag=*/AvmMemoryTag::FF);
};

test_kernel_lookup(false, direct_apply_opcodes, checks);
test_kernel_lookup(true, indirect_apply_opcodes, checks);
}

TEST_F(AvmKernelPositiveTests, kernelTimestamp)
{
uint32_t dst_offset = 42;
Expand Down Expand Up @@ -913,32 +879,6 @@ TEST_F(AvmKernelNegativeTests, incorrectIaTimestamp)
negative_test_incorrect_ia_kernel_lookup(apply_opcodes, checks, incorrect_ia, BAD_LOOKUP);
}

TEST_F(AvmKernelNegativeTests, incorrectIaCoinbase)
{
uint32_t dst_offset = 42;
FF incorrect_ia = FF(69);

// We test that the sender opcode is inlcuded at index x in the public inputs
auto apply_opcodes = [=](AvmTraceBuilder& trace_builder) {
trace_builder.op_coinbase(/*indirect*/ false, dst_offset);
};
auto checks = [=](bool indirect, const std::vector<Row>& trace) {
auto row =
std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_coinbase == FF(1); });
EXPECT_TRUE(row != trace.end());

expect_row(
row,
/*kernel_in_offset=*/COINBASE_SELECTOR,
/*ia=*/incorrect_ia, // Note the value generated above for public inputs is the same as the index read + 1
/*ind_a*/ indirect,
/*mem_addr_a=*/dst_offset,
/*w_in_tag=*/AvmMemoryTag::FF);
};

negative_test_incorrect_ia_kernel_lookup(apply_opcodes, checks, incorrect_ia, BAD_LOOKUP);
}

// KERNEL OUTPUTS
class AvmKernelOutputPositiveTests : public AvmKernelTests {
protected:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,11 @@ const std::unordered_map<OpCode, std::vector<OperandType>> OPCODE_WIRE_FORMAT =
{ OpCode::CHAINID, getter_format },
{ OpCode::VERSION, getter_format },
{ OpCode::BLOCKNUMBER, getter_format },
// COINBASE, -- not in simulator
{ OpCode::TIMESTAMP, getter_format },
// Execution Environment - Globals - Gas
{ OpCode::FEEPERL2GAS, getter_format },
{ OpCode::FEEPERDAGAS, getter_format },
// BLOCKL2GASLIMIT, -- not in simulator
// BLOCKDAGASLIMIT, -- not in simulator
//

// Execution Environment - Calldata
{ OpCode::CALLDATACOPY, { OperandType::INDIRECT, OperandType::UINT32, OperandType::UINT32, OperandType::UINT32 } },

Expand Down
4 changes: 0 additions & 4 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/execution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ VmPublicInputs Execution::convert_public_inputs(std::vector<FF> const& public_in
kernel_inputs[VERSION_SELECTOR] = public_inputs_vec[VERSION_OFFSET]; // Version
kernel_inputs[BLOCK_NUMBER_SELECTOR] = public_inputs_vec[BLOCK_NUMBER_OFFSET]; // Block Number
kernel_inputs[TIMESTAMP_SELECTOR] = public_inputs_vec[TIMESTAMP_OFFSET]; // Timestamp
kernel_inputs[COINBASE_SELECTOR] = public_inputs_vec[COINBASE_OFFSET]; // Coinbase
// PublicCircuitPublicInputs - GlobalVariables - GasFees
kernel_inputs[FEE_PER_DA_GAS_SELECTOR] = public_inputs_vec[FEE_PER_DA_GAS_OFFSET];
kernel_inputs[FEE_PER_L2_GAS_SELECTOR] = public_inputs_vec[FEE_PER_L2_GAS_OFFSET];
Expand Down Expand Up @@ -548,9 +547,6 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
case OpCode::TIMESTAMP:
trace_builder.op_timestamp(std::get<uint8_t>(inst.operands.at(0)), std::get<uint32_t>(inst.operands.at(1)));
break;
case OpCode::COINBASE:
trace_builder.op_coinbase(std::get<uint8_t>(inst.operands.at(0)), std::get<uint32_t>(inst.operands.at(1)));
break;
case OpCode::FEEPERL2GAS:
trace_builder.op_fee_per_l2_gas(std::get<uint8_t>(inst.operands.at(0)),
std::get<uint32_t>(inst.operands.at(1)));
Expand Down
3 changes: 0 additions & 3 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/fixed_gas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,8 @@ const std::unordered_map<OpCode, FixedGasTable::GasRow> GAS_COST_TABLE = {
{ OpCode::VERSION, make_cost(AVM_VERSION_BASE_L2_GAS, 0, AVM_VERSION_DYN_L2_GAS, 0) },
{ OpCode::BLOCKNUMBER, make_cost(AVM_BLOCKNUMBER_BASE_L2_GAS, 0, AVM_BLOCKNUMBER_DYN_L2_GAS, 0) },
{ OpCode::TIMESTAMP, make_cost(AVM_TIMESTAMP_BASE_L2_GAS, 0, AVM_TIMESTAMP_DYN_L2_GAS, 0) },
{ OpCode::COINBASE, make_cost(AVM_COINBASE_BASE_L2_GAS, 0, AVM_COINBASE_DYN_L2_GAS, 0) },
{ OpCode::FEEPERL2GAS, make_cost(AVM_FEEPERL2GAS_BASE_L2_GAS, 0, AVM_FEEPERL2GAS_DYN_L2_GAS, 0) },
{ OpCode::FEEPERDAGAS, make_cost(AVM_FEEPERDAGAS_BASE_L2_GAS, 0, AVM_FEEPERDAGAS_DYN_L2_GAS, 0) },
{ OpCode::BLOCKL2GASLIMIT, make_cost(AVM_BLOCKL2GASLIMIT_BASE_L2_GAS, 0, AVM_BLOCKL2GASLIMIT_DYN_L2_GAS, 0) },
{ OpCode::BLOCKDAGASLIMIT, make_cost(AVM_BLOCKDAGASLIMIT_BASE_L2_GAS, 0, AVM_BLOCKDAGASLIMIT_DYN_L2_GAS, 0) },
{ OpCode::CALLDATACOPY, make_cost(AVM_CALLDATACOPY_BASE_L2_GAS, 0, AVM_CALLDATACOPY_DYN_L2_GAS, 0) },
{ OpCode::L2GASLEFT, make_cost(AVM_L2GASLEFT_BASE_L2_GAS, 0, AVM_L2GASLEFT_DYN_L2_GAS, 0) },
{ OpCode::DAGASLEFT, make_cost(AVM_DAGASLEFT_BASE_L2_GAS, 0, AVM_DAGASLEFT_DYN_L2_GAS, 0) },
Expand Down
14 changes: 0 additions & 14 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/kernel_trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,16 +124,6 @@ FF AvmKernelTraceBuilder::op_block_number(uint32_t clk)
return perform_kernel_input_lookup(BLOCK_NUMBER_SELECTOR);
}

FF AvmKernelTraceBuilder::op_coinbase(uint32_t clk)
{
KernelTraceEntry entry = {
.clk = clk,
.operation = KernelTraceOpType::COINBASE,
};
kernel_trace.push_back(entry);
return perform_kernel_input_lookup(COINBASE_SELECTOR);
}

FF AvmKernelTraceBuilder::op_timestamp(uint32_t clk)
{
KernelTraceEntry entry = {
Expand Down Expand Up @@ -393,10 +383,6 @@ void AvmKernelTraceBuilder::finalize(std::vector<AvmFullRow<FF>>& main_trace)
dest.main_kernel_in_offset = BLOCK_NUMBER_SELECTOR;
dest.main_sel_q_kernel_lookup = 1;
break;
case KernelTraceOpType::COINBASE:
dest.main_kernel_in_offset = COINBASE_SELECTOR;
dest.main_sel_q_kernel_lookup = 1;
break;
case KernelTraceOpType::TIMESTAMP:
dest.main_kernel_in_offset = TIMESTAMP_SELECTOR;
dest.main_sel_q_kernel_lookup = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ class AvmKernelTraceBuilder {
CHAIN_ID,
VERSION,
BLOCK_NUMBER,
COINBASE,
TIMESTAMP,
FEE_PER_DA_GAS,
FEE_PER_L2_GAS,
Expand Down Expand Up @@ -78,7 +77,6 @@ class AvmKernelTraceBuilder {
FF op_chain_id(uint32_t clk);
FF op_version(uint32_t clk);
FF op_block_number(uint32_t clk);
FF op_coinbase(uint32_t clk);
FF op_timestamp(uint32_t clk);
// Globals - Gas
FF op_fee_per_da_gas(uint32_t clk);
Expand Down
6 changes: 0 additions & 6 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,10 @@ std::string to_string(OpCode opcode)
return "BLOCKNUMBER";
case OpCode::TIMESTAMP:
return "TIMESTAMP";
case OpCode::COINBASE:
return "COINBASE";
case OpCode::FEEPERL2GAS:
return "FEEPERL2GAS";
case OpCode::FEEPERDAGAS:
return "FEEPERDAGAS";
case OpCode::BLOCKL2GASLIMIT:
return "BLOCKL2GASLIMIT";
case OpCode::BLOCKDAGASLIMIT:
return "BLOCKDAGASLIMIT";
// Execution Environment - Calldata
case OpCode::CALLDATACOPY:
return "CALLDATACOPY";
Expand Down
3 changes: 0 additions & 3 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/opcode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,8 @@ enum class OpCode : uint8_t {
VERSION,
BLOCKNUMBER,
TIMESTAMP,
COINBASE,
FEEPERL2GAS,
FEEPERDAGAS,
BLOCKL2GASLIMIT,
BLOCKDAGASLIMIT,
// Execution Environment - Calldata
CALLDATACOPY,

Expand Down
13 changes: 0 additions & 13 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1381,19 +1381,6 @@ void AvmTraceBuilder::op_timestamp(uint8_t indirect, uint32_t dst_offset)
main_trace.push_back(row);
}

void AvmTraceBuilder::op_coinbase(uint8_t indirect, uint32_t dst_offset)
{
auto const clk = static_cast<uint32_t>(main_trace.size()) + 1;
FF ia_value = kernel_trace_builder.op_coinbase(clk);
Row row = create_kernel_lookup_opcode(indirect, dst_offset, ia_value, AvmMemoryTag::FF);
row.main_sel_op_coinbase = FF(1);

// Constrain gas cost
gas_trace_builder.constrain_gas(static_cast<uint32_t>(row.main_clk), OpCode::COINBASE);

main_trace.push_back(row);
}

void AvmTraceBuilder::op_fee_per_l2_gas(uint8_t indirect, uint32_t dst_offset)
{
auto const clk = static_cast<uint32_t>(main_trace.size()) + 1;
Expand Down
1 change: 0 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ class AvmTraceBuilder {
void op_version(uint8_t indirect, uint32_t dst_offset);
void op_block_number(uint8_t indirect, uint32_t dst_offset);
void op_timestamp(uint8_t indirect, uint32_t dst_offset);
void op_coinbase(uint8_t indirect, uint32_t dst_offset);
void op_fee_per_l2_gas(uint8_t indirect, uint32_t dst_offset);
void op_fee_per_da_gas(uint8_t indirect, uint32_t dst_offset);

Expand Down
Loading

0 comments on commit dd09b76

Please sign in to comment.