Page MenuHomePhabricator
Differential D18324

[electrum] refactor: move Send tab to its own module
ClosedPublic
Actions

Authored by PiRK on Jul 4 2025, 06:58.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC011b0b6915aa: [electrum] refactor: move Send tab to its own module
Summary

This move 1500+ lines of code from the main window god object to another module with limited interconnections. The main_window no longer needs to maintain the payement request and privkey sweeping state machines, and no longer needs to know the details of the zoo of widgets in the Send tab. The send tab still needs some knowledge of the main window API to trigger some statusbar or history updates, to access contacts stored in the Contacts tab, and to use some trivial helper functions that can easily be factored out in the future(get_decimal_point(), on_error(), show_transaaction(), password_dialog`...)

The send tab widget now deals with everything related to sending transactions, payment requests, invoices.

There is no change in behavior.

Depends on D18335

Test Plan

git show --color-moved=dimmed_zebra --color-moved-ws=allow-indentation-change

Run the application and try to test all the touched components (send transactions, view transaction details, send to contacts, paste a URI in the recipient edit, open an URI on the command line, open a payment request...)

Diff Detail

Repository
rABC Bitcoin ABC
Branch
refactor_amin_window
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 33774
Build 67026: Build Diffelectrum-tests · electrum-functional-tests
Build 67025: arc lint + arc unit

Event Timeline

PiRK created this revision.Jul 4 2025, 06:58
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 4 2025, 06:58
Harbormaster completed remote builds in B33757: Diff 54685.Jul 4 2025, 07:06
PiRK published this revision for review.Jul 4 2025, 07:11
PiRK added inline comments.
electrum/electrumabc/amount.py
155–159 ↗(On Diff #54685)

This is moved here from the main_window so that the various subwidgets of the Send tab only need the config, not the main_window

electrum/electrumabc_gui/qt/main_window.py
2107–2111 ↗(On Diff #54685)

I removed these pointless russian doll layouts (they used to contain more that 1 thing back in 2015: https://github.com/Bitcoin-ABC/ElectrumABC/commit/cf44e65ba8fc2e9be025d1d051487586af2c5721)

electrum/electrumabc_gui/qt/send_tab.py
98 ↗(On Diff #54685)

this is a new signal so the tab can signal that it would like to be shown. The main window connects this signal to self.show_send_tab. This avoids calling directly self.window.show_send_tab() from the tab's code.

PiRK edited the test plan for this revision. (Show Details)Jul 4 2025, 07:13
Fabien requested changes to this revision.Jul 4 2025, 07:45
Fabien added a subscriber: Fabien.

You need to split the minor refactors that remove the need for the main window before this change so it's closer to a move only diff.
Overall the code is still pretty bad but this doesn't make it worst. Ultimately only the main window (the parent) should access the send_tab, all the other methods that are called from the various widgets don't belong in the send_tab.

electrum/electrumabc_gui/qt/history_list.py
325 ↗(On Diff #54685)

This method should move to invoice_list or something like that, there is no reason for the history list to know about the full gui tree. I guess that's for another diff

electrum/electrumabc_gui/qt/invoice_list.py
124 ↗(On Diff #54685)

This should definitely be a copy() function from some util module

This revision now requires changes to proceed.Jul 4 2025, 07:45
PiRK updated this revision to Diff 54707.Jul 5 2025, 14:16
PiRK edited the summary of this revision. (Show Details)
PiRK added a parent revision: D18331: [electrum] fix update of amounts when changing unit.

rebase, addressed feedback in parent diffs, fixed more instances of amount_e being accessed in main_window

Harbormaster completed remote builds in B33774: Diff 54707.Jul 5 2025, 14:22
PiRK added inline comments.Jul 5 2025, 14:22
electrum/electrumabc_gui/qt/invoice_list.py
124 ↗(On Diff #54685)

I will do this in a separate diff with the other 15 places in the codebase that do the same thing.

Something like:

from .util import copy_to_clipboard

...
    copy_to_clipboard(text)
PiRK planned changes to this revision.Jul 5 2025, 14:25

actually this does not completely address the feedback on the send_tab being accessed from history_list

PiRK updated this revision to Diff 54722.Jul 7 2025, 11:55

rebase, move a few methods that access history_list back to main_window so that the SendTab does not need to know the history_list API

PiRK planned changes to this revision.Jul 7 2025, 12:01
PiRK added inline comments.
electrum/electrumabc_gui/qt/invoice_list.py
128–131 ↗(On Diff #54722)

this is now wrong

Harbormaster completed remote builds in B33784: Diff 54722.Jul 7 2025, 12:02
PiRK updated this revision to Diff 54728.Jul 7 2025, 15:15
PiRK edited the summary of this revision. (Show Details)
PiRK edited the test plan for this revision. (Show Details)
PiRK added a parent revision: D18335: [electrum] simplify and inline InvoiceList.chkVisible.
  • rebase on the recent stack of small refactorings that help disentangle a bit more the main_window and it's child widgets
  • revert most changes in invoice_list.py: we can decouple this module in another diff, for now we still need to access the main_window for some things
  • don't access send_tab from invoice_list, because invoice_list is a subwidget of the tab. Add signals and let the Send tab connect to the signal
  • fix a bunch of lines that were broken by this refactor (self.* -> self.send_tab.*...) and test them (payment requests, contacts...)
PiRK added inline comments.Jul 7 2025, 15:21
electrum/electrumabc_gui/qt/main_window.py
222 ↗(On Diff #54728)

we now need the contact list to be instantiated first, for the new signal -slot connection in create_send_tab

1754 ↗(On Diff #54728)

It is not great that the send tab is responsible for requesting to be shown, but the code is currently still messy in that way. Changing this deserves its own diff, if we want to do it.

Harbormaster completed remote builds in B33788: Diff 54728.Jul 7 2025, 15:43
Fabien accepted this revision.Jul 8 2025, 07:53
This revision is now accepted and ready to land.Jul 8 2025, 07:53
Closed by commit rABC011b0b6915aa: [electrum] refactor: move Send tab to its own module (authored by PiRK). · Explain WhyJul 8 2025, 12:08
This revision was automatically updated to reflect the committed changes.
PiRK added a commit: rABC011b0b6915aa: [electrum] refactor: move Send tab to its own module.

Revision Contents
Changeset List

PathSize
electrum/
electrumabc_gui/
qt/
4 lines
2 lines
2 lines
28 lines
1564 lines
28 lines
1600 lines
2 lines
2 lines
electrumabc_plugins/
fusion/
3 lines

Diff 54707

View Options

electrum/electrumabc_gui/qt/__init__.py

Loading...
View Options

electrum/electrumabc_gui/qt/address_list.py

Loading...
View Options

electrum/electrumabc_gui/qt/history_list.py

Loading...
View Options

electrum/electrumabc_gui/qt/invoice_list.py

Loading...
View Options

electrum/electrumabc_gui/qt/main_window.py

Loading...
View Options

electrum/electrumabc_gui/qt/paytoedit.py

Loading...
View Options

electrum/electrumabc_gui/qt/send_tab.py

Loading...
View Options

electrum/electrumabc_gui/qt/transaction_dialog.py

Loading...
View Options

electrum/electrumabc_gui/qt/utxo_list.py

Loading...
View Options

electrum/electrumabc_plugins/fusion/qt.py

Loading...