Conversation
There was a problem hiding this comment.
Pull request overview
This pull request represents a complete rewrite of the PLONK zero-knowledge proof verification system with significant architectural changes and dependency updates. The implementation transitions from a preliminary placeholder to a functional (though not yet fully optimized) PLONK verifier based on perturbing's plutus-plonk example, with emphasis on code clarity and proper point compression.
Changes:
- Upgraded Aiken compiler from v1.1.19 to v1.1.21 and stdlib from v2.1.0 to 3.0.0
- Completely rewrote PLONK implementation with modular structure: separate files for verifier logic, proof preparation, preinput handling, and point representations
- Introduced custom affine point types (G1Affine, G2Affine) with manual BLS12-381 compression implementation
- Reorganized tests: moved Groth16 tests to dedicated file (groth_tests.ak) and added comprehensive PLONK test suite
- Updated field arithmetic to use
field_primefrom stdlib's scalar module instead of custom constants
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| zkp/plutus.json | Updated compiler version metadata |
| zkp/aiken.toml | Upgraded compiler to v1.1.21 and stdlib to 3.0.0 |
| zkp/aiken.lock | Updated stdlib version in lock file |
| .github/workflows/continuous-integration.yml | Updated CI to use Aiken v1.1.21 |
| zkp/lib/tests/test_utils.ak | New helper for converting integers to 48-byte field elements |
| zkp/lib/tests/point_tests.ak | Comprehensive tests for G1/G2 point compression and validation |
| zkp/lib/tests/plonk_tests.ak | Full PLONK test suite with affine/projective test vectors |
| zkp/lib/tests/groth_tests.ak | Moved Groth16 tests from groth.ak to dedicated test file |
| zkp/lib/plonk/verifier.ak | Core PLONK verification logic with Fiat-Shamir challenges |
| zkp/lib/plonk/raw_affine.ak | Affine point representation and conversion utilities |
| zkp/lib/plonk/raw.ak | Projective point representation and conversion utilities |
| zkp/lib/plonk/proofs.ak | Proof preparation with challenge computation |
| zkp/lib/plonk/preinputs.ak | Verification key preparation and preprocessing |
| zkp/lib/plonk/plonk.ak | Removed old incomplete PLONK implementation |
| zkp/lib/common/blst_affine.ak | New BLS12-381 affine point compression implementation |
| zkp/lib/common/common.ak | Updated field arithmetic to use stdlib's field_prime |
| zkp/lib/bullet/bullet.ak | Updated to use field_prime instead of deprecated constants |
| zkp/lib/groth/groth.ak | Removed embedded tests (moved to groth_tests.ak) |
| README.md | Updated documentation to reflect PLONK status and implementation details |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else { | ||
| // Check that the point has the right order | ||
| let scalar_mult = g1_mul(p, bls12_381_r) | ||
| let scalar_mult = g1_mul(p, field_prime) |
There was a problem hiding this comment.
Using field_prime (the scalar field order) to check subgroup membership is incorrect. The subgroup check should multiply by the subgroup order r, not the base field prime p. In BLS12-381, the scalar field order r = 52435875175126190479447740508185965837690552500527637822603658699938581184513, while field_prime refers to the base field prime p = 4002409555221667393417789825735904156556882819939007885332058136124031650490837864442687629129015664037894272559787. This causes the subgroup check to fail because multiplying a G1 point by the base field prime does not necessarily result in the identity.
| } else { | ||
| // Check that the point has the right order | ||
| let scalar_mult = g2_mul(p, bls12_381_r) | ||
| let scalar_mult = g2_mul(p, field_prime) |
There was a problem hiding this comment.
Using field_prime (the scalar field order) to check subgroup membership is incorrect. The subgroup check should multiply by the subgroup order r, not the base field prime p. In BLS12-381, the scalar field order r = 52435875175126190479447740508185965837690552500527637822603658699938581184513, while field_prime refers to the base field prime p = 4002409555221667393417789825735904156556882819939007885332058136124031650490837864442687629129015664037894272559787. This causes the subgroup check to fail because multiplying a G2 point by the base field prime does not necessarily result in the identity.
| // I guess we don't need to decompress the generators? The aiken documentation is not clear on this. | ||
| // let g1_generator = g1.decompress(g1.generator) | ||
| // let g2_generator = g2.decompress(g2.generator) | ||
| // |
There was a problem hiding this comment.
This commented-out code with an uncertain note ("I guess we don't need to decompress the generators?") suggests confusion about the API. Either remove these comments if they're not needed, or clarify and implement them if they are necessary for correctness.
| // I guess we don't need to decompress the generators? The aiken documentation is not clear on this. | |
| // let g1_generator = g1.decompress(g1.generator) | |
| // let g2_generator = g2.decompress(g2.generator) | |
| // |
Completely rewrote the PLONK implementation. Tests are complete, but need optimization to succeed within execution budget.