Skip to content

feat: spark emergency exit - #1782

Open
ovitrif wants to merge 101 commits into
feat/breez-spark-lightningfrom
feat/spark-emergency-exit
Open

ovitrif wants to merge 101 commits into
feat/breez-spark-lightningfrom
feat/spark-emergency-exit

Conversation

@ovitrif

@ovitrif ovitrif commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1806
Twin: synonymdev/bitkit-ios#1217
Stacked on #1697

This PR adds Emergency exit, Spark's unilateral exit: if the Spark operators stop working, Spending moves to Savings on-chain without them. It builds on the Spark wallet in #1697.

Description

What this PR does

  • Lightning Connections › Spark shows Emergency exit while Spark holds funds. It opens an explainer, a quote (amount, fees, what arrives, what stays on Spark), fee funding, a confirm swipe, progress and the result.
  • Fee funding is paid from Savings, or sent from another wallet to a shown address.
  • Progress shows the steps and the timelock countdown, asks for more fees when needed, and offers Pay a higher fee when the network outbids the exit.
  • If the Savings payment is refused, Progress says "Your savings payment didn't go through. Pay again, or fund from another wallet." and offers Pay from savings again.
  • The exit runs in the background with Bitkit closed and notifies only when it needs you. It can take up to 4 weeks.
  • When it ends, leftover fee funding goes back to Savings, and the result shows what reached Savings and what stayed on Spark.
  • The exit's strings are translated in every locale.

