Skip to content

Conversation

@pdobacz
Copy link
Member

@pdobacz pdobacz commented Oct 21, 2025

I'm new to the bls party, but the coverage was telling me these functions are uncovered, and the EIP told me we should dispatch to them: https://eips.ethereum.org/EIPS/eip-2537#no-dedicated-mul-call

So a quick take - is this the way to go? (bls.cpp is at 100% now)

@codecov
Copy link

codecov bot commented Oct 21, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.31%. Comparing base (8b957b3) to head (49e209b).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1340      +/-   ##
==========================================
+ Coverage   87.27%   87.31%   +0.04%     
==========================================
  Files         169      169              
  Lines       25042    25058      +16     
  Branches     4118     4122       +4     
==========================================
+ Hits        21855    21879      +24     
+ Misses        531      527       -4     
+ Partials     2656     2652       -4     
Flag Coverage Δ
eest-develop 80.55% <100.00%> (+0.53%) ⬆️
eest-develop-gmp 16.12% <100.00%> (+0.19%) ⬆️
eest-legacy 11.18% <0.00%> (-0.01%) ⬇️
eest-legacy-silkpre 17.60% <0.00%> (-0.02%) ⬇️
evmone-unittests 83.75% <0.00%> (-0.06%) ⬇️

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

Components Coverage Δ
core 93.74% <100.00%> (+0.12%) ⬆️
tooling 88.01% <ø> (ø)
tests 84.16% <ø> (ø)
Files with missing lines Coverage Δ
test/state/precompiles.cpp 95.91% <100.00%> (+0.13%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pdobacz pdobacz requested a review from chfast October 21, 2025 11:31
Copy link
Member

@chfast chfast left a comment

Choose a reason for hiding this comment

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

I did something similar in #1046 for performance reasons, but there are additional details about BLST which complicates this. I think we should just remove procedures for single point multiplication if not used anywhere else.

@pdobacz
Copy link
Member Author

pdobacz commented Oct 21, 2025

I did something similar in #1046 for performance reasons, but there are additional details about BLST which complicates this. I think we should just remove procedures for single point multiplication if not used anywhere else.

Thx! For posterity, can we make note here of the complications? I couldn't find by quickly scanning the linked content, and the EIP is quite explicit about its recommendations.

OTOH: if the EIP says what it says, and we don't have a dedicated function in evmone, and yet we're measuring and reasoning about coverage using evmone, this approach becomes a bit inadequate - we could be missing coverage for the single multiplication case and not see it, while other clients might have been running (uncovered) single multiplication code. As a minimum, should we at least have a trivial input_size if, like in this PR, but both legs dispatching to the MSMs?

Copy link
Member

@chfast chfast left a comment

Choose a reason for hiding this comment

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

Fine to keep it.

@pdobacz pdobacz enabled auto-merge (squash) October 21, 2025 17:27
@pdobacz pdobacz merged commit c92e869 into master Oct 21, 2025
23 checks passed
@pdobacz pdobacz deleted the use-gx-mul branch October 21, 2025 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants