-
Notifications
You must be signed in to change notification settings - Fork 494
refactor: BLS EVM precompile integration #1515
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the BLS EVM precompile integration by updating precompile gadget interfaces to accept expected outputs and return errors, and aligns tests and implementations with the new signatures.
- Changed ECAdd, ECMapTo, and MSM functions to take an
expectedparameter and perform in-circuit assertions instead of returning points. - Updated tests to use the new function signatures, switched to
sw_bls12381types in witnesses, and updated scalar fields toecc.BLS12_377. - Added error propagation (
fmt.Errorf) in place of panics and introduced a unifiedScalarMulalias in thesw_bls12381implementation.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| std/evmprecompiles/bls_test.go | Updated test circuits to match new signatures and scalar field. |
| std/evmprecompiles/11-blsg1add.go | Refactored ECAddG1BLS signature to include expected output. |
| std/evmprecompiles/12-blsg1msm.go | Renamed ECMSMG1BLS to ECG1ScalarMulSumBLS with new API and error. |
| std/evmprecompiles/13-blsg2add.go | Refactored ECAddG2BLS signature to include expected output. |
| std/evmprecompiles/14-blsg2msm.go | Renamed ECMSMG2BLS to ECG2ScalarMulSumBLS, updated logic and errors. |
| std/evmprecompiles/16-blsmaptog1.go | Updated ECMapToG1BLS to accept expected output and return error. |
| std/evmprecompiles/17-blsmaptog2.go | Updated ECMapToG2BLS to accept expected output and return error. |
| std/algebra/emulated/sw_bls12381/g2.go | Added a ScalarMul alias for the GLV-based scalar mul method. |
Comments suppressed due to low confidence (3)
std/evmprecompiles/14-blsg2msm.go:17
- The documentation tag references [BLS12_G1MSM], but this function implements the G2 MSM precompile; update the reference to [BLS12_G2MSM].
// [BLS12_G1MSM]: https://eips.ethereum.org/EIPS/eip-2537
std/evmprecompiles/13-blsg2add.go:11
- [nitpick] This function panics on NewG2 errors but does not return an error; for consistency and better error handling, consider returning an error instead of panicking.
func ECAddG2BLS(api frontend.API, P, Q, expected *sw_bls12381.G2Affine) {
std/evmprecompiles/bls_test.go:54
- The test uses ecc.BLS12_377.ScalarField(), which may not match the intended BLS12-381 scalar field for these precompiles; consider using the correct BLS12-381 scalar field (e.g., ecc.BLS12381.ScalarField()).
err := test.IsSolved(circuit, witness, ecc.BLS12_377.ScalarField())
afa27f3 to
d215974
Compare
|
@yelhousni - a bit of changes in how we call the methods from Linea side. I'd merge it as I also want to modify the open #1489 PR and don't want to mix things up too much. I'll maybe have some further changes later on as things start to settle. |
dfa4e64 to
a1b012e
Compare
yelhousni
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any problem with the new interface. Let's merge it if it helps with the glue.
Description
Corresponding downstream usage in Consensys/linea-monorepo#915. This PR modifies the interfaces for the EVM precompile gadgets for better compatibility with the glue logic.
Type of change