Skip to content

Commit

Permalink
feat: avm e2e nested call + alu fix + cast fix (#6974)
Browse files Browse the repository at this point in the history 
Co-authored-by: fcarreiro <[email protected]>
Co-authored-by: dbanks12 <[email protected]>
Co-authored-by: jeanmon <[email protected]>
Co-authored-by: IlyasRidhuan <[email protected]>
  • Loading branch information
@Maddiaa0 @fcarreiro
@dbanks12 @jeanmon @IlyasRidhuan
5 people authored Jun 7, 2024
1 parent c6cbe39 commit b150b61
Show file tree
Hide file tree
Showing 7 changed files with 127 additions and 116 deletions.
2 changes: 1 addition & 1 deletion barretenberg/cpp/pil/avm/avm_alu.pil
View file
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,7 @@ namespace avm_alu(256);
// 128-bit multiplication and CAST need two rows in ALU trace. We need to ensure
// that another ALU operation does not start in the second row.
#[TWO_LINE_OP_NO_OVERLAP]
(op_mul * ff_tag + op_cast) * alu_sel' = 0;
(op_mul * u128_tag + op_cast) * alu_sel' = 0;

// ========= SHIFT LEFT/RIGHT OPERATIONS ===============================
// Given (1) an input b, within the range [0, 2**128-1],
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/relations/generated/avm/avm_alu.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ template <typename FF_> class avm_aluImpl {
{
Avm_DECLARE_VIEWS(54);

auto tmp = (((avm_alu_op_mul * avm_alu_ff_tag) + avm_alu_op_cast) * avm_alu_alu_sel_shift);
auto tmp = (((avm_alu_op_mul * avm_alu_u128_tag) + avm_alu_op_cast) * avm_alu_alu_sel_shift);
tmp *= scaling_factor;
std::get<54>(evals) += tmp;
}
Expand Down
10 changes: 8 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_common.hpp
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include "barretenberg/common/throw_or_abort.hpp"
#include "barretenberg/serialize/msgpack.hpp"
#include "barretenberg/vm/avm_trace/constants.hpp"
#include "barretenberg/vm/generated/avm_flavor.hpp"
Expand Down Expand Up @@ -45,8 +46,8 @@ static const uint32_t MAX_SIZE_INTERNAL_STACK = 1 << 16;
struct ExternalCallHint {
FF success;
std::vector<FF> return_data;
FF l2_gas_used;
FF da_gas_used;
uint32_t l2_gas_used;
uint32_t da_gas_used;
};

// Add support for deserialization of ExternalCallHint. This is implicitly used by serialize::read
Expand Down Expand Up @@ -169,6 +170,11 @@ struct ExecutionHints {
contract_instance_hints[instance.address] = instance;
}

if (it != data.data() + data.size()) {
throw_or_abort("Failed to deserialize ExecutionHints: only read" + std::to_string(it - data.data()) +
" bytes out of " + std::to_string(data.size()) + " bytes");
}

return { std::move(storage_value_hints), std::move(note_hash_exists_hints),
std::move(nullifier_exists_hints), std::move(l1_to_l2_message_exists_hints),
std::move(externalcall_hints), std::move(contract_instance_hints) };
Expand Down
81 changes: 36 additions & 45 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <set>
#include <string>
#include <sys/types.h>
#include <unordered_map>
#include <vector>

#include "barretenberg/common/throw_or_abort.hpp"
Expand Down Expand Up @@ -2619,15 +2620,16 @@ uint32_t AvmTraceBuilder::read_slice_to_memory(uint8_t space_id,
* @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,
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,
uint32_t function_selector_offset)
[[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,
[[maybe_unused]] uint32_t function_selector_offset)
{
// pc++;
auto clk = static_cast<uint32_t>(main_trace.size()) + 1;
const ExternalCallHint& hint = execution_hints.externalcall_hints.at(external_call_counter);
// We can load up to 4 things per row
Expand All @@ -2642,17 +2644,16 @@ void AvmTraceBuilder::op_call([[maybe_unused]] uint8_t indirect,
mem_trace_builder.indirect_read_and_load_from_memory(call_ptr, clk, IndirectRegister::IND_C, args_offset);

std::vector<uint32_t> first_row_load = {
uint32_t(read_ind_gas_offset.val), addr_offset, uint32_t(read_ind_args_offset.val), args_size
uint32_t(read_ind_gas_offset.val),
addr_offset,
uint32_t(read_ind_args_offset.val),
};
std::vector<uint32_t> first_row_values = {};
std::vector<FF> first_row_values = {};
for (uint32_t j = 0; j < first_row_load.size(); j++) {
// We just read and load to set up the constraints, we dont actually use these values for now.
auto mem_read = mem_trace_builder.read_and_load_from_memory(call_ptr,
clk,
register_order[j % register_order.size()],
first_row_load[j],
AvmMemoryTag::U32,
AvmMemoryTag::U0);
// info("Register order ", register_order[j]);
auto mem_read = mem_trace_builder.read_and_load_from_memory(
call_ptr, clk, register_order[j], first_row_load[j], AvmMemoryTag::FF, AvmMemoryTag::U0);
first_row_values.emplace_back(mem_read.val);
}

Expand All @@ -2662,7 +2663,6 @@ void AvmTraceBuilder::op_call([[maybe_unused]] uint8_t indirect,
.avm_main_ia = first_row_values[0], /* gas_offset */
.avm_main_ib = first_row_values[1], /* addr_offset */
.avm_main_ic = first_row_values[2], /* args_offset */
.avm_main_id = first_row_values[3], /* args_size */
.avm_main_ind_a = gas_offset,
.avm_main_ind_c = args_offset,
.avm_main_ind_op_a = FF(1),
Expand All @@ -2671,50 +2671,40 @@ void AvmTraceBuilder::op_call([[maybe_unused]] uint8_t indirect,
.avm_main_mem_idx_a = read_ind_gas_offset.val,
.avm_main_mem_idx_b = addr_offset,
.avm_main_mem_idx_c = read_ind_args_offset.val,
.avm_main_mem_idx_d = args_size,
.avm_main_mem_op_a = FF(1),
.avm_main_mem_op_b = FF(1),
.avm_main_mem_op_c = FF(1),
.avm_main_mem_op_d = FF(1),
.avm_main_pc = FF(pc++),
.avm_main_r_in_tag = FF(static_cast<uint32_t>(AvmMemoryTag::U32)),
.avm_main_r_in_tag = FF(static_cast<uint32_t>(AvmMemoryTag::FF)),
.avm_main_sel_external_call = FF(1),
});
clk++;
// Read the rest on a separate line, remember that the 4th operand is indirect
auto read_ind_ret_offset =
mem_trace_builder.indirect_read_and_load_from_memory(call_ptr, clk, IndirectRegister::IND_A, ret_offset);
std::vector<uint32_t> second_row_values = {};
std::vector<uint32_t> second_row_load = {
uint32_t(read_ind_ret_offset.val), function_selector_offset, ret_size, success_offset
};
for (uint32_t j = 0; j < second_row_load.size(); j++) {
// We just read and load to set up the constraints, we dont actually use these values for now.
auto mem_read = mem_trace_builder.read_and_load_from_memory(call_ptr,
clk,
register_order[j % register_order.size()],
second_row_load[j],
AvmMemoryTag::U32,
AvmMemoryTag::U0);
second_row_values.emplace_back(mem_read.val);
}
// We just read and load to set up the constraints, we dont actually use these values for now.
auto mem_read_ret = mem_trace_builder.read_and_load_from_memory(
call_ptr, clk, IntermRegister::IA, uint32_t(read_ind_ret_offset.val), AvmMemoryTag::FF, AvmMemoryTag::U0);
main_trace.push_back(Row{
.avm_main_clk = clk,
.avm_main_ia = second_row_values[0], /* ret_offset */
.avm_main_ib = second_row_values[1], /* function_selector_offset */
.avm_main_ic = second_row_values[2], /* ret_size */
.avm_main_id = second_row_values[3], /* success_offset */
.avm_main_ia = mem_read_ret.val, /* ret_offset */
.avm_main_ind_a = ret_offset,
.avm_main_ind_op_a = FF(1),
.avm_main_internal_return_ptr = FF(internal_return_ptr),
.avm_main_mem_idx_a = read_ind_ret_offset.val,
.avm_main_mem_idx_b = function_selector_offset,
.avm_main_mem_idx_c = ret_size,
.avm_main_mem_idx_d = success_offset,
.avm_main_mem_op_a = FF(1),
.avm_main_mem_op_b = FF(1),
.avm_main_mem_op_c = FF(1),
.avm_main_mem_op_d = FF(1),
.avm_main_pc = FF(pc),
.avm_main_r_in_tag = FF(static_cast<uint32_t>(AvmMemoryTag::FF)),
});
clk++;
auto mem_read_success = mem_trace_builder.read_and_load_from_memory(
call_ptr, clk, IntermRegister::IA, success_offset, AvmMemoryTag::U32, AvmMemoryTag::U0);
main_trace.push_back(Row{
.avm_main_clk = clk,
.avm_main_ia = mem_read_success.val, /* success_offset */
.avm_main_internal_return_ptr = FF(internal_return_ptr),
.avm_main_mem_idx_a = FF(success_offset),
.avm_main_mem_op_a = FF(1),
.avm_main_pc = FF(pc),
.avm_main_r_in_tag = FF(static_cast<uint32_t>(AvmMemoryTag::U32)),
});
Expand Down Expand Up @@ -3867,7 +3857,7 @@ std::vector<Row> AvmTraceBuilder::finalize(uint32_t min_trace_size, bool range_c
dest.avm_alu_op_eq_diff_inv = FF(src.alu_op_eq_diff_inv);

// Not all rows in ALU are enabled with a selector. For instance,
// multiplication over u128 is taking two lines.
// multiplication over u128 and cast is taking two lines.
if (AvmAluTraceBuilder::is_alu_row_enabled(src)) {
dest.avm_alu_alu_sel = FF(1);
}
Expand Down Expand Up @@ -3917,6 +3907,7 @@ std::vector<Row> AvmTraceBuilder::finalize(uint32_t min_trace_size, bool range_c
dest.avm_alu_a_hi = FF(src.hi_lo_limbs.at(1));
dest.avm_alu_p_sub_a_lo = FF(src.hi_lo_limbs.at(2));
dest.avm_alu_p_sub_a_hi = FF(src.hi_lo_limbs.at(3));
dest.avm_alu_p_a_borrow = FF(static_cast<uint8_t>(src.p_a_borrow));
dest.avm_alu_rng_chk_lookup_selector = FF(1);
}

Expand Down
8 changes: 8 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/tests/avm_cast.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,14 @@ TEST_F(AvmCastTests, sameTagU128)
uint256_t::from_uint128(a), FF(uint256_t::from_uint128(a)), 0, 1, AvmMemoryTag::U128, AvmMemoryTag::U128);
}

TEST_F(AvmCastTests, U128toFFWithBorrow)
{
uint128_t const a = (uint128_t{ 0x30644E72E131A029LLU } << 64) + uint128_t{ 0xB85045B68181585DLLU };
gen_trace(a, 0, 1, AvmMemoryTag::U128, AvmMemoryTag::FF);
validate_cast_trace(
uint256_t::from_uint128(a), FF(uint256_t::from_uint128(a)), 0, 1, AvmMemoryTag::U128, AvmMemoryTag::FF);
}

TEST_F(AvmCastTests, noTruncationFFToU32)
{
gen_trace(UINT32_MAX, 4, 9, AvmMemoryTag::FF, AvmMemoryTag::U32);
Expand Down
134 changes: 67 additions & 67 deletions barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2135,73 +2135,73 @@ TEST_F(AvmExecutionTests, kernelOutputHashExistsOpcodes)
validate_trace(std::move(trace));
}

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));
}
// 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));
// }

TEST_F(AvmExecutionTests, opGetContractInstanceOpcodes)
{
Expand Down
Loading

0 comments on commit b150b61

Please sign in to comment.