Skip to content

fix: show specific electrum error toasts - #1287

Open
jvsena42 wants to merge 10 commits into
masterfrom
fix/1177-electrum-error-toasts
Open

jvsena42 wants to merge 10 commits into
masterfrom
fix/1177-electrum-error-toasts

Conversation

@jvsena42

@jvsena42 jvsena42 commented Sep 16, 2026

Copy link
Copy Markdown
Member

Fixes #1177
Related to #1275

This PR shows the specific cause when connecting to a custom Electrum server fails, instead of the same generic message for every error.

Description

  • Shows a network-specific message when the server is on a different Bitcoin network, so the user knows to pick a server for the network Bitkit is using.
  • Shows a protocol-specific message when the TLS handshake fails against a peer that does not speak TLS, so the user knows to check that TCP or TLS matches the server port.
  • Shows a certificate-specific message when the handshake fails on certificate validation instead, so a self-signed Fulcrum or electrs on its SSL port is not told to check a protocol and port that are already correct.
  • Asks JSSE to verify the certificate's hostname during the probe handshake. A raw SSLSocket validates the chain but not the name, so a server reached by IP or alias that presents a CA-valid certificate for a different name used to probe clean and was only rejected afterwards by the node's own Electrum client, which validates the domain — the user got the generic message after a full node restart. It now fails in the probe and shows the certificate message.
  • Keeps the generic "could not establish a connection" message for unreachable hosts, non-Electrum servers and any other failure.
  • Moves the success and error toasts from the screen into the view model so the toast can use the probe's error instead of a plain success/failure result. Toast titles and test tags (ElectrumUpdatedToast, ElectrumErrorToast) are unchanged, so the e2e settings tests still match.
  • Returns Result.failure from LightningRepo.restartWithElectrumServer when the settings write after a successful restart fails, so that failure reaches the toast instead of escaping the caller's coroutine.
  • Takes the probe's SSLSocketFactory as a dependency instead of reading SSLSocketFactory.getDefault() inside it. Production still gets the platform default through Hilt; the change is so the TLS tests can hand the probe a factory of their own rather than swapping the JVM's default SSL configuration underneath it.

Out of Scope

  • Settings › Advanced › Electrum Server: choosing TCP on a TLS-only port still shows the generic message, because the probe reports that case as "not Electrum" rather than a protocol mismatch.
  • The new certificate cases are not in the journey below: they need a self-hosted Electrum server with a self-signed certificate, or one whose certificate names another host, which the journey's preconditions do not set up.
  • Settings › Advanced › Rapid Gossip Sync: still uses the old screen-side result toast pattern, and restartWithRgsServer still writes its setting unguarded; not part of the issue.
  • Locales: only the English strings are added; translations follow through the usual localization flow.
  • iOS: parity for the specific network and protocol messages on manual connect is a possible follow-up in bitkit-ios, along with porting the new journey below.

Design

Preview

Figma Before: wrong network After: wrong network
Figma Electrum server screen Before: generic toast for wrong network After: network mismatch toast
After: TLS on plain TCP port After: unreachable host
After: protocol mismatch toast After: generic toast for unreachable host

Captured on a dev (regtest) build connected to the staging Electrum server. The wrong-network case uses electrum.blockstream.info:50002 over TLS, the protocol case uses electrum.blockstream.info:50001 over TLS, and the unreachable case uses 10.255.255.1:9999. TLS is selected before the port is typed, because selecting a protocol rewrites a port that is still a default one.

QA Notes

Manual Tests

  • 1. Settings → Advanced → Electrum Server → select TLS, then enter electrum.blockstream.info / 50002 (selecting a protocol rewrites a default port, so set it first) → Connect To Host: warning toast "This server is on a different Bitcoin network. Choose a server for the network Bitkit is using."
  • 2. Electrum Server → keeping TLS selected, enter electrum.blockstream.info / 50001 → Connect To Host: warning toast "Secure connection failed. Check that the protocol (TCP or TLS) matches the server port."
  • 3. Electrum Server → enter 10.255.255.1 / 9999 → Connect To Host: after ~5s, warning toast "Bitkit could not establish a connection to Electrum."
  • 4. regression: After each failure above: "Currently connected to" still shows the previous server, and back → Electrum Server row is unchanged.
  • 5. regression: Electrum Server → enter a valid server for the current network → Connect To Host: success toast "Electrum Server Updated" and the connected server updates.
  • 6. Needs a self-hosted Electrum with a self-signed certificate: Electrum Server → enter that host / its SSL port, TLS → Connect To Host: warning toast "This server's certificate is not trusted. Use a server with a trusted certificate, or connect to its TCP port." Before this PR, and before the last commit on this branch, that case showed the protocol message instead.
  • 7. Needs an Electrum server reachable by IP whose TLS certificate is CA-valid but issued for another name: Electrum Server → enter that IP / its SSL port, TLS → Connect To Host: the same certificate toast, now without a node restart first.

