Fix "Save settings" crash on disabled port; show plugin version in tab - #4
Merged
nothing-stops-this-train merged 1 commit intoJul 28, 2026
Conversation
Two changes.
1. Saving settings with the HTTP port set to 0 ("disabled") crashed with
File ".../swapserver_gui/qt.py", line 223, in on_save
self.config.SWAPSERVER_PORT = new_port
...
File ".../electrum/simple_config.py", line 316, in delete_key
d.pop(key, None)
AttributeError: 'NoneType' object has no attribute 'pop'
on_save turned a spinbox value of 0 into None. SWAPSERVER_PORT is the
dotted key 'plugins.swapserver.port', so assigning None routes into
SimpleConfig's key-*deletion* branch, whose recursion dereferences the
intermediate 'plugins.swapserver' dict without checking it exists. This
plugin only ever writes 'plugins.swapserver_gui.*', so that dict normally
does not exist and the delete always raised. Since the port write came
first, its crash also silently discarded the fee, PoW and relay writes.
A disabled port is now stored as 0. Every reader tests the key for
truthiness (submarine_swaps.py, swapserver/server.py, and this plugin's
can_run/start_server/status), so 0 is behaviourally identical but takes
the ordinary write path. This is an upstream Electrum bug; the workaround
belongs here because users run released AppImages we cannot patch.
The writes moved into a GUI-agnostic swapserver_gui.save_settings() so
they are testable at all -- PyQt6 is unavailable in CI, which is why the
existing suite (an attribute-bag config fake) never caught this. on_save
now also catches save failures and reports them via show_error instead of
letting them escape into the Qt event loop.
2. The tab header shows the installed plugin version, so users can tell
which build they are on. _version.py is committed as the placeholder
'dev' and stamped by contrib/make_zip.sh into a staging copy at build
time, leaving the working tree untouched. CI stamps ${GITHUB_REF_NAME#v}
on v* tags and dev-g<sha7> otherwise, then verifies the stamp landed in
the zip -- so a release asset can never disagree with its tag.
Tests: settings persistence against a real SimpleConfig across the four
user_config shapes that reproduce (and don't reproduce) the crash, that
later writes survive a disabled port, that plugins.swapserver_gui.enabled
is not collaterally pruned, a static guard against direct SWAPSERVER_PORT
assignment in qt.py, version formatting, and an end-to-end check that a
stamped version reads back when the zip is loaded the way Electrum loads
it. 83 tests pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013zETFC6MhnsCuxvQDGxqV5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
1. Fix: "Save settings" crashes when the HTTP port is disabled
Saving settings with the HTTP port set to
0("disabled") crashed with:Root cause
on_saveturned a spinbox value of0intoNone.SWAPSERVER_PORTis registered by Electrum's bundled swapserver plugin as the dotted keyplugins.swapserver.port, so assigningNoneroutes intoSimpleConfig's key-deletion branch, whose recursion walks the dotted path without checking that each intermediate dict exists:This plugin only ever writes
plugins.swapserver_gui.*, so unless the user has separately enabled and configured the bundled swapserver plugin,plugins.swapserverdoes not exist and the delete always raises. Reproduced against a realSimpleConfig:user_configstate{}(fresh config)AttributeError: ... has no attribute 'get'{'plugins': {'swapserver_gui': {...}}}... has no attribute 'pop'← the reported traceback{'plugins': {'swapserver': {'port': 5455}}}{'plugins': {'swapserver': {}}}Because the port write came first, its crash also silently discarded the fee, PoW-target and relay writes — nothing was saved at all.
Fix
A disabled port is now stored as
0, neverNone. Every reader tests the key for truthiness (electrum/submarine_swaps.py,electrum/plugins/swapserver/server.py, and this plugin'scan_run/start_server/status), so0is behaviourally identical but takes the ordinary write path. Configs already holdingNonestill load correctly.This is an upstream Electrum bug; the workaround belongs here because users run released AppImages we cannot patch.
Two supporting changes:
swapserver_gui.save_settings(). PyQt6 is unavailable in CI, so anything reachable only throughqt.pyis effectively untestable — which is exactly why the existing suite (built on an attribute-bag config fake) never caught this.on_savenow catches save failures, logs them, and reports them viashow_errorinstead of letting them escape into the Qt event loop as an unhandled-exception traceback.2. Feature: plugin version in the tab header
The tab header now shows the installed version, dimmed, between the status text and the enable/disable button:
The git tag is the single source of truth.
_version.pyis committed with the placeholderdev;contrib/make_zip.shstamps the real value into a staging copy while building the archive, so the working tree is never modified. CI stamps${GITHUB_REF_NAME#v}onv*tags anddev-g<sha7>otherwise, then verifies the stamp actually landed in the zip.So a release asset can never disagree with the tag it was built from, a branch/PR zip is always traceable to its commit, and cutting a release needs no in-tree bump — just push the tag. Release builds render
v0.2.1; dev stamps render verbatim (vdevwould read as a release).Tests
83 pass (
python3 -m unittest discover -s tests). New coverage:tests/test_settings_save.py— drives a realSimpleConfig(not the fake) across all fouruser_configshapes above; asserts later writes survive a disabled port, values persist across a simulated restart, andplugins.swapserver_gui.enabledis not collaterally pruned by upstream's empty-parent-dict cleanup (that would uninstall the plugin on save). Plus a static AST guard against directSWAPSERVER_PORTassignment inqt.py— verified to fire on the pre-fix code at line 223, the exact line from the traceback.tests/test_version.py— version formatting, and the stamping contractmake_zip.shdepends on (drift there would silently shipdevin a release).tests/test_zip_plugin_load.py— end-to-end: a stamped version reaches the running module when the zip is loaded the way Electrum loads it; unstamped builds keep the placeholder; the working tree stays clean; malformed version strings fail the build rather than emitting a broken_version.py.Not covered
The
QLabelrendering and theshow_errordialog itself cannot be exercised — PyQt6 is not installable in this environment or in CI. The logic behind both (version formatting, the persist function and its failure mode) is covered; only the widget wiring is not.🤖 Generated with Claude Code
https://claude.ai/code/session_013zETFC6MhnsCuxvQDGxqV5