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

feat(avm): bytecode avm control flow #4253

Merged
merged 5 commits into from
Jan 29, 2024
Merged

feat(avm): bytecode avm control flow #4253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
051bed3
4209 - add a getter for pc in execution and adjust pc value after return
jeanmon Jan 25, 2024
17ff26b
4209 - unit test on internal_call and internal_return
jeanmon Jan 26, 2024
392670d
4209 - add unit test for calldatacopy and jump opcodes
jeanmon Jan 29, 2024
924b409
4209 - add unit test for nested calls
jeanmon Jan 29, 2024
7126294
4209 - review feedback addressed
jeanmon Jan 29, 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
10 changes: 9 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_execution.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,15 @@ std::vector<Row> Execution::gen_trace(std::vector<Instruction> const& instructio
{
AvmMiniTraceBuilder trace_builder{};

for (auto const& inst : instructions) {
// copied version of pc maintained in trace builder. The value of pc is evolving based
// on opcode logic and therefore is not maintained here. However, the next opcode in the execution
// is determined by this value which require read access to the code below.
uint32_t pc = 0;
auto const inst_size = instructions.size();

while ((pc = trace_builder.getPc()) < inst_size) {
auto inst = instructions.at(pc);

switch (inst.op_code) {
case OpCode::ADD:
trace_builder.add(inst.operands.at(0), inst.operands.at(1), inst.operands.at(2), inst.in_tag);
Expand Down
3 changes: 3 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,7 @@ std::vector<FF> AvmMiniTraceBuilder::return_op(uint32_t ret_offset, uint32_t ret
pos = ret_size;
}
}
pc = UINT32_MAX; // This ensures that no subsequent opcode will be executed.
return returnMem;
}

Expand All @@ -471,6 +472,8 @@ void AvmMiniTraceBuilder::halt()
.avmMini_internal_return_ptr = FF(internal_return_ptr),
.avmMini_sel_halt = FF(1),
});

pc = UINT32_MAX; // This ensures that no subsequent opcode will be executed.
}

