Skip to content

feat: move LN channel balances to spark - #1774

Open
ovitrif wants to merge 21 commits into
feat/breez-spark-lightningfrom
feat/move-to-spark
Open

ovitrif wants to merge 21 commits into
feat/breez-spark-lightningfrom
feat/move-to-spark

Conversation

@ovitrif

@ovitrif ovitrif commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1768

Twin: synonymdev/bitkit-ios#1212
Dependency: #1697
Companion: synonymdev/bitkit-e2e-tests#297

This PR adds Move to Spark: a one-time sheet and an action that pays the balance of a Lightning channel to the user's own Spark wallet over the channel, then offers to close the emptied channel. Blocktank is closing its channels, and moving the balance over Lightning costs only the routing fee, where a close followed by a deposit costs on-chain and claim fees and 3 confirmations. It stacks on #1697: review it against that base; it merges after #1697.

Description

  • Adds MoveToSparkRepo, which quotes the move against a new amountless Spark invoice: it starts from what the channels can send and leaves room for the routing fee. The fee shown is LDK's estimate plus a 2 sat margin, and the payment's routing fee is capped at exactly that.
  • Adds LightningRepo.payInvoiceOverLdk and estimateLdkRoutingFeesForAmount, so the move always goes over the channels and never falls back to Spark; the existing shouldPayWithSpark routing is unchanged.
  • Adds the Move to Spark screen: quote, swipe to move, then "Moved to Spark" with Close Connection, which opens the existing cooperative close; after a partial move it offers Move The Rest instead.
  • Shows "Amount too large" with a Move-half button when no route carries the amount, down to a 1,000 sat minimum, so a large balance can move in parts; any other failure, such as an expired invoice or a stopped node, shows "Could not move" with Try Again.
  • Adds a timed sheet that tells channel holders Blocktank is closing its connections and opens Move to Spark; it shows once, at the top priority, and is checked again once channels and Spark have loaded after a cold start.
  • Adds a Move To Spark button to Spending and Lightning Connections while a channel can still send at least 1,000 sats to a usable Spark wallet.
  • Keeps the move's own send and receive from raising the usual payment sent or received notices, in the foreground and from the background service, since the screen reports the outcome; a failure after the screen stopped waiting raises the usual failed notice.
  • While the payment is still in flight after the 2 minute wait, Done offers only OK, not Close Connection, since the amount may still return to the channel.
  • Fits the smallest screens: button labels stay on one line at 360 dp and on an iPhone SE (Later / Move Now, Cancel / Move Half, Later / Move Rest, Later / Close), the buttons sit at the bottom with the illustration taking the flexible height, and headlines put the white words first and the shorter accent last, as master's screens do.

Out of Scope

  • Activity: the move shows as one received payment, since the LDK send and the Spark receive share a payment hash; how a move should appear in Activity is left to the design pass.
  • Design: an AI-made first version from existing components; a Figma design and designer review follow.
  • Review follow-ups: move() and MoveToSparkViewModel unit tests — chore: move to spark review follow-ups #1802.
  • Emergency exit: Move to Spark should not be offered while a Spark emergency exit runs, but isExiting comes with the exit PR (feat: spark emergency exit #1782), not this PR's base. Every Move to Spark check goes through MoveToSparkMath.isSparkUsable in MoveToSparkRepo.kt, so whichever of the two PRs lands second changes that one line to spark.isReady && !spark.isExiting.

Design

N/A — no design available. UI pending design: this is an AI-made first version from existing components; a Figma design and designer review follow.

Screens

Pixel 6 emulator at 360 dp width, in flow order. Bitkit draws dark only, its system bars included, and no screen here shows a keyboard or system sheet.

Post-update sheet Spending entry Connections entry Finding a route
Too large Confirm: amount and fee Moving Done: part moved
Done: close offer Close confirmation After the close Could not move

Preview

The recordings below predate the small-screen pass, so they show earlier labels and headlines; the Screens section shows the current ones.

Sheet, too large and a partial move on the local Spark regtest (2x):

Sheet on Home, Later Not again after relaunch Too large, half, Move The Rest
f-a1-sheet.mp4
f-a1b-once.mp4
f-a4-too-large.mp4

Moving the rest, closing, and a wallet without a channel (2x):

Move the rest from Spending Close afterwards No channel, no sheet
f-a2-move.mp4
f-a3-close.mp4
f-a5-no-channel.mp4

QA Notes

Setup

  • A local Spark regtest: spark-sdk regtest/local (docker compose -f regtest/local/docker-compose.yml up), which runs the operators, the SSP with its Lightning node, an LNURL server, data sync, electrs and a chain API.
  • A dev build pointed at that stack. This PR has no switch for it; the tested build carried a local-only patch that sets, in SparkService, the stack's Spark config (from the stack's spark-config.json), its chain service, LNURL domain and sync URL, and sets Electrum to the stack's electrs. Missing prerequisite for reviewers: that patch, or a shared way to point dev builds at a local Spark stack.
  • In the app, open Lightning Connections, Manual Setup, and open a channel to the stack's SSP Lightning node (the node URI the stack prints) from a funded Savings balance; wait for it to confirm.

