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.

Sign up for GitHub

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 delegate call and storage address #9330

Merged
merged 6 commits into from
Oct 22, 2024
Merged

chore!: remove delegate call and storage address #9330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4e1d653
Remove delegate call and storage address.
LeilaWang Oct 22, 2024
8d23516
Merge remote-tracking branch 'origin/master' into lw/bye_bye_delegate…
LeilaWang Oct 22, 2024
5fcfccb
Update docs.
LeilaWang Oct 22, 2024
f2b7bc1
Fix test.
LeilaWang Oct 22, 2024
67c60d5
Remove links.
LeilaWang Oct 22, 2024
e6f1ff0
Fix tests.
LeilaWang Oct 22, 2024
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
2 changes: 0 additions & 2 deletions avm-transpiler/src/opcodes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ pub enum AvmOpcode {
// External calls
CALL,
STATICCALL,
DELEGATECALL,
RETURN,
REVERT_8,
REVERT_16,
Expand Down Expand Up @@ -161,7 +160,6 @@ impl AvmOpcode {
// Control Flow - Contract Calls
AvmOpcode::CALL => "CALL",
AvmOpcode::STATICCALL => "STATICCALL",
AvmOpcode::DELEGATECALL => "DELEGATECALL",
AvmOpcode::RETURN => "RETURN",
AvmOpcode::REVERT_8 => "REVERT_8",
AvmOpcode::REVERT_16 => "REVERT_16",
Expand Down
2 changes: 0 additions & 2 deletions avm-transpiler/src/transpile.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -800,7 +800,6 @@ fn handle_getter_instruction(
) {
enum EnvironmentVariable {
ADDRESS,
STORAGEADDRESS,
SENDER,
FUNCTIONSELECTOR,
TRANSACTIONFEE,
Expand All @@ -827,7 +826,6 @@ fn handle_getter_instruction(

let var_idx = match function {
"avmOpcodeAddress" => EnvironmentVariable::ADDRESS,
"avmOpcodeStorageAddress" => EnvironmentVariable::STORAGEADDRESS,
"avmOpcodeSender" => EnvironmentVariable::SENDER,
"avmOpcodeFeePerL2Gas" => EnvironmentVariable::FEEPERL2GAS,
"avmOpcodeFeePerDaGas" => EnvironmentVariable::FEEPERDAGAS,
Expand Down
1 change: 0 additions & 1 deletion barretenberg/cpp/pil/avm/constants_gen.pil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ namespace constants(256);
pol MEM_TAG_U128 = 6;
pol SENDER_KERNEL_INPUTS_COL_OFFSET = 0;
pol ADDRESS_KERNEL_INPUTS_COL_OFFSET = 1;
pol STORAGE_ADDRESS_KERNEL_INPUTS_COL_OFFSET = 1;
pol FUNCTION_SELECTOR_KERNEL_INPUTS_COL_OFFSET = 2;
pol IS_STATIC_CALL_KERNEL_INPUTS_COL_OFFSET = 3;
pol CHAIN_ID_KERNEL_INPUTS_COL_OFFSET = 4;
Expand Down
5 changes: 1 addition & 4 deletions barretenberg/cpp/pil/avm/kernel.pil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@ namespace main(256);
#[ADDRESS_KERNEL]
sel_op_address * (kernel_in_offset - constants.ADDRESS_KERNEL_INPUTS_COL_OFFSET) = 0;

#[STORAGE_ADDRESS_KERNEL]
sel_op_storage_address * (kernel_in_offset - constants.STORAGE_ADDRESS_KERNEL_INPUTS_COL_OFFSET) = 0;

#[SENDER_KERNEL]
sel_op_sender * (kernel_in_offset - constants.SENDER_KERNEL_INPUTS_COL_OFFSET) = 0;

Expand Down Expand Up @@ -174,7 +171,7 @@ namespace main(256);
//KERNEL_OUTPUT_SELECTORS * (side_effect_counter' - (side_effect_counter + 1)) = 0;

//===== LOOKUPS INTO THE PUBLIC INPUTS ===========================================
pol KERNEL_INPUT_SELECTORS = sel_op_address + sel_op_storage_address + sel_op_sender
pol KERNEL_INPUT_SELECTORS = sel_op_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_timestamp
+ sel_op_fee_per_l2_gas + sel_op_fee_per_da_gas + sel_op_is_static_call;
Expand Down
4 changes: 1 addition & 3 deletions barretenberg/cpp/pil/avm/main.pil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ namespace main(256);

pol commit sel_execution_end; // Toggle next row of last execution trace row.
// Used as a LHS selector of the lookup to enforce final gas values which correspond to
// l2_gas_remaining and da_gas_remaining values located at the row after last execution row.
// l2_gas_remaining and da_gas_remaining values located at the row after last execution row.
sel_execution_end' = sel_execution_row * (1 - sel_execution_row') * (1 - sel_first);

//===== PUBLIC COLUMNS=========================================================
Expand All @@ -47,7 +47,6 @@ namespace main(256);

// CONTEXT - ENVIRONMENT
pol commit sel_op_address;
pol commit sel_op_storage_address;
pol commit sel_op_sender;
pol commit sel_op_function_selector;
pol commit sel_op_transaction_fee;
Expand Down Expand Up @@ -218,7 +217,6 @@ namespace main(256);
// TODO: Very likely, we can remove these constraints as the selectors should be derived during
// opcode decomposition.
sel_op_address * (1 - sel_op_address) = 0;
sel_op_storage_address * (1 - sel_op_storage_address) = 0;
sel_op_sender * (1 - sel_op_sender) = 0;
sel_op_function_selector * (1 - sel_op_function_selector) = 0;
sel_op_transaction_fee * (1 - sel_op_transaction_fee) = 0;
Expand Down
109 changes: 44 additions & 65 deletions barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1403,57 +1403,53 @@ TEST_F(AvmExecutionTests, kernelInputOpcodes)
"0001" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::STORAGEADDRESS)) + // envvar STORAGEADDRESS
"0002" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::SENDER)) + // envvar SENDER
"0003" // dst_offset
"0002" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::FUNCTIONSELECTOR)) + // envvar FUNCTIONSELECTOR
"0004" // dst_offset
"0003" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::TRANSACTIONFEE)) + // envvar TRANSACTIONFEE
"0005" // dst_offset
"0004" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::CHAINID)) + // envvar CHAINID
"0006" // dst_offset
"0005" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::VERSION)) + // envvar VERSION
"0007" // dst_offset
"0006" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::BLOCKNUMBER)) + // envvar BLOCKNUMBER
"0008" // dst_offset
"0007" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::TIMESTAMP)) + // envvar TIMESTAMP
"0009" // dst_offset
"0008" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::FEEPERL2GAS)) + // envvar FEEPERL2GAS
"000A" // dst_offset
"0009" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::FEEPERDAGAS)) + // envvar FEEPERDAGAS
"000B" // dst_offset
"000A" // dst_offset
+ to_hex(OpCode::GETENVVAR_16) + // opcode GETENVVAR_16
"00" // Indirect flag
+ to_hex(static_cast<uint8_t>(EnvironmentVariable::ISSTATICCALL)) + // envvar ISSTATICCALL
"000C" // dst_offset
"000B" // dst_offset
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"0001" // ret offset 1
"000C"; // ret size 12
"000B"; // ret size 12

