fix: add lsp refund address

[ovitrif]

Fixes #1229

Counterpart: synonymdev/bitkit-ios#732

This PR:

1. Supplies Blocktank orders with a reusable native SegWit refund address from the active Bitkit wallet, rotating it after local wallet activity records a payment to that address.
2. Persists the address and external receive index in metadata backups, then validates ownership and reveals the derivation through the active LDK node before reuse.
3. Keeps native SegWit monitored through address-type changes, startup, metadata restore, and post-restore pruning so Blocktank refunds remain detectable.

Description

The refund address is allocated once through LDK, stored only after node persistence succeeds, and reused across unpaid or failed order attempts. Estimates do not allocate addresses. Concurrent order requests share the same unused destination, while lookup, reveal, ownership, cache-write, and cancellation failures stop before Blocktank submission.

Preview

QA Notes

Manual Tests

• 1. Dedicated dev wallet → switch the primary receive type to Taproot → Settings → Advanced → Address Type → disable Native SegWit monitoring: the warning explains the Blocktank refund requirement and Native SegWit remains enabled.
• 2a. Standard wallet → Move to Spending → create and fund a standard LSP order: the order opened on staging with the persisted wallet-owned refund pointer.
  • Order ae58765a-f228-4452-ac42-b960b34c855a; pointer index 2.
• 2b. Standard wallet → Move to Spending → Advanced → create an unpaid LSP order: retries and ten live standard/advanced order requests reused pointer index 2 across an app restart.
  • Final advanced order faa81e71-a9ca-4de9-860a-fabde5b4ec26.
• 2c. Trezor emulator → fund 100,000 sat → Move 20,000 sat to Spending: the signed order opened on staging. The captured POST /channels used the internal Bitkit wallet pointer at index 1, distinct from the external Trezor address.
  • Order 55b1d296-82b9-4545-bf91-981a22d1f6b2; orderExpiresAt=2026-09-10T16:38:44.238Z.
• 3. Restore the dedicated wallet and metadata from VSS before the incoming payment → confirm post-restore pruning retains Native SegWit and the refund pointer → pay the restored address on regtest: Core records the exact address and txid in activity.db; the next order rotates the pointer exactly once.
  • Payment tx 68875bfdbf1822b13e2452b3d097ff51015fe50bb4d19794361449954e2d137d; rotated order 9c5a0405-a65b-4dba-974b-696ba82d0089.
• 4. Staging paid order → leave the channel unclaimed until expiry → verify the backend refund record targets the supplied address → approve the refund through an authorized backend path → verify Bitkit records the payout.
  • Staging orders expire after about 48 hours. The public staging API has no expire or refund-approval endpoint. Verified backend source registers the refund record at expiry and requires a separate authorized approval before broadcast, so this acceptance step needs backend access and coordination.

Automated Checks

• BlocktankRepoTest.kt: covers one persisted allocation, unpaid and failed reuse across restarts, payment-driven rotation, concurrency, allocation-free estimates, ownership validation, reveal, persistence, failure, and cancellation gates.
• AppCacheDataTest.kt and BackupRepoTest.kt: cover the optional cross-platform JSON shape and metadata backup/restore pointer.
• LightningRepoTest.kt, SettingsDataTest.kt, and AddressTypePreferenceViewModelTest.kt: cover native SegWit normalization, disable protection, post-restore retention, and localized feedback.
• Pre-review head: the full DevDebug unit suite passed 2,360 tests across 157 files with zero failures, errors, or skips; Kotlin compile, detekt, and the merged DevDebug E2E build also passed.
• Pre-review live request capture: four fee estimates allocated no refund pointer; the first order allocated one pointer and sent it in the single POST /channels request.
• Review and CI-fix validation were not rerun locally per the requested implementation-only constraint.

[jvsena42]

No findings. Clean at the HIGH/MEDIUM bar.

Reviewed at head f9b1f6b against full files, the ldk-node fork (crates/bdk-wallet-aggregate), and the iOS twin. No gating on the core path — BlocktankRepo.createOrder is reached from Transfer -> Spending in release builds, so I treated this as live user-facing code and kept upgrade/restore paths in scope.

