Skip to content

feat: pay contacts via spark - #1754

Open
ovitrif wants to merge 53 commits into
feat/breez-spark-lightningfrom
feat/paykit-spark-route
Open

ovitrif wants to merge 53 commits into
feat/breez-spark-lightningfrom
feat/paykit-spark-route

Conversation

@ovitrif

@ovitrif ovitrif commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1807

Twin: synonymdev/bitkit-ios#1191
Dependency:

This PR routes Paykit contact payments through bitkit-core's Paykit router, so a wallet whose spending balance sits on Spark pays contacts directly over Lightning, or through a Spark withdrawal to a contact who only accepts on-chain bitcoin.

Description

  • Every bitcoin contact payment asks bitkit-core's router (paykitRoutePayment) how to pay: on amount Continue, when a payment request opens its confirm screen directly, and when the source is switched on a request's confirm screen. The router gets the contact's full endpoint list and what the selected source can spend:
    • Spending: all endpoints, the Lightning spendable (the Spark balance counts), and a Spark withdrawal leg to each on-chain endpoint.
    • Savings: the on-chain endpoints and the on-chain spendable.
    • USDT, hardware wallets and LNURL-withdraw keep their own flow.
  • What the router returns:
    • Direct pays the endpoint it names: Lightning (bolt11 first, then LNURL) or the on-chain address.
    • Service opens the confirm screen with Route: Spark and the leg's fee in the fee row. The swipe withdraws exactly the amount to the contact's address, and the contact receives exactly the amount entered.
    • None shows "Insufficient balance" when the amount exceeds what the source can spend, else "Can't pay this contact"; the router's reason goes to the log.
  • Quotes the Spark withdrawal of exactly the amount at medium speed, with Spark's fee and its L1 broadcast fee on top, only when the Spark balance covers both (SparkRepo.withdrawalRouteLeg).
  • Pays an on-chain payment request from Spark: the withdrawal goes to the request's address, the request's proof carries the withdrawal's txid and is checked right before the withdrawal sends, and a proof whose completion was cut short settles from Spark's payment list on the next reconcile.
  • Sends a Spark withdrawal only at the debit the user confirmed: a quote that rose since then fails with a fee-changed error and nothing is sent.
  • Ties each withdrawal to its attempt: withdrawals Spark already listed to the same address never settle a new request, and a started request proof is released once Spark shows the withdrawal never sent, so the request can be paid again.
  • Routes again when the source is switched on the confirm screen, for contact payments as well as requests. A request whose Savings can't cover the amount opens from Spending, so the router can take the Spark leg.
  • Counts a Spark leg's debit and fees in the payment warnings, and keeps the tags picked on the confirm screen, also for a withdrawal recovered after an interrupted send.
  • Offers Spending as a source for an on-chain-only contact while Spark can pay, also when Savings holds some funds but not enough. Before, the Spark route was offered only with Savings at dust.
  • Records a Spark withdrawal to a contact as an on-chain Sent activity with the contact, the amount and Spark's fee, never as a transfer. The Spark screen lists only real transfers under Transfer to Savings. A journal written before the send settles a withdrawal the app was killed during, from Spark's payment list on the next start, and holds back another withdrawal until then.
  • Builds on bitkit-core 0.8.0-spark.3, a prerelease of feat: pay contacts via spark bitkit-core#193.
  • The route needs no build flag.

What it needs to ship

  • Spark leg on a device: the withdrawal fee fix (L1 broadcast fee in the quote) and the transfer-marking fix landed after case 11 ran, and are covered by unit tests. A kill after a withdrawal is dispatched, and a restore while one is in flight, are not run on a device yet.
  • Fees: the confirm screen shows the leg's fee under the Lightning "Instant" label, which reads as a Lightning fee.
  • Merge order: the dependencies above first; this branch stacks on them.

Out of Scope

  • Cancelling a sent one-time payment request: not supported on Android on either rail, tracked separately.
  • A Spark-native Payment Endpoint: the Payment Endpoint Identifier spec has no Spark rail (section 5.2).

Design

N/A — no design available.

Preview

A wallet whose spending balance is held on Spark pays a contact who accepts only on-chain bitcoin. The router picks a Spark withdrawal, the confirm screen shows Route: Spark, and the contact receives exactly 20,000 sats on Spark's regtest chain (2x speed).

android-spark-j12x.mp4

QA Notes

Setup

  • Two wallets: two dev debug builds (regtest, staging backend) on separate emulators, each with a new wallet, no channel, a Pubky profile created in the app, and the other saved as a contact (bitkit://contact?pubky=<pubky>).
  • Spark: dev builds run Spark on Lightspark's hosted regtest. Fund Spending by a Spark payment from another funded regtest Spark wallet to an amountless invoice from Receive › Spending, or from Lightspark's web regtest faucet to the deposit address on Settings › Advanced › Lightning Connections › Spark.
  • Two chains: dev builds run the node on staging's regtest chain and Spark on Lightspark's, so a Spark withdrawal to a contact's address (case 11, J1) lands on Lightspark's chain and never shows in the payee's Savings; check it on Lightspark's regtest mempool. Fund a payer's Savings with ./lsp POST /regtest/chain/deposit (cases 10, 11).
  • On-chain-only contact (J1): synonymdev/bitkit-docker main with the marketplace and payment-requests profiles, its fixture-issuer as the contact, and the app built with E2E=true E2E_BACKEND=local E2E_LOCAL_HOST=127.0.0.1 E2E_HOMESERVER_PUBKY=$(./pubky-marketplace info | jq -r .homeserver_z32) and adb reverse 6286, 6287, 6288, 15411, 15412, 60001, 23735, 23000 and 23010.

Journeys

  • J1: new paykit-spark-route.xml — a wallet whose spending balance is on Spark pays an on-chain-only contact through a Spark withdrawal, and the contact receives exactly 20,000 sats

Manual Tests

Two contacts as above, both Spark only (no channel); either one can play payer or payee.

  • 1. Pay the contact from Spending, 300 sats: the payee's private endpoint is paid from Spark; the payer's activity shows the contact, the fee and the preimage; the payee's receive is linked to the payer.
  • 2. Pay again after the payee relaunched: the payee's private endpoint was rotated and republished, and the payment uses the new list version.
  • 3. The payee is closed while it is paid: on reopen it shows the receive linked to the payer, with no duplicate.
  • 4. The payee requests 200 sats (Contact › Pay › Request): the payer pays it from Spark, and the payee's request shows "Proof submitted".
  • 5. The payer dismisses a request: both sides show it as Rejected, and nothing is paid.
  • 6. The payer offers the payee an allowance of $1 a payment: a 300-sat request is paid automatically from Spark and the allowance shows what was paid; a request over $1 stays manual; after the allowance ends, a request stays manual.
  • 7. The amount pad refuses more than Spark can send; the largest amount it accepts pays with room for the fee.
  • 8. Kill the payer right after the swipe: on relaunch nothing is paid twice, and an unsent request is still payable.
  • 9. The payee proposes a daily 100-sat subscription; the payer subscribes: the first period is paid from Spark, and after a relaunch the subscription is still active with its next renewal.
  • 10. The payer has Spark and Savings (fund Savings with ./lsp POST /regtest/chain/deposit): Spending is the default source; switching to Savings pays the contact's on-chain endpoint, which is the payee's Savings address, and the payee's receive is linked to the payer.
  • 11. The payee requests 2,000 sats accepting Savings only; the payer, with Savings and Spark, switches the request's source to Spending: the confirm screen shows Route: Spark with the withdrawal's fee, the withdrawal pays exactly the amount to the payee's address, and the payee's request shows "Proof submitted".
  • 12. The payee requests 200 sats accepting both: the payer's confirm screen opens directly from Spending and pays over Lightning.
  • 13. Turn the payer's network off on the confirm screen and swipe: the payment fails with the payment-failed screen before anything is sent, and nothing is paid after reconnecting.

Evidence

Each case ran with dev debug builds on two regtest Android emulators (API 35), with Spark on Lightspark's hosted regtest and no channel. Both wallets were Spark only, except in cases 10 and 11, where the payer's Savings was funded too.

  • 1 at 1cbb4b6c9: 300 sats paid; the payer showed Sent 301 (fee 1) to the contact with its preimage, and the payee showed Received 300 from the contact. Again through the router, Direct bolt11: 120 sats at bd755b839, and 110 sats at 25b40e0fd on bitkit-core 0.8.0-spark.3.

case 1
case 1 on spark.3

  • 2 at 1cbb4b6c9: after the payee's relaunch the payer opened list version 5 after 0 and paid 150 sats.
f11-rotate2x.mp4
  • 3 at 1cbb4b6c9: the force-stopped payee received 250 sats; on relaunch Home showed Received 250 from the contact and no duplicate rows.

case 3 payer
case 3 payee

  • 4 at 1cbb4b6c9: a 200-sat request was paid, and the payee showed "Proof submitted".

case 4 payee
case 4 payer

  • 5 at bd755b839: both sides showed Rejected.
f5-reject-payer2x.mp4
  • 6 at bd755b839: the 300-sat request was paid automatically ("Paid automatically $0.23"); a 2,000-sat request was kept manual ("the allowance limit is reached"); after End Allowance, a 200-sat request was presented for manual payment.

case 6 payer
case 6 payee

  • 7 at 1cbb4b6c9: with 4,473 sendable, 6,000 and 44,731 could not be entered; 4,473 was paid, quoted at ± ₿ 12.
f9-above2x.mp4
  • 8 at bd755b839: two kills before Spark dispatched left the balance unchanged and the request payable; a kill after dispatch could not be timed, since Spark sends finish in under a second.

case 8
case 8 relaunch

  • 9 at 25b40e0fd: the first 100-sat period was paid from Spark and its proof queued; after a force-stop and relaunch the payer listed the subscription as Active, renewing the next day. Later periods not observed yet.
  • 10 at 25b40e0fd: from Savings, the router paid the payee's on-chain endpoint 15,000 sats at Normal (₿ 141); the payee showed Received 15,000 from the contact in Savings.
f4-onchain2x.mp4
  • 11 before 47a1484a8's fee fix: switching to Spending showed Route: Spark; Spark's withdrawal 365dc1df… paid exactly 2,000 sats to the payee's address and confirmed on Lightspark's chain (the payee's node runs on staging's chain, so its Savings doesn't show it); the proof was delivered and the payee showed "Proof submitted"; the payer's activity showed the contact. The confirm screen quoted ± ₿ 750 while Spark charged 2,190, and the activity read as a transfer: both fixed in 47a1484a8.
f5-onchain-payer2x.mp4
  • 12 at 47a1484a8: the confirm screen opened from Spending at Instant (± ₿ 1); 200 sats sent and received.
f5-router-request22x.mp4
  • 13 at 47a1484a8: Spark refused the send before it left ("Spark could not prepare the payment"), the payment-failed screen showed, and after reconnecting no payment had been made.
f9-offline2x.mp4
  • J1 at 28a63f8 on bitkit-core 0.8.0-swaps.3, before the move to the router: Spark's withdrawal 56263be8… paid exactly 20,000 sats to the contact. Not rerun since.

Paykit flows with a Spark balance

Flows from the v0.1 specs at pubky/paykit-rs cb8cba5. State a is Spark only, with no channel. State b is a channel too small for the amount, with Spark covering the rest. The manual case numbers above give the evidence.

  • State b, every flow: ⏭️ not staged. With about 9,000 sats on Spark, any channel the wallet can open has more outbound than Spark, so the node pays, and one payment can't split across a channel and Spark. Unit tests cover the Spark choice when a channel can't send (LightningRepoTest, PaykitAllowanceRepoTest).
    1. Pay a contact's Lightning endpoint: ✅ private endpoint (cases 1, 2), also through the router on 0.8.0-spark.3; ⏭️ public endpoint, because both test wallets hold a private link, which is paid first.
    1. Amountless Lightning endpoint: ✅ — contact endpoints are amountless invoices (case 1).
    1. On-chain-only contact through a Spark withdrawal: ⏭️ — both test wallets publish Lightning, which the router picks first; J1 passed at 28a63f8 before the router.
    1. Contact with both endpoints: ✅ Lightning from Spending by default; switching to Savings pays on-chain (case 10).
    1. Payment requests as payer: ✅ pay over Lightning (cases 4, 12), pay on-chain from Spark (case 11), reject (case 5); ⏭️ cancel before payment (not supported on Android on either rail, tracked separately), expired or past the deadline (not staged).
    1. Recurring requests and subscriptions: ✅ the first period paid from Spark, and the subscription survives a relaunch (case 9); ⏭️ later periods.
    1. Allowances: ✅ auto-pay from Spark with the ledger updated, over the limit kept manual, ended allowance doesn't pay (case 6).
    1. Payment conversion: ⏭️ — fixed-rate fiat requests are rail-agnostic; per-period quotes are never issued by Bitkit.
    1. Balance edges: ✅ above Spark refused at input, the sendable maximum paid with fee room (case 7), Spark unreachable at send fails before anything is sent (case 13).
    1. Kill mid-payment: ✅ a kill before Spark dispatched pays nothing and the request stays payable (case 8); ⏭️ a kill after dispatch.
    1. Spark-backed Lightning endpoint: ✅ private endpoint, rotated and republished after relaunch, linked to the contact (cases 1, 2, 3); ⏭️ public endpoint.
    1. On-chain endpoint: ✅ it is the Savings address, as Bitkit intends (per-contact reserved addresses), never Spark's deposit address; the receive is linked to the contact (case 10).
    1. Request as payee: ✅ Lightning (case 4) and on-chain (case 11), proof submitted; the on-chain receive is shown by the transaction on Lightspark's chain, since the payee's node runs on another chain.
    1. Recurring payments received into Spark: ✅ the first period (case 9); ⏭️ later periods.
    1. Allowances as payee: ✅ (case 6).
    1. Cancel a sent request: ⏭️ not supported on Android on either rail, tracked separately.
    1. Lightning sharing toggle: ⏭️ — Android has only the whole-sharing toggle, no Lightning-only one.
    1. Payee closed while paid: ✅ (case 3).
    1. Spark-native endpoint: not in spec — the Payment Endpoint Identifier spec has no Spark rail (section 5.2; extensions in section 8).

Automated Checks

  • added AppViewModelSendFlowTest cases: router Direct Lightning, Direct on-chain from Savings, Service leg, None with a short balance and None with an unpayable contact; the Spark leg runs only from Spending; a request's confirm screen pays the endpoint the router picks; an on-chain request paid through the Spark leg starts and completes its proof, and a failed leg releases an unsent proof; a source switch on confirmation reroutes; Spark leg fees count in the warnings; the confirm tags reach the withdrawal; an on-chain request Savings can't cover goes to Spark
  • added a PaykitPaymentStateBackupTest case: a Spark withdrawal proof keeps its rail through backup
  • added PaykitPaymentProofRepoTest cases: an on-chain proof completed with a Spark withdrawal's txid, one settled from Spark's payment list on reconcile, and one released when its withdrawal never sent
  • added SparkRepoTest cases: a withdrawal quoted as a route leg, no leg the balance cannot cover, a withdrawal recorded with its contact and fee, a second withdrawal refused while one is unsettled, a journaled withdrawal settled or dropped on start, a withdrawal refused right before it sends or above the confirmed debit, the journal kept when the activity fails to save, an earlier withdrawal never settling a new attempt, tags on a recovered withdrawal, a contact withdrawal not marked as a transfer, and kept off the Spark screen's transfers
  • ran just compile, just test, just lint and the androidTest compile on bitkit-core 0.8.0-spark.3: pass

Mainnet tests (run by the author)

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

Round 1

None yet: the flows above ran on regtest.

Round 2 (pending)

On the final heads, two mainnet wallets that are each other's Paykit contacts, small amounts.

  • Spark only, no channel: pay a contact's Lightning endpoint, both ways; Activity shows the contact and the fee.
  • A channel too small for the amount: the payment goes from Spark.
  • A contact with only an on-chain endpoint: paid through a Spark withdrawal, the fee includes the on-chain fee, and Activity shows the contact.
  • A payment request paid from Spark, over Lightning and on-chain: the payee's request shows paid and the proof matches.
  • An allowance: a request within it auto-pays from Spark, and the ledger shows it.
  • Receive: a contact pays our Spark-backed endpoint while the app is closed; on reopen it's linked to the contact and the endpoint has rotated.
  • Kill the payer mid-payment: on relaunch it settles once, with no double pay.
  • An amount above Spark plus the channel: refused with the app's copy, and nothing is spent.

Models used

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

Taken from the run logs of every session and subagent that planned or wrote this PR, which record the model and reasoning effort per turn. The claude-sonnet-5-5 planning runs were research and spec-mapping subagents. No independent agent review was run on this PR.

@ovitrif

ovitrif commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 036346a: adds the paykit-spark-route.xml journey (J1). Ran it on a regtest Android emulator against Lightspark's hosted regtest. The router picked a Spark withdrawal, and 56263be8… pays exactly 20,000 sats to the contact. The recording, setup and evidence are in the description.

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Regtest APK

Built from 31298c4 (run).

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

@ovitrif ovitrif added this to the 3.2.0 milestone Nov 3, 2026
@ovitrif

ovitrif commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

Added to the 3.2.0 milestone: Spark via Breez SDK is the 3.2.0 highlight, and this PR stacks on #1697.

@ovitrif ovitrif changed the title feat: pay on-chain contacts from spark feat: pay contacts via spark Nov 3, 2026
@ovitrif
ovitrif force-pushed the feat/breez-spark-lightning branch from ab34526 to ab1b09d Compare November 5, 2026 02:00
Rebuilt on the split Spark head; carries the Pubky Swaps route (#1732) as it was merged into this branch.
@ovitrif
ovitrif force-pushed the feat/paykit-spark-route branch from c08023a to f00fb41 Compare November 5, 2026 02:10
@ovitrif

ovitrif commented Nov 5, 2026

Copy link
Copy Markdown
Collaborator Author

Force-pushed: rebuilt on #1697's new head as one commit, f00fb41, with the same route changes. The previous head, c08023a, is kept on backup/paykit-spark-route-20261105.

@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 a7bef98e9..b22c2496c. My allowance-ledger MEDIUM is fixed in 590f925, and I confirmed it on the thread. b22c249 is clean.

One new MEDIUM, inline: the Android side is missing the fix iOS #1191 made in 2dcd5389c. It is reproduced by a unit test that fails at this head.

The base commits 09eac5b and 9f44c5b are reviewed on #1697.

Reviewer model: Claude Fable 5.1 (claude-fable-5-1), delta review; Claude Opus 5.5 (claude-opus-5-5), verification
Reasoning effort: not exposed

@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 b22c249. Independent static General review identified 3 confirmed findings. Recommended outcome: Request changes.

High — Discard routing results when the active payment changes. app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:4693-4725. Continue a Spark-funded payment to contact A while prepareSend is delayed, dismiss the sheet, and open contact B before A's quote returns. routePaykitPayment retains A's endpoints across the suspension and then copies A's route/service leg into the current SendUiState, which now belongs to B. PaykitRouteRepo.route suspends while quoting Spark endpoints. ContentView.kt:566-603 permits dismissal during ordinary route loading; hideSheet increments sendStateGeneration and clears the contact context without cancelling observeSendEvents. openContactPayment runs through launchScan independently of the send-event collector and can initialize B while A's routing call remains suspended. The routing result checks neither sendStateGeneration nor the original contact/sheet before updating the current state. Later routing uses the overwritten A endpoints; proceedWithServiceRoutedPayment sends to leg.endpoint.value while using B's current contact context. SendConfirmScreen displays the current contact profile as TO. A stale asynchronous quote can bind A's destination to B's displayed contact and send B's payment to A. Capture and revalidate payment generation/context before applying the result or emitting confirmation navigation. Counterexamples checked: Normal completion without dismissal is safe. The serialized send-event collector does not prevent sheet dismissal or a separate launchScan from changing the active payment.

Medium — Connect uncertain contact withdrawals to their eventual Spark outcome. app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:6962-6968. An ordinary contact payment without an incoming payment request reaches Spark, but send throws after dispatch. Spark either already lists the withdrawal or lists it on a subsequent event. handleServiceLegFailure treats both Sent and Unknown as pending and calls showUnresolvedOnchainSend with a null txid and null request. showUnresolvedOnchainSend creates an on-chain pending route with an empty payment identifier and observeResolution=false. SendPendingViewModel.initOnchain cannot observe the Spark journal: it only tracks an LDK OnchainSendAttempt, and returns without finding activity when txid is null. SparkRepo.recordWithdrawal can save the successful activity and clear the journal, but emits no pending-screen resolution. The Paykit proof resolution observer cannot help an ordinary contact payment because startSparkOnchainProof creates no request proof for it. The screen remains Payment Pending indefinitely even after Spark knows the payment succeeded, with no transaction details target. Handle Sent immediately and provide an outcome observer for ordinary uncertain Spark withdrawals. Counterexamples checked: Incoming requests can resolve through their payment proof, and sends returning normally use onSendSuccess; neither safeguard covers this ordinary-contact failure path.

Medium — Reconcile expired withdrawal journals on subsequent successful syncs. app/src/main/java/to/bitkit/repositories/SparkRepo.kt:911-914. A withdrawal send fails without creating a payment. The immediate successful sync runs during the ten-minute grace period and correctly retains its journal. The user then keeps the app in the foreground past that period. syncActivities at lines 895-905 is the only path that drops an unlisted journal after its grace period. Its callers are start and the one immediate sync following a failed send. Subsequent SdkEvent.Synced handling only calls refresh; refresh and refreshState neither settle nor expire the journal. withdraw rejects every new withdrawal while pendingSparkWithdrawal is non-null. withdrawalOutcome continues reporting Unknown while the journal remains. An attempt that never sent blocks all later Paykit Spark withdrawals beyond the advertised grace period until another Spark start occurs, such as an app restart or lifecycle restart. Settle the journal after successful SDK sync events as well. Counterexamples checked: The added grace-period test explicitly calls start after advancing the journal age, so it verifies restart recovery but does not exercise continuous foreground operation.

Security review and runtime validation remain incomplete and held for fixes. No builds, tests, devices, services or installations ran. Native bitkit-core/Breez SDK internals and immutable counterpart compatibility were not independently verified; prior Security reuse was not established.

Review details

General review
Model: GPT-6.1 Sol
Reasoning effort: high

@ovitrif
ovitrif requested a review from jvsena42 November 9, 2026 15:53

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

Medium — Unselected LNURL constraints block on-chain Spark withdrawals. In app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt:4744-4748, selecting an on-chain Spark service leg leaves LNURL metadata attached. Amount validation still applies that unselected endpoint's minimum, and execution may fetch an LNURL invoice before the service withdrawal. A fundable on-chain payment can therefore fail because of another endpoint's limits or callback availability. Apply amount and invoice checks only to the selected route.

High candidate — Positive withdrawal evidence can be lost from recovery classification. In app/src/main/java/to/bitkit/repositories/SparkRepo.kt:1010-1016, a pending/successful SDK event saves activity and clears the withdrawal journal. If the send response is then lost and the payment list temporarily lacks the withdrawal, outcome classification can return Unsent without consulting the saved activity, releasing the request proof for retry. This could permit a duplicate payment. The triggering SDK event-versus-list consistency contract was not inspected or reproduced, so this remains a candidate. Preserve durable positive evidence until recovery can classify the payment safely.

Review details

General review
Model: GPT-6.1 Sol
Reasoning effort: high

@jvsena42

jvsena42 commented Nov 9, 2026

Copy link
Copy Markdown
Member

@ovitrif this conflicts with feat/breez-spark-lightning; please rebase. I'll review it again once it merges cleanly.

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

Rechecked commit b905364. A Paykit route that returns after its payment was dismissed or replaced is now dropped, an uncertain Spark send to a contact now settles from Spark's withdrawal journal, and an expired journal is settled on every Spark sync. These address the earlier High and two Mediums.

Medium: a late Spark withdrawal settlement can take over an unrelated Send sheet (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt line 1183). The uncertain withdrawal recorded for the pending screen is not cleared when the sheet is closed; hideSheet clears the uncertain request ID but not this. If the user dismisses the pending screen and starts a different payment, the settlement from Spark's next sync only checks that some Send sheet is open, then shows the earlier withdrawal's success screen (clearing the current flow) or its failure, although the new payment was never sent. Clearing it in clearActiveContactPaymentContext, or applying the settlement only while that pending screen is shown, would fix it.

Review details

General review
Model: Claude Opus 5.5
Reasoning effort: high

@ovitrif

ovitrif commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@jvsena42 synced with feat/breez-spark-lightning in b905364 (base at 767f0eb); this PR merges cleanly now.

@ovitrif

ovitrif commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@pwltr re #1754 (review):

  • High, stale routing result: fixed in d79cced. routePaykitPayment captures the send state's generation and the active contact before the router call and drops the result when either changed. Test: a route that returns after its payment was dismissed never lands on the next payment.
  • Medium, uncertain withdrawal to an ordinary contact: fixed in 4439a24. A withdrawal Spark already lists opens success. Otherwise the pending screen follows the journal: it moves to success with the txid once Spark lists the withdrawal, or to the failure screen with Try Again once the journal closes as unsent. Tests: an uncertain spark send to a contact that spark already lists opens success, an uncertain spark send to a contact settles from the spark journal, a journal that closes tells the pending send how its withdrawal ended.
  • Medium, expired journal in a long foreground session: fixed in 5584e40. Every SdkEvent.Synced now settles an open journal. Test: a sync in a long foreground session closes an expired unlisted withdrawal.

@ovitrif

ovitrif commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@pwltr re #1754 (review):

  • Medium, unselected LNURL constraints: fixed in 0c74a5b. Continue now asks the router before it applies the LNURL minimum, so a Spark leg to the on-chain endpoint skips it. Confirm fetches no LNURL invoice for a leg, and request terms skip the LNURL minimum once a leg is routed. Test: a spark leg to the onchain endpoint ignores the unselected lnurl's limits.
  • High candidate, positive withdrawal evidence lost: fixed in 4bd0732. A pending or succeeded SDK event still saves the activity but no longer closes the journal. Only Spark's payment list or the send response closes it, so while the list lags the event, the outcome reads Unknown instead of Unsent. Test: a payment event keeps the journal until spark lists the withdrawal.

@ovitrif
ovitrif requested a review from pwltr November 9, 2026 17:25
@ovitrif

ovitrif commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@jvsena42 feat/breez-spark-lightning moved again, so I merged it once more at 7b7bdc0 in 370653f. This PR still merges cleanly.

@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 seven commits since b22c2496c. The head has since only merged the base branch; AppViewModel.kt is unchanged at 370653f.

  • My MEDIUM is fixed by 20ac89e. A withdrawal that Spark returns as FAILED now throws WithdrawalNotSent. The proof is released, the allowance attempt fails, and nothing is reported as sent. A FAILED result that only shows up later through the journal or sync path reads as Unsent once the journal closes. The failed txid also can't settle a retry.
  • One LOW is inline (4439a24), reproduced with a unit test.
  • Checked and clean:
    • 03f22e6: the pending screen moves to error only for its own request.
    • d79cced: a stale route is dropped before it is applied.
    • 5584e40: expiry is settled on each sync.
    • 0c74a5b: an LNURL's limits stay off a Spark leg.
    • 4bd0732: the journal is kept until Spark lists the send, so a list that lags behind never reads as Unsent.

The contact-payment device gate passed earlier (gate_spark_1754).

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

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

Rechecked commit 370653f, a merge of the updated base branch. Its conflict in Spark's activity sync is resolved correctly: the base's re-read before releasing failed sends runs first, and the journal settlement still runs after it.

Medium (still open from b905364): a late Spark withdrawal settlement can take over an unrelated Send sheet (app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt line 1183). The uncertain withdrawal recorded for the pending screen is still not cleared when the sheet closes, so after starting a different payment, the settlement replaces that payment's flow with the earlier withdrawal's success or failure. Clearing it in clearActiveContactPaymentContext, or applying the settlement only while that pending screen is shown, would fix it.

Review details

General review
Model: Claude Opus 5.5
Reasoning effort: high

@ovitrif

ovitrif commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

@pwltr re #1754 (review) (also #1754 (review)): fixed in 55e4991. Closing the send sheet now clears the uncertain withdrawal the pending screen waits on, so a late settlement no longer takes over a Send sheet opened afterwards. Test: a late spark settlement leaves a send sheet opened after its pending one closed.

@jvsena42 your LOW at #1754 (comment) is fixed in 31298c4; reply in the thread.

This push makes the approval stale. A re-approval is needed only for these two fixes (55e4991, 31298c4) plus the earlier base merge at 665641e (cac9d12).

@ovitrif
ovitrif requested review from jvsena42 and pwltr November 9, 2026 18:53

@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 55e4991 and 31298c4: no findings.

  • 31298c4 closes my LOW; thread resolved.
  • 55e4991 clears uncertainSparkWithdrawal in clearActiveContactPaymentContext, so hideSheet drops a late settlement before a new Send sheet can take it. That closes pwltr's thread. None of the clearing paths runs while the pending screen is still shown.

Reviewer model: Claude Fable 5.1 (claude-fable-5-1), 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.

5 participants