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 diff --git a/Bitkit/ViewModels/TransferViewModel.swift b/Bitkit/ViewModels/TransferViewModel.swift index c18497cf0..e8d84c72b 100644 --- a/Bitkit/ViewModels/TransferViewModel.swift +++ b/Bitkit/ViewModels/TransferViewModel.swift @@ -213,14 +213,18 @@ class TransferViewModel: ObservableObject { } } - /// Convenience initializer for testing and previews + /// 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() + sheetViewModel: SheetViewModel = SheetViewModel(), + transferDefaults: UserDefaults? = nil ) { let transferService = TransferService( + storage: transferDefaults.map { TransferStorage(defaults: $0) } ?? .shared, lightningService: lightningService, blocktankService: coreService.blocktank ) @@ -234,7 +238,10 @@ 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. + /// 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?, @@ -243,9 +250,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? = nil ) { let transferService = TransferService( + storage: transferDefaults.map { TransferStorage(defaults: $0) } ?? .shared, lightningService: lightningService, blocktankService: coreService.blocktank ) 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 + ) + } + } + } +} 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..96796a726 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 ) } @@ -223,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") @@ -255,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") 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")