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: makes ec addition opcode unsafe #8814

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
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 @@ -7,6 +7,11 @@

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 @@ -17,9 +22,39 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val
input.input1_x, input.input1_y, input.input1_infinite, has_valid_witness_assignments, builder);
auto input2_point = to_grumpkin_point(
input.input2_x, input.input2_y, input.input2_infinite, has_valid_witness_assignments, builder);
// Runtime checks that the inputs are not the point at infinity, as assumed by the function.
ASSERT(get_value(input.input1_infinite, builder) == 0);
ASSERT(get_value(input.input2_infinite, builder) == 0);

// Addition
cycle_group_ct result = input1_point + input2_point;
// 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 {
// Runtime checks that the inputs have not the same x coordinate, as assumed by the function.
ASSERT(input1_point.x.get_value() != input2_point.x.get_value());
result = input1_point.unconditional_add(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 @@ -35,6 +70,10 @@ void create_ec_add_constraint(Builder& builder, const EcAdd& input, bool has_val
} else {
builder.assert_equal(y_normalized.witness_index, input.result_y);
}
// Runtime check that the result is not the point at infinity, as assumed by the function.
ASSERT(infinite.get_value() == 0);
// 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 @@ -24,19 +24,18 @@ class EcOperations : public ::testing::Test {

size_t generate_ec_add_constraint(EcAdd& ec_add_constraint, WitnessVector& witness_values)
{
using cycle_group_ct = bb::stdlib::cycle_group<Builder>;
witness_values.push_back(0);
auto g1 = grumpkin::g1::affine_one;
cycle_group_ct input_point(g1);
// Doubling
cycle_group_ct result = input_point.dbl();
auto g2 = g1 + g1;
auto affine_result = g1 + g2;

// add: x,y,x2,y2
witness_values.push_back(g1.x);
witness_values.push_back(g1.y);
witness_values.push_back(g1.x);
witness_values.push_back(g1.y);
witness_values.push_back(result.x.get_value());
witness_values.push_back(result.y.get_value());
witness_values.push_back(g2.x);
witness_values.push_back(g2.y);
witness_values.push_back(affine_result.x);
witness_values.push_back(affine_result.y);
witness_values.push_back(fr(0));
witness_values.push_back(fr(0));
ec_add_constraint = EcAdd{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,17 @@ 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);
auto infinite = bool_ct(to_field_ct(input_infinite, 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();
}

// 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,4 +39,12 @@ 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
4 changes: 2 additions & 2 deletions yarn-project/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ You may also need to modify the [Dockerfile](yarn-project/yarn-project-base/Dock
`deploy-npm` script handles the releases of npm packages within yarn-project. But the initial release is a manual process:

1. Ensure relevant folders are copied in by docker in `yarn-project/yarn-project-base/Dockerfile` and `yarn-project/Dockerfile`
2. SSH into the CI
2. SSH into the CI.
3. Run the following:
```sh
cd project
Expand All @@ -86,6 +86,6 @@ COMMIT_TAG=<RELEASE_TAG_NUMBER_YOU_WANT e.g. aztec-packages-v0.8.8>
- Extract `VERSION` as the script shows (in the eg it should be 0.8.8)
- Skip the version existing checks like `if [ "$VERSION" == "$PUBLISHED_VERSION" ]` and `if [ "$VERSION" != "$HIGHER_VERSION" ]`. Since this is our first time deploying the package, `PUBLISHED_VERSION` and `HIGHER_VERSION` will be empty and hence these checks would fail. These checks are necessary in the CI for continual releases.
- Locally update the package version in package.json using `jq` as shown in the script.
- Do a dry-run
- Do a dry-run
- If dry run succeeds, publish the package!
5. Create a PR by adding your package into the `deploy-npm` script so next release onwards, CI can cut releases for your package.
Loading