feat: Groth16 Solidity contract with commitments#1063
feat: Groth16 Solidity contract with commitments#1063ivokub merged 17 commits intoConsensys:masterfrom
Conversation
ivokub
left a comment
There was a problem hiding this comment.
Looks good, thanks!
I'll try to think maybe we can omit the --commitment argument for gnark-solidity-checker. For now it works, but I'm quite certain it would haunt us in the future trying.
And also one part in the Solidity template -- I think it can be simplified a little to be more readable. Otherwise imo looks perfect. I tested locally and works end to end.
|
@ivokub I tried to address some comments you left. Thanks for the review |
This reverts commit 22e9d41.
|
I pushed some changes to simplify tests (have options modifying the hash functions for Solidity target automatically). I'll go about running the solidity checks when Consensys/gnark-solidity-checker#2 has been merged and lets see how the tests behave. |
ivokub
left a comment
There was a problem hiding this comment.
Lets wait for the CI pipeline to finish and I'll also create two new issues for tracking improvement effors, but I think the PR is now good to merge. We really appreciate the contribution and the implementation has been very good! Good work!
Description
This PR adds commitment verification to groth16 solidity verifier contract. Contract does not do foldings so uses first commitment for pairing check and uses sha256 hash as hash to field function. I added warning logs about these assumptions
Resolves: #524
Type of change
How has this been tested?
internal/backend/circuits/commit.go. Tests depends on changes at feat: add groth16 commitment verifier gnark-solidity-checker#2How has this been benchmarked?
Checklist:
golangci-lintdoes not output errors locally