Page MenuHomePhabricator

[electrum] misc cleanups of ElectrumWindow.broadcast_transaction
ClosedPublic

Authored by PiRK on Nov 24 2023, 11:40.

Details

Summary

Minor code cleanups:

  • remove unnecessary variables
  • invert some logic to remove nesting (indentatation) by returning early when possible
  • use x is not None instead of not x when x is an Optional[something]
  • move same-line comments to previous line
  • add a few typehints
Test Plan

send a transaction

Diff Detail

Repository
rABC Bitcoin ABC
Branch
broadcast_notif_multitx
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 25727
Build 51034: Build Diffelectrum-tests
Build 51033: arc lint + arc unit

Event Timeline

PiRK requested review of this revision.Nov 24 2023, 11:40

add typehint for callback parameter

electrum/electrumabc_gui/qt/main_window.py
2674–2675

These two vars are always redefined when needed

Fabien added inline comments.
electrum/electrumabc_gui/qt/main_window.py
2761

from the previous version it seems result can be None ?

electrum/electrumabc_gui/qt/main_window.py
2761

The only thing that could make it None is is self.network.broadcast_transaction(tx) returns None, but it does that only it we pass it a callback argument.

electrum/electrumabc_gui/qt/main_window.py
2761

The broadcast_done function takes it input from the output of the broadcast_thread function.

PiRK planned changes to this revision.Nov 24 2023, 14:54

as per discussion, I will add back the check for if result: just in case it can sometimes be None, to keep this diff a cleanup only with no possible change of behavior.

make sure result is not None before unpacking it. If None, just assign status False and empty msg, and let the following block call callbck(False)

This revision is now accepted and ready to land.Nov 24 2023, 15:10