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.

Sign up for GitHub

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

Jump to bottom

OrchardZSA backward compatability using traits #31

Merged
merged 123 commits into from
Aug 8, 2024
Merged

OrchardZSA backward compatability using traits #31

merged 123 commits into from
Aug 8, 2024

Conversation

YaoJGalteland
Copy link

@YaoJGalteland YaoJGalteland commented Jun 3, 2024 •
edited by ConstanceBeguier
Loading

This PR contains the following modifications:

  • 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.
  • Introduce a LookupRangeCheck trait that provides common methods for a lookup range check.
  • Use this new trait as a generic parameter in configs, chips and tests that are using lookup.
  • Create a new Lookup chip which is optimized for 4, 5 and 10-bit range check and add tests for this new chip.
  • Add init_from_private_point parameter in SinsemillaConfig. If this parameter is set to false, the SinsemillaChip is the same as the current SinsemillaChip used in vanilla circuit. Otherwise, the SinsemillaChip is modified to support hash from private point.

YaoGalteland added 30 commits April 23, 2024 22:31
add initial doc
80dfd60
clean up code
d3a6f42
remove cfg[test]
86bd33f
remove generator table trait, fix hash_to_point
989f29b
update code for hash
507a53c
fmt
2e31e97
fix run tests error
b02478a
minor update
e6d1dbe
test error
23a2951
Revert "test error"
301e7d8 
This reverts commit 23a2951.
re-naming structs and functions
c324006
fix complex type
af29df4
add round trip test to verify that the pinned verification key (repre…
1ea04bb 
…senting the circuit) is as expected
update serialized_proof_test_case
684c13e
update
a25a6ad
update short lookup test
42f1b0a
cargo fmt
2b89fae
fix Clippy error
8552c97
clean up code
f35cbe9
add initial files
f65d5a3
minor update
7e50d0b
remove pub(crate) in ecc
58f8b3f
add a line after cargo.toml
738eabe
add a line after cargo.toml
0fb6dd4
slide lines in base_field_elem
e2c9c30
slide lines in base_field_elem
5322c81
adjuct function orders for configure in sinsemilla/chip
46773e3
adjuct function orders for hash_message
3621e0e
adjuct function orders for hash_message
a30596b
cleanup code
b3cb453
@ConstanceBeguier
Rename Sinsemilla/MerkleChipOptimized
cd4d43a 
Sinsemilla45BChip         <- SinsemillaChipOptimized
Sinsemilla45BInstructions <- SinsemillaInstructionsOptimized
Merkle45BChip             <- MerkleChipOptimized
@ConstanceBeguier
Put together some imports
18e3e0d
@ConstanceBeguier
Fix LookupangeCheck loading in Sinsemilla
d6bcf3b 
Previously, the SinsemillaChip always loaded the lookup range check
without tag column and Sinsemilla45BChip always loaded the lookup
range check with tag column.
Now, we load lookup range check with or without tag according to the
lookup type (LookupRangeCheckConfig or LookupRangeCheck45BConfig).
@ConstanceBeguier
Fix Sinsemilla45BChip and Merkle45BChip
b5d8916 
It is now possible to use Sinsemilla/Merkle45BChip with non optimized lookup.
@ConstanceBeguier
Rename Sinsemilla/Merkle45BChip
1fbc7de 
SinsemillaWithPrivateInitChip <- Sinsemilla45BChip
SinsemillaWithPrivateInitInstructions <- Sinsemilla45BInstructions
MerkleWithPrivateInitChip <- Merkle45BChip
@ConstanceBeguier
Clean lookup range check tests
37e9380
@ConstanceBeguier
Rename some layouter images
eda965f
@ConstanceBeguier
Update some comments
6af9ebf
@ConstanceBeguier
Simplify Sinsemilla/Merkle chips thanks to enable_hash_from_private_p…
2021f5e 
…oint variable

We add enable_hash_from_private_point into SinsemillaChip in order to
put together SinsemillaChip and SinsemillaWithPrivateInitChip.
If enable_hash_from_private_point is set, it is possible to hash from
a private point. Otherwise, it is not allowed.
That allows also to put together MerkleChip and
MerkleWithPrivateInitChip.
@ConstanceBeguier
Refactoring
0d2512a
@ConstanceBeguier
Refactor
e2c9e93
@ConstanceBeguier
Remove MerkleSinsemillaInstructions
7f5c0ba
@ConstanceBeguier
Restore code coverage
41c7d2e
@ConstanceBeguier
Refactor
9c29f27
@ConstanceBeguier
Remove redundancy in Merkle tests
714fff0
@ConstanceBeguier
Rename MyCircuit into MyMagnitudeSignCircuit on fixed_mul/short tests
70af809
@ConstanceBeguier
Refactor
68d65e0
@ConstanceBeguier
Make cond_swap_config public in merkle_config
a8832c9
@ConstanceBeguier
Take into account Pablo's review
8a2cbee
PaulLaux
PaulLaux reviewed Aug 1, 2024
View reviewed changes
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.

Good overall, added some minor comments. In addition:

  • naming for 4_5b and 4_5B and 4-5b should be consistent (including file names).
  • some concern about the diif size for halo2_gadgets/src/utilities/lookup_range_check.rs

halo2_gadgets/src/ecc.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/ecc/chip/mul_fixed/short.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/sinsemilla.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/sinsemilla/chip.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/sinsemilla/chip.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/ecc.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/ecc/chip/mul_fixed/short.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/sinsemilla/merkle.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/utilities/cond_swap.rs Outdated Show resolved Hide resolved
halo2_gadgets/src/utilities/lookup_range_check.rs Show resolved Hide resolved
PaulLaux
PaulLaux approved these changes Aug 7, 2024
View reviewed changes
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.

Very nice

halo2_gadgets/src/sinsemilla/chip.rs Outdated Show resolved Hide resolved
@PaulLaux
Copy link

PaulLaux commented Aug 7, 2024

can you clarify this: #31 (comment)

@ConstanceBeguier ConstanceBeguier merged commit 1195c9a into zsa1 Aug 8, 2024
42 checks passed
@PaulLaux PaulLaux mentioned this pull request Aug 11, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ConstanceBeguier ConstanceBeguier ConstanceBeguier left review comments

@PaulLaux PaulLaux PaulLaux approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@YaoJGalteland @PaulLaux @ConstanceBeguier @dmidem