Skip to content

fix: reconnect electrum after the server drops the connection - #1532

Merged
ovitrif merged 3 commits into
masterfrom
fix/ldk-node-electrum-reconnect
Oct 16, 2026
Merged

ovitrif merged 3 commits into
masterfrom
fix/ldk-node-electrum-reconnect

Conversation

@ovitrif

@ovitrif ovitrif commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1526
Twin: synonymdev/bitkit-ios#971
Refs: synonymdev/ldk-node#127

Description

  • Bumps ldk-node to 0.7.0-rc.76 for 3.0.0 RC3, and points the marketplace journey at it. When the Electrum server dropped the connection while a call was waiting on it, ldk-node never rebuilt it: wallet sync, fee updates and every broadcast failed with Broken pipe until the node restarted. Bitkit iOS RC2 hit this with a Pubky Shop payment. ldk-node now replaces the connection once a call finds it broken, and retries a failed broadcast once on the new connection, all within each call's original deadline. 0.7.0-rc.76 is 0.7.0-rc.74 plus this fix, with no API change.

Out of Scope

Design

N/A — no UI changes.

Preview

N/A — no user-visible changes.

QA Notes

Journeys

N/A

Manual Tests

  • Drop the Electrum connection while a call waits on it, then pay on-chain → the payment is sent — needs an Electrum connection dropped mid-call.

    Steps (staging regtest, about 15 min)
    1. Build and install this PR's devDebug APK on an emulator (staging regtest is its default backend) and create a new wallet.
    2. Fund it: Receive › Savings shows the address in the QR code's content description (adb shell uiautomator dump), then
      curl -X POST https://api.stag0.blocktank.to/blocktank/api/v2/regtest/chain/deposit -H 'content-type: application/json' -d '{"address":"<bcrt1…>","amountSat":50000}'
      curl -X POST https://api.stag0.blocktank.to/blocktank/api/v2/regtest/chain/mine -H 'content-type: application/json' -d '{"count":1}'
    3. Start the bridge on the host and forward the emulator's port to it. It listens on 127.0.0.1:50099, forwards over TLS to the staging Electrum server and takes control files in /tmp/electrum-bridge:
      python3 scripts/electrum-bridge.py
      adb reverse tcp:50099 tcp:50099
      Re-run adb reverse if adb reconnects.
    4. Point Bitkit at it: Settings › Advanced › Electrum Server, host 127.0.0.1, port 50099, TCP, Connect To Host. Pass: "Currently connected to 127.0.0.1:50099" and the bridge prints two open lines. (adb shell input text can drop characters; type the host in pieces and check it.)
    5. Follow the log: adb logcat -s LDK.
    6. Arm a drop: touch /tmp/electrum-bridge/drop, and wait for the bridge to print drop: queued … -> closing (the next sync, up to a minute).
    7. Pay: adb shell am start -a android.intent.action.VIEW -d "bitcoin:<address>?amount=0.00002" to.bitkit.dev, Continue, Swipe To Pay.
    8. Pass: the log shows Incremental sync of on-chain wallet failed (EOF while parsing, then Broken pipe) followed within a second by Reconnected to Electrum server after a failed call.; the send shows "Bitcoin Sent" and the log Successfully broadcast transaction <txid>.
    9. Fail: every later sync and broadcast keeps logging Broken pipe with no Reconnected line, and the send ends as pending with an unknown outcome.

Automated Checks

  • added scripts/electrum-bridge.py — the bridge used for the manual test.
  • ran the manual test above on an Android 16 emulator at 0.7.0-rc.76: sync failed with EOF and Broken pipe, reconnected 2 ms later, payment c316914f… sent.
  • ran electrum.rs in fix: reconnect electrum after the server drops the connection ldk-node#127 — a dropped connection with a queued call no longer leaves broadcasts failing.

Bump ldk-node to 0.7.0-rc.75, which rebuilds the shared Electrum connection once a call finds it broken and retries a failed broadcast on the new connection.
@ovitrif ovitrif self-assigned this Oct 16, 2026
@ovitrif
ovitrif requested review from a team and coreyphillips and removed request for a team October 16, 2026 14:20
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-16T16:17:27.458376Z 5a8e435 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 0e13a52 (run).

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

@greptile-apps

greptile-apps Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 5/5

[Critical impact] No blocking issue was found in the reviewed changes.

Summary

Updates ldk-node-android from 0.7.0-rc.74 to 0.7.0-rc.76 to recover after Electrum disconnects.

  • On-chain payments recover after an Electrum connection drops.

