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

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

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

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

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 created this revision.Mon, Jul 1, 08:58

Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Jul 1, 08:58

PiRK requested review of this revision.Mon, Jul 1, 08:58

PiRK edited the summary of this revision. (Show Details)Mon, Jul 1, 08:59

Harbormaster completed remote builds in B29419: Diff 48419.Mon, Jul 1, 09:01

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

In D16391#371074, @PiRK wrote:

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.

Fabien accepted this revision.Mon, Jul 1, 18:54

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

Closed by commit rABC928d1f039a5c: [electrum] fix handling of unsupported p2sh scripts in the TransactionDialog (authored by PiRK). · Explain WhyTue, Jul 2, 11:03

This revision was automatically updated to reflect the committed changes.

PiRK added a commit: rABC928d1f039a5c: [electrum] fix handling of unsupported p2sh scripts in the TransactionDialog.