Checked and clean:

• Derivation and ownership. newAddressInfoForType(P2WPKH) -> node.onchainPayment().newAddressInfoForType(NATIVE_SEGWIT) -> fork get_new_address_info_for_account(OnchainWalletAccount::account_zero(P2WPKH)) -> reveal_next_address(External) + wallet.persist(persister). So it's account-0 external P2WPKH of the node's own seed, and the reveal is persisted to the VSS-backed KV store before the address is returned. Not a watch-only or Trezor account — those go through account_index != 0.
• Reuse check. The re-derive at :370-372 uses peek_address on the same account-0 key and requires both index and address to match, so a foreign pointer can't pass. The subsequent re-reveal through the index means BDK includes the script in sync even if the cursor were somehow lower.
• The P2WPKH wallet is guaranteed loaded when these calls run: it's forced into settings before setAddressTypesToMonitor, set_primary_address_type demotes the old primary to monitored, validateDisableMonitoring rejects P2WPKH, syncMonitoredTypesFromNode re-adds it, and the post-restore prune skips it. createOrder also gates on nodeId != null before touching the mutex.
• Address-type preference after an order. The pointer is persisted as (address, index) and always re-derived against P2WPKH regardless of selectedAddressType; switching primary to Taproot keeps the P2WPKH account loaded, and rollback also passes through SettingsStore.update, which re-applies the invariant.
• Reuse and privacy. The receive flow and the refund allocation both advance the same persisted BDK external cursor, so the refund address is never handed out as a receive address. Paykit private reservations use the same cursor. Reuse across orders is by design and rotates on isAddressUsed.
• Network correctness. Flavors have distinct application ids (separate app_cache.json), vssStoreIdPrefix scopes backups per network, and the derive-and-compare would reject a tb1/bc1 mismatch anyway. Worth noting this is exactly why the cross-network cache issue I filed on the iOS twin can't happen here.
• Persistence ordering. getBlocktankRefundAddress() -> cacheStore.update (DataStore updateData completes the disk write before returning) -> only then coreService.blocktank.newOrder. A crash or cancellation before submission leaves a persisted unused pointer that's reused next time; a failed newOrder reuses it too, which the repeated-orders test pins. ensureActive() plus runSuspendCatching preserve cancellation, and concurrent calls serialise on refundAddressMutex. Estimates never allocate.
• Backup and restore. The metadata envelope embeds the whole cache, so adding blocktankRefundAddress to AppCacheData carries it in every metadata backup with no payload edit — which is why removing the duplicate collector was right. Restore's resetBip21() clears only bip21/bolt11/onchainAddress, so the pointer survives. Even without the pointer, the refund lands on a revealed, persisted account-0 P2WPKH index that a restored node syncs, with P2WPKH monitoring forced on at build — I found no un-sweepable path.
• Trust boundaries and key material. No Blocktank-supplied value is consumed by the new code; the address flows client -> server only. The order log line no longer dumps the full options (which included the signature). Only a public address and an index are persisted and backed up.

Cross-repo parity with synonymdev/bitkit-ios#732 — the backup coverage is present here, just differently shaped, which corrects my initial assumption that Android had none. iOS needed explicit BackupPayloads / SettingsBackupConfig / BackupService edits because its cache keys live in UserDefaults behind an allow-list; yours falls out of the whole-cache envelope. JSON shape is identical on both (blocktankRefundAddress: {address, index}), Android tolerates unknown keys and range-checks the index, and settings backups from either platform pass through withRequiredNativeSegwitMonitoring() — so restores decode in both directions. Ownership-mismatch handling matches iOS exactly.

One residual I couldn't turn into a finding, recorded in case it's useful: if the BDK reveal cursor were ever lost while the metadata pointer survived, newAddress() could hand out index N before the next order re-reveals it. The cursor lives in the VSS-backed node KV store and I couldn't construct a restore path where that happens, so there's nothing to fix — noting it only because it's the one shape that would break the no-reuse property.

[jvsena42]

