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

perf: optimize Pedersen and IPA commitments gadgets #201

Conversation

yelhousni
Copy link
Contributor

@yelhousni yelhousni commented Jan 29, 2025

Description

This PR optimizes Pedersen and IPA commitments gadgets, through the use of joint_scalar_mul_be and scalar_mul_le implemented here arkworks-rs/r1cs-std#155.

Benchmark

  • The Pedersen check circuit goes from 3,575,519 r1cs to 2,455,364 r1cs.
  • The IPA verify circuit also goes from 77,175 r1cs to 73,367 r1cs.

arnaucube and others added 11 commits November 28, 2024 16:53
* add Nova benchmarks logic

* abstract benchmarks, add HyperNova & ProtoGalaxy's IVC benchmarks

* add profiler & flamegraph at benches

* add bench compile to github CI
* Update to latest arkworks

* Fix typos, fmt, and clippy
…s` renaming (privacy-scaling-explorations#176)

* rename frontends to experimental-frontends to make it explicit, as discussed in the last meeting

* rename upper_bound_ccs to compute_concrete_ccs to describe better the method logic

* migrate tests to use result instead of unwrap

motivation: in this way tests are like in-library code and there is no
differenciation between test code and library code in terms of style,
where errors are returned with `?` instead of `unwrap`. Same for
examples. This was already done partially like this in some of the
ProtoGalaxy tests, now on almost all the folding-schemes tests.

* update CI to work with the new dir name

* apply PR review suggestions
…e_T method (privacy-scaling-explorations#183)

* reduce ~65% proving time in HyperNova by removing the not-needed cs.finalize calls

* update compute_T (Nova & Mova) using Mova optimized approach from Mova's section 5.2 (https://eprint.iacr.org/2024/1220.pdf)
…xplorations#184)

* Reduce native computation by removing the unnecessary `step_native` call

* It is unnecessary to compute `x` for `CycleFoldCircuit` outside the circuit

* Remove `step_native` from `FCircuit`

* Rationale behind the check of public inputs against witnesses

* Format

* Fix examples
…ons#189)

* Simplify trait bounds by introducing `Sonobe{Field, Curve}`

* `ToEth` trait

* Simplify `Inputize` by separating `InputizeNonNative` from it

* Refactor

* Make CycleFold related interfaces cleaner

* Fmt and fix clippy

* Remove the `Sonobe` prefix from `Field` and `Curve` traits
… for the external inputs. (privacy-scaling-explorations#191)

* Update the FCircuit trait, so that it supports custom data structures for the external inputs.

This also eliminates the need of having the `external_inputs_len` method in the `FCircuit`.

The motivation for this change is that in most practical use cases, the
external inputs have a not-naive structure, and the old interface required that
to convert the external inputs structure into a vector of finite field
elements, which inside the FCircuit would be converted back into a custom data
structure. This is specially tedious when dealing with curve points, which
converting them from point to field elements and back to the point (both
outside (rust native) and inside the circuit (constraints)) is a bit
cumbersome.  With this update, it's much more straight forward to define the
FCircuit with custom external inputs data structures.

For the experimental-frontends, the external inputs keep being an array of field elements.

* remove Default from FCircuit::ExternalInputsVar, remove VecF & VecFpVar in examples/external_inputs.rs
…ling-explorations#193)

* `Decider::preprocess` now no longer takes `fs` as input

* Format

* Fix examples

* Fix solidity verifiers
privacy-scaling-explorations#195)

* Rename `Arith` to `ArithRelation`

* Introduce `Arith::degree` to avoid using the magic number `2`

* More getter methods for R1CS and CCS to avoid pub members
@yelhousni yelhousni changed the title perf: otimize Pedersen commitment perf: optimize Pedersen and IPA commitments gadgets Jan 30, 2025
@arnaucube
Copy link
Collaborator

This is really great!

Over these past couple of months, Sonobe was updated to use arkworks v0.5.0, which slightly changes the types, but I think that will have not much affect to the changes of this PR, but it might affect more the fork of r1cs-std.

How would you see on doing the following?

  • doing the PR of ark-r1cs-std repo (ark-r1cs-std PR) directly to the main arkworks repo https://github.com/arkworks-rs/r1cs-std
  • and the current sonobe PR be done from latest main branch, where independently of the arkworks r1cs-std being merged to main, we can already use your fork with the r1cs-std improvement (of the arkworks-rs/r1cs-std (v0.5.0))

Alternatively we can merge the current PR (since it points to the branch decider-circuit-bench) and then update that branch to latest Sonobe main, while in parallel porting the r1cs-std changes to the last arkworks-rs/r1cs-std version, but probably the above will be cleaner.

In any case, happy to do any option that it's easier and less time consuming for you. Thank you very much for this improvement!!! 🙌

@yelhousni
Copy link
Contributor Author

This is really great!

Over these past couple of months, Sonobe was updated to use arkworks v0.5.0, which slightly changes the types, but I think that will have not much affect to the changes of this PR, but it might affect more the fork of r1cs-std.

How would you see on doing the following?

  • doing the PR of ark-r1cs-std repo (ark-r1cs-std PR) directly to the main arkworks repo https://github.com/arkworks-rs/r1cs-std
  • and the current sonobe PR be done from latest main branch, where independently of the arkworks r1cs-std being merged to main, we can already use your fork with the r1cs-std improvement (of the arkworks-rs/r1cs-std (v0.5.0))

Alternatively we can merge the current PR (since it points to the branch decider-circuit-bench) and then update that branch to latest Sonobe main, while in parallel porting the r1cs-std changes to the last arkworks-rs/r1cs-std version, but probably the above will be cleaner.

In any case, happy to do any option that it's easier and less time consuming for you. Thank you very much for this improvement!!! 🙌

@yelhousni yelhousni closed this Feb 6, 2025
@yelhousni yelhousni deleted the perf/pedersen-commit branch February 6, 2025 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants