Skip to content

fix: point transfers to manual setup - #1127

Open
ovitrif wants to merge 17 commits into
masterfrom
fix/blocktank-sunset-sheet
Open

ovitrif wants to merge 17 commits into
masterfrom
fix/blocktank-sunset-sheet

Conversation

@ovitrif

@ovitrif ovitrif commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Twin: synonymdev/bitkit-android#1696

This PR replaces the "not available in your country" screens with the Blocktank-sunset design, which sends users to Manual Setup, and stops the transfer confirm from paying a Blocktank order now that Blocktank no longer sells channels.

Description

Blocktank stopped selling Lightning channels on 26 Oct. Its geocheck now answers 403 for everyone, so the app treats every user as geo-blocked.

  • Adds the "Transfer Unavailable" sheet: "MANUAL SETUP REQUIRED" with Learn More and Continue. Continue opens the existing Manual Setup flow unchanged, and Learn More opens the Bitkit blog in the browser.
  • Opens the sheet from the wallet screens' Transfer To Spending buttons while geo-blocked, so no Blocktank order is created:
    • Savings wallet: the Transfer To Spending button shows again and opens the sheet.
    • Spending wallet: the transfer button.
    • Hardware wallet: Transfer To Spending.
  • Stops Retry original on a pending Blocktank funding payment while geo-blocked: the sheet opens and nothing is broadcast.
  • Reworks "Fund your spending balance" while geo-blocked: the sheet's text replaces the "not available in your country" copy, Manual Setup moves to the top, Transfer from Savings, a new Transfer from HW wallet row and Use Other Wallet are disabled, and Learn More opens the Bitkit blog.
  • Shows the sheet's content on Receive › Spending › Receive Lightning Funds instead of the "not available in your country" screen.
  • Checks the geo state again when the confirm swipe or the hardware-wallet confirm runs, before an order is created or reused. A swipe no longer pays an order held from before the switch, and opens the sheet instead. Only an explicit "allowed" geocheck lets the payment through: a failed or unexpected check blocks it. A transfer whose funding was already broadcast still finishes. A signed hardware-wallet transaction whose broadcast failed is not retried while geo-blocked, since it would pay the old order.
  • Saves the transfer record of an order that was already paid even while geo-blocked, so recovering a transfer broadcast before the switch still finishes. Before, every geo-blocked user hit "Transfer unavailable" there.
  • Titles the Funding screen "Spending Balance", as in the design.
  • Repurposes Background Payments for Paykit notifications, since every push it relied on came from Blocktank: the settings screen, its intro, the Settings row and the Home card stay (the unused "Include amount" toggle goes), reworded for subscription reminders and payment requests. The prompt shows only to users with a Paykit subscription, and the preview shows the subscription reminder. Proposing or accepting a subscription, or accepting an allowance, asks for notification permission if it isn't granted. Neutral copy until the design copy lands.
  • Removes the Lightning ties of those notifications: the push device registration with Blocktank, its notification types and the handling of Blocktank's encrypted pushes.
  • Stops treating Blocktank's nodes as special. LDK no longer marks them as trusted (trustedPeers0conf, trustedPeersNoReserve), so the wallet broadcasts and fee-bumps its own force close with Blocktank. Before, LDK left that broadcast to Blocktank, whose node no longer acts, so a force-closed Blocktank channel kept its funds stuck in "claimable on channel close". The hardcoded Blocktank node list (Env.trustedLnPeers), the Blocktank info peer fetch and the trusted-peer section of the network-graph dump are gone.
  • Startup no longer connects to Blocktank nodes, and neither do channel-ready events or the LDK debug screen's restart. ldk-node reconnects the peers in its store and re-adds channel counterparties from the network graph. Instead, startup disconnects every stored peer with no channel and no claimable balance, which also drops it from the peer store. It skips the run while a closed channel's sweep is pending, since a sweep does not name its peer. The debug screen's "Peer Simulation" control is removed with it.
  • A Blocktank channel is now a normal channel in the close flows. Closing a connection in Settings offers Force Transfer when the cooperative close fails, and the savings transfer offers it once the cooperative retries give up, Blocktank channels included. The "cannot close" toast that sent the user back is gone, and so is the "channels skipped" note on the force-close toast (lightning__force_channels_skipped, removed in every locale). The RN migration's peer recovery no longer filters the Blocktank nodes out.
  • Reuses the sheet's translations from feat: stop offering new lightning channels #1102 and translates the new strings in every locale.

Learn More opens https://blog.bitkit.to/, where the announcement post appears once it is published.

Out of Scope

  • Views/Transfer, TransferViewModel, BlocktankService: deleting the purchase screens and the order and CJIT code.
  • Choose a Lightning provider screen and the channels-closing banner from feat: stop offering new lightning channels #1102, which this PR replaces.
  • Existing Blocktank channels: closing them.
  • Onboarding: the geo-blocked note "Bitkit does not currently provide Lightning services in your country" still shows on the first slide.
  • Funding option labels: the design writes "Manual setup" and "Use other wallet"; the existing "Manual Setup" and "Use Other Wallet" strings stay.

Design

Bitkit Wallet › Bitkit - Final Review v63 › Blocktank sunsetting:

Preview

Figma (left) next to the app (right):

Transfer Unavailable Fund your spending balance

Each entry point on a geo-blocked regtest wallet (2x):

Savings › Learn More › Continue Fund your spending balance Receive Lightning
savings-sheet.mp4
funding-screen.mp4
receive-screen.mp4

Notifications, repurposed for Paykit, on c124633:

Intro Settings

QA Notes

Setup

  • Debug build (regtest, staging LSP). Debug builds run the geocheck and Blocktank answers 403, so the app is geo-blocked with no extra setup.
  • Fresh wallet funded on-chain only: ./lsp POST /regtest/chain/deposit '{"address":"<savings address>","amountSat":100000}', then ./lsp POST /regtest/chain/mine '{"count":3}' from a bitkit-android checkout.

Journeys

  • J1: new transfer-unavailable-sheet.xml — the wallet screens' Transfer To Spending buttons open the sheet, Learn More opens bitkit.to, Continue opens Manual Setup, the Funding screen leads with Manual Setup with the other options disabled, and Receive's Lightning screen shows the sheet's content

Manual Tests

  • 1. Hold an order on the confirm screen, then turn geoblocking on and swipe → the sheet opens and nothing is paid — needs a debug hook to flip the geo state mid-flow; covered by unit tests

Automated Checks

  • added TransferViewModelTests.swift — a geo-blocked swipe pays no held order and creates no new one, and opens the sheet
  • added TransferServiceActivityTests.swift — a paid order still saves its transfer record while geo-blocked
  • added LightningServiceConfigTests.swift — the node config trusts no peer for zero-conf or force-close broadcasts; only stored peers with no channel and no claimable balance count as idle, and none while a sweep is pending. The removed startup connect is covered by deleting the function, not by a test.
  • updated ChannelPurchaseFlow.swift (integration) — connects to the LSP node from Blocktank info instead of the removed trusted-peer connect
  • ran the unit suite from unit-tests.yml at 7d93fb2 on an iPhone 17 simulator (iOS 26.5) — pass, except testCustomFeeWalletSwitchSurvivesFeeScreenNavigation, the known flake fix: prepare the wallet switch once on send review #1098 fixes
  • ran the transfer and send suites from unit-tests.yml on an iPhone 17 simulator (iOS 26.5) — pass, except testCustomFeeWalletSwitchSurvivesFeeScreenNavigation in SendConfirmationLifecycleTests.swift, a known flake that fix: prepare the wallet switch once on send review #1098 fixes

Side effects checked

  • Anchor reserve: perChannelReserveSats stays 1, so each open Blocktank channel now holds 1 sat of on-chain reserve. Spendable balance is effectively unchanged.
  • Force-close broadcast: anchor commitments usually need a CPFP through the anchor, which spends a confirmed on-chain UTXO. A wallet with no confirmed savings coin retries on each new block until it has one.
  • Zero-conf: trustedPeers0conf only gates accepting new unconfirmed inbound channels, so channels already open are unaffected.
  • Peer reconnects: checked at ldk-node v0.7.0-rc.77, which both apps pin. The node reconnects persisted peers, re-persists channel counterparties from the network graph after each RGS sync, and disconnect removes the peer from the store.
  • A Blocktank peer with a pending force close stays connected until its claim and sweep finish, which can take days for the timelock.

Evidence

J1 ran on 55b8a47, a Debug build on an iPhone 17 simulator (iOS 26.5) against the staging regtest LSP, with a fresh wallet funded with 100 000 sats on-chain. 89c527f only renames the Funding screen's title to "Spending Balance".

  • Savings › Transfer To Spending opened the sheet; Learn More opened bitkit.to in Safari, and Continue opened Manual Setup.
  • Lightning Connections › add connection showed Manual Setup first, Transfer from Savings, Transfer from HW wallet and Use Other Wallet disabled, and Learn More at the bottom. Transfer from Savings did nothing, Learn More opened bitkit.to, and Manual Setup opened.
  • Receive › Spending › Receive Lightning Funds showed the sheet's content titled "Transfer Unavailable" with a back button; Continue opened Manual Setup.
  • Spending › Transfer To Spending opened the sheet on e4b2d7f, before the button was renamed to Learn More.

