feat(wallet): send and receive bitcoin on-chain

[dmnyc]

Spark wallets hold a static Bitcoin deposit address and can pay to one, but neither was reachable from the app. Receiving on-chain meant finding the address somewhere else, and a deposit that arrived was invisible until it settled into the balance on its own.

Receive

A Lightning / Bitcoin selector on the receive sheet. The Bitcoin tab shows the deposit address as a QR and as text, with copy, share, and rotation. Older addresses stay valid for future deposits, so rotating costs nothing.

Pending deposits

Deposits are tracked from WalletStore, not from the receive screen.

A deposit arrives with no user action and takes three confirmations to mature, so nothing on screen prompts a refresh at the moment one lands. Tracking it from a view would have made "my money arrived" a fact you only learn by sitting on the right tab — which is exactly the anxious moment not to hide it. The store watches while a Spark wallet is connected, so any surface can show it.

The wallet dashboard gets a banner with the amount and current status, opening the receive sheet on the Bitcoin tab.

Each pending deposit offers its transaction id and a link to a block explorer. That's deliberate: DepositInfo reports only a matured / not-matured flag, with no confirmation count, so the wallet genuinely cannot say "2 of 3" — the explorer is the authoritative answer, and tapping through is the user choosing to ask it rather than the app phoning an explorer unprompted.

Claim failures

Claiming stays automatic and is unchanged. What's new is visibility when it fails.

The SDK's claimError was never read, so a deposit whose claim fee exceeded the automatic cap sat unclaimed with nothing said. Those now surface with the reason, and a retry that names the required fee and asks first — paying more than the automatic cap should be a deliberate choice, not a silent one.

Send

The existing field now accepts a Bitcoin address or BIP-21 URI alongside what it already took. A BIP-21 that also offers a Lightning invoice takes the invoice — cheaper and instant.

On-chain sends quote before they send. An on-chain fee is added on top of the amount rather than taken out of it, and doesn't scale with the amount, so a small send can lose a large share of itself to fees. A fee over a tenth of the amount says so and points at Lightning.

Send all funds quotes with the fee coming out of the balance instead — a send of the whole balance can't pay a fee on top.

Available balance

The send screen now shows the spendable balance.

On-chain this isn't a convenience. What clears is the amount plus the fee, so a send that looks affordable can fail on a total the user was never shown. A quote beyond the balance is refused with the total spelled out.

Stranded tokens

Draining warns when it would leave a token balance behind. balanceSats is bitcoin only, so emptying a wallet imported from an app that holds stablecoins strands those — in a wallet Wisp deliberately won't convert with.

This is disclosure, not a fix. The wallet would otherwise look empty while it wasn't. Moving those balances still needs the conversion work tracked separately.

Safety

Every quote is held and re-checked before sending. A screen that has drifted from what was agreed re-quotes rather than sending a different amount or destination, and each field participates in quote equality so nothing can be swapped between confirming and sending.

On instant (0-conf) claims

The SDK exposes them, and I built and then removed the feature. It can't work through this SDK version, and the reason is worth recording:

• Spark sells 0-conf by validating at broadcast and absorbing double-spend risk.
• But listUnclaimedDeposits doesn't report a deposit until it already has a confirmation. Verified directly: the SDK's unclaimed_deposits table stayed empty and the txid appeared nowhere in its storage for the entire time a real test deposit sat in the mempool.
• So the deposit becomes actionable at precisely the moment the plan stops existing. A live attempt returned No instant (0-conf) claim plan available.
• There's no quote API to ask whether a plan exists first, so the button could not be shown only when it would work.

A "Claim now" that we can't predict and that usually fails teaches people the wallet is broken, so it's out. The InstantClaim status is still parsed — if the SDK ever claims a deposit that way itself, the UI needs to know not to touch it mid-settle.

Testing

Unit tests cover the SDK-free models: deposit identity and claimability, failure classification and retry-worthiness, summary aggregation, fee totals under both policies, the disproportionate-fee threshold, the zero-amount divide guard, and that every quote field participates in equality.

