diff --git a/Bitkit/Components/CopyAddressCard.swift b/Bitkit/Components/CopyAddressCard.swift index 4c6906248..6938f936c 100644 --- a/Bitkit/Components/CopyAddressCard.swift +++ b/Bitkit/Components/CopyAddressCard.swift @@ -32,6 +32,7 @@ struct CopyAddressCard: View { .lineLimit(2) .truncationMode(.middle) .padding(.bottom, 12) + .accessibilityIdentifier(pair.type == .onchain ? "ReceiveOnchainAddress" : "ReceiveLightningAddress") HStack(spacing: 8) { if let editRoute { diff --git a/Bitkit/Services/CoreService.swift b/Bitkit/Services/CoreService.swift index bafe61e12..831b7ba9d 100644 --- a/Bitkit/Services/CoreService.swift +++ b/Bitkit/Services/CoreService.swift @@ -210,7 +210,14 @@ class ActivityService { } /// Marks every unseen activity across all wallets as seen, each under its own wallet id. - func markAllUnseenActivitiesAsSeen() async { + /// + /// `startedBefore` limits the pass to activity that already existed at that time. The restore + /// sweep passes the moment the restore began, so a payment that genuinely arrives while the + /// restore is still running keeps its unseen state and still notifies the user. #588 + /// + /// Returns `false` when the pass did not complete, so callers can keep any suppression they hold. + @discardableResult + func markAllUnseenActivitiesAsSeen(startedBefore cutoff: UInt64? = nil) async -> Bool { let timestamp = UInt64(Date().timeIntervalSince1970) do { @@ -221,16 +228,23 @@ class ActivityService { let id: String let walletId: String let isSeen: Bool + let createdAt: UInt64 switch activity { case let .onchain(onchain): id = onchain.id walletId = onchain.walletId isSeen = onchain.seenAt != nil + createdAt = onchain.timestamp case let .lightning(lightning): id = lightning.id walletId = lightning.walletId isSeen = lightning.seenAt != nil + createdAt = lightning.timestamp + } + + if let cutoff, createdAt > cutoff { + continue } if !isSeen { @@ -244,8 +258,11 @@ class ActivityService { if didMarkAny { activitiesChangedSubject.send() } + + return true } catch { Logger.error("Failed to mark all activities as seen: \(error)", context: "ActivityService") + return false } } diff --git a/Bitkit/ViewModels/AppViewModel.swift b/Bitkit/ViewModels/AppViewModel.swift index d60344933..97ddc602b 100644 --- a/Bitkit/ViewModels/AppViewModel.swift +++ b/Bitkit/ViewModels/AppViewModel.swift @@ -138,6 +138,20 @@ class AppViewModel: ObservableObject { private(set) var isQuickPayActive = false private var quickPayPaymentHash: String? + /// Txids for which a received-sheet presentation has already been started this session. + /// The received and confirmed LDK events for the same tx each call the presenter, so this + /// reserves the txid synchronously on the MainActor (before any await) to guarantee the sheet + /// is presented at most once and avoid a double-notification race. See issue #455. + private var receivedSheetInFlightTxids: Set = [] + + /// On-chain receives that arrived while the restore sweep ran, in arrival order, to present once + /// the hold lifts. Whatever the first restore sync emitted is dropped instead: it cannot tell a + /// payment arriving mid-scan from an unconfirmed one the scan replays. #588 + private(set) var restoreHeldReceives: [RestoreHeldReceive] = [] + + /// Whether the post-restore sweep is running, so a later sync cannot start a second one. + private var isCompletingRestoreHold = false + /// When a payment that was shown on the pending screen succeeds or fails, this is set so SendPendingScreen can navigate. /// Consumed by SendPendingScreen via consumeSendSheetPendingResolution. @Published var sendSheetPendingResolution: SendSheetPendingResolution? @@ -1202,7 +1216,144 @@ extension AppViewModel { // MARK: LDK Node Events +/// An on-chain receive held by the restore hold. `blockHeight` and `confirmationTime` are nil for a +/// mempool receive. +struct RestoreHeldReceive: Equatable { + let txid: String + let amountSats: Int64 + let blockHeight: UInt32? + let confirmationTime: UInt64? +} + extension AppViewModel { + /// Lifts the post-restore received-sheet suppression, once the activities the first post-restore + /// on-chain sync replayed have actually been marked seen. + /// + /// The flag must outlive the marking pass: clearing it up front reopens + /// `presentReceivedSheetForOnchainTransaction` while the pass is still running, and leaves it open + /// for good if the pass fails, so a historical tx can pop a "Received" sheet. #588 + /// + /// Records `syncedBlockHeight` as the restore's chain tip, so confirmations the restore already + /// scanned stay silent after the hold too, then presents the receives held during the sweep. + /// Call `beginCompletingPendingRestoreActivitySeen` first, synchronously in the event handler. + @MainActor + func completePendingRestoreActivitySeen( + syncedBlockHeight: UInt32, + markAllSeen: (UInt64) async -> Bool = { cutoff in + await CoreService.shared.activity.markAllUnseenActivitiesAsSeen(startedBefore: cutoff) + }, + presentReceive: ((String, Int64) -> Void)? = nil + ) async { + defer { isCompletingRestoreHold = false } + let restoreStartedAt = SettingsViewModel.shared.pendingRestoreActivitySeenSince + guard restoreStartedAt > 0 else { return } + guard await markAllSeen(restoreStartedAt) else { return } + SettingsViewModel.shared.restoreSyncedBlockHeight = syncedBlockHeight + SettingsViewModel.shared.pendingRestoreActivitySeenSince = 0 + + let held = restoreHeldReceives + restoreHeldReceives.removeAll() + let present = presentReceive ?? { [weak self] txid, amountSats in + self?.presentReceivedSheetForOnchainTransaction(txid: txid, amountSats: amountSats) + } + for receive in held { + if let blockHeight = receive.blockHeight, let confirmationTime = receive.confirmationTime { + guard Self.shouldPresentConfirmedOnlyReceive( + confirmationTime: confirmationTime, + blockHeight: blockHeight, + restoreSyncedBlockHeight: syncedBlockHeight + ) else { continue } + } + present(receive.txid, receive.amountSats) + } + } + + /// Starts lifting the restore hold on an on-chain `syncCompleted`, returning whether the caller + /// should run `completePendingRestoreActivitySeen`. Runs synchronously in the event handler: the + /// receives held so far came from the restore scan itself and are dropped as history, and only + /// one sweep runs, so a later sync cannot raise the recorded restore tip. #588 + func beginCompletingPendingRestoreActivitySeen() -> Bool { + guard SettingsViewModel.shared.pendingRestoreActivitySeen, !isCompletingRestoreHold else { return false } + restoreHeldReceives.removeAll() + isCompletingRestoreHold = true + return true + } + + /// Holds an on-chain receive while the restore hold is up, returning whether it was held. + /// `blockHeight` and `confirmationTime` are set for a confirmed event, nil for a mempool one. + @discardableResult + func holdReceiveDuringRestore( + txid: String, + amountSats: Int64, + blockHeight: UInt32? = nil, + confirmationTime: UInt64? = nil + ) -> Bool { + guard SettingsViewModel.shared.pendingRestoreActivitySeen else { return false } + guard amountSats > 0 else { return true } + Logger.debug("Skipping received sheet for tx \(txid) until the restore sweep finishes") + if !restoreHeldReceives.contains(where: { $0.txid == txid }) { + restoreHeldReceives.append( + RestoreHeldReceive(txid: txid, amountSats: amountSats, blockHeight: blockHeight, confirmationTime: confirmationTime) + ) + } + return true + } + + /// Shows the "received" sheet for an incoming on-chain tx, unless it was already shown. + /// Used by both the received (mempool) and confirmed (straight-to-confirmed) LDK events so a + /// tx that skips the mempool still notifies the user. See issue #455. + /// Max distance between a confirmed-only tx's block time and the device clock for it to count as a new + /// receive. A full wallet scan, as after a migration or when an address type starts being monitored, + /// replays confirmed events for old txs, and those stay silent. Absolute because block timestamps and + /// device clocks can each run ahead of the other. Matches `MAX_CONFIRMED_ONLY_AGE` on Android. + static let maxConfirmedOnlyReceiveAge: TimeInterval = 60 * 60 + + /// Whether a confirmed event is a new receive. Also skips any block at or below the tip the latest + /// seed restore scanned, since a later rescan replays those with block times that can still fall + /// inside the window. #588 + static func shouldPresentConfirmedOnlyReceive( + confirmationTime: UInt64, + blockHeight: UInt32, + now: Date = Date(), + isMigrating: Bool = MigrationsService.shared.isShowingMigrationLoading || MigrationsService.shared.needsPostMigrationSync, + restoreSyncedBlockHeight: UInt32? = nil + ) -> Bool { + guard !isMigrating else { return false } + guard blockHeight > (restoreSyncedBlockHeight ?? SettingsViewModel.shared.restoreSyncedBlockHeight) else { return false } + let age = abs(now.timeIntervalSince1970 - TimeInterval(confirmationTime)) + return age <= maxConfirmedOnlyReceiveAge + } + + private func presentReceivedSheetForOnchainTransaction(txid: String, amountSats: Int64) { + guard amountSats > 0 else { return } + + // Reserve the txid synchronously on the MainActor (no await between check and insert) so the + // received and confirmed events for the same tx can't both pass the seen-check and present the + // sheet twice. The persisted seenAt still handles cross-launch dedup; this closes the in-session + // concurrency race. + guard receivedSheetInFlightTxids.insert(txid).inserted else { return } + + let sats = UInt64(amountSats) + + Task { + // 500ms delay so the activity is written to the DB before the dedup/filter checks read it. + try? await Task.sleep(nanoseconds: 500_000_000) + + if await CoreService.shared.activity.isOnchainActivitySeen(txid: txid) { + return + } + + let shouldShow = await CoreService.shared.activity.shouldShowReceivedSheet(txid: txid, value: sats) + guard shouldShow else { return } + + await CoreService.shared.activity.markOnchainActivityAsSeen(txid: txid) + + await MainActor.run { + sheetViewModel.showSheet(.receivedTx, data: ReceivedTxSheetDetails(type: .onchain, sats: sats)) + } + } + } + func handleLdkNodeEvent(_ event: Event) { switch event { case let .paymentReceived(paymentId, _, amountMsat, _): @@ -1345,30 +1496,26 @@ extension AppViewModel { // MARK: New Onchain Transaction Events case let .onchainTransactionReceived(txid, details): - // Show notification for incoming transactions - if details.amountSats > 0 { - let sats = UInt64(abs(Int64(details.amountSats))) - - Task { - // Show sheet for new transactions or replacements with value changes - try? await Task.sleep(nanoseconds: 500_000_000) // 500ms delay - - if await CoreService.shared.activity.isOnchainActivitySeen(txid: txid) { - return - } - - let shouldShow = await CoreService.shared.activity.shouldShowReceivedSheet(txid: txid, value: sats) - guard shouldShow else { return } - - await CoreService.shared.activity.markOnchainActivityAsSeen(txid: txid) - - await MainActor.run { - sheetViewModel.showSheet(.receivedTx, data: ReceivedTxSheetDetails(type: .onchain, sats: sats)) - } - } + // Show notification for incoming transactions seen in the mempool, once any restore hold lifts + if !holdReceiveDuringRestore(txid: txid, amountSats: details.amountSats) { + presentReceivedSheetForOnchainTransaction(txid: txid, amountSats: details.amountSats) } - case let .onchainTransactionConfirmed(txid, _, blockHeight, _, _): + case let .onchainTransactionConfirmed(txid, _, blockHeight, confirmationTime, details): Logger.info("Transaction confirmed: \(txid) at block \(blockHeight)") + // Also notify when a tx goes straight to confirmed without a prior received event + if holdReceiveDuringRestore( + txid: txid, + amountSats: details.amountSats, + blockHeight: blockHeight, + confirmationTime: confirmationTime + ) { + break + } + if Self.shouldPresentConfirmedOnlyReceive(confirmationTime: confirmationTime, blockHeight: blockHeight) { + presentReceivedSheetForOnchainTransaction(txid: txid, amountSats: details.amountSats) + } else { + Logger.debug("Skipping received sheet for confirmed-only tx \(txid) confirmed at \(confirmationTime), height \(blockHeight)") + } case let .onchainTransactionReplaced(txid, conflicts): Logger.info("Transaction replaced: \(txid) by \(conflicts.count) conflict(s)") Task { @@ -1441,6 +1588,14 @@ extension AppViewModel { } } + // After a seed restore, the first on-chain sync has now discovered the historical txs. + // Mark them seen so they don't pop a "Received" sheet, and lift the restore suppression. #588 + if syncType == .onchainWallet, beginCompletingPendingRestoreActivitySeen() { + Task { @MainActor in + await self.completePendingRestoreActivitySeen(syncedBlockHeight: syncedBlockHeight) + } + } + if MigrationsService.shared.needsPostMigrationSync { Task { @MainActor in try? await CoreService.shared.activity.syncLdkNodePayments(LightningService.shared.listPayments() ?? []) diff --git a/Bitkit/ViewModels/SettingsViewModel.swift b/Bitkit/ViewModels/SettingsViewModel.swift index 32d3df71e..bb65b0e3f 100644 --- a/Bitkit/ViewModels/SettingsViewModel.swift +++ b/Bitkit/ViewModels/SettingsViewModel.swift @@ -458,6 +458,36 @@ class SettingsViewModel: NSObject, ObservableObject { set { UserDefaults.standard.set(newValue, forKey: Self.pendingRestoreAddressTypePruneKey) } } + private static let pendingRestoreActivitySeenSinceKey = "pendingRestoreActivitySeenSince" + + /// When a seed restore began, or 0 when no restore is being suppressed. + /// + /// Set as the restore starts, before the node is started, so the replayed historical txs cannot + /// slip a "Received" sheet through ahead of the flag. Doubles as the cutoff for the sweep that + /// marks those txs seen, so a payment arriving mid-restore is not swept up with them. Cleared in + /// AppViewModel's syncCompleted(.onchainWallet) handler once that sweep succeeds. #588 + var pendingRestoreActivitySeenSince: UInt64 { + get { UInt64(UserDefaults.standard.double(forKey: Self.pendingRestoreActivitySeenSinceKey)) } + set { UserDefaults.standard.set(Double(newValue), forKey: Self.pendingRestoreActivitySeenSinceKey) } + } + + /// Whether replayed restore activity is still being suppressed. + var pendingRestoreActivitySeen: Bool { + pendingRestoreActivitySeenSince > 0 + } + + private static let restoreSyncedBlockHeightKey = "restoreSyncedBlockHeight" + + /// Chain tip of the first on-chain sync after the latest seed restore, or 0 when none completed. + /// + /// Everything confirmed at or below it was already on chain when the restore scanned the wallet, so + /// it outlives the restore hold: a later rescan replays confirmations for those txs, and they must + /// stay silent however long after the hold their events are handled. #588 + var restoreSyncedBlockHeight: UInt32 { + get { UInt32(clamping: UserDefaults.standard.integer(forKey: Self.restoreSyncedBlockHeightKey)) } + set { UserDefaults.standard.set(Int(newValue), forKey: Self.restoreSyncedBlockHeightKey) } + } + /// After restore, disables monitoring for address types with zero balance. /// Keeps nativeSegwit as primary and monitored; only types with funds stay monitored. func pruneEmptyAddressTypesAfterRestore() async { diff --git a/Bitkit/Views/Onboarding/RestoreWalletView.swift b/Bitkit/Views/Onboarding/RestoreWalletView.swift index 684660fd6..9d3b9a3f4 100644 --- a/Bitkit/Views/Onboarding/RestoreWalletView.swift +++ b/Bitkit/Views/Onboarding/RestoreWalletView.swift @@ -273,12 +273,21 @@ struct RestoreWalletView: View { // Prevent settings changes from triggering backups before the actual restore runs BackupService.shared.setRestoring(true) + // Suppress "Received" sheets for the historical txs the restore replays. Set here, before + // the node is started, because startup sync begins as soon as the wallet exists - setting + // it on the Get Started tap left a window where replayed txs could still pop a sheet. #588 + SettingsViewModel.shared.pendingRestoreActivitySeenSince = UInt64(Date().timeIntervalSince1970) + SettingsViewModel.shared.restoreSyncedBlockHeight = 0 + // When restoring a wallet, monitor all address types to catch any existing funds SettingsViewModel.shared.monitorAllAddressTypes() _ = try StartupHandler.restoreWallet(mnemonic: bip39Mnemonic, bip39Passphrase: bip39Passphrase) try wallet.setWalletExistsState() } catch { + // The node never started, so no sync will lift the hold. Left in place it would silence + // every later on-chain receive, across retries and relaunches. #588 + SettingsViewModel.shared.pendingRestoreActivitySeenSince = 0 BackupService.shared.setRestoring(false) app.toast(error) } diff --git a/Bitkit/Views/Onboarding/WalletRestoreSuccess.swift b/Bitkit/Views/Onboarding/WalletRestoreSuccess.swift index 9d852fa69..9e0d90114 100644 --- a/Bitkit/Views/Onboarding/WalletRestoreSuccess.swift +++ b/Bitkit/Views/Onboarding/WalletRestoreSuccess.swift @@ -36,8 +36,13 @@ struct WalletRestoreSuccess: View { app.backupVerified = true wallet.isRestoringWallet = false - // Skip pruning if backup had explicit monitored address types let settings = SettingsViewModel.shared + + // Note: the "Received" sheet suppression for replayed historical txs is armed when the + // restore starts, in RestoreWalletView, not here - by this tap the node has already + // been syncing for a while. #588 + + // Skip pruning if backup had explicit monitored address types if !settings.restoredMonitoredTypesFromBackup { settings.pendingRestoreAddressTypePrune = true } diff --git a/BitkitTests/ConfirmedOnlyReceiveGuardTests.swift b/BitkitTests/ConfirmedOnlyReceiveGuardTests.swift new file mode 100644 index 000000000..e716d4141 --- /dev/null +++ b/BitkitTests/ConfirmedOnlyReceiveGuardTests.swift @@ -0,0 +1,113 @@ +@testable import Bitkit +import XCTest + +/// A confirmed event reaches the received sheet only for a recent block outside a migration, so the +/// confirmations a full wallet scan replays for old txs stay silent. #455, parity with +/// `NotifyPaymentReceivedHandler.canShowConfirmedOnly` on Android. +@MainActor +final class ConfirmedOnlyReceiveGuardTests: XCTestCase { + private let now = Date(timeIntervalSince1970: 1_790_000_000) + private let maxAge = AppViewModel.maxConfirmedOnlyReceiveAge + private let height: UInt32 = 900 + + private func blockTime(secondsFromNow offset: TimeInterval) -> UInt64 { + UInt64(now.timeIntervalSince1970 + offset) + } + + func testRecentConfirmationIsPresented() { + XCTAssertTrue( + AppViewModel.shouldPresentConfirmedOnlyReceive( + confirmationTime: blockTime(secondsFromNow: -30), + blockHeight: height, + now: now, + isMigrating: false, + restoreSyncedBlockHeight: 0 + ) + ) + } + + func testConfirmationAtTheWindowEdgeIsPresented() { + XCTAssertTrue( + AppViewModel.shouldPresentConfirmedOnlyReceive( + confirmationTime: blockTime(secondsFromNow: -maxAge), + blockHeight: height, + now: now, + isMigrating: false, + restoreSyncedBlockHeight: 0 + ) + ) + XCTAssertTrue( + AppViewModel.shouldPresentConfirmedOnlyReceive( + confirmationTime: blockTime(secondsFromNow: maxAge), + blockHeight: height, + now: now, + isMigrating: false, + restoreSyncedBlockHeight: 0 + ) + ) + } + + func testOldConfirmationReplayedByAScanIsSkipped() { + XCTAssertFalse( + AppViewModel.shouldPresentConfirmedOnlyReceive( + confirmationTime: blockTime(secondsFromNow: -maxAge - 1), + blockHeight: height, + now: now, + isMigrating: false, + restoreSyncedBlockHeight: 0 + ), + "a replayed historical confirmation would pop a Received sheet" + ) + } + + func testBlockTimeFarAheadOfTheDeviceClockIsSkipped() { + XCTAssertFalse( + AppViewModel.shouldPresentConfirmedOnlyReceive( + confirmationTime: blockTime(secondsFromNow: maxAge + 1), + blockHeight: height, + now: now, + isMigrating: false, + restoreSyncedBlockHeight: 0 + ) + ) + } + + func testRecentConfirmationIsSkippedDuringMigration() { + XCTAssertFalse( + AppViewModel.shouldPresentConfirmedOnlyReceive( + confirmationTime: blockTime(secondsFromNow: -30), + blockHeight: height, + now: now, + isMigrating: true, + restoreSyncedBlockHeight: 0 + ), + "the post-migration scan replays confirmations for migrated txs that are not yet marked seen" + ) + } + + /// #588, Android #1342: a rescan after the hold lifts replays a recent historical confirmation. + func testConfirmationTheRestoreAlreadyScannedIsSkippedAfterTheHold() { + XCTAssertFalse( + AppViewModel.shouldPresentConfirmedOnlyReceive( + confirmationTime: blockTime(secondsFromNow: -30), + blockHeight: height, + now: now, + isMigrating: false, + restoreSyncedBlockHeight: height + ), + "a historical receive confirmed within the hour would pop a Received sheet" + ) + } + + func testConfirmationAboveTheRestoreTipIsPresented() { + XCTAssertTrue( + AppViewModel.shouldPresentConfirmedOnlyReceive( + confirmationTime: blockTime(secondsFromNow: -30), + blockHeight: height + 1, + now: now, + isMigrating: false, + restoreSyncedBlockHeight: height + ) + ) + } +} diff --git a/BitkitTests/MarkAllUnseenActivitiesCutoffTests.swift b/BitkitTests/MarkAllUnseenActivitiesCutoffTests.swift new file mode 100644 index 000000000..aa5035dcf --- /dev/null +++ b/BitkitTests/MarkAllUnseenActivitiesCutoffTests.swift @@ -0,0 +1,103 @@ +@testable import Bitkit +import BitkitCore +import XCTest + +/// Regression cover for #588: the post-restore sweep marks the replayed historical activity as seen, +/// and must leave alone a payment that genuinely arrives while the restore is still running. +/// +/// Without the cutoff, such a payment was suppressed by `pendingRestoreActivitySeen` on arrival and +/// then marked seen by the sweep, so it never notified the user at all. +final class MarkAllUnseenActivitiesCutoffTests: XCTestCase { + private let testDbPath = NSTemporaryDirectory() + private let service = CoreService.shared.activity + + private let restoreStartedAt: UInt64 = 1_700_000_000 + + override func setUp() async throws { + try await super.setUp() + _ = try initDb(basePath: testDbPath) + try await Task.sleep(nanoseconds: 1_000_000_000) + } + + override func tearDown() async throws { + try await super.tearDown() + + let fileManager = FileManager.default + let dbPath = (testDbPath as NSString).appendingPathComponent("activity.db") + if fileManager.fileExists(atPath: dbPath) { + try fileManager.removeItem(atPath: dbPath) + } + } + + func testSweepMarksReplayedHistoryButSparesNewerActivity() async throws { + let replayed = "restore-replayed-history" + let arrivedDuringRestore = "arrived-mid-restore" + + try await service.insert(onchainActivity(id: replayed, txId: "old", timestamp: restoreStartedAt - 3600)) + try await service.insert( + onchainActivity(id: arrivedDuringRestore, txId: "new", timestamp: restoreStartedAt + 30) + ) + + let completed = await service.markAllUnseenActivitiesAsSeen(startedBefore: restoreStartedAt) + + let replayedSeenAt = try await seenAt(of: replayed) + let newerSeenAt = try await seenAt(of: arrivedDuringRestore) + + XCTAssertTrue(completed) + XCTAssertNotNil(replayedSeenAt, "replayed history should be marked seen by the sweep") + XCTAssertNil( + newerSeenAt, + "a payment that arrived during the restore must stay unseen, or it never notifies" + ) + } + + func testSweepWithoutACutoffStillMarksEverything() async throws { + let id = "no-cutoff" + try await service.insert(onchainActivity(id: id, txId: "any", timestamp: restoreStartedAt + 30)) + + let completed = await service.markAllUnseenActivitiesAsSeen() + + let markedSeenAt = try await seenAt(of: id) + + XCTAssertTrue(completed) + XCTAssertNotNil(markedSeenAt, "the post-migration caller passes no cutoff and expects a full sweep") + } + + // MARK: - Helpers + + private func seenAt(of id: String) async throws -> UInt64? { + guard case let .onchain(activity) = try await service.getActivity(id: id) else { + XCTFail("activity \(id) was not stored as on-chain") + return nil + } + return activity.seenAt + } + + private func onchainActivity(id: String, txId: String, timestamp: UInt64) -> Activity { + .onchain( + OnchainActivity( + walletId: WalletScope.default, + id: id, + txType: .received, + txId: txId, + value: 10000, + fee: 100, + feeRate: 1, + address: "bc1...", + confirmed: true, + timestamp: timestamp, + isBoosted: false, + boostTxIds: [], + isTransfer: false, + doesExist: true, + confirmTimestamp: nil, + channelId: nil, + transferTxId: nil, + contact: nil, + createdAt: nil, + updatedAt: nil, + seenAt: nil + ) + ) + } +} diff --git a/BitkitTests/RestoreActivitySeenSuppressionTests.swift b/BitkitTests/RestoreActivitySeenSuppressionTests.swift new file mode 100644 index 000000000..569a6704b --- /dev/null +++ b/BitkitTests/RestoreActivitySeenSuppressionTests.swift @@ -0,0 +1,211 @@ +@testable import Bitkit +import XCTest + +/// Regression cover for #588: the post-restore received-sheet suppression must outlive the pass that +/// marks the replayed activities as seen, and that pass must not sweep up payments that arrive while +/// the restore is still running. +/// +/// Clearing `pendingRestoreActivitySeenSince` up front reopened +/// `presentReceivedSheetForOnchainTransaction` while the marking pass was still running — and kept it +/// open when the pass failed — so a historical tx replayed by LDK could pop a "Received" sheet. +@MainActor +final class RestoreActivitySeenSuppressionTests: XCTestCase { + private let flagKey = "pendingRestoreActivitySeenSince" + private let heightKey = "restoreSyncedBlockHeight" + private let syncedHeight: UInt32 = 900 + private let restoreStartedAt: UInt64 = 1_700_000_000 + + override func setUp() { + super.setUp() + snapshotAppDefaults(flagKey, heightKey) + } + + func testSuppressionHoldsUntilTheMarkingPassFinishes() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + let app = AppViewModel() + var flagDuringPass: Bool? + + await app.completePendingRestoreActivitySeen(syncedBlockHeight: syncedHeight) { _ in + flagDuringPass = SettingsViewModel.shared.pendingRestoreActivitySeen + return true + } + + XCTAssertEqual(flagDuringPass, true, "suppression was lifted before the activities were marked seen") + XCTAssertFalse(SettingsViewModel.shared.pendingRestoreActivitySeen) + } + + func testSuppressionIsKeptWhenTheMarkingPassFails() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + let app = AppViewModel() + + await app.completePendingRestoreActivitySeen(syncedBlockHeight: syncedHeight) { _ in false } + + XCTAssertTrue( + SettingsViewModel.shared.pendingRestoreActivitySeen, + "a failed marking pass must keep the restore suppression, or replayed txs pop a sheet" + ) + } + + func testMarkingPassIsSkippedWhenNoRestoreIsPending() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = 0 + let app = AppViewModel() + var didRunPass = false + + await app.completePendingRestoreActivitySeen(syncedBlockHeight: syncedHeight) { _ in + didRunPass = true + return true + } + + XCTAssertFalse(didRunPass, "every on-chain sync would re-mark all activities seen") + XCTAssertFalse(SettingsViewModel.shared.pendingRestoreActivitySeen) + } + + /// The sweep is bounded by when the restore began, so a payment that genuinely arrives mid-restore + /// keeps its unseen state instead of being marked seen along with the replayed history. + func testMarkingPassIsBoundedByTheRestoreStartTime() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + let app = AppViewModel() + var passedCutoff: UInt64? + + await app.completePendingRestoreActivitySeen(syncedBlockHeight: syncedHeight) { cutoff in + passedCutoff = cutoff + return true + } + + XCTAssertEqual(passedCutoff, restoreStartedAt) + } + + /// The suppression is armed as the restore starts, not on the Get Started tap, because startup + /// sync begins as soon as the wallet exists. + func testSuppressionFlagIsDerivedFromTheStoredStartTime() { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + XCTAssertTrue(SettingsViewModel.shared.pendingRestoreActivitySeen) + XCTAssertEqual(SettingsViewModel.shared.pendingRestoreActivitySeenSince, restoreStartedAt) + + SettingsViewModel.shared.pendingRestoreActivitySeenSince = 0 + XCTAssertFalse(SettingsViewModel.shared.pendingRestoreActivitySeen) + } + + /// A rescan after the hold replays confirmations for the same history, so the restore tip must + /// outlive the hold. Android #1342. + func testCompletingTheHoldRecordsTheRestoreTip() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + SettingsViewModel.shared.restoreSyncedBlockHeight = 0 + let app = AppViewModel() + + await app.completePendingRestoreActivitySeen(syncedBlockHeight: syncedHeight) { _ in true } + + XCTAssertEqual(SettingsViewModel.shared.restoreSyncedBlockHeight, syncedHeight) + } + + func testFailedPassDoesNotRecordTheRestoreTip() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + SettingsViewModel.shared.restoreSyncedBlockHeight = 0 + let app = AppViewModel() + + await app.completePendingRestoreActivitySeen(syncedBlockHeight: syncedHeight) { _ in false } + + XCTAssertEqual(SettingsViewModel.shared.restoreSyncedBlockHeight, 0) + } + + /// A receive arriving while the sweep runs is held and presented once the hold lifts. + func testReceivesHeldDuringTheSweepArePresentedOnceTheHoldLifts() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + let app = AppViewModel() + XCTAssertTrue(app.beginCompletingPendingRestoreActivitySeen()) + + XCTAssertTrue(app.holdReceiveDuringRestore(txid: "a", amountSats: 1000)) + XCTAssertTrue(app.holdReceiveDuringRestore(txid: "b", amountSats: 2000)) + XCTAssertTrue(app.holdReceiveDuringRestore(txid: "a", amountSats: 1000)) + XCTAssertTrue(app.holdReceiveDuringRestore(txid: "c", amountSats: 0)) + + var presented: [String] = [] + await app.completePendingRestoreActivitySeen( + syncedBlockHeight: syncedHeight, + markAllSeen: { _ in true }, + presentReceive: { txid, _ in presented.append(txid) } + ) + + XCTAssertEqual(presented, ["a", "b"]) + XCTAssertTrue(app.restoreHeldReceives.isEmpty) + } + + /// The restore scan cannot tell a payment arriving mid-scan from an unconfirmed one it replays, + /// so whatever it emitted is dropped as history once its sync completes. + func testReceivesHeldDuringTheRestoreScanAreDropped() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + let app = AppViewModel() + app.holdReceiveDuringRestore(txid: "replayed", amountSats: 1000) + + XCTAssertTrue(app.beginCompletingPendingRestoreActivitySeen()) + XCTAssertTrue(app.restoreHeldReceives.isEmpty) + + var presented: [String] = [] + await app.completePendingRestoreActivitySeen( + syncedBlockHeight: syncedHeight, + markAllSeen: { _ in true }, + presentReceive: { txid, _ in presented.append(txid) } + ) + + XCTAssertTrue(presented.isEmpty, "an unconfirmed pre-restore receive would pop a Received sheet") + } + + /// A confirmation handled while the sweep runs is judged against the restore tip on replay. + func testConfirmationsHeldDuringTheSweepAreCheckedAgainstTheRestoreTip() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + let app = AppViewModel() + XCTAssertTrue(app.beginCompletingPendingRestoreActivitySeen()) + let now = UInt64(Date().timeIntervalSince1970) + + app.holdReceiveDuringRestore(txid: "old", amountSats: 1000, blockHeight: syncedHeight, confirmationTime: now) + app.holdReceiveDuringRestore(txid: "new", amountSats: 1000, blockHeight: syncedHeight + 1, confirmationTime: now) + + var presented: [String] = [] + await app.completePendingRestoreActivitySeen( + syncedBlockHeight: syncedHeight, + markAllSeen: { _ in true }, + presentReceive: { txid, _ in presented.append(txid) } + ) + + XCTAssertEqual(presented, ["new"]) + } + + /// Only the first sync after the restore sweeps, so a later one cannot raise the recorded tip. + func testOnlyOneSweepRunsAtATime() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + let app = AppViewModel() + + XCTAssertTrue(app.beginCompletingPendingRestoreActivitySeen()) + XCTAssertFalse(app.beginCompletingPendingRestoreActivitySeen()) + + await app.completePendingRestoreActivitySeen(syncedBlockHeight: syncedHeight) { _ in false } + + XCTAssertTrue(app.beginCompletingPendingRestoreActivitySeen(), "a failed sweep must let the next sync retry") + } + + func testHeldReceivesWaitWhenThePassFails() async { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = restoreStartedAt + let app = AppViewModel() + XCTAssertTrue(app.beginCompletingPendingRestoreActivitySeen()) + app.holdReceiveDuringRestore(txid: "a", amountSats: 1000) + + var presented: [String] = [] + await app.completePendingRestoreActivitySeen( + syncedBlockHeight: syncedHeight, + markAllSeen: { _ in false }, + presentReceive: { txid, _ in presented.append(txid) } + ) + + XCTAssertTrue(presented.isEmpty, "a held receive shown before the sweep could be replayed history") + XCTAssertEqual(app.restoreHeldReceives.map(\.txid), ["a"]) + } + + func testReceivesAreNotHeldWithoutARestore() { + SettingsViewModel.shared.pendingRestoreActivitySeenSince = 0 + let app = AppViewModel() + + XCTAssertFalse(app.holdReceiveDuringRestore(txid: "a", amountSats: 1000)) + XCTAssertFalse(app.beginCompletingPendingRestoreActivitySeen()) + XCTAssertTrue(app.restoreHeldReceives.isEmpty) + } +} diff --git a/changelog.d/next/588.fixed.md b/changelog.d/next/588.fixed.md new file mode 100644 index 000000000..c75daf731 --- /dev/null +++ b/changelog.d/next/588.fixed.md @@ -0,0 +1 @@ +Incoming on-chain transactions that confirm before being seen in the mempool now show the received notification. diff --git a/journeys/README.md b/journeys/README.md index 43c55d61c..7678c3fdf 100644 --- a/journeys/README.md +++ b/journeys/README.md @@ -157,6 +157,10 @@ journey PR, which is what made this file conflict on every merge. ## Not ported +**`onchain-receive/confirmed-only-background-notification.xml`.** It covers the notification Android's +`LightningNodeService` foreground service posts for a background onchain receive. iOS has no +foreground node service, and its notification extension handles only Blocktank pushes. + **`deeplinks/screen-deeplink.xml` and `sheet-deeplink.xml`.** These Android journeys exercise `bitkit://screen/...` routing with a dev-mode gate and a cold-start replay. iOS registers the `bitkit` URL scheme (`Bitkit/Info.plist`) and retains external URLs in `AppScene`, but `MainNavView` only routes web URLs, Pubky contacts, auth requests and callbacks, diff --git a/journeys/onchain-receive/README.md b/journeys/onchain-receive/README.md new file mode 100644 index 000000000..f1b5c4541 --- /dev/null +++ b/journeys/onchain-receive/README.md @@ -0,0 +1,46 @@ +# Onchain receive journeys + +These journeys cover the received sheet for onchain deposits (issue #455, Android #797). Ported from +`bitkit-android/journeys/onchain-receive` with synonymdev/bitkit-ios#588. + +ldk-node emits `onchainTransactionReceived` when the wallet sync finds a transaction in the mempool +and `onchainTransactionConfirmed` when it confirms. A transaction that is mined before any sync sees +it in the mempool produces only the confirmed event. `AppViewModel.handleLdkNodeEvent` routes both to +`presentReceivedSheetForOnchainTransaction`, which reserves the txid in-session and checks the +persisted seen state, so a tx shows one sheet whichever event reaches it first. + +A confirmed-only receive is shown only when its block timestamp is within one hour of the device +clock and no migration is running (`AppViewModel.shouldPresentConfirmedOnlyReceive`, matching +Android's `MAX_CONFIRMED_ONLY_AGE`). A full wallet scan replays old confirmations, which the window +keeps silent. After a seed restore, `RestoreWalletView` sets `pendingRestoreActivitySeenSince` before +the node starts, which holds every onchain received sheet until the first onchain sync completes; +that sync marks the activities that existed before the restore began as seen and clears the flag, so +the transactions it discovered stay silent when they later confirm while new deposits notify again. +That sync also records its chain tip in `restoreSyncedBlockHeight`, and confirmed-only receives at or +below it stay silent, so a replayed or late-handled confirmation of a pre-restore tx never shows a +sheet (Android #1342). The restore journey needs a throwaway simulator, since it uninstalls the app +and restores a public test seed; the rest is covered by `ConfirmedOnlyReceiveGuardTests`, +`RestoreActivitySeenSuppressionTests` and `MarkAllUnseenActivitiesCutoffTests`. + +## Adapted from Android + +- `confirmed-only-background-notification.xml` is not ported. It covers Android's + `LightningNodeService` foreground service, which posts a "Payment Received" notification while the + app is in the background. iOS has no foreground node service and the notification extension only + handles Blocktank pushes, so there is no iOS path to drive. +- `mempool-then-confirmed-single-sheet.xml` drops the final "no Payment Received notification" check + for the same reason. +- Android skips confirmed-only receives while a backup restore runs; iOS relies on the restore hold + above, which covers the same window. + +## Preconditions + +- Onboarded regtest wallet with the node running, built with `E2E_BUILD` against the local + `bitkit-docker` stack (see the suite-wide [README](../README.md#backend-preconditions)). Fund and + mine with the `lsp` helper from the sibling Android checkout. +- Wallet sync runs every 10s. For the confirmed-only journey, run the deposit and the mine in one + shell command, then check the log: an `Onchain transaction received` line for the txid means the + sync saw the mempool first and the run tested the other path. +- The app writes its log to the app group, not `os_log`: `logs/bitkit_*.log` under + `xcrun simctl get_app_container booted to.bitkit group.bitkit`. `LightningService` logs each onchain + event as `📥 Onchain transaction received: txid=…` or `✅ Onchain transaction confirmed: txid=…`. diff --git a/journeys/onchain-receive/confirmed-only-received-sheet.xml b/journeys/onchain-receive/confirmed-only-received-sheet.xml new file mode 100644 index 000000000..e7469ea19 --- /dev/null +++ b/journeys/onchain-receive/confirmed-only-received-sheet.xml @@ -0,0 +1,25 @@ + + + Covers issue #455 (Android #797). An onchain deposit the wallet first sees already confirmed, with + no prior mempool event, must show the received sheet once. ldk-node emits + onchainTransactionConfirmed without onchainTransactionReceived in that case. + + Precondition: onboarded regtest wallet, node running, app in the foreground on the home screen. + The deposit and the mine must run in one shell command so the 10s wallet sync does not see the + transaction in the mempool first. If the log shows "Onchain transaction received" for the txid, + the run tested the mempool path instead; repeat with a new address. + + + Tap Receive (id "Receive") and verify the Receive sheet opens (id "ReceiveScreen") + Tap the "Savings" receive tab (id "Tab-savings"), tap "Show Details" (id "ShowDetails") and read the address from id "ReceiveOnchainAddress" + Swipe the Receive sheet down to return to the home screen + Run: LOG="$(ls -t "$(xcrun simctl get_app_container booted to.bitkit group.bitkit)"/logs/bitkit_*.log | head -1)" + Run in one command: ../bitkit-android/lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":21797}' && ../bitkit-android/lsp POST /regtest/chain/mine '{"count":1}' + Wait up to 30s for the next wallet sync + Run: grep <txid> "$LOG" + Verify the log shows an "Onchain transaction confirmed" line for the txid and no "Onchain transaction received" line for it + Verify the received sheet (id "ReceivedTransaction") is visible with the deposited amount (id "ReceivedTransaction-primary" or "ReceivedTransaction-secondary", depending on the primary display setting) + Tap the sheet button (id "ReceivedTransactionButton") + Wait 10s and verify the received sheet does not appear again + + diff --git a/journeys/onchain-receive/mempool-then-confirmed-single-sheet.xml b/journeys/onchain-receive/mempool-then-confirmed-single-sheet.xml new file mode 100644 index 000000000..266e4f72f --- /dev/null +++ b/journeys/onchain-receive/mempool-then-confirmed-single-sheet.xml @@ -0,0 +1,23 @@ + + + Covers issue #455 (Android #797). Confirmed events now reach the received sheet, so a deposit + seen in the mempool first must not show a second sheet when it confirms. AppViewModel dedupes on + the txid in-session and on the persisted seen state across launches. + + Precondition: onboarded regtest wallet, node running, app in the foreground on the home screen. + + Adapted: the Android original ends by checking no "Payment Received" notification is posted. iOS + posts no local notification for an onchain receive, so that step is dropped. + + + Tap Receive (id "Receive") and verify the Receive sheet opens (id "ReceiveScreen") + Tap the "Savings" receive tab (id "Tab-savings"), tap "Show Details" (id "ShowDetails") and read the address from id "ReceiveOnchainAddress" + Swipe the Receive sheet down to return to the home screen + Run: ../bitkit-android/lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":14797}' + Wait up to 30s and verify the received sheet (id "ReceivedTransaction") is visible with the deposited amount + Tap the sheet button (id "ReceivedTransactionButton") + Run: ../bitkit-android/lsp POST /regtest/chain/mine '{"count":1}' + Run: grep <txid> "$(ls -t "$(xcrun simctl get_app_container booted to.bitkit group.bitkit)"/logs/bitkit_*.log | head -1)" and wait until an "Onchain transaction confirmed" line shows for the txid + Wait 30s after that event and verify the received sheet does not appear again + + diff --git a/journeys/onchain-receive/restore-recent-receive-stays-silent.xml b/journeys/onchain-receive/restore-recent-receive-stays-silent.xml new file mode 100644 index 000000000..c69df0dc0 --- /dev/null +++ b/journeys/onchain-receive/restore-recent-receive-stays-silent.xml @@ -0,0 +1,34 @@ + + + Covers Android issue #1342, fixed on iOS in #588. A deposit confirmed within the last hour, restored + from its seed, must not show the received sheet: its confirmation is inside the one-hour window, + and a replayed or late-handled confirmation could reach the presenter after the first sync had + lifted the restore hold. The first onchain sync after a restore records its chain tip, and + confirmed-only receives at or below it stay silent. A deposit mined after the restore still shows + the sheet. + + Precondition: a throwaway simulator (both halves uninstall the app; never run this on a device + holding a wallet you need). If onboarding does not show after reinstalling, the keychain survived + the uninstall: run `xcrun simctl keychain booted reset` and relaunch. The wallet is the public + BIP39 test vector "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon + abandon about", so other developers may have funded it on the shared regtest chain; only the + deposit made here is asserted on. Put the words on the simulator pasteboard with + `printf '<words>' | xcrun simctl pbcopy booted` and paste them into "Word-0" rather than typing + them. Run the whole journey within the hour, or the one-hour window alone keeps the sheet silent + and the run proves nothing. + + + Run: xcrun simctl uninstall booted to.bitkit, install and launch the build, accept the terms, tap "SkipIntro", "RestoreWallet" and "MultipleDevices-button" + Paste the test vector into "Word-0", tap "RestoreButton", then "GetStartedButton", and dismiss any intro sheet until the home screen shows (id "TotalBalance-primary") + Tap Receive (id "Receive"), tap the "Savings" receive tab (id "Tab-savings"), tap "Show Details" (id "ShowDetails"), read the address from id "ReceiveOnchainAddress", and swipe the Receive sheet down to the home screen + Run in one command: ../bitkit-android/lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":21342}' && ../bitkit-android/lsp POST /regtest/chain/mine '{"count":1}' + Verify within 30s the received sheet (id "ReceivedTransaction") shows 21 342 sats, then tap "ReceivedTransactionButton" + Run: xcrun simctl uninstall booted to.bitkit, install and launch the build and restore the same test vector the same way, tap "GetStartedButton" and dismiss any intro sheet + Wait 60s on the home screen + Verify the received sheet (id "ReceivedTransaction") never appeared and the home screen shows "TotalBalance-primary" + Run: grep "Skipping received sheet.*<txid>" "$(ls -t "$(xcrun simctl get_app_container booted to.bitkit group.bitkit)"/logs/bitkit_*.log | head -1)" + Verify the log skipped the deposit's txid, either as a confirmed-only tx or "until the restore sweep finishes" — which one depends on whether its confirmation was handled before or after the restore hold lifted + Read a new address as above and run in one command: ../bitkit-android/lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":21343}' && ../bitkit-android/lsp POST /regtest/chain/mine '{"count":1}' + Verify within 30s the received sheet (id "ReceivedTransaction") shows 21 343 sats + +