From 67362eb89a482fa4623f5252fa3b083e3e31c72f Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 29 Sep 2026 09:28:57 -0500 Subject: [PATCH 1/6] fix: clean up deleted private contacts --- Bitkit/Managers/ContactsManager.swift | 30 ++++- .../PaykitPaymentRequestService.swift | 18 ++- .../PrivatePaykitService+Contacts.swift | 3 +- Bitkit/Services/PubkyService.swift | 52 ++++++-- BitkitTests/PaykitContactLifecycleTests.swift | 111 ++++++++++++++++++ .../PaykitPaymentRequestServiceTests.swift | 44 +++++++ changelog.d/next/830.fixed.md | 1 + journeys/payment-requests/README.md | 2 + .../delete-and-readd-contact.xml | 23 ++++ 9 files changed, 266 insertions(+), 18 deletions(-) create mode 100644 BitkitTests/PaykitContactLifecycleTests.swift create mode 100644 changelog.d/next/830.fixed.md create mode 100644 journeys/payment-requests/delete-and-readd-contact.xml diff --git a/Bitkit/Managers/ContactsManager.swift b/Bitkit/Managers/ContactsManager.swift index edde0e68c..a51c67d97 100644 --- a/Bitkit/Managers/ContactsManager.swift +++ b/Bitkit/Managers/ContactsManager.swift @@ -114,7 +114,12 @@ struct ContactSection: Identifiable { @MainActor class ContactsManager: ObservableObject { - @Published var contacts: [PubkyContact] = [] + private var contactsRevision = 0 + + @Published var contacts: [PubkyContact] = [] { + didSet { contactsRevision += 1 } + } + @Published var isLoading = false @Published var hasLoaded = false @Published var loadErrorMessage: String? @@ -173,6 +178,7 @@ class ContactsManager: ObservableObject { return } + let revision = contactsRevision isLoading = true loadErrorMessage = nil defer { isLoading = false } @@ -225,6 +231,8 @@ class ContactsManager: ObservableObject { return (results, failures, missingFailures, firstError) } + guard contactsRevision == revision else { return } + if !records.isEmpty, loadedResult.contacts.isEmpty { if loadedResult.failures == loadedResult.missingFailures { await PrivatePaykitService.shared.pruneUnsavedContactState(savedPublicKeys: []) @@ -250,6 +258,7 @@ class ContactsManager: ObservableObject { Logger.info("Loaded \(contacts.count) contacts", context: "ContactsManager") } catch { + guard contactsRevision == revision else { return } if Self.isMissingContactsDataError(error) { await PrivatePaykitService.shared.pruneUnsavedContactState(savedPublicKeys: []) contacts = [] @@ -297,7 +306,12 @@ class ContactsManager: ObservableObject { } let receiverPaths = try await Self.relevantReceiverPaths(for: prefixedKey) - _ = try await PubkyService.saveContact(publicKey: prefixedKey, label: profile.name, receiverPaths: receiverPaths) + _ = try await PubkyService.saveContact( + publicKey: prefixedKey, + label: profile.name, + receiverPaths: receiverPaths, + restorePrivateConnection: true + ) Logger.info("Added contact \(PubkyPublicKeyFormat.redacted(prefixedKey))", context: "ContactsManager") @@ -342,7 +356,12 @@ class ContactsManager: ObservableObject { do { let profile = try await resolveContactProfile(publicKey: key, includePlaceholder: true) let receiverPaths = try await Self.relevantReceiverPaths(for: key) - _ = try await PubkyService.saveContact(publicKey: key, label: profile.name, receiverPaths: receiverPaths) + _ = try await PubkyService.saveContact( + publicKey: key, + label: profile.name, + receiverPaths: receiverPaths, + restorePrivateConnection: true + ) return .success(PubkyContact(publicKey: key, profile: profile)) } catch is CancellationError { return .failure(CancellationError()) @@ -423,12 +442,11 @@ class ContactsManager: ObservableObject { try await Task.detached { _ = try await PubkyService.removeContact(publicKey: prefixedKey) }.value - await PrivatePaykitService.shared.removeSavedContact(publicKey: prefixedKey) + contacts.removeAll { $0.publicKey == prefixedKey } Self.removeContactProfileOverride(publicKey: prefixedKey) + await PrivatePaykitService.shared.removeSavedContact(publicKey: prefixedKey) Logger.info("Removed contact \(PubkyPublicKeyFormat.redacted(prefixedKey))", context: "ContactsManager") - - contacts.removeAll { $0.publicKey == prefixedKey } } func deleteAllContacts() async throws { diff --git a/Bitkit/Services/PaykitPaymentRequestService.swift b/Bitkit/Services/PaykitPaymentRequestService.swift index b1630d55c..0966e6f09 100644 --- a/Bitkit/Services/PaykitPaymentRequestService.swift +++ b/Bitkit/Services/PaykitPaymentRequestService.swift @@ -530,8 +530,14 @@ struct PaykitPaymentRequestService { logIntakeFailures(intakeReports) let synchronizationDate = now() let records = try await sdk.paymentRequests() + let blockedPeers = try await sdk.linkedPeers().filter { $0.state == .blocked } + let availableRecords = records.filter { record in + !blockedPeers.contains { + PubkyPublicKeyFormat.matches($0.counterparty, record.counterparty) && $0.counterpartyReceiverPath == record.counterpartyReceiverPath + } + } var rejections: [IncomingPaykitPaymentRequestRejection] = [] - let incoming = records.compactMap { record in + let incoming = availableRecords.compactMap { record in switch PaykitPaymentRequest.parseIncoming(record: record, now: synchronizationDate) { case let .success(request): return request @@ -557,7 +563,7 @@ struct PaykitPaymentRequestService { let history = records.compactMap { PaykitPaymentRequest(historyRecord: $0, now: synchronizationDate) } - let subscriptions = records.compactMap { PaykitSubscription(record: $0) } + let subscriptions = availableRecords.compactMap { PaykitSubscription(record: $0) } return PaykitPaymentRequestSnapshot( incoming: incoming, history: history, @@ -767,6 +773,13 @@ struct PaykitPaymentRequestService { ) } + func ensurePaymentAllowed(_ request: PaykitPaymentRequest) async throws { + guard try await !sdk.linkedPeers().contains(where: { + $0.state == .blocked && PubkyPublicKeyFormat.matches($0.counterparty, request.counterparty) && + $0.counterpartyReceiverPath == request.counterpartyReceiverPath + }) else { throw PaykitPaymentRequestError.requestUnavailable } + } + func accept(_ request: PaykitPaymentRequest) async throws { guard !request.isExpired(at: now()) else { throw PaykitPaymentRequestError.requestExpired @@ -1290,6 +1303,7 @@ final class PaykitPaymentRequestManager { markApprovedForPayment: true, preservePending: !request.requiresAcceptance ) { + try await service.ensurePaymentAllowed($0) try await consumePrivatePaymentList() if $0.requiresAcceptance { try await service.accept($0) diff --git a/Bitkit/Services/PrivatePaykitService+Contacts.swift b/Bitkit/Services/PrivatePaykitService+Contacts.swift index ec744935a..0f459218e 100644 --- a/Bitkit/Services/PrivatePaykitService+Contacts.swift +++ b/Bitkit/Services/PrivatePaykitService+Contacts.swift @@ -191,7 +191,8 @@ extension PrivatePaykitService { ) for receiverPath in cleanupReceiverPaths { do { - let report = try await PaykitSdkService.shared.clearPrivatePaymentList(to: publicKey, receiverPath: receiverPath) + guard let report = try await PaykitSdkService.shared.clearPrivatePaymentList(to: publicKey, receiverPath: receiverPath) + else { continue } if !report.failedToQueue.isEmpty || !report.failedToDeliver.isEmpty { throw PrivatePaykitError.privateUnavailable } diff --git a/Bitkit/Services/PubkyService.swift b/Bitkit/Services/PubkyService.swift index 9442097fb..534403bb2 100644 --- a/Bitkit/Services/PubkyService.swift +++ b/Bitkit/Services/PubkyService.swift @@ -279,8 +279,15 @@ enum PubkyService { try await PaykitSdkService.shared.contactRecords() } - static func saveContact(publicKey: String, label: String?, receiverPaths: [String]? = nil) async throws -> Paykit.ContactRecord { - try await PaykitSdkService.shared.saveContact(publicKey: publicKey, label: label, receiverPaths: receiverPaths) + static func saveContact(publicKey: String, label: String?, receiverPaths: [String]? = nil, + restorePrivateConnection: Bool = false) async throws -> Paykit.ContactRecord + { + try await PaykitSdkService.shared.saveContact( + publicKey: publicKey, + label: label, + receiverPaths: receiverPaths, + restorePrivateConnection: restorePrivateConnection + ) } static func removeContact(publicKey: String) async throws -> Paykit.ContactRecord? { @@ -323,6 +330,7 @@ actor PaykitSdkService { private let paymentAdapter = PaykitSdkPaymentAdapter() private let operationLock = PaykitSdkOperationLock() private let pubkyClientConfig = PaykitSdkService.makePubkyClientConfig(localTestnetHost: Env.pubkyLocalTestnetHost) + private let sdkFactory: (() throws -> PaykitSdk)? private let bootstrapFactory: BootstrapFactory private var cachedBootstrap: PubkySessionBootstrap? private var isRepublishingIdentity = false @@ -331,8 +339,10 @@ actor PaykitSdkService { private var sdk: PaykitSdk? init( + sdkFactory: (() throws -> PaykitSdk)? = nil, bootstrapFactory: @escaping BootstrapFactory = PubkySessionBootstrap.withPubkyClientConfig(clientId:pubkyClient:) ) { + self.sdkFactory = sdkFactory self.bootstrapFactory = bootstrapFactory } @@ -598,17 +608,36 @@ actor PaykitSdkService { } } - func saveContact(publicKey: String, label: String?, receiverPaths: [String]? = nil) async throws -> Paykit.ContactRecord { + func saveContact( + publicKey: String, + label: String?, + receiverPaths: [String]? = nil, + restorePrivateConnection: Bool = false + ) async throws -> Paykit.ContactRecord { try await withStateRevisionTracking { sdk in - let existingPaths = try await sdk.contactRecord(publicKey: publicKey)?.receiverPaths ?? [] + let existing = try await sdk.contactRecord(publicKey: publicKey) + guard restorePrivateConnection || existing != nil else { throw PubkyServiceError.profileNotFound } + let existingPaths = existing?.receiverPaths ?? [] let contactPaths = Self.mergedReceiverPaths(existingPaths + (receiverPaths ?? [])) - return try await sdk.saveContact(update: Paykit.ContactUpdate(publicKey: publicKey, receiverPaths: contactPaths, label: label)) + let record = try await sdk.saveContact(update: Paykit.ContactUpdate(publicKey: publicKey, receiverPaths: contactPaths, label: label)) + if restorePrivateConnection { + for peer in try await sdk.linkedPeers() where peer.state == .blocked && PubkyPublicKeyFormat.matches(peer.counterparty, publicKey) { + _ = try await sdk.unblockPeer(counterparty: peer.counterparty, counterpartyReceiverPath: peer.counterpartyReceiverPath) + } + } + return record } } func removeContact(publicKey: String) async throws -> Paykit.ContactRecord? { try await withStateRevisionTracking { sdk in - try await sdk.removeContact(publicKey: publicKey) + let record = try await sdk.contactRecord(publicKey: publicKey) + let peers = try await sdk.linkedPeers().filter { PubkyPublicKeyFormat.matches($0.counterparty, publicKey) } + let receiverPaths = Set(record?.receiverPaths ?? []).union(peers.map(\.counterpartyReceiverPath)) + for receiverPath in receiverPaths.sorted() { + _ = try await sdk.blockPeer(counterparty: publicKey, counterpartyReceiverPath: receiverPath) + } + return try await sdk.removeContact(publicKey: publicKey) } } @@ -747,9 +776,14 @@ actor PaykitSdkService { func clearPrivatePaymentList( to counterparty: String, receiverPath: String - ) async throws -> PrivatePaymentListDeliveryReport { + ) async throws -> PrivatePaymentListDeliveryReport? { try await withStateRevisionTracking { sdk in - try await sdk.clearPrivatePaymentListAndProcessOutbound(counterparty: counterparty, counterpartyReceiverPath: receiverPath) + if try await sdk.linkedPeers().contains(where: { + $0.state == .blocked && PubkyPublicKeyFormat.matches($0.counterparty, counterparty) && $0.counterpartyReceiverPath == receiverPath + }) { + return nil + } + return try await sdk.clearPrivatePaymentListAndProcessOutbound(counterparty: counterparty, counterpartyReceiverPath: receiverPath) } } @@ -958,7 +992,7 @@ actor PaykitSdkService { return sdk } - let created = try PaykitSdk.withPaymentAdapterAndPubkyClientConfig( + let created = try sdkFactory?() ?? PaykitSdk.withPaymentAdapterAndPubkyClientConfig( stateStore: stateStore, sessionProvider: sessionProvider, paymentAdapter: paymentAdapter, diff --git a/BitkitTests/PaykitContactLifecycleTests.swift b/BitkitTests/PaykitContactLifecycleTests.swift new file mode 100644 index 000000000..cf9411bbf --- /dev/null +++ b/BitkitTests/PaykitContactLifecycleTests.swift @@ -0,0 +1,111 @@ +@testable import Bitkit +import Paykit +import XCTest + +@MainActor +final class PaykitContactLifecycleTests: XCTestCase { + func testDeletionBlocksEveryKnownReceiverBeforeRemovingContact() async throws { + let sdk = ContactLifecycleSdk(noPointer: .init()) + let service = PaykitSdkService(sdkFactory: { sdk }) + _ = try await service.removeContact(publicKey: sdk.publicKey) + XCTAssertNil(sdk.record) + XCTAssertEqual(sdk.events, ["block:bitkit/server", "block:bitkit/wallet", "remove"]) + XCTAssertTrue(sdk.peers.allSatisfy { $0.state == .blocked }) + } + + func testFailedBlockKeepsContactAvailableForDeletionRetry() async throws { + let sdk = ContactLifecycleSdk(noPointer: .init()) + sdk.failBlock = true + let service = PaykitSdkService(sdkFactory: { sdk }) + do { + _ = try await service.removeContact(publicKey: sdk.publicKey) + XCTFail("Expected the failed block to prevent deletion") + } catch {} + XCTAssertNotNil(sdk.record) + XCTAssertFalse(sdk.events.contains("remove")) + } + + func testOnlyExplicitReaddRestoresAllPrivateConnections() async throws { + let sdk = ContactLifecycleSdk(noPointer: .init()) + let service = PaykitSdkService(sdkFactory: { sdk }) + _ = try await service.removeContact(publicKey: sdk.publicKey) + do { + _ = try await service.saveContact(publicKey: sdk.publicKey, label: "Updated") + XCTFail("Background refresh must not recreate a deleted contact") + } catch {} + XCTAssertNil(sdk.record) + XCTAssertTrue(sdk.peers.allSatisfy { $0.state == .blocked }) + _ = try await service.saveContact(publicKey: sdk.publicKey, label: "Readded", restorePrivateConnection: true) + XCTAssertTrue(sdk.peers.allSatisfy { $0.state == .notLinked }) + XCTAssertEqual(sdk.record?.label, "Readded") + } + + func testBlockedPeerCleanupDoesNotAttemptNetworkDelivery() async throws { + let sdk = ContactLifecycleSdk(noPointer: .init()) + let service = PaykitSdkService(sdkFactory: { sdk }) + _ = try await service.removeContact(publicKey: sdk.publicKey) + let report = try await service.clearPrivatePaymentList(to: sdk.publicKey, receiverPath: PaykitReceiverPath.server) + XCTAssertNil(report) + } +} + +private final class ContactLifecycleSdk: PaykitSdk, @unchecked Sendable { + let publicKey = "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xg" + var events: [String] = [] + var failBlock = false + lazy var record: ContactRecord? = ContactRecord( + publicKey: publicKey, receiverPaths: [PaykitReceiverPath.wallet], label: "Contact", profile: nil, + profileFetchedAt: nil, createdAt: "2026-09-29T00:00:00Z", updatedAt: "2026-09-29T00:00:00Z", + publicContactMarkerStatus: .notPublished, publicContactMarkerReceiverPath: nil, + publicContactPublishedAt: nil, publicContactRemovedAt: nil, publicContactLastError: nil + ) + lazy var peers: [LinkedPeerRecord] = [PaykitReceiverPath.wallet, PaykitReceiverPath.server].map { + LinkedPeerRecord(counterparty: publicKey, counterpartyReceiverPath: $0, state: .linked, + lastSyncAt: nil, lastPrivateReceiveAt: nil, failureCount: 0, + localRecoveryAttemptId: nil, localRecoveryMarkerCreatedAt: nil, localRecoveryMarkerLastError: nil, + remoteRecoveryAttemptId: nil, remoteRecoveryMarkerObservedAt: nil) + } + + override func backupStateRevision() async throws -> String { + "revision" + } + + override func contactRecord(publicKey: String) async throws -> ContactRecord? { + record + } + + override func linkedPeers() async throws -> [LinkedPeerRecord] { + peers + } + + override func blockPeer(counterparty: String, counterpartyReceiverPath: String) async throws -> LinkedPeerRecord { + if failBlock { throw PubkyServiceError.profileNotFound } + events.append("block:\(counterpartyReceiverPath)") + let index = try XCTUnwrap(peers.firstIndex { $0.counterpartyReceiverPath == counterpartyReceiverPath }) + peers[index].state = .blocked + return peers[index] + } + + override func unblockPeer(counterparty: String, counterpartyReceiverPath: String) async throws -> LinkedPeerRecord { + let index = try XCTUnwrap(peers.firstIndex { $0.counterpartyReceiverPath == counterpartyReceiverPath }) + peers[index].state = .notLinked + return peers[index] + } + + override func removeContact(publicKey: String) async throws -> ContactRecord? { + events.append("remove") + defer { record = nil } + return record + } + + override func saveContact(update: ContactUpdate) async throws -> ContactRecord { + let saved = ContactRecord( + publicKey: update.publicKey, receiverPaths: update.receiverPaths, label: update.label, profile: nil, + profileFetchedAt: nil, createdAt: "2026-09-29T00:00:00Z", updatedAt: "2026-09-29T00:00:00Z", + publicContactMarkerStatus: .notPublished, publicContactMarkerReceiverPath: nil, + publicContactPublishedAt: nil, publicContactRemovedAt: nil, publicContactLastError: nil + ) + record = saved + return saved + } +} diff --git a/BitkitTests/PaykitPaymentRequestServiceTests.swift b/BitkitTests/PaykitPaymentRequestServiceTests.swift index 5ddcac728..8569008c7 100644 --- a/BitkitTests/PaykitPaymentRequestServiceTests.swift +++ b/BitkitTests/PaykitPaymentRequestServiceTests.swift @@ -191,6 +191,50 @@ final class PaykitPaymentRequestServiceTests: XCTestCase { XCTAssertTrue(app.claimContactPaymentContext(second)) } + func testBlockingPeerHidesRequestsFromAnEarlierSnapshot() async throws { + let now = Date(timeIntervalSince1970: 1_800_000_000) + let record = try paymentRequestRecord( + counterparty: "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy", + expiresAt: timestamp(now.addingTimeInterval(60)) + ) + let sdk = PaymentRequestSdkMock(records: [record]) + let manager = paymentRequestManager(sdk: sdk, clock: PaymentRequestTestClock(now)) + await manager.refresh() + XCTAssertEqual(manager.pendingRequests.count, 1) + await sdk.configureRecipients( + peers: [linkedPeer(counterparty: record.counterparty, path: record.counterpartyReceiverPath, state: .blocked)], + receiverPathsByPublicKey: [:] + ) + await manager.refresh() + XCTAssertTrue(manager.pendingRequests.isEmpty) + XCTAssertEqual(manager.historyRequests.map(\.paymentRequestId), [record.paymentRequestId]) + } + + func testBlockingAnAlreadyPresentedAcceptedRequestPreventsPayment() async throws { + let now = Date(timeIntervalSince1970: 1_800_000_000) + let record = try paymentRequestRecord( + counterparty: "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy", + state: .accepted + ) + let sdk = PaymentRequestSdkMock(records: [record]) + let manager = paymentRequestManager(sdk: sdk, clock: PaymentRequestTestClock(now)) + await manager.refresh() + let request = try XCTUnwrap(manager.pendingRequests.first) + await sdk.configureRecipients( + peers: [linkedPeer(counterparty: record.counterparty, path: record.counterpartyReceiverPath, state: .blocked)], + receiverPathsByPublicKey: [:] + ) + var consumed = false + do { + try await manager.prepareForPayment(request) { consumed = true } + XCTFail("Expected the deleted contact's request to be unavailable") + } catch { + XCTAssertEqual(error as? PaykitPaymentRequestError, .requestUnavailable) + } + XCTAssertFalse(consumed) + XCTAssertFalse(manager.isApprovedForPayment(request)) + } + func testRefreshMapsSupportedOneTimeBitcoinRequest() async throws { let now = Date(timeIntervalSince1970: 1_800_000_000) let currentOnchain = PublicPaykitService.MethodId.onchainMethodId(network: Env.network, scriptType: .p2wpkh) diff --git a/changelog.d/next/830.fixed.md b/changelog.d/next/830.fixed.md new file mode 100644 index 000000000..e9f38ab02 --- /dev/null +++ b/changelog.d/next/830.fixed.md @@ -0,0 +1 @@ +Deleting a contact now stops private payment requests until you add that contact again, and background refreshes no longer restore deleted contacts. diff --git a/journeys/payment-requests/README.md b/journeys/payment-requests/README.md index 805f1ea3f..9f58f338a 100644 --- a/journeys/payment-requests/README.md +++ b/journeys/payment-requests/README.md @@ -91,3 +91,5 @@ authenticated Pubky identities, saved as each other's contacts and linked on rec - Saved-contact recipient: `ReviewContactRecipient`. - Terminal feedback: `PaymentRequestUnavailableToast`. - Expiration feedback: `PaymentRequestExpiredToast`. + +`delete-and-readd-contact.xml` uses two Bitkit instances to verify that deleting a contact revokes private requests across restart and that explicitly adding the contact again restores a fresh private connection. It does not send funds. diff --git a/journeys/payment-requests/delete-and-readd-contact.xml b/journeys/payment-requests/delete-and-readd-contact.xml new file mode 100644 index 000000000..273b55ee8 --- /dev/null +++ b/journeys/payment-requests/delete-and-readd-contact.xml @@ -0,0 +1,23 @@ + + + Verifies that deleting a contact stops incoming private Payment Requests until the user explicitly adds the contact again. Requires two authenticated Bitkit instances saved as each other's contacts and linked on receiver path "bitkit/wallet". No payment needs to be sent. + + + On the requester, send the payer contact a Payment Request for 5,000 sats with the note "Before deletion" + On the payer, verify the Payment Request confirmation appears, then close it without paying + On the payer, open Contacts, open the requester's contact, choose Delete Contact, and confirm deletion + Verify the requester is absent from Contacts when the deletion confirmation appears + Leave Contacts and immediately reopen it, wait for the list refresh to finish, and verify the deleted contact does not reappear + Open Payment Requests and verify "Before deletion" has no available Pay action + On the requester, send another Payment Request to the payer with the note "After deletion" + On the payer, return Home and wait for two foreground request polling intervals + Verify no Payment Request confirmation appears for "After deletion" and no payable request from the deleted contact appears in Payment Requests + Restart the payer app, return Home, and wait for two foreground request polling intervals + Verify requests from the deleted contact remain unavailable for payment + On the payer, explicitly add the requester's Pubky key as a contact again + Keep both apps in the foreground until the private connection is established again + On the requester, send a new Payment Request with the note "After readd" + On the payer, open "After readd" from Payment Requests and verify its Payment Request confirmation shows the saved contact name + Close the Payment Request confirmation without paying + + From 75e8c9c94ab0338c88b0654df28e77af8b91c6f3 Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 29 Sep 2026 10:16:23 -0500 Subject: [PATCH 2/6] fix: harden contact deletion and payment authorization --- Bitkit/Managers/ContactsManager.swift | 159 ++++++++++-------- .../Localization/en.lproj/Localizable.strings | 1 + .../PaykitPaymentRequestService.swift | 15 +- Bitkit/Services/PubkyService.swift | 28 ++- Bitkit/Views/Contacts/ContactDetailView.swift | 2 + Bitkit/Views/Contacts/EditContactView.swift | 2 + .../Views/Wallets/Send/LnurlPayConfirm.swift | 9 + .../Wallets/Send/SendConfirmationView.swift | 15 ++ Bitkit/Views/Wallets/Send/SendSheet.swift | 7 +- BitkitTests/ContactsManagerTests.swift | 83 +++++++++ BitkitTests/PaykitContactLifecycleTests.swift | 92 +++++++++- .../PaykitPaymentRequestServiceTests.swift | 30 +++- journeys/payment-requests/README.md | 2 + .../delete-and-readd-contact.xml | 4 +- ...elete-contact-with-active-subscription.xml | 16 ++ 15 files changed, 378 insertions(+), 87 deletions(-) create mode 100644 journeys/payment-requests/delete-contact-with-active-subscription.xml diff --git a/Bitkit/Managers/ContactsManager.swift b/Bitkit/Managers/ContactsManager.swift index a51c67d97..0550417cf 100644 --- a/Bitkit/Managers/ContactsManager.swift +++ b/Bitkit/Managers/ContactsManager.swift @@ -115,6 +115,12 @@ struct ContactSection: Identifiable { @MainActor class ContactsManager: ObservableObject { private var contactsRevision = 0 + private var loadGeneration = 0 + private let contactRecords: @Sendable () async throws -> [ContactRecord] + + init(contactRecords: @escaping @Sendable () async throws -> [ContactRecord] = PubkyService.contactRecords) { + self.contactRecords = contactRecords + } @Published var contacts: [PubkyContact] = [] { didSet { contactsRevision += 1 } @@ -141,6 +147,7 @@ class ContactsManager: ObservableObject { } func reset() { + loadGeneration += 1 contacts = [] isLoading = false hasLoaded = false @@ -178,101 +185,107 @@ class ContactsManager: ObservableObject { return } - let revision = contactsRevision + loadGeneration += 1 + let generation = loadGeneration isLoading = true loadErrorMessage = nil - defer { isLoading = false } + defer { + if generation == loadGeneration { isLoading = false } + } Logger.info("Loading contacts for \(PubkyPublicKeyFormat.redacted(publicKey))", context: "ContactsManager") - do { - let records = try await Task.detached { - try await PubkyService.contactRecords() - }.value - - Logger.debug("Loaded \(records.count) SDK contact records", context: "ContactsManager") + while generation == loadGeneration { + try Task.checkCancellation() + let revision = contactsRevision + do { + let records = try await contactRecords() + + Logger.debug("Loaded \(records.count) SDK contact records", context: "ContactsManager") + + let loadedResult: (contacts: [PubkyContact], failures: Int, + missingFailures: Int, firstError: Error?) = await withTaskGroup(of: Result.self) { group in + let overrides = Self.loadContactProfileOverrides() + for record in records { + group.addTask { + do { + let contact = try await Self.contact(from: record, overrides: overrides, includePlaceholder: true) + return .success(contact) + } catch { + Logger.warn( + "Failed to load contact data for '\(PubkyPublicKeyFormat.redacted(record.publicKey))': \(error)", + context: "ContactsManager" + ) + return .failure(error) + } + } + } - let loadedResult: (contacts: [PubkyContact], failures: Int, - missingFailures: Int, firstError: Error?) = await withTaskGroup(of: Result.self) { group in - let overrides = Self.loadContactProfileOverrides() - for record in records { - group.addTask { - do { - let contact = try await Self.contact(from: record, overrides: overrides, includePlaceholder: true) - return .success(contact) - } catch { - Logger.warn( - "Failed to load contact data for '\(PubkyPublicKeyFormat.redacted(record.publicKey))': \(error)", - context: "ContactsManager" - ) - return .failure(error) + var results: [PubkyContact] = [] + var failures = 0 + var missingFailures = 0 + var firstError: Error? + + for await result in group { + switch result { + case let .success(contact): + results.append(contact) + case let .failure(error): + failures += 1 + if Self.isMissingContactsDataError(error) { + missingFailures += 1 + } + firstError = firstError ?? error } } + + return (results, failures, missingFailures, firstError) } - var results: [PubkyContact] = [] - var failures = 0 - var missingFailures = 0 - var firstError: Error? + guard contactsRevision == revision else { continue } - for await result in group { - switch result { - case let .success(contact): - results.append(contact) - case let .failure(error): - failures += 1 - if Self.isMissingContactsDataError(error) { - missingFailures += 1 - } - firstError = firstError ?? error + if !records.isEmpty, loadedResult.contacts.isEmpty { + if loadedResult.failures == loadedResult.missingFailures { + contacts = [] + hasLoaded = true + await PrivatePaykitService.shared.pruneUnsavedContactState(savedPublicKeys: []) + Logger.info("Contacts storage entries were missing, treating list as empty", context: "ContactsManager") + return } + throw loadedResult.firstError ?? PubkyServiceError.profileNotFound } - return (results, failures, missingFailures, firstError) - } + contacts = loadedResult.contacts.sorted { $0.displayName.localizedCaseInsensitiveCompare($1.displayName) == .orderedAscending } + hasLoaded = true + await PrivatePaykitService.shared + .pruneUnsavedContactState(savedPublicKeys: records.compactMap { PubkyPublicKeyFormat.normalized($0.publicKey) }) - guard contactsRevision == revision else { return } + if loadedResult.failures > 0 { + Logger.warn( + "Skipped \(loadedResult.failures) unreadable contacts while loading list", + context: "ContactsManager" + ) + } - if !records.isEmpty, loadedResult.contacts.isEmpty { - if loadedResult.failures == loadedResult.missingFailures { - await PrivatePaykitService.shared.pruneUnsavedContactState(savedPublicKeys: []) + Logger.info("Loaded \(contacts.count) contacts", context: "ContactsManager") + return + } catch { + guard contactsRevision == revision else { continue } + if Self.isMissingContactsDataError(error) { contacts = [] hasLoaded = true - Logger.info("Contacts storage entries were missing, treating list as empty", context: "ContactsManager") + loadErrorMessage = nil + await PrivatePaykitService.shared.pruneUnsavedContactState(savedPublicKeys: []) + Logger.info("Contacts storage missing, treating list as empty", context: "ContactsManager") return } - throw loadedResult.firstError ?? PubkyServiceError.profileNotFound - } - - contacts = loadedResult.contacts.sorted { $0.displayName.localizedCaseInsensitiveCompare($1.displayName) == .orderedAscending } - await PrivatePaykitService.shared - .pruneUnsavedContactState(savedPublicKeys: records.compactMap { PubkyPublicKeyFormat.normalized($0.publicKey) }) - hasLoaded = true - if loadedResult.failures > 0 { - Logger.warn( - "Skipped \(loadedResult.failures) unreadable contacts while loading list", - context: "ContactsManager" - ) - } - - Logger.info("Loaded \(contacts.count) contacts", context: "ContactsManager") - } catch { - guard contactsRevision == revision else { return } - if Self.isMissingContactsDataError(error) { - await PrivatePaykitService.shared.pruneUnsavedContactState(savedPublicKeys: []) - contacts = [] - hasLoaded = true - loadErrorMessage = nil - Logger.info("Contacts storage missing, treating list as empty", context: "ContactsManager") - return - } - - Logger.error("Failed to load contacts: \(error)", context: "ContactsManager") - if contacts.isEmpty { - loadErrorMessage = error.localizedDescription + Logger.error("Failed to load contacts: \(error)", context: "ContactsManager") + if contacts.isEmpty { + loadErrorMessage = error.localizedDescription + } + throw error } - throw error } } diff --git a/Bitkit/Resources/Localization/en.lproj/Localizable.strings b/Bitkit/Resources/Localization/en.lproj/Localizable.strings index 047164255..3003bf797 100644 --- a/Bitkit/Resources/Localization/en.lproj/Localizable.strings +++ b/Bitkit/Resources/Localization/en.lproj/Localizable.strings @@ -1127,6 +1127,7 @@ "contacts__import_select_none" = "Select none"; "contacts__import_friends_count" = "{count} friends"; "contacts__import_selected_count" = "{count} selected"; +"contacts__delete_active_subscription" = "End active subscriptions before deleting this contact."; "contacts__delete_title" = "Delete {name}?"; "contacts__delete_description" = "Are you sure you want to delete {name} from your contacts?"; "contacts__delete_confirm" = "Yes, Delete"; diff --git a/Bitkit/Services/PaykitPaymentRequestService.swift b/Bitkit/Services/PaykitPaymentRequestService.swift index 0966e6f09..d38d8b798 100644 --- a/Bitkit/Services/PaykitPaymentRequestService.swift +++ b/Bitkit/Services/PaykitPaymentRequestService.swift @@ -1292,18 +1292,31 @@ final class PaykitPaymentRequestManager { refreshTask = nil } + func ensurePaymentAllowed(_ request: PaykitPaymentRequest) async throws { + do { + try await service.ensurePaymentAllowed(request) + } catch { + approvedPaymentRequestIds.remove(request.id) + throw error + } + } + func prepareForPayment( _ request: PaykitPaymentRequest, consumePrivatePaymentList: () async throws -> Void = {} ) async throws { do { + if isApprovedForPayment(request) { + try await ensurePaymentAllowed(request) + return + } try await perform( request, resultingState: .accepted, markApprovedForPayment: true, preservePending: !request.requiresAcceptance ) { - try await service.ensurePaymentAllowed($0) + try await ensurePaymentAllowed($0) try await consumePrivatePaymentList() if $0.requiresAcceptance { try await service.accept($0) diff --git a/Bitkit/Services/PubkyService.swift b/Bitkit/Services/PubkyService.swift index 534403bb2..0c5dfc990 100644 --- a/Bitkit/Services/PubkyService.swift +++ b/Bitkit/Services/PubkyService.swift @@ -9,6 +9,7 @@ enum PubkyServiceError: LocalizedError { case sessionNotActive case authFailed(String) case profileNotFound + case activeSubscription var errorDescription: String? { switch self { @@ -20,6 +21,8 @@ enum PubkyServiceError: LocalizedError { return "Authentication failed: \(reason)" case .profileNotFound: return "Profile not found" + case .activeSubscription: + return "Contact has an active subscription" } } } @@ -619,13 +622,12 @@ actor PaykitSdkService { guard restorePrivateConnection || existing != nil else { throw PubkyServiceError.profileNotFound } let existingPaths = existing?.receiverPaths ?? [] let contactPaths = Self.mergedReceiverPaths(existingPaths + (receiverPaths ?? [])) - let record = try await sdk.saveContact(update: Paykit.ContactUpdate(publicKey: publicKey, receiverPaths: contactPaths, label: label)) if restorePrivateConnection { for peer in try await sdk.linkedPeers() where peer.state == .blocked && PubkyPublicKeyFormat.matches(peer.counterparty, publicKey) { _ = try await sdk.unblockPeer(counterparty: peer.counterparty, counterpartyReceiverPath: peer.counterpartyReceiverPath) } } - return record + return try await sdk.saveContact(update: Paykit.ContactUpdate(publicKey: publicKey, receiverPaths: contactPaths, label: label)) } } @@ -634,6 +636,28 @@ actor PaykitSdkService { let record = try await sdk.contactRecord(publicKey: publicKey) let peers = try await sdk.linkedPeers().filter { PubkyPublicKeyFormat.matches($0.counterparty, publicKey) } let receiverPaths = Set(record?.receiverPaths ?? []).union(peers.map(\.counterpartyReceiverPath)) + let now = Date() + let hasActiveSubscription = try await sdk.paymentRequests().contains { + PubkyPublicKeyFormat.matches($0.counterparty, publicKey) && + $0.state == .activeRecurring && + ($0.terms?.recurrence?.endsAt.flatMap(PaykitPaymentRequest.parseDate).map { $0 > now } ?? true) + } + guard !hasActiveSubscription else { throw PubkyServiceError.activeSubscription } + for peer in peers where peer.state == .linked { + do { + let report = try await sdk.clearPrivatePaymentListAndProcessOutbound( + counterparty: publicKey, + counterpartyReceiverPath: peer.counterpartyReceiverPath + ) + if !report.failedToQueue.isEmpty || !report.failedToDeliver.isEmpty { + Logger.warn("Failed to withdraw private endpoints before contact deletion", context: "PaykitSdkService") + } + } catch is CancellationError { + throw CancellationError() + } catch { + Logger.warn("Failed to withdraw private endpoints before contact deletion: \(error)", context: "PaykitSdkService") + } + } for receiverPath in receiverPaths.sorted() { _ = try await sdk.blockPeer(counterparty: publicKey, counterpartyReceiverPath: receiverPath) } diff --git a/Bitkit/Views/Contacts/ContactDetailView.swift b/Bitkit/Views/Contacts/ContactDetailView.swift index b33e006b8..ddca91ed1 100644 --- a/Bitkit/Views/Contacts/ContactDetailView.swift +++ b/Bitkit/Views/Contacts/ContactDetailView.swift @@ -266,6 +266,8 @@ struct ContactDetailView: View { accessibilityIdentifier: "ContactDeletedToast" ) navigation.path = [.contacts] + } catch PubkyServiceError.activeSubscription { + app.toast(type: .error, title: t("contacts__delete_active_subscription")) } catch { Logger.error("Failed to delete contact: \(error)", context: "ContactDetailView") app.toast(type: .error, title: t("contacts__delete_error")) diff --git a/Bitkit/Views/Contacts/EditContactView.swift b/Bitkit/Views/Contacts/EditContactView.swift index 09e4aa1c5..958e63563 100644 --- a/Bitkit/Views/Contacts/EditContactView.swift +++ b/Bitkit/Views/Contacts/EditContactView.swift @@ -129,6 +129,8 @@ struct EditContactView: View { accessibilityIdentifier: "ContactDeletedToast" ) navigation.path = [.contacts] + } catch PubkyServiceError.activeSubscription { + app.toast(type: .error, title: t("contacts__delete_active_subscription")) } catch { Logger.error("Failed to delete contact: \(error)", context: "EditContactView") app.toast(type: .error, title: t("contacts__delete_error")) diff --git a/Bitkit/Views/Wallets/Send/LnurlPayConfirm.swift b/Bitkit/Views/Wallets/Send/LnurlPayConfirm.swift index f1fc69715..db7f2a988 100644 --- a/Bitkit/Views/Wallets/Send/LnurlPayConfirm.swift +++ b/Bitkit/Views/Wallets/Send/LnurlPayConfirm.swift @@ -3,6 +3,7 @@ import LDKNode import SwiftUI struct LnurlPayConfirm: View { + @Environment(PaykitPaymentRequestManager.self) private var paykitPaymentRequestManager @EnvironmentObject var app: AppViewModel @EnvironmentObject var sheets: SheetViewModel @EnvironmentObject var wallet: WalletViewModel @@ -283,6 +284,14 @@ struct LnurlPayConfirm: View { paymentHash: paymentHash ) } + if let incomingPaymentRequest { + do { + try await paykitPaymentRequestManager.ensurePaymentAllowed(incomingPaymentRequest) + } catch { + await PaykitPaymentProofService.shared.failLightningPayment(paymentHash: paymentHash) + throw error + } + } lightningPaymentHash = paymentHash // Perform the Lightning payment (10s timeout → navigate to pending for hold invoices) diff --git a/Bitkit/Views/Wallets/Send/SendConfirmationView.swift b/Bitkit/Views/Wallets/Send/SendConfirmationView.swift index 8f774e2e5..d675e87dd 100644 --- a/Bitkit/Views/Wallets/Send/SendConfirmationView.swift +++ b/Bitkit/Views/Wallets/Send/SendConfirmationView.swift @@ -3,6 +3,7 @@ import LDKNode import SwiftUI struct SendConfirmationView: View { + @Environment(PaykitPaymentRequestManager.self) private var paykitPaymentRequestManager @EnvironmentObject var app: AppViewModel @EnvironmentObject var activityList: ActivityListViewModel @EnvironmentObject var contactsManager: ContactsManager @@ -839,6 +840,14 @@ struct SendConfirmationView: View { // For invoices with a built-in amount, pass sats: nil so LDK uses the invoice's // native millisatoshi precision instead of our truncated satoshi value. let paymentSats: UInt64? = invoice.amountSatoshis == 0 ? amount : nil + if let incomingPaymentRequest { + do { + try await paykitPaymentRequestManager.ensurePaymentAllowed(incomingPaymentRequest) + } catch { + await PaykitPaymentProofService.shared.failLightningPayment(paymentHash: paymentHash) + throw error + } + } do { try await wallet.sendWithTimeout( bolt11: invoice.bolt11, @@ -889,6 +898,12 @@ struct SendConfirmationView: View { incomingPaymentRequest, address: invoice.address ) + do { + try await paykitPaymentRequestManager.ensurePaymentAllowed(incomingPaymentRequest) + } catch { + await PaykitPaymentProofService.shared.failOnchainPayment(incomingPaymentRequest) + throw error + } onchainPaymentStarted = true } } diff --git a/Bitkit/Views/Wallets/Send/SendSheet.swift b/Bitkit/Views/Wallets/Send/SendSheet.swift index dcdd01f62..9651cd916 100644 --- a/Bitkit/Views/Wallets/Send/SendSheet.swift +++ b/Bitkit/Views/Wallets/Send/SendSheet.swift @@ -774,7 +774,6 @@ struct SendSheet: View { guard let context = app.contactPaymentContext, let request = context.incomingPaymentRequest else { return } - guard !paykitPaymentRequestManager.isApprovedForPayment(request) else { return } try await paykitPaymentRequestManager.prepareForPayment(request) { guard let privatePaymentContext = context.privatePaymentContext else { return } @@ -802,6 +801,12 @@ struct SendSheet: View { do { try await prepareIncomingPaymentRequest() try await PaykitPaymentProofService.shared.markOnchainPaymentStarted(request, address: address) + do { + try await paykitPaymentRequestManager.ensurePaymentAllowed(request) + } catch { + await PaykitPaymentProofService.shared.failOnchainPayment(request) + throw error + } } catch { await PaykitPaymentProofService.shared.cancelPreparation(request) throw error diff --git a/BitkitTests/ContactsManagerTests.swift b/BitkitTests/ContactsManagerTests.swift index 09ae6b7f1..4f3102235 100644 --- a/BitkitTests/ContactsManagerTests.swift +++ b/BitkitTests/ContactsManagerTests.swift @@ -1,5 +1,6 @@ @testable import Bitkit import BitkitCore +import Paykit import XCTest @MainActor @@ -11,6 +12,59 @@ final class ContactsManagerTests: XCTestCase { UserDefaults.standard.set(false, forKey: PaykitFeatureFlags.uiEnabledKey) } + func testInitialLoadPreservesUnchangedContactsAfterLocalMutations() async throws { + for deletesContact in [true, false] { + let first = contactRecord(key: "pubky" + String(repeating: "y", count: 52), name: "First") + let second = contactRecord(key: "pubky" + String(repeating: "z", count: 52), name: "Second") + let added = contactRecord(key: "pubky" + String(repeating: "r", count: 52), name: "Added") + let source = SuspendedContactRecords(records: [first, second]) + let manager = ContactsManager(contactRecords: { await source.load() }) + let load = Task { try await manager.loadContacts(for: "owner") } + while await !(source.isPaused) { + await Task.yield() + } + let expected: [ContactRecord] + if deletesContact { + manager.contacts.removeAll { $0.publicKey == second.publicKey } + expected = [first] + } else { + manager.contacts.append(makeContact(publicKey: added.publicKey)) + expected = [first, second, added] + } + await source.resume(with: expected) + try await load.value + XCTAssertEqual(Set(manager.contacts.map(\.publicKey)), Set(expected.map(\.publicKey))) + XCTAssertTrue(manager.hasLoaded) + XCTAssertFalse(manager.isLoading) + } + } + + func testResetStopsAnInvalidatedContactLoad() async throws { + let record = contactRecord(key: "pubky" + String(repeating: "y", count: 52), name: "Contact") + let source = SuspendedContactRecords(records: [record]) + let manager = ContactsManager(contactRecords: { await source.load() }) + let load = Task { try await manager.loadContacts(for: "owner") } + while await !(source.isPaused) { + await Task.yield() + } + manager.reset() + await source.resume(with: [record]) + try await load.value + XCTAssertTrue(manager.contacts.isEmpty) + XCTAssertFalse(manager.hasLoaded) + XCTAssertFalse(manager.isLoading) + } + + private func contactRecord(key: String, name: String) -> ContactRecord { + ContactRecord( + publicKey: key, receiverPaths: [PaykitReceiverPath.wallet], label: name, + profile: PaykitProfile(displayName: name, imageUri: nil, extraJson: nil), + profileFetchedAt: nil, createdAt: "2026-09-29T00:00:00Z", updatedAt: "2026-09-29T00:00:00Z", + publicContactMarkerStatus: .notPublished, publicContactMarkerReceiverPath: nil, + publicContactPublishedAt: nil, publicContactRemovedAt: nil, publicContactLastError: nil + ) + } + func testPubkyPublicKeyFormatNormalizesPrefixedAndUnprefixedKeys() { let rawKey = "3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xg" let prefixedKey = "pubky\(rawKey)" @@ -372,3 +426,32 @@ final class ContactsManagerTests: XCTestCase { Bitkit.PubkyContact(publicKey: publicKey, profile: makeProfile(publicKey: publicKey)) } } + +private actor SuspendedContactRecords { + private var records: [ContactRecord] + private var continuation: CheckedContinuation? + private var shouldPause = true + + var isPaused: Bool { + continuation != nil + } + + init(records: [ContactRecord]) { + self.records = records + } + + func load() async -> [ContactRecord] { + let snapshot = records + if shouldPause { + shouldPause = false + await withCheckedContinuation { continuation = $0 } + } + return snapshot + } + + func resume(with records: [ContactRecord]) { + self.records = records + continuation?.resume() + continuation = nil + } +} diff --git a/BitkitTests/PaykitContactLifecycleTests.swift b/BitkitTests/PaykitContactLifecycleTests.swift index cf9411bbf..47845519e 100644 --- a/BitkitTests/PaykitContactLifecycleTests.swift +++ b/BitkitTests/PaykitContactLifecycleTests.swift @@ -5,12 +5,51 @@ import XCTest @MainActor final class PaykitContactLifecycleTests: XCTestCase { func testDeletionBlocksEveryKnownReceiverBeforeRemovingContact() async throws { - let sdk = ContactLifecycleSdk(noPointer: .init()) - let service = PaykitSdkService(sdkFactory: { sdk }) - _ = try await service.removeContact(publicKey: sdk.publicKey) - XCTAssertNil(sdk.record) - XCTAssertEqual(sdk.events, ["block:bitkit/server", "block:bitkit/wallet", "remove"]) - XCTAssertTrue(sdk.peers.allSatisfy { $0.state == .blocked }) + for failWithdrawal in [false, true] { + let sdk = ContactLifecycleSdk(noPointer: .init()) + sdk.failWithdrawal = failWithdrawal + let service = PaykitSdkService(sdkFactory: { sdk }) + _ = try await service.removeContact(publicKey: sdk.publicKey) + XCTAssertNil(sdk.record) + XCTAssertEqual(sdk.events, ["clear:bitkit/wallet", "clear:bitkit/server", "block:bitkit/server", "block:bitkit/wallet", "remove"]) + XCTAssertTrue(sdk.peers.allSatisfy { $0.state == .blocked }) + } + } + + func testActiveSubscriptionPreventsDeletionUntilItEnds() async throws { + for (role, endsNaturally) in [(PaymentRequestLocalRole.payer, false), (.payer, true), (.payee, false)] { + let sdk = ContactLifecycleSdk(noPointer: .init()) + let terms = try PaymentRequestTerms( + amount: PaymentRequestAmount(value: "0.001", asset: "btc"), + paymentReference: PaymentReference(text: "subscription"), proposalExpiresAt: nil, + recurrence: PaymentRequestRecurrence(every: 1, unit: "month", startsAt: "2026-01-01T00:00:00Z", + anchor: "2026-01-01T00:00:00Z", endsAt: nil), + acceptedPaymentEndpointIdentifiers: ["lightning:bolt11"], metadata: PrivateJsonObject(text: "{}") + ) + sdk.requests = [PaymentRequestRecord( + counterparty: sdk.publicKey, counterpartyReceiverPath: PaykitReceiverPath.server, + paymentRequestId: "550e8400-e29b-41d4-a716-446655440000", localRole: role, state: .activeRecurring, + proposalStreamItemId: nil, proposalOutboundMessageId: nil, proposalOutboundStatus: nil, + proposalEventId: nil, terms: terms, acceptedEventId: nil, acceptedOutboundStatus: nil, + rejectedEventId: nil, rejectedOutboundStatus: nil, canceledEventId: nil, canceledOutboundStatus: nil, + paymentProofs: [], lastStreamItemId: nil, lastOutboundMessageId: nil, lastOutboundStatus: nil, + lastEventAt: nil, invalidReason: nil + )] + let service = PaykitSdkService(sdkFactory: { sdk }) + do { + _ = try await service.removeContact(publicKey: sdk.publicKey) + XCTFail("Expected active subscription to prevent deletion") + } catch PubkyServiceError.activeSubscription {} + XCTAssertNotNil(sdk.record) + XCTAssertTrue(sdk.events.isEmpty) + if endsNaturally { + sdk.requests[0].terms?.recurrence?.endsAt = "2026-02-01T00:00:00Z" + } else { + sdk.requests[0].state = .canceled + } + _ = try await service.removeContact(publicKey: sdk.publicKey) + XCTAssertNil(sdk.record) + } } func testFailedBlockKeepsContactAvailableForDeletionRetry() async throws { @@ -40,6 +79,26 @@ final class PaykitContactLifecycleTests: XCTestCase { XCTAssertEqual(sdk.record?.label, "Readded") } + func testFailedPrivateConnectionRestoreCanBeRetriedWithoutASavedContact() async throws { + for failPeerLookup in [true, false] { + let sdk = ContactLifecycleSdk(noPointer: .init()) + let service = PaykitSdkService(sdkFactory: { sdk }) + _ = try await service.removeContact(publicKey: sdk.publicKey) + sdk.failLinkedPeers = failPeerLookup + sdk.failUnblockPath = failPeerLookup ? nil : PaykitReceiverPath.server + do { + _ = try await service.saveContact(publicKey: sdk.publicKey, label: "Contact", restorePrivateConnection: true) + XCTFail("Expected restoration to fail") + } catch {} + XCTAssertNil(sdk.record) + sdk.failLinkedPeers = false + sdk.failUnblockPath = nil + _ = try await service.saveContact(publicKey: sdk.publicKey, label: "Contact", restorePrivateConnection: true) + XCTAssertNotNil(sdk.record) + XCTAssertTrue(sdk.peers.allSatisfy { $0.state == .notLinked }) + } + } + func testBlockedPeerCleanupDoesNotAttemptNetworkDelivery() async throws { let sdk = ContactLifecycleSdk(noPointer: .init()) let service = PaykitSdkService(sdkFactory: { sdk }) @@ -52,7 +111,11 @@ final class PaykitContactLifecycleTests: XCTestCase { private final class ContactLifecycleSdk: PaykitSdk, @unchecked Sendable { let publicKey = "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xg" var events: [String] = [] + var requests: [PaymentRequestRecord] = [] + var failWithdrawal = false var failBlock = false + var failLinkedPeers = false + var failUnblockPath: String? lazy var record: ContactRecord? = ContactRecord( publicKey: publicKey, receiverPaths: [PaykitReceiverPath.wallet], label: "Contact", profile: nil, profileFetchedAt: nil, createdAt: "2026-09-29T00:00:00Z", updatedAt: "2026-09-29T00:00:00Z", @@ -70,12 +133,26 @@ private final class ContactLifecycleSdk: PaykitSdk, @unchecked Sendable { "revision" } + override func paymentRequests() async throws -> [PaymentRequestRecord] { + requests + } + override func contactRecord(publicKey: String) async throws -> ContactRecord? { record } override func linkedPeers() async throws -> [LinkedPeerRecord] { - peers + if failLinkedPeers { throw PubkyServiceError.profileNotFound } + return peers + } + + override func clearPrivatePaymentListAndProcessOutbound( + counterparty: String, + counterpartyReceiverPath: String + ) async throws -> PrivatePaymentListDeliveryReport { + events.append("clear:\(counterpartyReceiverPath)") + if failWithdrawal { throw PubkyServiceError.sessionNotActive } + return PrivatePaymentListDeliveryReport(queued: [], cleared: [], failedToQueue: [], failedToDeliver: []) } override func blockPeer(counterparty: String, counterpartyReceiverPath: String) async throws -> LinkedPeerRecord { @@ -87,6 +164,7 @@ private final class ContactLifecycleSdk: PaykitSdk, @unchecked Sendable { } override func unblockPeer(counterparty: String, counterpartyReceiverPath: String) async throws -> LinkedPeerRecord { + if failUnblockPath == counterpartyReceiverPath { throw PubkyServiceError.profileNotFound } let index = try XCTUnwrap(peers.firstIndex { $0.counterpartyReceiverPath == counterpartyReceiverPath }) peers[index].state = .notLinked return peers[index] diff --git a/BitkitTests/PaykitPaymentRequestServiceTests.swift b/BitkitTests/PaykitPaymentRequestServiceTests.swift index 8569008c7..cffd2441e 100644 --- a/BitkitTests/PaykitPaymentRequestServiceTests.swift +++ b/BitkitTests/PaykitPaymentRequestServiceTests.swift @@ -207,7 +207,9 @@ final class PaykitPaymentRequestServiceTests: XCTestCase { ) await manager.refresh() XCTAssertTrue(manager.pendingRequests.isEmpty) - XCTAssertEqual(manager.historyRequests.map(\.paymentRequestId), [record.paymentRequestId]) + await sdk.setRecords([]) + await manager.refresh() + XCTAssertTrue(manager.historyRequests.isEmpty) } func testBlockingAnAlreadyPresentedAcceptedRequestPreventsPayment() async throws { @@ -235,6 +237,32 @@ final class PaykitPaymentRequestServiceTests: XCTestCase { XCTAssertFalse(manager.isApprovedForPayment(request)) } + func testApprovedPaymentRechecksBlockingWithoutRepeatingAcceptance() async throws { + let record = try paymentRequestRecord(counterparty: "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy") + let sdk = PaymentRequestSdkMock(records: [record]) + let manager = paymentRequestManager(sdk: sdk) + await manager.refresh() + let request = try XCTUnwrap(manager.pendingRequests.first) + var consumedCount = 0 + try await manager.prepareForPayment(request) { consumedCount += 1 } + try await manager.prepareForPayment(request) { consumedCount += 1 } + XCTAssertEqual(consumedCount, 1) + let accepted = await sdk.snapshot().acceptedRequests + XCTAssertEqual(accepted.count, 1) + await sdk.configureRecipients( + peers: [linkedPeer(counterparty: record.counterparty, path: record.counterpartyReceiverPath, state: .blocked)], + receiverPathsByPublicKey: [:] + ) + do { + try await manager.prepareForPayment(request) { consumedCount += 1 } + XCTFail("Expected blocked payment to be rejected") + } catch { + XCTAssertEqual(error as? PaykitPaymentRequestError, .requestUnavailable) + } + XCTAssertEqual(consumedCount, 1) + XCTAssertFalse(manager.isApprovedForPayment(request)) + } + func testRefreshMapsSupportedOneTimeBitcoinRequest() async throws { let now = Date(timeIntervalSince1970: 1_800_000_000) let currentOnchain = PublicPaykitService.MethodId.onchainMethodId(network: Env.network, scriptType: .p2wpkh) diff --git a/journeys/payment-requests/README.md b/journeys/payment-requests/README.md index 9f58f338a..04c64ab5d 100644 --- a/journeys/payment-requests/README.md +++ b/journeys/payment-requests/README.md @@ -93,3 +93,5 @@ authenticated Pubky identities, saved as each other's contacts and linked on rec - Expiration feedback: `PaymentRequestExpiredToast`. `delete-and-readd-contact.xml` uses two Bitkit instances to verify that deleting a contact revokes private requests across restart and that explicitly adding the contact again restores a fresh private connection. It does not send funds. + +`delete-contact-with-active-subscription.xml` requires an accepted open-ended payer subscription. It verifies that deletion explains why the contact must stay saved until the subscription ends, then that canceling, deleting, and readding does not revive it. No new payment is sent. Both contact-deletion journeys are mirrored on iOS and Android. diff --git a/journeys/payment-requests/delete-and-readd-contact.xml b/journeys/payment-requests/delete-and-readd-contact.xml index 273b55ee8..e74090701 100644 --- a/journeys/payment-requests/delete-and-readd-contact.xml +++ b/journeys/payment-requests/delete-and-readd-contact.xml @@ -1,6 +1,6 @@ - Verifies that deleting a contact stops incoming private Payment Requests until the user explicitly adds the contact again. Requires two authenticated Bitkit instances saved as each other's contacts and linked on receiver path "bitkit/wallet". No payment needs to be sent. + Verifies that deleting a contact stops incoming private Payment Requests until the user explicitly adds the contact again. Requires two authenticated Bitkit instances saved as each other's contacts and linked on receiver path "bitkit/wallet". The payer must have no active subscription with the requester. No payment needs to be sent. On the requester, send the payer contact a Payment Request for 5,000 sats with the note "Before deletion" @@ -8,7 +8,7 @@ On the payer, open Contacts, open the requester's contact, choose Delete Contact, and confirm deletion Verify the requester is absent from Contacts when the deletion confirmation appears Leave Contacts and immediately reopen it, wait for the list refresh to finish, and verify the deleted contact does not reappear - Open Payment Requests and verify "Before deletion" has no available Pay action + Open Payment Requests and wait for the request list to refresh, and verify "Before deletion" is no longer listed On the requester, send another Payment Request to the payer with the note "After deletion" On the payer, return Home and wait for two foreground request polling intervals Verify no Payment Request confirmation appears for "After deletion" and no payable request from the deleted contact appears in Payment Requests diff --git a/journeys/payment-requests/delete-contact-with-active-subscription.xml b/journeys/payment-requests/delete-contact-with-active-subscription.xml new file mode 100644 index 000000000..2dc3c983d --- /dev/null +++ b/journeys/payment-requests/delete-contact-with-active-subscription.xml @@ -0,0 +1,16 @@ + + + Verifies that an active subscription must end before its contact can be deleted. Requires two authenticated Bitkit instances saved as contacts, with an accepted open-ended subscription on the payer. No new payment needs to be sent. + + + On the payer, open Contacts, open the requester's contact, choose Delete Contact, and confirm deletion + Verify the error "End active subscriptions before deleting this contact." appears and the contact remains saved + Open Subscriptions and verify the active subscription is still visible + Cancel the subscription and verify it is no longer active + Return to the requester's contact, choose Delete Contact, and confirm deletion + Verify the deletion confirmation appears and the requester is absent from Contacts + Explicitly add the requester's Pubky key as a contact again and wait for the private connection to be established + Return Home and wait for two foreground request polling intervals + Verify the canceled subscription remains inactive and no missed subscription payment confirmation appears + + From 171d4491ec4b1f9981644a1b152c84472180200d Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 29 Sep 2026 18:05:32 +0100 Subject: [PATCH 3/6] fix: roll back failed contact re-add --- Bitkit/Services/PubkyService.swift | 20 +++++++++++++++++-- BitkitTests/PaykitContactLifecycleTests.swift | 18 ++++++++++++++--- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/Bitkit/Services/PubkyService.swift b/Bitkit/Services/PubkyService.swift index 0c5dfc990..eb20bf950 100644 --- a/Bitkit/Services/PubkyService.swift +++ b/Bitkit/Services/PubkyService.swift @@ -623,8 +623,24 @@ actor PaykitSdkService { let existingPaths = existing?.receiverPaths ?? [] let contactPaths = Self.mergedReceiverPaths(existingPaths + (receiverPaths ?? [])) if restorePrivateConnection { - for peer in try await sdk.linkedPeers() where peer.state == .blocked && PubkyPublicKeyFormat.matches(peer.counterparty, publicKey) { - _ = try await sdk.unblockPeer(counterparty: peer.counterparty, counterpartyReceiverPath: peer.counterpartyReceiverPath) + let blockedPeers = try await sdk.linkedPeers().filter { + $0.state == .blocked && PubkyPublicKeyFormat.matches($0.counterparty, publicKey) + } + do { + for peer in blockedPeers { + _ = try await sdk.unblockPeer(counterparty: peer.counterparty, counterpartyReceiverPath: peer.counterpartyReceiverPath) + } + return try await sdk.saveContact(update: Paykit.ContactUpdate(publicKey: publicKey, receiverPaths: contactPaths, label: label)) + } catch { + let restorationError = error + for peer in blockedPeers { + do { + _ = try await sdk.blockPeer(counterparty: peer.counterparty, counterpartyReceiverPath: peer.counterpartyReceiverPath) + } catch { + Logger.error("Failed to restore peer block after contact save failed: \(error)", context: "PaykitSdkService") + } + } + throw restorationError } } return try await sdk.saveContact(update: Paykit.ContactUpdate(publicKey: publicKey, receiverPaths: contactPaths, label: label)) diff --git a/BitkitTests/PaykitContactLifecycleTests.swift b/BitkitTests/PaykitContactLifecycleTests.swift index 47845519e..672173fd6 100644 --- a/BitkitTests/PaykitContactLifecycleTests.swift +++ b/BitkitTests/PaykitContactLifecycleTests.swift @@ -80,19 +80,27 @@ final class PaykitContactLifecycleTests: XCTestCase { } func testFailedPrivateConnectionRestoreCanBeRetriedWithoutASavedContact() async throws { - for failPeerLookup in [true, false] { + let failures: [(peerLookup: Bool, unblockPath: String?, saveContact: Bool)] = [ + (true, nil, false), + (false, PaykitReceiverPath.server, false), + (false, nil, true), + ] + for failure in failures { let sdk = ContactLifecycleSdk(noPointer: .init()) let service = PaykitSdkService(sdkFactory: { sdk }) _ = try await service.removeContact(publicKey: sdk.publicKey) - sdk.failLinkedPeers = failPeerLookup - sdk.failUnblockPath = failPeerLookup ? nil : PaykitReceiverPath.server + sdk.failLinkedPeers = failure.peerLookup + sdk.failUnblockPath = failure.unblockPath + sdk.failSaveContact = failure.saveContact do { _ = try await service.saveContact(publicKey: sdk.publicKey, label: "Contact", restorePrivateConnection: true) XCTFail("Expected restoration to fail") } catch {} XCTAssertNil(sdk.record) + XCTAssertTrue(sdk.peers.allSatisfy { $0.state == .blocked }) sdk.failLinkedPeers = false sdk.failUnblockPath = nil + sdk.failSaveContact = false _ = try await service.saveContact(publicKey: sdk.publicKey, label: "Contact", restorePrivateConnection: true) XCTAssertNotNil(sdk.record) XCTAssertTrue(sdk.peers.allSatisfy { $0.state == .notLinked }) @@ -116,6 +124,7 @@ private final class ContactLifecycleSdk: PaykitSdk, @unchecked Sendable { var failBlock = false var failLinkedPeers = false var failUnblockPath: String? + var failSaveContact = false lazy var record: ContactRecord? = ContactRecord( publicKey: publicKey, receiverPaths: [PaykitReceiverPath.wallet], label: "Contact", profile: nil, profileFetchedAt: nil, createdAt: "2026-09-29T00:00:00Z", updatedAt: "2026-09-29T00:00:00Z", @@ -177,6 +186,9 @@ private final class ContactLifecycleSdk: PaykitSdk, @unchecked Sendable { } override func saveContact(update: ContactUpdate) async throws -> ContactRecord { + if failSaveContact { + throw PubkyServiceError.profileNotFound + } let saved = ContactRecord( publicKey: update.publicKey, receiverPaths: update.receiverPaths, label: update.label, profile: nil, profileFetchedAt: nil, createdAt: "2026-09-29T00:00:00Z", updatedAt: "2026-09-29T00:00:00Z", From 8d6221054cb6a6abe6d99dbabfe7175fbc067f21 Mon Sep 17 00:00:00 2001 From: benk10 Date: Tue, 29 Sep 2026 21:56:27 +0100 Subject: [PATCH 4/6] fix: show fixed subscription end date --- Bitkit/Services/PubkyService.swift | 11 +++++++--- Bitkit/Views/Contacts/ContactDetailView.swift | 7 +++++-- Bitkit/Views/Contacts/EditContactView.swift | 7 +++++-- BitkitTests/PaykitContactLifecycleTests.swift | 20 +++++++++++++------ 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/Bitkit/Services/PubkyService.swift b/Bitkit/Services/PubkyService.swift index eb20bf950..845522f7f 100644 --- a/Bitkit/Services/PubkyService.swift +++ b/Bitkit/Services/PubkyService.swift @@ -9,7 +9,7 @@ enum PubkyServiceError: LocalizedError { case sessionNotActive case authFailed(String) case profileNotFound - case activeSubscription + case activeSubscription(endsAt: Date?) var errorDescription: String? { switch self { @@ -653,12 +653,17 @@ actor PaykitSdkService { let peers = try await sdk.linkedPeers().filter { PubkyPublicKeyFormat.matches($0.counterparty, publicKey) } let receiverPaths = Set(record?.receiverPaths ?? []).union(peers.map(\.counterpartyReceiverPath)) let now = Date() - let hasActiveSubscription = try await sdk.paymentRequests().contains { + let activeSubscriptions = try await sdk.paymentRequests().filter { PubkyPublicKeyFormat.matches($0.counterparty, publicKey) && $0.state == .activeRecurring && ($0.terms?.recurrence?.endsAt.flatMap(PaykitPaymentRequest.parseDate).map { $0 > now } ?? true) } - guard !hasActiveSubscription else { throw PubkyServiceError.activeSubscription } + guard activeSubscriptions.isEmpty else { + let latestEndDate = activeSubscriptions.compactMap { + $0.terms?.recurrence?.endsAt.flatMap(PaykitPaymentRequest.parseDate) + }.max() + throw PubkyServiceError.activeSubscription(endsAt: latestEndDate) + } for peer in peers where peer.state == .linked { do { let report = try await sdk.clearPrivatePaymentListAndProcessOutbound( diff --git a/Bitkit/Views/Contacts/ContactDetailView.swift b/Bitkit/Views/Contacts/ContactDetailView.swift index ddca91ed1..78eb09c97 100644 --- a/Bitkit/Views/Contacts/ContactDetailView.swift +++ b/Bitkit/Views/Contacts/ContactDetailView.swift @@ -266,8 +266,11 @@ struct ContactDetailView: View { accessibilityIdentifier: "ContactDeletedToast" ) navigation.path = [.contacts] - } catch PubkyServiceError.activeSubscription { - app.toast(type: .error, title: t("contacts__delete_active_subscription")) + } catch let PubkyServiceError.activeSubscription(endsAt) { + let description = endsAt.map { + t("subscriptions__expires_date", variables: ["date": $0.formatted(date: .long, time: .omitted)]) + } + app.toast(type: .error, title: t("contacts__delete_active_subscription"), description: description) } catch { Logger.error("Failed to delete contact: \(error)", context: "ContactDetailView") app.toast(type: .error, title: t("contacts__delete_error")) diff --git a/Bitkit/Views/Contacts/EditContactView.swift b/Bitkit/Views/Contacts/EditContactView.swift index 958e63563..934d5f3f6 100644 --- a/Bitkit/Views/Contacts/EditContactView.swift +++ b/Bitkit/Views/Contacts/EditContactView.swift @@ -129,8 +129,11 @@ struct EditContactView: View { accessibilityIdentifier: "ContactDeletedToast" ) navigation.path = [.contacts] - } catch PubkyServiceError.activeSubscription { - app.toast(type: .error, title: t("contacts__delete_active_subscription")) + } catch let PubkyServiceError.activeSubscription(endsAt) { + let description = endsAt.map { + t("subscriptions__expires_date", variables: ["date": $0.formatted(date: .long, time: .omitted)]) + } + app.toast(type: .error, title: t("contacts__delete_active_subscription"), description: description) } catch { Logger.error("Failed to delete contact: \(error)", context: "EditContactView") app.toast(type: .error, title: t("contacts__delete_error")) diff --git a/BitkitTests/PaykitContactLifecycleTests.swift b/BitkitTests/PaykitContactLifecycleTests.swift index 672173fd6..2f125b321 100644 --- a/BitkitTests/PaykitContactLifecycleTests.swift +++ b/BitkitTests/PaykitContactLifecycleTests.swift @@ -17,18 +17,24 @@ final class PaykitContactLifecycleTests: XCTestCase { } func testActiveSubscriptionPreventsDeletionUntilItEnds() async throws { - for (role, endsNaturally) in [(PaymentRequestLocalRole.payer, false), (.payer, true), (.payee, false)] { + let fixedEndTimestamp = "2100-02-01T00:00:00Z" + let cases: [(role: PaymentRequestLocalRole, endsAt: String?)] = [ + (.payer, nil), + (.payer, fixedEndTimestamp), + (.payee, nil), + ] + for testCase in cases { let sdk = ContactLifecycleSdk(noPointer: .init()) let terms = try PaymentRequestTerms( amount: PaymentRequestAmount(value: "0.001", asset: "btc"), paymentReference: PaymentReference(text: "subscription"), proposalExpiresAt: nil, recurrence: PaymentRequestRecurrence(every: 1, unit: "month", startsAt: "2026-01-01T00:00:00Z", - anchor: "2026-01-01T00:00:00Z", endsAt: nil), + anchor: "2026-01-01T00:00:00Z", endsAt: testCase.endsAt), acceptedPaymentEndpointIdentifiers: ["lightning:bolt11"], metadata: PrivateJsonObject(text: "{}") ) sdk.requests = [PaymentRequestRecord( counterparty: sdk.publicKey, counterpartyReceiverPath: PaykitReceiverPath.server, - paymentRequestId: "550e8400-e29b-41d4-a716-446655440000", localRole: role, state: .activeRecurring, + paymentRequestId: "550e8400-e29b-41d4-a716-446655440000", localRole: testCase.role, state: .activeRecurring, proposalStreamItemId: nil, proposalOutboundMessageId: nil, proposalOutboundStatus: nil, proposalEventId: nil, terms: terms, acceptedEventId: nil, acceptedOutboundStatus: nil, rejectedEventId: nil, rejectedOutboundStatus: nil, canceledEventId: nil, canceledOutboundStatus: nil, @@ -39,11 +45,13 @@ final class PaykitContactLifecycleTests: XCTestCase { do { _ = try await service.removeContact(publicKey: sdk.publicKey) XCTFail("Expected active subscription to prevent deletion") - } catch PubkyServiceError.activeSubscription {} + } catch let PubkyServiceError.activeSubscription(endsAt) { + XCTAssertEqual(endsAt, testCase.endsAt.flatMap(PaykitPaymentRequest.parseDate)) + } XCTAssertNotNil(sdk.record) XCTAssertTrue(sdk.events.isEmpty) - if endsNaturally { - sdk.requests[0].terms?.recurrence?.endsAt = "2026-02-01T00:00:00Z" + if testCase.endsAt != nil { + sdk.requests[0].terms?.recurrence?.endsAt = "2000-02-01T00:00:00Z" } else { sdk.requests[0].state = .canceled } From a182bae8ce4f2288c569793f348b2ef5102fb19c Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 30 Sep 2026 02:44:34 +0100 Subject: [PATCH 5/6] fix: recheck hardware send authorization on retry --- Bitkit/ViewModels/HwFundingSigner.swift | 8 ++- .../Views/Wallets/Send/HwSendSignView.swift | 4 +- Bitkit/Views/Wallets/Send/SendSheet.swift | 17 +++-- BitkitTests/HwFundingSignerTests.swift | 70 +++++++++++++++++-- .../PaykitPaymentRequestServiceTests.swift | 7 +- 5 files changed, 90 insertions(+), 16 deletions(-) diff --git a/Bitkit/ViewModels/HwFundingSigner.swift b/Bitkit/ViewModels/HwFundingSigner.swift index d941469d4..3ea29b3e0 100644 --- a/Bitkit/ViewModels/HwFundingSigner.swift +++ b/Bitkit/ViewModels/HwFundingSigner.swift @@ -454,7 +454,8 @@ final class HwSendCoordinator { address: String, sats: UInt64, satsPerVByte: UInt64, - beforeBroadcast: @escaping () async throws -> Void = {}, + beforeFirstBroadcast: @escaping () async throws -> Void = {}, + beforeBroadcastAttempt: @escaping () async throws -> Void = {}, afterBroadcast: @escaping (HwFundingBroadcastResult) async -> Void = { _ in } ) async throws -> HwFundingBroadcastResult { guard let walletId else { @@ -486,12 +487,13 @@ final class HwSendCoordinator { } if pendingPayment?.isPreparedForBroadcast != true { - try await beforeBroadcast() + try await beforeFirstBroadcast() pendingPayment?.isPreparedForBroadcast = true } - isBroadcastUnresolved = true do { + try await beforeBroadcastAttempt() + isBroadcastUnresolved = true let result = try await signer.broadcastSignedFunding(signed) await afterBroadcast(result) return result diff --git a/Bitkit/Views/Wallets/Send/HwSendSignView.swift b/Bitkit/Views/Wallets/Send/HwSendSignView.swift index 47d13c8e2..22ca954ae 100644 --- a/Bitkit/Views/Wallets/Send/HwSendSignView.swift +++ b/Bitkit/Views/Wallets/Send/HwSendSignView.swift @@ -10,6 +10,7 @@ struct HwSendSignView: View { @Binding var navigationPath: [SendRoute] let hwSend: HwSendCoordinator let prepareContactPayment: () async throws -> Void + let authorizeContactPayment: () async throws -> Void let completeContactPayment: (String) async -> Void let cancelContactPayment: () async -> Void @State private var signingTask: Task? @@ -115,7 +116,8 @@ struct HwSendSignView: View { address: invoice.address, sats: amount, satsPerVByte: UInt64(feeRate), - beforeBroadcast: prepareContactPayment, + beforeFirstBroadcast: prepareContactPayment, + beforeBroadcastAttempt: authorizeContactPayment, afterBroadcast: { result in await completeContactPayment(result.txId) } diff --git a/Bitkit/Views/Wallets/Send/SendSheet.swift b/Bitkit/Views/Wallets/Send/SendSheet.swift index 9651cd916..af5099efc 100644 --- a/Bitkit/Views/Wallets/Send/SendSheet.swift +++ b/Bitkit/Views/Wallets/Send/SendSheet.swift @@ -695,6 +695,7 @@ struct SendSheet: View { navigationPath: $navigationPath, hwSend: hwSend, prepareContactPayment: prepareHardwareContactPayment, + authorizeContactPayment: authorizeHardwareContactPayment, completeContactPayment: completeHardwareContactPayment, cancelContactPayment: cancelHardwareContactPayment ) @@ -801,18 +802,22 @@ struct SendSheet: View { do { try await prepareIncomingPaymentRequest() try await PaykitPaymentProofService.shared.markOnchainPaymentStarted(request, address: address) - do { - try await paykitPaymentRequestManager.ensurePaymentAllowed(request) - } catch { - await PaykitPaymentProofService.shared.failOnchainPayment(request) - throw error - } } catch { await PaykitPaymentProofService.shared.cancelPreparation(request) throw error } } + private func authorizeHardwareContactPayment() async throws { + guard let request = app.contactPaymentContext?.incomingPaymentRequest else { return } + do { + try await paykitPaymentRequestManager.ensurePaymentAllowed(request) + } catch { + await PaykitPaymentProofService.shared.failOnchainPayment(request) + throw error + } + } + private func completeHardwareContactPayment(txid: String) async { guard let request = app.contactPaymentContext?.incomingPaymentRequest, let address = app.scannedOnchainInvoice?.address diff --git a/BitkitTests/HwFundingSignerTests.swift b/BitkitTests/HwFundingSignerTests.swift index 8ed3139a8..75fecdf7f 100644 --- a/BitkitTests/HwFundingSignerTests.swift +++ b/BitkitTests/HwFundingSignerTests.swift @@ -193,6 +193,64 @@ final class HwFundingSignerTests: XCTestCase { ) } + func testCoordinatorRetryChecksAuthorizationBeforeRebroadcast() async { + let funding = MockHwFunding() + let connecting = MockHwConnecting() + let manager = HwWalletManager() + let coordinator = HwSendCoordinator( + walletId: "trezor:wallet", + signerFactory: { [self] _, address, satsPerVByte in + makeSigner( + funding: funding, + connecting: connecting, + feeRate: satsPerVByte, + address: address + ) + } + ) + var preparationCalls = 0 + var authorizationCalls = 0 + var isPaymentAllowed = true + let preparePayment: () async throws -> Void = { preparationCalls += 1 } + let authorizePayment: () async throws -> Void = { + authorizationCalls += 1 + if !isPaymentAllowed { + throw MockHwFunding.TestError() + } + } + funding.broadcastError = BroadcastError.ElectrumError(errorDetails: "offline") + + await assertThrowsAsync { + _ = try await coordinator.signAndBroadcast( + manager: manager, + address: "bc1qtest", + sats: 42000, + satsPerVByte: 2, + beforeFirstBroadcast: preparePayment, + beforeBroadcastAttempt: authorizePayment + ) + } + + funding.broadcastError = nil + isPaymentAllowed = false + await assertThrowsAsync { + _ = try await coordinator.signAndBroadcast( + manager: manager, + address: "bc1qtest", + sats: 42000, + satsPerVByte: 2, + beforeFirstBroadcast: preparePayment, + beforeBroadcastAttempt: authorizePayment + ) + } + + XCTAssertEqual(preparationCalls, 1) + XCTAssertEqual(authorizationCalls, 2) + XCTAssertEqual(funding.signCalls, 1) + XCTAssertEqual(funding.broadcastCalls, 1) + XCTAssertFalse(coordinator.hasPendingBroadcast) + } + func testCoordinatorCancelDropsSignedPaymentAfterFailedBroadcast() async throws { let funding = MockHwFunding() let connecting = MockHwConnecting() @@ -252,7 +310,8 @@ final class HwFundingSignerTests: XCTestCase { ) } ) - var beforeBroadcastCalls = 0 + var preparationCalls = 0 + var authorizationCalls = 0 var completedTransactionIds: [String] = [] funding.broadcastError = error @@ -262,7 +321,8 @@ final class HwFundingSignerTests: XCTestCase { address: "bc1qtest", sats: 42000, satsPerVByte: 2, - beforeBroadcast: { beforeBroadcastCalls += 1 }, + beforeFirstBroadcast: { preparationCalls += 1 }, + beforeBroadcastAttempt: { authorizationCalls += 1 }, afterBroadcast: { completedTransactionIds.append($0.txId) } ) } @@ -277,7 +337,8 @@ final class HwFundingSignerTests: XCTestCase { address: "bc1qtest", sats: 42000, satsPerVByte: 2, - beforeBroadcast: { beforeBroadcastCalls += 1 }, + beforeFirstBroadcast: { preparationCalls += 1 }, + beforeBroadcastAttempt: { authorizationCalls += 1 }, afterBroadcast: { completedTransactionIds.append($0.txId) } ) @@ -285,7 +346,8 @@ final class HwFundingSignerTests: XCTestCase { XCTAssertEqual(funding.signCalls, 1) XCTAssertEqual(funding.broadcastCalls, 2) XCTAssertEqual(funding.broadcastTransactions, [funding.signedTx.serializedTx, funding.signedTx.serializedTx]) - XCTAssertEqual(beforeBroadcastCalls, 1) + XCTAssertEqual(preparationCalls, 1) + XCTAssertEqual(authorizationCalls, 2) XCTAssertEqual(completedTransactionIds, [funding.broadcastTxId]) } diff --git a/BitkitTests/PaykitPaymentRequestServiceTests.swift b/BitkitTests/PaykitPaymentRequestServiceTests.swift index cffd2441e..d41175a4f 100644 --- a/BitkitTests/PaykitPaymentRequestServiceTests.swift +++ b/BitkitTests/PaykitPaymentRequestServiceTests.swift @@ -237,7 +237,7 @@ final class PaykitPaymentRequestServiceTests: XCTestCase { XCTAssertFalse(manager.isApprovedForPayment(request)) } - func testApprovedPaymentRechecksBlockingWithoutRepeatingAcceptance() async throws { + func testApprovedPaymentRechecksBlockingBeforeSendWithoutRepeatingAcceptance() async throws { let record = try paymentRequestRecord(counterparty: "pubky3rsduhcxpw74snwyct86m38c63j3pq8x4ycqikxg64roik8yw5xy") let sdk = PaymentRequestSdkMock(records: [record]) let manager = paymentRequestManager(sdk: sdk) @@ -253,13 +253,16 @@ final class PaykitPaymentRequestServiceTests: XCTestCase { peers: [linkedPeer(counterparty: record.counterparty, path: record.counterpartyReceiverPath, state: .blocked)], receiverPathsByPublicKey: [:] ) + var sendCalls = 0 do { - try await manager.prepareForPayment(request) { consumedCount += 1 } + try await manager.ensurePaymentAllowed(request) + sendCalls += 1 XCTFail("Expected blocked payment to be rejected") } catch { XCTAssertEqual(error as? PaykitPaymentRequestError, .requestUnavailable) } XCTAssertEqual(consumedCount, 1) + XCTAssertEqual(sendCalls, 0) XCTAssertFalse(manager.isApprovedForPayment(request)) } From 94197004b345f86cd5788872e14e48511683dc15 Mon Sep 17 00:00:00 2001 From: benk10 Date: Wed, 30 Sep 2026 11:03:21 +0100 Subject: [PATCH 6/6] fix: preserve uncertain hardware payment retries --- Bitkit/Services/PubkyService.swift | 5 +- Bitkit/ViewModels/HwFundingSigner.swift | 12 +- Bitkit/Views/Wallets/Send/SendSheet.swift | 7 +- BitkitTests/HwFundingSignerTests.swift | 112 +++++++++++++----- BitkitTests/PaykitContactLifecycleTests.swift | 49 ++++++++ 5 files changed, 145 insertions(+), 40 deletions(-) diff --git a/Bitkit/Services/PubkyService.swift b/Bitkit/Services/PubkyService.swift index 38df9e1e5..235229baf 100644 --- a/Bitkit/Services/PubkyService.swift +++ b/Bitkit/Services/PubkyService.swift @@ -659,9 +659,10 @@ actor PaykitSdkService { ($0.terms?.recurrence?.endsAt.flatMap(PaykitPaymentRequest.parseDate).map { $0 > now } ?? true) } guard activeSubscriptions.isEmpty else { - let latestEndDate = activeSubscriptions.compactMap { + let endDates = activeSubscriptions.compactMap { $0.terms?.recurrence?.endsAt.flatMap(PaykitPaymentRequest.parseDate) - }.max() + } + let latestEndDate = endDates.count == activeSubscriptions.count ? endDates.max() : nil throw PubkyServiceError.activeSubscription(endsAt: latestEndDate) } for peer in peers where peer.state == .linked { diff --git a/Bitkit/ViewModels/HwFundingSigner.swift b/Bitkit/ViewModels/HwFundingSigner.swift index 3ea29b3e0..2a9157e9e 100644 --- a/Bitkit/ViewModels/HwFundingSigner.swift +++ b/Bitkit/ViewModels/HwFundingSigner.swift @@ -493,7 +493,16 @@ final class HwSendCoordinator { do { try await beforeBroadcastAttempt() - isBroadcastUnresolved = true + } catch { + if pendingPayment?.hasBroadcastAttempted != true { + pendingPayment = nil + } + throw error + } + + isBroadcastUnresolved = true + pendingPayment?.hasBroadcastAttempted = true + do { let result = try await signer.broadcastSignedFunding(signed) await afterBroadcast(result) return result @@ -579,5 +588,6 @@ final class HwSendCoordinator { let request: PaymentRequest let signedTx: HwFundingSignedTx var isPreparedForBroadcast = false + var hasBroadcastAttempted = false } } diff --git a/Bitkit/Views/Wallets/Send/SendSheet.swift b/Bitkit/Views/Wallets/Send/SendSheet.swift index 3545e6529..c00b202df 100644 --- a/Bitkit/Views/Wallets/Send/SendSheet.swift +++ b/Bitkit/Views/Wallets/Send/SendSheet.swift @@ -810,12 +810,7 @@ struct SendSheet: View { private func authorizeHardwareContactPayment() async throws { guard let request = app.contactPaymentContext?.incomingPaymentRequest else { return } - do { - try await paykitPaymentRequestManager.ensurePaymentAllowed(request) - } catch { - await PaykitPaymentProofService.shared.failOnchainPayment(request) - throw error - } + try await paykitPaymentRequestManager.ensurePaymentAllowed(request) } private func completeHardwareContactPayment(txid: String) async { diff --git a/BitkitTests/HwFundingSignerTests.swift b/BitkitTests/HwFundingSignerTests.swift index 75fecdf7f..c223d925c 100644 --- a/BitkitTests/HwFundingSignerTests.swift +++ b/BitkitTests/HwFundingSignerTests.swift @@ -193,34 +193,67 @@ final class HwFundingSignerTests: XCTestCase { ) } - func testCoordinatorRetryChecksAuthorizationBeforeRebroadcast() async { - let funding = MockHwFunding() - let connecting = MockHwConnecting() - let manager = HwWalletManager() - let coordinator = HwSendCoordinator( - walletId: "trezor:wallet", - signerFactory: { [self] _, address, satsPerVByte in - makeSigner( - funding: funding, - connecting: connecting, - feeRate: satsPerVByte, - address: address + func testCoordinatorDeniedRetryPreservesAttemptedPayment() async throws { + let broadcastErrors: [Error] = [ + HwTransferError.broadcastUncertain, + BroadcastError.ElectrumError(errorDetails: "offline"), + ] + + for broadcastError in broadcastErrors { + let funding = MockHwFunding() + let connecting = MockHwConnecting() + let manager = HwWalletManager() + let coordinator = HwSendCoordinator( + walletId: "trezor:wallet", + signerFactory: { [self] _, address, satsPerVByte in + makeSigner( + funding: funding, + connecting: connecting, + feeRate: satsPerVByte, + address: address + ) + } + ) + var preparationCalls = 0 + var authorizationCalls = 0 + var isPaymentAllowed = true + let preparePayment: () async throws -> Void = { preparationCalls += 1 } + let authorizePayment: () async throws -> Void = { + authorizationCalls += 1 + if !isPaymentAllowed { + throw MockHwFunding.TestError() + } + } + funding.broadcastError = broadcastError + + await assertThrowsAsync { + _ = try await coordinator.signAndBroadcast( + manager: manager, + address: "bc1qtest", + sats: 42000, + satsPerVByte: 2, + beforeFirstBroadcast: preparePayment, + beforeBroadcastAttempt: authorizePayment ) } - ) - var preparationCalls = 0 - var authorizationCalls = 0 - var isPaymentAllowed = true - let preparePayment: () async throws -> Void = { preparationCalls += 1 } - let authorizePayment: () async throws -> Void = { - authorizationCalls += 1 - if !isPaymentAllowed { - throw MockHwFunding.TestError() + + funding.broadcastError = nil + isPaymentAllowed = false + await assertThrowsAsync { + _ = try await coordinator.signAndBroadcast( + manager: manager, + address: "bc1qtest", + sats: 42000, + satsPerVByte: 2, + beforeFirstBroadcast: preparePayment, + beforeBroadcastAttempt: authorizePayment + ) } - } - funding.broadcastError = BroadcastError.ElectrumError(errorDetails: "offline") - await assertThrowsAsync { + XCTAssertTrue(coordinator.hasPendingBroadcast) + XCTAssertEqual(funding.broadcastCalls, 1) + + isPaymentAllowed = true _ = try await coordinator.signAndBroadcast( manager: manager, address: "bc1qtest", @@ -229,25 +262,42 @@ final class HwFundingSignerTests: XCTestCase { beforeFirstBroadcast: preparePayment, beforeBroadcastAttempt: authorizePayment ) + + XCTAssertEqual(preparationCalls, 1) + XCTAssertEqual(authorizationCalls, 3) + XCTAssertEqual(funding.signCalls, 1) + XCTAssertEqual(funding.broadcastCalls, 2) + XCTAssertEqual(funding.broadcastTransactions, [funding.signedTx.serializedTx, funding.signedTx.serializedTx]) } + } + + func testCoordinatorDeniedFirstAttemptDropsPreparedPayment() async { + let funding = MockHwFunding() + let manager = HwWalletManager() + let coordinator = HwSendCoordinator( + walletId: "trezor:wallet", + signerFactory: { [self] _, address, satsPerVByte in + makeSigner( + funding: funding, + connecting: MockHwConnecting(), + feeRate: satsPerVByte, + address: address + ) + } + ) - funding.broadcastError = nil - isPaymentAllowed = false await assertThrowsAsync { _ = try await coordinator.signAndBroadcast( manager: manager, address: "bc1qtest", sats: 42000, satsPerVByte: 2, - beforeFirstBroadcast: preparePayment, - beforeBroadcastAttempt: authorizePayment + beforeBroadcastAttempt: { throw MockHwFunding.TestError() } ) } - XCTAssertEqual(preparationCalls, 1) - XCTAssertEqual(authorizationCalls, 2) XCTAssertEqual(funding.signCalls, 1) - XCTAssertEqual(funding.broadcastCalls, 1) + XCTAssertEqual(funding.broadcastCalls, 0) XCTAssertFalse(coordinator.hasPendingBroadcast) } diff --git a/BitkitTests/PaykitContactLifecycleTests.swift b/BitkitTests/PaykitContactLifecycleTests.swift index 992dfa135..474f9ee26 100644 --- a/BitkitTests/PaykitContactLifecycleTests.swift +++ b/BitkitTests/PaykitContactLifecycleTests.swift @@ -62,6 +62,55 @@ final class PaykitContactLifecycleTests: XCTestCase { } } + func testDeletionDateRequiresEveryActiveSubscriptionToHaveValidEnd() async throws { + let fixedEndTimestamp = "2099-02-01T00:00:00Z" + let laterEndTimestamp = "2100-02-01T00:00:00Z" + + func subscription(publicKey: String, endsAt: String?) throws -> PaymentRequestRecord { + let terms = try PaymentRequestTerms( + amount: PaymentRequestAmount(value: "0.001", asset: "btc"), + paymentReference: PaymentReference(text: "subscription"), proposalExpiresAt: nil, + recurrence: PaymentRequestRecurrence(every: 1, unit: "month", startsAt: "2026-01-01T00:00:00Z", + anchor: "2026-01-01T00:00:00Z", endsAt: endsAt), + acceptedPaymentEndpointIdentifiers: ["lightning:bolt11"], conversion: nil, paymentDeadline: nil, + metadata: PrivateJsonObject(text: "{}") + ) + return PaymentRequestRecord( + counterparty: publicKey, counterpartyReceiverPath: PaykitReceiverPath.server, + paymentRequestId: UUID().uuidString, localRole: .payer, state: .activeRecurring, + proposalStreamItemId: nil, proposalOutboundMessageId: nil, proposalOutboundStatus: nil, + proposalEventId: nil, terms: terms, acceptedEventId: nil, acceptedOutboundStatus: nil, + rejectedEventId: nil, rejectedOutboundStatus: nil, canceledEventId: nil, canceledOutboundStatus: nil, + conversionQuotes: [], paymentProofs: [], lastStreamItemId: nil, lastOutboundMessageId: nil, + lastOutboundStatus: nil, lastEventAt: nil, invalidReason: nil + ) + } + + let cases: [(endsAt: [String?], malformedIndex: Int?, expectedEnd: String?)] = [ + ([fixedEndTimestamp, nil], nil, nil), + ([fixedEndTimestamp, fixedEndTimestamp], 1, nil), + ([fixedEndTimestamp, laterEndTimestamp], nil, laterEndTimestamp), + ] + + for testCase in cases { + let sdk = ContactLifecycleSdk(noPointer: .init()) + sdk.requests = try testCase.endsAt.map { try subscription(publicKey: sdk.publicKey, endsAt: $0) } + if let malformedIndex = testCase.malformedIndex { + sdk.requests[malformedIndex].terms?.recurrence?.endsAt = "not-a-date" + } + let service = PaykitSdkService(sdkFactory: { sdk }) + + do { + _ = try await service.removeContact(publicKey: sdk.publicKey) + XCTFail("Expected active subscriptions to prevent deletion") + } catch let PubkyServiceError.activeSubscription(endsAt) { + XCTAssertEqual(endsAt, testCase.expectedEnd.flatMap(PaykitPaymentRequest.parseDate)) + } + XCTAssertNotNil(sdk.record) + XCTAssertTrue(sdk.events.isEmpty) + } + } + func testFailedBlockKeepsContactAvailableForDeletionRetry() async throws { let sdk = ContactLifecycleSdk(noPointer: .init()) sdk.failBlock = true