Repository navigation
feat: blocktank sunset - #1127
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
9d5e276 to
1883357
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Turn off Blocktank channel purchases outright: every transfer-to-spending entry and the Lightning receive paths that bought liquidity open the Transfer Unavailable sheet, then Manual Setup, with no network call. Remove the geocheck, the purchase flow, order and CJIT creation, and their tests, strings and journeys. Keep paid-order recovery, the refused-funding release, reading existing orders, the trusted-peers removal and the idle-peer sweep.
This comment has been minimized.
This comment has been minimized.
Remove Blocktank gift-code claims: a scanned gift code now shows the Transfer Unavailable sheet. Drop the GEO workflow env, and restore the Fund Spending Balance layout with the purchase options shown disabled.
6104b95 to
23b8b66
Compare
@jvsena42 thanks. This PR has since been rewritten at 23b8b66. Blocktank's order, CJIT and gift endpoints return 404, so the app no longer has a geocheck: every former purchase path shows the Transfer Unavailable sheet, and Cancel is offered for any Blocktank funding that never reached the network. Item 2 (an allowed region showing no Cancel) therefore no longer applies. The description covers the new head, and device results on it will follow in the QA comment. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23b8b6638b
ℹ️ 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".
ben-kaufman
left a comment
There was a problem hiding this comment.
Reviewed 23b8b6638b77bde5ec1ba0d8942011929bbae3da. No new material correctness issues found.
E2E is still running. The linked device QA report covers 6104b956, so device verification of the current head remains unconfirmed. Independent security coverage is incomplete.
Reviewer model: gpt-6-astra
Reasoning effort: high
Drop what nothing reaches once Blocktank is read-only: paid-order watching and channel opening, order transfer records and their balance, the refund-address store, the transfer intro flags, and helpers the purchase flow left behind. An accepted Blocktank funding still resolves so later sends are not blocked. A scanned gift code gets the scanner's invalid-code error. Translate the transfer cancel note.
|
This PR was refactored into a proper Blocktank sunset. Blocktank no longer sells channels and its order, CJIT and gift endpoints return 404, so the app now turns channel purchases off outright: no geocheck call and no geo state remain, and every former purchase path leads to the sunset sheet and Manual Setup. The description is rewritten for the new head. Comments and reviews from before the refactor are hidden as outdated: they reviewed the geocheck-based version, which no longer exists. Now:
|
|
QA for 86d820a: 🟢 Refused funding: Retry original, Cancel and a later sendA Blocktank funding whose broadcast the backend refused (`min relay fee not met, 0 < 141`, txid `164ff408…`) leaves Savings at 300 000 with no outgoing or pending transaction. A new send opens Payment Pending for it; Retry original opens the Transfer Unavailable sheet and broadcasts nothing (`Stopped a retry of a transfer to spending`); Cancel shows the transfer note, checks the network and offers a 422-sat fee, then sends the coins back (`Released Blocktank funding 164ff408… through b0442225…`); after one block a new 10 000-sat send goes through.
🟢 Every former purchase path shows the sheetThe Home suggestion card, Savings › Transfer To Spending, Spending › Transfer To Spending and Receive › Spending › Receive Lightning Funds open Transfer Unavailable; Learn More opens blog.bitkit.to and Continue opens Manual Setup. Lightning Connections › + lists Manual Setup first and enabled, with the other three options disabled; tapping Transfer from Savings does nothing. The app log has no request to Blocktank's purchase API.
🟢 A scanned gift code gets the regular unsupported-code errorOpening `bitkit://gift-abc123-3000` shows the "Decoding Error" toast Bitkit gives for data it can't read, and no sheet opens.
Note Staging creates no Blocktank orders, so a local debug build sent a fabricated-order funding prepared at a zero fee rate; the backend refused its real broadcast. The unmodified 86d820a build was then installed over that wallet and ran every other step shown. |
jvsena42
left a comment
There was a problem hiding this comment.
Re-reviewed to 4634789 (the Blocktank sunset commits and serialized transfer creation). No findings.
4634789holds anNSLockaround a single insert, so there is no deadlock.- Dropping paid-order recovery keeps the sent activity, the order entry and refund detection.
- A scanned or deep-linked gift code shows the "Decoding Error" toast, with no crash and nothing sent. Manual entry enables Continue and then shows the same toast, which matches Android's manual entry.
- An order-backed transfer now settles straight away rather than waiting for the channel. No v3.0.1 wallet can still hold a pending Blocktank channel, so nobody sees a different balance.
The rewrite push dismissed my earlier approval at 6104b95.
Device gate (partial): 4634789, not run yet. Pending: transfer/transfer-unavailable-sheet.xml, the gift deep link, and a re-run of the refused-funding Cancel path on this head.
jvsena42
left a comment
There was a problem hiding this comment.
Approving. The re-review at 4634789 had no findings, and the device gate passed. One LOW inline on the journey text.
Device gate: 4634789, iPhone 17 simulator, regtest. Staging no longer creates Blocktank orders and this head creates none either, so I staged the refused funding on the earlier harness build and installed this head over it with data kept. That matches a user upgrading with a refused funding.
- A refused funding blocks a new send and opens Payment Pending. Retry original shows the fee-rate dialog, then the Transfer Unavailable sheet, and nothing is broadcast.
- Cancel shows the transfer note and the 703 sat fee, then logs
Released Blocktank funding d3a8b3e2…5490 through d95472f8…b804and the "Payment cancelled" toast. A new 20 000 sat send then went out. No PIN was set this run, so there was no PIN step. transfer/transfer-unavailable-sheet.xml: all 11 steps hold, and the sheet opens 0.9 s after the tap with no Blocktank request. Step 6's identifier is the inline LOW.- Gift code by deep link (
bitkit://gift-abc123-3000) and by scanner paste: the "Decoding Error" toast shows, with no sheet, no crash, no request and nothing sent.
Perf: not relevant.
The LOW is open: fix it here or in a follow-up issue, your choice; reply on the thread saying which.
|
Pushed 6966886: maps Android's |
jvsena42
left a comment
There was a problem hiding this comment.
Re-approving at 6966886. The only new commit adds the add_connection_button / NavigationAction row to the identifier table in journeys/README.md, which settles my LOW on step 6. It changes docs only, so the device gate at 4634789 still covers the app code.
Device gate: 6966886 (app code 4634789).
Twin: synonymdev/bitkit-android#1696
This PR turns Blocktank channel purchases off in the app. Blocktank no longer sells channels, and its order, CJIT and gift endpoints now return 404, so every path that used to buy a channel shows the Transfer Unavailable sheet and leads to Manual Setup.
Description
Out of Scope
Design
Bitkit Wallet › Bitkit - Final Review v63 › Blocktank sunsetting:
Preview
Figma (left) next to the app (right):
Recordings of every path on the current head follow in the QA comment.
QA Notes
Setup
./lsp POST /regtest/chain/deposit '{"address":"<savings address>","amountSat":100000}', then./lsp POST /regtest/chain/mine '{"count":3}'from a bitkit-android checkout.Journeys
transfer-unavailable-sheet.xml— the wallet screens' Transfer To Spending buttons open the sheet, Learn More opens https://blog.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 contentManual Tests
bitkit://gift-abc123-3000→ the scanner's regular unsupported-code error, no sheetAutomated Checks
ReceiveLiquidityDecisionTests.swift,ReceiveSheetSessionTests.swiftandTabBarReceiveTests.swift— a Spending receive above inbound capacity, or with none, leads to Transfer UnavailablePaykitPaymentProofServiceTests— a refused Blocktank funding is releasableTransferServiceActivityTestsandOnchainSendAttemptServiceTests— an accepted Blocktank funding resolves without a transfer record and no longer blocks sendsScannerManagerTestscase — a scanned gift code gets the regular invalid-code toast and no sheettestConcurrentTransferCreationKeepsEveryRecordinTransferServiceActivityTests— transfers created concurrently are all keptLightningServiceConfigTests.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 pendingTransferViewModelTestspurchase and limits cases,TransferViewModelHwTests,BlocktankViewModelCjitTests,SpendingConfirmTotalTests, the channel-purchase integration test, the refund-address tests, the order and CJIT cases inBlocktankTests, and the fee-increase toast and route-replace casesunit-tests.ymlselection at 86d820a on an iPhone simulator — pass, exceptPaykitPaymentRequestServiceTests.testCapabilityDiscoveryBoundsConcurrentReadsWithoutWaitingForSlowPeer, which times out the same way before this rewrite and drives only a mocked Paykit SDK;swiftformat --lintandvalidate-translations.jspassSide effects checked
perChannelReserveSatsstays 1, so each open Blocktank channel holds 1 sat of on-chain reserve. Spendable balance is effectively unchanged.disconnectremoves the peer from the store.Evidence
J1 and tests 2 and 4 ran on 86d820a, a Debug build on an iPhone 16 Pro simulator (iOS 18.5) against the staging regtest LSP, and pass: QA comment. The refused funding was staged with a local debug build; every other step ran on the unmodified build. 4634789 only serializes transfer creation again.
Gaps
Models used
claude-opus-5-5(reasoning:Not exposed)claude-opus-5-5(reasoning:Not exposed)claude-sonnet-5-5(reasoning:Not exposed)Metadata note: the session records name the models but do not expose their reasoning settings.