Fix "Save settings" crash on disabled port; show plugin version in tab

.github/workflows/build-plugin.yml

@@ -52,10 +52,35 @@ jobs:
           cd electrum_swapgui
           python -m unittest discover -s tests -v
 
+      - name: Determine plugin version
+        id: version
+        run: |
+          cd electrum_swapgui
+          if [[ "$GITHUB_REF" == refs/tags/v* ]]; then
+            # The tag is the single source of truth for a release version.
+            version="${GITHUB_REF_NAME#v}"
+            if [ -z "$version" ]; then
+              echo "::error::tag $GITHUB_REF_NAME yields an empty version"
+              exit 1
+            fi
+          else
+            # Not a release: stamp a dev marker traceable to the commit, so any
+            # zip handed out from a branch or PR build can be identified.
+            version="dev-g$(git rev-parse --short=7 HEAD)"
+          fi
+          echo "version=$version" >> "$GITHUB_OUTPUT"
+          echo "plugin version: $version"
+
       - name: Build plugin zip
         run: |
           cd electrum_swapgui
-          bash contrib/make_zip.sh "$GITHUB_WORKSPACE/dist"
+          bash contrib/make_zip.sh "$GITHUB_WORKSPACE/dist" "${{ steps.version.outputs.version }}"
+
+      - name: Verify the stamped version
+        run: |
+          cd "$GITHUB_WORKSPACE"
+          unzip -p dist/swapserver_gui.zip swapserver_gui/_version.py \
+            | grep -Fx '__version__ = "${{ steps.version.outputs.version }}"'
 
       - name: Upload plugin zip artifact
         uses: actions/upload-artifact@v4

README.md

@@ -41,13 +41,33 @@ listener can be shut down cleanly (`swapserver_gui.py:ManagedHttpSwapServer`).
 plugins/swapserver_gui/
   manifest.json        # available_for: ["qt"], min_electrum_version
   __init__.py          # registers plugins.swapserver_gui.autostart
