This adds a README.md file for the package which includes the custom
EC-Schnorr-DCRv0 specification along with the design details and
rationale for why its various characteristics were chosen.
This reworks the Schnorr signature parsing code to be more consistent
with the ecdsa parsing code and pave the way for eventually being able
to remove the reliance on big ints.
It also adds a full set of signature parsing tests that include 100%
coverage of all branches in the parsing code and test for the explicit
reason for the failure to ensure that each test is actually testing the
intended condition.
In particular, the parsing code has been changed as follows:
- Reject signatures that have the r and/or s components out of
the allowed ranges for their respective type
- Make use of the specialized mod N scalar and field val types
- Introduce new error codes for signature errors and return them
This modifies the SetBytes and SetByteSlice methods of the FieldVal type
to return whether or not the provided value was greater than or equal to
the field prime and adds tests to ensure it works as expected.
This allows callers to easily check if a given uint256 value will fit
within the field element without modular reduction and makes it more
consistent with the ModNScalar type.
Finally, it updates all of the callers in the repository accordingly.
This makes use of the PubKey method on secp256k1.PrivateKey to convert a
private key to a public key instead of manually performing the
conversion.
This is desirable since secp256k1 provides the method specifically for
this purpose and it also has the benefit of allowing secp256k1 to
perform the task without round tripping through big integers which
results in slightly faster derivation.
The following benchmarks show a before and after comparison of the
affected operations:
benchmark old ns/op new ns/op delta
----------------------------------------------------------
BenchmarkDeriveHardened 3702 3671 -0.84%
BenchmarkDeriveNormal 4053 3999 -1.33%
benchmark old allocs new allocs delta
----------------------------------------------------------
BenchmarkDeriveHardened 14 14 +0.00%
BenchmarkDeriveNormal 15 15 +0.00%
benchmark old bytes new bytes delta
----------------------------------------------------------
BenchmarkDeriveHardened 1360 1360 +0.00%
BenchmarkDeriveNormal 1409 1409 +0.00%
This adds a modification of a new test vector added to BIP32 which tests
for the case of a child key with leading zeros. It was modified to
ensure it tests the intended condition with the new hash function Decred
uses.
This adds several tests to help ensure the signature verification code
works as intended with invalid signatures by crafting signatures that
are specifically designed to hit the various failure cases and checking
the verification failed for that explicit reason.
This updates the Error and ErrorCode types to provide support for
errors.Is and errors.As and adds a full set of tests for the error
infrastructure so the error handling is more consistent with the rest of
the secp256k1 module.
While here, it removes the unused ErrPubKeyOffCurve error and renames
the error creation func to signatureError to be more consistent.
This modifies the signing code to use extra data that specifically
identifies the custom schnorr signature scheme used when generating
RFC6979 nonces to ensure the same nonce is not generated for the same
message and key as for other signing algorithms such as ECDSA.
While nobody realistically should ever be signing the same message with
the same private key with both Schnorr and ECDSA at the same time, it's
safer to ensure that even if they do, the same nonce value isn't used
since that could lead to revealing the private key.
It also updates the relevant tests to use the new deterministic nonces
and signatures which have been independently verified with the Sage
computer algebra system.
Currently, the secp256k1 package is rather tightly bound to ECDSA
signatures which makes them the only real first-class citizen in the
code. In an effort to make it clear the ECDSA is only one possible
digital signature algorithms and to enable Schnorr signatures to also be
made first class citizens, this decouples all code related to producing
and parsing ECDSA signatures into a separate package under secp256k1
named ecdsa.
This is a fairly large change in terms of the number of lines changed,
however, the vast majority of the changes are mechanical to deal with
the package name changes and moved code. The only real new code is
documentation for the new package.
The following is a high level overview of the changes:
- Rename signature.go to ecdsa/signature.go
- Rename signature_test.go to ecdsa/signature_test.go
- Rename sigerror.go to ecdsa/error.go
- Rename SignatureErrorCode type to ErrorCode
- Rename SignatureError type to Error
- Rename sigerror_test.go to ecdsa/error_test.go
- Rename signature_bench_test.go to ecdsa/bench_test.go
- Move signing-related examples from example_test.go to
ecdsa/example_test.go
- Update all moved code to reference secp256k1 types and funcs
- Remove Sign from the secp256k1.PrivateKey type in favor of ecdsa.Sign
- Add ecdsa/README.md to describe the new package
- Add ecdsa/doc.go to provide package documentation via godoc
- Move signing examples from README.md to new ecdsa/README.md
- Update txscript and rpcserver to use the new package methods
This moves the code for generating an RFC6979 nonce and the related
tests into separate files named nonce.go and nonce_test.go,
respectively. The nonce is not specific to ecdsa and therefore it is
being split out so it remains accessible to multiple signing algorithms.
This exports the internal scalar that is used to house the private key
so that it can be used by external callers without having to round trip
through serializing the bytes.
This add a new method to the PublicKey type named AsJacobian which
converts the public key into a Jacobian point with Z=1. This allows the
public key to be treated a Jacobian point in the secp256k1 group in
calculations.
It also adds an associated test to ensure proper functionality.
This exports the decompressY func as DecompressY in order to allow
external callers to perform optimized elliptic curve point decompression
for a given X coordinate when possible.
It also moves the function and related tests from pubkey.go and
pubkey_test.go to curve.go and curve_test.go, respectively, since it
more correctly belongs there given it is not specific to public keys.
This is part of exposing the ec group operations so callers are not
forced to round trip through big ints.
This exports the scalarBaseMultJacobian func as ScalarBaseMultNonConst
in order to allow external callers to perform optimized elliptic curve
point multiplication of the group base point with a scalar in Jacobian
project coordinates in addition to more clearly indentifying that the
function is not constant time.
This is part of exposing the ec group operations so callers are not
forced to round trip through big ints in affine space.
This exports the scalarMultJacobian func as SclarMultNonConst in order
to allow external callers to perform optimized elliptic curve point
multiplication with a scalar in Jacobian project coordinates in addition
to more clearly indentifying that the function is not constant time.
This is part of exposing the ec group operations so callers are not
forced to round trip through big ints in affine space.
This exports the doubleJacobian func as DoubleNonConst in order to allow
external callers to perform optimized elliptic curve point doubling in
Jacobian project coordinates in addition to more clearly indentifying
that the function is not constant time.
This is part of exposing the ec group operations so callers are not
forced to round trip through big ints in affine space.
This exports the addJacobian func as AddNonConst in order to allow
external callers to perform optimized elliptic curve point addition in
Jacobian project coordinates in addition to more clearly indentifying
that the function is not constant time.
This is part of exposing the ec group operations so callers are not
forced to round trip through big ints in affine space.
This exports the jacobianPoint type as JacobianPoint in order to allow
external callers to perform optimized elliptic curve math in Jacobian
projective coordinates.
This is part of exposing the ec group operations so callers are not
forced to round trip through big ints in affine space.
This removes the bool return from the internal schnorrVerify function
since it is entirely unnecessary given an error is already returned if
the signature fails to verify. It is also more consistent with the rest
of the code.
This exposes a new function on the FieldVal type named IsZeroBit which
operates similarly to IsZero except it returns a 0 or 1.
This is being provided because it is not possible in Go to convert from
a bool to numeric value in constant time and many constant-time
operations require a numeric value.
Since the type is now exported for external use, consumers would
otherwise have no way to perform the conversion.
This exposes a new function on the FieldVal type named IsOneBit which
operates similarly to IsOne except it returns a 0 or 1.
This is being provided because it is not possible in Go to convert from
a bool to numeric value in constant time and many constant-time
operations require a numeric value.
Since the type is now exported for external use, consumers would
otherwise have no way to perform the conversion.
This exposes a new function on the FieldVal type named IsOddBit which
operates similarly to IsOdd except it returns a 0 or 1.
This is being provided because it is not possible in Go to convert from
a bool to numeric value in constant time and many constant-time
operations require a numeric value.
Since the type is now exported for external use, consumers would
otherwise have no way to perform the conversion.
This exports the fieldVal type as FieldVal in order to allow external
callers to perform optimized field math. The intent is ultimately to
expose the group operations as well so callers are not forced to round
trip through big ints.
This updates the documentation for the field value type in preparation
for exporting the type as follows:
- Add a primer to the main field value type which warns loudly about the
requirements for the caller to ensure proper preconditions and
explains the concepts of normalization and magnitude accordingly
- Update all methods to explicitly call out their preconditions
- Update all methods to explicitly call out the resulting normalization
and magnitude
- Update all methods that are constant time to explicitly state it
This moves the SetHex method on the field value type to the associated
test file so it is only avilable during the tests which is its intended
purpose. It is not constant time and therefore shouldn't be available
to callers. This is in preparation for exporting the type.
This modifies the MulInt method on a field value to take a uint8 instead
of a uint in order to make it harder to misuse it since it has a
precondition which requires to the caller to ensure internal overflows
are avoided. In practice, the code currently only ever calls it with a
max value of 8, but in order to prepare for exporting the type, it's
nicer to make it harder to misuse.
This modifies the AddInt method on a field value to take a uint16
instead of a uint in order to make it impossible to misuse it since it
has a precondition of a max value of 2^26 - 1. In practice, the code
currently only ever calls it with 7, but in order to prepare for
exporting the type, it's nicer to prevent the potential misuse.
This modifies the SetInt method on a field value to take a uint16
instead of a uint in order to make it impossible to misuse it since it
has a precondition of a max value of 2^26 - 1. In practice, the code
currently only ever calls it with 0 and 1, but in order to prepare for
exporting the type, it's nicer to prevent the potential misuse.
This modifies the CurveParams struct that is exported to independently
define the params instead of directly embedding elliptic.CurveParams and
updates KoblitzCurve to directly embed elliptic.CurveParams instead to
prevent possible misuse.
This is desirable because elliptic.CurveParams also defines methods
which provide a non-constant and generic implementation of the curve
based on big ints that does not work properly with the secp256k1 curve
due to its use of a different a value in the curve equation. The
adaptor code works around that limitation by embedding the
elliptic.CurveParams and overriding the method implementations with the
correct calculations via the KoblitzCurve type.
In other words, prior to the change, the caller could have easily
misused the params to perform incorrect math by calling the
non-overridden methods on the type returned by the Params method instead
of the overridden version obtained via the S256 method.
That is no longer possible with this change since the Params method now
only contains the parameters and does not embed elliptic.CurveParams.
This modifies the various EC operations to reduce the overall number of
normalizations that are needed by requiring the caller to provide
normalized points instead of blindly normalizing the inputs. It also
updates the operations to ensure the result is normalized along with
updating the comments to call out the new semantics and updating all of
the callers accordingly to ensure the new requirements are met.
This results in less overall normalizations because the caller has more
information about whether or not normalization is needed and can
therefore avoid it in many cases, particularly when it is using the
result that is already normalized as part of a chain of operations.
The following benchmarks show a before and after comparison of the
affected operations:
benchmark old ns/op new ns/op delta
-----------------------------------------------------------------
BenchmarkAddJacobian 569 442 -22.32%
BenchmarkAddJacobianNotZOne 1087 1080 -0.64%
BenchmarkScalarBaseMult 46966 46164 -1.71%
BenchmarkScalarBaseMultJacobian 34218 34165 -0.15%
BenchmarkScalarBaseMultLarge 47077 46819 -0.55%
BenchmarkScalarMult 155318 148019 -4.70%
BenchmarkSigVerify 176007 168422 -4.31%
BenchmarkSign 57636 57561 -0.13%
BenchmarkSignCompact 57883 57653 -0.40%
BenchmarkRecoverCompact 206361 197419 -4.33%
This modifies signature verification to perform the final equality check
in Jacobian projective space to avoid an extra expensive field inversion
by taking advantage of the fact the R component of the signature can be
converted to Jacobian projective space and compared directly so long as
the two possible cases that arise from R being mod N and the original x
coordinate used to create it being mod P.
The following is a before and after comparison of verification of a
typical signature:
benchmark old ns/op new ns/op delta
-----------------------------------------------------
BenchmarkSigVerify 189505 177147 -6.52%
benchmark old allocs new allocs delta
-----------------------------------------------------
BenchmarkSigVerify 34 34 +0.00%
benchmark old bytes new bytes delta
-----------------------------------------------------
BenchmarkSigVerify 1633 1633 +0.00%
The MaybeAcceptDependents function was added as a part of the logic to
track tickets with non-approved inputs separately in a staging pool.
Given it modifies the contents of the mempool, it needs to hold a write
mutex as opposed to a read mutex as otherwise it can modify the mempool
out from under readers.
While here, comment the required locking semantics on the other
functions that were added as a part of the staging logic as well.
This removes the GenerateKey function from the schnorr package since
nothing uses it and it is expected that the caller will use
secp256k1.GeneartePrivateKey instead since the package works with that
type as private keys.
This reduces the number of internal copies of private keys that are made
when serializing them and parsing them from bytes in order to help
reduce the attack surface.
This removes the hash func callback parameter for hashing the commitment
data from the internal sign and verify functions in favor of just
calling the real hash function directly.
This is being done for several reasons:
- It is not safe to use just any hash function, rather the hash function
must have certain properties, so it's safer to make it explicit
- There is only a single hash function, blake256, being used and thus
having an extra unneeded parameter is not desirable
- Any modifications to the commitment data invalidate existing
signatures, so the internals of the function have to realistically be
updated when making any changes anyway
This modifies the internal schnorrVerify function to accept the
signature as a type instead of its raw serialized form. This will
ultimately facilitate making it easier to switch over to more efficient
types for the r and s components.
The existing signing and verification tests have several issues:
- They are using data that is not easily independently verifiable by
other implementations
- The known good test data is using a hash function that is, per the
comments, "horribly broken" instead of using the real hash function
- They are not actually testing anything for the real hash function
since they are doing the equivalent of sign(x) == sign(x)
- The randomness aspects are not actually all that random since they
just involve shuffling the order of the test data instead of actually
producing random data each run
- They aren't consistent with the way tests are written in the rest of
the code
Consequently, this reworks the signing and signature verification tests
to resolve all of those issues. The following is a high-level overview
of the changes:
- Create new known good test data that is easily reproducible and
has been independently verified with the Sage computer algebra system
- The test data includes the original messages so the resulting hashes
are independently verifiable and have a known source
- The test data includes variations of signing the same data with
different keys and nonces, both deterministically generated via
RFC6979 and random, and signing different data with the same keys
- Add random tests that generate random keys and messages with each run
from a new random seed and log that seed in the event of failure
- Ensure mutating a random bit in each good signature results in that
mutated signature failing to verify the original message
- Ensure mutating a random bit in each message hash that was
originally signed results in the original good signature failing to
verify the new mutated message
This removes the pubNonceX and Y parameters from the internal signing
function since they're always called with nil and if the tweaking
functionality is desired in the future, it should take a point to
operate on instead of individual big ints.