The steps above are also written as a journey, committed in this PR as
journeys/settings/electrum-server-error-toasts.xml, that passes on this branch on a Pixel 9
emulator. Its row is added to the suites table and the cross-platform table in journeys/README.md.

journeys/settings/electrum-server-error-toasts.xml
diff --git a/journeys/settings/electrum-server-error-toasts.xml b/journeys/settings/electrum-server-error-toasts.xml
new file mode 100644
index 000000000..eb3431ba3
--- /dev/null
+++ b/journeys/settings/electrum-server-error-toasts.xml
@@ -0,0 +1,40 @@
+<journey name="electrum server error toasts">
+  <description>
+    Verifies that a rejected custom Electrum server shows a warning toast whose description names the
+    cause: a wrong-network server and a TLS handshake against a plain TCP port get their own messages,
+    and an unreachable host keeps the generic one. The server is probed before the node stops, so a
+    rejected server is never persisted and the wallet stays on its current server.
+
+    Precondition: onboarded dev wallet on the default staging regtest server (Settings shows Electrum
+    Server "Auto"). Start on the wallet home screen. Toasts never appear in `android layout` and last
+    about 2s, so assert them from a screen recording or a screenshot taken 2-6s after tapping connect.
+    Type hosts in short chunks and verify the field text; `adb shell input text` drops characters.
+    Do not tap "Reset To Default" unless the starting server was the default. Tapping a protocol row
+    rewrites the port whenever it is empty or still one of 51002/50002/51001/50001, to that
+    protocol's default for the build's network (60002 for TLS on the regtest dev build), so always
+    set the protocol before typing the port and never re-tap a protocol row afterwards.
+  </description>
+  <actions>
+    <action>Tap the menu icon (testTag "HeaderMenu")</action>
+    <action>Tap Settings (testTag "DrawerSettings")</action>
+    <action>Tap the Advanced tab (testTag "Tab-advanced")</action>
+    <action>Tap Electrum Server (testTag "ElectrumConfig")</action>
+    <action>Verify "Currently connected to" is visible and note the connected server (testTag "Connected")</action>
+    <action>Select "TLS" under protocol (testTag "ElectrumProtocol")</action>
+    <action>Replace the host (testTag "HostInput") with "electrum.blockstream.info" and the port (testTag "PortInput") with "50002", then verify the port field still reads "50002"</action>
+    <action>Hide the keyboard and tap Connect To Host (testTag "ConnectToHost")</action>
+    <action>Verify a warning toast "Electrum Connection Failed" appears with the description "This server is on a different Bitcoin network. Choose a server for the network Bitkit is using."</action>
+    <action>Verify the connected server (testTag "Connected") is unchanged</action>
+    <action>Replace the port (testTag "PortInput") with "50001" without tapping a protocol row, keeping "TLS" selected, and verify the port field still reads "50001"</action>
+    <action>Hide the keyboard and tap Connect To Host (testTag "ConnectToHost")</action>
+    <action>Verify a warning toast "Electrum Connection Failed" appears with the description "Secure connection failed. Check that the protocol (TCP or TLS) matches the server port."</action>
+    <action>Replace the host (testTag "HostInput") with "10.255.255.1" and the port (testTag "PortInput") with "9999"</action>
+    <action>Hide the keyboard and tap Connect To Host (testTag "ConnectToHost")</action>
+    <action>Verify a warning toast "Electrum Connection Failed" appears with the description "Bitkit could not establish a connection to Electrum." after the ~5s connect timeout</action>
+    <action>Verify the connected server (testTag "Connected") is unchanged</action>
+    <action>Tap Reset To Default (testTag "ResetToDefault")</action>
+    <action>Verify the host and port fields show the default server, which is the connected one, and Connect To Host (testTag "ConnectToHost") is disabled</action>
+    <action>Tap back (testTag "NavigationBack")</action>
+    <action>Verify the Electrum Server row (testTag "ElectrumConfig") still shows "Auto"</action>
+  </actions>
+</journey>