/**
Expand Down
2 changes: 2 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ class AvmMiniTraceBuilder {
std::vector<Row> finalize();
void reset();

uint32_t getPc() const { return pc; }

// Addition with direct memory access.
void add(uint32_t a_offset, uint32_t b_offset, uint32_t dst_offset, AvmMemoryTag in_tag);

Expand Down
259 changes: 230 additions & 29 deletions barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,24 @@
#include <string>
#include <utility>

namespace {
void gen_proof_and_validate(std::vector<uint8_t> const& bytecode,
std::vector<Row>&& trace,
std::vector<FF> const& calldata)
{
auto circuit_builder = AvmMiniCircuitBuilder();
circuit_builder.set_trace(std::move(trace));
EXPECT_TRUE(circuit_builder.check_circuit());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a general GTest comment (just in case): EXPECT_ should only be used to check what the test case if testing, and not be used to check preconditions for the test case to run correctly. (I'm not sure which case is here).

For example, if a test is

TestDivision() {
a = getnumber();
b = getnumber()
ASSERT_NEQ(b, 0);
c = div(a,b);

EXPECT_EQ(c, a/b);
}

Then the first assert should not be an EXPECT because it's a precondition check.

Another comment (related to the above): if an EXPECT fails, the test does not stop. But if an ASSERT fails, it does.

Another comment (related): ASSERTs on subroutines/helper functions do not work as expected. Some extra work is needed: https://github.com/google/googletest/blob/main/docs/advanced.md#asserting-on-subroutines

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fcarreiro Thanks for the pointer. The check_circuit() is part of the test rather than a precondition check in this context.

In the context of deterministic tests, I am not sure it matters so much. You will want to have everything green anyway. The ASSERT will allow you to save CPU time.


auto composer = honk::AvmMiniComposer();
auto verifier = composer.create_verifier(circuit_builder);

auto proof = avm_trace::Execution::run_and_prove(bytecode, calldata);

EXPECT_TRUE(verifier.verify_proof(proof));
}
} // namespace

namespace tests_avm {
using namespace avm_trace;
using bb::utils::hex_to_bytes;
Expand Down Expand Up @@ -61,17 +79,8 @@ TEST_F(AvmMiniExecutionTests, basicAddReturn)
EXPECT_EQ(instructions.at(1).operands.at(0), 0);

auto trace = Execution::gen_trace(instructions, std::vector<FF>{});
auto trace_verif = trace;
validate_trace_proof(std::move(trace));

auto circuit_builder = AvmMiniCircuitBuilder();
circuit_builder.set_trace(std::move(trace_verif));
auto composer = honk::AvmMiniComposer();
auto verifier = composer.create_verifier(circuit_builder);

auto proof = Execution::run_and_prove(bytecode, std::vector<FF>{});

EXPECT_TRUE(verifier.verify_proof(proof));
gen_proof_and_validate(bytecode, std::move(trace), std::vector<FF>{});
}

// Positive test for SET and SUB opcodes
Expand Down Expand Up @@ -126,17 +135,7 @@ TEST_F(AvmMiniExecutionTests, setAndSubOpcodes)
auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avmMini_sel_op_sub == 1; });
EXPECT_EQ(row->avmMini_ic, 10000); // 47123 - 37123 = 10000

auto trace_verif = trace;
validate_trace_proof(std::move(trace));

auto circuit_builder = AvmMiniCircuitBuilder();
circuit_builder.set_trace(std::move(trace_verif));
auto composer = honk::AvmMiniComposer();
auto verifier = composer.create_verifier(circuit_builder);

auto proof = Execution::run_and_prove(bytecode, std::vector<FF>{});

EXPECT_TRUE(verifier.verify_proof(proof));
gen_proof_and_validate(bytecode, std::move(trace), std::vector<FF>{});
}

// Positive test for multiple MUL opcodes
Expand Down Expand Up @@ -209,17 +208,219 @@ TEST_F(AvmMiniExecutionTests, powerWithMulOpcodes)
trace.begin(), trace.end(), [](Row r) { return r.avmMini_sel_op_mul == 1 && r.avmMini_pc == 13; });
EXPECT_EQ(row->avmMini_ic, 244140625); // 5^12 = 244140625

auto trace_verif = trace;
validate_trace_proof(std::move(trace));
gen_proof_and_validate(bytecode, std::move(trace), std::vector<FF>{});
}

auto circuit_builder = AvmMiniCircuitBuilder();
circuit_builder.set_trace(std::move(trace_verif));
auto composer = honk::AvmMiniComposer();
auto verifier = composer.create_verifier(circuit_builder);
// Positive test about a single internal_call and internal_return
// Code of internal routine is SET U32 value 123456789 at memory address 7
// The bytecode execution is:
// SET U32 val. 222111000 at memory address 4
// CALL internal routine
// ADD M[4] with M[7] and output in M[9]
// Internal routine bytecode is at the end.
// Bytecode layout: SET INTERNAL_CALL ADD RETURN SET INTERNAL_RETURN
// 0 1 2 3 4 5
TEST_F(AvmMiniExecutionTests, simpleInternalCall)
{
std::string bytecode_hex = "27" // SET 39 = 0x27
"03" // U32
"0D3D2518" // val 222111000 = 0xD3D2518
"00000004" // dst_offset 4
"25" // INTERNALCALL 37
"00000004" // jmp_dest
"00" // ADD
"03" // U32
"00000004" // addr a 4
"00000007" // addr b 7
"00000009" // addr c9
"34" // RETURN
"00000000" // ret offset 0
"00000000" // ret size 0
"27" // SET 39 = 0x27
"03" // U32
"075BCD15" // val 123456789 = 0x75BCD15
"00000007" // dst_offset 7
"26" // INTERNALRETURN 38
;

auto proof = Execution::run_and_prove(bytecode, std::vector<FF>{});
auto bytecode = hex_to_bytes(bytecode_hex);
auto instructions = Execution::parse(bytecode);

EXPECT_TRUE(verifier.verify_proof(proof));
EXPECT_EQ(instructions.size(), 6);

// We test parsing step for INTERNALCALL and INTERNALRETURN.

// INTERNALCALL
EXPECT_EQ(instructions.at(1).op_code, OpCode::INTERNALCALL);
EXPECT_EQ(instructions.at(1).operands.size(), 1);
EXPECT_EQ(instructions.at(1).operands.at(0), 4);

// INTERNALRETURN
EXPECT_EQ(instructions.at(5).op_code, OpCode::INTERNALRETURN);

auto trace = Execution::gen_trace(instructions, std::vector<FF>{});

// Expected sequence of PCs during execution
std::vector<FF> pc_sequence{ 0, 1, 4, 5, 2, 3 };

for (size_t i = 0; i < 6; i++) {
EXPECT_EQ(trace.at(i + 1).avmMini_pc, pc_sequence.at(i));
}

// Find the first row enabling the addition selector.
auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avmMini_sel_op_add == 1; });
EXPECT_EQ(row->avmMini_ic, 345567789);

gen_proof_and_validate(bytecode, std::move(trace), std::vector<FF>{});
}

// Positive test with some nested internall calls
// We use the following functions (internal calls):
// F1: ADD(2,3,2) M[2] = M[2] + M[3]
// F2: MUL(2,3,2) M[2] = M[2] * M[3]
// G: F1 SET(17,3) F2 where SET(17,3) means M[3] = 17
// MAIN: SET(4,2) SET(7,3) G
// Whole execution should compute: (4 + 7) * 17 = 187
// Bytecode layout: SET(4,2) SET(7,3) INTERNAL_CALL_G RETURN BYTECODE(F2) BYTECODE(F1) BYTECODE(G)
// 0 1 2 3 4 6 8
// BYTECODE(F1): ADD(2,3,2) INTERNAL_RETURN
// BYTECODE(F2): MUL(2,3,2) INTERNAL_RETURN
// BYTECODE(G): INTERNAL_CALL(6) SET(17,3) INTERNAL_CALL(4) INTERNAL_RETURN
TEST_F(AvmMiniExecutionTests, nestedInternalCalls)
{
auto internalCallHex = [](std::string const& dst_offset) {
return "25"
"000000" +
dst_offset;
};

auto setHex = [](std::string const& val, std::string const& dst_offset) {
return "2701" // SET U8
+ val + "000000" + dst_offset;
};

const std::string tag_address_arguments = "01" // U8
"00000002" // addr a 2
"00000003" // addr b 3
"00000002"; // addr c 2

const std::string return_hex = "34" // RETURN
"00000000" // ret offset 0
"00000000"; // ret size 0

const std::string internal_ret_hex = "26";
const std::string add_hex = "00";
const std::string mul_hex = "02";

const std::string bytecode_f1 = add_hex + tag_address_arguments + internal_ret_hex;
const std::string bytecode_f2 = mul_hex + tag_address_arguments + internal_ret_hex;
const std::string bytecode_g =
internalCallHex("06") + setHex("11", "03") + internalCallHex("04") + internal_ret_hex;

std::string bytecode_hex = setHex("04", "02") + setHex("07", "03") + internalCallHex("08") + return_hex +
bytecode_f2 + bytecode_f1 + bytecode_g;

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

EXPECT_EQ(instructions.size(), 12);

// Expected sequence of opcodes
std::vector<OpCode> const opcode_sequence{ OpCode::SET, OpCode::SET,
OpCode::INTERNALCALL, OpCode::RETURN,
OpCode::MUL, OpCode::INTERNALRETURN,
OpCode::ADD, OpCode::INTERNALRETURN,
OpCode::INTERNALCALL, OpCode::SET,
OpCode::INTERNALCALL, OpCode::INTERNALRETURN };

for (size_t i = 0; i < 12; i++) {
EXPECT_EQ(instructions.at(i).op_code, opcode_sequence.at(i));
}

auto trace = Execution::gen_trace(instructions, std::vector<FF>{});

// Expected sequence of PCs during execution
std::vector<FF> pc_sequence{ 0, 1, 2, 8, 6, 7, 9, 10, 4, 5, 11, 3 };

for (size_t i = 0; i < 6; i++) {
EXPECT_EQ(trace.at(i + 1).avmMini_pc, pc_sequence.at(i));
}

// Find the first row enabling the multiplication selector.
auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avmMini_sel_op_mul == 1; });
EXPECT_EQ(row->avmMini_ic, 187);
EXPECT_EQ(row->avmMini_pc, 4);

gen_proof_and_validate(bytecode, std::move(trace), std::vector<FF>{});
}

// Positive test with JUMP and CALLDATACOPY
// We test bytecode which first invoke CALLDATACOPY on a FF array of two values.
// Then, a JUMP call skips a SUB opcode to land to a DIV operation and RETURN.
// Calldata: [13, 156]
// Bytecode layout: CALLDATACOPY JUMP SUB DIV RETURN
// 0 1 2 3 4
TEST_F(AvmMiniExecutionTests, jumpAndCalldatacopy)
{
std::string bytecode_hex = "1F" // CALLDATACOPY 31 (no in_tag)
"00000000" // cd_offset
"00000002" // copy_size
"0000000A" // dst_offset // M[10] = 13, M[11] = 156
"23" // JUMP 35
"00000003" // jmp_dest (DIV located at 3)
"01" // SUB
"06" // FF
"0000000B" // addr 11
"0000000A" // addr 10
"00000001" // addr c 1 (If executed would be 156 - 13 = 143)
"03" // DIV
"06" // FF
"0000000B" // addr 11
"0000000A" // addr 10
"00000001" // addr c 1 (156 / 13 = 12)
"34" // RETURN
"00000000" // ret offset 0
"00000000" // ret size 0
;

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

EXPECT_EQ(instructions.size(), 5);

// We test parsing steps for CALLDATACOPY and JUMP.

// CALLDATACOPY
EXPECT_EQ(instructions.at(0).op_code, OpCode::CALLDATACOPY);
EXPECT_EQ(instructions.at(0).operands.size(), 3);
EXPECT_EQ(instructions.at(0).operands.at(0), 0);
EXPECT_EQ(instructions.at(0).operands.at(1), 2);
EXPECT_EQ(instructions.at(0).operands.at(2), 10);

// JUMP
EXPECT_EQ(instructions.at(1).op_code, OpCode::JUMP);
EXPECT_EQ(instructions.at(1).operands.size(), 1);
EXPECT_EQ(instructions.at(1).operands.at(0), 3);

auto trace = Execution::gen_trace(instructions, std::vector<FF>{ 13, 156 });

// Expected sequence of PCs during execution
std::vector<FF> pc_sequence{ 0, 1, 3, 4 };

for (size_t i = 0; i < 4; i++) {
EXPECT_EQ(trace.at(i + 1).avmMini_pc, pc_sequence.at(i));
}

// Find the first row enabling the division selector.
auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avmMini_sel_op_div == 1; });
EXPECT_EQ(row->avmMini_ic, 12);

// Find the first row enabling the subtraction selector.
row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avmMini_sel_op_sub == 1; });
// It must have failed as subtraction was "jumped over".
EXPECT_EQ(row, trace.end());

gen_proof_and_validate(bytecode, std::move(trace), std::vector<FF>{ 13, 156 });
}

// Negative test detecting an invalid opcode byte.
Expand Down
Loading