Re-reviewed at head after the last push, funds-focused. Clean — no findings. Recording the trace since a refund address is a fund-misdirection surface and a negative result is worth having on file.

Fund misdirection. Allocation goes allocateBlocktankRefundAddress (BlocktankRepo.kt:382) → LightningService.newAddressInfoForType(P2WPKH) (:632) → fork get_new_address_info_for_type → OnchainWalletAccount::account_zero(P2WPKH) → reveal_next_address(External) with wallet.persist(persister) before returning. Always this seed, account 0, external chain — never a watch-only/HW account, since those need account_index != 0. Reuse is a pure peek_address and the check requires both index and address string to match (BlocktankRepo.kt:371-373), so a stale pointer from another seed cannot pass; blank or out-of-range pointers throw before any LSP call. WipeWalletUseCase calls cacheStore.reset() (:70), so a new wallet on the same install starts null. Nothing from the Blocktank response is consumed — the address flows client→server only, and estimateOrderFee never allocates.

Address-type coupling. The fork's set_primary_address_type demotes the old primary into runtime_config.monitored rather than unloading it, so P2WPKH survives a Taproot switch without a restart. Every settings write path re-applies withRequiredNativeSegwitMonitoring() (SettingsStore.update, restoreFromBackup, build(), updateAddressType, syncMonitoredTypesFromNode); prune skips DEFAULT_ADDRESS_TYPE_STRING (LightningRepo.kt:1059) and validateDisableMonitoring rejects P2WPKH first (:1017). Both callers of removeAddressTypeFromMonitor are gated. On gap limit: the reveal is persisted to the VSS-backed node KV store and getBlocktankRefundAddress re-reveals through the cached index before reuse (:375), so a restored node includes the script in sync. The Paykit reservation repo and the normal receive flow advance the same BDK cursor, so the refund index cannot later be handed out as a receive address.

Persistence ordering. cacheStore.update (DataStore updateData, disk write completes before return) runs before coreService.blocktank.newOrder. A crash or cancellation in between leaves a valid, unused pointer that is reused next time. The whole-cache metadata observer (BackupRepo.kt:281-291) projects only backupStatuses away, so the pointer change marks METADATA required, and restore's resetBip21() doesn't touch it.

Cancellation. createOrder is on runSuspendCatching, ensureActive() brackets the allocation, and refundAddressMutex serialises concurrent orders. Cancelling between the LDK reveal and the DataStore write can orphan one revealed index — harmless, since it stays monitored and is never issued.

