Skip to content

Conversation

@xgaozoyoe
Copy link
Collaborator

@xgaozoyoe xgaozoyoe commented Nov 23, 2021

  1. Fix a bug in main_gate.select
  2. Add ECC add.
  3. Add ECC add tests.
  4. Extract generic apis from base instructions and external instructions.

@kilic
Copy link
Contributor

kilic commented Nov 26, 2021

This PR resulted in making ecc traits more mature. Thanks!

So I have borrowed some stuff from this PR and make it available on master. Then this PR should more became ecc add op related.

Some critics for current status of this PR are:

  • In general case we can optimise for a = 0 case which is I think the most of the cases we will be dealing with for example ecdsa on secp256k1 and recursion with bn254. So we can now enjoy a_is_zero function in ecc chips to skip some constrains in addition op.

  • a, b and identity are constant numbers so we shouldn't assign those integers as witnesses since verifier can introduce any a or b they like. Instead we can use selector colums to perform those operation. So master now has such functionality to add constant numbers

  • We should have two rns instances for general ecc case one is for base field and the other is for scalar field. And this is already applied in master now.

Also you can enjoy parameter_a parameter_b functions in ecc chips now.

So I will ask you to rebase this PR onto master and add addition operation to both general_ecc and base_field_ecc cases and consume new functionalities whenever you can.

@xgaozoyoe
Copy link
Collaborator Author

Merged master into ecc-opps and fixed related conflicts.

Copy link
Contributor

@kilic kilic left a comment

Choose a reason for hiding this comment

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

Second request is made

@kilic
Copy link
Contributor

kilic commented Dec 6, 2021

Thank you!

@kilic kilic merged commit fd25145 into privacy-ethereum:master Dec 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants