8000 feat(crypto): adding `secp256k1` with Ethereum format by sergio-mena · Pull Request #2534 · cometbft/cometbft · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(crypto): adding secp256k1 with Ethereum format #2534

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

Merged
merged 38 commits into from
Dec 23, 2024

Conversation

sergio-mena
Copy link
Contributor
@sergio-mena sergio-mena commented Mar 7, 2024

Added support for the secp256k1-eth crypto codec.
This includes functionality for generating and verifying
Ethereum-compatible signatures using the secp256k1 curve.

Post Merge follow up


PR checklist

  • Tests written/updated
  • Changelog entry added in .changelog (we use unclog to manage our changelog)
  • Updated relevant documentation (docs/ or spec/) and code comments
  • Title follows the Conventional Commits spec

@sergio-mena sergio-mena added crypto Cryptography-related wip Work in progress labels Mar 7, 2024
@sergio-mena sergio-mena self-assigned this Mar 7, 2024
@adizere adizere modified the milestones: 2024-Q2, 2024-Q3 Jul 18, 2024
github-merge-queue bot pushed a commit that referenced this pull request Aug 6, 2024
… and thread it through the code (#3517)

Contributes to #2424

Precondition for #2534

This PR is inspired in the core idea of #2539, by @tac0turtle, whereby
we make `privval` more flexible to generate privval files in all the
curves we support in CometBFT natively.

The flag `key-type`, which was only part of command
`gen-validator`before this PR, is now part of all commands that _may_
generate priv validator keys (`start`, `unsafe-reset-all`,
`unsafe-reset-priv-validator`, `init`).

Importantly, node key, and node key file generation is still hard-coded
to `ed25519` (no need to change it for the time being).

Still TODO:
* Add unit tests at various places to test the different key types

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
mergify bot pushed a commit that referenced this pull request Aug 6, 2024
… and thread it through the code (#3517)

Contributes to #2424

Precondition for #2534

This PR is inspired in the core idea of #2539, by @tac0turtle, whereby
we make `privval` more flexible to generate privval files in all the
curves we support in CometBFT natively.

The flag `key-type`, which was only part of command
`gen-validator`before this PR, is now part of all commands that _may_
generate priv validator keys (`start`, `unsafe-reset-all`,
`unsafe-reset-priv-validator`, `init`).

Importantly, node key, and node key file generation is still hard-coded
to `ed25519` (no need to change it for the time being).

Still TODO:
* Add unit tests at various places to test the different key types

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec

---------

Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
(cherry picked from commit bd06fec)

# Conflicts:
#	cmd/cometbft/commands/gen_validator.go
#	node/errors.go
#	node/node_test.go
#	privval/file.go
melekes added a commit that referenced this pull request Aug 6, 2024
… and thread it through the code (backport #3517) (#3630)

Contributes to #2424

Precondition for #2534

This PR is inspired in the core idea of #2539, by @tac0turtle, whereby
we make `privval` more flexible to generate privval files in all the
curves we support in CometBFT natively.

The flag `key-type`, which was only part of command
`gen-validator`before this PR, is now part of all commands that _may_
generate priv validator keys (`start`, `unsafe-reset-all`,
`unsafe-reset-priv-validator`, `init`).

Importantly, node key, and node key file generation is still hard-coded
to `ed25519` (no need to change it for the time being).

Still TODO:
* Add unit tests at various places to test the different key types

---

#### PR checklist

- [x] Tests written/updated
- [x] Changelog entry added in `.changelog` (we use
[unclog](https://github.com/informalsystems/unclog) to manage our
changelog)
- [x] Updated relevant documentation (`docs/` or `spec/`) and code
comments
- [x] Title follows the [Conventional
Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec
<hr>This is an automatic backport of pull request #3517 done by
[Mergify](https://mergify.com).

---------

Co-authored-by: Sergio Mena <sergio@informal.systems>
Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
@bermuell bermuell marked this pull request as ready for review December 19, 2024 10:30
@bermuell bermuell requested review from a team as code owners December 19, 2024 10:30
Copy link
Contributor Author
@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

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

LGTM. Only minor comments.
Thanks a lot @bermuell for taking this over the finish line! 🚀

Copy link
Contributor
@jmalicevic jmalicevic left a comment

Choose a reason for hiding this comment

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

This PR was implemented by @bermuell and approving based on feedback from @sergio-mena and a pass over it myself. Thank you for this!

Copy link
Contributor
@melekes melekes left a comment

Choose a reason for hiding this comment

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

Thanks @sergio-mena and @bermuell 👍

@melekes melekes removed the wip Work in progress label Dec 20, 2024
@jmalicevic jmalicevic added this pull request to the merge queue Dec 23, 2024
Merged via the queue into main with commit d95e35a Dec 23, 2024
36 checks passed
@jmalicevic jmalicevic deleted the sergio/secp256k1-eth branch December 23, 2024 16:03
github-merge-queue bot pushed a commit that referenced this pull request Dec 25, 2024
Follow-up to #2534

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Cryptography-related
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants
0