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).
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.
Note: the option to load a backup from SD card when the device already has a seed
has been removed. The device always returns an error when attempting this.
regression from cea4238b81
```
975.04 | E | plugin.DeviceMgr | failed to create client for digitalbitbox at <REDACTED>: AttributeError("'NoneType' object has no attribute 'get_passphrase'")
Traceback (most recent call last):
File "...\electrum\electrum\plugin.py", line 620, in list_pairable_device_infos
soft_device_id = client.get_soft_device_id()
File "...\electrum\electrum\plugins\hw_wallet\plugin.py", line 251, in get_soft_device_id
root_fp = self.request_root_fingerprint_from_device()
File "...\electrum\electrum\plugin.py", line 362, in wrapper
return run_in_hwd_thread(partial(func, *args, **kwargs))
File "...\electrum\electrum\plugin.py", line 352, in run_in_hwd_thread
return func()
File "...\electrum\electrum\plugins\hw_wallet\plugin.py", line 264, in request_root_fingerprint_from_device
child_of_root_xpub = self.get_xpub("m/0'", xtype='standard')
File "...\electrum\electrum\plugins\digitalbitbox\digitalbitbox.py", line 115, in get_xpub
reply = self._get_xpub(bip32_path)
File "...\electrum\electrum\plugins\digitalbitbox\digitalbitbox.py", line 110, in _get_xpub
if self.check_device_dialog():
File "...\electrum\electrum\plugins\digitalbitbox\digitalbitbox.py", line 197, in check_device_dialog
if not self.password_dialog(msg):
File "...\electrum\electrum\plugins\digitalbitbox\digitalbitbox.py", line 159, in password_dialog
password = self.handler.get_passphrase(msg, False)
AttributeError: 'NoneType' object has no attribute 'get_passphrase'
```
in particular, ledger: fix sign_message for some wallets
```
156.02 | E | plugins.ledger.ledger |
Traceback (most recent call last):
File "...\electrum\electrum\plugins\ledger\ledger.py", line 1265, in sign_message
result = base64.b64decode(self.client.sign_message(message, address_path))
File "...\Python310\site-packages\ledger_bitcoin\client.py", line 230, in sign_message
sw, response = self._make_request(self.builder.sign_message(message_bytes, bip32_path), client_intepreter)
File "...\Python310\site-packages\ledger_bitcoin\command_builder.py", line 176, in sign_message
bip32_path: List[bytes] = bip32_path_from_string(bip32_path)
File "...\Python310\site-packages\ledger_bitcoin\common.py", line 68, in bip32_path_from_string
return [int(p).to_bytes(4, byteorder="big") if "'" not in p
File "...\Python310\site-packages\ledger_bitcoin\common.py", line 68, in <listcomp>
return [int(p).to_bytes(4, byteorder="big") if "'" not in p
ValueError: invalid literal for int() with base 10: '84h'
```
Regression from df2bd61de6, where the
default hardened char was changed from "'" to "h". Note that there was
no corresponding wallet db upgrade, so some files use one char and
others use the other.
```
$ python3 -O
Python 3.10.6 (main, Mar 10 2023, 10:55:28) [GCC 11.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> assert (x := 2)
>>> x
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
NameError: name 'x' is not defined
```
pity. it looked to be a neat and concise pattern.
The keystore does not have a "client" field.
One is supposed to use the "get_client" method instead (the generic API is `plugin.get_client(keystore)`)
Remnants of old code.
scenario1:
- 2of2 multisig wallet with trezor1 and trezor2 keystores
- only trezor2 connected
- previously we would pair first keystore with connected device and then display error.
now we will pair the device with the correct keystore on the first try
scenario2:
- standard wallet with trezor1 keystore
- trezor2 connected (different device)
- previously we would pair trezor2 with the keystore and then display error.
now we will prompt the user to select which device to pair with (out of one)
related: #5789
this is a regression from #5721
Removed the `TxInput.is_coinbase` method as I think it is a confusing API,
instead we now have `TxInput.is_coinbase_input` and `TxInput.is_coinbase_output`.
related #5872
