fix: clean up deleted private contacts #830
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -114,7 +114,18 @@ | |
|
|
||
| @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) { | ||
|
Check warning on line 121 in Bitkit/Managers/ContactsManager.swift
|
||
| self.contactRecords = contactRecords | ||
| } | ||
|
|
||
| @Published var contacts: [PubkyContact] = [] { | ||
| didSet { contactsRevision += 1 } | ||
| } | ||
|
|
||
| @Published var isLoading = false | ||
| @Published var hasLoaded = false | ||
| @Published var loadErrorMessage: String? | ||
|
|
@@ -136,6 +147,7 @@ | |
| } | ||
|
|
||
| func reset() { | ||
| loadGeneration += 1 | ||
| contacts = [] | ||
| isLoading = false | ||
| hasLoaded = false | ||
|
|
@@ -173,97 +185,107 @@ | |
| return | ||
| } | ||
|
|
||
| loadGeneration += 1 | ||
| let generation = loadGeneration | ||
| isLoading = true | ||
| loadErrorMessage = nil | ||
| defer { | ||
| if generation == loadGeneration { isLoading = false } | ||
| } | ||
|
|
||
| Logger.info("Loading contacts for \(PubkyPublicKeyFormat.redacted(publicKey))", 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<PubkyContact, Error>.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) | ||
| } | ||
|
|
||
| guard contactsRevision == revision else { continue } | ||
|
|
||
| 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 | ||
| } | ||
|
|
||
| contacts = loadedResult.contacts.sorted { $0.displayName.localizedCaseInsensitiveCompare($1.displayName) == .orderedAscending } | ||
| hasLoaded = true | ||
| await PrivatePaykitService.shared | ||
| .pruneUnsavedContactState(savedPublicKeys: records.compactMap { PubkyPublicKeyFormat.normalized($0.publicKey) }) | ||
|
|
||
| if loadedResult.failures > 0 { | ||
| Logger.warn( | ||
| "Skipped \(loadedResult.failures) unreadable contacts while loading list", | ||
| context: "ContactsManager" | ||
| ) | ||
| } | ||
|
|
||
| Logger.info("Loaded \(contacts.count) contacts", context: "ContactsManager") | ||
| return | ||
| } catch { | ||
| guard contactsRevision == revision else { continue } | ||
| if Self.isMissingContactsDataError(error) { | ||
| contacts = [] | ||
| hasLoaded = true | ||
| loadErrorMessage = nil | ||
| await PrivatePaykitService.shared.pruneUnsavedContactState(savedPublicKeys: []) | ||
| 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 | ||
| } | ||
| throw error | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -297,7 +319,12 @@ | |
| } | ||
|
|
||
| let receiverPaths = try await Self.relevantReceiverPaths(for: prefixedKey) | ||
| _ = try await PubkyService.saveContact( | ||
| publicKey: prefixedKey, | ||
| label: profile.name, | ||
| receiverPaths: receiverPaths, | ||
| restorePrivateConnection: true | ||
| ) | ||
|
|
||
| Logger.info("Added contact \(PubkyPublicKeyFormat.redacted(prefixedKey))", context: "ContactsManager") | ||
|
|
||
|
|
@@ -342,7 +369,12 @@ | |
| 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, | ||
| restorePrivateConnection: true | ||
| ) | ||
| return .success(PubkyContact(publicKey: key, profile: profile)) | ||
| } catch is CancellationError { | ||
| return .failure(CancellationError()) | ||
|
|
@@ -423,12 +455,11 @@ | |
| try await Task.detached { | ||
| _ = try await PubkyService.removeContact(publicKey: prefixedKey) | ||
| }.value | ||
| contacts.removeAll { $0.publicKey == prefixedKey } | ||
| Self.removeContactProfileOverride(publicKey: prefixedKey) | ||
| await PrivatePaykitService.shared.removeSavedContact(publicKey: prefixedKey) | ||
|
|
||
| Logger.info("Removed contact \(PubkyPublicKeyFormat.redacted(prefixedKey))", context: "ContactsManager") | ||
| } | ||
|
|
||
| func deleteAllContacts() async throws { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -191,7 +191,8 @@ extension PrivatePaykitService { | |
| ) | ||
| for receiverPath in cleanupReceiverPaths { | ||
| do { | ||
| guard let report = try await PaykitSdkService.shared.clearPrivatePaymentList(to: publicKey, receiverPath: receiverPath) | ||
| else { continue } | ||
|
Comment on lines
+194
to
+195
There was a problem hiding this comment.
Knowledge Base Used: Paykit payment modes
Member
There was a problem hiding this comment. This is not conditional. There was a problem hiding this comment. You’re right — I missed the ordering across the two services. The cleanup must occur before the SDK contact removal/blocking (or otherwise explicitly clear the list before blocking). Given that ordering, the
Contributor
Author
There was a problem hiding this comment. Correct about the ordering. Deletion now attempts empty-list delivery to linked receivers inside the existing SDK operation lock before blocking them, so another publication cannot run between withdrawal and revocation. A delivery failure still allows local blocking and contact removal. Post-block cleanup continues to skip network delivery. Tests cover the ordering and failed withdrawal. This preserves the withdrawal attempt, but it cannot retract invoices already shared and can still wait on the SDK network timeout. Commit: 75e8c9c.
Member
There was a problem hiding this comment. Resolved. The best-effort clear now runs before |
||
| if !report.failedToQueue.isEmpty || !report.failedToDeliver.isEmpty { | ||
| throw PrivatePaykitError.privateUnavailable | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deleting a contact hides an accepted subscription instead of ending it, and re-adding the contact re-presents every missed period.
Once the peer is blocked, the subscription drops out of
subscriptions, socancel()throwsrequestUnavailable(1447). The SDK also refuses cancel on a blocked peer, so this cannot be fixed by cancelling afterwards.subscriptionAcceptedAt(1738-1743) is never pruned, whiledismissedSubscriptionPaymentIds.formIntersection(activeRecurringRequestIds)(1755) drops the hidden subscription's dismissals.After an explicit re-add unblocks the peer,
requests(through:acceptedAt:)regenerates every unpaid period since the original acceptance, including dismissed ones, and each is presented. Nothing pays without a confirm. Before this change the subscription stayed visible and cancellable.Fix: cancel (or warn about) the counterparty's active payer subscriptions before
blockPeer, and move the acceptance anchor to the re-add date on unblock.Same in synonymdev/bitkit-android#1372.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deletion now refuses to proceed while this contact has an active subscription, whether we are payer or payee, and shows "End active subscriptions before deleting this contact." The check runs before endpoint withdrawal or blocking. Canceled or naturally ended subscriptions allow deletion. This keeps the existing cancellation and in-flight proof safeguards intact. Covered in
PaykitContactLifecycleTests.swiftand the mirrored subscription-deletion journey.Commit: 75e8c9c.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works. One LOW refinement: the guard counts fixed-term subscriptions whose
endsAtis in the future (PubkyService.swift:659), butcanCancel(at:)returns false wheneverrecurrence.endsAt != nil(PaykitSubscription.swift:478). A payer who accepted a fixed-term request can neither cancel it nor delete the contact until the term ends, while the refusal tells them to end the subscription. Bitkit creates requests withendsAt: nil, so this needs a proposal from another Paykit client, andisProposalActionable(at:)accepts those. Either allow cancelling fixed-term subscriptions or name the end date in the refusal. Same in synonymdev/bitkit-android#1372.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the fixed-term end date to the deletion refusal. Open-ended subscriptions keep the cancel-first message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved in 8d62210. The refusal now names the end date.