Gaps

    1. (held order, then geo-blocked swipe) was not run on a device; the unit tests cover it.
  • Hardware wallet Transfer To Spending was not run: no Trezor emulator. It opens the same sheet as the other buttons.

Models used

  • Planning/scoping: claude-opus-5-5 (reasoning: Not exposed)
  • Implementation: claude-opus-5-5 (reasoning: Not exposed)
  • Review: claude-sonnet-5-5 (reasoning: Not exposed)

Metadata note: the session records name the models but do not expose their reasoning settings.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T13:43:29.098112Z 44ad53b Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@talosmachina talosmachina left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 findings. Replaces the geo-blocked transfer screens with the Transfer Unavailable sheet (Continue to Manual Setup), re-checks the geo state before a swipe or hardware confirm creates or reuses a Blocktank order, and keeps saving the transfer record of an order that was already paid. Reviewed 44ad53b, full tier, reasoned from the code: iOS does not build on this reviewer's Linux host, so nothing was compiled or run here.

Merge confidence: 4/5. Unit tests cover the swipe and the paid-record paths, but Run Tests, Run Integration Tests and build-local were still pending at review time, and the sheet flows rest on the author's recordings and J1.

What I checked, and 5 candidates I ruled out

Read in full: TransferViewModel.swift (order selection, hardware confirm, broadcast retry, failure handling), TransferService.swift, GeoService.swift, TransferUnavailableSheet.swift, SheetIntro.swift, FundingOptions.swift, the three wallet screens, ReceiveCjitGeoBlocked.swift.
Entry points traced: every navigation to .spendingIntro, .spendingAmount, .spendingIntroHw and .spendingAmountHw (Savings, Spending and Hardware wallet screens, FundingOptions, SpendingIntroView). Suggestions and LightningConnectionsView reach these only through .fundingOptions, which is gated. Both blocktank.createOrder callers (SpendingConfirm, SpendingHwSign) go through currentOrder, which now runs ensureTransferAvailable before reusing or creating an order.
createTransfer callers: fundPaidOrder and restoreAcceptedTransfer always pass a funding txid, so the narrowed geo guard lets recovery of a paid order through. The manual-channel and close paths pass no lspOrderId and never hit the guard.
Checks run here: node scripts/validate-translations.js (0 errors), git diff --check (clean). CI: SwiftFormat, validate and detect-changes green; tests and build pending.

Ruled out

  • Hardware retry stuck while geo-blocked: a pending signed broadcast now throws at ensureTransferAvailable and leaves the Sign screen's buttons disabled. This is the stated intent of 44ad53b, since the retry would pay an unpaid order. The only case that loses anything is an uncertain broadcast that had already reached the network before the switch, which needs a signing timeout to straddle the sunset.
  • Geocheck failure lets a swipe through: checkGeoStatus falls back to not blocked on a network error, so the swipe goes on to createOrder, which Blocktank refuses server-side. This is the existing fallback, not new behaviour.
  • E2E builds: GEO=false and no CHECK_GEOBLOCK leave isTransferUnavailable false. The test-only initializers default to { false }, so existing transfer tests are unaffected.
  • Sheet over an active sheet: every showSheet(.transferUnavailable) caller is a navigation screen, never inside another sheet. Receive renders TransferUnavailableContent inline.
  • Spending wallet button now shown while blocked: it opens the sheet, which is the intent.

