Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Remove endomorphism coefficient from ecc_add_gate #3115

Merged
merged 4 commits into from
Oct 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -254,21 +254,17 @@ TEST_F(UltraHonkComposerTests, test_elliptic_gate)
uint32_t x3 = circuit_builder.add_variable(p3.x);
uint32_t y3 = circuit_builder.add_variable(p3.y);

circuit_builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1, 1 });
circuit_builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });

grumpkin::fq beta = grumpkin::fq::cube_root_of_unity();
affine_element p2_endo = p2;
p2_endo.x *= beta;
p3 = affine_element(element(p1) + element(p2_endo));
p3 = affine_element(element(p1) + element(p2));
x3 = circuit_builder.add_variable(p3.x);
y3 = circuit_builder.add_variable(p3.y);
circuit_builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, beta, 1 });
circuit_builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });

p2_endo.x *= beta;
p3 = affine_element(element(p1) - element(p2_endo));
p3 = affine_element(element(p1) - element(p2));
x3 = circuit_builder.add_variable(p3.x);
y3 = circuit_builder.add_variable(p3.y);
circuit_builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, beta.sqr(), -1 });
circuit_builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, -1 });

auto composer = UltraComposer();
prove_and_verify(circuit_builder, composer, /*expected_result=*/true);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,11 +174,7 @@ template <typename Flavor> void create_some_elliptic_curve_addition_gates(auto&
grumpkin::g1::affine_element p1 = grumpkin::g1::affine_element::random_element();
grumpkin::g1::affine_element p2 = grumpkin::g1::affine_element::random_element();

grumpkin::fq beta_scalar = grumpkin::fq::cube_root_of_unity();
grumpkin::g1::affine_element p2_endo = p2;
p2_endo.x *= beta_scalar;

grumpkin::g1::affine_element p3(grumpkin::g1::element(p1) - grumpkin::g1::element(p2_endo));
grumpkin::g1::affine_element p3(grumpkin::g1::element(p1) - grumpkin::g1::element(p2));

uint32_t x1 = circuit_builder.add_variable(p1.x);
uint32_t y1 = circuit_builder.add_variable(p1.y);
Expand All @@ -187,7 +183,7 @@ template <typename Flavor> void create_some_elliptic_curve_addition_gates(auto&
uint32_t x3 = circuit_builder.add_variable(p3.x);
uint32_t y3 = circuit_builder.add_variable(p3.y);

circuit_builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, beta_scalar, -1 });
circuit_builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, -1 });
}

template <typename Flavor> void create_some_ecc_op_queue_gates(auto& circuit_builder)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,7 @@ TEST_F(SumcheckTests, RealCircuitUltra)
grumpkin::g1::affine_element p1 = grumpkin::g1::affine_element::random_element();
grumpkin::g1::affine_element p2 = grumpkin::g1::affine_element::random_element();

grumpkin::fq beta_scalar = grumpkin::fq::cube_root_of_unity();
grumpkin::g1::affine_element p2_endo = p2;
p2_endo.x *= beta_scalar;

grumpkin::g1::affine_element p3(grumpkin::g1::element(p1) - grumpkin::g1::element(p2_endo));
grumpkin::g1::affine_element p3(grumpkin::g1::element(p1) + grumpkin::g1::element(p2));

uint32_t x1 = builder.add_variable(p1.x);
uint32_t y1 = builder.add_variable(p1.y);
Expand All @@ -340,7 +336,7 @@ TEST_F(SumcheckTests, RealCircuitUltra)
uint32_t x3 = builder.add_variable(p3.x);
uint32_t y3 = builder.add_variable(p3.y);

builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, beta_scalar, -1 });
builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });

// Add some RAM gates
uint32_t ram_values[8]{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -708,7 +708,7 @@ TEST_F(join_split_tests, test_0_input_notes_and_detect_circuit_change)
// The below part detects any changes in the join-split circuit
constexpr uint32_t CIRCUIT_GATE_COUNT = 49492;
constexpr uint32_t GATES_NEXT_POWER_OF_TWO = 65535;
const uint256_t VK_HASH("986c3fe747d2f1b84fd9dea37a22c27bd4e1006900f458decf2da20442a7395a");
const uint256_t VK_HASH("cc3b14fad5465fe9b8c714e8a5d79012b86a70f6e37dfc23054e6def7eb1770f");

auto number_of_gates_js = result.number_of_gates;
std::cout << get_verification_key()->sha256_hash() << std::endl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,21 +167,17 @@ TYPED_TEST(ultra_plonk_composer, test_elliptic_gate)
uint32_t x3 = builder.add_variable(p3.x);
uint32_t y3 = builder.add_variable(p3.y);

builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1, 1 });
builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });

grumpkin::fq beta = grumpkin::fq::cube_root_of_unity();
affine_element p2_endo = p2;
p2_endo.x *= beta;
p3 = affine_element(element(p1) + element(p2_endo));
p3 = affine_element(element(p1) + element(p2));
x3 = builder.add_variable(p3.x);
y3 = builder.add_variable(p3.y);
builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, beta, 1 });
builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });

