From fb1d7420860a35e68b987e790abdaba18595219b Mon Sep 17 00:00:00 2001 From: Jean M <132435771+jeanmon@users.noreply.github.com> Date: Mon, 29 Jan 2024 18:23:07 +0100 Subject: [PATCH] feat(avm): bytecode avm control flow (#4253) Resolves #4209 --- .../vm/avm_trace/AvmMini_execution.cpp | 10 +- .../vm/avm_trace/AvmMini_trace.cpp | 3 + .../vm/avm_trace/AvmMini_trace.hpp | 2 + .../vm/tests/AvmMini_execution.test.cpp | 259 ++++++++++++++++-- 4 files changed, 244 insertions(+), 30 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_execution.cpp index 1a9da79a4d9..1cbf7b5e353 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_execution.cpp @@ -161,7 +161,15 @@ std::vector Execution::gen_trace(std::vector 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); diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.cpp index 08952de4e1b..88aac4a09e6 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.cpp @@ -452,6 +452,7 @@ std::vector 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; } @@ -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. } /** diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.hpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.hpp index 0d58753111e..381f8f1586b 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm_trace/AvmMini_trace.hpp @@ -26,6 +26,8 @@ class AvmMiniTraceBuilder { std::vector 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); diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp index f74218e10f3..afe4386816d 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp @@ -10,6 +10,24 @@ #include #include +namespace { +void gen_proof_and_validate(std::vector const& bytecode, + std::vector&& trace, + std::vector const& calldata) +{ + auto circuit_builder = AvmMiniCircuitBuilder(); + circuit_builder.set_trace(std::move(trace)); + EXPECT_TRUE(circuit_builder.check_circuit()); + + 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; @@ -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{}); - 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{}); - - EXPECT_TRUE(verifier.verify_proof(proof)); + gen_proof_and_validate(bytecode, std::move(trace), std::vector{}); } // Positive test for SET and SUB opcodes @@ -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{}); - - EXPECT_TRUE(verifier.verify_proof(proof)); + gen_proof_and_validate(bytecode, std::move(trace), std::vector{}); } // Positive test for multiple MUL opcodes @@ -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{}); +} - 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{}); + 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{}); + + // Expected sequence of PCs during execution + std::vector 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{}); +} + +// 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 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{}); + + // Expected sequence of PCs during execution + std::vector 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{}); +} + +// 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{ 13, 156 }); + + // Expected sequence of PCs during execution + std::vector 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{ 13, 156 }); } // Negative test detecting an invalid opcode byte.