Reviews (2) · Last reviewed commit: "fix: keep electrum reconnects within eac..." · Reviewed by Greptile

@talosmachina talosmachina 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.

No findings. Bumps ldk-node-android to 0.7.0-rc.75, which replaces the shared Electrum client after a broken connection and retries a broadcast once on the new connection, plus a changelog fragment for #1526. Reviewed a22e4c6, full tier.

What I checked, and 4 candidates I ruled out

Read in full: gradle/libs.versions.toml, changelog.d/next/1526.fixed.md, the configureChainSource call in LightningService.kt
Upstream diff: v0.7.0-rc.74...v0.7.0-rc.75 in synonymdev/ldk-node changes src/chain/electrum.rs, tests/integration_tests_rust.rs and version strings only. The bindings are unchanged apart from the version, so the body's "rc.74 plus this fix, with no API change" holds. src/chain/electrum.rs at the tag (dc3e53d) is the head I reviewed on synonymdev/ldk-node#127.
CI: CI build, lint, detekt and build-local green on a22e4c6, so rc.75 resolves from the registry and compiles against the app. The E2E run was still in progress when I reviewed.

Ruled out

  • Unrelated changes riding along in rc.75: the merge commits for synonymdev/ldk-node#125 and #126 between the two tags add no file changes beyond the test, and the tag sits 2 commits ahead of ldk-node main with nothing behind.
  • Reconnect and retry stretching a send past its deadline: in ldk-node the reconnect runs after the failed call, with the configured socket timeout. Bitkit sets connectionTimeoutSecs = Env.walletSyncTimeoutSecs (10 s), not the 255 s ceiling, so the reconnect and the retried broadcast are each bounded by a 10 s socket timeout, not the 255 s ceiling. The open review point on synonymdev/ldk-node#127 about keeping this inside the original deadline would land in a later rc, not this pin.
  • The deferred concurrent-reconnect race (failed connect alongside a successful one): its outcome is an Unknown broadcast that the next call recovers from, and the ldk-node PR explicitly leaves it for a later rc.
  • nativeDebugSymbols pinned to a different version: it reads the same libs.ldk.node.android entry, so both move together.

Merge confidence: 4/5, the app has no test that drops an Electrum connection (the manual check in the body is still open) and the E2E run had not finished.

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code scan at a22e4c6: no findings. I reviewed it as a wallet-SDK bump.

Checked:

  • ldk-node-android 0.7.0-rc.74 → rc.75. Upstream, v0.7.0-rc.74..v0.7.0-rc.75 is 571a1da fix: reconnect electrum after the connection broke plus the release commit. It touches only src/chain/electrum.rs and an integration test. The UDL, persistence, KV/VSS and channel-monitor code are unchanged, and Cargo.lock changes only the ldk-node version (bdk, electrum-client 0.24.1 and lightning unchanged). There is no migration surface.
  • The v2.5.1 security fix is carried: lightning is 0.2.7 at rc.66.1, rc.72, rc.74 and rc.75.
  • Reconnect logic: only "connection broken" errors reconnect (io errors other than WouldBlock/TimedOut, JSON EOF). Arc::ptr_eq collapses concurrent failures into one reconnect, and a failed connect keeps the old client. A broadcast reconnects once and retries once. TimedOut stays Unknown with no retry. An already-delivered tx maps to Accepted, or to Unknown via "already known", never to Rejected. A slow server does not cause reconnect churn.
  • No app API change: OnchainSendResult mapping (LightningService.kt:1007-1009) is unchanged, and the build is green.
  • iOS twin synonymdev/bitkit-ios#971 (draft) pins the same rc.75.

Performance-relevant (reconnect of background work): at most one connect per failed call, no loop and no sleep. Worst case is about 2–3 connects every 10 s with the server unreachable.

Note: the rc.75 tag sits on the still-open synonymdev/ldk-node#127, while rc.74 was tagged on main. If #127 changes before it merges, rc.75 becomes an orphan tag.

Device gate (partial): a22e4c6 — not run yet. Pending: the #1526 path on master and on this PR (break the connection with airplane mode, foreground, send on-chain; expect Reconnected to Electrum server after a failed call. and an Accepted result), the reconnect rate per minute while offline, the first sync after reconnect, and onchain-accepted-result.xml / onchain-original-payment-retry.xml. The device result will follow.

@ovitrif
ovitrif marked this pull request as draft October 16, 2026 15:08
@jvsena42

Copy link
Copy Markdown
Member

