Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

precompiles: Implement secp256k1 ECDSA recovery using EVMMAX #688

Merged
merged 11 commits into from
Oct 2, 2023

Conversation

chfast
Copy link
Member

@chfast chfast commented Aug 18, 2023

Implement Elliptic Curve Digital Signature Algorithm (ECDSA)
Public Key Recovery algorithm for secp256k1 curve
using EVMMAX primitives.

This can be used to provide ecrecovery EVM precompile
but also to verify signatures in Ethereum transactions.

This work has been done at ETHPrague Hackathon
https://devfolio.co/projects/evmmax-ecrecovery-bd49

@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #688 (7f0d0e0) into master (78ad5fe) will increase coverage by 0.13%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #688      +/-   ##
==========================================
+ Coverage   97.60%   97.73%   +0.13%     
==========================================
  Files          95       98       +3     
  Lines        8645     9235     +590     
==========================================
+ Hits         8438     9026     +588     
- Misses        207      209       +2     
Flag Coverage Δ
blockchaintests 62.37% <ø> (ø)
statetests 62.23% <0.00%> (-11.24%) ⬇️
statetests-silkpre 26.56% <70.24%> (+3.91%) ⬆️
unittests 95.64% <96.61%> (+0.06%) ⬆️

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

Files Coverage Δ
lib/evmone_precompiles/ecc.hpp 100.00% <100.00%> (ø)
lib/evmone_precompiles/secp256k1.cpp 100.00% <100.00%> (ø)
test/state/precompiles.cpp 86.84% <100.00%> (+2.63%) ⬆️
test/unittests/evmmax_secp256k1_test.cpp 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@chfast chfast requested review from axic, hugo-dc, gumb0 and rodiazet August 18, 2023 13:28
return z;
}

uint256 scalar_inv(const ModArith<uint256>& m, const uint256& x) noexcept
Copy link
Member

Choose a reason for hiding this comment

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

Can we add a comment (or extend the one below) to include a reference to the expmod.tmpl file?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. The commit messages have all the information how to generate this code, but I will add this information here too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added detailed instruction how this was generated.

TEST(evmmax, secp256k1_calculate_u1)
{
// u1 = -zr^(-1)
const auto z = 0x31d6fb860f6d12cee6e5b640646089bd5883d586e43de3dedc75695c11ac2da9_u256;
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be possible to document the source of the hardcoded values throughout this file?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we just dumped these intermediate value from some other implementation or used PyEC to compute this example. At this point we may just remove these tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, will remove the intermediate result tests, but we still could document the values from the add and mul tests (which are more like end-result tests). If you don't have it handy, I can try to recreate, unless you think this is overdoing.

@pdobacz pdobacz force-pushed the evmmax_secp256k1 branch 2 times, most recently from fe62d43 to 9c39559 Compare September 26, 2023 18:47
Add new library `evmone_precompiles` with the intention to contain
EVMMAX-based implementations of Ethereum precompiles.
@pdobacz pdobacz force-pushed the evmmax_secp256k1 branch 2 times, most recently from 0eef2f0 to aeb740b Compare September 28, 2023 10:58
chfast and others added 9 commits September 29, 2023 18:33
Add generic procedures for point arithmetic on Elliptic Curves.

Co-authored-by: rodiazet <rodiazet@ethereum.org>
Co-authored-by: pdobacz <5735525+pdobacz@users.noreply.github.com>
Generated by addchain (https://github.com/mmcloughlin/addchain).
addchain search 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffefffffc2d > secp256k1_field_inv.acc
addchain gen -tmpl expmod.tmpl secp256k1_field_inv.acc > secp256k1_field_inv.cpp
Main part generated by addchain (https://github.com/mmcloughlin/addchain).
addchain search 0x3fffffffffffffffffffffffffffffffffffffffffffffffffffffffbfffff0c > secp256k1_sqrt.acc
addchain gen -tmpl expmod.tmpl secp256k1_sqrt.acc > secp256k1_sqrt.cpp
Generated by addchain (https://github.com/mmcloughlin/addchain).
addchain search 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd036413f > secp256k1_scalar_inv.acc
addchain gen -tmpl expmod.tmpl secp256k1_scalar_inv.acc > secp256k1_scalar_inv.cpp

Co-authored-by: pdobacz <5735525+pdobacz@users.noreply.github.com>
Calculate y coordinate of a secp256k1 point
having x coordinate and y parity.

Co-authored-by: Paweł Bylica <pawel@ethereum.org>
Add implementation of procedure that converts EC Point (uncompressed
public key) to Ethereum address by Keccak hash.

Co-authored-by: Paweł Bylica <pawel@ethereum.org>
Implement Elliptic Curve Digital Signature Algorithm (ECDSA)
Public Key Recovery algorithm for secp256k1 curve
using EVMMAX primitives.

This can be used to provide ecrecovery EVM precompile
but also to verify signatures in Ethereum transactions.

This work has been done at ETHPrague Hackathon
https://devfolio.co/projects/evmmax-ecrecovery-bd49

Co-authored-by: Alex Beregszaszi <alex@rtfs.hu>
Co-authored-by: Andrei Maiboroda <andrei@ethereum.org>
Co-authored-by: rodiazet <rodiazet@ethereum.org>
Co-authored-by: Hugo De La Cruz <jhugodc@gmail.com>
Co-authored-by: pdobacz <5735525+pdobacz@users.noreply.github.com>
Use EVMMAX-based secp256k1 ecrecovery implementation for the precompile.

Co-authored-by: Paweł Bylica <pawel@ethereum.org>
@chfast chfast merged commit 099a229 into master Oct 2, 2023
@chfast chfast deleted the evmmax_secp256k1 branch October 2, 2023 10:10
@chfast chfast mentioned this pull request Nov 24, 2023
12 tasks
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants