Skip to content

feat: move LN channel balances to spark - #1774

Merged
ovitrif merged 46 commits into
masterfrom
feat/move-to-spark
Nov 11, 2026
Merged

ovitrif merged 46 commits into
masterfrom
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 default shouldPayWithSpark routing is unchanged.
  • Adds the Move to Spark screen: quote, swipe to move, then "Moved to Spark" with Close, which opens the existing cooperative close; after a partial move it offers Move The Rest instead.
  • Shows "Amount too large" with Move Half 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.
  • Waits up to 2 minutes for the payment's outcome. A payment still in flight then shows "Moving to Spark" with OK only, since its amount may still return to the channel; a failure that arrives later shows "Could not move" and the usual failed notice, and a success that arrives later shows "Moved to Spark".
  • 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, after the fee reserve.
  • 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.
  • Records a move as one Activity row, the channel send, with the moved amount and the routing fee, whichever leg arrives first; the Spark receive with the same payment hash only marks a pending or failed send as succeeded.
  • Fits the smallest screens: button labels stay on one line at 360 dp and on an iPhone SE, 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: a move shows as an outgoing payment for now; a designed Move to Spark row is bug: move to spark activity loses direction and fee #1819.
  • Design: an AI-made first version from existing components; a Figma design and designer review follow.
  • Review follow-ups: move() and MoveToSparkViewModel unit tests, keeping the outcome after leaving the screen, and keeping the move's hash across a restart — chore: move to spark review follow-ups #1802.
  • Translations: the Move to Spark strings ship in English only; their translations come with the Spark translation follow-up.
  • 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.

Preview

Android emulator (Pixel profile) at 896 × 1994, 320 dpi, at 2x speed.

Sheet on Home, Later Too large, Move Half Move the rest Close afterwards
rec-sheet2x.mp4
rec-too-large2x.mp4
rec-move-rest2x.mp4
rec-close2x.mp4

Screens

Android emulator (Pixel profile) at 896 × 1994, 320 dpi, in flow order. Bitkit draws dark only, its system bars included.

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

QA Notes

Setup

  • A local Spark regtest: the spark profile from feat: local spark regtest network bitkit-docker#37 (./spark up), which runs three Spark operators, the SSP with its Lightning node, the LNURL and data-sync servers and a chain API on bitkit-docker's chain, compatible with Spark SDK 0.28. It does not publish the SSP node's P2P port (9735); publish it in a compose override to open a channel from the app.
  • A dev build (gradlew assembleDevDebug) with SPARK_LOCAL_CONFIG_B64 set to ./spark config --base64 and the Spark chain service pointed at http://127.0.0.1:23090/api, then adb reverse for 23535–23537, 23049, 23090 and the SSP node's P2P port, and Electrum (60001). This PR has no switch for it; the app-side settings land in separate PRs.
  • In the app, fund Savings (./spark send <address> 500000), then open Lightning Connections, Manual Setup, and open a channel to the SSP node; confirm it with ./spark mine 6.
  • Manual test 1 needs a route limit the stack cannot set: the tested build carried a QA hook, a qa-move file in the app's files folder (limit=<sats>, fail, slow).

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, offers a move only when 1,000 sats fit beside the fee reserve, maps no route (including a send that finds no route on a live invoice) to too large, refuses with nothing to move or Spark not ready
  • added MoveToSparkViewModelTest.kt — a failure or success that arrives after the wait changes the in-flight screen only for the move that still owns it
  • added ActivityServiceMoveToSparkTest.kt and a SparkRepoTest.kt case — a move keeps one outgoing Activity row with the routing fee, whichever leg arrives first
  • updated NotifyPaymentReceivedHandlerTest.kt and AppViewModelSendFlowTest.kt — no received notice for the move, the new sheet wired in

Evidence

Tested at eccf3648a on the stack and build from Setup, Android emulator (Pixel profile) at 896 × 1994, 320 dpi, one 400,000 sat channel to the SSP; the Preview and Screens above are this run. Later commits (f7dd3ec and 3341321) change only the in-flight screen and Activity rows, which this run does not show.

  • J1 sheet: once the channel confirmed, the sheet showed on Home; Later closed it.
  • 1: with the route limit, the full balance showed "Amount too large"; Move Half moved a part with a 0 sat routing fee (direct channel), and Done offered Move The Rest.
  • J1 move and close: Move To Spark moved the rest, Done offered Close, and Close then Close returned to Lightning Connections with no open connection and the balance on Spark.
  • Could not move: with the fail hook, the screen showed "Could not move" with Try Again.

