From e8c835f3aa3ff93ced7b883d6bb6afbccdb56fab Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 21 Oct 2026 09:11:03 -0600 Subject: [PATCH 1/2] perf: search only loaded wallet accounts --- Bitkit/Services/CoreService.swift | 10 +- .../AddressSearchCoordinatorTests.swift | 135 ++++++++++++++++-- 2 files changed, 133 insertions(+), 12 deletions(-) diff --git a/Bitkit/Services/CoreService.swift b/Bitkit/Services/CoreService.swift index 4be21d6d9..d8add9d4d 100644 --- a/Bitkit/Services/CoreService.swift +++ b/Bitkit/Services/CoreService.swift @@ -1964,14 +1964,18 @@ actor AddressSearchCoordinator { (false, .external), (true, .internal), ] - let primaryAccounts = addressTypesToSearch.map { LDKNode.OnchainWalletAccount(addressType: $0, accountIndex: 0) } + var primaryAccounts = addressTypesToSearch.map { LDKNode.OnchainWalletAccount(addressType: $0, accountIndex: 0) } let companionAccounts: [LDKNode.OnchainWalletAccount] do { - companionAccounts = try await listAccounts().filter { $0.accountIndex > 0 }.sorted { + let accounts = try await listAccounts() + primaryAccounts = primaryAccounts.filter { primary in + accounts.contains { $0.accountIndex == 0 && $0.addressType == primary.addressType } + } + companionAccounts = accounts.filter { $0.accountIndex > 0 }.sorted { ($0.accountIndex, $0.addressType.stringValue) < ($1.accountIndex, $1.addressType.stringValue) } } catch { - Logger.warn("Failed to list companion accounts for address search: \(error)", context: "CoreService.AddressSearch") + Logger.warn("Failed to list wallet accounts for address search: \(error)", context: "CoreService.AddressSearch") companionAccounts = [] } // Preserve account-zero priority, including its change addresses, before searching companion accounts. diff --git a/BitkitTests/AddressSearchCoordinatorTests.swift b/BitkitTests/AddressSearchCoordinatorTests.swift index 9e2cccfaa..f48e48cfa 100644 --- a/BitkitTests/AddressSearchCoordinatorTests.swift +++ b/BitkitTests/AddressSearchCoordinatorTests.swift @@ -21,10 +21,23 @@ final class AddressSearchCoordinatorTests: XCTestCase { func testCompanionReceiveAddressAfterChangeUsesSeparateSearchWindow() async throws { let key = "addressSearch_lastUsedReceiveIndex_nativeSegwit_account3" defaults.set(400, forKey: key) + let calls = AddressSearchCallLog() let search = AddressSearchCoordinator( defaults: defaults, - listAccounts: { [OnchainWalletAccount(addressType: .nativeSegwit, accountIndex: 3)] }, + listAccounts: { + await calls.record("list") + return [ + OnchainWalletAccount(addressType: .nativeSegwit, accountIndex: 3), + OnchainWalletAccount(addressType: .taproot, accountIndex: 2), + OnchainWalletAccount(addressType: .nativeSegwit, accountIndex: 0), + OnchainWalletAccount(addressType: .legacy, accountIndex: 2), + ] + }, deriveAddresses: { account, keychain, start, count in + XCTAssertTrue(account.accountIndex > 0 || account.addressType == .nativeSegwit) + if start == 0 { + await calls.record("\(account.addressType.stringValue):\(account.accountIndex):\(keychain)") + } XCTAssertEqual(count, 200) if account.accountIndex == 3, keychain == .external, start == 1200 { return [LightningService.AddressDerivationInfo(address: "server-receive", index: 1200)] @@ -39,14 +52,32 @@ final class AddressSearchCoordinatorTests: XCTestCase { XCTAssertEqual(result, "server-receive") XCTAssertEqual(defaults.integer(forKey: key), 1200) XCTAssertNil(defaults.object(forKey: "addressSearch_lastUsedReceiveIndex_nativeSegwit")) + let recorded = await calls.events + XCTAssertEqual(recorded, [ + "list", "nativeSegwit:0:external", "nativeSegwit:0:internal", + "legacy:2:external", "taproot:2:external", "nativeSegwit:3:external", + ]) } func testAccountZeroChangeKeepsPriorityAndExistingCacheKey() async throws { + let calls = AddressSearchCallLog() let search = AddressSearchCoordinator( defaults: defaults, - listAccounts: { [OnchainWalletAccount(addressType: .nativeSegwit, accountIndex: 3)] }, + listAccounts: { + [ + OnchainWalletAccount(addressType: .legacy, accountIndex: 0), + OnchainWalletAccount(addressType: .nativeSegwit, accountIndex: 3), + OnchainWalletAccount(addressType: .nativeSegwit, accountIndex: 0), + ] + }, deriveAddresses: { account, keychain, start, _ in XCTAssertEqual(account.accountIndex, 0, "Account zero should resolve before companion derivation") + if start == 0 { + await calls.record("\(account.addressType.stringValue):\(account.accountIndex):\(keychain)") + } + if account.accountIndex == 3 { + return [LightningService.AddressDerivationInfo(address: "server-receive", index: 0)] + } return account.addressType == .nativeSegwit && keychain == .internal && start == 200 ? [LightningService.AddressDerivationInfo(address: "owned-change", index: 203)] : [] } @@ -57,6 +88,9 @@ final class AddressSearchCoordinatorTests: XCTestCase { ) XCTAssertEqual(result, "owned-change") XCTAssertEqual(defaults.integer(forKey: "addressSearch_lastUsedChangeIndex_nativeSegwit"), 203) + let recorded = await calls.events + XCTAssertEqual(recorded, ["nativeSegwit:0:external", "legacy:0:external", "nativeSegwit:0:internal"]) + XCTAssertNil(defaults.object(forKey: "addressSearch_lastUsedReceiveIndex_nativeSegwit_account3")) } func testMatchedCompanionReceiveIndexExtendsPersistedWindowByOneThousand() async throws { @@ -87,7 +121,9 @@ final class AddressSearchCoordinatorTests: XCTestCase { defaults.set(1000, forKey: key) let search = AddressSearchCoordinator( defaults: defaults, - listAccounts: { [] }, + listAccounts: { + LDKNode.AddressType.allAddressTypes.map { OnchainWalletAccount(addressType: $0, accountIndex: 0) } + }, deriveAddresses: { account, keychain, start, count in guard account.accountIndex == 0, account.addressType == .nativeSegwit, keychain == .internal else { return [] } XCTAssertLessThanOrEqual(start + count, 2001) @@ -119,7 +155,9 @@ final class AddressSearchCoordinatorTests: XCTestCase { func testUnregisteredAddressDoesNotMatchAndSearchStaysBounded() async throws { let search = AddressSearchCoordinator( defaults: defaults, - listAccounts: { [] }, + listAccounts: { + LDKNode.AddressType.allAddressTypes.map { OnchainWalletAccount(addressType: $0, accountIndex: 0) } + }, deriveAddresses: { account, _, start, count in XCTAssertEqual(account.accountIndex, 0) XCTAssertLessThan(start, 1000) @@ -135,16 +173,85 @@ final class AddressSearchCoordinatorTests: XCTestCase { } func testDerivationFailureDoesNotSelectTransactionOutput() async throws { + for error in [AppError(serviceError: .nodeNotSetup), CancellationError()] as [Error] { + let search = AddressSearchCoordinator( + defaults: defaults, + listAccounts: { [OnchainWalletAccount(addressType: .nativeSegwit, accountIndex: 3)] }, + deriveAddresses: { _, _, _, _ in throw error } + ) + let result = try await search.runAddressSearch( + details: details(["unverified-first-output"]), value: 15000, + currentWalletAddress: "", selectedAddressType: .nativeSegwit + ) + XCTAssertNil(result) + XCTAssertNil(defaults.object(forKey: "addressSearch_lastUsedReceiveIndex_nativeSegwit_account3")) + XCTAssertNil(defaults.object(forKey: "addressSearch_lastUsedChangeIndex_nativeSegwit_account3")) + } + } + + func testEmptyAccountListIsReadAgainForTheNextSearch() async throws { + let calls = AddressSearchCallLog() let search = AddressSearchCoordinator( defaults: defaults, - listAccounts: { [OnchainWalletAccount(addressType: .nativeSegwit, accountIndex: 3)] }, - deriveAddresses: { _, _, _, _ in throw AppError(serviceError: .nodeNotSetup) } + listAccounts: { + let count = await calls.record("list") + return count == 1 ? [] : [OnchainWalletAccount(addressType: .taproot, accountIndex: 0)] + }, + deriveAddresses: { account, keychain, start, _ in + await calls.record("derive") + XCTAssertEqual(account.addressType, .taproot) + XCTAssertEqual(account.accountIndex, 0) + XCTAssertEqual(keychain, .external) + XCTAssertEqual(start, 0) + return [LightningService.AddressDerivationInfo(address: "newly-loaded", index: 0)] + } ) - let result = try await search.runAddressSearch( - details: details(["unverified-first-output"]), value: 15000, + let first = try await search.runAddressSearch( + details: details(["newly-loaded"]), value: 15000, currentWalletAddress: "", selectedAddressType: .nativeSegwit ) - XCTAssertNil(result) + XCTAssertNil(first) + XCTAssertNil(defaults.object(forKey: "addressSearch_lastUsedReceiveIndex_taproot")) + let firstCalls = await calls.events + XCTAssertEqual(firstCalls, ["list"]) + + let second = try await search.runAddressSearch( + details: details(["newly-loaded"]), value: 15000, + currentWalletAddress: "", selectedAddressType: .nativeSegwit + ) + XCTAssertEqual(second, "newly-loaded") + XCTAssertEqual(defaults.object(forKey: "addressSearch_lastUsedReceiveIndex_taproot") as? Int, 0) + let allCalls = await calls.events + XCTAssertEqual(allCalls, ["list", "list", "derive"]) + } + + func testAccountListingFailureKeepsPrimaryFallback() async throws { + for error in [AppError(serviceError: .nodeNotSetup), CancellationError()] as [Error] { + let calls = AddressSearchCallLog() + let search = AddressSearchCoordinator( + defaults: defaults, + listAccounts: { throw error }, + deriveAddresses: { account, keychain, start, _ in + XCTAssertEqual(account.accountIndex, 0) + if start == 0 { + await calls.record("\(account.addressType.stringValue):\(keychain)") + } + return account.addressType == .nativeSegwit && keychain == .internal && start == 0 + ? [LightningService.AddressDerivationInfo(address: "owned-change", index: 7)] : [] + } + ) + let result = try await search.runAddressSearch( + details: details(["owned-change"]), value: 15000, + currentWalletAddress: "", selectedAddressType: .taproot + ) + XCTAssertEqual(result, "owned-change") + XCTAssertEqual(defaults.integer(forKey: "addressSearch_lastUsedChangeIndex_nativeSegwit"), 7) + let recorded = await calls.events + XCTAssertEqual(recorded, [ + "taproot:external", "legacy:external", "nestedSegwit:external", "nativeSegwit:external", + "taproot:internal", "legacy:internal", "nestedSegwit:internal", "nativeSegwit:internal", + ]) + } } private func details(_ addresses: [String]) -> BitkitCore.TransactionDetails { @@ -159,3 +266,13 @@ final class AddressSearchCoordinatorTests: XCTestCase { ) } } + +private actor AddressSearchCallLog { + private(set) var events: [String] = [] + + @discardableResult + func record(_ event: String) -> Int { + events.append(event) + return events.count + } +} From 54e8820bfd8ee5a1af071fadea182077938867c0 Mon Sep 17 00:00:00 2001 From: benk10 Date: Thu, 29 Oct 2026 11:28:56 +0200 Subject: [PATCH 2/2] fix: keep payment confirmation busy after unlock --- .../Wallets/Send/SendConfirmationView.swift | 9 ++-- .../SendConfirmationLifecycleTests.swift | 51 ++++++++++++++++++- BitkitTests/SendConfirmationSwipeTests.swift | 15 ++++++ changelog.d/next/1039.fixed.md | 2 +- 4 files changed, 71 insertions(+), 6 deletions(-) diff --git a/Bitkit/Views/Wallets/Send/SendConfirmationView.swift b/Bitkit/Views/Wallets/Send/SendConfirmationView.swift index 799e2305b..564d721c6 100644 --- a/Bitkit/Views/Wallets/Send/SendConfirmationView.swift +++ b/Bitkit/Views/Wallets/Send/SendConfirmationView.swift @@ -382,7 +382,8 @@ struct SendConfirmationView: View { hasStartedAutomaticPayment: hasStartedAutomaticPayment, isFeeRateMissing: isFeeRateMissing, feeRateLoadFailed: wallet.feeRateLoadFailed, - isPreparingRequest: preparingRequest != nil + isPreparingRequest: preparingRequest != nil, + isSubmittingPayment: isSubmittingPayment ), isSubmitted: hasStartedAutomaticPayment, swipeProgress: $swipeProgress @@ -806,6 +807,7 @@ struct SendConfirmationView: View { } private func submitPayment(isAutomatic: Bool = false) async throws { + guard !isSubmittingPayment else { return } guard preparingRequest == nil, walletSwitchContext == nil else { throw CancellationError() } if isAutomatic, isFeeRateMissing { guard !Self.automaticPaymentAwaitsFeeRate(isFeeRateMissing: isFeeRateMissing, feeRateLoadFailed: wallet.feeRateLoadFailed) else { @@ -1012,9 +1014,10 @@ struct SendConfirmationView: View { hasStartedAutomaticPayment: Bool, isFeeRateMissing: Bool, feeRateLoadFailed: Bool, - isPreparingRequest: Bool = false + isPreparingRequest: Bool = false, + isSubmittingPayment: Bool = false ) -> Bool { - isPreparingRequest || hasStartedAutomaticPayment || (isFeeRateMissing && !feeRateLoadFailed) + isSubmittingPayment || isPreparingRequest || hasStartedAutomaticPayment || (isFeeRateMissing && !feeRateLoadFailed) } private func openManualCoinSelection() async throws { diff --git a/BitkitTests/SendConfirmationLifecycleTests.swift b/BitkitTests/SendConfirmationLifecycleTests.swift index 4f23ba88f..62dafc610 100644 --- a/BitkitTests/SendConfirmationLifecycleTests.swift +++ b/BitkitTests/SendConfirmationLifecycleTests.swift @@ -460,19 +460,66 @@ final class SendConfirmationLifecycleTests: XCTestCase { XCTAssertEqual(setup.pinChecks, [false], "The payment starts once and is not cancelled while it awaits the PIN") } + func testRecreatedConfirmationDoesNotAuthorizeWhilePaymentIsSubmitting() async throws { + snapshotAppDefaultsDomain() + snapshotAppGroupDefaults("home_screen_display_currency_code_v1", "home_screen_display_currency_symbol_v1") + let settings = SettingsViewModel.shared + let pinEnabled = settings.pinEnabled + addTeardownBlock { @MainActor in settings.pinEnabled = pinEnabled } + settings.pinEnabled = true + settings.requirePinForPayments = true + settings.useBiometrics = false + settings.coinSelectionMethod = .autopilot + let app = AppViewModel() + let wallet = WalletViewModel() + let sheets = SheetViewModel() + sheets.activeSheetConfiguration = SheetConfiguration(id: .send, data: nil) + app.contactPaymentContext = ContactPaymentContext(publicKey: "pubkytest", isInitialSubscriptionPayment: true) + app.scannedLightningInvoice = LightningInvoice( + bolt11: "test-invoice", paymentHash: Data(), amountSatoshis: 6007, + timestampSeconds: 0, expirySeconds: 0, isExpired: false, + description: nil, networkType: .regtest, payeeNodeId: nil + ) + app.selectedWalletToPayFrom = .lightning + wallet.sendAmountSats = 6007 + wallet.totalLightningSats = 1_000_000 + let setup = SendConfirmationTestSetup() + setup.isPending = false + var isSubmittingPayment = true + var path: [SendRoute] = [] + + for _ in 0 ..< 2 { + let window = host( + app: app, + wallet: wallet, + sheets: sheets, + path: Binding(get: { path }, set: { path = $0 }), + setup: setup, + isSubmittingPayment: Binding(get: { isSubmittingPayment }, set: { isSubmittingPayment = $0 }) + ) + try await Task.sleep(for: .milliseconds(500)) + close(window) + } + + XCTAssertTrue(setup.pinChecks.isEmpty, "Restoring an in-flight payment must not request authorization again") + XCTAssertTrue(path.isEmpty, "Ignoring the duplicate entry must preserve the original payment's navigation") + XCTAssertTrue(isSubmittingPayment, "The original payment owns the retained submission flag") + } + private func host( app: AppViewModel, wallet: WalletViewModel, sheets: SheetViewModel, path: Binding<[SendRoute]>, navigation: SendConfirmationTestNavigation? = nil, - setup: SendConfirmationTestSetup? = nil + setup: SendConfirmationTestSetup? = nil, + isSubmittingPayment: Binding = .constant(false) ) -> UIWindow { let hwSend = HwSendCoordinator() let makeConfirmation = { (isSetupPending: Bool) in SendConfirmationView( navigationPath: path, - isSubmittingPayment: .constant(false), + isSubmittingPayment: isSubmittingPayment, hwSend: hwSend, requestPinCheck: { guard let setup else { diff --git a/BitkitTests/SendConfirmationSwipeTests.swift b/BitkitTests/SendConfirmationSwipeTests.swift index 47985501b..b0e71074f 100644 --- a/BitkitTests/SendConfirmationSwipeTests.swift +++ b/BitkitTests/SendConfirmationSwipeTests.swift @@ -150,6 +150,21 @@ final class SendConfirmationSwipeTests: XCTestCase { )) } + func testManualPaymentKeepsTheSwipeBusyWhenConfirmationIsRecreated() { + XCTAssertTrue(SendConfirmationView.isSwipeLoading( + hasStartedAutomaticPayment: false, + isFeeRateMissing: false, + feeRateLoadFailed: false, + isSubmittingPayment: true + )) + XCTAssertFalse(SendConfirmationView.isSwipeLoading( + hasStartedAutomaticPayment: false, + isFeeRateMissing: false, + feeRateLoadFailed: false, + isSubmittingPayment: false + )) + } + func testSwipeStopsItsSpinnerOnceTheFeeRateLoadFailed() { XCTAssertFalse(SendConfirmationView.isSwipeLoading( hasStartedAutomaticPayment: false, diff --git a/changelog.d/next/1039.fixed.md b/changelog.d/next/1039.fixed.md index a46649f74..ac1d67e07 100644 --- a/changelog.d/next/1039.fixed.md +++ b/changelog.d/next/1039.fixed.md @@ -1 +1 @@ -Fixed payment requests losing their amount or getting stuck on loading after unlocking the app. +Fixed payment requests losing their amount or getting stuck on loading after unlocking the app, and kept in-flight payments busy after unlock.