Repository navigation
Conversation
bf469f4 to
ef7130e
Compare
01334a9 to
4ed36e0
Compare
a1412d5 to
a1c333c
Compare
9982a55 to
d964a5f
Compare
eab67dd to
567e1a8
Compare
|
QA: 🟢 Restore Spark exit dataExit data backs up as its own category. After a wipe and restore, the Pixel's Spark balance and claimed deposits came back, but the log counted 0 imported leaves because the exit data was restored after Spark had already synced. Ran on 567e1a8; the order is fixed in f96bae9, and a rerun is pending.
🟢 Claim a depositTransfer to Spending: Spark claimed each deposit after 3 confirmations, keeping 297 sats of 15,000 and 198 of 2,500. Ran on an earlier head of this PR, before the 27 Oct rebase. 🟢 Receive with zero balance, then payWith no channel and a 0 Spark balance, the Pixel received 1,024 sats over Lightning and paid 128 from them two minutes later: Lightning received into Spark is spendable at once. The send's activity showed a fee of 0, fixed in 972e5ec. Ran on 567e1a8.
🟢 Move Spending to SavingsTransfer to Savings withdrew 12,973 sats from Spark for a 1,470 sat fee; the withdrawal showed in Savings unconfirmed after 22 seconds and confirmed after 31 minutes. Ran on an earlier head of this PR, before the 27 Oct rebase; a later run on a build of #1710 lists the withdrawal as a Transfer from Spending.
🟢 Fees against BlocktankOn the same wallet, a 15,000 sat Transfer to Spending cost 907 sats in all through Spark (6.0%) and 1,584 through Blocktank (10.6%). Moving back to Savings cost 1,470 sats through Spark, where Blocktank channel closes cost 183 to 1,113. 🔴 Pay a Paykit request over SparkThe iPhone paid this Pixel's 204 sat Paykit request over Spark and it arrived at once, but the iPhone stayed on Payment Pending; fixed in 024b6bb and synonymdev/bitkit-ios@735e972e, not rerun on a device yet. An earlier attempt failed because this Pixel republished a private invoice already paid through Spark, fixed in 972e5ec.
|
901ecd8 to
2ba664e
Compare
|
Removed from the 3.1.0 milestone: this draft is the deliverable of a finished Spark experiment and does not ship in 3.1.0. |
024b6bb to
d7be600
Compare
|
Retargeted this draft to master before merging its base, #1696, so deleting that branch would not close it. |
There was a problem hiding this comment.
detekt found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
Rebuilt this branch on master after the Blocktank sunset (#1696). Master removed the old Transfer to Spending flow, so Spark now has its own amount, confirm and deposit screens, reached from Funding and the transfer buttons while Spark is ready. The deposit screen offers Breez's early claim at the fee the provider quotes for that deposit. The history was rewritten into new commits, so the shas under Evidence predate the rebuild. This PR stays a draft. |
cab0f89 to
fa49b5a
Compare
ab34526 to
ab1b09d
Compare
|
QA after the split, on ab1b09d: dev build on an Android 15 emulator, Spark on its hosted regtest, Savings funded with 200,000 sats on staging regtest.
J1 to the confirm screen and Learn more, then Savings › Transfer To Spending with no intro (2x speed).
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Ready for review. 3.1 is cut, so this opens the Spark work for 3.2. Before marking it ready:
The QA section lists every case with its revision, and names what's still untested on a device. The emergency exit (#1782) and pay-contacts-via-Spark (#1754) stay stacked on this PR. Some screens may change once the design thread in #design-ux is answered. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b76a136de
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
[Critical impact] The PR changes how Lightning payments and wallet funds move across Bitkit. Fix the hidden exit-data import errors, missed pending-send results, and missing funding recovery before merging. Findings
SummaryAdds a Breez-backed Spark wallet alongside LDK and restores transfers between Savings and Spending.
No runtime tests or Android visual captures were performed. Acknowledged scope from Diagram%%{init: {'theme': 'neutral'}}%%
flowchart TD
Savings[Savings or hardware wallet] --> Funding[Confirm amount and fees]
Funding --> Deposit[Send to Spark deposit address]
Deposit --> Speed{Claim speed}
Speed -->|Instant| Claim[Request early claim within accepted fee]
Speed -->|Later| Mature[Wait for confirmations]
Claim --> Spark[Spark Spending balance]
Mature --> Spark
Payment[Lightning payment] --> Capacity{Channels can carry it?}
Capacity -->|Yes| LDK[LDK channels]
Capacity -->|No| Spark
Spark --> Withdrawal[Quoted withdrawal to Savings]
Spark --> Exit[Export exit data]
Exit --> Merge[Merge stored backup before first upload]
Merge --> VSS[Encrypted VSS backup]
Reviews (1) · Last reviewed commit: "fix: harden spark backup, wipe, sends an..." · Reviewed by Greptile |
|
Triage: in 3.2.0. This PR closed no issue, so I filed #1805 to carry the milestone, moved 3.2.0 from this PR to that issue and added |
talosmachina
left a comment
There was a problem hiding this comment.
No findings. Adds a Breez SDK on Spark wallet: Lightning sends and receives fall back to it when no channel can carry the payment, Savings funds it through the restored Transfer to Spending flow, and its unilateral exit data is backed up to VSS. Reviewed 9b76a13, oversized tier (8,442 added lines in 110 files), static: on the Bitkit track an auto review does not build the branch.
What I checked, and 4 candidates I ruled out
Read in full: SparkService.kt, SparkRepo.kt, the LightningRepo.kt and BackupRepo.kt changes, SparkFundingViewModel.kt, the workflow and Gradle changes.
Not read: the transfer screens, settings screens and the remaining UI files.
CI: lint, build-local and compose-ui green; e2e still pending when reviewed.
Ruled out
- Double send of one invoice:
sendsInFlightplus the completed/pending lookup inpayInvoicerefuse a second send. - Exit backup overwriting a stored one before merge:
uploadSparkExitBackupmerges the remote copy first and fails rather than replace one it could not read. - Spark reconnecting during a wipe:
isWipingis set on the service beforewipeStorage, andconnectrefuses while it is set. - Release without the Breez key:
release.ymlfails the job whenBREEZ_API_KEYis empty; the internal build passes and leaves Spark off, as its comment says.
Merge confidence: 3/5, the oversized tier: the UI remainder was not read.
| outcome(it)?.let { result -> WatchResult.Complete(result) } ?: WatchResult.Continue() | ||
| } | ||
| } | ||
| lightningRepo.settledSparkSendEvent(hash)?.let(::outcome)?.also { watch.cancel() } ?: watch.await() |
There was a problem hiding this comment.
This lookup also runs after an LDK send. If an invoice first fails through Spark and is retried once a channel has enough capacity, the old Spark PaymentFailed cancels the LDK watcher here and shows the retry as failed while LDK may still be paying it. Could we settle from the rail used for the current attempt, and cover retries in both directions?
The reverse direction also affects Activity: after a failed LDK attempt succeeds through Spark, onPayment writes the succeeded activity and emits PaymentSuccessful, but handlePaymentSuccessful calls refreshPaymentActivity, which reads the retained LDK failure and overwrites that row as failed in CoreService.processBolt11. That refresh needs the same distinction.





