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

docs: Fix sign in definition of CURVE.n #49

Merged
merged 1 commit into from
Jan 28, 2022
Merged

Conversation

dsernst
Copy link
Contributor

@dsernst dsernst commented Jan 28, 2022

Fixes docs to correctly describe sign in Group Order definition (CURVE.n) to be +, not -.

Code was correct:

n: _2n ** BigInt(252) + BigInt('27742317777372353535851937790883648493'),

Also matches standard as described elsewhere like RFC 8032.

@dsernst
Copy link
Contributor Author

dsernst commented Jan 28, 2022

Not a big deal but do also want to note my confusion on why you call this value n.

DJB's original Ed25519 paper (https://ed25519.cr.yp.to/ed25519-20110926.pdf) and Wikipedia (https://en.wikipedia.org/wiki/EdDSA#Ed25519) both call it cursive l.

RFC 8032 (https://datatracker.ietf.org/doc/html/rfc8032#section-6) calls it L in section 5.1, and q in section 6.

@paulmillr
Copy link
Owner

@dsernst i've first developed secp256k1, and order was called n there. So i've copied for consistency. Do you know why everyone uses different symbols?

@dsernst
Copy link
Contributor Author

dsernst commented Jan 28, 2022

Do you know why everyone uses different symbols?

In my experience, cryptographers are not great at naming things 😄. So many random letters, so many techniques named after people rather than what's happening. It's all way more obtuse than ideal.

That's one thing that made your library stand out... you've done a great job keeping it reasonable, simple as possible, clean, etc. Breath of fresh air.

@paulmillr paulmillr merged commit 67d8059 into paulmillr:main Jan 28, 2022
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