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
Branch
txinputs_fetched_inputs
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 25111
Build 49810: Build Diffelectrum-tests
Build 49809: arc lint + arc unit

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