Decisions for review

  • Trust. The exit needs no operator: the app signs and broadcasts every step itself. On mainnet it reads the chain only from https://bitkit.to/electrs and broadcasts there first, with mempool.space and blockstream.info as fallbacks.
  • Funding key. The CPFP fees come from a key on its own account (m/84'/{coin}'/7797'/0/<index>, a fresh index per exit), so ldk-node never spends the exit's funding.
  • Fee funding. Savings or another wallet pays one amount, which the SDK splits into one output per step, so a lost fee race strands one step instead of the chain after it. A top-up asks only for what is still missing, at least the 294-sat dust limit.
  • Refused Savings payment. Pay from savings re-signs the same payment over the inputs it already spends, so at most one version can confirm. Cancel waits for it and keeps the exit.
  • Spark during an exit. No Lightning sends or receives on Spark, and leaf optimization stays off so no swap hands away an exiting leaf. The balance shows as in transfer to Savings on Home and Savings. Cancel works only while nothing was paid; funding already at the address goes back to Savings.
  • Saved before sent. Every build is saved before broadcast and every broadcast is saved as it is answered, so a killed run never sends a step twice.
  • Stalls. A broken fee chain rebuilds the exit; after 3 rebuilds with nothing confirming, Progress says it is not moving. A step that is only underpriced is not rebuilt; Progress offers Pay a higher fee instead.
  • What stays on Spark. Leaves too small to pay their own exit, and fee-ladder leaves that can't pay their refund, stay on Spark, and the quote says how much. A leaf an older owner already claimed on-chain is dropped and named on Progress.
  • Leftover sweep. It pays the network's cheapest rate and waits up to about 4 weeks for fees to drop before keeping the change at the funding address, whose derivation path the result shows.
  • Glow. Stall handling, waits for leaves that may still move, and top-up fees follow Breez's reference wallet, breez/glow-web. The lost fee race, lost leaves and background runs are our own handling, since Glow has none.

Background run on Android

  • A WorkManager job runs every 15 minutes with Bitkit closed, also after a kill, a reboot or an app update. It sends the signed steps and connects Spark alone, never ldk-node.

Notifications on Android

  • One deadline alarm, 12 blocks into the operators' ~50-block window, is pushed back by every background run that keeps up. When it fires, it says why and when to open Bitkit by.
  • Needing more fees, an exit that is not moving, and a failed exit notify at once.

Out of Scope

  • Funding one Savings payment per step: ldk-node sends to one address only, so the SDK fans out the single payment.

Risks for review

  • Mainnet. No exit has run on mainnet. Fee-ladder leaves and package relay at the three hosts are untested with a real exit. About 50 blocks after a step matures, the operators may publish their own version, paid from the leaf. A user who restricted Bitkit's background use or force-stopped it gets no background runs or alarm until the next launch, and Progress says so.
  • Cost. At 2 sat/vB, leaves under about 4,096 sats cost more to exit than they hold, so a typical balance recovers about 75% after fees, in about 13 to 14 days.

Design

N/A — no design yet. The emergency exit screens follow the Transfer to Savings screens; docs/screens-map.md lists them as todo. UI polish is pending design.

Preview

Emergency exit at 42adfd7 on an Android emulator (896×1994, 320 dpi) in Bitkit's dark theme. Start, Pay from savings and Fund from another wallet run live against Spark's hosted regtest, with Savings on Spark's regtest chain and notifications denied; Pay from savings sends the fee funding from Savings and the exit goes on once it confirms. Fund from another wallet is paid in two parts by another wallet, and the waits for the payments are cut. Running exit, Redo and sweep, and Exit complete are staged from saved exit records on a debug build, each state shown in turn; Done with the leftover swept is shown through a local hook that is not in this PR.

Start: intro, quote, funding choices, confirm Pay from savings Fund from another wallet
a-start2x.mp4
a-savings2x.mp4
a-external2x.mp4
Running exit: timelock and sweep Redo and sweep Exit complete
a-running2x.mp4
a-redo2x.mp4
a-complete2x.mp4

Screens

Every exit screen, at 42adfd7 on an Android emulator (896×1994, 320 dpi) in Bitkit's dark theme. Start, Paid from savings and the notification prompt are from the live exits above. The other Progress and Result screens are staged from saved exit records on a debug build; Done, leftover swept and the Pay a higher fee rates are shown through a local hook that is not in this PR, since that state and the fee quote can't be staged from a record alone. Pay a higher fee reads 0 sat/vB because the staged record saves no fee rate.

Start
Spark detail during an exit Intro Quote with the can't-exit note Funding choices
Funding address Partial payment hint Notification permission Confirm, paying from savings
Progress
Needs funding Needs funding, partial Paid from savings Payment seen
Running, network fees rose Pay a higher fee
Result
Leftover sending Done, leftover swept Stopped Stopped, balance stays on Spark
Lost to an older owner

QA Notes

Setup

  • A dev build on Spark's regtest chain, set up as in feat: add spark via breez sdk #1697's QA Notes (ESPLORA_URL and the faucet), runs the exit from the explainer to Progress; the timelocks of about 1,900 blocks keep a full exit out of reach there.
  • The full exits ran on a local regtest stack: spark-sdk 0.27.2 against the last public Spark operator (1 Oct), patched to accept 0.27.2's status filters and with fee ladders off, since the current operator image is private. Every leaf there is an anchored, pre-ladder leaf.

Journeys

  • new transfer/spark-emergency-exit.xml — with a Spark balance, Emergency exit opens the explainer, the quote lists amount, fees and what arrives, and funding shows Pay from savings and a funding address from another wallet; stops before the swipe.

Manual Tests

  • 27. Dev on Spark's regtest chain, Spark funded from the faucet → Emergency exit, fund from another wallet with the faucet, swipe → Progress shows fee funding confirmed, then publishing with confirmed steps counting up and Ready in a countdown; kill and reopen the app mid-exit → it continues without sending a step twice; at the end Savings receives about the quoted amount and the transfer settles — hours of regtest blocks.
  • 28. Same as 27 with Pay from savings → the funding payment shows as an ordinary payment, not a transfer to Spending, and the exit starts once it confirms — Spark's regtest chain.
  • 29. During an exit → Spending shows the Spark balance as in transfer to Savings, Lightning receives and sends do not use Spark, and Transfer to Spending shows the Transfer Unavailable sheet — a running exit.
  • 30. Exit started but not funded → Cancel exit on Progress puts Spark back as it was — needs a Spark balance.
  • 31. Dev, an exit running → kill Bitkit from the background (not Force stop) → the next background run logs Driving the Spark exit in background, connects Spark and checks the exit with no screen open; with Wi-Fi and data off past the deadline alarm, a notification names the time to open Bitkit by — a running exit.
  • 32. Dev on Spark's regtest chain, Emergency exit with Pay from savings → right after the swipe Progress shows Paid from savings, waiting for confirmation, with the transaction ID and no address or QR; with Fund from another wallet, a payment seen in the mempool shows Payment seen, waiting for confirmation — Spark's regtest chain.
  • 33. An exit that ends with leftover fee funding (a Redo that needed no funding) → Emergency exit and the Spark screen still open the result, which shows the sweep sending, then sent with its transaction ID, or kept as dust; a fresh explainer shows only after that — needs an exit to end, about 2,000 regtest blocks.
  • 34. A second exit on the same wallet → its funding address differs from the first exit's — two exits.
  • 35. Relaunch Bitkit with an exit whose refunds wait for their timelock → before Spark reconnects the log shows no non-BIP68-final or missingorspent broadcast — a running exit near its timelock.
  • 37. Dev, an exit running and Bitkit closed → a Spark exit notification (deadline alarm, or a lost or failed result) → tapping it opens the exit's Progress inside the Spark screen, and OK goes back to Spark — a running exit, also after the app process was killed.
  • 38. An exit that ends Done → Progress shows the amount that reached Savings and the sweep's transaction ID — needs an exit to end, about 2,000 regtest blocks.
  • 39. An exit whose step is accepted but unconfirmed for more than 6 blocks, or whose rate the network's next-block rate passed → Progress shows the fee line and Pay a higher fee; its screen lists Fast and Normal above the exit's rate with what each costs from Savings (rows disabled and a note when Savings can't cover them); paying shows Paid from savings, waiting for confirmation, and once it confirms the waiting steps go out again at the new rate — a running exit with fees above its rate.
  • 40. An exit whose Done verdict lists cooperative leaves the operators could not co-sign → the exit stays in progress and says they wait for the operators; it retries them, and after 1,008 blocks Done names the amount the operators moved on-chain and still hold — operators that refuse to co-sign.

Automated Checks

  • added SparkExitRecordTest.kt — the record round trips with every phase and status, the recovery keeps spark-sdk's serde shape, a recovery written by the Breez CLI and a record written by Bitkit iOS are read, an unknown version is refused, and the store saves atomically and sets an unreadable record aside
  • added SparkExitDriverTest.kt — waits for confirmed funding, builds and saves before broadcasting, Valid broadcasts ready steps once until due, rejections surface after 3, Done ends with leftover funding, Redo rebuilds as a new generation that never sends the old hex (also without the SDK), a rebuild short of fees asks only for what the SDK's total lacks beyond what the address holds (50 sats on the local regtest run's numbers, 2,000 after a Redo whose input lives on as 13,000 of change) and adopts several small top-ups together, the tick that ends the exit sweeps its leftover until a node accepts it, dust is not swept, a top-up ignores CPFP change, cooperative failures retry, an unverified leaf waits, a restart mid-exit resumes from the file without repeating broadcasts, and the keyless pass broadcasts only steps whose parents confirmed and timelocks matured
  • added SparkExitFundingTest.kt — what arrives, the fees and the fee funding of a quote, the uneconomic gate, the funding shortfall, the countdown height, the funding UTXO choice, whether Savings can pay, the ETA per network, the funding path, WIF decoding and CPFP child txids
  • added SparkExitChainTest.kt — packages go out as a raw JSON array and single steps as plain hex, the unversioned path and the next host are tried, a node's refusal wins over an unavailable host, tip and UTXOs are read, and node answers are classified
  • added SparkExitRepoTest.kt — loading settles exit transfers no running exit owns, including the transfer of a record set aside as unreadable; a background run from a cold start (Spark not connected, no UI) publishes from the stored recovery against a mock regtest mempool, connects Spark and checks, and a restarted process sends nothing twice; runs whose steps keep being refused stop pushing the deadline alarm back; a background run of an ended exit with leftover to sweep keeps going without starting Spark
  • added SparkExitSweepTest.kt — bech32, bech32m, P2PKH and P2SH addresses decode to their scripts, dust limits follow Bitcoin Core per output type, a sweep pays its inputs less the fee and skips dust, the signed transaction carries the signer's witnesses and keeps the unsigned txid, and an unsigned input is refused
  • added SparkExitAlertsTest.kt — a stalled exit's notification names refused steps, no network, background restriction or battery saver (battery optimization alone names no cause), asks for fee funding, and stays quiet when nothing needs the user
  • added SparkExitRegtestTest.kt (unit, opt-in with SPARK_EXIT_REGTEST=1) — Spark's regtest mempool answers the tip and a funding address, and its node's per-transaction answer to a package is read as retry later
  • added SparkExitRegtestTest.kt (instrumented, opt-in with -e sparkExitRegtest true) — on a device against Spark's regtest, a new wallet's quote has nothing to exit, the funding key derives with bitkit-core into a working CPFP signer, and a leftover sweep PSBT is signed and finalized by the SDK's singleKeyCpfpSigner
  • updated SparkExitDriverTest.kt, SparkExitFundingTest.kt, SparkExitRecordTest.kt, SparkExitRepoTest.kt, SparkExitChainTest.kt — Done after a Redo that needed no funding sweeps the earlier generation's change in one transaction, an exit that ended while its funding address could not be read ends again once it can, each exit takes a funding key index no earlier exit used and a restart signs with its own record's, without the SDK a refund waits for its CSV from its parent's confirmation and a step whose input someone else spent is not sent again, a refund an older state spent ends the exit after two rebuilds reproduce it, a lost leaf or one recoverFunds refuses is left out and the rest rebuilt, a rebuild whose every leaf cannot be exited fails, Progress waits for a payment from savings or one seen in the mempool and asks only for what is missing, the quote names what it skips, and the deadline warns within half the 50-block window on every network
  • updated SparkExitDriverTest.kt, SparkExitChainTest.kt, SparkExitRecordTest.kt, SparkExitRepoTest.kt, SparkExitViewModelTest.kt — two packages for one shared node where theirs confirms and ours is dropped rebuild from the same funding without resending the dead transactions, and wait for the rebuild interval sending nothing meanwhile; a child still in the mempool is not dead; a step unpaid 3 times in a row rebuilds once and then surfaces, and any other answer resets the count; a transaction is unknown only when every host answers 404; only a zero-fee refusal reads as unpaid; the new record fields round trip; a background run whose step keeps going unpaid without Spark lets the deadline alarm fire; a tick that ends short of 68 sats asks for 294 at once, and Progress asks for at least 294 before the funding address was read
  • updated SparkExitViewModelTest.kt — a stopped exit that lost a 32,768-sat leaf and could not exit 1,696 says ₿ 1 696 stays on Spark, from Spark's live balance, and claims nothing when only the lost leaf is left
  • added SparkRepoTest.kt case — an exit quote waits for a running leaf optimization and out the counter-swap grace of one cut short before it syncs
  • added SparkExitViewModelTest.kt — after Fund from another wallet, Back and Continue offer both funding choices again
  • updated LightningConnectionsViewModelTest.kt, AppViewModelSendFlowTest.kt — construct the exit repository
  • updated SparkExitDriverTest.kt, SparkExitFundingTest.kt, SparkExitRecordTest.kt, SparkExitRepoTest.kt, SparkExitAlertsTest.kt, SparkRepoTest.kt — a refused step rebuilds until 3 rebuilds confirmed nothing, then reads as not moving, and a confirmation starts the count over; an unpaid step rebuilds and never counts as a rejection; a step accepted but unconfirmed for more than 6 blocks, or refused for a nonzero fee under a node's or the mempool's floor, reads as underpriced and does not rebuild, while 0 < and a lost replacement race do not; a unilateral Unverified step is not sent and rebuilds once a block, but waits while another unilateral step is ready, and the keyless pass still sends it; an empty rebuild waits for a FundsNotFound leaf at 6/12/24-block gaps, lets it go after 1,008 blocks, waits 6 blocks for a stopped leaf, ends at once for an unrecoverable leaf that is not stopped, and never strikes a stopped leaf; the first build keeps the SDK's exit data and a rebuild imports it; a sweep refused for its fee is rebuilt at the next block's rate, becomes dust when nothing else is left, stays final for any other refusal, is replaced after 144 blocks unless the replacement would be dust, and ends when a replaced version confirms; an unknown-version record is salvaged with its funding, exit data and transfer, an ended one stays ended, and the repo keeps such an exit running; the critical-path countdown; the amount received over several generations; per-input top-up sizing and pacing; the exit data backup is skipped while an exit runs
  • updated SparkExitDriverTest.kt, SparkExitFundingTest.kt, SparkExitRecordTest.kt, SparkExitAlertsTest.kt, SparkExitViewModelTest.kt — every prepare passes the exit's funding strategy; fan-out outputs are not new funding; the funding split counts the quote's entries; the pre-start hint adds each payment's input fee; OperatorsUnavailable waits; a higher fee the funding covers needs nothing more, otherwise the SDK's shortfall less what the address holds plus one input's fee, at least dust; a paid higher fee rebuilds at the new rate once its payment confirms, and one with nothing to pay rebuilds on the next tick; the rates offered sit strictly above the exit's; the offer holds only while underpriced or below the network; leaves the operators moved on-chain retry at once and at doubling gaps, are let go after a week, and a co-signed retry becomes the next generation that ends the exit; Done keeps the sweep that landed before the retry; the alarm names a low fee before any missed-run cause; a salvaged ended record keeps its leftover
  • updated SparkExitRepoTest.kt — a refused savings payment is shown and one still unanswered is not; paying again re-signs the refused payment over its own inputs; a cancel while savings pays again waits for the payment and then keeps the exit
  • updated SparkExitChainTest.kt — mainnet reads only bitkit.to/electrs and broadcasts there first, then mempool.space and blockstream.info; a package on bitkit.to/electrs goes once to /txs/package; reads never reach a broadcast-only host; with one read host a 404 means unknown and a 503 or an unreachable host throws

Test results

Breez SDK 0.28.0 rerun on d9d3c87, local Spark regtest plus one exit on Spark's hosted regtest (QA comment):

  • 🟢 Pay from savings with per-node funding, the fee-change sweep of all 5 leftover outputs, and the partial top-up with no second round.
  • 🟢 Pay a higher fee (paid and covered by the funding), the evicted fee-change sweep retried, a 0.27.2 record resumed to Done.
  • 🟢 Lost race on a per-node node, stale state with one notification, the rebuilt-from-chain note, and "not moving" with its notification once.
  • 🟢 Hosted regtest: fan-out and the first node packages accepted.
  • 🟠 Lone-transaction package retry not reached: the hosted host now answers /tx normally; unit tests cover it.
  • ⏭️ The failed cooperative leaf retry and OperatorsUnavailable wait need a watchtower-moved leaf, which neither stack produced in time.

The fixes after that run rest on the unit tests above.

Local regtest exits, from the QA write-up (spark-sdk 0.27.2 against the last public operator, Android emulator):

Case Result
Quote with all 3 operators stopped ✅ 90,112 to exit, 4,537 fees, 5,713 funding, 9,888 staying on Spark
Fee funding from another wallet ✅ exactly 5,713
Kill mid-exit, relaunch ✅ continued without sending a step twice
Full exit to Savings ✅ 91,288 swept, 1,907 blocks from the first broadcast (about 13 days at mainnet block times)
Operators publish the refunds first (Redo) ✅ generation 2, no funding needed, 93,200 swept
Top-up after a short build (on 5edd5a8ce) ✅ asked only for the 68 sats missing, with a notification
Leftover sweep (on 5edd5a8ce) ✅ 1,626 and 626 back to Savings
Pay from savings, then new funding address per exit (on 5edd5a8ce) ✅ paid, waiting for confirmation, with the txid
Partial funding (on f1507f29d) ✅ ₿ 500 of ₿ 1,329 received, send ₿ 829 more; Continue waits
Single leaf lost to an older owner (on f1507f29d) ✅ leaf dropped, 558 swept back, Failed, one notification, result kept until opened

The same write-up's headless runs on the SDK's calls show a stale refund from an older owner winning when the app sleeps past the 100 blocks after a maturity with the operators down, which the lost-leaf handling above now ends with a Failed exit instead of an endless Redo.

🟢 Exit start and funding from another wallet, Pay from savings, Running exit in the background
Exit start and funding from another wallet

On 1adfc47 on Spark's regtest chain (J9's steps by hand): with 79,703 on Spark the quote showed 65,536 to exit, 8,923 network fees, about 56,613 to receive, 9,211 fee funding and 14,167 staying on Spark, and Fund from another wallet showed the funding address and amount. Recording in Preview.


Pay from savings

On 3e60f04, a fresh wallet with 69,802 on Spark (part of 32: Pay from savings): the quote named the 12,458 it can't exit, the swipe asked for notification permission, Progress showed Paid from savings, waiting for confirmation, with the transaction ID and no QR until the funding confirmed about a minute later, then published 1 of 23 steps. Recording in Preview.


Running exit in the background

On 3e60f04, the 4-leaf exit started on 326a6aa (21 of 30 steps confirmed, waiting for the timelock) loaded on the new build: Progress showed the background, battery optimization and notifications-off notes, and Savings showed From Spending (±4d) 90,400 (part of 29: the countdown). On bd931ac, with Bitkit's process killed, the periodic job published nothing due, connected Spark alone, checked the exit as Valid, saved the record and ended in 9 seconds; after a reboot the job and the deadline alarm came back, and background restriction held both (part of 31: the background run). The 326a6aa run found three bugs, fixed there: the fan-out confirm Spark's regtest mempool never reported, the foreground loop that never started, and the timelock step shown as done too early. Recording in Preview.


🟢 Full exit with the operators down, Redo after the operators publish first, Funding short then topped up
Full exit with the operators down

On 4771ead, with a local config patch not in this PR, on a local regtest stack (spark-sdk 0.27.2 against the last public Spark operator, fee ladders off, since the current operator image is private): with the 3 operators stopped the quote showed 90,112 to exit, 4,537 fees and 5,713 funding; funded exactly from another wallet, the exit built 25 transactions, resumed after a force-stop without sending a step twice, published each refund within a minute of maturity, and swept 91,288 to Savings 1,907 blocks after the first broadcast, ending on Exit complete with 18 small amounts left on Spark. Screenshot in Preview.


Redo after the operators publish first

On 4771ead, a second exit paid from savings was force-stopped until 2,000 blocks past maturity, the operators' watchtower published all 5 refunds, and the relaunch rebuilt as generation 2 with no funding and swept 93,200 to Savings. It left 3,514 sats of funding change unswept and hidden after Done, on the first exit's funding address, which cc2641c fixes (33, 34). Recording in Preview.


Funding short then topped up

On the same stack, a harness making Bitkit's SDK calls showed a rebuild needing 9,085 sats with 9,035 held over 5 UTXOs, then reaching Done after a 1,000 sat top-up; Bitkit asked for the whole total instead of the 50 missing, fixed in aec3163.


🟢 Pay from savings after viewing the funding address
Pay from savings after viewing the funding address

On 1adfc47, after Fund from another wallet, Back and Continue reopened the funding address with no Pay from savings button. Fixed in b4ec299, which resets the choice on Back; SparkExitViewModelTest.kt covers it, not rerun on a device.


🟠 Exit end and notifications on the new build
Exit end and notifications on the new build

Not run: on Spark's regtest the running exits wait for timelocks of about 1,900 blocks, so the result kept reachable with its sweep state (33), a second exit's new funding address (34) and the relaunch without early refunds (35) rest on the unit tests; the deadline and funding notifications (rest of 31) are covered by SparkExitAlertsTest.kt, and Payment seen for an outside payment (rest of 32) by SparkExitFundingTest.kt.


🟠 Full in-app exit on Spark's hosted regtest
Full in-app exit on Spark's hosted regtest

On 52bac5d, a dev build on Spark's hosted regtest, with a fresh wallet holding ₿ 39 901 on Spark and ₿ 21 000 in Savings:

  • ✅ Start and funding: the quote showed ₿ 32 768 to exit, ₿ 11 714 network fees, about ₿ 21 054 to receive, ₿ 19 668 fee funding and ₿ 7 133 staying on Spark. Pay from savings sent ₿ 20 294 (₿ 626 fee) in one payment. Progress showed Paid from savings, waiting for confirmation, with its transaction ID; once it confirmed a block later, the exit published 1 of 38 steps and showed the timelock countdown.
  • 🟠 Done: still running. The leaves' timelocks of about 3,900 blocks at about 31 s a block take about 33 h. I'll update this line when the exit ends.
p2-2x.mp4

Open

  • Hosted-regtest exit: A full exit by a harness on SDK 0.27.2, making the same calls as the app's driver, reached Done on Spark's hosted regtest: all 19 steps confirmed and the sweep confirmed. The app's own full exit there is running; see Test results.
  • Mainnet exit and fee-ladder leaves: under Mainnet tests.

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

  • Read-only check on a fresh wallet with nothing funded: Spark connected and synced on https://bitkit.to/electrs, Emergency exit opened up to the quote (Nothing can be exited right now), and the SDK's fee estimate and the exit's chain reads went to https://bitkit.to/electrs only. Nothing was broadcast. On 30f87e5, with debug-only logging (result not posted).

Round 2 (pending)

On the final heads, per the small mainnet exit: a fresh wallet, a low-fee week (about 2 sat/vB) and about 20,000 sats in Savings for the fees.

  • Read-only check again on the final head.
  • Deposit about 50,000 sats into Spark and wait for it to settle; the quote lists the leaves it moves, what stays on Spark and the total fee.
  • Start, with the fees paid from Savings: Home shows the amount in transfer with a countdown, and Spark stops sending and receiving.
  • Each step confirms, with packages accepted at https://bitkit.to/electrs, and none is published by the operators first.
  • Reminders: a notification when a step needs the app opened, in time for its window of about 50 blocks.
  • Restart the app mid-exit: it resumes and nothing is broadcast twice.
  • Fee-ladder leaves: their refunds go out and confirm (unit tests only so far).
  • The leaves are swept to Savings, Savings gains the exited amount minus the fees, and leftover fee funding returns to Savings.
  • The exit screen shows Done, with one completion notification.

Models used

  • Planning/scoping: claude-opus-5-5 (reasoning: medium), claude-sonnet-5-5 (reasoning: high)
  • Implementation: claude-opus-5-5 (reasoning: medium)
  • Review: claude-sonnet-5-5 (reasoning: high)

Taken from the run logs of every session and subagent that planned, wrote or reviewed this PR, which record the model and reasoning effort per turn. The claude-sonnet-5-5 planning runs were research and audit subagents.

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 1a4450d (run).

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

@ovitrif

ovitrif commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator Author

QA on a92ee43: dev build on an Android 15 emulator, Spark on its hosted regtest, Savings funded with 200,000 sats on staging regtest.

  • 🟢 J1 spark-transfer-to-spending.xml from feat: add spark via breez sdk #1697, to check the exit's gating leaves the transfer flow alone: every step held, with the same figures as on feat: add spark via breez sdk #1697 (Max 199,635, up to 696 Instant, ≈ 99 Later), and Savings › Transfer To Spending opened the amount screen with no intro.
  • 🟠 spark-emergency-exit.xml: stopped at step 6. With a ₿ 0 Spark balance the Spark screen hides Emergency exit and disables Transfer to Savings, as designed; the rest needs a Spark balance from Lightspark's regtest faucet.

J1 on this PR, and the exit journey up to the Spark screen with no balance (2x speed).

J1 flow No intro on the second entry Exit journey, ₿ 0 on Spark
j1-exit-a2x.mp4
j1-exit-b2x.mp4
exit-journey2x.mp4

When another wallet's package confirms a shared tree node, our CPFP child for it is dropped and every later step that spends its change can never confirm, while the SDK check still answers Valid. Detect that child on a Valid check and rebuild from the same funding. As a safety net, a step unpaid or missing inputs 3 times in a row rebuilds once, and those sends count as no rejection until then.
Unpaid sends no longer count as rejections, so without the SDK a step whose fee chain lost a race never surfaced and every background run looked on track. A step unpaid 3 times in a row now lets the deadline alarm fire.
The funding state was read before the tick, so a tick that ended short of fees showed the raw shortfall (68 sats on qa-03) until the next tick. Read it again after a tick that changed the phase, and floor the fallback ask at the 294-sat dust limit.
The stopped result read the amount outside the exit, which holds only while the exit runs, from a balance kept only during SDK ticks. It now reads Spark's live balance beyond the exit's leaves in any phase, as iOS does.
@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

