crypto.secp256k1.curve: clean up code and tests #165
Conversation
| def isJacobianOnS256Curve(x, y, z): | ||
| """ | ||
| isJacobianOnS256Curve returns boolean if the point (x,y,z) is on the | ||
| secp256k1 curve. | ||
| Elliptic curve equation for secp256k1 is: y^2 = x^3 + 7 | ||
| In Jacobian coordinates, Y = y/z^3 and X = x/z^2 | ||
| Thus: | ||
| (y/z^3)^2 = (x/z^2)^3 + 7 | ||
| y^2/z^6 = x^3/z^6 + 7 | ||
| y^2 = x^3 + 7*z^6 | ||
| """ | ||
| fv = FieldVal | ||
| y2, z2, x3, result = fv(), fv(), fv(), fv() | ||
| y2.squareVal(y).normalize() | ||
| z2.squareVal(z) | ||
| x3.squareVal(x).mul(x) | ||
| result.squareVal(z2).mul(z2).mulInt(7).add(x3).normalize() | ||
| return y2.equals(result) |
There was a problem hiding this comment.
Moving this to the test file, since it's only used there.
| # 16 instead of 1024 because CPU-intensive. | ||
| for _ in range(16): | ||
| for _ in range(1024): |
There was a problem hiding this comment.
I had lowered this down because this test was too slow, but the optimization in #160 allows us to put it back as it was.
| be. NAF is convenient in that on average, only 1/3rd of its values are | ||
| non-zero. This is algorithm 3.30 from [GECC]. | ||
| NAF takes a positive integer k and returns the Non-Adjacent Form (NAF) as | ||
| two byte slices. The first is where 1s will be. The second is where -1s |
There was a problem hiding this comment.
I guess, technically, it's not a slice in the golang sense. Maybe ByteArray, or array of bytes is better for us.
| @@ -206,30 +246,32 @@ def scalarBaseMult(self, k): | |||
|
|
|||
| def splitK(self, k): | |||
| """ | |||
| k: integer | |||
| Args: | |||
| k: integer | |||
There was a problem hiding this comment.
maybe k (int): A big endian integer modulo the curve order.
| Uncompressed: | ||
| <format byte = 0x04><32-byte X coordinate><32-byte Y coordinate> | ||
|
|
||
| It does not support the hybrid format, however. | ||
| """ | ||
| if len(pubKeyStr) == 0: | ||
| raise DecredError("empty pubkey string") |
| @@ -339,10 +380,18 @@ def publicKey(self, k): | |||
|
|
|||
| def parsePubKey(self, pubKeyStr): | |||
There was a problem hiding this comment.
I don't think the arg is named correctly. Str is string correct? But it looks like this accepts something bytes-like.
If I'm correct, just pubKey is better.
| return ByteArray(x.bytes()).int(), ByteArray(y.bytes()).int() | ||
|
|
||
|
|
||
| def fromHex(hx): | ||
| """ | ||
| fromHex converts the passed hex string into an integer. This is only | ||
| meant for the hard-coded constants so errors in the source code can bet |
| def parsePubKey(self, pubKeyStr): | ||
| def parsePubKey(self, pubKey): | ||
| """ |
There was a problem hiding this comment.
We have a class in this module called PublicKey, so pubKey is not great. I've regularly named byte-array-like things with a trailing capital B, so pubKeyB.
This improves docstrings and comments in
crypto.secp256k1.curveand its tests, while doing some assorted cleanup too.