Skip to content

Commit

Permalink
fix unconstrained witnesses and do add accum in batch mul
Browse files Browse the repository at this point in the history
  • Loading branch information
ledwards2225 committed Sep 5, 2023
1 parent 7e92181 commit e52d884
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,11 @@ template <typename Curve, bool goblin_flag = false> class GeminiVerifier_ {
// achieved through a builder Simulator, the stdlib codepath should become the only codepath.
if constexpr (Curve::is_stdlib_type) {
std::vector<GroupElement> 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<goblin_flag>(commitments, { one, r_inv });
C0_r_neg = GroupElement::template batch_mul<goblin_flag>(commitments, { one, -r_inv });
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ template <typename Curve, bool goblin_flag = false> 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<GroupElement> commitments = { claim.commitment, quotient_commitment };
std::vector<Fr> scalars = { one, claim.opening_pair.challenge };
P_0 = GroupElement::template batch_mul<goblin_flag>(commitments, scalars);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ template <typename Curve, bool goblin_flag = false> 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<Commitment> commitments;
Expand All @@ -201,7 +201,7 @@ template <typename Curve, bool goblin_flag = false> 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<Fr> inverse_vanishing_evals;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -40,7 +43,13 @@ element<C, Fq, Fr, G> element<C, Fq, Fr, G>::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
Expand All @@ -61,10 +70,12 @@ element<C, Fq, Fr, G> element<C, Fq, Fr, G>::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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,7 @@ template <typename UseGoblinFlag> 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());
}
};
Expand Down

0 comments on commit e52d884

Please sign in to comment.