Exercised end to end on the simulator against a real wallet: an on-chain deposit of 38,475 sats and a second of 19,890 sats were received at the generated address, appeared as pending deposits, and were claimed automatically once matured.

Not exercised against a live SSP: the claim-failure retry path, which needs a deposit whose fee genuinely exceeds the cap. It's modeled and unit-tested but has not been seen to fire.

Notes for review

OnchainSendSpeed duplicates WithdrawOnchainSpeed from #452. Done — #452 merged, so the two are now one OnchainSpeed. Renamed rather than keeping either name: it serves both the whole-balance withdraw and a send to a chosen address, and the fee tiers are the same question either way.

"Send all funds" here overlaps #452's whole-balance withdraw in wallet settings — the same underlying operation reached two ways. I'd keep both. They serve different intents: emptying a wallet to close it out or move off the device is a settings action someone goes looking for deliberately, while sending everything to an address is just the far end of a normal send. Removing either one sends some users hunting through the wrong screen.

What's worth consolidating is the code behind them, not the entry points. The speed type is done; one shared drain path is still worth doing later.

---

Review changes (96eafea, 76e0b1d)

Both blocking defects from the review are fixed, and main is merged in.

Double-send. The quote is now consumed before the first suspension — SparkWallet is @MainActor, so guard-and-consume is atomic — and the send carries an idempotency key, which the SSP executes exactly once. pay() guards on entry too, since .disabled(inFlight) only applies after a re-render. Lightning survived the same shape only because the SDK rejects an already-paid invoice; an address plus an amount has no such protection, so both sends landed and the user paid twice.

BIP-21 with a Lightning invoice. Bolt11InvoiceDetails.invoice.bolt11 exists in 0.23.1 and was being dropped, so payInvoice received the raw URI and every such paste failed. The case now carries the invoice and the pay path uses it, with regression tests.

Also taken from the review: a 10-minute quote TTL so a stale fee can't execute; listOnchainDeposits keeping its last snapshot on a transient failure instead of blanking pending money out of the banner; the send section gated on supportsOnchainSend; the receive sheet's duplicate 15s poll dropped in favor of the store's 30s one; the claim alert no longer .destructive; the UInt64 conversion clamped; and awaitingConfirmations plus an orphaned doc comment removed.

Left as-is per the review note: the claim-error mapping and decline-reason order stay correct against the pinned 0.23.1, not SDK main.

[barrydeen]

Review — Request changes

I reviewed this against the full PR branch plus the pinned SDK (breez-sdk-spark-swift 0.23.1), not just the diff. One blocking funds-loss defect, one blocking functional defect. Details and patches below.

Blocking 1 (funds loss): double-tap sends twice on-chain

WalletView.swift:1063-1080 — the Pay button's only re-entrancy guard is .disabled(!canProceed || inFlight), which takes effect after a re-render. inFlight is set inside Task { await pay() }, so a second tap landing in the same frame (double-tap, tremor, Switch Control, ~16 ms window) spawns a second pay() that runs to completion — pay() (WalletView.swift:1273) never checks inFlight itself.

That alone wouldn't matter if the model consumed the quote — but SparkWallet.swift:358-397 holds preparedOnchainSend until after await sdk.sendPayment returns. Two concurrent executeSendOnchain calls both pass held.quote == quote (neither has cleared it yet) and both broadcast. And idempotencyKey: nil (SparkWallet.swift:380) discards the exact SDK primitive built for this ("providing the same idempotency key for multiple requests will ensure that only one payment is made").

