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

Feature/poseidon opt goff #15

Merged
merged 4 commits into from
Mar 4, 2020
Merged

Feature/poseidon opt goff #15

merged 4 commits into from
Mar 4, 2020

Conversation

arnaucube
Copy link
Contributor

@arnaucube arnaucube commented Mar 3, 2020

Optimize Poseidon migrating from *big.Int to goff generated finite field
operations.

Also fixes #4 and updates the Mimc7 usage of the CheckBigIntArrayInField, and updates BabyJubJub EDDSA to the new Poseidon methods.

Benchmarks:
Tested on a Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz, with 16GB of RAM.

  • Before the optimizations:
BenchmarkPoseidon-4        	     470	   2489678 ns/op
BenchmarkPoseidonLarge-4   	     476	   2530568 ns/op
BenchmarkPoseidon-4        	     766	   1550013 ns/op
BenchmarkPoseidonLarge-4   	     782	   1547572 ns/op
  • With the changes of this PR, where uses goff generated code instead of *big.Int:
BenchmarkPoseidon-4        	    9638	    121651 ns/op
BenchmarkPoseidonLarge-4   	    9781	    119921 ns/op

More than 10x improvement (from 1.547.572 ns/op to 119.921 ns/op).

Warning: do not merge until we check the license

Optimize Poseidon migrating from *big.Int to goff generated finite field
operations.

Benchmarks:
Tested on a Intel(R) Core(TM) i5-7200U CPU @ 2.50GHz, with 16GB of RAM.

- Before the optimizations:
```
BenchmarkPoseidon-4                  470           2489678 ns/op
BenchmarkPoseidonLarge-4             476           2530568 ns/op
```

- With the optimizations of #12:
```
BenchmarkPoseidon-4                  766           1550013 ns/op
BenchmarkPoseidonLarge-4             782           1547572 ns/op
```

- With the changes of this PR, where uses goff generated code instead of *big.Int:
```
BenchmarkPoseidon-4                 9638            121651 ns/op
BenchmarkPoseidonLarge-4            9781            119921 ns/op
```
@arnaucube arnaucube added the buidl To be implemented label Mar 3, 2020
@arnaucube arnaucube requested a review from ed255 March 3, 2020 15:36
@arnaucube arnaucube self-assigned this Mar 3, 2020
poseidon/poseidon.go Outdated Show resolved Hide resolved
poseidon/poseidon.go Outdated Show resolved Hide resolved
utils/utils.go Outdated Show resolved Hide resolved
@ed255
Copy link
Contributor

ed255 commented Mar 4, 2020

The code looks good to me! Now the only remaining thing to merge this is resolve any doubt about licensing.

  • All our code is published under GPLv3 and the copyright is held by 0kims (thus giving 0kims the possibility of distributing under a different license).
  • Goff is Apache 2, which allows commercial distribution without disclosing the source code with the only requirements of:
    • Showing the Apache 2 copyright notice with the distribution
    • Stating the list of changes made to the code

So I think Apache 2 would cause any problem in either the GPLv3 licensed distribution of our libraries nor a possible commercial distribution under a private license (as long as the two requirements of the Apache 2 for Goff are met).

@arnaucube
Copy link
Contributor Author

After internal confirmation that there are no issues with the license, will merge this PR to master.

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buidl To be implemented
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a function to check if a big int fits in the ZK field
2 participants