Device gate (partial): a22e4c6 — broken-connection sends on master (rc.74) and this PR (rc.75). Reconnect counts and journeys still running.

Emulator Gate_Throwaway against the staging Electrum ssl://electrs.bitkit.stag0.blocktank.to:9999, wallet funded 3 × 200k. From master run 3 on, traffic went through a local CONNECT proxy that can withhold replies and then FIN the 9999 connections, so the server drops the connection while a call is waiting.

Run Build How the connection broke Send result Notes
m1 master airplane 90 s Accepted 5× Broken pipe … failed to lookup address while offline; first sync after 16.2 s
m2 master app frozen 18 min (SIGSTOP) Accepted the server never closed the idle sockets
m3 master reply withheld 10 s, then FIN during send refused before broadcast: SyncUnhealthyError='Wallet sync failed before send' next sync 3.5 s
m4 master FIN 3 s after a request, idle — recovered silently, sync 6.5 s
m5 master FIN 5 s after a request, during send refused before broadcast (SyncUnhealthy) queued sync stuck until the 10 s timeout
p1 PR airplane 125 s Accepted Failed to reconnect to Electrum server: failed to lookup address information after each failed call (7 in 2 min); first sync after 4.4 s
p2 PR FIN 5 s after a request, during send (same as m5) Accepted no error surfaced, fee update 8.7 s
p3 PR reply withheld, then FIN during send (same as m3) refused before broadcast (SyncUnhealthy) 15:29:12.593 Reconnected to Electrum server after a failed call. 227 ms after the failed sync; next sync 3.5 s
p4 PR drop, then send Accepted

The #1526 symptom (persistent Broken pipe, outcome unknown) did not reproduce on master in 5 attempts on this setup. The PR still differs from master where it matters: in the same server-side drop during a send (m5 / p2), master refused the send and the PR sent it. The reconnect path fires (p3). While offline the PR makes at most one connect attempt per failed call.

Seen on master, outside this PR: a native SIGABRT at 14:48:36 UTC, A Tokio 1.x context was found, but it is being shutdown., when the node restarted after background → foreground.

jvsena42
jvsena42 previously approved these changes Oct 16, 2026

@jvsena42 jvsena42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving at a22e4c6. The code scan from my earlier review stands, with no findings.

Device gate: a22e4c6 — journeys: 2 passed. perf: the first sync after an airplane drop took 4.4 s (master 16.2 s), and after the PR's reconnect 3.5 s; steady state was 657–693 ms idle on both builds. While offline the PR made 7 reconnect attempts in 2 min (one per failed call, no loop). No ANR, OOM or crash on the PR build. l10n: no UI text changed.

  • onchain-accepted-result.xml: 11/11 pass. Both sends showed SendSuccess, and the TXID matched Successfully broadcast transaction in the log. The Max send went through the >50% fee dialog.
  • onchain-original-payment-retry.xml: 12/12 pass against a real network fault. A local proxy swallowed the broadcast and refused connections for 40 s, giving Failed to broadcast transaction due to timeout … deadline has elapsed and Payment Pending. The retry kept the original input, Back without the PIN broadcast nothing, and the approved retry broadcast 3a132712..fd82 at 281 sats.
  • Broken connection during a send, the same server-side FIN 5 s after a request on both builds: master refused the send with SyncUnhealthyError='Wallet sync failed before send', and the PR sent it (Accepted). The PR's path fired as Reconnected to Electrum server after a failed call. 227 ms after a failed sync.

⚠️ Not verified:

  • The #1526 symptom itself (persistent Broken pipe, outcome unknown) did not reproduce on master in 5 attempts on the emulator.
  • Android stops the node in the background, so that state can only arise in the foreground.
  • Retrying broadcast of and two calls queued on one dropped connection were never reached.

Details and logs: #1532 (comment)

Bump ldk-node to 0.7.0-rc.76, which adds the deadline bound to the Electrum reconnect and broadcast retry from 0.7.0-rc.75, and point the marketplace journey at it.
@ovitrif
ovitrif marked this pull request as ready for review October 16, 2026 16:15
@ovitrif

ovitrif commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

@jvsena42 thanks for the device gate. This PR now pins 0.7.0-rc.76 (5a8e435) instead of rc.75. The only change in ldk-node is that the reconnect and the broadcast retry stay within each call's original deadline (synonymdev/ldk-node#127, approved by @ben-kaufman). The push dismissed your approval; could you re-approve?

@ovitrif
ovitrif requested a review from jvsena42 October 16, 2026 16:15
@ovitrif
ovitrif enabled auto-merge October 16, 2026 16:15
@jvsena42

