Page MenuHomePhabricator

[electrum] fix handling of unsupported p2sh scripts in the TransactionDialog
ClosedPublic

Authored by PiRK on Jul 1 2024, 08:58.

Details

Summary

Fix support for "unsupported" p2sh schemes. This codebase claims to not make any particular effort to support parsing of p2sh scripts besides standard multisig, but we still don't want the application to crash or show an error message when users paste an externally generated valid hex transaction.

This was likeky broken in the recent major Transaction refactoring. The problem is that TxInput.type can be undefined (None) in some cases, and the TxInput.to_coin_dict function expects it to be always defined.

Cleanup the TxInput.to_coin_dict function slightly: parse_scriptsig function no longer raises (at least intentionnaly, if it does it is a bug that needs to be fixed), so those few lines of code were unreachable. Now that we set txin.type = ScriptType.unknown in parse_scriptsig, we can use this to detect the case of unsupported p2sh.

Test Plan
python test_runner.py

Menu Tools > Load Transaction > From text
Check that there is no error message when pasting the transactions from the new tests, and the transaction dialog shows useful data and allows broadcasting the tx (it will fail with a "Transaction already in block chain" message for these particular transactions).

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Jul 1 2024, 08:58
In D16391#371074, @PiRK wrote:

Screenshot from 2024-07-01 11-02-16.png (708×822 px, 88 KB)

Note that the p2sh address is correctly displayed here because the tx dialog is updated with data fetched from the fulcrum server (previous transactions). If I repeat the test without internet connection, we see the expected "UnknownAddress" instead.

This revision is now accepted and ready to land.Jul 1 2024, 18:54