diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp index 7f12fea58d26..f46d55ae71f2 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/gemini/gemini.hpp @@ -242,8 +242,11 @@ template class GeminiVerifier_ { // achieved through a builder Simulator, the stdlib codepath should become the only codepath. if constexpr (Curve::is_stdlib_type) { std::vector commitments = { batched_f, batched_g }; - auto one = Fr::from_witness(r.get_context(), 1); - // TODO(#707): these batch muls are not optimal since we are performing an unnecessary mul by 1. + auto builder = r.get_context(); + auto one = Fr(builder, 1); + // TODO(#707): these batch muls include the use of 1 as a scalar. This is handled appropriately as a non-mul + // (add-accumulate) in the goblin batch_mul but is done inefficiently as a scalar mul in the conventional + // emulated batch mul. C0_r_pos = GroupElement::template batch_mul(commitments, { one, r_inv }); C0_r_neg = GroupElement::template batch_mul(commitments, { one, -r_inv }); } else { diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp index 281aa0a67af7..99bbda063bdc 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/kzg/kzg.hpp @@ -85,7 +85,7 @@ template class KZG { // evaluation is not equal to zero). if constexpr (Curve::is_stdlib_type) { auto builder = verifier_transcript.builder; - auto one = Fr::from_witness(builder, 1); + auto one = Fr(builder, 1); std::vector commitments = { claim.commitment, quotient_commitment }; std::vector scalars = { one, claim.opening_pair.challenge }; P_0 = GroupElement::template batch_mul(commitments, scalars); diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp index 93e2e56ac11f..f53925af24e1 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/honk/pcs/shplonk/shplonk.hpp @@ -192,7 +192,7 @@ template class ShplonkVerifier_ { // using a builder Simulator. if constexpr (Curve::is_stdlib_type) { auto builder = nu.get_context(); - evaluation_zero = Fr::from_witness(builder, 0); + evaluation_zero = Fr(builder, 0); // Containers for the inputs to the final batch mul std::vector commitments; @@ -201,7 +201,7 @@ template class ShplonkVerifier_ { // [G] = [Q] - ∑ⱼ ρʲ / ( r − xⱼ )⋅[fⱼ] + G₀⋅[1] // = [Q] - [∑ⱼ ρʲ ⋅ ( fⱼ(X) − vⱼ) / ( r − xⱼ )] commitments.emplace_back(Q_commitment); - scalars.emplace_back(Fr::from_witness(builder, 1)); // Fr(1) + scalars.emplace_back(Fr(builder, 1)); // Fr(1) // Compute {ẑⱼ(r)}ⱼ , where ẑⱼ(r) = 1/zⱼ(r) = 1/(r - xⱼ) std::vector inverse_vanishing_evals; diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp index a666611bec01..8217b08c2a18 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/primitives/biggroup/biggroup_goblin.hpp @@ -13,6 +13,9 @@ namespace stdlib { * natively (without constraints) under the hood, and the final result is obtained by queueing an equality operation via * the builder. The components of the result are returned as indices into the variables array from which the resulting * accumulator point is re-constructed. + * @note Because this is the only method for performing Goblin-style group operations (Issue #707), it is sometimes used + * in situations where one of the scalars is 1 (e.g. to perform P = P_0 + z*P_1). In this case, we perform a simple add + * accumulate instead of a mul-then_accumulate. * * @tparam C CircuitBuilder * @tparam Fq Base field @@ -40,7 +43,13 @@ element element::goblin_batch_mul(const std::vector< auto& scalar = scalars[i]; // Populate the goblin-style ecc op gates for the given mul inputs - auto op_tuple = builder->queue_ecc_mul_accum(point.get_value(), scalar.get_value()); + ecc_op_tuple op_tuple; + bool scalar_is_constant_equal_one = scalar.get_witness_index() == IS_CONSTANT && scalar.get_value() == 1; + if (scalar_is_constant_equal_one) { // if scalar is 1, there is no need to perform a mul + op_tuple = builder->queue_ecc_add_accum(point.get_value()); + } else { // otherwise, perform a mul-then-accumulate + op_tuple = builder->queue_ecc_mul_accum(point.get_value(), scalar.get_value()); + } // Add constraints demonstrating that the EC point coordinates were decomposed faithfully. In particular, show // that the lo-hi components that have been encoded in the op wires can be reconstructed via the limbs of the @@ -61,10 +70,12 @@ element element::goblin_batch_mul(const std::vector< y_hi.assert_equal(point.y.binary_basis_limbs[2].element + shift * point.y.binary_basis_limbs[3].element); // Add constraints demonstrating proper decomposition of scalar into endomorphism scalars - auto z_1 = Fr::from_witness_index(builder, op_tuple.z_1); - auto z_2 = Fr::from_witness_index(builder, op_tuple.z_2); - auto beta = G::subgroup_field::cube_root_of_unity(); - scalar.assert_equal(z_1 - z_2 * beta); + if (!scalar_is_constant_equal_one) { + auto z_1 = Fr::from_witness_index(builder, op_tuple.z_1); + auto z_2 = Fr::from_witness_index(builder, op_tuple.z_2); + auto beta = G::subgroup_field::cube_root_of_unity(); + scalar.assert_equal(z_1 - z_2 * beta); + } } // Populate equality gates based on the internal accumulator point diff --git a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp index b593487cfb1e..7bfc45452d17 100644 --- a/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp +++ b/circuits/cpp/barretenberg/cpp/src/barretenberg/stdlib/recursion/honk/verifier/verifier.test.cpp @@ -222,10 +222,7 @@ template class RecursiveVerifierTest : public testing:: // Create a recursive verification circuit for the proof of the inner circuit create_outer_circuit(inner_circuit, outer_circuit); - if (outer_circuit.failed()) { - info(outer_circuit.err()); - } - EXPECT_EQ(outer_circuit.failed(), false); + EXPECT_EQ(outer_circuit.failed(), false) << outer_circuit.err(); EXPECT_TRUE(outer_circuit.check_circuit()); } };