Skip to content

Conversation

@chfast
Copy link
Member

@chfast chfast commented Sep 11, 2025

Replace using of projective coordinates for elliptic curve point to Jacobian coordinates. Jacobian formulas are faster, easier to use and some of the furmulas are independent of the a parameter of a curve (should be handy for implementing secp256r1).

@chfast chfast requested a review from rodiazet September 11, 2025 14:12
const auto s2 = y2 * z1z1z1;
const auto h = u2 - u1;
const auto r = s2 - s1;
assert(r != 0); // TODO: Untested.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is actually impossible. I will have a follow up PR which detects equal points in more efficient way.

@chfast
Copy link
Member Author

chfast commented Sep 11, 2025

Benchmarks:

precompile<PrecompileId::ecrecover, evmmax_cpp>_mean                  -0.1681         -0.1679        236316        196602        236222        196555
precompile<PrecompileId::ecadd, evmmax_cpp>_mean                      +0.0008         +0.0008          1279          1280          1278          1279
precompile<PrecompileId::ecmul, evmmax_cpp>_mean                      -0.1980         -0.1981        107387         86121        107365         86100
precompile<PrecompileId::ecpairing, evmmax_cpp>_mean                  +0.0037         +0.0038       1808361       1815120       1807932       1814780
OVERALL_GEOMEAN                                                       -0.0952         -0.0952             0             0             0             0

Copy link
Member

@rodiazet rodiazet left a comment

Choose a reason for hiding this comment

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

LGTM. Add one comment

z3 = z3 + t0;
const auto z1z1 = z1 * z1;
const auto u2 = x2 * z1z1;
const auto t0 = z1 * z1z1;
Copy link
Member

Choose a reason for hiding this comment

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

why not z1z1z1 but t0?

Copy link
Member Author

Choose a reason for hiding this comment

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

@codecov
Copy link

codecov bot commented Sep 11, 2025

Codecov Report

❌ Patch coverage is 98.14815% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.13%. Comparing base (c271408) to head (5f83379).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
lib/evmone_precompiles/ecc.hpp 98.01% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1312      +/-   ##
==========================================
- Coverage   87.15%   87.13%   -0.02%     
==========================================
  Files         167      167              
  Lines       24761    24740      -21     
  Branches     4058     4064       +6     
==========================================
- Hits        21581    21558      -23     
- Misses        537      538       +1     
- Partials     2643     2644       +1     
Flag Coverage Δ
eest-develop 17.86% <5.55%> (+0.02%) ⬆️
eest-develop-gmp 15.89% <5.55%> (+0.02%) ⬆️
eest-fusaka 13.40% <0.00%> (+0.01%) ⬆️
eest-legacy 11.13% <0.00%> (+<0.01%) ⬆️
eest-legacy-silkpre 17.65% <93.51%> (-0.10%) ⬇️
eest-static 18.68% <93.51%> (-0.10%) ⬇️
evmone-unittests 83.71% <98.14%> (-0.03%) ⬇️

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

Components Coverage Δ
core 93.26% <98.14%> (-0.05%) ⬇️
tooling 88.35% <ø> (ø)
tests 84.12% <ø> (ø)
Files with missing lines Coverage Δ
lib/evmone_precompiles/bn254.cpp 100.00% <100.00%> (ø)
lib/evmone_precompiles/secp256k1.cpp 100.00% <100.00%> (ø)
lib/evmone_precompiles/ecc.hpp 97.12% <98.01%> (-1.14%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Replace using of projective coordinates for elliptic curve point to
Jacobian coordinates. Jacobian formulas are faster, easier to use
and some of the furmulas are independent of the a parameter of a curve
(should be handy for implementing secp256r1).
@chfast chfast merged commit d3bda06 into master Sep 11, 2025
21 of 22 checks passed
@chfast chfast deleted the ecc_jac branch September 11, 2025 14:59
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