Automated Checks

  • Unit tests added: ElectrumConfigViewModelTest.kt covers the toast type, title, description, test tag and loading state for network mismatch, protocol mismatch, untrusted certificate, unreachable, not-Electrum, other failures and success.
  • ElectrumProbeServiceTest.kt pins how a failed TLS handshake is classified: a certificate path failure or an unverified peer becomes UntrustedCertificate, while a handshake timeout or a non-TLS reply stays ProtocolMismatch. It also probes a real TLS Electrum server whose certificate is trusted but issued for another name and asserts UntrustedCertificate, plus the paired case of a certificate that does name the host, which must still succeed. Reverting the hostname-verification line locally fails the first of those two and leaves the second passing. Both TLS tests build their own SSLContext from the JDK's own JSSE provider and pass its socket factory to the probe, so they no longer read or write any JVM-wide SSL state: Robolectric installs Conscrypt as the JVM's first security provider and never removes it, which made the paired success case fail on Linux CI while passing on macOS.
  • LightningRepoTest.kt pins that a failed settings write after a successful restart comes back as Result.failure instead of throwing out of the returned result.
  • Ran locally: just compile, just test, just lint.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jvsena42 jvsena42 self-assigned this Sep 16, 2026
@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

This PR should not merge until thrown repository exceptions reset the loading state and produce failure feedback.

Findings

  1. P1 Exceptions Leave Loading Stuck

Summary

This PR moves custom Electrum connection-result toast handling into the view model and selects specific descriptions for network and TLS protocol mismatches.

  • Adds dedicated network- and protocol-mismatch string resources.
  • Preserves generic messaging for other connection failures.
  • Adds view-model tests for successful and failed connection-result toasts.
  • Introduces an exception-handling regression when the repository throws rather than returning a failed Result.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[User submits Electrum server] --> B[View model starts loading]
    B --> C[LightningRepo probes and restarts]
    C -->|Success Result| D[Show updated toast]
    C -->|Failure Result| E{Probe error type}
    E -->|Network mismatch| F[Show network guidance]
    E -->|Protocol mismatch| G[Show TCP/TLS guidance]
    E -->|Other failure| H[Show generic error]
    C -->|Thrown exception| I[Coroutine exits]
    I --> J[Loading remains enabled and no toast appears]
Loading

Reviews (1) · Last reviewed commit: "fix: show specific electrum error toasts"

@github-actions

