halo2 icon indicating copy to clipboard operation
halo2 copied to clipboard

Commitment scheme abstraction

Open kilic opened this issue 2 years ago • 4 comments

Having pse/halo2curves which reimplements pse/pairing with upstream traits and also reexports zcash/pasta_curves it is now possible to keep original commitment scheme rather than replacing it.

Also since we won't have to deal with mimicking traits in pse/pasta_curves at pse/pairing there must be less diffs in this fork and won't require any maintenance effort regarding those traits.

This PR introduces an abstraction technique for commitment schemes. It keeps the original IPA and adds two KZG variants and allows circuit developer to specify a commitment scheme for a specific circuit without conditional compilation.

Regarding the changes at constraint system side most of them are for boundary definitions of functions which mostly specifies the commitment scheme. I don't think this abstraction change would make its way to the upstream. However it can be a discussion base for a working group if idea is found useful somehow.

For further rebasing and maintenance concerns:

mod poly:

30 files changed, 3365 insertions(+), 1411 deletions(-)

This is admittedly a huge diff. However current replacement approach change for mod poly is also not small.

Integration for halo2_proofs except mod poly:

35 files changed, 623 insertions(+), 578 deletions(-)

There are some formatting/folding related differences that should reduce sense of this diff. There is also a ~100 liner block diff w/o an actual change. I think it is for an unrecognized line shift.

Integration for halo2_gadgets:

37 files changed, 117 insertions(+), 88 deletions(-)

Which looks ok.

kilic avatar Jun 02 '22 15:06 kilic

@kilic will take a look into this.

I'll try to see if there's a way on which we can do the same work you did here but resulting in less conflicts when merging upstream changes.

I'll need a day or two as I'll parallelize this with my keccak work :)

CPerezz avatar Jun 06 '22 09:06 CPerezz

Hey @kilic why did you rebase upstream head here??? I thought this was being done in another PR.

See: #64

CPerezz avatar Jun 16 '22 07:06 CPerezz

@CPerezz

I thought In the end this PR should be on top of recent upstream and also under other changes that we introduce? It was already started 3 or 5 commits behind of the current upstream head.

kilic avatar Jun 16 '22 12:06 kilic

@kilic makes sense to me. But we should ping @NoCtrlZ since I think is working on it.

CPerezz avatar Jun 16 '22 12:06 CPerezz

I belive this can be closed right @kilic @han0110 ??

CPerezz avatar Jan 16 '23 23:01 CPerezz