Journeys

  • J1: new move-to-spark.xml — sheet once, entry button, move with routing fee, close afterwards, nothing for a wallet without a channel

Manual Tests

  • 1. With a route that cannot carry the balance, open Move To Spark → "Amount too large" offers to move half; repeating it moves a part that routes, and Done offers Move The Rest — needs a route limit the local stack cannot set (tested with a local route-limit hook)

Automated Checks

  • added MoveToSparkRepoTest.kt — quote fits the routing fee under the channel limit, grows back once the real fee is known, keeps a smaller target, maps no route to too large, refuses with nothing to move or Spark not ready
  • updated AppViewModelSendFlowTest.kt — wires the new sheet and repository

Evidence

Tested at 927472a4e (this PR's feature commit; later commits change only the journey text), dev regtest build with the local-only patch from Setup, Pixel 6 API 35 emulator, one 400,000 sat channel to the SSP:

  • J1 sheet: on a cold launch the sheet shows on Home; Later closes it, and after a relaunch it does not show again.

  • 1: with a local 150,000 sat route-limit hook, ₿ 392,993 and ₿ 196,496 showed "Amount too large"; ₿ 98,248 moved with a ₿ 0 routing fee, and Done offered Move The Rest.

  • J1 move: without the hook, Move To Spark in Spending moved the rest, ₿ 296,719, with a ₿ 0 routing fee (direct channel to the SSP); Done offered Close Connection.

  • J1 close: Close Connection, then Close, returned to Lightning Connections with no open connection, and Spark held ₿ 394,967, the sum of both moves.

  • J1 no channel: a new wallet showed no Move To Spark button and no sheet.

  • The Lightning Connections entry button was driven on an earlier build of this change: it moved ₿ 77,967 and closed.

  • just compile, just lint and just test pass.

  • Small-screen pass at 62526d3bf: the sheet, too large, confirm, partial and full Done screens captured at 360 dp width; every button label stays on one line and the buttons sit at the bottom. just compile, just lint and just test pass.

  • Rebased onto the rebuilt Spark base ab1b09d14 (without the emergency exit) at 967292c09: just compile, just lint and just test pass. The device runs above ran before this rebase; the Move to Spark code is the same apart from checking isReady where it checked isUsable, which included the removed exit state.

  • Independent review fixes at d8b7d32c2, merged with the Spark base 2f483bd14 (route-only too large, fee cap at the shown fee, no Close while in flight, background notice gate, move guard): just compile, just lint and just test pass.

  • Bot review fixes at f715f9da2 (a move failure matched by payment id, raw route errors, the Connections button reading live channels, no second failure notice): just compile, just lint and just test pass.

Gaps

  • The review fixes were not driven on a device; J1 and 1 ran on earlier heads, as listed above.

  • Only direct channels to the SSP were tested; a multi-hop route with a real routing fee was not set up locally.

  • Activity shows the move as a single received payment (see Out of Scope).

  • Mainnet runs on phones come at polish time.

Models used

  • Planning/scoping: claude-opus-5-5 (reasoning: medium)
  • Implementation: claude-opus-5-5 (reasoning: medium)
  • Review: claude-opus-5-5 (reasoning: Not exposed)

Metadata note: the review subagents' run records give the model but not a reasoning setting.

@github-actions

github-actions Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from c1f0b53 (run).

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

@ovitrif

ovitrif commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 95f993e89: the Move To Spark button in Lightning Connections now sits below the open channels instead of above the Spark section, next to the channel it moves and clear of the Spark row and detail that the Spark exit work is changing on the base branch. Layout only; compile and lint pass.

@ovitrif

ovitrif commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 84f49451a..62526d3bf, a small-screen pass on the Move to Spark screens:

  • Button labels stay on one line at 360 dp: Later / Move Now, Cancel / Move Half, Later / Move Rest, Later / Close (the amount left the button; Close replaces Close Connection).
  • Bottom buttons sit at the screen bottom; the illustration takes the flexible height and shrinks so the sheet copy stays whole.
  • Headlines put the white words first and the shorter accent last, as master's screens do, with the purple accent of the Lightning screens.
  • All 15 locales follow the same headline shape and short labels.

Screenshots at 360 dp are in Preview. just compile, just lint and just test pass.

@ovitrif
ovitrif force-pushed the feat/breez-spark-lightning branch from ab34526 to ab1b09d Compare November 5, 2026 02:00
@ovitrif
ovitrif force-pushed the feat/move-to-spark branch from 62526d3 to 967292c Compare November 5, 2026 02:10
@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto the rebuilt Spark base ab1b09d14, which drops the emergency exit, and force-pushed with a lease (this PR is a draft): 62526d3bf → 967292c09.

  • ContentView and LightningConnectionsViewModel conflicts kept the new base's lines and only the Move to Spark route, entry and canMoveToSpark additions.
  • 967292c09 adapts to the removed exit: Spark readiness is SparkState.isReady (the removed isUsable also excluded a running exit), and the screen formats amounts itself instead of using the exit's helper.

just compile, just lint and just test pass. The device runs and screens in the description were taken before the rebase on the same Move to Spark code.

@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Merged the new base head 4632e7057 (Spark chain service on Bitkit electrs) into this PR as fe4488ae5. It merged without conflicts and changes only the base's Env.kt, SparkService.kt and SparkChainServiceTest.kt; no Move to Spark file changed. just compile and SparkChainServiceTest.kt pass locally; CI runs the full suite.

@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Reran the failed CI jobs at fe4488ae5. compose-ui / ui-tests hit the 45-minute job limit inside the emulator step without writing a test report, and the build gate failed only because of that. Build and unit tests passed, the previous head and the base's 4632e7057 passed the same checks, and the merge changed no Move to Spark file.

@ovitrif ovitrif changed the title feat: channel balances move to spark at low fees feat: move LN channel balances to spark Nov 5, 2026
@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Dropped "at the lowest fee" from this PR's changelog line, to match the new title: the line now says what the change does, not how cheaply.

@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed cae73d244: every Move to Spark check, including the move's own guard, now goes through MoveToSparkMath.isSparkUsable. Move to Spark must not be offered during a Spark emergency exit, and isExiting arrives with #1782, not this PR's base. Whichever of the two PRs lands second changes that one line to spark.isReady && !spark.isExiting; this is now under Out of Scope in the description. just compile, MoveToSparkRepoTest.kt and just lint pass.

@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Merged the new base head 4e22b9ca3 (Breez SDK 0.28.0) into this PR as 09219e448. It merged without conflicts; Move to Spark's code needed no change for the 0.28 API. just compile, just test and just lint pass locally.

@ovitrif

ovitrif commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator Author

Pushed d8b7d32c2: fixes from an independent pre-review pass, and the Spark base 2f483bd14 (master merged into #1697) merged in.

  • Only a missing route (RouteNotFound, ROUTE_NOT_FOUND, RETRIES_EXHAUSTED) shows "Amount too large"; any other failure, such as an expired invoice or a stopped node, shows "Could not move" with Try Again.
  • The fee shown is LDK's estimate plus the 2 sat margin, and the payment's routing fee cap is exactly that, so it never pays more than shown.
  • While a payment is still in flight after the 2 minute wait, Done offers only OK, not Close Connection.
  • The background service no longer posts a received notice for the move; a failure after the screen stopped waiting raises the usual failed notice.
  • A second move call while one is in progress does nothing.

The remaining low-severity items are in #1802. just compile, just lint and just test pass.

@ovitrif
ovitrif marked this pull request as ready for review November 6, 2026 17:19
@ovitrif
ovitrif requested review from a team, ben-kaufman and pwltr and removed request for a team November 6, 2026 17:19
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 6, 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-11-06T17:24:09.142713Z d8b7d32 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.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d8b7d32c21

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@greptile-apps

greptile-apps Bot commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerView in GreptileConfidence Score: 4/5

[Critical impact] The PR adds an explicit path for moving users’ Lightning funds into Spark.

The PR appears safe to merge, with a non-blocking duplicate-error notice to fix.

Findings

  1. P2 Failed moves show two errors ▶

Summary

Adds a flow to move channel funds to the user’s Spark wallet over Lightning.

  • People can move channel funds to Spark and choose what to do next.
  • Eligible channel holders can find Move to Spark from the app.
  • Move payments skip the usual sent and received notices.

Acknowledged by ovitrif: payment-id failure matching, the Connections button’s channel source, and move() unit tests are deferred to #1802. Activity presentation and design polish are also deferred. Emergency-exit gating awaits #1782 because its state is absent from this base. These acknowledged items are excluded from the findings.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Open Move to Spark] --> B[Create Spark invoice]
  B --> C[Quote amount and routing fee]
  C --> D[User swipes to move]
  D --> E[Pay through LDK with fee cap]
  E --> F{Payment result}
  F -->|Succeeded| G[Refresh wallet]
  G --> H{Funds remain?}
  H -->|Yes| I[Offer Move Rest]
  H -->|No| J[Offer channel close]
  F -->|No route| K[Offer Move Half]
  F -->|Other failure| L[Offer Try Again]
  F -->|Wait timed out| M[Show in flight without close]
Loading

Reviews (1) · Last reviewed commit: "chore: format move to spark repo" · Reviewed by Greptile

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

No findings. Adds Move to Spark: quotes and pays a Spark invoice over the LDK channels only (payInvoiceOverLdk, fee capped at the quoted fee), waits up to 2 min for settlement, mutes the payment's own send/receive notices, and surfaces it from a one-time timed sheet, Spending and Lightning Connections. Reviewed d8b7d32, full tier, against feat/breez-spark-lightning.

What I checked, and 7 candidates I ruled out

Read in full: MoveToSparkRepo.kt, MoveToSparkViewModel.kt, MoveToSparkTimedSheet.kt, the MoveToSparkScreen.kt Done branch, the LightningRepo, AppViewModel and NotifyPaymentReceivedHandler hunks.
Call sites traced: payInvoiceWithLdk to LightningService.send (sendUsingAmount with routeParametersWithFeeCap), estimateRoutingFeesForAmount, nodeEvents (LDK and onSparkEvent both emit into it), NotifyPaymentReceived.Command.from from both AppViewModel and LightningNodeService.
Not built here: auto reviews of this repo are static; CI is the gate.
CI at review time: build-and-test, lint, detekt, build-local green; the build gate red only because its run was cancelled (BUILD_RESULT: cancelled); e2e shards and compose-ui still running.

Ruled out

  • Settlement event missed: the settled collector starts UNDISPATCHED before the send, so a fast PaymentSuccessful is not lost.
  • Fee over what the user saw: the cap is feeSats * 1000 msat and feeSats is the quoted fee, so LDK refuses a route above it and that reads as TooLarge, never as an overpay.
  • Amount + fee over the outbound limit: a quote is kept only when amountSats + feeSats <= limitSats; later rounds fall back to the last fitting quote.
  • Failure toast doubling the screen's error: isWatchedMovePayment mutes it while the screen waits; after a timeout the app announces a late failure, matching the InFlight copy.
  • Received sheet for the Spark side: skipped in shouldShowLightning, which both the foreground and the background service go through.
  • Close offered while funds may still return: the InFlight branch shows OK only.
  • Spark fallback paying from Spark to Spark: payInvoiceOverLdk and estimateLdkRoutingFeesForAmount bypass the rail selection entirely.

Merge confidence: 4/5, move() / payAndWait (the path that sends funds) has no test, only quote and the math do, and e2e was still running.

Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (the runtime does not report it)

@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed f715f9da2: fixes for the bot review.

  • A failed move whose PaymentFailed has no payment hash is matched by the payment id payInvoiceOverLdk returns, so it no longer waits 2 minutes and reports in flight.
  • A raw NodeException.RouteNotFound from dispatch counts as no route, so it offers Move Half.
  • The Connections Move To Spark button reads the live channels, as the move does, so it hides while the node is stopped.
  • A failure the move screen received stays marked after its wait ends, so the app shows no second failure notice.

#1802 now holds only the missing unit tests. just compile, just lint and just test pass.

@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Triage: stays in 3.2.0. This PR closes #1768, which is already in the 3.2.0 milestone; now that this PR is ready for review, the issue moved to 👀 Review on the project. Twin: synonymdev/bitkit-ios#1212.

@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 591d3c060: the Move to Spark sheet no longer shows in E2E builds. The e2e suites open new channels, and the sheet, at the top timed-sheet priority, then covered Home, failing lightning_security, lnurl_transfer, multi_address_local and send (home chrome or Send never showed). J1 still covers the sheet. just compile, just lint and just test pass.

@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed b295a6e3e: reverted the E2E skip, so the Move to Spark sheet shows in test builds as it does in the app. The e2e suites handle the sheet instead, on the companion branch feat/move-to-spark in synonymdev/bitkit-e2e-tests, which this PR's e2e run picks up.

@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

Cancelled and re-ran the failed e2e jobs of https://github.com/synonymdev/bitkit-android/actions/runs/42273241535 so they check out synonymdev/bitkit-e2e-tests#297 at 70d01c1: the attempt in progress had started on an earlier commit of that companion branch. This PR's code is unchanged.

@ovitrif

ovitrif commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

e2e on 977cfd0a9: send and lightning_security fail before reaching any Move to Spark step. LND gets no_route paying the app's amountless invoice, which the Spark base d4a0dd16a now receives on Spark below 1,000,000 sats of channel inbound. #1697 fails the same two suites at that head (https://github.com/synonymdev/bitkit-android/actions/runs/42298688984), so the fix belongs there; I've handed it over. The suites the Move to Spark intro affected (multi_address_local, lnurl_transfer) pass with synonymdev/bitkit-e2e-tests#297.

This branch has not been deployed

No deployments
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.

2 participants