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
Switch to even-y tiebreaker for pubkeys #192
Conversation
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.
looks good to me otherwise
bip-0340.mediawiki
Outdated
@@ -126,7 +126,7 @@ The following conventions are used, with constants as defined for [https://www.s | |||
*** Let ''y = c<sup>(p+1)/4</sup> mod p''. | |||
*** Fail if ''c ≠ y<sup>2</sup> mod p''. | |||
*** Return the unique point ''P'' such that ''x(P) = x'' and ''y(P) = y'', or fail if no such point exists. | |||
** The function ''point(x)'', where ''x'' is a 32-byte array, returns the point ''P = lift_x(int(x))''. | |||
** The function ''point(x)'', where ''x'' is a 32-byte array, returns ''P = lift_x(int(x))'' or ''P = -lift_x(int(x))'', such that ''y(P) & 1 = 0''. |
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.
nit: perhaps slightly better returns ''P = lift_x(int(x))'' if ''y(P) & 1 = 0'' and ''-P'' otherwise
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.
Mathematically this is correct but algorithmically defining point
via lift_x
somehow abuses lift_x
: the latter makes effort to get the squared-y point, but this effort is then discarded by the former anyway.
Previously point
and lift_x
were the same except for the parsing of an integer via int
. I suggest to get rid of one of them and instead have point_even_y
and point_squared_y
(or lift_x_even_y
and lift_x_squared_y
). The resulting functions should not perform the integer parsing, because everywhere else in the pseudocode we have an explicit call to 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.
by the way, I think this discussion confirms the obversation of #191 (comment)
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.
"where x
is a 32-byte array, calculates P = lift_x(int(x))
and returns P
if is_even(P)
or -P
otherwise" might read better?
Alternatively, "point(x)
returns the point P
where x(P)=x
and is_even(P)
. This may be achieved by calculating P=lift_x(x)
and returning P
if is_even(P)
and -P
otherwise" or similar might work?
Concept ACK, looks good except nits. :)
|
Saying
We only use "oddness" once when talking about choosing which R point, so doesn't seem worth worrying about to me. |
Changed things a bit. |
ACK :) |
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.
ACK
ACK |
Fixed a few more remaining mentions of "point". |
Addresses #191.
TODO: adjust reference code, but we'll probably only want to do that after addressing #190 as well.