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.

Sign up for GitHub

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

Jump to bottom

tests: NAF and splitK in crypto.secp256k1.curve #74

Merged
merged 8 commits into from
Feb 14, 2020
Merged

tests: NAF and splitK in crypto.secp256k1.curve #74

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d1deb2d
Convert ./tests/unit/crypto/secp256k1/test_curve.py to pytest format.
teknico Feb 11, 2020
4b4ce41
Add NAF tests to curve.py . Also move randBytes to util.helpers.
teknico Feb 11, 2020
066c2a9
Added tests for curve.splitK, fixed a double bug in it.
teknico Feb 12, 2020
b5acdfb
Run black and isort.
teknico Feb 12, 2020
ede58c1
Use a dictionary instead of dataclasses to keep supporting Python 3.6.
teknico Feb 13, 2020
f82569a
Remove the redundant dataclasses import too.
teknico Feb 13, 2020
bded824
Moved comments per review.
teknico Feb 13, 2020
9964baa
Changes per review:
teknico Feb 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions decred/decred/crypto/secp256k1/curve.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ def __init__(self, curve, k, x, y):
self.pub = PublicKey(curve, x, y)


def randFieldElement(): # c elliptic.Curve, rand io.Reader) (k *big.Int, err error) {
def randFieldElement():
"""
randFieldElement returns a random element of the field underlying the given
curve using the procedure given in [NSA] A.2.1.
Expand All @@ -155,7 +155,7 @@ def generateKey():

class KoblitzCurve:
def __init__(
self, P, N, B, Gx, Gy, BitSize, H, q, byteSize, lamda, beta, a1, b1, a2, b2
self, P, N, B, Gx, Gy, BitSize, H, q, byteSize, lambda_, beta, a1, b1, a2, b2
):
self.P = P
self.N = N
Expand All @@ -166,7 +166,7 @@ def __init__(
self.H = H
self.q = q
self.byteSize = byteSize
self.lamda = lamda
self.lambda_ = lambda_
buck54321 marked this conversation as resolved.
Show resolved Hide resolved
self.beta = beta
self.a1 = a1
self.b1 = b1
Expand Down Expand Up @@ -210,10 +210,10 @@ def splitK(self, k):
# being a bottleneck.
# c1 = round(b2 * k / n) from step 4.
# Rounding isn't really necessary and costs too much, hence skipped
c1 = (self.b2 + k) // self.N
c1 = (self.b2 * k) // self.N
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This (and the similar one below) is a bugfix: the original Go code has a bigInt.Mul here, and tests fail without this change.

Copy link
Member

Choose a reason for hiding this comment

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

Weird that we never hit an issue with this with signature verification. Hmmm.

# c2 = round(b1 * k / n) from step 4 (sign reversed to optimize one step)
# Rounding isn't really necessary and costs too much, hence skipped
c2 = (self.b1 + k) // self.N
c2 = (self.b1 * k) // self.N
# k1 = k - c1 * a1 - c2 * a2 from step 5 (note c2's sign is reversed)
tmp1 = c1 * self.a1
tmp2 = c2 * self.a2
Expand Down Expand Up @@ -935,17 +935,15 @@ def __init__(self):
BitSize=bitSize,
H=1,
q=(p + 1) // 4,
# new(big.Int).Div(new(big.Int).Add(secp256k1.P, big.NewInt(1)), big.NewInt(4)),
# Provided for convenience since this gets computed repeatedly.
# lambda is a reserved keyword in Python, so misspelling on purpose.
byteSize=bitSize / 8,
# Next 6 constants are from Hal Finney's bitcointalk.org post:
# https://bitcointalk.org/index.php?topic=3238.msg45565#msg45565
# May he rest in peace.
#
# They have also been independently derived from the code in the
# EndomorphismVectors function in gensecp256k1.go.
lamda=fromHex(
lambda_=fromHex(
"5363AD4CC05C30E0A5261C028812645A122E22EA20816678DF02967C1B23BD72"
),
beta=FieldVal.fromHex(
Expand Down
20 changes: 20 additions & 0 deletions decred/decred/util/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import os
from os.path import expanduser
import platform
import random
import shutil
import sys
from tempfile import TemporaryDirectory
Expand All @@ -21,6 +22,25 @@
from appdirs import AppDirs


def sign(x):
"""
x: number
"""
if x < 0:
return -1
elif x > 0:
return 1
else:
return 0


random.seed(0)
Copy link
Member

Choose a reason for hiding this comment

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

Can't set the random seed in production code. I'm hesitant to add any functions here that are just for tests. How about a test utilities module somewhere?

Copy link
Collaborator Author

@teknico teknico Feb 13, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Can't set the random seed in production code. I'm hesitant to add any functions here that are just for tests.

Makes sense.

How about a test utilities module somewhere?

Test code does not live in packages and modules. Test discovery is performed by pytest according to its own rules. For cases like this, pytest offers fixtures, I'll change randBytes and sign as such.

The remaining question is whether setting the seed should be done in the fixture or in the test that uses it: I lean towards the latter.



def randBytes(low=0, high=50):
return bytes(random.randint(0, 255) for _ in range(random.randint(low, high)))


def formatTraceback(e):
"""
Format a traceback for an exception.
Expand Down
Loading