wallet: move checks for {sign,verify,encrypt,decrypt}_message from UIs to wallet.py

[SomberNight]

Previously we had wallet.sign_message and wallet.decrypt_message, and all preliminary checks had to be done by the caller. This meant Qt/qml/CLI all duplicated some subset of all desired checks.

Now we have wallet.sign_message, wallet.verify_message, wallet.decrypt_message, wallet.encrypt_message all in wallet.py, and all checks are self-contained inside. (The two new methods are classmethods, as they don't require a wallet, and that's why they did not exist before)

note: this is somewhat related to #10787 but I wanted to clean this code up before we decide to strip/not-strip whitespaces as it really should be un-duped.

[SomberNight]

note: this is mostly boring clean-up and refactoring. It is in low-risk code, and code that I am familiar with. Even though the diff is large, unless someone proactively reviews it, I will self-merge to reduce review burden.

[f321x]

[...] unless someone proactively reviews it, I will self-merge to reduce review burden.

I let the clanker have a look, mostly found some nits, sharing here fyi:

See output (click to expand)

1. Behaviour changes to the public CLI/RPC (undocumented)

Same probe run against base and head:

Call	Base	Head
verifymessage("not-an-address", sig, msg)	False	raises UserFacingException
verifymessage("bc1q…yue0 ", sig, msg) (trailing space)	False	raises
verifymessage(addr, sig, b"bytes")	True	raises
verifymessage(addr, "üüü…", msg)	raises ValueError	False ✅ fix
signmessage(<multisig p2sh addr>, m)	returns a signature	raises
encrypt(pubkey, b"bytes")	works	raises

• verifymessage is the consequential one. A script doing [ "$(electrum verifymessage …)" = "true" ] used to get false for a malformed address; now it errors out.
• The contract is internally inconsistent: verify_message raises on a bad address but returns False on bad base64 — same class of bad user input. Pick one. (Returning False for both, with UIs pre-validating the address for a nicer message, matches what the Qt dialog already does.)
• The multisig signmessage change is a genuine improvement — worth keeping. Confirmed base signs with cosigner Nr.1's key and produces a signature Electrum's own verifymessage then rejects (False). But it is still removed functionality.

➡️ All six deserve a changed:/fix: line in RELEASE-NOTES.

---

2. Defects

2.1 type() where repr() was meant — wallet.py:3289, wallet.py:3311

raise UserFacingException(f"pubkey must be a hex string instead of {type(pubkey)}")

is_hex_str returns False for non-str, and the assert above guarantees str, so this branch is only reachable when pubkey is a str that isn't hex. The message therefore always reads:

pubkey must be a hex string instead of <class 'str'>

Base printed instead of 'zz'. Straight regression in diagnosability — should be {pubkey!r}.

2.2 Dead try/except TypeError — wallet.py:3306-3309

assert isinstance(message, str), ...          # 3305
try:
    message = util.to_bytes(message)
except TypeError:
    raise UserFacingException(f"message must be a str instead of {type(message)}") from None

to_bytes only raises TypeError for non-str/bytes/bytearray, which the preceding assert already excludes. Unreachable, and carries the same type()/repr() problem. Reduce to message = util.to_bytes(message).

2.3 "Nicer error msg if pubkey is unrelated" only covers half the wallets — wallet.py:3290-3298

decrypt(deterministic, unrelated pubkey) -> UserFacingException: Pubkey unrelated to wallet.
decrypt(imported,      unrelated pubkey) -> KeyError: '03b7a2c9…'

The Imported_Wallet fast path carries # FIXME missing check: pubkey should be related to wallet, so a Qt user on an imported wallet still sees KeyError('03b7a2…') in the error box. The check is cheap here — Imported_KeyStore.keypairs is a pubkey-keyed dict:

if pubkey not in self.keystore.keypairs:
    raise UserFacingException(_("Pubkey unrelated to wallet."))

O(1), retires the FIXME, and avoids exactly the slow reverse-lookup the fast path exists to skip.

2.4 Redundant work + dead assert — wallet.py:3247-3260

self.get_txin_type(address) is called twice (txin_type, script_type), and assert script_type != "address" is unreachable: "address" isn't in the ['p2pkh','p2wpkh','p2wpkh-p2sh'] whitelist checked at 3248, so that path already raised. Collapse to one variable; drop the assert (or keep it and drop the comment, which no longer describes a reachable state).

2.5 Imports the PR orphaned

File	Now-unused
commands.py	binascii (L32), is_hex_str, to_bytes (L53)
gui/qt/main_window.py	electrum_ecc as ecc (L48), bfh (L57)

CI won't catch these — the flake8 selection is F5,F6,F7,F8, so F401 is off — but all five are newly dead because of this diff.

---

3. Design

3.1 isinstance(self, …) in the base class replaces a working override — wallet.py:3286, 3290

The PR deletes Imported_Wallet.decrypt_message and re-implements it as if isinstance(self, Imported_Wallet) inside Abstract_Wallet, plus if isinstance(self, Multisig_Wallet) for the unsupported case. Abstract_Wallet now names two of its own subclasses — the one place the refactor moves away from cleaner structure.

