From 85664b433f59c83426b6725200575fb4658c9b49 Mon Sep 17 00:00:00 2001 From: iakovenkos Date: Mon, 30 Mar 2026 20:08:41 +0000 Subject: [PATCH 1/3] Non-ZK Goblin: fix ECCVM transcript builder and op queue for is_zk=false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - transcript_builder: gate hiding op handling on is_zk so index 0 goes through the normal main loop when is_zk=false (root bug: sentinel x-coordinate was written to ECCVM Px without infinity flag, causing ECCVM/Translator accumulated_result mismatch) - ecc_op_queue: add get_is_zk() getter; remove (0,0) coordinate override in eq_and_reset that destroyed the infinity flag (the transcript builder already zeros coords for infinity points) - eccvm_flavor: pass is_zk to compute_rows - goblin.hpp: add set_op_queue_zk() setter - mock_circuits: restructure for non-ZK — only last-prepended circuit gets 4 leading no-ops (translator padding), every circuit gets eq_and_reset, end random ops gated on is_zk - goblin_verifier.test: set op_queue_zk(false), add BB_DISABLE_ASSERTS - goblin.cpp: remove debug trace checker --- .../cpp/src/barretenberg/eccvm/eccvm_flavor.hpp | 5 ++--- .../src/barretenberg/eccvm/transcript_builder.hpp | 12 ++++++++---- .../cpp/src/barretenberg/goblin/goblin.cpp | 2 -- .../cpp/src/barretenberg/goblin/goblin.hpp | 2 ++ .../barretenberg/goblin/goblin_verifier.test.cpp | 5 ++++- .../cpp/src/barretenberg/goblin/mock_circuits.hpp | 14 ++++++++------ .../cpp/src/barretenberg/op_queue/ecc_op_queue.hpp | 5 +---- 7 files changed, 25 insertions(+), 20 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp index d518e24aa6a1..ba38bae8827f 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/eccvm_flavor.hpp @@ -598,9 +598,8 @@ class ECCVMFlavor { #endif { // compute rows for the three different sections of the ECCVM execution trace - // Note: the first operation (index 0) is always a hiding op with random Px, Py values - const auto transcript_rows = - ECCVMTranscriptBuilder::compute_rows(builder.op_queue->get_eccvm_ops(), builder.get_number_of_muls()); + const auto transcript_rows = ECCVMTranscriptBuilder::compute_rows( + builder.op_queue->get_eccvm_ops(), builder.get_number_of_muls(), builder.op_queue->get_is_zk()); const std::vector msms = builder.get_msms(); const auto point_table_rows = ECCVMPointTablePrecomputationBuilder::compute_rows(CircuitBuilder::get_flattened_scalar_muls(msms)); diff --git a/barretenberg/cpp/src/barretenberg/eccvm/transcript_builder.hpp b/barretenberg/cpp/src/barretenberg/eccvm/transcript_builder.hpp index 8964f5d08142..0accc8a2e67c 100644 --- a/barretenberg/cpp/src/barretenberg/eccvm/transcript_builder.hpp +++ b/barretenberg/cpp/src/barretenberg/eccvm/transcript_builder.hpp @@ -133,7 +133,8 @@ class ECCVMTranscriptBuilder { * @return A vector of TranscriptRows */ static std::vector compute_rows(const std::vector& vm_operations, - const uint32_t total_number_of_muls) + const uint32_t total_number_of_muls, + bool is_zk = true) { const size_t num_vm_entries = vm_operations.size(); // The transcript contains an extra zero row at the beginning and the accumulated state at the end @@ -173,7 +174,9 @@ class ECCVMTranscriptBuilder { // Handle hiding op (index 0) separately before the main loop. // The hiding op has random (non-curve) Px, Py values for ZK purposes - we skip EC computation // and just record the raw field elements. Uses opcode 3 (q_eq=1, q_reset=1). - { + // When is_zk=false, there is no hiding op and index 0 is a regular op processed by the main loop. + size_t main_loop_start = 0; + if (is_zk) { const ECCVMOperation& hiding_entry = vm_operations[0]; TranscriptRow& hiding_row = transcript_state[1]; @@ -189,12 +192,13 @@ class ECCVMTranscriptBuilder { msm_accumulator_trace[0] = Element::infinity(); intermediate_accumulator_trace[0] = Element::infinity(); msm_count_at_transition_inverse_trace[0] = 0; + main_loop_start = 1; } // during the first iteration over the ECCOpQueue, the operations are being performed using Jacobian (a.k.a. // projective) coordinates and the base point coordinates are recorded in the transcript. at the same time, the - // transcript logic is being populated (starting from index 1, since index 0 is the hiding op handled above) - for (size_t i = 1; i < num_vm_entries; i++) { + // transcript logic is being populated + for (size_t i = main_loop_start; i < num_vm_entries; i++) { TranscriptRow& row = transcript_state[i + 1]; const ECCVMOperation& entry = vm_operations[i]; diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.cpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.cpp index 0bfa37ef6a86..bda0d945258e 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.cpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.cpp @@ -11,7 +11,6 @@ #include "barretenberg/common/bb_bench.hpp" #include "barretenberg/ecc/curves/grumpkin/grumpkin.hpp" #include "barretenberg/eccvm/eccvm_flavor.hpp" -#include "barretenberg/eccvm/eccvm_trace_checker.hpp" #include "barretenberg/eccvm/eccvm_verifier.hpp" #include "barretenberg/goblin/goblin_verifier.hpp" #include "barretenberg/goblin/merge_verifier.hpp" @@ -39,7 +38,6 @@ void Goblin::prove_eccvm() // Scope the builder so it (and any circuit data) is freed before proving ECCVMProver eccvm_prover = [&]() { ECCVMBuilder eccvm_builder(op_queue); - info("ECCVM STATUS: ", ECCVMTraceChecker::check(eccvm_builder)); return ECCVMProver(eccvm_builder, transcript); }(); auto [eccvm_proof, opening_claim] = eccvm_prover.construct_proof(); diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index 543e78266051..a57ff35e9506 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -30,6 +30,8 @@ class Goblin { bool avm_mode = false; public: + void set_op_queue_zk(bool is_zk) { op_queue->set_is_zk(is_zk); } + using MegaBuilder = MegaCircuitBuilder; using Fr = bb::fr; using Transcript = NativeTranscript; diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/goblin/goblin_verifier.test.cpp index 904d4fa6f2a3..0551cbbb854d 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin_verifier.test.cpp @@ -60,8 +60,8 @@ class GoblinRecursiveVerifierTests : public testing::Test { */ static ProverOutput create_goblin_prover_output(Builder* outer_builder = nullptr, const size_t num_circuits = 5) { - Goblin goblin; + goblin.set_op_queue_zk(false); GoblinMockCircuits::construct_and_merge_mock_circuits(goblin, num_circuits); // Merge the ecc ops from the newly constructed circuit @@ -101,6 +101,7 @@ class GoblinRecursiveVerifierTests : public testing::Test { */ TEST_F(GoblinRecursiveVerifierTests, NativeVerification) { + BB_DISABLE_ASSERTS(); auto [proof, merge_commitments, _] = create_goblin_prover_output(); auto transcript = std::make_shared(); @@ -125,6 +126,8 @@ TEST_F(GoblinRecursiveVerifierTests, NativeVerification) */ TEST_F(GoblinRecursiveVerifierTests, Basic) { + BB_DISABLE_ASSERTS(); + Builder builder; auto [proof, merge_commitments, recursive_merge_commitments] = create_goblin_prover_output(&builder); diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp index c7d718f36dee..59edd994c948 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp @@ -146,17 +146,19 @@ class GoblinMockCircuits { static void construct_and_merge_mock_circuits(Goblin& goblin, const size_t num_circuits = 3) { - using Fq = curve::Grumpkin::ScalarField; for (size_t idx = 0; idx < num_circuits - 1; ++idx) { MegaCircuitBuilder builder{ goblin.op_queue }; if (idx == num_circuits - 2) { - // Last circuit appended needs to begin with a no-op for translator to be shiftable + // Last-prepended circuit: its subtable ends up at the top of the table. + // Add leading no-op (for translator shiftable polynomials) + 3 no-ops + // (padding positions [1..3] that the Translator skips in accumulation). + builder.queue_ecc_no_op(); + builder.queue_ecc_no_op(); + builder.queue_ecc_no_op(); builder.queue_ecc_no_op(); - // Add random ops at START for Translator ZK (lands at beginning of op queue table) - randomise_op_queue(builder, TranslatorCircuitBuilder::NUM_RANDOM_OPS_START); - // Add hiding op for ECCVM ZK (prepended to ECCVM ops at row 1) - builder.queue_ecc_hiding_op(Fq::random_element(), Fq::random_element()); } + // Every circuit starts with an eq_and_reset + builder.queue_ecc_eq(); construct_simple_circuit(builder); goblin.prove_merge(); // Pop the merge proof from the queue, Goblin will be verified at the end diff --git a/barretenberg/cpp/src/barretenberg/op_queue/ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/op_queue/ecc_op_queue.hpp index cc65245fd48b..26fdf59bea98 100644 --- a/barretenberg/cpp/src/barretenberg/op_queue/ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/op_queue/ecc_op_queue.hpp @@ -64,6 +64,7 @@ class ECCOpQueue { public: void set_is_zk(bool _is_zk) { is_zk = _is_zk; } + bool get_is_zk() const { return is_zk; } static const size_t OP_QUEUE_SIZE = 1 << CONST_OP_QUEUE_LOG_SIZE; /** @@ -316,10 +317,6 @@ class ECCOpQueue { { auto expected = accumulator; accumulator.self_set_infinity(); - if (expected.is_point_at_infinity()) { - expected.x = Fq(0); - expected.y = Fq(0); - } EccOpCode op_code{ .eq = true, .reset = true }; // Store eccvm operation append_eccvm_op(ECCVMOperation{ .op_code = op_code, .base_point = expected }); From 9d61dbd2d6cefa038fd8d5c2d05c032d3512d4e2 Mon Sep 17 00:00:00 2001 From: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com> Date: Tue, 31 Mar 2026 05:34:57 +0000 Subject: [PATCH 2/3] Changes to make merge easier --- .../barretenberg/goblin/goblin_verifier.test.cpp | 5 +---- .../cpp/src/barretenberg/goblin/mock_circuits.hpp | 14 ++++++-------- .../cpp/src/barretenberg/op_queue/ecc_op_queue.hpp | 11 +---------- 3 files changed, 8 insertions(+), 22 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin_verifier.test.cpp b/barretenberg/cpp/src/barretenberg/goblin/goblin_verifier.test.cpp index 0551cbbb854d..904d4fa6f2a3 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin_verifier.test.cpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin_verifier.test.cpp @@ -60,8 +60,8 @@ class GoblinRecursiveVerifierTests : public testing::Test { */ static ProverOutput create_goblin_prover_output(Builder* outer_builder = nullptr, const size_t num_circuits = 5) { + Goblin goblin; - goblin.set_op_queue_zk(false); GoblinMockCircuits::construct_and_merge_mock_circuits(goblin, num_circuits); // Merge the ecc ops from the newly constructed circuit @@ -101,7 +101,6 @@ class GoblinRecursiveVerifierTests : public testing::Test { */ TEST_F(GoblinRecursiveVerifierTests, NativeVerification) { - BB_DISABLE_ASSERTS(); auto [proof, merge_commitments, _] = create_goblin_prover_output(); auto transcript = std::make_shared(); @@ -126,8 +125,6 @@ TEST_F(GoblinRecursiveVerifierTests, NativeVerification) */ TEST_F(GoblinRecursiveVerifierTests, Basic) { - BB_DISABLE_ASSERTS(); - Builder builder; auto [proof, merge_commitments, recursive_merge_commitments] = create_goblin_prover_output(&builder); diff --git a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp index 59edd994c948..c7d718f36dee 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp @@ -146,19 +146,17 @@ class GoblinMockCircuits { static void construct_and_merge_mock_circuits(Goblin& goblin, const size_t num_circuits = 3) { + using Fq = curve::Grumpkin::ScalarField; for (size_t idx = 0; idx < num_circuits - 1; ++idx) { MegaCircuitBuilder builder{ goblin.op_queue }; if (idx == num_circuits - 2) { - // Last-prepended circuit: its subtable ends up at the top of the table. - // Add leading no-op (for translator shiftable polynomials) + 3 no-ops - // (padding positions [1..3] that the Translator skips in accumulation). - builder.queue_ecc_no_op(); - builder.queue_ecc_no_op(); - builder.queue_ecc_no_op(); + // Last circuit appended needs to begin with a no-op for translator to be shiftable builder.queue_ecc_no_op(); + // Add random ops at START for Translator ZK (lands at beginning of op queue table) + randomise_op_queue(builder, TranslatorCircuitBuilder::NUM_RANDOM_OPS_START); + // Add hiding op for ECCVM ZK (prepended to ECCVM ops at row 1) + builder.queue_ecc_hiding_op(Fq::random_element(), Fq::random_element()); } - // Every circuit starts with an eq_and_reset - builder.queue_ecc_eq(); construct_simple_circuit(builder); goblin.prove_merge(); // Pop the merge proof from the queue, Goblin will be verified at the end diff --git a/barretenberg/cpp/src/barretenberg/op_queue/ecc_op_queue.hpp b/barretenberg/cpp/src/barretenberg/op_queue/ecc_op_queue.hpp index 26fdf59bea98..20282f55f542 100644 --- a/barretenberg/cpp/src/barretenberg/op_queue/ecc_op_queue.hpp +++ b/barretenberg/cpp/src/barretenberg/op_queue/ecc_op_queue.hpp @@ -60,7 +60,7 @@ class ECCOpQueue { // Tracks number of muls and size of eccvm in real time as the op queue is updated EccvmRowTracker eccvm_row_tracker; - bool is_zk = false; + bool is_zk = true; public: void set_is_zk(bool _is_zk) { is_zk = _is_zk; } @@ -158,15 +158,6 @@ class ECCOpQueue { throw_or_abort("Hiding op must be set before calling get_eccvm_ops()"); } eccvm_ops_reconstructed.insert(eccvm_ops_reconstructed.begin(), hiding_op_for_eccvm); - } else { - // Point base_point; - // base_point.x = Fq(0); - // base_point.y = Fq(0); - - // ECCVMOperation eccvm_ops = - // ECCVMOperation{ .op_code = { .eq = true, .reset = true }, .base_point = base_point }; - - // eccvm_ops_reconstructed.insert(eccvm_ops_reconstructed.begin(), eccvm_ops); } } return eccvm_ops_reconstructed; From 9c334ef0891cbd19d75035af8844191ee07ef5d5 Mon Sep 17 00:00:00 2001 From: federicobarbacovi <171914500+federicobarbacovi@users.noreply.github.com> Date: Tue, 31 Mar 2026 05:35:55 +0000 Subject: [PATCH 3/3] set avm ode --- barretenberg/cpp/src/barretenberg/goblin/goblin.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp index a57ff35e9506..6cd85564cf55 100644 --- a/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp +++ b/barretenberg/cpp/src/barretenberg/goblin/goblin.hpp @@ -30,6 +30,7 @@ class Goblin { bool avm_mode = false; public: + void set_avm_mode(bool mode) { avm_mode = mode; } void set_op_queue_zk(bool is_zk) { op_queue->set_is_zk(is_zk); } using MegaBuilder = MegaCircuitBuilder;