Precompiled contracts for EIP2357/BLS12 #42
Conversation
virgil-serbanuta
force-pushed
the
virgil/eip2357-precompiled
branch
3 times, most recently
from
ea99e16 to
87f6a7f
Compare
virgil-serbanuta
force-pushed
the
virgil/eip2357-precompiled
branch
from
5676a69 to
706bd60
Compare
| // TODO: implement `bls12G1Msm` as a hook, using Pippenger's algorithm (blst_p1s_mult_pippenger) | ||
| // However, note that the implementation of `g1_lincomb_fast` has the | ||
| // following comment: | ||
| // | ||
| // * @remark While this function is significantly faster than g1_lincomb_naive(), we refrain from | ||
| // * using it in security-critical places (like verification) because the blst Pippenger code has not | ||
| // * been audited. In those critical places, we prefer using g1_lincomb_naive() which is much simpler. | ||
| // | ||
| // https://github.com/ethereum/c-kzg-4844/blob/cc33b779cd3a227f51b35ce519a83cf91d81ccea/src/common/lincomb.c#L54-L56 |
There was a problem hiding this comment.
@dwightguth, could you please take a look at this and advise?
There was a problem hiding this comment.
Even if the version preferred by the client is relatively straightforward, we should still implement this as a hook using established libraries. We do not want to be responsible for the maintenance burden of ensuring a cryptographic function is secure, which is a more onerous burden than simply ensuring functional correctness as seen here.
I tested them with the following filter and they passed: |
Could you please execute the tests with your new modifications and open a PR on the |
| // TODO: implement `bls12G1Msm` as a hook, using Pippenger's algorithm (blst_p1s_mult_pippenger) | ||
| // However, note that the implementation of `g1_lincomb_fast` has the | ||
| // following comment: | ||
| // | ||
| // * @remark While this function is significantly faster than g1_lincomb_naive(), we refrain from | ||
| // * using it in security-critical places (like verification) because the blst Pippenger code has not | ||
| // * been audited. In those critical places, we prefer using g1_lincomb_naive() which is much simpler. | ||
| // | ||
| // https://github.com/ethereum/c-kzg-4844/blob/cc33b779cd3a227f51b35ce519a83cf91d81ccea/src/common/lincomb.c#L54-L56 |
There was a problem hiding this comment.
Even if the version preferred by the client is relatively straightforward, we should still implement this as a hook using established libraries. We do not want to be responsible for the maintenance burden of ensuring a cryptographic function is secure, which is a more onerous burden than simply ensuring functional correctness as seen here.
Co-authored-by: Roberto Rosmaninho <[email protected]>
Co-authored-by: Roberto Rosmaninho <[email protected]>
Co-authored-by: Roberto Rosmaninho <[email protected]>
Co-authored-by: Roberto Rosmaninho <[email protected]>
Co-authored-by: Roberto Rosmaninho <[email protected]>
virgil-serbanuta
force-pushed
the
virgil/eip2357-precompiled
branch
from
ece3d9e to
9186b88
Compare
|
@dwightguth I implemented multi-scalar-multiplication as a hook that just calls the BLST's implementation for Pippenger's algorithm. Does the PR look better now? |
Robertorosmaninho
dismissed
dwightguth’s stale review
Dismissing the review to unblock the PR from being merged, as the modification request was already done. @dwightguth, once you can, please give us your review, and if needed, Virgil will open a new PR to address it, ok?
No description provided.