Skip to content
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

precompiles: Implement EIP-2537's bls12_g1mul #994

Merged
merged 1 commit into from
Sep 11, 2024
Merged

Conversation

rodiazet
Copy link
Collaborator

@rodiazet rodiazet commented Sep 10, 2024

Implementation of the bls12_g1mul precompile: E1 affine point's multiplication from BLS12-381 curve by a scalar according to the EIP-2537 spec https://eips.ethereum.org/EIPS/eip-2537#abi-for-g1-multiplication.

Depends on #982

@rodiazet rodiazet added precompiles Related to EVM precompiles Prague Changes for Prague upgrade labels Sep 10, 2024
@chfast chfast changed the title precompiles: Implement `EIP-2537's bls12_g1mul precompiles: Implement EIP-2537's bls12_g1mul Sep 10, 2024
Copy link

codecov bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.88%. Comparing base (35b7475) to head (2a99823).
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #994      +/-   ##
==========================================
+ Coverage   93.84%   93.88%   +0.03%     
==========================================
  Files         146      146              
  Lines       15460    15485      +25     
==========================================
+ Hits        14509    14538      +29     
+ Misses        951      947       -4     
Flag Coverage Δ
eof_execution_spec_tests 17.39% <0.00%> (-0.03%) ⬇️
ethereum_tests 27.76% <0.00%> (-0.05%) ⬇️
ethereum_tests_silkpre 19.45% <0.00%> (-0.04%) ⬇️
execution_spec_tests 19.02% <67.85%> (+0.09%) ⬆️
unittests 88.98% <64.28%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
lib/evmone_precompiles/bls.cpp 100.00% <100.00%> (ø)
test/state/precompiles.cpp 87.15% <100.00%> (+2.17%) ⬆️
test/unittests/precompiles_bls_test.cpp 100.00% <100.00%> (ø)

@rodiazet rodiazet force-pushed the bls-g1-add branch 4 times, most recently from 7bbddcf to 2ca9a94 Compare September 10, 2024 10:50
Base automatically changed from bls-g1-add to master September 10, 2024 11:05
lib/evmone_precompiles/bls.cpp Outdated Show resolved Hide resolved
@@ -6,6 +6,8 @@ namespace evmone::crypto::bls
{
namespace
{
using namespace intx::literals;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed any more.

const uint8_t _y[64], const uint8_t _c[32]) noexcept
{
blst_scalar scalar;
blst_scalar_from_bendian(&scalar, _c);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder what it actually does. Because the later blst_p1_mult just uses the bytes. Are they the same bytes as the input?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answer: it seem blst expects bytes but in little-endian order.

@@ -16,4 +16,10 @@ inline constexpr auto BLS_FIELD_MODULUS =
[[nodiscard]] bool g1_add(uint8_t _rx[64], uint8_t _ry[64], const uint8_t _x0[64],
const uint8_t _y0[64], const uint8_t _x1[64], const uint8_t _y1[64]) noexcept;

/// Scalar multiplication in BLS12-381 curve group.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention this is in the G1 subgroup and that the subgroup check is performed.

blst_p1 p;
blst_p1_from_affine(&p, &*p_affine);

if (!blst_p1_in_g1(&p))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The subgroup check is also a multiplication. Do you think can we combine both multiplication for better performance?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be surprised if they implement group check just by multiplication by group order. There are planty of algorithms which make it faster. I will check the implementation of blst_p1_in_g1

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two different implementations of this check in blst. Non of them allows this kind of optimization.

lib/evmone_precompiles/bls.hpp Outdated Show resolved Hide resolved
@rodiazet rodiazet force-pushed the bls-g1-mul branch 2 times, most recently from 2f2076a to 34f483b Compare September 11, 2024 13:00
@rodiazet rodiazet merged commit 07c8a0e into master Sep 11, 2024
24 checks passed
@rodiazet rodiazet deleted the bls-g1-mul branch September 11, 2024 13:29
rodiazet added a commit that referenced this pull request Sep 13, 2024
Implementation of the bls12_g2add precompile: E2 affine points' addition
from BLS12-381 curve according to the EIP-2537 spec
https://eips.ethereum.org/EIPS/eip-2537#abi-for-g2-addition.

Depends on #994
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Prague Changes for Prague upgrade precompiles Related to EVM precompiles
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants