Page MenuHomePhabricator

[electrum] minor cleanup of Transaction.fetch_input_data
ClosedPublic

Authored by PiRK on Sep 21 2023, 07:40.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC0365ac9bdb97: [electrum] minor cleanup of Transaction.fetch_input_data
Summary
  • put comments on previous line to let the automatic code formatting do its job
  • save some indentation levels
  • add a few typehints
  • use f-strings
  • tx_cache_put is always called with a valid Transaction, no need to check for tx is not None
  • move imports to the top of the file (they are always imported anyway by other modules, and multiple imports add no significant additional cost)
Test Plan

send a transaction between two different wallets; in the receiving wallet double-click on the tx in the history, check that the dialog knows the input amount

Diff Detail

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

Event Timeline

PiRK requested review of this revision.Sep 21 2023, 07:40
Fabien requested changes to this revision.Sep 21 2023, 08:16
Fabien added a subscriber: Fabien.
Fabien added inline comments.
electrum/electrumabc/transaction.py
1742–1744 ↗(On Diff #42308)
1769 ↗(On Diff #42308)

it can be further improved with a structure like:

if tx is None:
    # tx was not in cache or wallet.transactions, mark
    # it for download below (this branch can also execute
    # in the unlikely case where there was an error above)
    # Remember the input# as well as the prevout_n
    need_dl_txids[prevout_hash].append((i, n))
    continue

[...]
This revision now requires changes to proceed.Sep 21 2023, 08:16

remove more indentation (this requires some duplication of inps.append(inp))

need_dl_txids[prevout_hash].append((i, n)) also needs duplicating. Add comments.

remove another indentation by appending "already complete" inputs first

Fabien added inline comments.
electrum/electrumabc/transaction.py
1778 ↗(On Diff #42311)

All that for the sake of a print, if you can improve on the message you can simply use if tx is None or not tx.raw

This revision is now accepted and ready to land.Sep 21 2023, 13:49

simplify the code by removing a debbuging log, fix the comment