QA on 4df5c75f4: dev build on an Android 15 emulator, against a local Spark regtest stack with the operators stopped, so every exit is unilateral.

  • 🟢 Lost race on a shared tree node: another owner's CPFP child replaced this wallet's child on the node cad843ee (the competing child spends the node's anchor at a higher fee). The exit carried on by itself: its next node 2fe35690 was accepted 9.7 min after the loss, and the exit swept 66,775 sats to savings (689d3b9d).
  • 🟢 Stale state (an older owner publishes its refund first): the exit ends Failed with exactly one notification, "₿ 65 536 of your exit can't be recovered".
  • 🟢 The stopped screen now says "Nothing could be recovered. ₿ 3 392 stays on Spark." It stays until it's seen, and reopening Emergency exit shows the intro.
  • 🔴 A leaf lost to an older state comes back in the next exit, on iOS too (feat: spark emergency exit bitkit-ios#1217):
    • The operators still list it as ON_CHAIN under this wallet, so the next quote counted it: ₿ 131 072 over 2 leaves instead of ₿ 65 536.
    • Its refund fails with "inputs missing" on every tick, and the driver logs "diverged on-chain" and rebuilds after each confirmation, about 10 min per rebuild.
    • It then drops the leaf again as "claimed by an older state" and sends a second "can't be recovered" notification for a loss the user was already told about.
    • The exit itself finished Done. Leaves already reported as lost should be left out of later quotes and exits.

Stale-state result, kept until seen:

@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Fix for the lost leaf that came back in the next exit (QA comment), in 8eac6db.

Why: after an older state claims a leaf on-chain, the operators keep listing it as ON_CHAIN under the wallet. The lost leaves lived only on the exit record, so the next exit quoted the leaf again, rebuilt around its spent refund, and reported the same loss a second time.

What changed:

  • Leaves lost to an older state are now kept per wallet and network, next to the exit record, and outlive it. They're cleared only when the wallet is wiped. They aren't in the backup: a restored wallet finds the loss again in one rebuild.
  • The quote leaves them out. When prepareRecoverFunds(All) lists one, the rest are prepared again with Specific, so the amount, leaf count, fees and funding come from the SDK without it. When every listed leaf is lost, the quote shows the existing "nothing to exit" state and no exit can start.
  • When an exit's unexitable-leaf fallback (RecoverableOnly) lists a leaf outside the exit, a lost one included, only the exit's own leaves are prepared and built.
  • The "can't be recovered" notification posts only for a leaf not already reported lost.

The leaf id the app logs for a dropped leaf is the refund's node_id, which spark-sdk 0.27.2 sets to the leaf id, and the app logs it only for leaves of its own quote. In the regtest operator DB, case 5's older owner is a third wallet that sent one of the two leaves to each app wallet. The two ids differ in one hex digit (01a1aa9e-4d2a-…, 01a1aa9e-4d2d-…), so the crossed reading is most likely a misread, not an app bug.

Unit tests cover the store outliving a new exit record, the lost leaf left out of the quote and the selection, an all-lost quote with nothing to exit, and no second notification. Not rerun on a device yet.

@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

QA on 8eac6db67: dev build on an Android 15 emulator, against a local Spark regtest stack with the operators stopped. I installed it over the previous build, keeping the wallet.

  • 🟢 Case 5 again (an older owner claims the leaf first): the exit ends Failed with exactly one notification, and spark-exit-lost-leaves.json records the leaf.
  • 🟢 With only lost leaves left, the quote shows ₿ 0 and "Nothing can be exited right now.", and Continue is disabled.
  • 🟢 A new exit after that quotes only the live leaf (₿ 65 536 over 1 leaf). It logs no "diverged" or "inputs missing", sends no second notification, and swept 65,830 sats to Done (dd202b0c).
  • 🟢 A leaf lost before this update was already in the file at the first quote after the update, so it never came back.

The 🔴 lost-leaf bug from my QA comment on 4df5c75 is fixed. Not covered: the later commits that move the mainnet chain service to Bitkit's electrs. This regtest build points the chain service at the local stack.

pwltr
pwltr previously requested 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 42adfd7.

Medium — Revalidate saved confirmations in SDK-free recovery

app/src/main/java/to/bitkit/repositories/SparkExitDriver.kt:1058–1068: persisted Confirmed transactions are skipped without querying current chain status. After a reorg, SDK-free recovery can retain an orphaned parent confirmation and permanently mark dependent missing-input failures as spent, leaving the exit stalled. Requery confirmations and invalidate dependent conclusions after a reorganization.

Medium — Enforce the quoted mining-fee ceiling on exit funding sends

app/src/main/java/to/bitkit/repositories/SparkExitRepo.kt:882–888: funding sends retain quoted fee rates but omit approvedMiningFeeSats. A changed input set can therefore increase the absolute mining fee after confirmation at the same sat/vB. Carry the approved fee through initial and fee-bump sends, rejecting or requoting when preparation exceeds it.

Review details

General review

Model: GPT-6.1 Sol

Reasoning effort: high

@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 the 7 own commits since 80912ba96: one LOW (inline), approving.

Checked and clean:

  • Interrupted start (ac7103ebf): startLocked (SparkExitRepo.kt:688-724) holds Spark, creates the transfer and saves the record once, with its transferId. A failed create or save releases the hold and settles the transfer. If the app is killed in between, settleOrphanedTransfers settles the leftover transfer on the next load. _state.record is published only after the save.
  • No double count after an exit (cdbe3674c): a zero read during an exit keeps nothing, so Spending doesn't count the exited balance again.
  • Leftover sweep note (6cd48b76c): the sweep txid shows only once the sweep is accepted, in the same order as iOS leftoverLine. The removed SparkExitLeftoverSwept tag has no remaining users.
  • Exit screen alignment: every accented amount is rendered through an accent helper, and the removed *_txid strings have no references left.

The LOW is reproduced by a unit test. With operators unreachable, Spark reads 0 and the kept balance shows 10,000. After holdForExit() → releaseExitHold() → refresh, shownBalanceSats is 0. Test name: SparkRepoTest › verifier cancelled exit keeps the kept balance while spark reads zero, failing at 42adfd7d8 with "expected:<10000> but was:<0>"; the other 72 tests in the class pass.

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

@ovitrif

ovitrif commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 4234458.

Fixes for @pwltr's review:

  • Saved confirmations in SDK-free recovery (17e525e): recovery now asks the chain again for saved confirmations. After a reorg it drops any conclusion that depended on an orphaned parent, instead of keeping the exit stalled.
  • Mining fee ceiling on exit funding (48f0723, 4234458): the initial funding send and the fee bump now carry the confirmed approvedMiningFeeSats, and a send that would pay more is refused before broadcast. The next confirm approves the fee that was refused, so the user isn't asked to confirm the same fee twice.

Also in this push:

  • @jvsena42's kept-balance LOW (9ab94f1): fixed, and answered in its thread.
  • Spark base (36450df): I merged the Spark base bdf3584 into Exit. The WalletRepo.kt conflict keeps both sides: the Spark real and shown balances, plus isExiting.

@jvsena42, your approval at 42adfd7 is stale only because of this push: 42adfd7...4234458

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

Delta review of the four commits since my approval at 42adfd7 (head 4234458).

My earlier LOW about the kept balance after a cancelled exit is fixed by 9ab94f1. holdForExit() now saves the kept balance before it clears it, and releaseExitHold() puts it back on cancel and on a start that failed before saving.

New in this delta:

  • MEDIUM: since 48f0723, a savings funding refused at start for a higher prepared fee has no way to retry (inline).
  • LOW: keptBeforeExit is not cleared when an exit ends or the wallet is wiped (inline).

The reorg re-asking in 17e525e and the fee-bump floor in 4234458 look correct: saved confirmations are re-asked, reorged steps and their dependants are reset, and the bump approves at least the refused fee.

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

@ovitrif

ovitrif commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 065f5af. It has @jvsena42's two findings from 5848657365 and the latest Spark base (4a29667) merged in.

  • Exit start refused for a higher savings fee (1dcb54f): the refused prepared fee becomes a floor for the next savings estimate. Funding shows it, and the next swipe approves it. A fees-changed toast replaces the generic error.
  • Kept balance after an ended exit or a wipe (b8eb8e2): keptBeforeExit is cleared in setExiting(false) and wipe().

The iOS twins are on synonymdev/bitkit-ios#1217.

@jvsena42, a re-review is needed only because of this push: 4234458...065f5af

@ovitrif
ovitrif requested a review from jvsena42 November 10, 2026 20:54
pwltr
pwltr previously requested 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.

Medium — Bound leftover-sweep fees independently of the chain host.

SparkExitChain.kt, SparkExitDriver.kt and SparkExitSweep.kt derive sweep economics from a single read host. Host fee estimates and refusal-rate text can increase the sweep rate without an independent absolute fee ceiling; the final check only preserves a dust-sized output. A compromised host can therefore consume nearly all leftover funding in miner fees or stall recovery with false UTXO data. This concerns the leftover funding address, not theft of the exit destination's funds. Apply a trusted fee budget and treat host responses as untrusted inputs.

Medium candidate — Recheck confirmation evidence before declaring exit leaves lost.

Host-reported confirmations feed isInputSpent classification. Repeated SDK Redo cycles can accumulate strikes and remove a refund leaf as claimed by an older state. The source chain is present, but exploitability depends on unverified SDK verdict behavior; no end-to-end reproduction was performed.

Low candidate — A one-block missing-payment window can ask for funding twice.

A single host's delayed indexing or false 404 can clear the own-payment marker and restore the funding prompt. If the user pays again, both payments remain at the seed-derived address and can be adopted or swept, but incur extra fees.

Low candidate — Restore or failed salvage may leave funding without automatic recovery.

The scoped files show record deletion after failed salvage and local funding-index persistence. Seed recovery may therefore need manual discovery and sweeping. Android Auto Backup configuration was outside this scope, so fresh-device restore behavior remains unverified.

Review details

General
Model: GPT-6.1 Sol
Reasoning effort: high

Security
Model: Kimi K3
Reasoning effort: high

@ovitrif

ovitrif commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 52bac5d, with @pwltr's review and the latest Spark base (50bc48b) merged in.

  • Medium, leftover-sweep fee set by the chain host (f6ac4e6): the sweep rate is now capped at the higher of the exit's own rate and Bitkit's own fast rate from Blocktank, which isn't the chain host. The cap covers both places the host could raise it: a refusal that lifts the sweep to the next-block rate, and the replacement after 144 blocks. Once at the cap, the same sweep is sent again instead of replaced. Capping the rate caps the total fee too. The failing-first test has a host quoting 5,000 sat/vB.
  • Medium candidate, strikes dropping a refund leaf: not changed. A strike needs an "inputs missing" answer while the parents count as confirmed (SparkExitDriver.kt:1223), and two Redo rebuilds that reproduce the same refund (SparkExitRecord.kt:553-558).
    • Those confirmations come from the read host (:409, :432, :715), which on mainnet is the only one (SparkExitChain.kt:166-168) and is also the SDK's own chain service (Env.kt:143-144, SparkService.kt:168-171).
    • So a host that can fake a confirmation already drives the SDK's own verdicts, Redo included. A recheck against that same host would add nothing.
    • An honest host that is slow to index only delays confirmations. It can't invent one.
  • Low, a one-block miss asks for funding again (52bac5d): a payment from Savings is forgotten only after 3 blocks unseen, and any read that finds it starts the count over.
  • Low candidate, restore after a failed salvage:
    • Real, but not a small change. The next funding index is stored only on the device (CacheStore.sparkExitNextFundingIndexes), and the backup holds only the SDK exit state.
    • On a fresh device a new exit still finds funding at index 0, but funding at index 1 or higher would need recovering by hand.
    • A fix needs a funding-address scan or a backed-up index, so I left it for a follow-up.

The iOS twins are on synonymdev/bitkit-ios#1217.

@pwltr, a re-review is needed only because of this push: 065f5af...52bac5d

@ovitrif
ovitrif dismissed pwltr’s stale review November 10, 2026 22:45

Addressed in f6ac4e6, 52bac5d; the two candidates are answered in the PR comment

@ovitrif
ovitrif requested a review from pwltr November 10, 2026 22:45
@ovitrif

ovitrif commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator Author

QA for 52bac5d:
A full in-app emergency exit on Spark's hosted regtest, Pay from savings, on an Android emulator:

  • ✅ Start and funding: the quote, one Savings payment of ₿ 20 294, Paid from savings with its transaction ID, then funding confirmed and the first step published.
  • 🟠 Done: still running. Hosted regtest's timelocks of about 3,900 blocks take about 33 h. I'll update the QA section's line and post here when it ends.

The recording is in the QA section under Test results.

@ovitrif

ovitrif commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Merged the Spark base at 6cdf592 (#1697: an unresolved Spark retry now stays held across restarts) into this PR. The only conflict was in SparkRepo.start: it now takes back the uncertain retries before the exit-aware connect. SparkServiceTest.kt's new base test passes isExiting = false. just compile, just test and just lint pass.

re-review is needed only because of this push: 52bac5d...1a4450d

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

Two independent reviews.

needs changing before merge

  • A start cancelled midway leaves the exit running with leaf optimization on (app/src/main/java/to/bitkit/ui/settings/lightning/SparkExitViewModel.kt:175-201)
  • A failure-only Redo can leave the exit rebuilding forever (app/src/main/java/to/bitkit/repositories/SparkExitDriver.kt:250-260)

worth doing, does not block

  • Opening the exit from a notification cancels its own navigation (app/src/main/java/to/bitkit/ui/ContentView.kt:398-407)
  • Amount to send from another wallet can show only in fiat (app/src/main/java/to/bitkit/ui/settings/lightning/SparkExitComponents.kt:46-49)
  • Spending hides the value left on Spark once exited leaves are published (app/src/main/java/to/bitkit/usecases/DeriveBalanceStateUseCase.kt:58-59)
  • Background exit job stays queued when there is no exit (app/src/main/java/to/bitkit/repositories/SparkExitRepo.kt:211)
  • Confirmed leftover sweeps still appear as sending (app/src/main/java/to/bitkit/ui/settings/lightning/SparkExitProgressScreen.kt:344-348)

nits

  • Journey README points at the wrong iOS PR (journeys/README.md:202)
  • Misplaced KDoc on the notification request flag (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:1419)

the reviewers disagree, your call

  • A rebuild that adopts nothing stops a valid generation from being broadcast (app/src/main/java/to/bitkit/repositories/SparkExitDriver.kt:460-465)
  • Preparing payments can spend Spark leaves after an exit starts (app/src/main/java/to/bitkit/repositories/SparkRepo.kt:414)
  • Notification taps can cancel their own navigation (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:1428-1431)

Comment on lines +175 to +201
fun start() {
viewModelScope.launch {
_uiState.update { it.copy(isStarting = true) }
sparkExitRepo.start(_uiState.value.fundFromSavings)
.onSuccess {
savingsFeeFloorSats = null
_effects.emit(SparkExitEffect.Started)
}
.onFailure {
// Savings never pays more than the fee confirmed: the next funding asks for the refused one.
val changed = it.cause as? OnchainSendChangedError
if (changed != null) {
changed.preparedFeeSats?.let { fee -> savingsFeeFloorSats = fee }
ToastEventBus.send(
type = Toast.ToastType.INFO,
title = context.getString(R.string.lightning__spending_confirm__fees_changed_title),
)
} else if (it !is SparkExitStartCancelled) {
// A cancel during the start already said so on progress.
ToastEventBus.send(it)
}
// The exit started but paying its fee funding failed: progress shows where to fund it.
if (sparkExitRepo.state.value.isExiting) _effects.emit(SparkExitEffect.Started)
}
_uiState.update { it.copy(isStarting = false) }
}
}

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.

needs changing before merge: A start cancelled midway leaves the exit running with leaf optimization on

If the user leaves the Lightning Connections screens while an exit is starting, the start is cancelled after its record is saved. The exit then runs with Spark's leaf optimization still on, nothing drives it, and the Pay from savings payment never happens. SparkExitViewModel.start() runs sparkExitRepo.start() in viewModelScope. That view model is scoped to the Routes.ConnectionsNav back stack entry (exitViewModel in ContentView.kt). startLocked does these steps first (SparkExitRepo.kt:703-736): - holds Spark - creates the transfer - saves and publishes the record Only then come its cancellable waits: - awaitOptimizationIdle(), which waits up to 2 minutes while an optimization runs - the requote SDK call - the reconnect During the spinner the Confirm screen keeps its back button and drawer icon enabled (SparkExitConfirmScreen.kt:62-63). Drawer > Wallet goes through navigateToHome, which pops the graph and clears the view model. The start then never reaches any of these: - sparkRepo.setExiting(true, reconnect = true) (:753) - work.schedulePeriodic() (:760) - onAppForegrounded() (:764) - payFunding (:767) What the user is left with: - An active record on disk, with Spark held. - Spark still connected with autoEnabled = true for the rest of the process. SparkRepo.start cannot correct this, because SparkService.connect returns early while sdk != null. - No background work until the next ON_START. - No savings payment and no error. Progress offers only the funding address, because the payment was never refused. This is the leaf swap hazard the PR says it prevents. A smaller variant: backing out to Funding without leaving the graph drops SparkExitEffect.Started. It is a no-replay SharedFlow that only Confirm collects, so the user stays on Funding. There savingsFundingFee now fails on the null pending and says savings can't cover the fee. How I confirmed it: traced the lines above, and two independent reviewers found the same path. I could not run the unit tests (the private Maven packages need GitHub Packages credentials) and did not reproduce it on a device. Fix: run start, payHigherFee, retrySavingsPayment and cancel in the repo's own scope, or NonCancellable once the record is saved. Navigate to Progress from repo state rather than a one-shot effect.

Comment on lines +460 to +465
if (!needsUnverifiedRebuild(sent)) return sent
// A check cannot settle a step the SDK could not read; a fresh build reads the chain again.
Logger.info("Rebuilding Spark exit over steps the SDK could not read", context = TAG)
val rebuilding = sent.copy(isRebuilding = true, unverifiedRebuiltAtHeight = tip, isRedoRebuild = false)
save(rebuilding)
build(rebuilding, sdk)

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.

the reviewers disagree, your call: A rebuild that adopts nothing stops a valid generation from being broadcast

An exit generation that is still Valid can stop being broadcast for hours to days if the Unverified rebuild it triggers adopts nothing. - check() saves isRebuilding = true before calling build() (SparkExitDriver.kt:460-465). - If the SDK's prepare then skips every exit leaf as Unverified, FundsNotFound or OperatorsUnavailable, build() returns at :220-223 with retrySince set and isRebuilding still true. - Every later tick takes the Running if isRebuilding branch (:169-173): - Without the SDK it returns before the keyless pass. - With the SDK, build() stops at the retry gate. The gap there doubles from 6 to 12, 24 and so on, up to 144 blocks (SparkExitMath.isRetryDue). - So check(), broadcastReady and broadcastWithoutSdk never run. Matured steps of the still-valid generation are not sent, which can run past the roughly 50-block operator window. I traced this but did not reproduce it. Whether prepare skips leaves while check reports Unverified depends on the SDK. Suggested fix: clear isRebuilding when a rebuild over a Valid verdict adopts nothing, so the old generation keeps going while the wait runs.

Objection: Real, but blocking rather than non-blocking: The Valid branch saves isRebuilding, and an empty prepare with waiting leaves preserves it. Subsequent ticks bypass checking and keyless broadcasting during the retry gaps, which can leave mature transactions unsent through the operator window.

Comment on lines +398 to +407
val isSparkExitRequested by appViewModel.isSparkExitRequested.collectAsStateWithLifecycle()
LaunchedEffect(isSparkExitRequested) {
if (!isSparkExitRequested) return@LaunchedEffect
// Waits for the navigation graph, which a cold start from the notification sets up only after this runs.
navController.currentBackStackEntryFlow.first()
val showsResult = appViewModel.onSparkExitOpened()
// Inside the connections graph, so the exit screens find their shared view model and Done goes back to Spark.
navController.navigateTo(Routes.SparkDetail)
if (showsResult) navController.navigateTo(Routes.SparkExitProgress)
}

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.

worth doing, does not block: Opening the exit from a notification cancels its own navigation

Tapping an exit notification on a cold start most likely leaves the user on Home instead of opening the exit. - The LaunchedEffect at ContentView.kt:398-407 is keyed on isSparkExitRequested. - AppViewModel.onSparkExitOpened() sets that flag to false before it suspends in sparkExitRepo.load(), which is a withContext(bgDispatcher) call. - The flag change recomposes ContentView, which cancels the running effect. - When load() returns, withContext throws CancellationException. Neither navigateTo runs, and the request has already been cleared. On a cold start the first load() reads the record file, Room and bitkit-core, which takes longer than a frame. On a warm start it is a race. I traced this but did not run it on a device. QA case 37 is still unchecked. Fix: clear the flag only after navigating, or collect isSparkExitRequested.filter { it } in LaunchedEffect(Unit).

Comment on lines +46 to +49
/** An amount as plain text for copy that names it, in the user's primary unit. */
@Composable
internal fun sparkExitAmountText(sats: ULong): String =
rememberMoneyText(sats.toLong(), showSymbol = true)?.replace(Regex("</?accent>"), "") ?: sats.toString()

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.

worth doing, does not block: Amount to send from another wallet can show only in fiat

The amount to send from another wallet is shown in the user's primary unit. With fiat as the primary unit, the funding screen says something like "Send at least $ 3.21 to this address". - sparkExitAmountText (SparkExitComponents.kt:46-49) uses rememberMoneyText with the primary display unit. - It is the only amount the user is given in each of these places: - the funding address text (:119-121) - the partial-payment line (:136-139) - SparkExitFundingScreen.kt:84 - The QR code carries only the address. The exit needs an exact on-chain amount plus input fees. A fiat figure converted at the other wallet's rate can underpay, which leaves Continue disabled. When rates have not loaded, the fallback prints a bare number with no unit. Suggested fix: always show sats here, or put a BIP21 URI with the amount in the QR.

Comment on lines +58 to +59
val sparkSats = sparkRepo.state.value.shownSpendingSats.safe() +
(sparkExitRepo.state.value.outsideExitSats ?: 0uL).safe()

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.

worth doing, does not block: Spending hides the value left on Spark once exited leaves are published

During an exit, Spending stops counting the Spark value the exit leaves behind once Spark's balance no longer includes the exited leaves. - DeriveBalanceStateUseCase.kt:58-59 adds SparkExitState.outsideExitSats. - That value is balance - leafTotalSats, and it is null whenever the balance is not above the exit's leaf total (SparkExitFunding.kt:316-319). - The KDoc at SparkExitRepo.kt:1097-1100 and the comment in SparkRepo.exitBackup both say published leaves leave Spark's balance. - From then on the balance holds only the skipped leaves, which is less than leafTotalSats. Spending shows 0, and Home's total drops by what stays on Spark (14,167 sats in the hosted-regtest QA run) for the weeks the exit takes. - sparkBalanceSats is also null after a restart until a tick with the SDK sets it. The new DeriveBalanceStateUseCaseTest case only covers the balance before any leaf is published. Suggested fix: keep the quote's unexitable amount in the record and count that, or sum Spark's leaves outside record.leafIds.

private fun isAutomaticPaykitWorkPaused(): Boolean =
isPaymentRequestPollingStopped.value || privatePaykitRepo.isProfileDeletionInProgress.value

/** Drives a Spark emergency exit every minute while Bitkit is in the foreground. */

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.

nit: Misplaced KDoc on the notification request flag

AppViewModel.kt:1419 puts "Drives a Spark emergency exit every minute while Bitkit is in the foreground." on _isSparkExitRequested. That sentence describes startSparkExitDriving. CLAUDE.md also asks for no comments on private members.

runSuspendCatching {
ensurePaymentDeadline(paymentDeadlineAt)
if (!sparkService.isConnected) throw SparkError.SendRefused("Spark is not connected")
if (_state.value.isExiting) throw SparkError.SendRefused("Spark is in an emergency exit")

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.

the reviewers disagree, your call: Preparing payments can spend Spark leaves after an exit starts

An already preparing payment can submit on Spark after an exit starts. The exit check precedes several suspending calls and is never repeated or serialized with holdForExit(). I reproduced this with the unchanged method body: pausing onBeforeSend, activating the hold, then resuming called sparkService.send successfully with isExiting=true. A background payment can therefore spend leaves the exit has already selected.

Objection: Real, but non-blocking rather than blocking: Real: isExiting is checked only at line 414. The final pre-send check at line 437 calls only onBeforeSend(), so a send already past prepare when holdForExit() flips the flag still reaches sparkService.send. Not blocking: it needs a send in flight at the moment the user starts the exit, and the start's requote() already picks up any send that finished before it, so only a submit after requote spends quoted leaves.

Comment on lines +250 to +260
if (built.transactions.isEmpty()) {
// A first build of only cooperative leaves the operators could not co-sign is kept: they are retried for a
// week like any failed leaf. A rebuild like it keeps the recovery it has and tries again on the next tick.
if (built.failed.isNotEmpty() && record.recovery == null) {
val adopted = record.adopt(built, nowMillis())
.copy(waitingLeafIds = waitingLeafIds(record, prepared.skipped, stopped))
save(adopted)
Logger.info("Built Spark exit of only '${built.failed.size}' failed cooperative leaves", context = TAG)
return adopted
}
if (built.failed.isNotEmpty()) return record.copy(failedLeafCount = built.failed.size)

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.

needs changing before merge: A failure-only Redo can leave the exit rebuilding forever

A Redo that leaves only cooperative recoveries the operators refuse to sign can rebuild forever. This return preserves the old recovery and isRebuilding, without starting failedLeafRetrySince. I ran the driver with that response through tips 1000 to 2008: every tick rebuilt and the failed leaves never entered the saved recovery. The one-week release and leftover sweep were never reached.

Comment on lines +1428 to +1431
suspend fun onSparkExitOpened(): Boolean {
_isSparkExitRequested.update { false }
sparkExitRepo.load()
return sparkExitRepo.state.value.showsResult

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.

the reviewers disagree, your call: Notification taps can cancel their own navigation

Tapping an exit notification can leave the user on the current screen instead of opening Progress. onSparkExitOpened() clears the flag that keys the effect in ContentView before awaiting load(). If loading spans a recomposition, the changed key cancels the coroutine before navigation. I reproduced cancellation before navigation by suspending the load through a recomposition, which breaks QA case 37 on a slow or cold start.

Objection: Real, but non-blocking rather than blocking: Confirmed: the LaunchedEffect(isSparkExitRequested) in ContentView calls onSparkExitOpened(), which sets the key to false and then suspends in load() (withContext(bgDispatcher)). A recomposition before it resumes restarts the effect, the old coroutine is cancelled at the withContext return, and since the flag is already cleared nothing navigates. It is UX only and a second tap recovers, so not blocking.

Comment on lines +344 to +348
sweep?.txid != null && sweep.isAccepted -> SparkExitTxNote(
text = stringResource(R.string.lightning__spark_exit__leftover_sending)
.replace("{amount}", sparkExitAmountAccent(sweep.valueSats)),
txid = sweep.txid,
testTag = "SparkExitLeftoverSending",

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.

worth doing, does not block: Confirmed leftover sweeps still appear as sending

Progress still describes a confirmed leftover sweep as sending. sweepLanded() retains leftover and sets isConfirmed=true, while this branch checks only isAccepted. A Done record with one confirmed sweep therefore keeps selecting leftover_sending; only earlier sweeps receive the completed copy.

@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 the four commits since 4234458ee (head 1a4450dd2): both of my findings are fixed and nothing new. Approving.

  • MEDIUM, refused savings funding (1dcb54f7f): fixed as offered on the thread. SparkExitViewModel.kt:185-187 stores the refused preparedFeeSats as savingsFeeFloorSats, and SparkExitRepo.kt:655 prices the next funding at maxOf(estimate, floor). That value is the approval ceiling at :913, so LightningRepo.kt:1919-1922 lets the second swipe through. Covered by the new SparkExitRepoTest and SparkExitViewModelTest cases.
  • LOW, keptBeforeExit (b8eb8e243): cleared in wipe() (SparkRepo.kt:243) and in setExiting(false) (:269). cancel() still restores the hold first (SparkExitRepo.kt:1001-1002).
  • Sweep fee cap (f6ac4e6d8): the ceiling is max(exit rate, Blocktank fast rate) and never below the confirmed exit rate (SparkExitDriver.kt:797, :947-949). At the ceiling the same sweep is re-sent.
  • Three blocks unseen (52bac5dbd): constant only; a known read still resets the count.
  • Merge 1a4450dd2: restoreUncertainSends() stays ahead of connect(walletIndex, isExiting) (SparkRepo.kt:212-215); it only reads the cache.

coreyphillips's threads at this head (including the two marked "needs changing before merge") are his to settle; this approval does not cover them.

Device gate: transfer/spark-emergency-exit.xml passed at a8e1462c0 and was not re-run. The delta changes the refused-fee retry and the leftover sweep cap, which the journey does not reach; the new unit tests cover both. perf: not relevant (Blocktank fees are read only after a sweep refusal or an overdue replacement). CI is green at 1a4450dd2.

Reviewer model: Claude Fable 5.2 (claude-fable-5-2), Claude Opus 5.5 (claude-opus-5-5)
Reasoning effort: not exposed

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.

6 participants