Suggested: keep Imported_Wallet.decrypt_message as an override (or add _pubkey_to_addr_index() for subclasses), and express the multisig case as an overridable can_decrypt_message() / NotImplementedError on Multisig_Wallet.

3.2 verify_message / encrypt_message don't belong on Abstract_Wallet

Both are @classmethod but never touch cls (@staticmethod would at least be accurate), and neither needs a wallet — as the PR description says. The consequence: commands.encrypt, a @command('') with no wallet, must call Abstract_Wallet.encrypt_message(…), and qedaemon.py now imports electrum.wallet solely to reach a pure function.

Suggested: module-level functions in bitcoin.py / crypto.py; thin Abstract_Wallet wrappers can stay if call-site symmetry matters.

3.3 Type validation duplicated at two layers

commands.py adds 18 lines of isinstance → UserFacingException for four commands; wallet.py then re-asserts the same invariants. There's a defensible reading (RPC boundary = user error, wallet internals = programming error), but the same fact is stated twice in different dialects. Having the wallet methods raise UserFacingException on type errors and dropping the commands.py preamble is strictly less code, same behaviour.

3.4 The consolidation stops short of .strip()

Whitespace handling is still per-UI and still inconsistent:

Caller	address	message	signature
Qt (main_window.py:2165)	strip	strip	no strip
QML	strip	strip	strip
commands.py	none	none	none

A Qt user pasting a signature with a trailing newline gets "Wrong signature" (validate=True rejects it → False); the same paste works in QML. Given #10787 just landed fixing this asymmetry in the other direction, pulling .strip() into verify_message/sign_message looks like the natural completion.

3.5 Smaller consistency items

• sign_message uses bitcoin.is_address (3241); verify_message uses bare is_address (3269). Adjacent new methods, both imported, two spellings.
• message: str reassigned to bytes in verify_message (3277) and encrypt_message (3307). Harmless at runtime, but the PR is explicitly about type hints — a separate message_bytes local keeps them honest.
• Translated and untranslated strings mixed on the same UserFacingException surface: _("Invalid Bitcoin address.") next to the raw f-string at 3289.
• Continuation lines at 3253-3255 are indented one column past their opening quote at 3252.

---

4. QML

4.1 Both new signals are unreachable through the only dialog that listens

SignVerifyMessageDialog.qml:203 gates Verify on enabled: _addressValid && signature.text, where _addressValid = bitcoin.isAddress(addressField.text) — and QEBitcoin.isAddress is the same is_address that verify_message tests before raising. The QML check runs on unstripped text while Python strips first, so the Python check is strictly more permissive: verifyMessageError can never fire from here.

Same on the sign side — visible: canSignMessage already excludes multisig and watching-only, enabled: _addressMine covers the third check, leaving only the txin-type case, which no supported wallet type reaches.

That's a new signal, a new import and ~20 lines of QML for paths the UI already prevents. Defensible as defense-in-depth, but worth deciding deliberately. If they ever do fire, the user gets both the red Invalid! field state and a modal error dialog.

4.2 Duplicated handlers

The two Connections blocks (L215-236) have byte-identical bodies. One local function showError(error) would do.

---

5. Qt

Moving do_verify / do_encrypt onto self.thread (99ab58d) buys uniform error routing through ElectrumWindow.on_error, which does handle UserFacingException cleanly via show_error — that part is correct. The cost: two sub-millisecond pure-CPU operations now queue on the shared wallet TaskThread, behind e.g. a hardware-wallet signing prompt. Low risk, but no upside for verify specifically.

---

6. Worth calling out as good

• Hoisting msg_sign out of the ElectrumWindow class body is a real i18n fix (probably unintentional): as a class attribute, the _() calls ran at import time, freezing translations before the language was selected. Now evaluated per-call. Same msgids, so existing .po entries survive the move (push_locale.py globs electrum/**/*.py).
• except ValueError instead of binascii.Error around b64decode is a genuine bug fix with a correctly-explanatory comment — base raised an uncaught ValueError on a non-ASCII signature.
• Narrowing except BaseException to except ecc.InvalidECPointException is safe: in electrum_ecc 0.0.7 every ECPubkey(bytes) parse failure funnels through _x_and_y_from_pubkey_bytes, which raises exactly that.
• Hardware wallets are safe from the new is_watching_only() gate — Hardware_KeyStore.is_watching_only() hard-returns False. Plugin-level sign_message/decrypt_message are keystore methods and untouched.

---

7. Testing

No tests added for any consolidated method. Existing coverage only exercises imported wallets, so untested:

• the entire new deterministic-wallet decrypt_message branch (pubkeys_to_address → get_address_index → "Pubkey unrelated")
• multisig refusal
• watching-only refusals (sign and decrypt)
• the unicode-signature fix — the one change here that fixes a crash, and the most valuable regression test in the set

---

[SomberNight]

Thanks. I took a few of the llm suggestions in https://github.com/spesmilo/electrum/compare/566937f87888ec601ccf477fc4684fb2fd397dc0..92e938f4bf5340c2ba032bf19784b053fa4e3d90.

Behaviour changes to the public CLI/RPC (undocumented)
Call // Base // Head
signmessage(<multisig p2sh addr>, m) // returns a signature // raises

That's a bug on master I did not know about. Is now fixed.