-
Notifications
You must be signed in to change notification settings - Fork 166
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 hash to point from RFC9380 #510
base: drandmerge
Are you sure you want to change the base?
Conversation
fc565eb
to
c04199d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, add better comments to refer to which part of the RFC you've implemented, that will make review easier.
dst.SetBytes(b[:]) | ||
} | ||
|
||
func feFromBn(dst *fieldElement, src *big.Int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same, add a comment saying what it does
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not exactly, it converts from big.Int, its fixed !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to have some tests testing both, going both directions and weird testing edge cases (all 1s, all 0s, etc.)
return dst, nil | ||
} | ||
|
||
func curve25519Elligator2(u fieldElement) (xn, xd, yn, yd fieldElement) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't this also be used for Curve25519? Why put it in Edwards25519?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it can, though our Curve25519 doesn't use fieldElement
but only big.Int
, it would make sense to put it there but it would be of no use unfortunately.
49ec0f3
to
3275d0b
Compare
3275d0b
to
45763cc
Compare
dst.SetBytes(b[:]) | ||
} | ||
|
||
func feFromBn(dst *fieldElement, src *big.Int) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be good to have some tests testing both, going both directions and weird testing edge cases (all 1s, all 0s, etc.)
9b8b1a8
to
8b5d5fe
Compare
Quality Gate failedFailed conditions |
c17b790
to
a607602
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just have one comment on expandMessageXOF
, the rest looks good to me :)
group/edwards25519/point.go
Outdated
h.Reset() | ||
return uniformBytes, nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that we can remove the last h.Reset()
, as you always reset it prior to updating its state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Its fixed, thank you very much !
Quality Gate failedFailed conditions |
Add Hash() on Ed25519