Skip to content

Conversation

@ec2
Copy link
Contributor

@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
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?

@ec2
Copy link
Contributor 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.

3 participants