+  _version.py          # plugin version, stamped from the git tag at build time
   swapserver_gui.py    # GUI-agnostic server lifecycle + history helpers
   qt.py                # the "Swap Server" tab + Plugin(load_wallet/close_wallet)
 tests/                 # unit + HTTP end-to-end tests (no GUI needed)
 contrib/make_zip.sh    # build the external-plugin zip
 contrib/regtest_demo/  # one-command local regtest + nostr demo
 ```
 
+## Versioning
+
+The tab header shows the installed plugin version, so users can tell which build
+they are on. **The git tag is the single source of truth.** `_version.py` is
+committed with the placeholder `dev`; `contrib/make_zip.sh` stamps the real value
+into a staging copy while building the archive, so the working tree is never
+modified:
+
+```bash
+bash contrib/make_zip.sh dist 0.2.1   # -> tab shows "v0.2.1"
+bash contrib/make_zip.sh dist         # -> tab shows "dev"
+```
+
+CI passes `${GITHUB_REF_NAME#v}` for `refs/tags/v*` builds and `dev-g<sha7>`
+otherwise, then verifies the stamp landed in the zip. A release asset therefore
+can never disagree with the tag it was built from, and any zip from a branch or
+PR build is traceable to its commit. To cut a release, push a `vX.Y.Z` tag —
+nothing in the tree needs bumping.
+
 ## Installing
 
 **As an external zip (for users):**
@@ -83,6 +103,24 @@ ELECTRUM_SRC=/path/to/electrum python3 -m unittest discover -s tests -v
 - `tests/test_http_endpoint.py` — starts the real `ManagedHttpSwapServer`, does a
   live `GET /getpairs`, validates the JSON, and asserts the port is released on
   stop.
+- `tests/test_settings_save.py` — persisting settings against a **real**
+  `SimpleConfig`, including disabling the HTTP port (see the note below).
+- `tests/test_version.py` / `tests/test_zip_plugin_load.py` — version formatting
+  and the end-to-end build-time stamping chain.
+
+### A note on the HTTP port
+
+A disabled HTTP port is stored as `0`, never `None`. `SWAPSERVER_PORT` is the
+dotted key `plugins.swapserver.port`, and assigning `None` to it routes into
+`SimpleConfig`'s key-*deletion* branch, whose recursion dereferences the
+intermediate `plugins.swapserver` dict without checking that it exists — which it
+usually does not, since this plugin only writes `plugins.swapserver_gui.*`. The
+result was `AttributeError: 'NoneType' object has no attribute 'pop'` on every
+"Save settings" with the port disabled. Every reader tests the key for
+truthiness, so `0` is equivalent and takes the safe write path. This is an
+upstream Electrum bug; the workaround lives here because users run released
+AppImages. All writes go through `swapserver_gui.save_settings`, and a static
+guard in the tests keeps direct assignments from creeping back into `qt.py`.
 
 For a **full GUI e2e** — a real Electrum Qt GUI (offscreen) with the plugin
 loaded, auto-starting the server and serving a real `/getpairs` — see

contrib/make_zip.sh

@@ -15,25 +15,72 @@
 # The zip is a plain archive: no signing key or secret is required to build it.
 # The end user authorises it locally (with their own plugin password) the first
 # time they install it via Electrum's Plugins dialog.
+#
+# Usage:  make_zip.sh [OUT_DIR] [VERSION]
+#
+# VERSION (or the PLUGIN_VERSION environment variable) is stamped into
+# swapserver_gui/_version.py inside the archive, and is what the plugin shows in
+# its tab header.  The stamping happens in a temporary staging copy, so building
+# never modifies the working tree.  With no version given the committed
+# placeholder ('dev') is kept.
 set -euo pipefail
 
 HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 ROOT="$(dirname "$HERE")"                 # electrum_swapgui/ (our repo)
 PLUGIN_DIR="$ROOT/plugins/swapserver_gui"
 OUT_DIR="${1:-$ROOT/dist}"
+VERSION="${2:-${PLUGIN_VERSION:-}}"
 OUT="$OUT_DIR/swapserver_gui.zip"
 
 if [ ! -f "$PLUGIN_DIR/manifest.json" ]; then
     echo "error: $PLUGIN_DIR/manifest.json not found" >&2
     exit 1
 fi
+if [ ! -f "$PLUGIN_DIR/_version.py" ]; then
+    echo "error: $PLUGIN_DIR/_version.py not found" >&2
+    exit 1
+fi
 
 mkdir -p "$OUT_DIR"
+# Resolve to an absolute path before we cd into the staging dir below.
+OUT_DIR="$(cd "$OUT_DIR" && pwd)"
+OUT="$OUT_DIR/swapserver_gui.zip"
 rm -f "$OUT"
 
-# Zip from plugins/ so paths are prefixed with 'swapserver_gui/'. Exclude
-# caches and any local pyc files.
-( cd "$ROOT/plugins" && \
+# Stage the package so the version can be stamped without touching the tree.
+STAGE="$(mktemp -d)"
+trap 'rm -rf "$STAGE"' EXIT
+cp -r "$PLUGIN_DIR" "$STAGE/swapserver_gui"
+rm -rf "$STAGE/swapserver_gui/__pycache__"
+
+if [ -n "$VERSION" ]; then
+    # Reject anything that would need quoting inside the Python string literal,
+    # so a bad tag fails the build instead of emitting a broken _version.py.
+    case "$VERSION" in
+        *[!A-Za-z0-9.+_-]*)
+            echo "error: refusing to stamp unsafe version string: $VERSION" >&2
+            exit 1 ;;
+    esac
+    python3 - "$STAGE/swapserver_gui/_version.py" "$VERSION" <<'PY'
+import re, sys
+path, version = sys.argv[1], sys.argv[2]
+with open(path, encoding="utf-8") as f:
+    src = f.read()
+new, n = re.subn(r'^__version__ = ".*"$', f'__version__ = "{version}"',
+                 src, count=1, flags=re.MULTILINE)
+if n != 1:
+    raise SystemExit(f"error: could not find __version__ assignment in {path}")
+with open(path, "w", encoding="utf-8") as f:
+    f.write(new)
+PY
+    echo "stamped version: $VERSION"
+else
+    echo "no version given; keeping the committed placeholder"
+fi
+
+# Zip from the staging dir so paths are prefixed with 'swapserver_gui/'.
+# Exclude caches and any local pyc files.
+( cd "$STAGE" && \
   zip -r -X "$OUT" swapserver_gui \
       -x '*/__pycache__/*' -x '*.pyc' >/dev/null )
 

plugins/swapserver_gui/_version.py

@@ -0,0 +1,34 @@
+#!/usr/bin/env python
+#
+# swapserver_gui - a Qt GUI plugin for Electrum's submarine swap server.
+# This file is released into the public domain (The Unlicense); see LICENSE.
+#
+# The plugin version, stamped into the zip at build time.
+#
+# The value committed here is deliberately the placeholder ``dev``: the release
+# version is the *git tag*, and ``contrib/make_zip.sh`` rewrites the literal
+# below in a staging copy of the package while building the archive (the working
+# tree is never modified).  ``.github/workflows/build-plugin.yml`` passes
+# ``${GITHUB_REF_NAME#v}`` for ``refs/tags/v*`` builds and ``dev-g<sha7>`` for
+# every other build, so a release zip can never disagree with the tag it was
+# built from and a dev zip is always traceable to the commit that produced it.
+#
+# Keep this module free of imports: ``contrib/make_zip.sh`` rewrites it with a
+# line-oriented substitution, and the test suite parses it with ``ast`` without
+# executing it.
+
+__version__ = "dev"
+
+
+def format_version(version: str) -> str:
+    """Render a version string for display in the GUI.
+
+    Real releases are numeric and conventionally shown with a leading ``v`` to
+    match the git tag / GitHub release name (``0.2.1`` -> ``v0.2.1``).  The
+    placeholder and CI dev stamps (``dev``, ``dev-g1a2b3c``) are not versions in
+    that sense, so they are shown verbatim -- ``vdev`` would read as a release.
+    """
+    version = (version or "").strip()
+    if not version:
+        return "unknown"
+    return "v" + version if version[0].isdigit() else version

plugins/swapserver_gui/qt.py

@@ -22,10 +22,12 @@
 
 from .swapserver_gui import (
     SwapServerGuiPlugin, SwapServerError, get_swap_history, get_swap_summary,
+    save_settings,
 )
 # NB: not ``from . import pow as swap_pow`` -- that form breaks when Electrum
 # loads this plugin from a zip.  See the long comment in swapserver_gui.py.
 swap_pow = importlib.import_module('.pow', __package__)
+_version = importlib.import_module('._version', __package__)
 
 # Above this many bits a proof-of-work grind stops being a "wait a few minutes"
 # affair (see the estimate shown next to the spinbox), so we ask for confirmation.
@@ -62,6 +64,15 @@ def __init__(self, plugin: 'Plugin', window: 'ElectrumWindow') -> None:
         self.status_label = QLabel()
         self.status_label.setTextFormat(Qt.TextFormat.RichText)
         header.addWidget(self.status_label, 1)
+        # Which build of the plugin this is. Stamped into the zip at build time
+        # from the git tag, so it matches the GitHub release (see _version.py).
+        self.version_label = QLabel(_version.format_version(_version.__version__))
+        self.version_label.setToolTip(
+            _("Installed version of the Swap Server (GUI) plugin."))
+        self.version_label.setTextInteractionFlags(
+            Qt.TextInteractionFlag.TextSelectableByMouse)
+        self.version_label.setEnabled(False)  # renders dimmed, like secondary text
+        header.addWidget(self.version_label)
         self.toggle_btn = QPushButton()
         self.toggle_btn.clicked.connect(self.on_toggle)
         header.addWidget(self.toggle_btn)
@@ -200,10 +211,14 @@ def _relays_from_widget(self) -> str:
         return ",".join(parts)
 
     def on_save(self) -> None:
-        new_port = self.port_spin.value() or None
+        # 0 == "disabled"; never None. Assigning None to SWAPSERVER_PORT crashes
+        # Electrum -- see the docstring of swapserver_gui.save_settings.
+        new_port = self.port_spin.value()
         new_relays = self._relays_from_widget()
         new_pow = self.pow_spin.value()
-        old_port = self.config.SWAPSERVER_PORT
+        # Normalise the old value too: configs written by earlier versions of
+        # this plugin may hold None, which must compare equal to a disabled 0.
+        old_port = int(self.config.SWAPSERVER_PORT or 0)
         old_relays = self.config.NOSTR_RELAYS or ""
         old_pow = int(self.config.SWAPSERVER_POW_TARGET or 0)
 
@@ -219,11 +234,25 @@ def on_save(self) -> None:
                         "Continue?")):
                 return
 