Why this is on-chain-specific: the pre-existing Lightning path has the same UI shape, but a double-paid invoice fails closed (the SDK rejects an already-paid invoice — see friendlyPayError's "already paid" branch). An on-chain address+amount has no natural idempotency: both sends succeed and the user pays 2×. Patch (the model fix is the airtight one — SparkWallet is @MainActor, so guard+consume runs atomically before the first suspension; the view guard is belt-and-braces):

// SparkWallet.swift:271
private var preparedOnchainSend: (quote: OnchainSendQuote, prepared: PrepareSendPaymentResponse, idempotencyKey: String)?

// prepareSendOnchain, replacing the store on success:
let idempotencyKey = UUID().uuidString
preparedOnchainSend = (quote, prepared, idempotencyKey)
return .success(quote)

// executeSendOnchain, replacing the guard + send block:
guard let held = preparedOnchainSend, held.quote == quote else {
    return .failure(.other("This quote expired. Check the amount and try again."))
}
// Consume BEFORE the first suspension: a second concurrent call (second tap
// in the same frame) must find nothing to send. The idempotency key is the
// second layer — the SSP executes a key exactly once.
preparedOnchainSend = nil
// ... then sendPayment with:
        request: SendPaymentRequest(
            prepareResponse: held.prepared,
            options: .bitcoinAddress(confirmationSpeed: sdkSpeed),
            idempotencyKey: held.idempotencyKey
        )
// (delete the old `preparedOnchainSend = nil` after the response)

// WalletView.swift pay():
private func pay() async {
    // The button's `.disabled(inFlight)` only applies after a re-render.
    guard !inFlight else { return }
    inFlight = true; defer { inFlight = false }

No behavior regression: on throw, the held quote is now consumed instead of orphaned — but the view already clears onchainQuote on any failure, forcing a re-quote either way.

Blocking 2 (documented behavior doesn't work): BIP-21 + Lightning invoice is dropped

The PR body promises "A BIP-21 that also offers a Lightning invoice takes the invoice." It can't: SparkWallet.swift:591-594 returns .bolt11(amountSats:) carrying only the amount — the invoice string (inv.invoice.bolt11, which exists on Bolt11InvoiceDetails in 0.23.1) is discarded. pay() (WalletView.swift:1285-1286) then feeds the raw BIP-21 URI into payInvoice, which fails to decode. Every BIP-21-with-lightning paste errors. Fail-closed (no loss), but 100% broken. Patch — thread the invoice through (all other constructors pass nil, behavior unchanged):

// LnurlResolver.swift
case bolt11(amountSats: Int64?, invoice: String?)
// update the 5 `case .bolt11(let amt)` patterns (WalletView x3, LnurlResolver x2) to `case .bolt11(let amt, _)`

// SparkWallet.swift bip21 branch:
return .bolt11(amountSats: inv.amountMsat.map { Int64($0 / 1000) }, invoice: inv.invoice.bolt11)
// direct-bolt11 branch + WalletStore.swift NWC fallback: add `, invoice: nil`

// WalletView.swift pay():
case .bolt11:
    if case .bolt11(_, let inv?) = inputType {
        result = await store.payInvoice(inv)
    } else {
        result = await store.payInvoice(normalizeInvoice(input))
    }

Non-blocking (verified correct, plus minor hardening)

• Claim-error mapping correct. DepositClaimError.maxDepositClaimFeeExceeded(tx, vout, maxFee, requiredFeeSats, requiredFeeRateSatPerVbyte) — the 4th-position binding (SparkWallet.swift:440) is the right field, and passing it as MaxFee.fixed fails closed if fees rise further. Decline-reason order (maxBps, quotedBps, quotedSats, SparkWallet.swift:460) also matches 0.23.1. (Note: SDK main has since reshaped InstantClaimStatus — this code is correct against the pinned 0.23.1, don't "fix" it toward main.)
• Fee policy math correct per SDK docs: feesExcluded = recipient gets amount, feesIncluded = drain. deliveredSats, totalSats, dust guard all consistent.
• Address rotation safe: SDK documents "Previous ones remain valid."
• Manual-claim cap parity true: auto-claimer and nil-retry both use .networkRecommended(leewaySatPerVbyte: 5).
• NWC can't reach on-chain UI: classification requires the Spark SDK's parse, and the NWC fallback never produces .bitcoinAddress — so unused supportsOnchainSend/isOnchain is dead code, not a hole (execution also guards fail-closed). Suggest gating the section on it or deleting the helper.
• Minor hardening worth considering: listOnchainDeposits collapses load errors to empty (SparkWallet.swift:430), hiding pending deposits on transient failure — keep the previous value instead. No quote TTL — an hours-old quote executes at a stale fee (overpay or stuck tx); consider a 5–10 min expiry. awaitingConfirmations is test-only dead code. Alert "Claim" uses .destructive for a non-destructive action. The view's 15s poll duplicates the store's 30s poll. feeSats.map(UInt64.init) traps on negative (unreachable from SDK data, but a guard is free). Stray comment in OnchainReceiveTests.swift:89-92.

OnchainSendSpeed/WithdrawOnchainSpeed duplication is acknowledged in the PR body — fine, consolidate after #452 lands.

[dmnyc]

Both blocking defects confirmed and fixed in 96eafea. Thanks — the double-send in particular is a real funds-loss path and I'd missed that the guard only bites after a re-render.

Blocking 1 — double-send

Verified all three legs: pay() sets inFlight without checking it, executeSendOnchain cleared preparedOnchainSend only after await sdk.sendPayment returned, and idempotencyKey: nil discarded the primitive. The SDK docs are explicit — "providing the same idempotency key for multiple requests will ensure that only one payment is made."

Took the patch as written: quote consumed before the first suspension (@MainActor, so guard-and-consume is atomic), an idempotency key generated at quote time and passed at send time, and the entry guard in pay().

Your framing of why this is on-chain-specific is the part I should have reasoned about myself: Lightning fails closed because the SDK rejects an already-paid invoice, while an address plus an amount has no natural idempotency, so both sends land.

Blocking 2 — BIP-21 with a Lightning invoice

Confirmed: Bolt11InvoiceDetails.invoice.bolt11 exists in 0.23.1 and I was dropping it, so payInvoice got handed the raw URI. The case now carries the invoice through and the pay path prefers it. Added regression tests for the carried-invoice shape and for an amountless carried invoice still requiring an amount.

Non-blocking — taken

• Quote TTL: a quote older than 10 minutes is refused rather than executed at a stale fee.
• listOnchainDeposits on error: keeps the last known snapshot instead of returning empty. You're right that this mattered — it made pending money disappear from the banner at exactly the moment someone is watching for it.
• supportsOnchainSend: gated the send section on it rather than leaving it dead.
• Duplicate polling: the sheet now refreshes once on open and leaves the 30s cadence to the store.
• Claim alert: no longer .destructive.
• UInt64.init trap: clamped at the WalletStore call site, which is where the Int64 actually crosses over.
• awaitingConfirmations removed along with the orphaned doc comment in the tests.

Noted, not changed

Kept the claim-error mapping and decline-reason order as-is against the pinned 0.23.1, per your note that SDK main has since reshaped InstantClaimStatus — worth remembering when the pin moves.

OnchainSendSpeed / WithdrawOnchainSpeed still duplicated pending #452.

Builds clean, tests pass, running on the simulator.

[barrydeen]

Re-reviewed the post-review changes (96eafea, 76e0b1d). Both blocking defects are correctly fixed:

• Double-send: quote consumed before the first suspension (@MainActor-atomic), idempotency key generated at quote time and passed to sendPayment, pay() guards on entry, and the view clears the quote on failure so a spent quote can't be retried.
• BIP-21 invoice: the invoice is carried through and preferred in pay(); all .bolt11 patterns updated branch-wide, regression tests added.

All non-blocking items taken as noted (TTL, deposit snapshot retention, supportsOnchainSend gate, poll dedupe, .destructive removal, UInt64 clamp, dead-code removal). OnchainSpeed rename is complete with zero leftover references.

Merging. Follow-up needed: executeWithdrawOnchain (from #452) still has the same triple-defect shape this PR just fixed — preparedWithdrawal never consumed, no entry guard on send(), idempotencyKey: nil. Worth its own PR before anyone taps that dialog twice.