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

feat: fix v computation in ECDSA signature #385

Merged
merged 5 commits into from
Apr 26, 2023
Merged

feat: fix v computation in ECDSA signature #385

merged 5 commits into from
Apr 26, 2023

Conversation

ivokub
Copy link
Collaborator

@ivokub ivokub commented Apr 21, 2023

I referred to other implementations to understand how v is computed when creating an ECDSA signature. I had assumed that v gives if y coordinate of the ephemeral point is small or large (e.h. bigger than half the order). But due to different encoding order, it was actually the oddity of y.

This is confirmed also in Ethereum Yellow paper (Appendix F) and makes also a lot more sense.

@ivokub ivokub added the bug Something isn't working label Apr 21, 2023
@ivokub ivokub added this to the v0.10.0 milestone Apr 21, 2023
@ivokub ivokub requested a review from yelhousni April 21, 2023 08:33
@ivokub ivokub self-assigned this Apr 21, 2023
Copy link
Collaborator

@yelhousni yelhousni left a comment

Choose a reason for hiding this comment

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

Ok makes sense!
image

@ivokub ivokub merged commit 2a781ae into develop Apr 26, 2023
6 checks passed
@ivokub ivokub deleted the fix/ecdsa-v branch April 26, 2023 09:00
p4u pushed a commit to vocdoni/gnark-crypto that referenced this pull request May 4, 2023
* fix: first ecdsa v bit is oddity

* fix: recover oddity of public key y

* chore: generate

* fix: add check for s

* chore: generate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants