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.

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

Add ecdsa support to signer #1052

Closed
wants to merge 3 commits into from
Closed

Conversation

LGLO
Copy link
Contributor

@LGLO LGLO commented Jul 5, 2023

This adds support for ECDSA in signer, so it allows to submit transactions to nodes were ECDSA is used as blocks signing scheme.

If there is any interest, I'm happy to work on this PR, please let me know if:

  • message hashing algorithm should be parametrized (instead of hardcoded blake2b)
  • some common parts, that are copy-paste from signer::sr25519 should be extracted (to utils.rs ?)
  • example should stay (I'm not convinced there is much added value in it)

@LGLO LGLO requested a review from a team as a code owner July 5, 2023 10:28
@cla-bot-2021
Copy link

cla-bot-2021 bot commented Jul 5, 2023

User @LGLO, please sign the CLA here.

@jsdw
Copy link
Collaborator

jsdw commented Jul 10, 2023

Ooh thank you for your contribution; it'd be great to get ECDSA support added!

I'll have a more thorough look over this in the next couple of days!

Offhand, the example can probably go for now (or alternately be added to the book alongside the sr25519 example; all examples are included somewhere in the book). Also, we have no real Nix knowledge on the team and so the flake files should go as well (we wouldn't be able to keep them uptodate or whatever so would rather keep such things out of the repo). The implementation itself looks good though; I'll have a think about code reuse but for now I think it's all good on that front! Running cargo fmt and cargo clippy --fix might remove the other CI issues :)

@jsdw jsdw mentioned this pull request Jul 12, 2023
@jsdw
Copy link
Collaborator

jsdw commented Jul 12, 2023

I'll fix up the issues etc in #1064 and get this merged (I haven't got a good setup for pushing commits to fork branches which is the only reason I made a new one); thankyou for your awesome contribution :)

@jsdw jsdw closed this Jul 12, 2023
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.

2 participants