Copy link
Copy Markdown
Member

Delta review a22e4c6..5a8e435 (ldk-node rc.75 → rc.76): no findings. My approval above was for a22e4c6.

Checked:

  • Upstream v0.7.0-rc.75..v0.7.0-rc.76 touches only src/chain/electrum.rs and version strings. Cargo.lock changes only the ldk-node version, with lightning / lightning-transaction-sync 0.2.7 and electrum-client 0.24.1 unchanged. There are no UDL, persistence or wallet changes.
  • reconnect_main_client now runs in spawn_blocking. It re-checks Arc::ptr_eq before connecting, takes the write lock only for the pointer swap after connect_main() returns, and reports !is_current() on a failed connect. Sync, fee and balance calls detach the reconnect and return their error at once, so no lock is held across a connect.
  • broadcast_with_timeout fixes the deadline up front. A reconnect past the deadline, a failed reconnect or no time left all return Unknown, and the retry runs with the remaining time only. No new path can report Rejected for a possibly-delivered transaction; the app maps Unknown to pending plus the original-payment retry (LightningService.kt:1009). The total send wait is capped at 15 s, so a late break now shows Payment Pending rather than a late Accepted.
  • v0.7.0-rc.76 is on ldk-node main (fix: reconnect electrum after the server drops the connection ldk-node#127 merged), and the Maven artifact and release are published. This resolves the orphan-tag note from my first review. fix: recover on-chain sends after the app returns from the background bitkit-ios#971 pins the same tag commit.

Device gate (partial): 5a8e435 — not re-run yet. The PR-side results in #1532 (comment) were measured on rc.75; the master rows stand. Pending on the rc.76 APK:

  • onchain-accepted-result.xml;
  • onchain-original-payment-retry.xml (the swallowed-broadcast case should now end at ~15 s in Payment Pending);
  • the drop-during-send runs;
  • reconnect attempts per 2 min offline;
  • a reconnect stalled at the TLS handshake during a send (expect Payment Pending within ~15 s and fast-failing syncs).

@ovitrif

ovitrif commented Oct 16, 2026

Copy link
Copy Markdown
Collaborator Author

@jvsena42 thanks for the delta check. GitHub still needs an approval on 5a8e435 to merge, because your a22e4c6 approval was dismissed by the push. Could you approve this head?

@ben-kaufman ben-kaufman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

utAck

@ovitrif
ovitrif merged commit 92741ea into master Oct 16, 2026
21 checks passed
@ovitrif
ovitrif deleted the fix/ldk-node-electrum-reconnect branch October 16, 2026 17:26
@jvsena42

Copy link
Copy Markdown
Member

Device results on 5a8e435 (ldk-node rc.76), recorded after merge. The final head 0e13a52 adds only scripts/electrum-bridge.py.

The CI APK was run on Gate_Throwaway behind the same local CONNECT proxy as the rc.75 runs.

  • onchain-accepted-result.xml: pass. A 1 000-sat send and a Max send (through the >50% fee dialog) both reach "Bitcoin Sent", and TXID matches the broadcast. The success screen has no SendSuccess or Details testTags (TXID is tagged), so the journey's tag names are stale.
  • Swallowed broadcast: the timeout now ends the broadcast at 15.1 s and Payment Pending shows:
    17:36:36.775 INFO  [ldk_node::wallet:1962] Created new transaction b87e4b81…f613 sending 2000sats on-chain
    17:36:52.237 ERROR [ldk_node::chain::electrum:1078] Failed to broadcast transaction due to timeout b87e4b81…f613: deadline has elapsed
    
  • onchain-original-payment-retry.xml: pass on a clean run. The winning tx 80637c63…2714 reused the original input a6c5cefa…b8a7:0. Action 8 exposed an app bug that predates this PR and is now filed as system back on the recovery pin screen blocks retry original payment until restart #1548.
  • FIN 5 s after a request during a send: Accepted (4381ffc6…3267). The pre-send sync waited ~12 s.
  • Airplane mode for 2 min: 7 Failed to reconnect to Electrum server: failed to lookup address… attempts, 14–24 s apart, the same as rc.75. The thread count went 94 → 74 offline and 85 after reconnect, so it did not grow.
  • ⚠️ Not run: the TLS-handshake stall during a send. The run stopped when the PR merged.

No crash or ANR.

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.

bug: on-chain payment after a dropped electrum connection never broadcasts and shows outcome unknown

4 participants