From 76863becee18da5b81e3de5727a37d646445ffd6 Mon Sep 17 00:00:00 2001 From: fcarreiro Date: Wed, 18 Sep 2024 14:46:42 +0000 Subject: [PATCH] done --- .../vm/avm/generated/verifier.cpp | 21 ++++++++++++++++--- .../vm/avm/tests/arithmetic.test.cpp | 4 ++-- .../vm/avm/tests/bitwise.test.cpp | 2 +- .../barretenberg/vm/avm/tests/cast.test.cpp | 15 +++---------- .../vm/avm/tests/control_flow.test.cpp | 2 +- .../vm/avm/tests/execution.test.cpp | 4 ++-- .../vm/avm/tests/indirect_mem.test.cpp | 2 +- .../vm/avm/tests/mem_opcodes.test.cpp | 2 +- .../barretenberg/vm/avm/tests/memory.test.cpp | 2 +- .../barretenberg/vm/avm/tests/slice.test.cpp | 15 ++++--------- .../bb-pil-backend/templates/verifier.cpp.hbs | 17 +++++++++++---- 11 files changed, 47 insertions(+), 39 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/generated/verifier.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/generated/verifier.cpp index bd132af86f62..979985cc00f7 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/generated/verifier.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/generated/verifier.cpp @@ -1,6 +1,7 @@ // AUTOGENERATED FILE #include "barretenberg/vm/avm/generated/verifier.hpp" +#include "barretenberg/common/log.hpp" #include "barretenberg/vm/constants.hpp" #include "barretenberg/commitment_schemes/zeromorph/zeromorph.hpp" @@ -67,6 +68,7 @@ bool AvmVerifier::verify_proof(const HonkProof& proof, const auto circuit_size = transcript->template receive_from_prover("circuit_size"); if (circuit_size != key->circuit_size) { + vinfo("Circuit size mismatch: expected", key->circuit_size, " got ", circuit_size); return false; } @@ -99,7 +101,8 @@ bool AvmVerifier::verify_proof(const HonkProof& proof, sumcheck.verify(relation_parameters, alpha, gate_challenges); // If Sumcheck did not verify, return false - if (sumcheck_verified.has_value() && !sumcheck_verified.value()) { + if (!sumcheck_verified.has_value() || !sumcheck_verified.value()) { + vinfo("Sumcheck failed"); return false; } @@ -109,28 +112,34 @@ bool AvmVerifier::verify_proof(const HonkProof& proof, FF main_kernel_inputs_evaluation = evaluate_public_input_column(public_inputs[0], circuit_size, mle_challenge); if (main_kernel_inputs_evaluation != claimed_evaluations.main_kernel_inputs) { + vinfo("main_kernel_inputs_evaluation failed"); return false; } FF main_kernel_value_out_evaluation = evaluate_public_input_column(public_inputs[1], circuit_size, mle_challenge); if (main_kernel_value_out_evaluation != claimed_evaluations.main_kernel_value_out) { + vinfo("main_kernel_value_out_evaluation failed"); return false; } FF main_kernel_side_effect_out_evaluation = evaluate_public_input_column(public_inputs[2], circuit_size, mle_challenge); if (main_kernel_side_effect_out_evaluation != claimed_evaluations.main_kernel_side_effect_out) { + vinfo("main_kernel_side_effect_out_evaluation failed"); return false; } FF main_kernel_metadata_out_evaluation = evaluate_public_input_column(public_inputs[3], circuit_size, mle_challenge); if (main_kernel_metadata_out_evaluation != claimed_evaluations.main_kernel_metadata_out) { + vinfo("main_kernel_metadata_out_evaluation failed"); return false; } FF main_calldata_evaluation = evaluate_public_input_column(public_inputs[4], circuit_size, mle_challenge); if (main_calldata_evaluation != claimed_evaluations.main_calldata) { + vinfo("main_calldata_evaluation failed"); return false; } FF main_returndata_evaluation = evaluate_public_input_column(public_inputs[5], circuit_size, mle_challenge); if (main_returndata_evaluation != claimed_evaluations.main_returndata) { + vinfo("main_returndata_evaluation failed"); return false; } @@ -147,8 +156,14 @@ bool AvmVerifier::verify_proof(const HonkProof& proof, transcript); auto pairing_points = PCS::reduce_verify(opening_claim, transcript); - auto verified = key->pcs_verification_key->pairing_check(pairing_points[0], pairing_points[1]); - return sumcheck_verified.value() && verified; + bool zeromoprh_verified = key->pcs_verification_key->pairing_check(pairing_points[0], pairing_points[1]); + + if (!zeromoprh_verified) { + vinfo("ZeroMorph failed"); + return false; + } + + return true; } } // namespace bb diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/arithmetic.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/arithmetic.test.cpp index ea2042526479..e5338cb206da 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/arithmetic.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/arithmetic.test.cpp @@ -407,7 +407,7 @@ TEST_F(AvmArithmeticTestsFF, addition) std::vector const returndata = { 37, 4, 11, 0, 41 }; - validate_trace(std::move(trace), public_inputs, calldata, returndata, true); + validate_trace(std::move(trace), public_inputs, calldata, returndata); } // Test on basic subtraction over finite field type. @@ -615,7 +615,7 @@ TEST_F(AvmArithmeticTestsFF, mixedOperationsWithError) trace_builder.op_return(0, 0, 0); auto trace = trace_builder.finalize(); - validate_trace(std::move(trace), public_inputs, calldata, {}, true); + validate_trace(std::move(trace), public_inputs, calldata, {}); } // Test of equality on FF elements diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/bitwise.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/bitwise.test.cpp index 921225665ea0..dff1ed51258e 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/bitwise.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/bitwise.test.cpp @@ -489,7 +489,7 @@ TEST_P(AvmBitwiseTestsAnd, AllAndTest) auto trace = trace_builder.finalize(); common_validate_bit_op(trace, 0, a, b, output, FF(0), FF(1), FF(2), mem_tag); - validate_trace(std::move(trace), public_inputs, {}, { output }, true); + validate_trace(std::move(trace), public_inputs, {}, { output }); } INSTANTIATE_TEST_SUITE_P(AvmBitwiseTests, AvmBitwiseTestsAnd, diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp index b9985009ba36..728a6be7e433 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/cast.test.cpp @@ -66,10 +66,7 @@ class AvmCastTests : public ::testing::Test { uint32_t src_address, uint32_t dst_address, AvmMemoryTag src_tag, - AvmMemoryTag dst_tag, - bool force_proof = false - - ) + AvmMemoryTag dst_tag) { auto const& row = trace.at(main_row_idx); EXPECT_THAT(row, @@ -106,13 +103,7 @@ class AvmCastTests : public ::testing::Test { ALU_ROW_FIELD_EQ(in_tag, static_cast(dst_tag)), ALU_ROW_FIELD_EQ(sel_alu, 1))); - // 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), public_inputs, calldata, {}, true); - } else { - validate_trace(std::move(trace), public_inputs, calldata); - } + validate_trace(std::move(trace), public_inputs, calldata); } }; @@ -217,7 +208,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, true); + validate_cast_trace(256'000'000'203UL, 203, 10, 11, AvmMemoryTag::U64, AvmMemoryTag::U8); } TEST_F(AvmCastTests, indirectAddrWrongResolutionU64ToU8) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp index 833c7cd357eb..e7dd7d952ac1 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/control_flow.test.cpp @@ -87,7 +87,7 @@ TEST_F(AvmControlFlowTests, simpleCall) EXPECT_EQ(halt_row->main_pc, FF(CALL_PC)); EXPECT_EQ(halt_row->main_internal_return_ptr, FF(1)); } - validate_trace(std::move(trace), public_inputs, {}, {}, true); + validate_trace(std::move(trace), public_inputs, {}, {}); } TEST_F(AvmControlFlowTests, simpleJump) diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp index bb6302e86626..9cc37fa30f23 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/execution.test.cpp @@ -102,7 +102,7 @@ TEST_F(AvmExecutionTests, basicAddReturn) ElementsAre(VariantWith(0), VariantWith(0), VariantWith(0))))); auto trace = gen_trace_from_instr(instructions); - validate_trace(std::move(trace), public_inputs, {}, {}, true); + validate_trace(std::move(trace), public_inputs, {}, {}); } // Positive test for SET and SUB opcodes @@ -167,7 +167,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.main_sel_op_sub == 1; }); EXPECT_EQ(row->main_ic, 10000); // 47123 - 37123 = 10000 - validate_trace(std::move(trace), public_inputs, {}, {}, true); + validate_trace(std::move(trace), public_inputs, {}, {}); } // Positive test for multiple MUL opcodes diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/indirect_mem.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/indirect_mem.test.cpp index b9e3bb1b302f..04278192822d 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/indirect_mem.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/indirect_mem.test.cpp @@ -68,7 +68,7 @@ TEST_F(AvmIndirectMemTests, allIndirectAdd) EXPECT_EQ(row->main_sel_mem_op_b, FF(1)); EXPECT_EQ(row->main_sel_mem_op_c, FF(1)); - validate_trace(std::move(trace), public_inputs, {}, {}, true); + validate_trace(std::move(trace), public_inputs, {}, {}); } // Testing a subtraction operation with direct input operands a, b, and an indirect diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/mem_opcodes.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/mem_opcodes.test.cpp index 84eed13b773c..9974c09e0bae 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/mem_opcodes.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/mem_opcodes.test.cpp @@ -403,7 +403,7 @@ TEST_F(AvmMemOpcodeTests, indirectMovInvalidAddressTag) MEM_ROW_FIELD_EQ(r_in_tag, static_cast(AvmMemoryTag::U32)), MEM_ROW_FIELD_EQ(sel_resolve_ind_addr_c, 1))); - validate_trace(std::move(trace), public_inputs, {}, {}, true); + validate_trace(std::move(trace), public_inputs, {}, {}); } /****************************************************************************** diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/memory.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/memory.test.cpp index 4a48913c483c..ae72d396771d 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/memory.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/memory.test.cpp @@ -78,7 +78,7 @@ TEST_F(AvmMemoryTests, mismatchedTagAddOperation) EXPECT_EQ(row->mem_r_in_tag, FF(static_cast(AvmMemoryTag::U8))); EXPECT_EQ(row->mem_tag, FF(static_cast(AvmMemoryTag::FF))); - validate_trace(std::move(trace), public_inputs, calldata, {}, true); + validate_trace(std::move(trace), public_inputs, calldata, {}); } // Testing an equality operation with a mismatched memory tag. diff --git a/barretenberg/cpp/src/barretenberg/vm/avm/tests/slice.test.cpp b/barretenberg/cpp/src/barretenberg/vm/avm/tests/slice.test.cpp index 85e85f80c31b..1d6bb2b63ed0 100644 --- a/barretenberg/cpp/src/barretenberg/vm/avm/tests/slice.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/avm/tests/slice.test.cpp @@ -47,10 +47,7 @@ class AvmSliceTests : public ::testing::Test { trace = trace_builder.finalize(); } - void validate_single_calldata_copy_trace(uint32_t col_offset, - uint32_t copy_size, - uint32_t dst_offset, - bool proof_verif = false) + void validate_single_calldata_copy_trace(uint32_t col_offset, uint32_t copy_size, uint32_t dst_offset) { // Find the first row enabling the calldata_copy selector auto row = std::ranges::find_if( @@ -111,11 +108,7 @@ class AvmSliceTests : public ::testing::Test { SLICE_ROW_FIELD_EQ(sel_cd_cpy, 0), SLICE_ROW_FIELD_EQ(sel_start, 0))); - if (proof_verif) { - validate_trace(std::move(trace), public_inputs, calldata, {}, true); - } else { - validate_trace(std::move(trace), public_inputs, calldata); - } + validate_trace(std::move(trace), public_inputs, calldata); } VmPublicInputs public_inputs; @@ -131,7 +124,7 @@ class AvmSliceTests : public ::testing::Test { TEST_F(AvmSliceTests, simpleCopyAllCDValues) { gen_single_calldata_copy(false, 12, 0, 12, 25); - validate_single_calldata_copy_trace(0, 12, 25, true); + validate_single_calldata_copy_trace(0, 12, 25); } TEST_F(AvmSliceTests, singleCopyCDElement) @@ -329,7 +322,7 @@ TEST_F(AvmSliceNegativeTests, wrongCDValueInCalldataVerifier) trace_builder.op_return(0, 0, 0); trace = trace_builder.finalize(); - validate_trace(std::move(trace), public_inputs, { 2, 3, 4, 5, 7 }, {}, true, true); + validate_trace(std::move(trace), public_inputs, { 2, 3, 4, 5, 7 }, {}, false, true); } TEST_F(AvmSliceNegativeTests, disableMemWriteEntry) diff --git a/bb-pilcom/bb-pil-backend/templates/verifier.cpp.hbs b/bb-pilcom/bb-pil-backend/templates/verifier.cpp.hbs index dbb2ad78685e..208cc1c3bced 100644 --- a/bb-pilcom/bb-pil-backend/templates/verifier.cpp.hbs +++ b/bb-pilcom/bb-pil-backend/templates/verifier.cpp.hbs @@ -1,6 +1,7 @@ // AUTOGENERATED FILE #include "barretenberg/vm/{{snakeCase name}}/generated/verifier.hpp" +#include "barretenberg/common/log.hpp" #include "barretenberg/vm/constants.hpp" #include "barretenberg/commitment_schemes/zeromorph/zeromorph.hpp" @@ -66,6 +67,7 @@ bool {{name}}Verifier::verify_proof(const HonkProof& proof, [[maybe_unused]] con const auto circuit_size = transcript->template receive_from_prover("circuit_size"); if (circuit_size != key->circuit_size) { + vinfo("Circuit size mismatch: expected", key->circuit_size, " got ", circuit_size); return false; } @@ -98,7 +100,8 @@ bool {{name}}Verifier::verify_proof(const HonkProof& proof, [[maybe_unused]] con sumcheck.verify(relation_parameters, alpha, gate_challenges); // If Sumcheck did not verify, return false - if (sumcheck_verified.has_value() && !sumcheck_verified.value()) { + if (!sumcheck_verified.has_value() || !sumcheck_verified.value()) { + vinfo("Sumcheck failed"); return false; } @@ -109,13 +112,13 @@ bool {{name}}Verifier::verify_proof(const HonkProof& proof, [[maybe_unused]] con {{#each public_cols}} FF {{col}}_evaluation = evaluate_public_input_column(public_inputs[{{idx}}], circuit_size, mle_challenge); if ({{col}}_evaluation != claimed_evaluations.{{col}}) { + vinfo("{{col}}_evaluation failed"); return false; } {{/each}} // Execute ZeroMorph rounds. See https://hackmd.io/dlf9xEwhTQyE3hiGbq4FsA?view for a complete description of the // unrolled protocol. - auto opening_claim = ZeroMorph::verify(circuit_size, commitments.get_unshifted(), commitments.get_to_be_shifted(), @@ -126,8 +129,14 @@ bool {{name}}Verifier::verify_proof(const HonkProof& proof, [[maybe_unused]] con transcript); auto pairing_points = PCS::reduce_verify(opening_claim, transcript); - auto verified = key->pcs_verification_key->pairing_check(pairing_points[0], pairing_points[1]); - return sumcheck_verified.value() && verified; + bool zeromoprh_verified = key->pcs_verification_key->pairing_check(pairing_points[0], pairing_points[1]); + + if (!zeromoprh_verified) { + vinfo("ZeroMorph failed"); + return false; + } + + return true; } } // namespace bb