From c37ce5bd441aaadf61e3c46b733ff73ac81861a5 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 16 Sep 2026 09:32:17 -0300 Subject: [PATCH 1/6] refactor: make TransferStorage's UserDefaults injectable The initializer took a `suiteName` but was `private`, so `shared` was the only instance that could ever exist and the parameter was unreachable. That left `TransferService(storage:)` with no usable seam. Replace it with the injected-defaults initializer used elsewhere in the app (QuickPaySpendStore, BlocktankRefundAddressProvider, WatchOnlyAccountService). `shared` still binds to `.standard`, so no shipped behaviour changes. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AcCTBgiMafXxw71MWBGB2T --- Bitkit/Services/TransferStorage.swift | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/Bitkit/Services/TransferStorage.swift b/Bitkit/Services/TransferStorage.swift index 2d32493c1..dc73fc283 100644 --- a/Bitkit/Services/TransferStorage.swift +++ b/Bitkit/Services/TransferStorage.swift @@ -14,12 +14,8 @@ class TransferStorage { transfersChangedSubject.eraseToAnyPublisher() } - private init(suiteName: String? = nil) { - if let suiteName { - defaults = UserDefaults(suiteName: suiteName) ?? .standard - } else { - defaults = .standard - } + init(defaults: UserDefaults = .standard) { + self.defaults = defaults } /// Insert a new transfer From 5757e0d65572abe431862cacd6b1e22ee28a4568 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 16 Sep 2026 09:36:03 -0300 Subject: [PATCH 2/6] refactor: let TransferViewModel scope transfer persistence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both convenience initializers hardcoded `TransferService(...)` with `storage:` omitted, so every caller — tests included — wrote through `TransferStorage.shared` to `UserDefaults.standard`. Take `transferDefaults` and forward it into `TransferService(storage:)`. It is a `UserDefaults` rather than a `TransferStorage` for the same reason the hardware initializer builds the service internally: `TransferStorage.swift` is compiled into BitkitTests as well, so passing the type across the module boundary would need `Bitkit.`-qualification at every call site. Defaults to `.standard`, so production call sites are unchanged. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AcCTBgiMafXxw71MWBGB2T --- Bitkit/ViewModels/TransferViewModel.swift | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/Bitkit/ViewModels/TransferViewModel.swift b/Bitkit/ViewModels/TransferViewModel.swift index c18497cf0..f8e1bf2fb 100644 --- a/Bitkit/ViewModels/TransferViewModel.swift +++ b/Bitkit/ViewModels/TransferViewModel.swift @@ -213,14 +213,17 @@ class TransferViewModel: ObservableObject { } } - /// Convenience initializer for testing and previews + /// Convenience initializer for testing and previews. `transferDefaults` scopes transfer + /// persistence: tests pass an isolated suite so mock transfers never reach the app's own store. convenience init( coreService: CoreService = .shared, lightningService: LightningService = .shared, currencyService: CurrencyService = .shared, - sheetViewModel: SheetViewModel = SheetViewModel() + sheetViewModel: SheetViewModel = SheetViewModel(), + transferDefaults: UserDefaults = .standard ) { let transferService = TransferService( + storage: TransferStorage(defaults: transferDefaults), lightningService: lightningService, blocktankService: coreService.blocktank ) @@ -234,7 +237,9 @@ class TransferViewModel: ObservableObject { } /// Convenience initializer for hardware-wallet transfer tests. Builds the `TransferService` - /// inside the app module so callers don't construct cross-module service types. + /// inside the app module so callers don't construct cross-module service types — `transferDefaults` + /// is a `UserDefaults` for the same reason, since `TransferStorage` is compiled into both modules. + /// Tests pass an isolated suite so mock transfers never reach the app's own store. convenience init( hwFunding: HwTransferFunding?, hwConnecting: HwTransferConnecting?, @@ -243,9 +248,11 @@ class TransferViewModel: ObservableObject { hwTimeouts: (reconnect: Double, compose: Double, sign: Double, broadcast: Double) = (reconnect: 30, compose: 45, sign: 120, broadcast: 120), coreService: CoreService = .shared, lightningService: LightningService = .shared, - sheetViewModel: SheetViewModel = SheetViewModel() + sheetViewModel: SheetViewModel = SheetViewModel(), + transferDefaults: UserDefaults = .standard ) { let transferService = TransferService( + storage: TransferStorage(defaults: transferDefaults), lightningService: lightningService, blocktankService: coreService.blocktank ) From 7db406414ece6006ac40b6025bac2a2fb4436337 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 16 Sep 2026 10:03:35 -0300 Subject: [PATCH 3/6] test: add shared app-state isolation helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BitkitTests is hosted in the Bitkit app, so `UserDefaults.standard` is the app's own preferences and anything a test writes there lands in the developer's wallet. The repo already solves this three different ways in a dozen files; collect the best of each into one place: - `makeIsolatedDefaults()` — a per-test UUID suite, emptied before and removed after, from the pattern in PublicPaykitServiceTests. - `snapshotAppDefaults(_:)` — restore-or-remove, handling the absent case, from PubkyAuthURLSchemeTests (the only correct version in the target). - `guardAppDefaults(_:)` — fails a test that leaves a key changed, so a regression of this kind goes red instead of corrupting the simulator. The guard reports only the key name. These keys hold large encoded blobs and asserting on equality dumps both of them as hex, burying the one line that says what to do about it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AcCTBgiMafXxw71MWBGB2T --- BitkitTests/AppStateIsolation.swift | 64 +++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 BitkitTests/AppStateIsolation.swift diff --git a/BitkitTests/AppStateIsolation.swift b/BitkitTests/AppStateIsolation.swift new file mode 100644 index 000000000..2033f643b --- /dev/null +++ b/BitkitTests/AppStateIsolation.swift @@ -0,0 +1,64 @@ +import Foundation +import XCTest + +/// Helpers that keep a test suite off the host app's own state. +/// +/// `BitkitTests` is hosted in the Bitkit app (`TEST_HOST` in the project settings), so +/// `UserDefaults.standard` *is* the app's preferences and anything a test writes there lands in the +/// developer's wallet. Issue #733 is what that looks like in practice: mock transfer records left +/// behind by a test run pinned a permanent "TRANSFER IN PROGRESS" banner on the real wallet. +/// +/// Reach for these in order of preference: +/// 1. `makeIsolatedDefaults()` when the code under test accepts injected defaults — nothing touches +/// the app's domain at all. +/// 2. `snapshotAppDefaults(_:)` when it does not, so the keys are put back afterwards. +/// 3. `guardAppDefaults(_:)` on suites that should write nothing, to keep it that way. +extension XCTestCase { + /// A `UserDefaults` suite unique to this test, emptied before it runs and removed afterwards. + func makeIsolatedDefaults(_ label: String = #function, file: StaticString = #filePath, line: UInt = #line) throws -> UserDefaults { + let suiteName = "\(type(of: self)).\(label).\(UUID().uuidString)" + let defaults = try XCTUnwrap(UserDefaults(suiteName: suiteName), "Could not open suite \(suiteName)", file: file, line: line) + defaults.removePersistentDomain(forName: suiteName) + addTeardownBlock { defaults.removePersistentDomain(forName: suiteName) } + return defaults + } + + /// Restores `keys` in `UserDefaults.standard` when the test ends, removing any that are absent + /// now. Use when the code under test has no seam for injected defaults. + func snapshotAppDefaults(_ keys: String...) { + let defaults = UserDefaults.standard + let snapshot = keys.map { (key: $0, value: defaults.object(forKey: $0)) } + addTeardownBlock { + for entry in snapshot { + if let value = entry.value { + defaults.set(value, forKey: entry.key) + } else { + defaults.removeObject(forKey: entry.key) + } + } + } + } + + /// Fails the test if it leaves any of `keys` in `UserDefaults.standard` changed. The regression + /// guard for #733: a suite that should be writing to an isolated suite goes red here instead of + /// silently corrupting the wallet on the simulator. + func guardAppDefaults(_ keys: String..., file: StaticString = #filePath, line: UInt = #line) { + let defaults = UserDefaults.standard + let before = keys.map { (key: $0, value: defaults.object(forKey: $0) as? NSObject) } + addTeardownBlock { + for entry in before where defaults.object(forKey: entry.key) as? NSObject != entry.value { + // Deliberately not interpolating the values: these keys hold large encoded blobs, and + // dumping both of them buries the one line that says what to do about it. + XCTFail( + """ + '\(entry.key)' in UserDefaults.standard was modified by this test, which writes to the host app's \ + own preferences. Inject an isolated suite with makeIsolatedDefaults(), or snapshot the key with \ + snapshotAppDefaults(_:) if the code under test has no seam for it. + """, + file: file, + line: line + ) + } + } + } +} From 8b1425510ed441cae4f50ba18a7b27504ce56be9 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 16 Sep 2026 10:03:35 -0300 Subject: [PATCH 4/6] fix: keep transfer unit tests out of the app's UserDefaults MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #733. TransferViewModelHwTests built a real TransferService, so any test driving a successful mock broadcast reached `fundPaidOrder` and persisted a transfer record into the app's own preferences. Built from the fixtures, those records name `order123`, which is not a real Blocktank order — `resolveChannelId` fails on every sync, `isSettled` never flips, and the wallet is left showing a permanent "TRANSFER IN PROGRESS" banner with a balance inflated by the phantom amount. Point the three transfer suites at an isolated defaults suite and guard the `transfers` key so they cannot drift back. Verified on a simulator that was carrying two of these phantom records: 63 tests pass and `transfers` is byte-identical afterwards. Removing the forwarding again fails exactly the two tests that reach `fundPaidOrder`, which is the guard doing its job. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AcCTBgiMafXxw71MWBGB2T --- .../TransferServiceActivityTests.swift | 9 ++- BitkitTests/TransferViewModelHwTests.swift | 14 +++- BitkitTests/TransferViewModelTests.swift | 67 ++++++++++++------- 3 files changed, 62 insertions(+), 28 deletions(-) diff --git a/BitkitTests/TransferServiceActivityTests.swift b/BitkitTests/TransferServiceActivityTests.swift index b7212a44b..f44fcc108 100644 --- a/BitkitTests/TransferServiceActivityTests.swift +++ b/BitkitTests/TransferServiceActivityTests.swift @@ -13,9 +13,12 @@ import XCTest final class TransferServiceActivityTests: XCTestCase { private let testDbPath = NSTemporaryDirectory() private let activity = Bitkit.CoreService.shared.activity + private var transferDefaults: UserDefaults! override func setUp() async throws { try await super.setUp() + transferDefaults = try makeIsolatedDefaults() + guardAppDefaults("transfers") _ = try initDb(basePath: testDbPath) try await Task.sleep(nanoseconds: 1_000_000_000) } @@ -29,7 +32,11 @@ final class TransferServiceActivityTests: XCTestCase { } private func makeService() -> Bitkit.TransferService { - Bitkit.TransferService(lightningService: .shared, blocktankService: Bitkit.CoreService.shared.blocktank) + Bitkit.TransferService( + storage: Bitkit.TransferStorage(defaults: transferDefaults), + lightningService: .shared, + blocktankService: Bitkit.CoreService.shared.blocktank + ) } func testPendingToSpendingActivityDoesNotStoreShortChannelId() async throws { diff --git a/BitkitTests/TransferViewModelHwTests.swift b/BitkitTests/TransferViewModelHwTests.swift index edf24f5b6..c5962e2da 100644 --- a/BitkitTests/TransferViewModelHwTests.swift +++ b/BitkitTests/TransferViewModelHwTests.swift @@ -7,6 +7,17 @@ import XCTest /// and guards against re-entry. The device orchestration itself is covered by `HwFundingSignerTests`. @MainActor final class TransferViewModelHwTests: XCTestCase { + /// A successful mock broadcast reaches `fundPaidOrder`, which persists a transfer record. Without + /// an isolated suite that record lands in the app's own preferences and never settles, because + /// the mock order id is not a real Blocktank order (#733). + private var transferDefaults: UserDefaults! + + override func setUpWithError() throws { + try super.setUpWithError() + transferDefaults = try makeIsolatedDefaults() + guardAppDefaults("transfers") + } + private func makeViewModel( funding: MockHwFunding, connecting: MockHwConnecting, @@ -17,7 +28,8 @@ final class TransferViewModelHwTests: XCTestCase { hwFunding: funding, hwConnecting: connecting, hwFeeRateProvider: { feeRate }, - hwTimeouts: timeouts + hwTimeouts: timeouts, + transferDefaults: transferDefaults ) } diff --git a/BitkitTests/TransferViewModelTests.swift b/BitkitTests/TransferViewModelTests.swift index 142bd628c..1d37d7ee8 100644 --- a/BitkitTests/TransferViewModelTests.swift +++ b/BitkitTests/TransferViewModelTests.swift @@ -3,9 +3,24 @@ import BitkitCore import XCTest final class TransferViewModelTests: XCTestCase { + /// The convenience initializer builds a real `TransferService`; an isolated suite keeps these + /// tests off the host app's own transfer store, and `guardAppDefaults` keeps it that way (#733). + private var transferDefaults: UserDefaults! + + override func setUpWithError() throws { + try super.setUpWithError() + transferDefaults = try makeIsolatedDefaults() + guardAppDefaults("transfers") + } + + @MainActor + private func makeViewModel() -> TransferViewModel { + TransferViewModel(transferDefaults: transferDefaults) + } + @MainActor func testDisplayOrderPrefersUiStateOrder() { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let baseOrder = makeOrder(id: "base", clientBalanceSat: 100_000, lspBalanceSat: 50000) let updatedOrder = makeOrder(id: "updated", clientBalanceSat: 150_000, lspBalanceSat: 75000) @@ -23,7 +38,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testSpendingLimitsCapsAtLspMaxClientBalanceWhenOnchainExceedsIt() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() var feeCallBalances: [UInt64] = [] // The liquidity calc reports no receiving room (maxLspBalance = 0) because the client // balance saturates the channel — the regression this guards against. @@ -52,7 +67,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testSpendingLimitsUsesFullBalanceWhenLspInfoUnavailable() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() var feeCallBalances: [UInt64] = [] let values = TransferValues( defaultLspBalance: Self.lspBalance, @@ -78,7 +93,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testSpendingLimitsIsZeroWhenLiquidityReportsZeroClientBalance() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let values = TransferValues( defaultLspBalance: Self.lspBalance, minLspBalance: Self.lspBalance, @@ -104,7 +119,7 @@ final class TransferViewModelTests: XCTestCase { /// came to 265,727 against 265,726 available. @MainActor func testSpendingMaxIsAffordableWhenTheServiceFeeRisesWithTheClientBalance() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let available: UInt64 = 265_726 let quotes: [UInt64: UInt64] = [available: 4165, 261_561: 4128] var feeCalls: [UInt64] = [] @@ -132,7 +147,7 @@ final class TransferViewModelTests: XCTestCase { /// is dearer than the first and no ordering assumption holds. Capping alone would not fix this. @MainActor func testSpendingMaxIsAffordableWhenTheServiceFeeFallsWithTheClientBalance() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let available: UInt64 = 266_478 let quotes: [UInt64: UInt64] = [available: 1798, 264_680: 1800, 264_678: 1801, 264_677: 1801] var feeCalls: [UInt64] = [] @@ -157,7 +172,7 @@ final class TransferViewModelTests: XCTestCase { /// an earlier balance would verify an order that is never created. @MainActor func testSpendingMaxRequotePricesTheSplitTheOrderWillUse() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let available: UInt64 = 266_478 let maxChannel: UInt64 = 1_403_872 let quotes: [UInt64: UInt64] = [available: 1798, 264_680: 1800, 264_678: 1801, 264_677: 1801] @@ -189,7 +204,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testSpendingMaxKeepsTheLastCandidateWhenTheRequoteFails() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let available: UInt64 = 266_478 let quotes: [UInt64: UInt64] = [available: 1798, 264_680: 1800] @@ -209,7 +224,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testSpendingMaxFallsBackWhenTheRoundsAreExhausted() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let available: UInt64 = 266_478 // The fee rises as fast as the balance steps down, so no candidate ever becomes affordable. let quotes: [UInt64: UInt64] = [available: 1800, 264_678: 2000, 264_478: 2200, 264_278: 2400] @@ -234,7 +249,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testAdvancedCapacityKeepsTheLspMaxWhenTheBudgetCoversIt() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() var quoteCount = 0 let settled = await viewModel.settleAdvancedLspBalance( @@ -254,7 +269,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testAdvancedCapacitySettlesBelowTheLspMaxWhenTheFeeOutgrowsTheBudget() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() var quotedCapacities: [UInt64] = [] let resolved = await viewModel.settleAdvancedLspBalance( @@ -277,7 +292,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testAdvancedCapacityIsNilWhenEvenTheMinimumIsUnaffordable() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let settled = await viewModel.settleAdvancedLspBalance( clientBalance: Self.advancedClientBalance, @@ -293,7 +308,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testAdvancedCapacityAdvertisesTheLspMaxWhenTheQuoteIsUnavailable() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let settled = await viewModel.settleAdvancedLspBalance( clientBalance: Self.advancedClientBalance, @@ -308,7 +323,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testAdvancedCapacityStopsAtTheLastAffordableCapacityWhenARequoteFails() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let settled = await viewModel.settleAdvancedLspBalance( clientBalance: Self.advancedClientBalance, @@ -331,7 +346,7 @@ final class TransferViewModelTests: XCTestCase { /// ceiling instead of being advertised. Whatever comes back must still be affordable. @MainActor func testAdvancedCapacityNeverAdvertisesAnOverBudgetCandidate() async throws { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() // Steep to 200k, then near-flat — the linear guess between the two ends underestimates the fee. let fee: (UInt64) -> UInt64 = { 1000 + min($0, 200_000) / 20 + $0.saturatingSub(200_000) / 1000 } var quotedCapacities: [UInt64] = [] @@ -356,7 +371,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testUpdateAdvancedTransferValuesSettlesTheMaxAndClearsTheFlag() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let values = TransferValues( defaultLspBalance: 1_500_000, minLspBalance: 50000, @@ -379,7 +394,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testUpdateAdvancedTransferValuesLeavesAnAffordableMaxUntouched() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let values = TransferValues( defaultLspBalance: 100_000, minLspBalance: 50000, @@ -403,7 +418,7 @@ final class TransferViewModelTests: XCTestCase { /// first entry, where `transferValues` is still zeroed. @MainActor func testUpdateAdvancedTransferValuesHoldsTheFlagWhileTheBudgetIsRead() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let values = TransferValues( defaultLspBalance: 1_500_000, minLspBalance: 50000, @@ -433,7 +448,7 @@ final class TransferViewModelTests: XCTestCase { /// No range to settle means no reason to pay for the budget round trip. @MainActor func testUpdateAdvancedTransferValuesSkipsTheBudgetReadWithoutARange() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let values = TransferValues( defaultLspBalance: 50000, minLspBalance: 50000, @@ -460,7 +475,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testCanFundOrderRejectsAnAmountOverTheBudget() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let canFund = await viewModel.canFundOrder( clientBalance: 260_000, @@ -474,7 +489,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testCanFundOrderAcceptsAnAmountThatFitsTheBudget() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let canFund = await viewModel.canFundOrder( clientBalance: 260_000, @@ -488,7 +503,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testCanFundOrderDoesNotBlockWhenTheBudgetIsUnknown() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let canFund = await viewModel.canFundOrder( clientBalance: 260_000, @@ -503,7 +518,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testCanFundOrderDoesNotBlockWhenTheQuoteFails() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let canFund = await viewModel.canFundOrder( clientBalance: 260_000, @@ -518,7 +533,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testCanFundAdvancedOrderRejectsACapacityOverTheBudget() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let canFund = await viewModel.canFundAdvancedOrder( clientBalance: 260_000, @@ -532,7 +547,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testCanFundAdvancedOrderDoesNotBlockWhenTheBudgetIsUnknownOrUnquoted() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() let unsizedBudget = await viewModel.canFundAdvancedOrder( clientBalance: 260_000, @@ -553,7 +568,7 @@ final class TransferViewModelTests: XCTestCase { @MainActor func testHwFundingBudgetIsNilWithoutDeviceCapabilities() async { - let viewModel = TransferViewModel() + let viewModel = makeViewModel() // No hardware capabilities injected, so the funding guards degrade to non-blocking. let budget = await viewModel.hwFundingBudget(walletId: "wallet-1") From de059da8c5e10e310bec8365f7a155d9fe16d4be Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 16 Sep 2026 12:06:03 -0300 Subject: [PATCH 5/6] test: put the suite's remaining view models on the isolated store MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two sites still built a view model against the standard defaults: the no-capabilities error test and the funding budget test. Neither writes today — the first returns at the signer guard and the second only reads availability — so this is not a fix for a live leak, but the class comment says every view model in the suite uses the isolated store, and now they do. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AcCTBgiMafXxw71MWBGB2T --- BitkitTests/TransferViewModelHwTests.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/BitkitTests/TransferViewModelHwTests.swift b/BitkitTests/TransferViewModelHwTests.swift index c5962e2da..96796a726 100644 --- a/BitkitTests/TransferViewModelHwTests.swift +++ b/BitkitTests/TransferViewModelHwTests.swift @@ -235,7 +235,7 @@ final class TransferViewModelHwTests: XCTestCase { } func testConfirmWithoutHwCapabilitiesSurfacesGenericError() { - let vm = TransferViewModel() // no signer injected + let vm = TransferViewModel(transferDefaults: transferDefaults) // no signer injected vm.onTransferToSpendingHwConfirm(order: .mock(), walletId: "trezor:wallet") if case .generic = vm.hwTransferError {} else { XCTFail("expected .generic error") @@ -267,7 +267,8 @@ final class TransferViewModelHwTests: XCTestCase { hwFunding: funding, hwConnecting: MockHwConnecting(), hwFeeRateProvider: { 2 }, - hwAddressProvider: { "bcrt1qtest" } + hwAddressProvider: { "bcrt1qtest" }, + transferDefaults: transferDefaults ) let budget = await vm.hwFundingBudget(walletId: "trezor:wallet") From a721ea08efc13241f1bdf3f0fd8b32ef1b8c6a23 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 17 Sep 2026 07:50:01 -0300 Subject: [PATCH 6/6] fix: keep TransferViewModel's default on the shared transfer storage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The convenience initialisers built `TransferStorage(defaults: .standard)` when no suite was passed. That reads and writes the same key as `TransferStorage.shared` but is a second instance with its own change subject, and `BackupService` only listens to `.shared` — so a transfer saved through a default-constructed view model would never mark the wallet backup as required. Nothing shipped goes through these initialisers today (the app uses the designated one with an explicit service; the rest are previews and tests), so no user is affected. But the default should behave as it did before the storage seam was added: `transferDefaults` is now optional, nil means `.shared`, and only a test suite gets its own instance. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AcCTBgiMafXxw71MWBGB2T --- Bitkit/ViewModels/TransferViewModel.swift | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/Bitkit/ViewModels/TransferViewModel.swift b/Bitkit/ViewModels/TransferViewModel.swift index f8e1bf2fb..e8d84c72b 100644 --- a/Bitkit/ViewModels/TransferViewModel.swift +++ b/Bitkit/ViewModels/TransferViewModel.swift @@ -213,17 +213,18 @@ class TransferViewModel: ObservableObject { } } - /// Convenience initializer for testing and previews. `transferDefaults` scopes transfer - /// persistence: tests pass an isolated suite so mock transfers never reach the app's own store. + /// Convenience initializer for testing and previews. Leave `transferDefaults` nil to persist through + /// `TransferStorage.shared`, whose change notifications drive backups; tests pass an isolated suite + /// so mock transfers never reach the app's own store. convenience init( coreService: CoreService = .shared, lightningService: LightningService = .shared, currencyService: CurrencyService = .shared, sheetViewModel: SheetViewModel = SheetViewModel(), - transferDefaults: UserDefaults = .standard + transferDefaults: UserDefaults? = nil ) { let transferService = TransferService( - storage: TransferStorage(defaults: transferDefaults), + storage: transferDefaults.map { TransferStorage(defaults: $0) } ?? .shared, lightningService: lightningService, blocktankService: coreService.blocktank ) @@ -239,7 +240,8 @@ class TransferViewModel: ObservableObject { /// Convenience initializer for hardware-wallet transfer tests. Builds the `TransferService` /// inside the app module so callers don't construct cross-module service types — `transferDefaults` /// is a `UserDefaults` for the same reason, since `TransferStorage` is compiled into both modules. - /// Tests pass an isolated suite so mock transfers never reach the app's own store. + /// Leave it nil to persist through `TransferStorage.shared`; tests pass an isolated suite so mock + /// transfers never reach the app's own store. convenience init( hwFunding: HwTransferFunding?, hwConnecting: HwTransferConnecting?, @@ -249,10 +251,10 @@ class TransferViewModel: ObservableObject { coreService: CoreService = .shared, lightningService: LightningService = .shared, sheetViewModel: SheetViewModel = SheetViewModel(), - transferDefaults: UserDefaults = .standard + transferDefaults: UserDefaults? = nil ) { let transferService = TransferService( - storage: TransferStorage(defaults: transferDefaults), + storage: transferDefaults.map { TransferStorage(defaults: $0) } ?? .shared, lightningService: lightningService, blocktankService: coreService.blocktank )