Reviewer model: claude-opus-5-5
Reasoning effort: Unknown (the review run's accessible metadata does not record it)

@ovitrif ovitrif mentioned this pull request Oct 26, 2026
13 tasks
@ovitrif

ovitrif commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 89c527f, following Aldert's updated Blocktank-sunsetting frames in Figma (links in the description):

  • The sheet's left button reads Learn More.
  • Fund your spending balance leads with Manual Setup while geo-blocked. Transfer from Savings, a new Transfer from HW wallet row and Use Other Wallet are disabled, Learn More sits at the bottom, and the title reads Spending Balance.
  • Receive › Spending › Receive Lightning Funds shows the sheet's content instead of separate receive copy.

Design comparisons and new recordings are in the description.

@ovitrif

ovitrif commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 2061c33: Learn More, on the sheet and on the Funding screen, now opens https://blog.bitkit.to/, where the sunset article appears once it is published, so the link needs no further change. The recordings show the earlier bitkit.to target.

@ovitrif
ovitrif force-pushed the fix/blocktank-sunset-sheet branch from 9d5e276 to 1883357 Compare October 26, 2026 17:25
@ovitrif

ovitrif commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Rebased onto master after #1130 (head 1883357) and stopped registering for Blocktank push notifications. The Background Payments screens stay for now: they are the only place the app asks for notification permission, which Paykit reminders need.

@ovitrif

ovitrif commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 8a79eb0:

  • Background Payments is repurposed for Paykit notifications. The screens, intro, Settings row and Home card are reworded for subscription reminders and payment requests. The prompt shows only to users with a Paykit subscription, the preview shows the subscription reminder, and the unused Include amount toggle is gone.
  • Blocktank's notification types and encrypted-push handling are removed, along with the push registration.
  • Proposing or accepting a subscription, or accepting an allowance, asks for notification permission if it hasn't been answered.
  • The confirm geocheck now fails closed, so only an explicit allowed answer lets a held order be paid.

@ovitrif

ovitrif commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 7d93fb2: Blocktank's nodes are no longer special.

  • LDK no longer trusts them for zero-conf or to broadcast our force closes, so a force close with Blocktank now broadcasts and fee-bumps from this wallet. Before, LDK skipped that broadcast for trusted peers, and with Blocktank's node no longer acting, the funds stayed in "claimable on channel close".
  • Env.trustedLnPeers, the Blocktank peer fetch, the startup and channel-ready connects, the fallback and the debug Peer Simulation are removed.
  • Startup now disconnects stored peers with no channel and no claimable balance, and does nothing while a sweep is pending.
  • The close flows treat a Blocktank channel like any other, so Force Transfer is offered once the cooperative close fails.

The PR body lists the side effects checked.

@ovitrif

ovitrif commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator Author

QA:
Tested on an iPhone Air at c124633 (this PR's head at the time), on regtest against staging Blocktank in read-only mode, paired with a Pixel running synonymdev/bitkit-android#1696. The trusted-peer fix was verified with an equivalent local change before 7d93fb2 landed.

✅ Transfer Unavailable sheet

Savings › Transfer To Spending opens the sheet. Learn More opens blog.bitkit.to and Continue opens Manual Setup.

SheetLearn More

✅ Manual setup still works

A channel to the staging Blocktank node opens with no service fee, confirms, and routes Lightning payments both ways between the two phones.

Manual SetupAfter swipeHome

✅ Receive with a channel

Receive › Spending shows a normal invoice. The content for a wallet with no inbound capacity wasn't reachable with channels open, so it isn't covered.

✅ Notifications for Paykit

Settings › Notifications shows the Paykit copy with the "Open Bitkit to review payment" preview. On a fresh install, accepting a subscription asks for notification permission. With the app open, the subscription-due reminder shows the in-app sheet, plus a duplicate banner tracked in #1136.

✅ Allowance

A request from the Pixel within this phone's $5 cap is paid automatically. One over it (10,000 sats, about $8) asks for approval, then pays.

✅ Transfer to Savings with Blocktank

The cooperative close failed because LND's closing fee (141 sats) was below LDK's minimum (182), so it fell back to a force close. ldk-node never broadcast that close, because the config trusted Blocktank's node to broadcast it, and 87,020 sats stayed claimable. Without the trusted peers the close broadcast on first launch and 86,896 sats came back after the timelock. That fix is in this PR.

StuckRecovered

@piotr-iohk piotr-iohk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QA review

Scope: Reviewed the full PR diff against its merge base, at 7d93fb2f.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

The unavailable sheet, disabled funding rows, paid-order recovery, local force-close broadcast, and Paykit notification prompts match the change as written. Compared with bitkit-android#1696 at 9e10dc31, those flows agree. That twin's on-chain recovery still pays a created Blocktank order as well; this was a comparison of the shared flow, not a review of that PR.

Run Tests passed on this commit. The new swipe and paid-record tests do not cover a retained unpaid funding attempt. This review did not run tests or a device.

Device testing: not performed in this review.

Suggested additional test cases:

  • iOS. Setup: debug regtest build so the geocheck returns 403, with a node-wallet Blocktank funding attempt left pending, rejected, or unknown, and that order still created and unexpired. Action: open Send, or the pending payment, and confirm Retry original. Expected: no transaction is broadcast and the order stays unpaid.

Findings

  • [MEDIUM] Unpaid Blocktank funding can still be retried — inline at Bitkit/ViewModels/TransferViewModel.swift:421.

Reviewer model: grok-4.7
Reasoning effort: high

Comment thread Bitkit/ViewModels/TransferViewModel.swift
@ovitrif

ovitrif commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed 77b01a4: Retry original on a pending, rejected or unknown Blocktank funding attempt now stops at the geocheck. Unless it says allowed, the Transfer Unavailable sheet opens and nothing is broadcast. This answers the unpaid-order retry finding in #1127 (comment).

@ovitrif
ovitrif requested a review from piotr-iohk October 27, 2026 00:38
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.

3 participants