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

fix(avm): re-enable ext call test #7147

Merged
merged 1 commit into from
Jun 24, 2024
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
72 changes: 27 additions & 45 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2553,22 +2553,22 @@ uint32_t AvmTraceBuilder::read_slice_to_memory(uint8_t space_id,
* stored
* @param function_selector_offset An index in memory pointing to the function selector of the external call (TEMP)
*/
void AvmTraceBuilder::op_call([[maybe_unused]] uint8_t indirect,
[[maybe_unused]] uint32_t gas_offset,
[[maybe_unused]] uint32_t addr_offset,
[[maybe_unused]] uint32_t args_offset,
[[maybe_unused]] uint32_t args_size,
[[maybe_unused]] uint32_t ret_offset,
[[maybe_unused]] uint32_t ret_size,
[[maybe_unused]] uint32_t success_offset,
void AvmTraceBuilder::op_call(uint8_t indirect,
uint32_t gas_offset,
uint32_t addr_offset,
uint32_t args_offset,
uint32_t args_size,
uint32_t ret_offset,
uint32_t ret_size,
uint32_t success_offset,
[[maybe_unused]] uint32_t function_selector_offset)
{
auto clk = static_cast<uint32_t>(main_trace.size()) + 1;
const ExternalCallHint& hint = execution_hints.externalcall_hints.at(external_call_counter);

gas_trace_builder.constrain_gas_for_external_call(
clk, static_cast<uint32_t>(hint.l2_gas_used), static_cast<uint32_t>(hint.da_gas_used));
// We can load up to 4 things per row

auto [resolved_gas_offset,
resolved_addr_offset,
resolved_args_offset,
Expand All @@ -2577,70 +2577,52 @@ void AvmTraceBuilder::op_call([[maybe_unused]] uint8_t indirect,
resolved_success_offset] =
unpack_indirects<6>(indirect, { gas_offset, addr_offset, args_offset, args_size, ret_offset, success_offset });

// Should read the address next to read_gas as well (tuple of gas values)
auto read_gas = constrained_read_from_memory(
// Should read the address next to read_gas as well (tuple of gas values (l2Gas, daGas))
auto read_gas_l2 = constrained_read_from_memory(
call_ptr, clk, resolved_gas_offset, AvmMemoryTag::FF, AvmMemoryTag::U0, IntermRegister::IA);
auto read_gas_da = mem_trace_builder.read_and_load_from_memory(
call_ptr, clk, IntermRegister::IB, read_gas_l2.direct_address + 1, AvmMemoryTag::FF, AvmMemoryTag::U0);
auto read_addr = constrained_read_from_memory(
call_ptr, clk, resolved_addr_offset, AvmMemoryTag::FF, AvmMemoryTag::U0, IntermRegister::IC);
auto read_args = constrained_read_from_memory(
call_ptr, clk, resolved_args_offset, AvmMemoryTag::FF, AvmMemoryTag::U0, IntermRegister::ID);
bool tag_match = read_gas.tag_match && read_addr.tag_match && read_args.tag_match;
bool tag_match = read_gas_l2.tag_match && read_gas_da.tag_match && read_addr.tag_match && read_args.tag_match;

// We read the input and output addresses in one row as they should contain FF elements
main_trace.push_back(Row{
.main_clk = clk,
.main_ia = read_gas.val, /* gas_offset */
.main_ic = read_addr.val, /* addr_offset */
.main_id = read_args.val, /* args_offset */
.main_ind_addr_a = FF(read_gas.indirect_address),
.main_ia = read_gas_l2.val, /* gas_offset_l2 */
.main_ib = read_gas_da.val, /* gas_offset_da */
.main_ic = read_addr.val, /* addr_offset */
.main_id = read_args.val, /* args_offset */
.main_ind_addr_a = FF(read_gas_l2.indirect_address),
.main_ind_addr_c = FF(read_addr.indirect_address),
.main_ind_addr_d = FF(read_args.indirect_address),
.main_internal_return_ptr = FF(internal_return_ptr),
.main_mem_addr_a = FF(read_gas.direct_address),
.main_mem_addr_a = FF(read_gas_l2.direct_address),
.main_mem_addr_b = FF(read_gas_l2.direct_address + 1),
.main_mem_addr_c = FF(read_addr.direct_address),
.main_mem_addr_d = FF(read_args.direct_address),
.main_pc = FF(pc),
.main_r_in_tag = FF(static_cast<uint32_t>(AvmMemoryTag::FF)),
.main_sel_mem_op_a = FF(1),
.main_sel_mem_op_b = FF(1),
.main_sel_mem_op_c = FF(1),
.main_sel_mem_op_d = FF(1),
.main_sel_op_external_call = FF(1),
.main_sel_resolve_ind_addr_a = FF(static_cast<uint32_t>(read_gas.is_indirect)),
.main_sel_resolve_ind_addr_a = FF(static_cast<uint32_t>(read_gas_l2.is_indirect)),
.main_sel_resolve_ind_addr_c = FF(static_cast<uint32_t>(read_addr.is_indirect)),
.main_sel_resolve_ind_addr_d = FF(static_cast<uint32_t>(read_args.is_indirect)),
.main_tag_err = FF(static_cast<uint32_t>(!tag_match)),
});
clk++;
// Read the rest on a separate line, remember that the 4th operand is indirect
auto read_ret = constrained_read_from_memory(
IlyasRidhuan marked this conversation as resolved.
Show resolved Hide resolved
call_ptr, clk, resolved_ret_offset, AvmMemoryTag::FF, AvmMemoryTag::U0, IntermRegister::IA);
main_trace.push_back(Row{
.main_clk = clk,
.main_ia = read_ret.val, /* ret_offset */
.main_ind_addr_a = FF(read_ret.indirect_address),
.main_internal_return_ptr = FF(internal_return_ptr),
.main_mem_addr_a = FF(read_ret.direct_address),
.main_pc = FF(pc),
.main_r_in_tag = FF(static_cast<uint32_t>(AvmMemoryTag::FF)),
.main_sel_mem_op_a = FF(1),
.main_sel_resolve_ind_addr_a = FF(static_cast<uint32_t>(read_ret.is_indirect)),
});
clk++;
auto mem_read_success = constrained_read_from_memory(
call_ptr, clk, resolved_success_offset, AvmMemoryTag::U32, AvmMemoryTag::U0, IntermRegister::IA);
main_trace.push_back(Row{
.main_clk = clk,
.main_ia = mem_read_success.val, /* success_offset */
.main_internal_return_ptr = FF(internal_return_ptr),
.main_mem_addr_a = FF(success_offset),
.main_pc = FF(pc),
.main_r_in_tag = FF(static_cast<uint32_t>(AvmMemoryTag::U32)),
.main_sel_mem_op_a = FF(1),
});
clk++;
// The return data hint is used for now, we check it has the same length as the ret_size
ASSERT(hint.return_data.size() == ret_size);
// Write the return data to memory
uint32_t num_rows = write_slice_to_memory(
call_ptr, clk, resolved_ret_offset, AvmMemoryTag::U0, AvmMemoryTag::FF, internal_return_ptr, hint.return_data);
clk += num_rows;
// Write the success flag to memory
write_slice_to_memory(call_ptr,
clk,
resolved_success_offset,
Expand Down
145 changes: 77 additions & 68 deletions barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2129,74 +2129,83 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes)
validate_trace(std::move(trace), public_inputs);
}

// TEST_F(AvmExecutionTests, opCallOpcodes)
// {
// std::string bytecode_preamble;
// // Gas offset preamble
// bytecode_preamble += to_hex(OpCode::SET) + // opcode SET for gas offset indirect
// "00" // Indirect flag
// "03" // U32
// "00000010" // val 16 (address where gas offset is located)
// "00000011" + // dst_offset 17
// to_hex(OpCode::SET) + // opcode SET for value stored in gas offset
// "00" // Indirect flag
// "03" // U32
// "00000011" // val i
// "00000000";
// // args offset preamble
// bytecode_preamble += to_hex(OpCode::SET) + // opcode SET for args offset indirect
// "00" // Indirect flag
// "03" // U32
// "00000100" // val i
// "00000012" + // dst_offset 0
// to_hex(OpCode::SET) + // opcode SET for value stored in args offset
// "00" // Indirect flag
// "03" // U32
// "00000012" // val i
// "00000001";
// // ret offset preamble
// bytecode_preamble += to_hex(OpCode::SET) + // opcode SET for ret offset indirect
// "00" // Indirect flag
// "03" // U32
// "00000008" // val i
// "00000004" + // dst_offset 0
// to_hex(OpCode::SET) + // opcode SET for value stored in ret offset
// "00" // Indirect flag
// "03" // U32
// "00000002" // val i
// "00000007";
// std::string bytecode_hex = bytecode_preamble // SET gas, addr, args size, ret offset, success, function
// selector
// + to_hex(OpCode::CALL) + // opcode CALL
// "15" // Indirect flag
// "00000000" // gas offset
// "00000001" // addr offset
// "00000002" // args offset
// "00000003" // args size offset
// "00000004" // ret offset
// "00000007" // ret size
// "0000000a" // success offset
// "00000006" // function_selector_offset
// + to_hex(OpCode::RETURN) + // opcode RETURN
// "00" // Indirect flag
// "00000008" // ret offset 8
// "00000003"; // ret size 3

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

// std::vector<FF> calldata = {};
// std::vector<FF> returndata = {};

// // Generate Hint for call operation
// auto execution_hints = ExecutionHints().with_externalcall_hints(
// { { .success = 1, .return_data = { 9, 8 }, .l2_gas_used = 0, .da_gas_used = 0 } });

// auto trace = Execution::gen_trace(instructions, returndata, calldata, public_inputs_vec, execution_hints);
// EXPECT_EQ(returndata, std::vector<FF>({ 9, 8, 1 })); // The 1 represents the success

// validate_trace(std::move(trace), public_inputs);
// }
TEST_F(AvmExecutionTests, opCallOpcodes)
{
// Calldata for l2_gas, da_gas, contract_address, nested_call_args (4 elements),
std::vector<FF> calldata = { 17, 10, 34802342, 1, 2, 3, 4 };
std::string bytecode_preamble;
// Set up Gas offsets
bytecode_preamble += to_hex(OpCode::SET) + // opcode SET for gas offset indirect
"00" // Indirect flag
"03" // U32
"00000000" // val 0 (address where gas tuple is located)
"00000011"; // dst_offset 17
// Set up contract address offset
bytecode_preamble += to_hex(OpCode::SET) + // opcode SET for args offset indirect
"00" // Indirect flag
"03" // U32
"00000002" // val 2 (where contract address is located)
"00000012"; // dst_offset 18
// Set up args offset
bytecode_preamble += to_hex(OpCode::SET) + // opcode SET for ret offset indirect
"00" // Indirect flag
"03" // U32
"00000003" // val 3 (the start of the args array)
"00000013"; // dst_offset 19
// Set up args size offset
bytecode_preamble += to_hex(OpCode::SET) + // opcode SET for ret offset indirect
"00" // Indirect flag
"03" // U32
"00000004" // val 4 (the length of the args array)
"00000014"; // dst_offset 20
// Set up the ret offset
bytecode_preamble += to_hex(OpCode::SET) + // opcode SET for ret offset indirect
"00" // Indirect flag
"03" // U32
"00000100" // val 256 (the start of where to write the return data)
"00000015"; // dst_offset 21
// Set up the success offset
bytecode_preamble += to_hex(OpCode::SET) + // opcode SET for ret offset indirect
"00" // Indirect flag
"03" // U32
"00000102" // val 258 (write the success flag at ret_offset + ret_size)
"00000016"; // dst_offset 22

std::string bytecode_hex = to_hex(OpCode::CALLDATACOPY) + // opcode CALLDATACOPY
"00" // Indirect flag
"00000000" // cd_offset
"00000007" // copy_size
"00000000" // dst_offset
+ bytecode_preamble // Load up memory offsets
+ to_hex(OpCode::CALL) + // opcode CALL
"3f" // Indirect flag
"00000011" // gas offset
"00000012" // addr offset
"00000013" // args offset
"00000014" // args size offset
"00000015" // ret offset
"00000002" // ret size
"00000016" // success offset
"00000017" // function_selector_offset
+ to_hex(OpCode::RETURN) + // opcode RETURN
"00" // Indirect flag
"00000100" // ret offset 8
"00000003"; // ret size 3 (extra read is for the success flag)

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

std::vector<FF> returndata = {};

// Generate Hint for call operation
auto execution_hints = ExecutionHints().with_externalcall_hints(
{ { .success = 1, .return_data = { 9, 8 }, .l2_gas_used = 0, .da_gas_used = 0 } });

auto trace = Execution::gen_trace(instructions, returndata, calldata, public_inputs_vec, execution_hints);
EXPECT_EQ(returndata, std::vector<FF>({ 9, 8, 1 })); // The 1 represents the success

validate_trace(std::move(trace), public_inputs);
}

TEST_F(AvmExecutionTests, opGetContractInstanceOpcodes)
{
Expand Down
Loading