Closes #1805
Twin: synonymdev/bitkit-ios#1128
This PR keeps Lightning in Bitkit after Blocktank stopped selling channels: on top of the Blocktank sunset (#1696), Spark becomes the easy option where Transfer to Spending now leads to manual setup. Savings to Spending funds a Breez SDK on Spark wallet through the restored transfer screens, Lightning sends and receives fall back to it whenever no channel can carry the payment. Existing channels keep working until they close, and ldk-node keeps running the on-chain wallet.
The emergency exit lives in the stacked exit PR, #1782, and the Paykit route over Spark in #1754, both stacked on this one.
Description
What ships in 3.2
https://bitkit.to/electrs.SPARK_EXITbackup category.Transfer to Spending on Spark
HwSendViewModel.signAndBroadcasttakes the address from its caller), so the restored intro, amount, sign and signed screens send to the Spark deposit address, priced from the device's account, with the same speed choice and setting-up screen. The hardware amount step reuses the restored amount screen with its old test tags, instead of a second copy of it. The send is recorded as a transfer.hasSeenTransferIntroandhasSeenSpendingIntroin settings and their migration from the React Native app, removed by the sunset.Spark wallet and Lightning
breez_sdk_spark:bindings-android:0.28.0) withSparkServiceandSparkRepo, so that a Spark wallet starts beside the node from the same recovery phrase and passphrase. Spark derives its own keys onm/8797555', so there is no new phrase to back up.realTimeSyncServerUrl = null), its LNURL server (lnurlDomain = null) and its default external input parsers (useDefaultExternalInputParsers = false) in Spark's config on every network, so the app sends no traffic to Breez's data-sync orbreez.tipsLNURL servers, offers no Lightning address, and the SDK sends nothing the user scans to third-party parsers.https://bitkit.to/electrs) as its chain service, in place of the SDK's default mempool.space and blockstream.info; regtest and staging keep the SDK's default.BREEZ_API_KEYbuild property, so that mainnet builds without a key keep Spark off; regtest builds use Lightspark's hosted regtest, which needs no key.ESPLORA_URLbuild property for regtest builds, which puts the node's chain source on an Esplora endpoint, plusscripts/spark-regtest-esplora.pyandscripts/spark-regtest-fund.py, so a dev build runs on Spark's regtest chain and QA can fund it and test Savings to Spending and the claim without mainnet.BREEZ_API_KEYfrom thereleaseenvironment in the release workflow and fails the release when it is missing, so Spark can't ship switched off; internal release builds pass it through when therelease-internalenvironment has it. A repo admin has to addBREEZ_API_KEYto thereleaseenvironment (and torelease-internalfor internal builds with Spark).SPARK_EXIT, encrypted like the other backups. It is exported once Spark has connected and synced, and again, at most once a minute, whenever the SDK reportsUnilateralExitStateChanged; until Spark has synced the backup waits and is not marked as backed up. The first upload in each app session reads the stored backup and merges it into Spark before replacing it, and fails rather than replace a backup it could not read, so a failed or skipped restore never loses the stored exit data. On restore it is imported right after Spark connects, without contacting the operators, so a wallet restored on a new device can still exit on-chain while the operators are unreachable; a failed restore of it shows as an incomplete restore. The category shows in Backup settings only where Spark runs.Transfer to Savings selection
Paykit on Spark
Screen layout
Out of Scope
Risks for review
SPARK_EXIT; upstream VSS accepts request bodies up to 1 GiB by default, but a large wallet's upload against Synonym's VSS is untested.250 × sat/vB + 750sats.CLAIM_TX_SIZE_VBYTES,deposits.rs), so it is a flat cost that weighs heavily on small transfers. The confirm screen shows it as Spending fee, with To spending after it, for the speed the user picks.libbreez_sdk_spark_bindings.soadds about 11 MB per ABI (arm64, uncompressed).ESPLORA_URL.Design
Restored transfer screens, on the latest
Bitkit - Handoffpage of the Bitkit Figma file (frame names fromdocs/screens-map.md; the Spark speed rows, Spending fee and setting-up states have no frame yet):Figma has no Spark or unlimited-receiving state for the Lightning Connections header, so No limit uses the same style and arrow as the spending figure beside it; @aldertnl, a design for this state is still open.
UI pending design: design was requested in the #design-ux Spark 3.2 thread, and some screens may change after the designers answer. The exit UI polish list isn't part of this PR.
Preview
Transfer to Spending on Spark's regtest chain: the Spend card, both intros, Funding, the amount pad with 25%, Max and delete, confirm with Instant and Later, and Learn more; then an Instant transfer Spark added after 1 confirmation (
1adfc47).t1-intro-amount-confirm2x.mp4
t2-instant-settingup2x.mp4
The setting-up screen for a Later transfer (
1adfc47), and for an Instant transfer the provider offered no early claim for (3e60f04).t3-later-settingup2x.mp4
t4-instant-unavailable2x.mp4
Earlier Spark cases: the Spending invoice from Spark without channels (1,
7aa44f6), and Spark in Lightning Connections with a pending deposit (10,07a1df7, balance staged with a local hook).r1-receive2x.mp4
r72x.mp4
Transfer to Savings selection on Spark's regtest chain (
b0169b275, channel rows staged with a local hook): Spark alone goes straight to confirm with its fee; with channels, Advanced lists Spark and each channel, all on, and confirm shows only what is selected.case1-spark-only-no-channels2x.mp4
case2-channels-and-spark2x.mp4
case3-partial-selection2x.mp4
Channels selected with Spark off (no Spark fee), nothing selected (Continue disabled), and channels with no Spark balance (no Spark row) (
b0169b275).case3b-channels-selected-spark-off2x.mp4
case4-nothing-selected2x.mp4
case6-channels-only-no-spark2x.mp4
Spark only picked from the list beside two channels, then swiped (
5edd5a8ce): no channel closes, and Savings receives 97,711, the 99,901 balance less the 2,190 Spark fee shown.case5-spark-only-selected-swipe2x.mp4
Screens
Transfer to Spending
Transfer to Savings
Lightning Connections
QA Notes
Setup
BREEZ_API_KEY=<key>inlocal.properties, and small amounts.python3 scripts/spark-regtest-esplora.py --port 3999. It fronts Spark's regtest mempool API (the SDK's own regtest default,sdk_builder.rs) and adds the two Esplora calls it lacks and ldk-node needs:/fee-estimatesat start and/block/<hash>/statusfor Lightning sync.ESPLORA_URL=http://10.0.2.2:3999 ./gradlew assembleDevDebug, so the node's chain source is Esplora on Spark's chain instead of Electrum on Blocktank's staging chain. For a phone, run the proxy with--bind 0.0.0.0and use the host's address.FAUCET_USERNAME=… FAUCET_PASSWORD=… python3 scripts/spark-regtest-fund.py <Savings address> 100000through Lightspark's API faucet (credentials issued by Lightspark; Breez's CI uses the same,faucet.rs).POST /txwithsendrawtransaction RPC error: {"code":-1}, whilePOST /v1/txs/packageaccepted the same transaction. If a Savings send fails that way, forward/txto the package endpoint (a one-element JSON array) in the proxy.TransferViewModelTest.kt. Channels with no Spark balance was opened through the debug screen linkbitkit://screen/savings-availability, since Spending was 0.ESPLORA_URLstays on Blocktank's staging chain: Spark start-up, invoices and screens work, but its Savings to Spending deposit is never seen by Spark.Journeys
transfer/spark-transfer-to-spending.xml— with Spark ready, the suggestion card opens the Spending Balance intro and Funding, Transfer from Savings opens the Transfer to Spending intro and the amount screen; 25% and Max fill the amount, confirm shows the four tiles with Instant and Later, Later updates the tiles, Learn more opens and returns; the intros show once.transfer/transfer-unavailable-sheet.xml— now needs Spark not running, since a ready Spark replaces the sheet.amount-limits/transfer-spending-over-max.xml— the amount pad caps at Max with the Spending Balance Maximum toast.amount-limits/transfer-spending-preset-delete.xml— after 25%, Max or a unit switch, each delete removes one digit.transfer/spending-confirm-amount-change.xml— a lower price at swipe pays at once; a higher one asks for another swipe.hardware-wallet/transfer-to-spending.xml— a paired Trezor transfers into Spark through amount, sign and signed to the setting-up screen.hardware-wallet/passphrase-transfer-to-spending.xml— a hidden wallet signs with its live session, re-prompts after the session drops, and refuses a wrong passphrase.hardware-wallet/transfer-to-spending-node-warmup.xml— a hardware transfer started during node warm-up reaches the sign screen.transfer/savings-select-funds.xml— Select funds lists Spark and each channel, all on; confirm shows only what is selected, with Spark fee only while Spark is selected; Transfer all restores everything; nothing selected disables Continue; stops before the swipe.Manual Tests
ESPLORA_URLon Spark's regtest chain, Savings funded from the faucet → Transfer to Spending; after 1 confirmation Spark claims the deposit and Spending shows it; Lightning Connections lists it as claimed — Spark's regtest chain.Imported Spark exit databefore Spark is used, and Spark's balance and deposits match — needs two installs.Merged stored Spark exit databefore it replaces the stored backup — needs two installs and control of VSS reachability.Automated Checks
SparkRepoTest.ktcases — Instant claims early within the ceiling and reaches Claimed, asks above it, shows the repriced fee from a repriced claim, pays a repriced claim at the new fee, is unavailable with only the standard claim or a declined claim, says Spark has not seen a deposit never listed, and Later makes no claim call; a restored exit backup is imported once Spark connects, one from another network is skipped, a send recorded with fee 0 is re-recorded with the fee a later sync reports, a withdrawal is listed with its fee and its arrival marked as a transfer, and a Spark send refused before it leaves (fee over the limit, past its deadline) is a typed refusal while a failure inside the send is pendingSparkFundingViewModelTest.kt— Max is Savings less the network fee and 25% a quarter of it, Max is 0 without Savings, Continue refuses an amount under the Instant fee, prices and opens confirm, the swipe asks again when the fee rose and refuses when Savings no longer cover it, sends at the quoted fee rates and follows the claim at the chosen speed, and a hardware transfer prices from the device and hands signing a transfer requestSparkClaimOfferTest.kt— a quote maps to the standard claim, an automatic early claim within the cap, or a choice above the cap or with automatic claiming off, and each claim outcome maps to claimed, waiting, repriced or declinedSparkServiceTest.kt— Lightning sends return before completion; the config turns off Breez data sync, the Lightning address server and the default external input parsers on every network; the Later fee prices the mature claim at the half-hour rate (198 sats at 2 sat/vB); connect is refused during a wallet wipe; wipe deletes Spark's storage and a staged exit data restoreContentViewTest.kt— the spending start route shows the intro until seen, for Savings and hardware wallets, and setting up drops the confirm screen from the back stackPrivatePaykitRepoTest.kt— a private invoice paid through Spark is replaced on the next refreshPaykitPaymentProofRepoTest.kt,PaykitAllowancePayerTest.kt,PaykitAllowanceRepoTest.kt,AppViewModelSendFlowTest.kt— a Lightning proof settles from Spark when its event was missed and is removed when Spark failed the send, a send neither rail knows was never made, a Spark send is never read as unsent before Spark has synced, Spark becoming ready or syncing settles open allowance attempts and republishes the Paykit endpoints, Spark sends keep the deadline and fee limit, a receive recorded without its event gets its contact, and the send limit leaves room for Spark's service feeBackupRepoTest.kt—SPARK_EXITuploads the exported state and restores it, defers without uploading until Spark has connected and synced, merges the stored backup into Spark before the first upload, never replaces a stored backup it could not read, reports a failed restore as incomplete, and uploads when Spark was ready and synced before observing startedEditInvoiceVMTest.kt— Receive asks for no liquidity or CJIT while Spark can receiveDeriveBalanceStateUseCaseTest.kt— Spark balance counts toward Spending, caps max send, and pending deposits count as in transferTransferViewModelTest.kt— a selection moves only the picked channels and Spark: Spark only closes no channel and doesn't wait for a peer, channels only leave Spark untouched, both move both; a swap skips Spark when it is deselected and is sized by the selected channels; a quote left from a channel selection never swaps a Spark-only transfer; Spark's fee is priced once per balance and gives up after 15 s; a failed Spark withdrawal is reported while the selected channels still close; Spark is never withdrawn at a fee the user was not shown, and is withdrawn once per confirmed transferSparkRepoTest.kt— a Spark withdrawal pays the quote its fee estimate priced while it holds and asks again once it expired or the address changed; its arrival is marked a transfer by the txid its send returned; a withdrawal above the confirmed fee, or leaving dust, never leaves; a failure inside a send ends as failed once Spark has synced and holds no record of it; an invoice in flight, pending or paid is not sent again; the exit backup waits for Spark's first sync; a wipe fails when Spark's storage is not deleted and clears Spark's state; a Max send keeps the routing fee Spark quotes for its whole balance; a Later deposit reads as claimed once Spark claims it; a payment event settles even when the balance read fails; a restore whose exit data Spark rejects fails and keeps it staged; a sent withdrawal succeeds when the refresh after it failsPaykitPaymentProofRepoTest.kt,PaykitAllowancePayerTest.kt— a Spark retry decides a Lightning payment the node failedSparkFundingViewModelTest.kt— a funding signed but not accepted opens its recovery instead of an errorPaymentFailureReasonExtTest.ktcase — Spark and SDK failures show the existing payment failed copyNotifyPaymentReceivedHandlerTest.kt— an on-chain receive from a Spark withdrawal is a transfer with no received sheetLightningRepoTest.kt— a Spark send is refused while a wallet restore is pending, or when one begins during authorizationLightningRepoTest.kt,WalletRepoTest.kt,WalletViewModelTest.kt— construct the new Spark dependency, idle, so existing channel paths are unchangedQA
Each result at the revision it ran on. Dev builds ran on Android 15 and 16 emulators on Spark's hosted regtest; mainnet cases on a Pixel 10 Pro.
spark-transfer-to-spending.xmlon 4e22b9c, Breez SDK 0.28.0: passed, with Savings funded with 100,000 sats on staging regtest. It passed before on ab1b09d, and by hand on 1adfc47 (recording in Preview).TransferViewModelTest.ktandNotifyPaymentReceivedHandlerTest.kt, not run on a device.SPARK_EXITmerge before the first upload; covered bySparkRepoTest.kt,TransferViewModelTest.ktandBackupRepoTest.ktonly. The default external input parsers change is not run on a device either.https://bitkit.to/electrs/fee-estimates, so Spark's fee source works against Bitkit's Esplora.Models used
claude-opus-5-5(reasoning:low)claude-opus-5-5(reasoning:low)claude-sonnet-5-5(reasoning:Unknown)The review's session exposes no reasoning-effort setting, so its effort is Unknown.