Notably verifymessage and decrypt(message) were silently ignoring trailing garbage
or inserted non-base64 characters present in signatures/ciphertext.
(both the CLI commands and in the GUI)
I think it is much cleaner and preferable to treat such signatures/ciphertext as invalid.
In fact I find it surprising that base64.b64decode(validate=False) is the default.
Perhaps we should create a helper function for it that set validate=True and use that.
This new `Transaction.verify_sig_for_txin` function is an instance method of `Transaction` instead of `PartialTransaction`.
It takes a complete txin, a pubkey and a signature, and verifies the signature.
- `get_preimage_script` is renamed to `get_scriptcode_for_sighash` and now effectively has two implementations:
- the old impl became `PartialTxInput.get_scriptcode_for_sighash`
- this assumes we are the ones constructing a spending txin and can have knowledge beyond what will be revealed onchain
- the new impl is in the base class, `TxInput.get_scriptcode_for_sighash`
- this assumes the txin is already "complete", and mimics a consensus-verifier by extracting the required fields
from the already complete witness/scriptSig and the scriptpubkey of the funding utxo
- `serialize_preimage` now does not require a PartialTransaction, it also works on the base class Transaction
-----
I intend to use this for debugging only atm: I noticed TxBatcher sometimes creates invalid signatures by seeing
that bitcoind rejects txs with `mandatory-script-verify-flag-failed (Signature must be zero for failed CHECK(MULTI)SIG operation)`.
However the txs in question have multiple txins, with some txins containing multiple signatures, and bitcoind does not tell us
which txin/signature is invalid. Knowing which signature is invalid would be a start, but I can now add some temp debug logging
to `serialize_preimage` to compare the message being signed with the message being verified.
As can be seen from the tests, the signature and the pubkey needs to be manually extracted from the txin to be verified:
we still don't have a script interpreter so we don't have logic to "verify a txin". However this new code adds logic
to verify a signature for a txin/pubkey combo (which is a small part of an interpreter/verifier).
- if password is needed, await future and request it from GUI
- run asyncio task for each TxBatch, so that batches can
request the password concurrently
Unlike a full bitcoin node, we rarely (if ever) validate the signatures of arbitrary transactions,
so I don't think these DOS issues can really be used against us.
ref https://rubin.io/bitcoin/2025/03/11/core-vuln-taproot-dos/
ref https://github.com/bitcoin/bitcoin/pull/24105
btw what is not explained in either source link is that the lack of caching is much
more serious for taproot as bip-342 lifted the 10 kbyte max size for scriptPubKeys.
The class TxBatcher handles the creation, broadcast and replacement
of replaceable transactions. Callers (LNWatcher, SwapManager) use
methods add_payment_output and add_sweep_info. Transactions
created by TxBatcher may combine sweeps and outgoing payments.
Transactions created by TxBatcher will have their fee bumped
automatically (this was only the case for sweeps before).
TxBatcher manages several TxBatches. TxBatches are created
dynamically when needed.
The GUI does not touch txbatcher transactions:
- wallet.get_candidates_for_batching excludes txbatcher
transactions
- RBF dialogs do not work with txbatcher transactions
wallet:
- instead of reading config variables, make_unsigned_transaction
takes new parameters: base_tx, send_change_to_lighting
tests:
- unit tests in test_txbatcher.py (replaces test_sswaps.py)
- force all regtests to use MPP, so that we sweep transactions
with several HTLCs. This forces the payment manager to aggregate
first-stage HTLC tx inputs. second-stage are not batched for now.
Add support for key-path-spending taproot utxos into transaction.py.
- no wallet support yet
- add some psbt, and minimal descriptor support
- preliminary work towards script-path spends
Instead of some functions operating with hex strings,
and others using bytes, this consolidates most things to use bytes.
This mainly focuses on bitcoin.py and transaction.py,
and then adapts the API usages in other files.
Notably,
- scripts,
- pubkeys,
- signatures
should be bytes in almost all places now.
- implement it specifically for the "singlesig trezor" case
- aimed to be generic enough that support for more complex scripts
and other keystores could be added later
qml: use backend sighash check and add user confirmation path
qt: use backend sighash check and add user confirmation path
qml: include get_warning_for_risk_of_burning_coins_as_fees test in txdetails
qt: add warning icon to sighash warning
add sighash and fee checks to wallet.sign_transaction, making all warnings fatal unless ignore_warnings is set to True
tests: test sign_transaction on both code paths with ignore_warnings True and False,
raise specific exceptions TransactionPotentiallyDangerousException and TransactionDangerousException
Somewhat a follow-up to 649ce979ab.
This adds some safety belts so we don't accidentally sign a tx that
contains a dummy address.
Specifically we check that tx does not contain output for dummy addr:
- in wallet.sign_transaction
- in network.broadcast_transaction
The second one is perhaps redundant, but I think it does not hurt.
decorators (instead of overloading JsonDB._convert_dict and
_convert_value)
- stored_in for elements of a StoreDict
- stored_as for singletons
- extra register methods are defined for key conversions
This commit was adapted from the jsonpatch branch
In one wallet, before this, make_unsigned_transaction() took 120 sec,
now it takes ~8 sec.
hot path:
```
make_unsigned_transaction (electrum/wallet.py:1696)
add_input_info (electrum/wallet.py:2261)
utxo (electrum/transaction.py:289)
tx_from_any (electrum/transaction.py:1232)
deserialize (electrum/transaction.py:805)
<listcomp> (electrum/transaction.py:805)
parse_output (electrum/transaction.py:706)
__init__ (electrum/transaction.py:127)
scriptpubkey (electrum/transaction.py:173)
get_address_from_output_script (electrum/transaction.py:672)
```
We were re-calculating txin.address from the prevtx-utxo on every call,
e.g. in electrum/gui/qt/utxo_list.py#L103 (for every utxo in the wallet).
For a wallet with ~1300 utxos, UTXOList.update() took ~3.2 sec before this,
now it's ~0.8 sec.
When exporting a tx as qr code, the prev txs are omitted to save space.
This causes problems with offline signers: software electrum signers will
just warn and then proceed, but hw devices will typically error.
If checked, we download prev (parent) txs from the network, asynchronously.
This allows calculating the fee and showing "input addresses".
We could also SPV-verify the tx, to fill in missing tx_mined_status
(block height, blockhash, timestamp, short ids), but this is not done currently.
Note that there is no clean way to do this with electrum protocol 1.4:
`blockchain.transaction.get_merkle(tx_hash, height)` requires knowledge of the block height.
Loosely based on 6112fe0e51