github-actions Bot commented Sep 16, 2026

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 552e06c (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — no HIGH/MEDIUM findings at head a008aae12. (Posted as a comment because GitHub does not allow approving your own PR.)

Inline: LOW — Certificate failures on a correct TLS port get the "check TCP or TLS" message

Checked: Traced the error types from ElectrumProbeService.probe through LightningRepo.validateElectrumServer and restartWithElectrumServer into the view model: NetworkMismatch and ProtocolMismatch are thrown unwrapped from inside runSuspendCatching (the inner runCatching at ElectrumProbeService.kt:91-93 re-throws existing ElectrumProbeErrors as-is), and restartWithElectrumServer returns the probe failure without wrapping, so the when in toServerErrorDescriptionRes sees the concrete subclass. restartWithElectrumServer has a single caller (ElectrumConfigViewModel.connectToServer), and on master that caller's runCatching already turned a thrown settings write into an error toast, so the new Result.failure return keeps the user-facing outcome identical while fixing the stuck-loading path greptile flagged; runSuspendCatching still propagates cancellation. ElectrumServer.fromUserInput cannot throw, so dropping the view model's runCatching leaves no unguarded throw before the repository call. Toasts go through ToastEventBus, collected by the activity-scoped AppViewModel, and the success path's settings write re-seeds host/port through observeState as before.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — no HIGH/MEDIUM findings at head 57b4e9415. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Follow-up on 57b4e94: the certificate case is now classified separately. One pre-existing LOW (the probe skips hostname verification) is posted as a reply on the earlier thread. Re-reviewed at 57b4e94. toTlsProbeError walks the handshake failure's cause chain (bounded at 8, cycle-safe) and maps CertificateException, CertPathValidatorException and SSLPeerUnverifiedException to the new UntrustedCertificate. On Conscrypt an untrusted or self-signed chain arrives as SSLHandshakeException -> CertificateException -> CertPathValidatorException, and an expired cert adds CertificateExpiredException (a CertificateException subclass), so both now get the certificate toast. The genuine mismatch cases keep ProtocolMismatch: a plaintext server answering the ClientHello fails with an SSLException/SSLProtocolException that has no certificate cause, and a handshake timeout is a SocketTimeoutException. TLS against a TCP port never reaches the handshake (TCP returns the plain socket at ElectrumProbeService.kt:111), so it still ends as NotElectrum as before. settings__es__server_error_certificate sits alphabetically before _description in strings.xml and is resolved through toServerErrorDescriptionRes, not hardcoded. The new probe tests exercise the new classifier and the view-model test fails on the old when (it would fall to the generic string). The rest of the PR (repository runSuspendCatching around the settings write, toast mapping for network/protocol) is unchanged by this commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Self contained change

@jvsena42
jvsena42 requested review from ovitrif and removed request for ovitrif September 18, 2026 12:36
ovi-reviewer[bot]

This comment was marked as resolved.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jvsena42

Copy link
Copy Markdown
Member Author

The CI failure of ElectrumProbeServiceTest > probe accepts a certificate issued for the host it connected to was the test harness, not the production change. Fixed in f9ce270.

Cause. Robolectric installs Conscrypt as the JVM's first security provider and never removes it, and unit tests share one JVM, so every test class that runs after a Robolectric one resolves SSLContext.getInstance("TLS"), TrustManagerFactory and friends to Conscrypt. The TLS harness built its fake Electrum server from that provider, so on CI the server socket was a Conscrypt one. A Conscrypt server socket cannot finish a handshake on a current JDK — it reflects into java.net.InetAddress, which the module system refuses without --add-opens — so the server dropped the connection right after the client handshake and the probe read EOF, reporting NotElectrum. The wrong-name test still passed because the client rejects the certificate name before the server ever gets that far, which is why only the positive case failed.

It passes locally because Conscrypt ships no darwin-aarch64 native in conscrypt-openjdk-uber 2.5.2, so Robolectric's install throws UnsatisfiedLinkError on an Apple Silicon machine and SunJSSE stays in charge. Test class order mattered too: on a clean build the classes run roughly alphabetically and this one lands at position 90, but locally an incremental build had hoisted it to position 1, ahead of every Robolectric class.

Reproduced by running the compiled test classes on the Gradle test classpath under Linux x86_64: LightningNodeServiceTest followed by ElectrumProbeServiceTest fails the positive case, the same pair passes on macOS, and ElectrumProbeServiceTest alone passes on both.

Fix. The TLS tests now build one SSLContext pinned to the JDK's own JSSE provider, holding both the key managers that serve the certificate and the trust managers that trust it, and use it for both ends. ElectrumProbeService takes its SSLSocketFactory as a constructor dependency — Hilt provides the platform default, so production behaviour is unchanged — and the tests hand it the factory from that context. SSLContext.setDefault and its teardown restore are gone, so the tests neither read nor write JVM-wide SSL state.

The production change is unchanged and still correct: hostname verification behaved identically under SunJSSE and Conscrypt, on JDK 17 and 21, on macOS and Linux. Negative control rerun: with the endpointIdentificationAlgorithm line removed, probe rejects a certificate issued for another host fails and probe accepts a certificate issued for the host it connected to passes, so the pair still discriminates.

Local: just compile, just test (2608 passing, 0 failures), just lint all green.

jvsena42 and others added 2 commits September 18, 2026 10:47
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rror-toasts

# Conflicts:
#	journeys/README.md

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — no blocking HIGH/MEDIUM findings at head 2e36ed64b. (Posted as a comment because GitHub does not allow approving your own PR.)

Inline: LOW — Selecting TLS after typing port 50002 silently rewrites the port to 60002, so the journey's stated state does not hold

Checked: Follow-up on 5291fe2 (hostname verification), f9ce270 (tests) and 3c03cfc (journey): the earlier hostname LOW is fixed. Re-reviewed the three own commits since 57b4e94. Hostname fix (5291fe2): endpointIdentificationAlgorithm = "HTTPS" is applied through the get/set SSLParameters round trip at ElectrumProbeService.kt:125, before startHandshake() at :126, on the socket built with createSocket(plain, server.host, port, true), so the name checked is the user-typed host. On Android this reaches Conscrypt's TrustManagerImpl.checkTrusted(chain, authType, session, parameters) via RootTrustManager -> NetworkSecurityTrustManager -> getTrustedChainForServer(certs, authType, socket) (the app has no network security config), which throws CertificateException on a name mismatch; that is wrapped as SSLHandshakeException and isCertificateFailure() (:242-253) maps it to UntrustedCertificate -> settings__es__server_error_certificate, so a hostname mismatch shows the certificate toast, as the PR body states. Self-signed certs are unchanged: chain validation still fails with a CertPathValidatorException/CertificateException cause, same class as before this commit; hostname verification never turns a previously-rejected chain into an accepted one. IP hosts: the node's client is ldk-node 0.7.0-rc.66 -> electrum-client 0.24 with use-rustls-ring and ConfigBuilder::new() defaults (validate_domain = true, ldk-node/src/chain/electrum.rs:565-568), which passes the URL's host to ServerName::try_from (IP literal -> IpAddress, requires an iPAddress SAN; DNS name -> dNSName SAN, no CN fallback). Conscrypt's OkHostnameVerifier has the same IP-SAN/DNS-SAN split and no CN fallback since API 28 (minSdk), so a server the probe now rejects is one the node would reject after the restart; the paired positive test (IP SAN 127.0.0.1) shows a correctly named certificate still passes. .onion hosts over TLS never reach the TLS branch: plain.connect fails DNS resolution and reports Unreachable (:109-113), unchanged. Provider decoupling (f9ce270): HttpModule.provideSslSocketFactory returns SSLSocketFactory.getDefault(), the same factory the probe called inline before, and ElectrumProbeService is the only consumer of the binding. Tests: tlsContext pins SunJSSE for the context, key managers and trust managers and hands its socket factory to the probe, so no JVM-wide SSL state is read or written; the negative test discriminates because the only difference from the passing positive test is the SAN, so it can only fail through name checking. CI testDevDebugUnitTest passed at head 2e36ed6. Journey: every testTag it names exists at head (HeaderMenu, DrawerSettings, Tab-advanced via Tab-${tab.name.lowercase()}, ElectrumConfig, Connected, HostInput, PortInput, ElectrumProtocol on the TLS row, ConnectToHost, ResetToDefault, NavigationBack), the Auto label logic and the disabled-Connect assertion match AdvancedSettingsViewModel.electrumHost and enabled = !isLoading && hasEdited || !isConnected, the suites and cross-platform tables are updated, and no bitkit-ios twin exists.

Comment thread journeys/settings/electrum-server-error-toasts.xml
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — no blocking HIGH/MEDIUM findings at head 8f00d51b2. (Posted as a comment because GitHub does not allow approving your own PR.)

Checked: Follow-up: the earlier journey finding is fixed. Follow-up on 8f00d51: the earlier port-rewrite LOW is fixed. ElectrumConfigViewModel.setProtocol (ElectrumConfigViewModel.kt:115-130) replaces the port with protocol.getDefaultPort() whenever it is empty or in defaultElectrumPorts (51002/50002/51001/50001), and getDefaultPort() returns 60002 for TLS on REGTEST (ElectrumServer.kt:110), so the old order rewrote 50002. The journey now selects TLS before typing host and port, verifies the field still reads 50002, and changes the port to 50001 without re-tapping a protocol row; setPort only trims, so both typed values survive. A failed connect writes no settings, so observeState does not re-seed the fields between steps. The description records the rewrite rule accurately, including the network-dependent default. No later step taps a protocol row.

ovi-reviewer[bot]

This comment was marked as resolved.

@ovitrif ovitrif left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utAck

@ovitrif

ovitrif commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

@jvsena42 conflicts

ovitrif
ovitrif previously approved these changes Sep 18, 2026
@ovitrif
ovitrif enabled auto-merge September 18, 2026 16:09
@jvsena42

Copy link
Copy Markdown
Member Author

Both points are covered centrally in #1275 (comment): the conflict is the shared journeys/README.md row that every PR in this batch touches, and a merge of current master is running across all of them; the red e2e shards failed in setup (snapshot load, adb exit 1, app never reaching Home), not in an assertion.

I will confirm the result on this PR once the merge lands and CI re-runs on the new head.

…rror-toasts

# Conflicts:
#	journeys/README.md

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ✅ Approve


Reaudit: diff 5 files.

Findings:
N/A

Audit:
Audited - no findings.

QA: emu-1 redroid container on Android 15, dev build 2.4.1 (188)

  1. passed: Network mismatch warning appeared; the connected server stayed unchanged.

    Test 1:
    Settings → Advanced → Electrum Server → select TLS, then enter `electrum.blockstream.info` / `50002`…
    1.mp4
  2. passed: TLS on port 50001 showed the protocol warning; the connected server stayed unchanged.

    Test 2:
    Electrum Server → keeping TLS selected, enter `electrum.blockstream.info` / `50001` → Connect To Host:…
    2.mp4
  3. passed: The unreachable endpoint showed the generic connection warning; the connected server stayed unchanged.

    Test 3:
    Electrum Server → enter `10.255.255.1` / `9999` → Connect To Host: after ~5s, warning toast "Bitkit could not…
    3.mp4
  4. passed: Every failure preserved the connected server and the Advanced row.

    Test 4:
    `regression:` After each failure above: "Currently connected to" still shows the previous server, and back →…
    4.mp4
  5. passed: The valid regtest server showed the success toast and updated the connected server.

    Test 5:
    `regression:` Electrum Server → enter a valid server for the current network → Connect To Host: success toast…
    5.mp4
  6. passed: The self-signed TLS endpoint showed the certificate warning; the connected server stayed unchanged.

    Test 6:
    Needs a self-hosted Electrum with a self-signed certificate: Electrum Server → enter that host / its SSL…
    6.mp4
  7. passed: The CA-valid certificate reached by IP showed the certificate warning without a node restart.

    Test 7:
    Needs an Electrum server reachable by IP whose TLS certificate is CA-valid but issued for another name:…
    7.mp4

Tip

Test 1 worth a journey:

  • Open the menu and go to Settings
  • Open Advanced and then Electrum Server
  • Select TLS
  • Enter electrum.blockstream.info and port 50002
  • Tap Connect To Host
  • Verify the network warning and unchanged connected server

Test 2 worth a journey:

  • Keep TLS selected
  • Change the port to 50001
  • Tap Connect To Host
  • Verify the protocol warning and unchanged connected server

Test 3 worth a journey:

  • Enter 10.255.255.1 and port 9999
  • Tap Connect To Host
  • Wait for the connection timeout
  • Verify the generic warning and unchanged connected server

Test 4 worth a journey:

  • Verify the connected server after each failed connection
  • Tap Reset To Default
  • Verify the default fields and disabled Connect button
  • Go back
  • Verify the Electrum Server row still shows Auto

Test 5 worth a journey:

  • Open Advanced and then Electrum Server
  • Enter a valid regtest Electrum host and port
  • Tap Connect To Host
  • Verify the success toast and updated connected server

Coverage:
Unit tests: 90% - ElectrumProbeServiceTest.kt drives both TLS cases through the injected factory, so the certificate tests fail if the probe stops using it; the Hilt provider has no test of its own.
QA: 7 of 7 manual tests passed


Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)

ovi-reviewer[bot]

This comment was marked as resolved.

@jvsena42
jvsena42 requested a review from ovitrif September 18, 2026 20:19

@ovi-reviewer ovi-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: ✅ Approve


Reaudit: diff 1 file.

Findings:
N/A

Audit:
Awaits approval.

QA: carried

  1. ⏭️ skipped: tested on dc458c7, recording
  2. ⏭️ skipped: tested on dc458c7, recording
  3. ⏭️ skipped: tested on dc458c7, recording
  4. ⏭️ skipped: tested on dc458c7, recording
  5. ⏭️ skipped: tested on dc458c7, recording
  6. ⏭️ skipped: tested on dc458c7, recording
  7. ⏭️ skipped: tested on dc458c7, recording

Coverage:
QA: 7 of 7 manual tests passed


Reviewed by gpt-5.6-sol-high via gh-pr-review-loop skill
Commands: @ovi-reviewer review · test · retest · audit (author or owner)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Electrum server errors show a generic message, discarding the probe's specific cause

2 participants