-        # persist
-        self.config.SWAPSERVER_PORT = new_port
-        self.config.SWAPSERVER_FEE_MILLIONTHS = self.fee_spin.value()
-        self.config.SWAPSERVER_POW_TARGET = new_pow
-        self.config.NOSTR_RELAYS = new_relays
+        # persist. A failure here must not escape into the Qt event loop: it
+        # would surface as an unhandled-exception dialog (or just a traceback on
+        # the console) with the user left unsure what, if anything, was saved.
+        try:
+            save_settings(
+                self.config,
+                port=new_port,
+                fee_millionths=self.fee_spin.value(),
+                pow_target=new_pow,
+                relays=new_relays,
+            )
+        except Exception as e:
+            self.plugin.logger.exception("failed to save swap server settings")
+            self.window.show_error(
+                _("Could not save the swap server settings: {}").format(e))
+            # Put the widgets back in sync with what is actually stored, so the
+            # form never claims a value that was not persisted.
+            self.load_settings_into_widgets()
+            return
         self.load_settings_into_widgets()
         # A port/relay change needs a server restart. So does *raising* the PoW
         # target, since the running server is announcing with a nonce that no

plugins/swapserver_gui/swapserver_gui.py

@@ -474,6 +474,57 @@ def status(self) -> Dict[str, Any]:
         return data
 
 
