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.

Sign up for GitHub

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

Jump to bottom

evmmax: Implement ecadd and ecmul bn254 precompiles #716

Merged
merged 1 commit into from
Nov 9, 2023
Merged

evmmax: Implement ecadd and ecmul bn254 precompiles #716

merged 1 commit into from
Nov 9, 2023

Conversation

rodiazet
Copy link
Collaborator

@rodiazet rodiazet commented Oct 4, 2023

  • Implement ecadd and ecmul precompiles using evmmax API.
  • Add unit tests.
  • Enable tests in CI

@rodiazet rodiazet requested review from pdobacz and chfast October 4, 2023 11:13
@codecov
Copy link

codecov bot commented Oct 4, 2023 •
edited
Loading

Codecov Report

Merging #716 (b0948bc) into master (8155010) will decrease coverage by 0.03%.
The diff coverage is 98.38%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #716      +/-   ##
==========================================
- Coverage   97.71%   97.69%   -0.03%     
==========================================
  Files         102      105       +3     
  Lines        9417     9665     +248     
==========================================
+ Hits         9202     9442     +240     
- Misses        215      223       +8
Flag Coverage Δ
blockchaintests 62.23% <ø> (ø)
statetests 60.65% <0.00%> (-4.06%) ⬇️
statetests-silkpre 27.51% <79.83%> (+1.45%) ⬆️
unittests 95.48% <86.69%> (-0.24%) ⬇️

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

Files Coverage Δ
lib/evmone_precompiles/bn254.cpp 100.00% <100.00%> (ø)
test/unittests/evmmax_bn254_add_test.cpp 100.00% <100.00%> (ø)
test/unittests/evmmax_bn254_mul_test.cpp 100.00% <100.00%> (ø)
test/state/precompiles.cpp 87.07% <87.87%> (+0.23%) ⬆️

... and 1 file with indirect coverage changes

pdobacz
pdobacz reviewed Oct 4, 2023
View reviewed changes
Copy link
Collaborator

@pdobacz pdobacz left a comment

Choose a reason for hiding this comment

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

👍 Just some suggestions to make conventions consistent with secp256k1 code

lib/evmone_precompiles/bn254.cpp Show resolved Hide resolved
lib/evmone_precompiles/bn254.hpp Outdated Show resolved Hide resolved
lib/evmone_precompiles/bn254.cpp Outdated Show resolved Hide resolved
@rodiazet rodiazet force-pushed the bn254-add-mul branch 2 times, most recently from cfd76da to e1dc8b2 Compare October 5, 2023 10:21
@rodiazet rodiazet requested a review from pdobacz October 5, 2023 10:24
pdobacz
pdobacz approved these changes Oct 5, 2023
View reviewed changes
Copy link
Collaborator

@pdobacz pdobacz left a comment

Choose a reason for hiding this comment

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

Thank you for the updates, LGTM but would suggest another pair of eyes here

@chfast @rodiazet
evmmax: Implement ecadd and ecmul bn254 precompiles
b0948bc 
Implement ecadd and ecmul precompiles for bn254 curve
using EVMMAX-like primitives.

Co-authored-by: rodiazet <rodiazet@ethereum.org>
@chfast chfast changed the title Implement ecadd and ecmul for bn254 curve points evmmax: Implement ecadd and ecmul bn254 precompiles Nov 9, 2023
@chfast chfast merged commit 9e9681d into master Nov 9, 2023
2 of 3 checks passed
@chfast chfast deleted the bn254-add-mul branch November 9, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pdobacz pdobacz pdobacz approved these changes

@chfast chfast chfast approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rodiazet @chfast @pdobacz