Skip to content

Commit

Permalink
chore: Remove endomorphism coefficient from ecc_add_gate (#3115)
Browse files Browse the repository at this point in the history
Resolves #2608 

# Checklist:
Remove the checklist to signal you've completed it. Enable auto-merge if
the PR is ready to merge.
- [x] If the pull request requires a cryptography review (e.g.
cryptographic algorithm implementations) I have added the 'crypto' tag.
- [ ] I have reviewed my diff in github, line by line and removed
unexpected formatting changes, testing logs, or commented-out code.
- [x] Every change is related to the PR description.
- [x] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
this pull request to relevant issues (if any exist).
  • Loading branch information
kevaundray authored Oct 30, 2023
1 parent 16abb5a commit d294987
Show file tree
Hide file tree
Showing 9 changed files with 25 additions and 98 deletions.
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

0 comments on commit d294987

Please sign in to comment.