From 64f54e006b09b0069ed95b31c6e564a49026d224 Mon Sep 17 00:00:00 2001 From: IlyasRidhuan Date: Fri, 21 Jun 2024 11:23:56 +0000 Subject: [PATCH] fix(avm): re-enable ext call test --- .../barretenberg/vm/avm_trace/avm_trace.cpp | 72 ++++----- .../vm/tests/avm_execution.test.cpp | 145 ++++++++++-------- 2 files changed, 104 insertions(+), 113 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp index d0fc170cb0d0..d6fced0e1c04 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp @@ -2553,14 +2553,14 @@ 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(main_trace.size()) + 1; @@ -2568,7 +2568,7 @@ void AvmTraceBuilder::op_call([[maybe_unused]] uint8_t indirect, gas_trace_builder.constrain_gas_for_external_call( clk, static_cast(hint.l2_gas_used), static_cast(hint.da_gas_used)); - // We can load up to 4 things per row + auto [resolved_gas_offset, resolved_addr_offset, resolved_args_offset, @@ -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(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(read_gas.is_indirect)), + .main_sel_resolve_ind_addr_a = FF(static_cast(read_gas_l2.is_indirect)), .main_sel_resolve_ind_addr_c = FF(static_cast(read_addr.is_indirect)), .main_sel_resolve_ind_addr_d = FF(static_cast(read_args.is_indirect)), .main_tag_err = FF(static_cast(!tag_match)), }); clk++; - // Read the rest on a separate line, remember that the 4th operand is indirect - auto read_ret = constrained_read_from_memory( - 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(AvmMemoryTag::FF)), - .main_sel_mem_op_a = FF(1), - .main_sel_resolve_ind_addr_a = FF(static_cast(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(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, diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp index 076f70cabae1..b61b004e091a 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp @@ -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 calldata = {}; -// std::vector 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({ 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 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 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({ 9, 8, 1 })); // The 1 represents the success + + validate_trace(std::move(trace), public_inputs); +} TEST_F(AvmExecutionTests, opGetContractInstanceOpcodes) {