Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore!: remove coinbase and unimplemented block gas limit opcodes from AVM #8408

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
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
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
Loading