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

[to upstream] Add a LookupRangeCheck trait and minor modifications in preparation for ZSA #32

Open
wants to merge 102 commits into
base: main
Choose a base branch
from

Conversation

ConstanceBeguier
Copy link

@ConstanceBeguier ConstanceBeguier commented Jul 4, 2024

This PR contains the following modifications:

  1. Introduce a LookupRangeCheck trait that provides common methods for a lookup range check.
  2. Use this new trait as a generic parameter in configs, chips and tests that are using lookup.
  3. Add tests for Lookup, ECC, Merkle, and Sinsemilla to verify that the verification key and the proof have not been modified by comparing them to those saved in a file.
  4. Add the EccPointQ enum to handle public and private points for Q, which is the initialization point for the hashing function.

To adjust existing code to these modifications, developers will just need to import halo2_gadgets::utilities::lookup_range_check::LookupRangeCheck.
This Orchard PR demonstrates how to adapt the Orchard code.

Upstream PR: zcash#823

ConstanceBeguier and others added 30 commits December 7, 2023 16:10
It is now possible to create a Point from a constant.
This functionality is required to evaluate the old nullifier.
- for non split_notes, nf_old = Extract_P([PRF^{nfOrchard}_{nk}(rho_old) + psi_nf) mod q_P] NullifierK + cm_old)
- for split notes, nf_old = Extract_P([PRF^{nfOrchard}_{nk}(rho_old) + psi_nf) mod q_P] NullifierK + cm_old + NullifierL)
Short range checks on 4 and 5 bits are now performed with only one lookup (instead of 2).
To do that, we added a column `table_short_range_tag` in the lookup table.
This new column `table_short_range_tag` contains the value
- 4 for rows used in short range check on 4 bits
- 5 for rows used in short range check on 5 bits
- 0 for rows used in short range check on 10 bits

Disable tests on i686 and code coverage in CI
It is now possible to perform a mux between two points or between two non-identity points.
`mux(choice, left, right)` will return `left` when `choice=0` and `right` when `choice=1`.
`choice` must be constrained to `{0, 1}` outside the gate.

It is no longer needed to expose `from_coordinates_unchecked`.
…oint (#22)

To share ZEC and ZSA hash computations in Orchard circuit's note commitment evaluation, we need to compute a Sinsemille hash from a private input point.
This reverts commit 23a2951.
YaoGalteland and others added 11 commits May 30, 2024 11:46
…om:QED-it/halo2 into improve-backward-compatability-without-zsa
…rcuit

Previously, the functions test_against_stored_vk and test_against_stored_proof
each generated a verification key (vk). This operation can be quite slow.
We are combining these two tests to generate the vk only once.
PallasLookupRangeCheck <- PallasLookupRC
PallasLookupRangeCheckConfig <- PallasLookupRCConfig
Copy link

@PaulLaux PaulLaux left a comment

Choose a reason for hiding this comment

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

Added some minor comments and questions

halo2_gadgets/src/ecc.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/ecc.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/sinsemilla.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/sinsemilla.rs Show resolved Hide resolved
halo2_gadgets/src/sinsemilla/chip/hash_to_point.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/utilities/lookup_range_check.rs Outdated Show resolved Hide resolved
@PaulLaux PaulLaux changed the title Add a LookupRangeCheck trait and minor modifications in preparation for ZSA [to upstream] Add a LookupRangeCheck trait and minor modifications in preparation for ZSA Jul 18, 2024
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