Skip to content

Commit

Permalink
fix(revert): "chore!: change ec_add to unsafe implementation (but muc…
Browse files Browse the repository at this point in the history
…h better perf)" (#8722)

Reverts #8374 as it broke prover tests
  • Loading branch information
ludamad authored Sep 24, 2024
1 parent 0337fe9 commit 9a1b5b5
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,6 @@

namespace acir_format {

// This functions assumes that:
// 1. none of the points are infinity
// 2. the points are on the curve
// 3a. the points have not the same abssissa, OR
// 3b. the points are identical (same witness index or same value)
template <typename Builder>
void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_valid_witness_assignments)
{
Expand All @@ -24,32 +19,7 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val
input.input2_x, input.input2_y, input.input2_infinite, has_valid_witness_assignments, builder);

// Addition
// Check if operands are the same
bool x_match = false;
if (!input1_point.x.is_constant() && !input2_point.x.is_constant()) {
x_match = (input1_point.x.get_witness_index() == input2_point.x.get_witness_index());
} else {
if (input1_point.x.is_constant() && input2_point.x.is_constant()) {
x_match = (input1_point.x.get_value() == input2_point.x.get_value());
}
}
bool y_match = false;
if (!input1_point.y.is_constant() && !input2_point.y.is_constant()) {
y_match = (input1_point.y.get_witness_index() == input2_point.y.get_witness_index());
} else {
if (input1_point.y.is_constant() && input2_point.y.is_constant()) {
y_match = (input1_point.y.get_value() == input2_point.y.get_value());
}
}

cycle_group_ct result;
// If operands are the same, we double.
if (x_match && y_match) {
result = input1_point.dbl();
} else {
result = input1_point.unconditional_add(input2_point);
}

cycle_group_ct result = input1_point + input2_point;
cycle_group_ct standard_result = result.get_standard_form();
auto x_normalized = standard_result.x.normalize();
auto y_normalized = standard_result.y.normalize();
Expand All @@ -65,8 +35,6 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val
} else {
builder.assert_equal(y_normalized.witness_index, input.result_y);
}
// TODO: remove the infinite result, because the function should always return a non-zero point.
// But this requires an ACIR serialisation change and it will be done in a subsequent PR.
if (infinite.is_constant()) {
builder.fix_witness(input.result_infinite, infinite.get_value());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,10 @@ bb::stdlib::cycle_group<Builder> to_grumpkin_point(const WitnessOrConstant<FF>&
bool has_valid_witness_assignments,
Builder& builder)
{
using bool_ct = bb::stdlib::bool_t<Builder>;
auto point_x = to_field_ct(input_x, builder);
auto point_y = to_field_ct(input_y, builder);
// We assume input_infinite is boolean, so we do not add a boolean gate
bool_t infinite(&builder);
if (!input_infinite.is_constant) {
infinite.witness_index = input_infinite.index;
infinite.witness_bool = get_value(input_infinite, builder) == FF::one();
} else {
infinite.witness_index = IS_CONSTANT;
infinite.witness_bool = input_infinite.value == FF::one();
}
auto infinite = bool_ct(to_field_ct(input_infinite, builder));

// When we do not have the witness assignments, we set is_infinite value to true if it is not constant
// else default values would give a point which is not on the curve and this will fail verification
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,4 @@ bb::stdlib::cycle_group<Builder> to_grumpkin_point(const WitnessOrConstant<FF>&
bool has_valid_witness_assignments,
Builder& builder);

template <typename Builder, typename FF> FF get_value(const WitnessOrConstant<FF>& input, Builder& builder)
{
if (input.is_constant) {
return input.value;
}
return builder.variables[input.index];
}

} // namespace acir_format

1 comment on commit 9a1b5b5

@AztecBot
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'C++ Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.05.

Benchmark suite Current: 9a1b5b5 Previous: aabd2d8 Ratio
commit(t) 8946052540 ns/iter 8508471892 ns/iter 1.05

This comment was automatically generated by workflow using github-action-benchmark.

CC: @ludamad @codygunton

Please sign in to comment.