From 36fcba2d318c13165df794813c0b14ad021c4baa Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 26 Apr 2024 12:53:09 +0000 Subject: [PATCH 1/2] 6019: re-enable proof in execution tests and some code cleaning --- barretenberg/cpp/src/barretenberg/bb/main.cpp | 19 ++++---- .../vm/avm_trace/avm_execution.cpp | 19 +------- .../vm/avm_trace/avm_execution.hpp | 2 - .../barretenberg/vm/tests/avm_common.test.hpp | 1 + .../vm/tests/avm_execution.test.cpp | 46 ++++--------------- 5 files changed, 20 insertions(+), 67 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/bb/main.cpp b/barretenberg/cpp/src/barretenberg/bb/main.cpp index 83c019a3401..68ebe2bfc67 100644 --- a/barretenberg/cpp/src/barretenberg/bb/main.cpp +++ b/barretenberg/cpp/src/barretenberg/bb/main.cpp @@ -556,17 +556,14 @@ void avm_prove(const std::filesystem::path& bytecode_path, bool avm_verify(const std::filesystem::path& proof_path) { std::filesystem::path vk_path = proof_path.parent_path() / "vk"; - - // Actual verification temporarily stopped (#4954) - // std::vector const proof = many_from_buffer(read_file(proof_path)); - // - // std::vector vk_bytes = read_file(vk_path); - // auto circuit_size = from_buffer(vk_bytes, 0); - // auto _num_public_inputs = from_buffer(vk_bytes, sizeof(size_t)); - // auto vk = AvmFlavor::VerificationKey(circuit_size, num_public_inputs); - // - // std::cout << avm_trace::Execution::verify(vk, proof); - // return avm_trace::Execution::verify(vk, proof); + std::vector const proof = many_from_buffer(read_file(proof_path)); + std::vector vk_bytes = read_file(vk_path); + auto circuit_size = from_buffer(vk_bytes, 0); + auto num_public_inputs = from_buffer(vk_bytes, sizeof(size_t)); + auto vk = AvmFlavor::VerificationKey(circuit_size, num_public_inputs); + + std::cout << avm_trace::Execution::verify(vk, proof); + return avm_trace::Execution::verify(vk, proof); std::cout << 1; return true; diff --git a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp index 746003beb50..7cf6154fac8 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_execution.cpp @@ -27,22 +27,8 @@ namespace bb::avm_trace { * @param bytecode A vector of bytes representing the bytecode to execute. * @param calldata expressed as a vector of finite field elements. * @throws runtime_error exception when the bytecode is invalid. - * @return A zk proof of the execution. + * @return The verifier key and zk proof of the execution. */ -HonkProof Execution::run_and_prove(std::vector const& bytecode, std::vector const& calldata) -{ - auto instructions = Deserialization::parse(bytecode); - auto trace = gen_trace(instructions, calldata); - auto circuit_builder = bb::AvmCircuitBuilder(); - circuit_builder.set_trace(std::move(trace)); - - auto composer = AvmComposer(); - auto prover = composer.create_prover(circuit_builder); - auto verifier = composer.create_verifier(circuit_builder); - auto proof = prover.construct_proof(); - return proof; -} - std::tuple Execution::prove(std::vector const& bytecode, std::vector const& calldata) { @@ -51,9 +37,6 @@ std::tuple Execution::prove(std::vector gen_trace(std::vector const& instructions, std::vector const& calldata = {}); - static bb::HonkProof run_and_prove(std::vector const& bytecode, std::vector const& calldata = {}); - static std::tuple prove(std::vector const& bytecode, std::vector const& calldata = {}); static bool verify(AvmFlavor::VerificationKey vk, HonkProof const& proof); diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_common.test.hpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_common.test.hpp index 710082b8f4b..57ba229942e 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_common.test.hpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_common.test.hpp @@ -8,6 +8,7 @@ #include #include +#include #include #include #include \ No newline at end of file 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 f7aec7b173a..595f4326ff7 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp @@ -4,11 +4,6 @@ #include "barretenberg/vm/avm_trace/avm_common.hpp" #include "barretenberg/vm/avm_trace/avm_deserialization.hpp" #include "barretenberg/vm/avm_trace/avm_opcode.hpp" -#include -#include -#include -#include -#include namespace tests_avm { @@ -17,26 +12,6 @@ using namespace testing; using bb::utils::hex_to_bytes; -namespace { - -void gen_proof_and_validate(std::vector const& bytecode, - std::vector&& trace, - std::vector const& calldata) -{ - auto circuit_builder = AvmCircuitBuilder(); - circuit_builder.set_trace(std::move(trace)); - EXPECT_TRUE(circuit_builder.check_circuit()); - - auto composer = AvmComposer(); - auto verifier = composer.create_verifier(circuit_builder); - - auto proof = avm_trace::Execution::run_and_prove(bytecode, calldata); - - // TODO(#4944): uncomment the following line to revive full verification - // EXPECT_TRUE(verifier.verify_proof(proof)); -} -} // namespace - class AvmExecutionTests : public ::testing::Test { public: AvmTraceBuilder trace_builder; @@ -84,7 +59,7 @@ TEST_F(AvmExecutionTests, basicAddReturn) ElementsAre(VariantWith(0), VariantWith(0), VariantWith(0))))); auto trace = Execution::gen_trace(instructions); - gen_proof_and_validate(bytecode, std::move(trace), {}); + validate_trace(std::move(trace), true); } // Positive test for SET and SUB opcodes @@ -149,8 +124,7 @@ TEST_F(AvmExecutionTests, setAndSubOpcodes) // Find the first row enabling the subtraction selector auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avm_main_sel_op_sub == 1; }); EXPECT_EQ(row->avm_main_ic, 10000); // 47123 - 37123 = 10000 - - gen_proof_and_validate(bytecode, std::move(trace), {}); + validate_trace(std::move(trace), true); } // Positive test for multiple MUL opcodes @@ -230,7 +204,7 @@ TEST_F(AvmExecutionTests, powerWithMulOpcodes) trace.begin(), trace.end(), [](Row r) { return r.avm_main_sel_op_mul == 1 && r.avm_main_pc == 13; }); EXPECT_EQ(row->avm_main_ic, 244140625); // 5^12 = 244140625 - gen_proof_and_validate(bytecode, std::move(trace), {}); + validate_trace(std::move(trace), true); } // Positive test about a single internal_call and internal_return @@ -297,7 +271,7 @@ TEST_F(AvmExecutionTests, simpleInternalCall) auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avm_main_sel_op_add == 1; }); EXPECT_EQ(row->avm_main_ic, 345567789); - gen_proof_and_validate(bytecode, std::move(trace), {}); + validate_trace(std::move(trace), true); } // Positive test with some nested internall calls @@ -377,7 +351,7 @@ TEST_F(AvmExecutionTests, nestedInternalCalls) EXPECT_EQ(row->avm_main_ic, 187); EXPECT_EQ(row->avm_main_pc, 4); - gen_proof_and_validate(bytecode, std::move(trace), {}); + validate_trace(std::move(trace), true); } // Positive test with JUMP and CALLDATACOPY @@ -451,7 +425,7 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy) // 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 }); + validate_trace(std::move(trace), true); } // Positive test with MOV. @@ -499,7 +473,7 @@ TEST_F(AvmExecutionTests, movOpcode) EXPECT_EQ(row->avm_main_ia, 19); EXPECT_EQ(row->avm_main_ic, 19); - gen_proof_and_validate(bytecode, std::move(trace), {}); + validate_trace(std::move(trace), true); } // Positive test with CMOV. @@ -555,7 +529,7 @@ TEST_F(AvmExecutionTests, cmovOpcode) EXPECT_EQ(row->avm_main_ic, 3); EXPECT_EQ(row->avm_main_id, 5); - gen_proof_and_validate(bytecode, std::move(trace), {}); + validate_trace(std::move(trace), true); } // Positive test with indirect MOV. @@ -603,7 +577,7 @@ TEST_F(AvmExecutionTests, indMovOpcode) EXPECT_EQ(row->avm_main_ia, 255); EXPECT_EQ(row->avm_main_ic, 255); - gen_proof_and_validate(bytecode, std::move(trace), {}); + validate_trace(std::move(trace), true); } // Positive test for SET and CAST opcodes @@ -644,7 +618,7 @@ TEST_F(AvmExecutionTests, setAndCastOpcodes) auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avm_main_sel_op_cast == 1; }); EXPECT_EQ(row->avm_main_ic, 19); // 0XB813 --> 0X13 = 19 - gen_proof_and_validate(bytecode, std::move(trace), {}); + validate_trace(std::move(trace), true); } // Negative test detecting an invalid opcode byte. From 51b68b6f125e815c900b8156a4eb8daf251e33b3 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 26 Apr 2024 13:33:38 +0000 Subject: [PATCH 2/2] 6019: Enable proving for a couple of AVM unit tests --- .../vm/tests/avm_arithmetic.test.cpp | 2 +- .../barretenberg/vm/tests/avm_bitwise.test.cpp | 2 +- .../src/barretenberg/vm/tests/avm_cast.test.cpp | 13 ++++++++++--- .../vm/tests/avm_control_flow.test.cpp | 2 +- .../barretenberg/vm/tests/avm_execution.test.cpp | 16 ++++++++-------- .../vm/tests/avm_indirect_mem.test.cpp | 2 +- .../vm/tests/avm_mem_opcodes.test.cpp | 2 +- .../barretenberg/vm/tests/avm_memory.test.cpp | 2 +- 8 files changed, 24 insertions(+), 17 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_arithmetic.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_arithmetic.test.cpp index 09791115ca4..30601dd613e 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_arithmetic.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_arithmetic.test.cpp @@ -511,7 +511,7 @@ TEST_F(AvmArithmeticTestsFF, mixedOperationsWithError) trace_builder.halt(); auto trace = trace_builder.finalize(); - validate_trace(std::move(trace)); + validate_trace(std::move(trace), true); } // Test of equality on FF elements diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_bitwise.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_bitwise.test.cpp index 71b8babbfd5..256501f41ea 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_bitwise.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_bitwise.test.cpp @@ -500,7 +500,7 @@ TEST_P(AvmBitwiseTestsAnd, AllAndTest) FF ff_b = FF(uint256_t::from_uint128(b)); FF ff_output = FF(uint256_t::from_uint128(output)); common_validate_bit_op(trace, 0, ff_a, ff_b, ff_output, FF(0), FF(1), FF(2), mem_tag); - validate_trace(std::move(trace)); + validate_trace(std::move(trace), true); } INSTANTIATE_TEST_SUITE_P(AvmBitwiseTests, AvmBitwiseTestsAnd, diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_cast.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_cast.test.cpp index 8991c8d6bbe..634370471c2 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_cast.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_cast.test.cpp @@ -55,7 +55,8 @@ class AvmCastTests : public ::testing::Test { uint32_t src_address, uint32_t dst_address, AvmMemoryTag src_tag, - AvmMemoryTag dst_tag + AvmMemoryTag dst_tag, + bool force_proof = true ) { @@ -102,7 +103,13 @@ class AvmCastTests : public ::testing::Test { alu_row_next, AllOf(Field("op_cast", &Row::avm_alu_op_cast, 0), Field("op_cast_prev", &Row::avm_alu_op_cast_prev, 1))); - validate_trace(std::move(trace)); + // We still want the ability to enable proving through the environment variable and therefore we do not pass + // the boolean variable force_proof to validate_trace second argument. + if (force_proof) { + validate_trace(std::move(trace), true); + } else { + validate_trace(std::move(trace)); + } } }; @@ -190,7 +197,7 @@ TEST_F(AvmCastTests, indirectAddrTruncationU64ToU8) trace = trace_builder.finalize(); gen_indices(); - validate_cast_trace(256'000'000'203UL, 203, 10, 11, AvmMemoryTag::U64, AvmMemoryTag::U8); + validate_cast_trace(256'000'000'203UL, 203, 10, 11, AvmMemoryTag::U64, AvmMemoryTag::U8, true); } TEST_F(AvmCastTests, indirectAddrWrongResolutionU64ToU8) diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_control_flow.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_control_flow.test.cpp index 5e7631bebfc..b944655160e 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_control_flow.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_control_flow.test.cpp @@ -54,7 +54,7 @@ TEST_F(AvmControlFlowTests, simpleCall) EXPECT_EQ(halt_row->avm_main_pc, FF(CALL_ADDRESS)); EXPECT_EQ(halt_row->avm_main_internal_return_ptr, FF(AvmTraceBuilder::CALLSTACK_OFFSET + 1)); } - validate_trace(std::move(trace)); + validate_trace(std::move(trace), true); } TEST_F(AvmControlFlowTests, simpleJump) 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 595f4326ff7..118a802e365 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_execution.test.cpp @@ -204,7 +204,7 @@ TEST_F(AvmExecutionTests, powerWithMulOpcodes) trace.begin(), trace.end(), [](Row r) { return r.avm_main_sel_op_mul == 1 && r.avm_main_pc == 13; }); EXPECT_EQ(row->avm_main_ic, 244140625); // 5^12 = 244140625 - validate_trace(std::move(trace), true); + validate_trace(std::move(trace)); } // Positive test about a single internal_call and internal_return @@ -271,7 +271,7 @@ TEST_F(AvmExecutionTests, simpleInternalCall) auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avm_main_sel_op_add == 1; }); EXPECT_EQ(row->avm_main_ic, 345567789); - validate_trace(std::move(trace), true); + validate_trace(std::move(trace)); } // Positive test with some nested internall calls @@ -351,7 +351,7 @@ TEST_F(AvmExecutionTests, nestedInternalCalls) EXPECT_EQ(row->avm_main_ic, 187); EXPECT_EQ(row->avm_main_pc, 4); - validate_trace(std::move(trace), true); + validate_trace(std::move(trace)); } // Positive test with JUMP and CALLDATACOPY @@ -425,7 +425,7 @@ TEST_F(AvmExecutionTests, jumpAndCalldatacopy) // It must have failed as subtraction was "jumped over". EXPECT_EQ(row, trace.end()); - validate_trace(std::move(trace), true); + validate_trace(std::move(trace)); } // Positive test with MOV. @@ -473,7 +473,7 @@ TEST_F(AvmExecutionTests, movOpcode) EXPECT_EQ(row->avm_main_ia, 19); EXPECT_EQ(row->avm_main_ic, 19); - validate_trace(std::move(trace), true); + validate_trace(std::move(trace)); } // Positive test with CMOV. @@ -529,7 +529,7 @@ TEST_F(AvmExecutionTests, cmovOpcode) EXPECT_EQ(row->avm_main_ic, 3); EXPECT_EQ(row->avm_main_id, 5); - validate_trace(std::move(trace), true); + validate_trace(std::move(trace)); } // Positive test with indirect MOV. @@ -577,7 +577,7 @@ TEST_F(AvmExecutionTests, indMovOpcode) EXPECT_EQ(row->avm_main_ia, 255); EXPECT_EQ(row->avm_main_ic, 255); - validate_trace(std::move(trace), true); + validate_trace(std::move(trace)); } // Positive test for SET and CAST opcodes @@ -618,7 +618,7 @@ TEST_F(AvmExecutionTests, setAndCastOpcodes) auto row = std::ranges::find_if(trace.begin(), trace.end(), [](Row r) { return r.avm_main_sel_op_cast == 1; }); EXPECT_EQ(row->avm_main_ic, 19); // 0XB813 --> 0X13 = 19 - validate_trace(std::move(trace), true); + validate_trace(std::move(trace)); } // Negative test detecting an invalid opcode byte. diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_indirect_mem.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_indirect_mem.test.cpp index 9d2696a6dd3..4c09d701d3c 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_indirect_mem.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_indirect_mem.test.cpp @@ -63,7 +63,7 @@ TEST_F(AvmIndirectMemTests, allIndirectAdd) EXPECT_EQ(row->avm_main_mem_op_b, FF(1)); EXPECT_EQ(row->avm_main_mem_op_c, FF(1)); - validate_trace(std::move(trace)); + validate_trace(std::move(trace), true); } // Testing a subtraction operation with direct input operands a, b, and an indirect diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_mem_opcodes.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_mem_opcodes.test.cpp index 59e0ac2497a..6d5f3544922 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_mem_opcodes.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_mem_opcodes.test.cpp @@ -391,7 +391,7 @@ TEST_F(AvmMemOpcodeTests, indirectMovInvalidAddressTag) Field(&Row::avm_mem_r_in_tag, static_cast(AvmMemoryTag::U32)), Field(&Row::avm_mem_ind_op_c, 1))); - validate_trace(std::move(trace)); + validate_trace(std::move(trace), true); } /****************************************************************************** diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/avm_memory.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/avm_memory.test.cpp index e2f95c50d07..aed1f0ac079 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/avm_memory.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/avm_memory.test.cpp @@ -69,7 +69,7 @@ TEST_F(AvmMemoryTests, mismatchedTagAddOperation) EXPECT_EQ(row->avm_mem_r_in_tag, FF(static_cast(AvmMemoryTag::U8))); EXPECT_EQ(row->avm_mem_tag, FF(static_cast(AvmMemoryTag::FF))); - validate_trace(std::move(trace)); + validate_trace(std::move(trace), true); } // Testing an equality operation with a mismatched memory tag.