+def save_settings(
+        config: 'SimpleConfig',
+        *,
+        port: int,
+        fee_millionths: int,
+        pow_target: int,
+        relays: str,
+) -> None:
+    """Persist the swap-server settings edited in the GUI.
+
+    Lives here rather than in ``qt.py`` so it can be tested against a real
+    ``SimpleConfig``: PyQt6 is not installed in CI, so anything importable only
+    through ``qt.py`` is effectively untestable.
+
+    ``port`` MUST be an ``int``; ``0`` means "HTTP endpoint disabled".  Passing
+    ``None`` for a disabled port -- the obvious way to express it, since
+    ``SWAPSERVER_PORT`` defaults to ``None`` -- crashes Electrum:
+
+        AttributeError: 'NoneType' object has no attribute 'pop'
+
+    ``SWAPSERVER_PORT`` is registered by Electrum's bundled swapserver plugin as
+    the *dotted* key ``plugins.swapserver.port``
+    (electrum/plugins/swapserver/__init__.py).  Assigning ``None`` to a ConfigVar
+    routes into ``SimpleConfig._set_key_in_user_config``'s key-*deletion* branch,
+    whose recursion walks the dotted path without checking that each intermediate
+    dict exists (electrum/simple_config.py, ``delete_key``)::
+
+        prefix, suffix = key.split('.', 1)
+        d2 = d.get(prefix)          # None when 'plugins.swapserver' was never written
+        empty = delete_key(d2, suffix)
+
+    This plugin only ever writes ``plugins.swapserver_gui.*``, so unless the user
+    has separately enabled and configured the *bundled* swapserver plugin, the
+    ``plugins.swapserver`` sub-dict does not exist and the delete always raises.
+
+    Storing ``0`` takes the ordinary write branch and is behaviourally identical:
+    every reader of the key tests it for truthiness rather than comparing it to
+    ``None`` -- ``electrum/submarine_swaps.py`` (``if self.config.SWAPSERVER_PORT:``),
+    ``electrum/plugins/swapserver/server.py``, and this plugin's own ``can_run``,
+    ``start_server`` and ``status``.
+
+    This is an upstream Electrum bug, but the fix has to live here: users run
+    released AppImages we cannot patch.
+    """
+    assert isinstance(port, int), f"port must be an int, got {port!r}"
+    config.SWAPSERVER_PORT = port
+    config.SWAPSERVER_FEE_MILLIONTHS = int(fee_millionths)
+    config.SWAPSERVER_POW_TARGET = int(pow_target)
+    config.NOSTR_RELAYS = relays
+
+
 def get_swap_history(wallet: 'Abstract_Wallet') -> List[Dict[str, Any]]:
     """Confirmed swaps served by this node (mirrors the bundled swapserver
     plugin's ``get_history`` command, but as a plain sync helper)."""