Prevent 'KeyIsEncryptedException' from being thrown when signing with a
'LowRSigningKey'-wrapped, encrypted HD key, due to breakage of the
apparent invariant that the 'keyCrypter' field of 'ECKey' should be null
whenever the key isn't encrypted.
When signing with a wrapped, encrypted HD key, the original key is
decrypted and then re-wrapped as a 'LowRSigningKey' instance. This was
blindly copying the 'keyCrypter' property of the decrypted key. But
'DeterministicKey::getKeyCrypter' returns non-null if its parent does,
even if the actual field is null, and the decrypted HD key has the same
parent as the encrypted original. Thus, blindly copying the property
(rather than the field) breaks the above invariant.
Fixes issue #7241 with blind voting, caused by the earlier PR #7238
which introduced low-R nonce grinding.
To slightly save storage & bandwidth, create low-R signatures in places
besides tx ScriptSigs & witnesses, such as merit lists, proofs of burn,
filter, alert & accounting authentication data.
(Also, to make setup of mock keys a little easier, bypass wrapping of
keys that are already instances of 'LowRSigningKey' in the factory
method, 'LowRSigningKey.from(ECKey)'.)
Provide a 'BisqWallet' subclass of 'o.b.w.Wallet', in order to override
the 'signTransaction' method used internally by the bitcoinj SendRequest
API, so that it always produces txs with low-R signatures.
Also modify 'WalletService.signTransactionInput' to likewise wrap wallet
keys, so that the provided tx input is given a low-R signature.
Finally, modify the manual signing logic in TradeWalletService to wrap
multisig & deposit input keys, which should cover all tx signing in the
Bisq application.
Implement low-R nonce grinding with the class 'LowRSigningKey', which
always produces low-R signatures, consistent with the behaviour of
Bitcoin Core/Knots (post-2018), Sparrow, Coldcard and possibly other
wallets with recent updates, for hopefully improved privacy and slightly
lower and more predictable tx fees. Canonical DER-encoded signatures are
usually either 71 or 70 bytes (starting with hex 3045 or 3044 resp.),
with roughly 50-50 odds, depending on whether they are high-R or low-R.
(Less than 1% of the time, they will be shorter than 70 bytes, because
of the variable length bigint R & S encodings.) So trying different
nonces for low-R saves half-a-byte on average, at the cost of doubling
the average signing time.
To this end, provide the class 'CountingHMacDSAKCalculator' to supply a
custom nonce to 'o.b.c.s.ECDSASigner'. The first invocation of the
k-calculator instance matches the output of the RFC 6979 compliant
Bouncy Castle version, but subsequent invocations increment an internal
counter supplied as additional data/entropy to the HMAC, as mentioned in
section 3.6 of the RFC, until a low-R signature results. In this way, a
deterministic signing algorithm exactly matching that of (post-2018
versions of) Bitcoin Core results.
Also add unit tests, with test vectors taken from the RFC (which only
covers the NIST curves, unfortunately, not secp256k1), and test vectors
generated from signed txs created by the 'bitcoin-tx' command.
Replace the streaming of Map entry sets to pick out a single entry by
key equality, and instead do a lookup into the map. Also, optimise the
date range filtering in 'TradeStatisticsManager::getTradeStatisticsList'
by using 'RangeUtils::subSet' to avoid scanning the entire collection.
(This method is applicable, as the trade statistics set is navigable and
naturally sorted by date.)
Reject any custom receiver address which wasn't encoded as Bech32m if
the witness program version is greater than zero. These are currently
accepted by bitcoinj but are now invalid and would fail to parse if our
fork was updated to understand Bech32m, to support sending to P2TR
addresses, which the upstream version appears to. (Thus, the presence of
such malformed receivers would not be an issue at present, but might
cause complications in the future.)
Set the burn cap of a candidate to zero if he has an invalid receiver
address, that is, one that bitcoinj cannot parse. This prevents trade
failure when creating the DPT, by making such BM inactive and
distributing their share to the other BM. (Setting the burn cap to zero
is a little more robust than simply filtering out such candidates, as
'BurningManService' handles subsequent share redistribution better than
'(DelayedPayoutTx|BtcFee)ReceiverService'.)
While this case should normally never occur, due to UI validation of the
custom receiver address, there are at least two ways a BM could
invalidate his own receiver address if so inclined:
1) He could simply bypass the UI validation;
2) He could manually create a compensation issuance tx with a change
address type unrecognised by bitcoinj, such as P2TR, as the address
field is pulled straight from the RPC JSON by each full DAO node.
Thus, it is necessary to check both change and custom addresses.
These are both redundant now and will always return true. Also add a
missing past check for Proposal 412 activation to 'RefundManager',
instead of just defaulting to the current date, in case of any very old
disputes involving DPTs created prior to the activation.
Handle the exceptional case of a receiver address chosen from a cycle
where the candidate somehow got more than one compensation proposal
accepted. Either the last custom address or first issuance change
address is supposed to be chosen for the receiver address, but in case
of a tie at that vote result height, take the address that comes
first in lexicographic order.
This avoids needless hashing & equality comparisons of instances of
'BurningManCandidate', which are quite large mutable objects (so should
probably use reference equality anyway, and not be used as keys).
Also rearrange a couple of (package) private methods.
Avoid streaming over the entire proposals list to find a matching txId,
for every 'Issuance' & 'CompensationProposal' pair used to construct and
add a compensation model to the burn output model of each candidate.
Instead, stream over the proposals list once, doing lookups by txId of
each matching issuance, which uses the TreeMap 'DaoState.issuanceMap',
thereby taking O(n*log(n)) time.
