Move `SignSchnorr` to `KeyPair`. This makes `CKey::SignSchnorr` now
compute a `KeyPair` object and then call `KeyPair::SignSchorr`. The
notable changes are:
* Move the merkle_root tweaking out of the sign function and into
the KeyPair constructor
* Remove the temporary secp256k1_keypair object and have the
functions access m_keypair->data() directly
Add a `KeyPair` class which wraps the `secp256k1_keypair`. This keeps
the secret data in secure memory and enables passing the
`KeyPair` object directly to libsecp256k1 functions expecting a
`secp256k1_keypair`.
Motivation: when passing `CKeys` for taproot outputs to libsecp256k1 functions,
the first step is to create a `secp256k1_keypair` data type and use that
instead. This is so the libsecp256k1 function can determine if the key
needs to be negated, e.g., when signing.
This is a bit clunky in that it creates an extra step when using a `CKey`
for a taproot output and also involves copying the secret data into a
temporary object, which the caller must then take care to cleanse. In
addition, the logic for applying the merkle_root tweak currently
only exists in the `SignSchnorr` function.
In a later commit, we will add the merkle_root tweaking logic to this
function, which will make the merkle_root logic reusable outside of
signing by using the `KeyPair` class directly.
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
This method was introduced as a pre-requirement for the v2 transport
protocol back then (see PR #14047, commit 463921bb), when it was still
BIP151. With the replacement BIP324, this is not needed anymore, and
it's also unlikely that any other proposal would need to negate private
keys at this abstraction level.
(If there is really demand, it's trivial to reintroduce the method.)
Making the `GenerateRandomKey` helper available to other modules via
key.{h.cpp} allows us to create random private keys directly at
instantiation of CKey, in contrast to the two-step process of creating
the instance and then having to call `MakeNewKey(...)`.
Instead of storing the key material as an std::vector (with secure allocator),
use a secure_unique_ptr to a 32-byte array, and use nullptr for invalid keys.
This means a smaller CKey type, and no secure/dynamic memory usage for invalid
keys.
libsecp256k1's secp256k1_schnorrsig_sign only follows BIP340 exactly
if an aux_rand32 argument is passed. When no randomness is used
(as is the case in the current codebase here), there is no impact
on security between not providing aux_rand32 at all, or providing
an empty one. Yet, for repeatability/testability it is simpler
to always use an all-zero one.
This makes calling code less verbose and less fragile. Also, by adding
the CKey::data() member function, it is now possible to call HexStr()
with a CKey object.
The serialization/deserialization methods for the classes CExtKey and
CExtPubKey were only used in the BIP32 unit tests, where the relevant parts are
removed as well.
e306be7429 Use 72 byte dummy signatures when watching only inputs may be used (Andrew Chow)
48b1473c89 Use 71 byte signature for DUMMY_SIGNATURE_CREATOR (Andrew Chow)
18dfea0dd0 Always create 70 byte signatures with low R values (Andrew Chow)
Pull request description:
When creating signatures for transactions, always make one which has a 32 byte or smaller R and 32 byte or smaller S value. This results in signatures that are always less than 71 bytes (32 byte R + 32 byte S + 6 bytes DER + 1 byte sighash) with low R values. In most cases, the signature will be 71 bytes.
Because R is not mutable in the same way that S is, a low R value can only be found by trying different nonces. RFC 6979 for deterministic nonce generation has the option to specify additional entropy, so we simply use that and add a uin32_t counter which we increment in order to try different nonces. Nonces are sill deterministically generated as the nonce used will the be the first one where the counter results in a nonce that results in a low R value. Because different nonces need to be tried, time to produce a signature does increase. On average, it takes twice as long to make a signature as two signatures need to be created, on average, to find one with a low R.
Having a fixed size signature makes size calculations easier and also saves half a byte of transaction size, on average.
DUMMY_SIGNATURE_CREATOR has been modified to produce 71 byte dummy signatures instead of 72 byte signatures.
Tree-SHA512: 3cd791505126ce92da7c631856a97ba0b59e87d9c132feff6e0eef1dc47768e81fbb38bfbe970371bedf9714b7f61a13a5fe9f30f962c81734092a4d19a4ef33
When extra entropy is not specified by the caller, CKey::Sign will
now always create a signature that has a low R value and is at most
70 bytes. The resulting signature on the stack will be 71 bytes when
the sighash byte is included.
Using low R signatures means that the resulting DER encoded signature
will never need to have additional padding to account for high R
values.
In the case of CKey's destructor, it seems to have been an oversight in
f4d1fc259 not to delete it. At this point, it results in the move
constructors/assignment operators for CKey being deleted, which may have
a performance impact.
Remove the nType and nVersion as parameters to all serialization methods
and functions. There is only one place where it's read and has an impact
(in CAddress), and even there it does not impact any of the recursively
invoked serializers.
Instead, the few places that need nType or nVersion are changed to read
it directly from the stream object, through GetType() and GetVersion()
methods which are added to all stream classes.
Replace these with vectors allocated from the secure allocator.
This avoids mlock syscall churn on stack pages, as well as makes
it possible to get rid of these functions.
Please review this commit and the previous one carefully that
no `sizeof(vectortype)` remains in the memcpys and memcmps usage
(ick!), and `.data()` or `&vec[x]` is used as appropriate instead of
&vec.