auto bytecode = hex_to_bytes(bytecode_hex);
auto instructions = Deserialization::parse(bytecode);

ASSERT_THAT(instructions, SizeIs(13));
ASSERT_THAT(instructions, SizeIs(12));

// ADDRESS
EXPECT_THAT(instructions.at(0),
Expand All @@ -1463,126 +1459,114 @@ TEST_F(AvmExecutionTests, kernelInputOpcodes)
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::ADDRESS)),
VariantWith<uint16_t>(1)))));

// STORAGEADDRESS
EXPECT_THAT(instructions.at(1),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::STORAGEADDRESS)),
VariantWith<uint16_t>(2)))));

// SENDER
EXPECT_THAT(instructions.at(2),
EXPECT_THAT(instructions.at(1),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::SENDER)),
VariantWith<uint16_t>(3)))));
VariantWith<uint16_t>(2)))));

// FUNCTIONSELECTOR
EXPECT_THAT(
instructions.at(3),
instructions.at(2),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::FUNCTIONSELECTOR)),
VariantWith<uint16_t>(4)))));
VariantWith<uint16_t>(3)))));

// TRANSACTIONFEE
EXPECT_THAT(instructions.at(4),
EXPECT_THAT(instructions.at(3),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::TRANSACTIONFEE)),
VariantWith<uint16_t>(5)))));
VariantWith<uint16_t>(4)))));

