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

SSZ Multiproof #57

Open
wants to merge 37 commits into
base: develop
Choose a base branch
from
Open

SSZ Multiproof #57

wants to merge 37 commits into from

Conversation

ec2
Copy link
Member

@ec2 ec2 commented Jan 23, 2024

TODO:
[x] Clean up some duplicate code
[x] Consider removing some of the preprocessor tests which only test individual circuits
[x] Fix spec tests

This PR reduces the amount of advice cells being used. We merklize a whole beacon header in circuit when what we really want is just a merkle proof for a couple specific fields.
Before:

read params from ./params/kzg_bn254_21.srs
Gate Chip | Phase 0: 11535114 advice cells
Total 2059 fixed cells
Total range check advice cells to lookup per phase: [1222736, 0, 0]
Gate Chip | Phase 0: 1599532 advice cells
Total 1795 fixed cells
Total range check advice cells to lookup per phase: [4096, 0, 0]
test tests::test_both_circuit_sepolia has been running for over 60 seconds
test tests::test_both_circuit_sepolia ... ok

After:

read params from ./params/kzg_bn254_21.srs
Gate Chip | Phase 0: 11182651 advice cells
Total 2035 fixed cells
Total range check advice cells to lookup per phase: [1188896, 0, 0]
Gate Chip | Phase 0: 1598616 advice cells
Total 1767 fixed cells
Total range check advice cells to lookup per phase: [4096, 0, 0]

@ec2 ec2 marked this pull request as ready for review January 24, 2024 06:48
lightclient-circuits/src/committee_update_circuit.rs Outdated Show resolved Hide resolved
Comment on lines 91 to 94
.as_ref()
.iter()
.map(|v| builder.main().load_witness(F::from(*v as u64)))
.collect_vec();
Copy link
Member

Choose a reason for hiding this comment

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

we have this in 5 or more places now (incl step circuit), might be worth putting this in a util method for this?

lightclient-circuits/src/ssz_merkle.rs Outdated Show resolved Hide resolved
lightclient-circuits/src/ssz_merkle.rs Outdated Show resolved Hide resolved
lightclient-circuits/src/committee_update_circuit.rs Outdated Show resolved Hide resolved
preprocessor/src/rotation.rs Outdated Show resolved Hide resolved
preprocessor/src/rotation.rs Outdated Show resolved Hide resolved
preprocessor/src/step.rs Outdated Show resolved Hide resolved
preprocessor/src/step.rs Outdated Show resolved Hide resolved
test-utils/src/lib.rs Outdated Show resolved Hide resolved
@ec2
Copy link
Member Author

ec2 commented Feb 2, 2024

I've removed the patch to halo2curves in the PR as well :D

@ec2 ec2 requested a review from nulltea February 2, 2024 07:50
@nulltea nulltea changed the base branch from main to develop March 5, 2024 13:33
Copy link
Member

@nulltea nulltea left a comment

Choose a reason for hiding this comment

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

My point re. code repetition still stands. Otherwise LGMT

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.

2 participants