p2_endo.x *= beta;
p3 = affine_element(element(p1) - element(p2_endo));
p3 = affine_element(element(p1) - element(p2));
x3 = builder.add_variable(p3.x);
y3 = builder.add_variable(p3.y);
builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, beta.sqr(), -1 });
builder.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, -1 });

TestFixture::prove_and_verify(builder, composer, /*expected_result=*/true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ template <typename FF> struct ecc_add_gate_ {
uint32_t y2;
uint32_t x3;
uint32_t y3;
FF endomorphism_coefficient;
FF sign_coefficient;
};
template <typename FF> struct ecc_dbl_gate_ {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -383,12 +383,10 @@ template <typename FF> void UltraCircuitBuilder_<FF>::create_poly_gate(const pol
/**
* @brief Create an elliptic curve addition gate
*
* @details x and y are defined over scalar field. Addition can handle applying the curve endomorphism to one of the
* points being summed at the time of addition.
* @details x and y are defined over scalar field.
*
* @param in Elliptic curve point addition gate parameters, including the the affine coordinates of the two points being
* added, the resulting point coordinates and the selector values that describe whether the endomorphism is used on the
* second point and whether it is negated.
* added, the resulting point coordinates and the selector values that describe whether the second point is negated.
*/
template <typename FF> void UltraCircuitBuilder_<FF>::create_ecc_add_gate(const ecc_add_gate_<FF>& in)
{
Expand All @@ -411,25 +409,16 @@ template <typename FF> void UltraCircuitBuilder_<FF>::create_ecc_add_gate(const
can_fuse_into_previous_gate = can_fuse_into_previous_gate && (q_arith[this->num_gates - 1] == 0);
can_fuse_into_previous_gate = can_fuse_into_previous_gate && (q_m[this->num_gates - 1] == 0);

// TODO(@zac-williamson #2608 remove endomorphism coefficient)
bool endomorphism_present = in.endomorphism_coefficient != 1;
if (can_fuse_into_previous_gate) {
q_3[this->num_gates - 1] = in.endomorphism_coefficient;
q_4[this->num_gates - 1] = in.endomorphism_coefficient.sqr();
q_1[this->num_gates - 1] = in.sign_coefficient;

// TODO(@zac-williamson #2608) Change this back to 1 when pedersen refactor is complete.
// This is temporary stopgap. We can't support both a double gate and support the ecc enodmorphism
// without pushing the degree of the constraint above 5, which breaks ultraplonk.
// The pedersen refactor will remove all uses of the endomorphism
q_elliptic[this->num_gates - 1] = endomorphism_present ? 0 : 1;
q_elliptic[this->num_gates - 1] = 1;
} else {
w_l.emplace_back(this->zero_idx);
w_r.emplace_back(in.x1);
w_o.emplace_back(in.y1);
w_4.emplace_back(this->zero_idx);
q_3.emplace_back(in.endomorphism_coefficient);
q_4.emplace_back(in.endomorphism_coefficient.sqr());
q_3.emplace_back(0);
q_4.emplace_back(0);
q_1.emplace_back(in.sign_coefficient);

q_arith.emplace_back(0);
Expand All @@ -438,12 +427,7 @@ template <typename FF> void UltraCircuitBuilder_<FF>::create_ecc_add_gate(const
q_c.emplace_back(0);
q_sort.emplace_back(0);
q_lookup_type.emplace_back(0);

// TODO(@zac-williamson #2608) Change this back to 1 when pedersen refactor is complete.
// This is temporary stopgap. We can't support both a double gate and support the ecc enodmorphism
// without pushing the degree of the constraint above 5, which breaks ultraplonk.
// The pedersen refactor will remove all uses of the endomorphism
q_elliptic.emplace_back(endomorphism_present ? 0 : 1);
q_elliptic.emplace_back(1);
q_aux.emplace_back(0);
++this->num_gates;
}
Expand Down Expand Up @@ -2771,8 +2755,7 @@ inline FF UltraCircuitBuilder_<FF>::compute_genperm_sort_identity(FF q_sort_valu
}

/**
* @brief Elliptic curve identity gate methods implement elliptic curve point addition. The gate is enhanced to handle
* the case where one of the points is automatically scaled by the endomorphism constant β or negated
* @brief Elliptic curve identity gate methods implement elliptic curve point addition.
*
*
* @details The basic equation for the elliptic curve in short weierstrass form is y^2 == x^3 + a * x + b.
Expand All @@ -2785,48 +2768,12 @@ inline FF UltraCircuitBuilder_<FF>::compute_genperm_sort_identity(FF q_sort_valu
* If we assume that the points being added are distinct and not invereses of each other (so their x coordinates
* differ), then we can rephrase this equality:
* x_3 * (x_2 - x_1)^2 = ((y_2 - y_1)^2 - (x_2 - x_1) * (x_2^2 - x_1^2))
* Let's say we want to apply the endomorphism to the (x_2, y_2) point at the same time and maybe change the sign of
* y_2:
*
* (x_2, y_2) = (β * x_2', sign * y_2')
* x_3 * (β * x_2' - x_1)^2 = ((sign * y_2' - y_1)^2 - (β * x_2' - x_1) * ((β * x_2')^2 - x_1^2))
*
* Let's open the brackets and group the terms by β, β^2, sign:
*
* x_2'^2 * x_3 * β^2 - 2 * β * x_1 * x_2' * x_3 - x_1^2 * x_3 = sign^2 * y_2'^2 - 2 * sign * y_1 * y_2 + y_1^2 - β^3
* * x_2'^3 + β * x_1^2 * x_2' + β^2 * x_1 * x_2'^2 - x_1^3
*
* β^3 = 1
* sign^2 = 1 (at least we always expect sign to be set to 1 or -1)
*
* sign * (-2 * y_1 * y_2) + β * (2 * x_1 * x_2' * x_3 +x_1^2 * x_2') + β^2 * (x_1 * x_2'^2 - x_2'^2 * x_3) + (x_1^2 *
* x_3 + y_2'^2 + y_1^2 - x_2'^3 - x_1^3) = 0
* This is the equation computed in x_identity and scaled by α
*
* Now let's deal with the y equation:
* y_3 = λ * (x_3 - x_1) + y_1 = (y_2 - y_1) * (x_3 - x_1) / (x_2 - x_1) + y_1 = ((y_2 - y_1) * (x_3 - x_1) + y_1 *
* (x_2 - x_1)) / (x_2 - x_1)
*
* (x_2 - x_1) * y_3 = (y_2 - y_1) * (x_3 - x_1) + y_1 * (x_2 - x_1)
*
* Let's substitute (x_2, y_2) = (β * x_2', sign * y_2'):
*
* β * x_2' * y_3 - x_1 * y_3 - sign * y_2' * x_3 + y_1 * x_3 + sign * y_2' * x_1 - y_1 * x_1 - β * y_1 * x_2' + x_1
* * y_1 = 0
*
* Let's group:
*
* sign * (-y_2' * x_3 + y_2' * x_1) + β * (x_2' * x_3 + y_1 * x_2') + (-x_1 * y_3 + y_1 * x_3 - x_1 * y_1 +
* x_1 * y_1) = 0
*
*/

/**
* @brief Compute the identity of the arithmetic gate fiven all coefficients
* @brief Compute the identity of the arithmetic gate given all coefficients
*
* @param q_1_value 1 or -1 (the sign). Controls whether we are subtracting or adding the second point
* @param q_3_value The endomorphism coefficient β, if we are using the endomorphism here
* @param q_4_value β² if we need it
* @param w_2_value x₁
* @param w_3_value y₁
* @param w_1_shifted_value x₂
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ TEST(ultra_circuit_constructor, test_elliptic_gate)
affine_element p1 = crypto::pedersen_commitment::commit_native({ barretenberg::fr(1) }, 0);

affine_element p2 = crypto::pedersen_commitment::commit_native({ barretenberg::fr(1) }, 1);
;
affine_element p3(element(p1) + element(p2));

uint32_t x1 = circuit_constructor.add_variable(p1.x);
Expand All @@ -125,15 +124,15 @@ TEST(ultra_circuit_constructor, test_elliptic_gate)
uint32_t x3 = circuit_constructor.add_variable(p3.x);
uint32_t y3 = circuit_constructor.add_variable(p3.y);

circuit_constructor.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1, 1 });
circuit_constructor.create_ecc_add_gate({ x1, y1, x2, y2, x3, y3, 1 });

auto saved_state = UltraCircuitBuilder::CircuitDataBackup::store_full_state(circuit_constructor);
bool result = circuit_constructor.check_circuit();

EXPECT_EQ(result, true);
EXPECT_TRUE(saved_state.is_same_state(circuit_constructor));

circuit_constructor.create_ecc_add_gate({ x1 + 1, y1, x2, y2, x3, y3, 1, 1 });
circuit_constructor.create_ecc_add_gate({ x1 + 1, y1, x2, y2, x3, y3, 1 });

EXPECT_EQ(circuit_constructor.check_circuit(), false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,6 @@ cycle_group<Composer> cycle_group<Composer>::unconditional_add(const cycle_group
.y2 = other.y.get_witness_index(),
.x3 = result.x.get_witness_index(),
.y3 = result.y.get_witness_index(),
.endomorphism_coefficient = 1,
.sign_coefficient = 1,
};
context->create_ecc_add_gate(add_gate);
Expand Down Expand Up @@ -342,7 +341,6 @@ cycle_group<Composer> cycle_group<Composer>::unconditional_subtract(const cycle_
.y2 = other.y.get_witness_index(),
.x3 = result.x.get_witness_index(),
.y3 = result.y.get_witness_index(),
.endomorphism_coefficient = 1,
.sign_coefficient = -1,
};
context->create_ecc_add_gate(add_gate);
Expand Down