Tested again at 4e5dc9105 on the same stack and emulator, with a fresh wallet and one channel to the SSP, and no hook switched on.

  • J1 no channel: with Savings funded and no channel, Home showed no sheet, and Spending and Lightning Connections had no Move entry.

1-no-channel2x.mp4

  • J1 not again: once the channel confirmed, the sheet showed and Later closed it. After a force-stop and relaunch, Home showed no sheet and the Spending entry stayed.

2a-sheet2x.mp4

2b-relaunch2x.mp4

  • In-flight: with the SSP paused through the two-minute wait, the screen showed Moving to Spark; once the SSP resumed, the late success turned it into Moved to Spark with a 0 sat routing fee.

3-in-flight-staged2x.mp4

  • Activity: after a move that settled at once and after the late-settled one, All Activity showed one Sent row, Move to Spark 395,458, with no received row; its detail shows Successful with fee 0.

4-activity2x.mp4

4b-activity-late2x.mp4

Gaps

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

Mainnet tests (run by the author)

These run on real devices with mainnet funds, by the author. Reviewers and review bots don't need to run them, and they don't block review.

Round 1

None yet.

Round 2 (pending)

  • M1: intro — after updating with a funded channel, the post-update sheet shows once on Home and opens Move to Spark
  • M2: move sheet — Move To Spark quotes the channel balance with a routing fee that fits under the channel's limit
  • M3: move — swiping moves the balance over the channel to the Spark wallet, with a routing fee no higher than the one shown
  • M4: channel close — Close Connection on Done closes the emptied channel cooperatively
  • M5: balance after — Spark holds the moved amount and the closed channel's remainder returns to Savings

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 c9f4357 (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
… feat/move-to-spark

# Conflicts:
#	app/src/main/java/to/bitkit/repositories/SparkRepo.kt
#	app/src/test/java/to/bitkit/repositories/SparkRepoTest.kt
@ovitrif

ovitrif commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

323adc9 merges base 97951b8, which conflicted in SparkRepo.recordActivity. The base now saves an activity only after a successful read and retries a failed write at the next sync. A move's Spark receive keeps marking its channel send succeeded, and a failed write of that update is now retried the same way. The two move tests in SparkRepoTest sit next to the base's new ones.

@ovitrif
ovitrif requested a review from jvsena42 November 10, 2026 12:33

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

Reviewed 323adc9.

Low candidate — Preserve move notification suppression across process restart.

MoveToSparkRepo.kt stores move-identification hashes only in memory. If the process dies while a move is in flight, late settlement may produce ordinary send and receive notifications for the same self-transfer. The reviewed accounting path still converges to a single activity; this candidate concerns notification accuracy.

Low candidate — Verify fee refresh after receive-first settlement.

SparkRepo.kt marks an existing pending send successful when its matching Spark receive completes, preserving the send record’s fee. If that fee is still zero, the activity could temporarily understate it. The later ActivityRepo.handlePaymentEvent correction path was outside the scoped review, so a persistent defect is unverified. The move screen uses the actual paid fee, and the approved routing-fee cap remains enforced.

Review details

General review

Model: GPT-6.1 Sol

Reasoning effort: high

Security review

Model: Kimi K3 (OpenRouter/Moonshot)

Reasoning effort: high

@piotr-iohk piotr-iohk 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.

QA review

Scope: Re-reviewed the full PR at 323adc9 after no usable baseline was supplied and the comparison base changed from 99b81bcc to 97951b8. Coverage is the whole diff against that merge base.

No new actionable code findings.

The move creates an amountless Spark invoice and pays it only through payInvoiceOverLdk. The quoted amount plus the shown routing fee (the LDK estimate plus 2 sats) stays within the live outbound HTLC limit, and that shown fee is the payment cap. A missing route offers a smaller amount down to 1,000 sats. The sheet and both Move To Spark buttons stay hidden when the fee reserve leaves less than 1,000 sats. A failure received during the wait stays marked reported, so the screen error stands alone. After the two-minute wait, a later failure still raises the usual failure notice, and the screen changes to Could not move or Moved to Spark only while that same move is still showing.

Leaving the screen, or killing the app, still drops the in-memory outcome and payment hash. That limitation remains synonymdev/bitkit-android#1802, already deferred on this PR. Compared with bitkit-ios#1212 at 0ce947b, the quote, fee cap, and too-large recovery match. iOS keeps an in-flight move when its screen closes. Android drops that wait with the screen. bitkit-e2e-tests#297 at b004e5e was checked only for dismissing the intro when it covers Home. Dependency bitkit-android#1697 is this PR's base 97951b8. The channel-only send uses that base's LDK pay path.

Validation: this review did not run the unit suite or a device. build-and-test passed on this head. The inspected tests cover quote bounds, a late stored outcome, activity replacement, and the late screen update. Device testing: not performed in this review. The author reports a device run at eccf3648a, and a later J1 pass was reported at a6a84323d. Those builds predate the settlement listener and the send-hash commit on this head. Local e2e for send, multi_address, and onchain was still pending, so those jobs are not evidence here.

Reviewer model: grok-4.7
Reasoning effort: high

@ovitrif

ovitrif commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

@pwltr

Low candidate — Preserve move notification suppression across process restart.

Out of scope: tracked in #1802, which this PR lists under Out of Scope (keeping the move's hash across a restart). A process death mid-move only costs a duplicate notification; the activity still converges to one row.

Low candidate — Verify fee refresh after receive-first settlement.

Confirmed and fixed in 5315c36. bitkit-core stamps updatedAt when the Spark receive settles the pending send, so the node's later success update looked older and, with no status change, was skipped, leaving the fee at 0. A fee change now passes that check too. ActivityServiceMoveToSparkTest.kt covers it, and it fails without the fix. The twin fix goes into synonymdev/bitkit-ios#1212 with its next push.

@ovitrif

ovitrif commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

4e5dc91:

  • Merged the Spark base at 7c8b0ee. One conflict in ContentView.kt: the Spending wallet keeps the base's shownBalanceSats and the Move entry.
  • Removed the Move strings' translations; the English strings stay, and the translations follow in the Spark translation follow-up.

@ovitrif
ovitrif requested a review from pwltr November 10, 2026 15:23
pwltr
pwltr previously approved these changes Nov 10, 2026

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

Reviewed 5315c36, the merge of the updated base branch and 4e5dc91. A move's channel send now keeps the node's routing fee even when its Spark receive settled it first: the activity takes the fee when the node reports the succeeded send, and a recorded success is still never undone. The merge keeps the base's shown Spark balance beside the Move to Spark entry on the spending wallet and Lightning connections. The move-to-Spark translations are left to a follow-up; those screens read in English until then. No new findings.

Review details

General review
Model: Claude Opus 5.5
Reasoning effort: high

jvsena42
jvsena42 previously approved these changes Nov 10, 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.

Re-review of 5315c36, the base merge and 4e5dc91: no findings.

  • Routing fee kept: CoreService.kt:903-914 lets a node update through the staleness check when it changes the stored fee. Status downgrades are still blocked earlier (:884-896). The next sync rewrites the same row, and the Spark receive path returns early once the send succeeded, so the two writers don't overwrite each other back and forth.
  • Merge resolution: SparkRepo.kt:938-949 follows the base's unsaved-activity tracking.
  • Translations: 4e5dc91 removes the move-to-Spark keys from every locale in full, so no locale is left with a partial set. MissingTranslation is off, so lint is unaffected. Until the follow-up lands, these screens read in English in other locales.

Reviewer model: claude-fable-5-1 (delta review), claude-opus-5-5 (coordination)
Reasoning effort: not exposed (Fable subagent), default (Opus)

@ovitrif
ovitrif changed the base branch from feat/breez-spark-lightning to master November 11, 2026 11:23
@ovitrif
ovitrif dismissed stale reviews from jvsena42, pwltr, and ben-kaufman November 11, 2026 11:23

The base branch was changed.

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Retargeted to master: #1697, the Spark base this PR is stacked on, is merging now, and deleting its branch would close this PR.

@ovitrif ovitrif mentioned this pull request Nov 11, 2026
5 of 45 tasks
@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

@pwltr @jvsena42 #1697 merged, so this PR now targets master, and the base change dismissed your approvals at 4e5dc91. The head is unchanged and merges cleanly into master, so I'm requesting your review again.

@ovitrif
ovitrif requested review from jvsena42 and pwltr November 11, 2026 11:25

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

No new material findings at c9f4357. The PR's documented follow-ups remain deferred.

Current-head device journeys and local app checks were not run in this review. The author's earlier regtest runs used direct channels. Multi-hop routing with a nonzero fee remains untested.

Reviewer model: gpt-6.1-sol, claude-opus-5-5
Reasoning effort: xhigh (gpt-6.1-sol), high (claude-opus-5-5)

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Merged: approved by @ben-kaufman at c9f4357, required checks green, no open threads.

@ovitrif
ovitrif merged commit 3c71d58 into master Nov 11, 2026
23 checks passed
@ovitrif
ovitrif deleted the feat/move-to-spark branch November 11, 2026 13:01
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.

feat: move LN channel balances to spark

6 participants