// CHAINID
EXPECT_THAT(instructions.at(5),
EXPECT_THAT(instructions.at(4),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::CHAINID)),
VariantWith<uint16_t>(6)))));
VariantWith<uint16_t>(5)))));

// VERSION
EXPECT_THAT(instructions.at(6),
EXPECT_THAT(instructions.at(5),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::VERSION)),
VariantWith<uint16_t>(7)))));
VariantWith<uint16_t>(6)))));

// BLOCKNUMBER
EXPECT_THAT(instructions.at(7),
EXPECT_THAT(instructions.at(6),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::BLOCKNUMBER)),
VariantWith<uint16_t>(8)))));
VariantWith<uint16_t>(7)))));

// TIMESTAMP
EXPECT_THAT(instructions.at(8),
EXPECT_THAT(instructions.at(7),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::TIMESTAMP)),
VariantWith<uint16_t>(9)))));
VariantWith<uint16_t>(8)))));

// FEEPERL2GAS
EXPECT_THAT(instructions.at(9),
EXPECT_THAT(instructions.at(8),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::FEEPERL2GAS)),
VariantWith<uint16_t>(10)))));
VariantWith<uint16_t>(9)))));

// FEEPERDAGAS
EXPECT_THAT(instructions.at(10),
EXPECT_THAT(instructions.at(9),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::FEEPERDAGAS)),
VariantWith<uint16_t>(11)))));
VariantWith<uint16_t>(10)))));

// ISSTATICCALL
EXPECT_THAT(instructions.at(11),
EXPECT_THAT(instructions.at(10),
AllOf(Field(&Instruction::op_code, OpCode::GETENVVAR_16),
Field(&Instruction::operands,
ElementsAre(VariantWith<uint8_t>(0),
VariantWith<uint8_t>(static_cast<uint8_t>(EnvironmentVariable::ISSTATICCALL)),
VariantWith<uint16_t>(12)))));
VariantWith<uint16_t>(11)))));

// Public inputs for the circuit
std::vector<FF> calldata;

FF sender = 1;
FF address = 2;
// NOTE: address doesn't actually exist in public circuit public inputs,
// so storage address is just an alias of address for now
FF storage_address = address;
FF function_selector = 4;
FF transaction_fee = 5;
FF chainid = 6;
FF version = 7;
FF blocknumber = 8;
FF timestamp = 9;
FF feeperl2gas = 10;
FF feeperdagas = 11;
FF is_static_call = 12;
FF function_selector = 3;
FF transaction_fee = 4;
FF chainid = 5;
FF version = 6;
FF blocknumber = 7;
FF timestamp = 8;
FF feeperl2gas = 9;
FF feeperdagas = 10;
FF is_static_call = 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, timestamp, feeperl2gas, feeperdagas, is_static_call,
address, sender, function_selector, transaction_fee, chainid, version,
blocknumber, timestamp, feeperl2gas, feeperdagas, is_static_call,
};

// Set up public inputs to contain the above values
// TODO: maybe have a javascript like object construction so that this is readable
// Reduce the amount of times we have similar code to this
//
public_inputs_vec[STORAGE_ADDRESS_PCPI_OFFSET] = address;
public_inputs_vec[STORAGE_ADDRESS_PCPI_OFFSET] = storage_address;
public_inputs_vec[ADDRESS_PCPI_OFFSET] = address;
public_inputs_vec[SENDER_PCPI_OFFSET] = sender;
public_inputs_vec[FUNCTION_SELECTOR_PCPI_OFFSET] = function_selector;
public_inputs_vec[TRANSACTION_FEE_PCPI_OFFSET] = transaction_fee;
Expand All @@ -1609,11 +1593,6 @@ TEST_F(AvmExecutionTests, kernelInputOpcodes)
std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_address == 1; });
EXPECT_EQ(address_row->main_ia, address);

// Check storage address
auto storage_addr_row =
std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_storage_address == 1; });
EXPECT_EQ(storage_addr_row->main_ia, storage_address);

// Check sender
auto sender_row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.main_sel_op_sender == 1; });
EXPECT_EQ(sender_row->main_ia, sender);
Expand Down
Loading
Loading