One cross-repo note: the iOS twin (synonymdev/bitkit-ios#732) has a real bug in its legacy-key migration, where the network predicate can't separate testnet/signet/regtest. It does not apply here — each flavor is a separate application id with its own app_cache.json, and this diff has no legacy-key migration at all.

[jvsena42]

Journey coverage for the LSP refund address

Ran these against a wiped, freshly onboarded dev build of this branch (emulator, staging Blocktank). All three pass. Files are not committed — the diff is below if you want to pick them up.

Journey	Result
refund-address-allocated-and-reused.xml	PASS
refund-address-rotates-after-use.xml	PASS
native-segwit-monitoring-required.xml	PASS

Allocation and reuse — app_cache.json had no blocktankRefundAddress on the fresh wallet. First order (25% of a 500 000 sat Savings balance) cached {"address":"bcrt1q52072j0sqsdktrcy0ncpcusrs9hzs68f8cyva5","index":2}; a second order left it byte-identical.

Rotation — deposited 25 000 sat to that cached address and mined; Savings went 500 000 → 525 000. The next order rotated to bcrt1qpxz6lczjjsn4xvcsesucavcc2lnrdwwzywxzdg at index 3.

Monitoring guard — switched the primary type to Taproot first so the older "address type is currently selected" error couldn't mask the new one. Tapping the Native SegWit monitor toggle raised Native SegWit monitoring is required to receive Blocktank refunds., the toggle stayed on, and settings.json kept addressTypesToMonitor: ['nativeSegwit','taproot']. Worth noting the new check is ordered first, so it also wins over "has balance" — P2WPKH held 525 000 sat at the time.

Two things the journeys can't reach

1. The value Blocktank actually recorded is unverified. GET /channels/:id doesn't echo refundOnchainAddress, and this branch replaced the Buying channel with … line that used to print the full CreateOrderOptions with one that omits it. So the journeys assert the address the app stored and would send, not what the LSP received. Adding the refund address back to that log line would close this cheaply.
2. No real refund payout. Staging sets orderExpiresAt = createdAt + 48h with no endpoint to shorten or force an expiry, and channelExpiryWeeks bottoms out at 1 week (we hardcode 6), so payment.state2 can't be driven to refundAvailable/refunded in a session.

One behavioural note

Because the cached address is reused until it's seen used on-chain, concurrent pending orders all share a single refund address. If two of them refund before another order is created, both payouts land on the same address — rotation is triggered by the next createOrder, not by the refund itself. Fine if that's intended; flagging it since it's the one case where the reuse window is visible on-chain.

Diff — journeys/blocktank-refund-address/ (4 files)

diff --git a/journeys/blocktank-refund-address/README.md b/journeys/blocktank-refund-address/README.md
new file mode 100644
index 000000000..f75506cd9
--- /dev/null
+++ b/journeys/blocktank-refund-address/README.md
@@ -0,0 +1,52 @@
+# Blocktank refund address journeys
+
+These journeys exercise the automatic LSP refund address added on
+`fix/1229-blocktank-refund-address`. Every Blocktank channel order now carries a
+`refundOnchainAddress` taken from the wallet's own Native SegWit (P2WPKH) chain, and Native SegWit
+monitoring can no longer be switched off.
+
+## What the feature does
+- `BlocktankRepo.getBlocktankRefundAddress()` allocates a P2WPKH address on the **first** order and
+  caches it in `AppCacheData.blocktankRefundAddress` (`files/datastore/app_cache.json`).
+- Later orders **reuse** the cached address; it is only rotated once
+  `coreService.isAddressUsed(address)` reports it as used on-chain.
+- The cached entry is re-derived and compared before reuse, so a cache belonging to another wallet
+  is rejected instead of being sent to the LSP.
+- `nativeSegwit` is force-added to `addressTypesToMonitor` on node start, on every settings update,
+  and on backup restore, so a refund can always be seen.
+
+## Mandatory setup
+1. **Wipe and reinstall the dev build**, then onboard a fresh wallet — the allocation journey
+   asserts that `blocktankRefundAddress` is absent before the first order.
+2. **Fund Savings with at least ~150 000 sat.** The order is only created once the amount screen can
+   quote a valid channel; 25% of the balance times 3 must clear the LSP's `minChannelSizeSat`
+   (100 000 at time of writing). 500 000 sat via
+   `./lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":500000}'` plus
+   `./lsp POST /regtest/chain/mine '{"count":3}'` is comfortable.
+3. **The node must be connected to the LSP** before the amount screen can produce a max.
+
+## Gotchas
+- **The order is created on Continue, not on swipe.** `TransferViewModel.onSpendingAmountContinue`
+  calls `blocktankRepo.createOrder` before the confirm screen, so a journey can create several orders
+  by backing out of Confirm without ever paying one. Do not pay them.
+- **Reaching the amount screen costs three taps**: Spending tile -> "Transfer from Savings" ->
+  "Get Started" (the intro only appears the first time).
+- **A "Wallet Backup" sheet interrupts** the first walk back to home after the balance arrives.
+  Dismiss it with "Later".
+- **The address index is not 0.** Index 0 is consumed by the Savings receive address used to fund the
+  wallet, so the first refund address lands a couple of indexes higher — assert `index >= 0` and
+  compare indexes across orders, never a literal.
+- **Switch the primary type to Taproot before testing the monitoring guard.** Native SegWit is the
+  default primary type, so the older "address type is currently selected" error would mask the new
+  one. The new check is ordered first, so it also wins over "has balance".
+- **`android layout` does not surface the `MonitorToggle-*` test tags** on this screen; find the
+  toggles by their `checkable` interaction and row order (legacy, nested, native segwit, taproot).
+- **The toast is not in the layout tree** — capture a screenshot to read it.
+- **`android layout` occasionally emits more than one JSON document**; pipe through `head -1`.
+
+## What these journeys cannot cover
+The LSP does not echo `refundOnchainAddress` back from `GET /channels/:id`, and this branch's
+"Buying channel with" log line deliberately omits it, so the journeys assert the address the app
+*stored and would send*, not the value Blocktank recorded. A real refund payout is also out of reach:
+staging sets `orderExpiresAt` to `createdAt + 48h` and exposes no endpoint to shorten or force an
+expiry, so `payment.state2` cannot be driven to `refundAvailable`/`refunded` in a test session.
diff --git a/journeys/blocktank-refund-address/native-segwit-monitoring-required.xml b/journeys/blocktank-refund-address/native-segwit-monitoring-required.xml
new file mode 100644
index 000000000..6389e157c
--- /dev/null
+++ b/journeys/blocktank-refund-address/native-segwit-monitoring-required.xml
@@ -0,0 +1,35 @@
+<journey name="Native SegWit Monitoring Required For Refunds">
+  <description>
+    Covers the settings guard added by `fix/1229-blocktank-refund-address`: because refunds are always
+    paid to a P2WPKH address, Native SegWit monitoring can no longer be turned off in
+    Settings > Advanced > Address Types, even when another address type is the selected one.
+    Preconditions: onboarded dev wallet with the node connected.
+  </description>
+  <actions>
+    <action>
+      Launch the Bitkit app and open Settings > Advanced > Address Types (testTag "AddressTypePreference")
+    </action>
+    <action>
+      Verify that the Native SegWit monitoring toggle (testTag "MonitorToggle-nativeSegwit") is on
+    </action>
+    <action>
+      Select Taproot as the address type so that Native SegWit is no longer the currently selected type
+    </action>
+    <action>
+      Verify that the Native SegWit monitoring toggle is still on
+    </action>
+    <action>
+      Tap the Native SegWit monitoring toggle (testTag "MonitorToggle-nativeSegwit") to try to disable it
+    </action>
+    <action>
+      Verify that a toast reading "Native SegWit monitoring is required to receive Blocktank refunds." is shown
+    </action>
+    <action>
+      Verify that the Native SegWit monitoring toggle is still on
+    </action>
+    <action>
+      Run `adb shell "run-as to.bitkit.dev cat files/datastore/settings.json"` and verify that
+      `addressTypesToMonitor` still contains "nativeSegwit"
+    </action>
+  </actions>
+</journey>
diff --git a/journeys/blocktank-refund-address/refund-address-allocated-and-reused.xml b/journeys/blocktank-refund-address/refund-address-allocated-and-reused.xml
new file mode 100644
index 000000000..653e5476a
--- /dev/null
+++ b/journeys/blocktank-refund-address/refund-address-allocated-and-reused.xml
@@ -0,0 +1,53 @@
+<journey name="Blocktank Refund Address Allocated And Reused">
+  <description>
+    Covers `fix/1229-blocktank-refund-address`: every Blocktank channel order must carry a
+    `refundOnchainAddress` derived from the active wallet's Native SegWit (P2WPKH) chain, cached in
+    `AppCacheData.blocktankRefundAddress`, and reused for subsequent orders while the address stays
+    unused on-chain. Preconditions: freshly wiped dev build, onboarded wallet, node connected to the
+    LSP, and a funded Savings balance large enough to quote a Savings-to-Spending transfer.
+  </description>
+  <actions>
+    <action>
+      Run `adb shell "run-as to.bitkit.dev cat files/datastore/app_cache.json"` and verify that no
+      `blocktankRefundAddress` entry exists yet
+    </action>
+    <action>
+      Launch the Bitkit app and go to the wallet home screen
+    </action>
+    <action>
+      Open the Transfer to Spending flow and reach the spending amount screen (testTag "SpendingAmount")
+    </action>
+    <action>
+      Wait for the maximum amount to finish loading, then tap the "25%" quick button (testTag "SpendingAmountQuarter")
+    </action>
+    <action>
+      Tap "Continue" (testTag "SpendingAmountContinue") and wait for the Blocktank order to be created
+    </action>
+    <action>
+      Verify the spending confirm screen is shown with the order's fee breakdown
+    </action>
+    <action>
+      Run `adb shell "run-as to.bitkit.dev cat files/datastore/app_cache.json"` and verify that
+      `blocktankRefundAddress` now exists with a non-blank `address` starting with "bcrt1q" and an `index` of 0 or greater
+    </action>
+    <action>
+      Verify that logcat contains a "Buying channel with" line for the order that was just created
+    </action>
+    <action>
+      Navigate back out of the transfer flow to the wallet home screen without paying the order
+    </action>
+    <action>
+      Open the Transfer to Spending flow again and reach the spending amount screen (testTag "SpendingAmount")
+    </action>
+    <action>
+      Wait for the maximum amount to finish loading, then tap the "25%" quick button (testTag "SpendingAmountQuarter")
+    </action>
+    <action>
+      Tap "Continue" (testTag "SpendingAmountContinue") and wait for the second Blocktank order to be created
+    </action>
+    <action>
+      Run `adb shell "run-as to.bitkit.dev cat files/datastore/app_cache.json"` and verify that
+      `blocktankRefundAddress` still holds the exact same `address` and `index` as after the first order
+    </action>
+  </actions>
+</journey>
diff --git a/journeys/blocktank-refund-address/refund-address-rotates-after-use.xml b/journeys/blocktank-refund-address/refund-address-rotates-after-use.xml
new file mode 100644
index 000000000..cb900815e
--- /dev/null
+++ b/journeys/blocktank-refund-address/refund-address-rotates-after-use.xml
@@ -0,0 +1,37 @@
+<journey name="Blocktank Refund Address Rotates After Use">
+  <description>
+    Covers the rotation half of `fix/1229-blocktank-refund-address`: once the cached refund address
+    has been used on-chain, the next Blocktank order must allocate and cache a fresh P2WPKH address
+    at a higher derivation index instead of reusing the burned one. Preconditions: the
+    "Blocktank Refund Address Allocated And Reused" journey has already run, so
+    `blocktankRefundAddress` is cached and still unused.
+  </description>
+  <actions>
+    <action>
+      Run `adb shell "run-as to.bitkit.dev cat files/datastore/app_cache.json"` and record the cached
+      `blocktankRefundAddress` `address` and `index`
+    </action>
+    <action>
+      Run `./lsp POST /regtest/chain/deposit` with the recorded refund address and an amount of 25000 sat
+    </action>
+    <action>
+      Run `./lsp POST /regtest/chain/mine` with a count of 2 to confirm the deposit
+    </action>
+    <action>
+      Launch the Bitkit app, go to the wallet home screen, and wait for the Savings balance to reflect the deposit
+    </action>
+    <action>
+      Open the Transfer to Spending flow and reach the spending amount screen (testTag "SpendingAmount")
+    </action>
+    <action>
+      Wait for the maximum amount to finish loading, then tap the "25%" quick button (testTag "SpendingAmountQuarter")
+    </action>
+    <action>
+      Tap "Continue" (testTag "SpendingAmountContinue") and wait for the Blocktank order to be created
+    </action>
+    <action>
+      Run `adb shell "run-as to.bitkit.dev cat files/datastore/app_cache.json"` and verify that
+      `blocktankRefundAddress` now holds a different `address` than the recorded one, with an `index` greater than the recorded index
+    </action>
+  </actions>
+</journey>

The README records the setup traps: the order is created on Continue, not on swipe (so a journey can create several orders and back out without paying any), a Wallet Backup sheet interrupts the first walk back to home, index 0 is consumed by the funding address so never assert a literal index, and android layout doesn't surface the MonitorToggle-* test tags on that screen.

🤖 Generated with Claude Code

[jvsena42]

✅ tAck

• Address allocation
• Address rotation
• Order creation with address

Couldn't test:

• Order refund, because it takes some hours