From 575815286903c6a883ad7899f39be11b07dbb7e5 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 17 Sep 2026 06:20:24 -0300 Subject: [PATCH 01/10] fix: notify confirmed-only onchain receives Co-Authored-By: Claude Opus 5 (1M context) --- .../androidServices/LightningNodeService.kt | 1 - .../domain/commands/NotifyPaymentReceived.kt | 23 +- .../commands/NotifyPaymentReceivedHandler.kt | 60 ++++- .../java/to/bitkit/viewmodels/AppViewModel.kt | 1 + .../LightningNodeServiceTest.kt | 72 ++++++ .../NotifyPaymentReceivedHandlerTest.kt | 239 ++++++++++++++++-- .../viewmodels/AppViewModelSendFlowTest.kt | 55 ++++ changelog.d/next/797.fixed.md | 1 + 8 files changed, 420 insertions(+), 32 deletions(-) create mode 100644 changelog.d/next/797.fixed.md diff --git a/app/src/main/java/to/bitkit/androidServices/LightningNodeService.kt b/app/src/main/java/to/bitkit/androidServices/LightningNodeService.kt index 79849ab9c9..1c101a1b07 100644 --- a/app/src/main/java/to/bitkit/androidServices/LightningNodeService.kt +++ b/app/src/main/java/to/bitkit/androidServices/LightningNodeService.kt @@ -104,7 +104,6 @@ class LightningNodeService : Service() { } private suspend fun handlePaymentReceived(event: Event) { - if (event !is Event.PaymentReceived && event !is Event.OnchainTransactionReceived) return val command = NotifyPaymentReceived.Command.from(event, includeNotification = true) ?: return notifyPaymentReceivedHandler(command).onSuccess { result -> diff --git a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceived.kt b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceived.kt index 1e9c92a93b..a68ff4159c 100644 --- a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceived.kt +++ b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceived.kt @@ -1,6 +1,7 @@ package to.bitkit.domain.commands import org.lightningdevkit.ldknode.Event +import org.lightningdevkit.ldknode.TransactionDetails import to.bitkit.models.NewTransactionSheetDetails import to.bitkit.models.NotificationDetails @@ -14,10 +15,18 @@ sealed interface NotifyPaymentReceived { override val includeNotification: Boolean = false, ) : Command + /** + * An incoming onchain transaction. [confirmedBlockHeight] is set when the wallet first saw the + * transaction already confirmed, without a prior mempool event. + */ data class Onchain( - val event: Event.OnchainTransactionReceived, + val txid: String, + val details: TransactionDetails, + val confirmedBlockHeight: UInt? = null, override val includeNotification: Boolean = false, - ) : Command + ) : Command { + val isConfirmedOnly: Boolean get() = confirmedBlockHeight != null + } companion object { fun from(event: Event, includeNotification: Boolean = false): Command? = @@ -28,7 +37,15 @@ sealed interface NotifyPaymentReceived { ) is Event.OnchainTransactionReceived -> Onchain( - event = event, + txid = event.txid, + details = event.details, + includeNotification = includeNotification, + ) + + is Event.OnchainTransactionConfirmed -> Onchain( + txid = event.txid, + details = event.details, + confirmedBlockHeight = event.blockHeight, includeNotification = includeNotification, ) diff --git a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt index 5aa105096a..12d46e145e 100644 --- a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt +++ b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt @@ -10,14 +10,21 @@ import to.bitkit.models.NewTransactionSheetDirection import to.bitkit.models.NewTransactionSheetType import to.bitkit.models.msatCeilOf import to.bitkit.repositories.ActivityRepo +import to.bitkit.repositories.BackupRepo +import to.bitkit.repositories.LightningRepo +import to.bitkit.services.MigrationService import to.bitkit.utils.Logger import javax.inject.Inject import javax.inject.Singleton +import kotlin.math.absoluteValue @Singleton class NotifyPaymentReceivedHandler @Inject constructor( @IoDispatcher private val ioDispatcher: CoroutineDispatcher, private val activityRepo: ActivityRepo, + private val lightningRepo: LightningRepo, + private val backupRepo: BackupRepo, + private val migrationService: MigrationService, private val receivedNotificationContent: ReceivedNotificationContent, ) { private val presentationClaimsLock = Any() @@ -86,7 +93,7 @@ class NotifyPaymentReceivedHandler @Inject constructor( private fun presentationKey(command: NotifyPaymentReceived.Command): String? = when (command) { is NotifyPaymentReceived.Command.Lightning -> command.event.paymentId?.let { "lightning:$it" } - is NotifyPaymentReceived.Command.Onchain -> "onchain:${command.event.txid}" + is NotifyPaymentReceived.Command.Onchain -> "onchain:${command.txid}" } private suspend fun shouldShowLightning(command: NotifyPaymentReceived.Command.Lightning): Boolean { @@ -96,17 +103,46 @@ class NotifyPaymentReceivedHandler @Inject constructor( } private suspend fun shouldShowOnchain(command: NotifyPaymentReceived.Command.Onchain): Boolean { - activityRepo.handleOnchainTransactionReceived(command.event.txid, command.event.details) - if (command.event.details.amountSats <= 0) return false + if (command.isConfirmedOnly) { + if (command.details.amountSats <= 0) return false + if (!canShowConfirmedOnly(command)) return false + activityRepo.handleOnchainTransactionConfirmed(command.txid, command.details) + } else { + activityRepo.handleOnchainTransactionReceived(command.txid, command.details) + if (command.details.amountSats <= 0) return false + } delay(DELAY_FOR_ACTIVITY_SYNC_MS) val shouldShowSheet = retryShouldShowReceivedSheet( - command.event.txid, - command.event.details.amountSats.toULong(), + command.txid, + command.details.amountSats.toULong(), ) return shouldShowSheet } + private suspend fun canShowConfirmedOnly(command: NotifyPaymentReceived.Command.Onchain): Boolean { + val blockHeight = command.confirmedBlockHeight ?: return false + if (backupRepo.isRestoring.value) { + Logger.debug("Skipping confirmed-only receive '${command.txid}' during restore", context = TAG) + return false + } + if (migrationService.isShowingMigrationLoading.value || migrationService.needsPostMigrationSync()) { + Logger.debug("Skipping confirmed-only receive '${command.txid}' during migration", context = TAG) + return false + } + val bestBlockHeight = lightningRepo.getStatus()?.currentBestBlock?.height + val depth = bestBlockHeight?.let { it.toLong() - blockHeight.toLong() } + if (depth == null || depth.absoluteValue > MAX_CONFIRMED_ONLY_BLOCK_DEPTH) { + Logger.debug( + "Skipping confirmed-only receive '${command.txid}' at height '$blockHeight' " + + "with best block '$bestBlockHeight'", + context = TAG, + ) + return false + } + return true + } + private suspend fun markAsSeen(command: NotifyPaymentReceived.Command) { when (command) { is NotifyPaymentReceived.Command.Lightning -> { @@ -114,7 +150,7 @@ class NotifyPaymentReceivedHandler @Inject constructor( activityRepo.markActivityAsSeen(paymentId) } - is NotifyPaymentReceived.Command.Onchain -> activityRepo.markOnchainActivityAsSeen(command.event.txid) + is NotifyPaymentReceived.Command.Onchain -> activityRepo.markOnchainActivityAsSeen(command.txid) } } @@ -134,11 +170,11 @@ class NotifyPaymentReceivedHandler @Inject constructor( direction = NewTransactionSheetDirection.RECEIVED, paymentHashOrTxId = when (command) { is NotifyPaymentReceived.Command.Lightning -> command.event.paymentHash - is NotifyPaymentReceived.Command.Onchain -> command.event.txid + is NotifyPaymentReceived.Command.Onchain -> command.txid }, sats = when (command) { is NotifyPaymentReceived.Command.Lightning -> msatCeilOf(command.event.amountMsat).toLong() - is NotifyPaymentReceived.Command.Onchain -> command.event.details.amountSats + is NotifyPaymentReceived.Command.Onchain -> command.details.amountSats }, ) @@ -152,5 +188,13 @@ class NotifyPaymentReceivedHandler @Inject constructor( private const val DELAY_FOR_ACTIVITY_SYNC_MS = 500L private const val RETRY_DELAY_MS = 300L private const val MAX_RETRIES = 3 + + /** + * Max distance in blocks between a confirmed-only transaction and the node's best block for it to + * count as a new receive. Older confirmations, such as those replayed by a full wallet scan after a + * restore, stay silent. The distance is absolute because an unsynced best block (e.g. genesis on a + * fresh node) lags behind the wallet scan and must not let the replay through. + */ + private const val MAX_CONFIRMED_ONLY_BLOCK_DEPTH = 2L } } diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 1000f56c7b..3e0e7fd1ef 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -1574,6 +1574,7 @@ class AppViewModel @Inject constructor( private suspend fun handleOnchainTransactionConfirmed(event: Event.OnchainTransactionConfirmed) { activityRepo.handleOnchainTransactionConfirmed(event.txid, event.details) + notifyPaymentReceived(event) } private suspend fun handleOnchainTransactionEvicted(event: Event.OnchainTransactionEvicted) { diff --git a/app/src/test/java/to/bitkit/androidServices/LightningNodeServiceTest.kt b/app/src/test/java/to/bitkit/androidServices/LightningNodeServiceTest.kt index 8ad9f6a731..2b6d47feaa 100644 --- a/app/src/test/java/to/bitkit/androidServices/LightningNodeServiceTest.kt +++ b/app/src/test/java/to/bitkit/androidServices/LightningNodeServiceTest.kt @@ -25,10 +25,12 @@ import org.junit.Rule import org.junit.Test import org.junit.runner.RunWith import org.lightningdevkit.ldknode.Event +import org.lightningdevkit.ldknode.TransactionDetails import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.doAnswer import org.mockito.kotlin.doSuspendableAnswer +import org.mockito.kotlin.eq import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -455,6 +457,76 @@ class LightningNodeServiceTest : BaseUnitTest() { assertEquals($$"Received ₿ 100 ($0.10)", body) } + @Test + fun `confirmed-only onchain receive in background shows notification`() = test { + val sheet = NewTransactionSheetDetails( + type = NewTransactionSheetType.ONCHAIN, + direction = NewTransactionSheetDirection.RECEIVED, + paymentHashOrTxId = "confirmed_txid", + sats = 5000L, + ) + val notification = NotificationDetails( + title = context.getString(R.string.notification__received__title), + body = "Received ₿ 5 000", + ) + whenever(notifyPaymentReceivedHandler.invoke(any())) + .thenReturn(Result.success(NotifyPaymentReceived.Result.ShowNotification(sheet, notification))) + startService() + testScheduler.advanceUntilIdle() + + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + capturedHandler?.invoke( + Event.OnchainTransactionConfirmed( + txid = "confirmed_txid", + blockHash = "block_hash", + blockHeight = 100u, + confirmationTime = 0uL, + details = details, + ), + ) + testScheduler.advanceUntilIdle() + + val expectedCommand = NotifyPaymentReceived.Command.Onchain( + txid = "confirmed_txid", + details = details, + confirmedBlockHeight = 100u, + includeNotification = true, + ) + verify(notifyPaymentReceivedHandler).invoke(expectedCommand) + verify(notifyPaymentReceivedHandler).present(eq(expectedCommand), any(), any()) + verify(cacheStore).setBackgroundReceive(sheet) + val receivedNotifications = Shadows.shadowOf(context.notificationManager).allNotifications.filter { + it.extras.getString(Notification.EXTRA_TITLE) == context.getString(R.string.notification__received__title) + } + assertEquals(1, receivedNotifications.size) + } + + @Test + fun `skipped confirmed-only onchain receive shows no notification`() = test { + whenever(notifyPaymentReceivedHandler.invoke(any())) + .thenReturn(Result.success(NotifyPaymentReceived.Result.Skip)) + startService() + testScheduler.advanceUntilIdle() + + capturedHandler?.invoke( + Event.OnchainTransactionConfirmed( + txid = "old_txid", + blockHash = "block_hash", + blockHeight = 1u, + confirmationTime = 0uL, + details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()), + ), + ) + testScheduler.advanceUntilIdle() + + val notification = Shadows.shadowOf(context.notificationManager).allNotifications.find { + it.extras.getString(Notification.EXTRA_TITLE) == context.getString(R.string.notification__received__title) + } + assertNull(notification) + verify(notifyPaymentReceivedHandler, never()).present(any(), any(), any()) + verify(cacheStore, never()).setBackgroundReceive(any()) + } + @Test fun `pending payment success in background shows notification`() = test { val sentTitle = context.getString(R.string.wallet__toast_payment_sent_title) diff --git a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt index af7a08cdbc..976d061dce 100644 --- a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt +++ b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt @@ -1,10 +1,13 @@ package to.bitkit.domain.commands import android.content.Context +import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import org.junit.Before import org.junit.Test +import org.lightningdevkit.ldknode.BestBlock import org.lightningdevkit.ldknode.Event +import org.lightningdevkit.ldknode.NodeStatus import org.lightningdevkit.ldknode.TransactionDetails import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull @@ -23,7 +26,10 @@ import to.bitkit.models.NewTransactionSheetDirection import to.bitkit.models.NewTransactionSheetType import to.bitkit.models.WalletScope import to.bitkit.repositories.ActivityRepo +import to.bitkit.repositories.BackupRepo import to.bitkit.repositories.CurrencyRepo +import to.bitkit.repositories.LightningRepo +import to.bitkit.services.MigrationService import to.bitkit.test.BaseUnitTest import java.math.BigDecimal import kotlin.test.assertEquals @@ -32,11 +38,19 @@ import kotlin.test.assertNotNull import kotlin.test.assertTrue class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { + companion object { + private const val BEST_BLOCK_HEIGHT = 1_000u + } private val context: Context = mock() private val activityRepo: ActivityRepo = mock() private val currencyRepo: CurrencyRepo = mock() private val settingsStore: SettingsStore = mock() + private val lightningRepo: LightningRepo = mock() + private val backupRepo: BackupRepo = mock() + private val migrationService: MigrationService = mock() + private val isRestoring = MutableStateFlow(false) + private val isShowingMigrationLoading = MutableStateFlow(false) private lateinit var sut: NotifyPaymentReceivedHandler @@ -45,6 +59,10 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { whenever(context.getString(R.string.notification__received__title)).thenReturn("Payment Received") whenever(context.getString(any(), any())).thenReturn("Received amount") whenever(settingsStore.data).thenReturn(flowOf(SettingsData())) + whenever(backupRepo.isRestoring).thenReturn(isRestoring) + whenever(migrationService.isShowingMigrationLoading).thenReturn(isShowingMigrationLoading) + whenever { migrationService.needsPostMigrationSync() }.thenReturn(false) + givenBestBlockHeight(BEST_BLOCK_HEIGHT) whenever(currencyRepo.convertSatsToFiat(any(), anyOrNull())).thenReturn( Result.success( ConvertedAmount( @@ -61,6 +79,9 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { sut = NotifyPaymentReceivedHandler( ioDispatcher = testDispatcher, activityRepo = activityRepo, + lightningRepo = lightningRepo, + backupRepo = backupRepo, + migrationService = migrationService, receivedNotificationContent = ReceivedNotificationContent( context = context, currencyRepo = currencyRepo, @@ -133,12 +154,8 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { val details = mock { on { amountSats } doReturn 5000L } - val event = mock { - on { txid } doReturn "txid456" - on { this.details } doReturn details - } whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = NotifyPaymentReceived.Command.Onchain(event = event) + val command = NotifyPaymentReceived.Command.Onchain(txid = "txid456", details = details) val result = sut(command) @@ -163,12 +180,8 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { val details = mock { on { amountSats } doReturn 5000L } - val event = mock { - on { txid } doReturn "txid456" - on { this.details } doReturn details - } whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(false) - val command = NotifyPaymentReceived.Command.Onchain(event = event) + val command = NotifyPaymentReceived.Command.Onchain(txid = "txid456", details = details) val result = sut(command) @@ -182,12 +195,8 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { val details = mock { on { amountSats } doReturn 7500L } - val event = mock { - on { txid } doReturn "txid789" - on { this.details } doReturn details - } whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = NotifyPaymentReceived.Command.Onchain(event = event) + val command = NotifyPaymentReceived.Command.Onchain(txid = "txid789", details = details) sut(command) sut.claimPresentation(command) @@ -205,12 +214,8 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { val details = mock { on { amountSats } doReturn 5000L } - val event = mock { - on { txid } doReturn "txid456" - on { this.details } doReturn details - } whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(false) - val command = NotifyPaymentReceived.Command.Onchain(event = event) + val command = NotifyPaymentReceived.Command.Onchain(txid = "txid456", details = details) sut(command) @@ -326,4 +331,198 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { assertFalse(sut.claimPresentation(command) { false }) assertTrue(sut.claimPresentation(command)) } + + @Test + fun `confirmed-only recent onchain receive returns ShowSheet`() = test { + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand(txid = "txidConfirmed", details = details, blockHeight = BEST_BLOCK_HEIGHT) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.ShowSheet) + assertEquals(NewTransactionSheetType.ONCHAIN, result.sheet.type) + assertEquals(NewTransactionSheetDirection.RECEIVED, result.sheet.direction) + assertEquals("txidConfirmed", result.sheet.paymentHashOrTxId) + assertEquals(5000L, result.sheet.sats) + inOrder(activityRepo) { + verify(activityRepo).handleOnchainTransactionConfirmed("txidConfirmed", details) + verify(activityRepo).shouldShowReceivedSheet("txidConfirmed", 5000uL) + } + verify(activityRepo, never()).handleOnchainTransactionReceived(any(), any()) + + assertTrue(sut.present(command) {}) + verify(activityRepo).markOnchainActivityAsSeen("txidConfirmed", WalletScope.default) + } + + @Test + fun `confirmed-only onchain receive returns ShowNotification when includeNotification is true`() = test { + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand( + txid = "txidConfirmed", + details = details, + blockHeight = BEST_BLOCK_HEIGHT - 2u, + includeNotification = true, + ) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.ShowNotification) + assertEquals("txidConfirmed", result.sheet.paymentHashOrTxId) + assertEquals("Payment Received", result.notification.title) + } + + @Test + fun `confirmed-only onchain receive ahead of an unsynced best block returns ShowSheet within depth`() = test { + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand(txid = "txidAhead", details = details, blockHeight = BEST_BLOCK_HEIGHT + 1u) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.ShowSheet) + } + + @Test + fun `received then confirmed onchain payment is presented once`() = test { + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val received = NotifyPaymentReceived.Command.Onchain(txid = "txidOnce", details = details) + val confirmed = confirmedCommand(txid = "txidOnce", details = details, blockHeight = BEST_BLOCK_HEIGHT) + var presentationCount = 0 + + val receivedResult = sut(received).getOrThrow() + assertTrue(receivedResult is NotifyPaymentReceived.Result.ShowSheet) + assertTrue(sut.present(received) { presentationCount += 1 }) + + val confirmedResult = sut(confirmed).getOrThrow() + + assertTrue(confirmedResult is NotifyPaymentReceived.Result.Skip) + assertFalse(sut.present(confirmed) { presentationCount += 1 }) + assertEquals(1, presentationCount) + verify(activityRepo, never()).handleOnchainTransactionConfirmed(any(), any()) + } + + @Test + fun `confirmed-only onchain receive at an old height returns Skip`() = test { + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand(txid = "txidOld", details = details, blockHeight = BEST_BLOCK_HEIGHT - 3u) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.Skip) + verify(activityRepo, never()).handleOnchainTransactionConfirmed(any(), any()) + verify(activityRepo, never()).shouldShowReceivedSheet(any(), any()) + } + + @Test + fun `confirmed-only onchain receive far ahead of the best block returns Skip`() = test { + givenBestBlockHeight(0u) + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand(txid = "txidReplay", details = details, blockHeight = BEST_BLOCK_HEIGHT) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.Skip) + verify(activityRepo, never()).shouldShowReceivedSheet(any(), any()) + } + + @Test + fun `confirmed-only onchain receive returns Skip when the best block is unknown`() = test { + whenever(lightningRepo.getStatus()).thenReturn(null) + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand(txid = "txidUnknown", details = details, blockHeight = BEST_BLOCK_HEIGHT) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.Skip) + verify(activityRepo, never()).shouldShowReceivedSheet(any(), any()) + } + + @Test + fun `confirmed-only onchain send returns Skip`() = test { + val details = TransactionDetails(amountSats = -5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand(txid = "txidSent", details = details, blockHeight = BEST_BLOCK_HEIGHT) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.Skip) + verify(activityRepo, never()).handleOnchainTransactionConfirmed(any(), any()) + verify(activityRepo, never()).shouldShowReceivedSheet(any(), any()) + } + + @Test + fun `confirmed-only onchain receive returns Skip while a restore is in progress`() = test { + isRestoring.value = true + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand(txid = "txidRestore", details = details, blockHeight = BEST_BLOCK_HEIGHT) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.Skip) + verify(activityRepo, never()).handleOnchainTransactionConfirmed(any(), any()) + verify(activityRepo, never()).shouldShowReceivedSheet(any(), any()) + } + + @Test + fun `confirmed-only onchain receive returns Skip while a migration is in progress`() = test { + whenever(migrationService.needsPostMigrationSync()).thenReturn(true) + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand(txid = "txidMigration", details = details, blockHeight = BEST_BLOCK_HEIGHT) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.Skip) + verify(activityRepo, never()).shouldShowReceivedSheet(any(), any()) + } + + @Test + fun `from maps a confirmed onchain event to a confirmed-only command`() { + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + val event = Event.OnchainTransactionConfirmed( + txid = "txidMapped", + blockHash = "blockHash", + blockHeight = BEST_BLOCK_HEIGHT, + confirmationTime = 0uL, + details = details, + ) + + val command = NotifyPaymentReceived.Command.from(event, includeNotification = true) + + assertEquals( + NotifyPaymentReceived.Command.Onchain( + txid = "txidMapped", + details = details, + confirmedBlockHeight = BEST_BLOCK_HEIGHT, + includeNotification = true, + ), + command, + ) + } + + private fun givenBestBlockHeight(height: UInt) { + val status = mock { + on { currentBestBlock } doReturn BestBlock(blockHash = "bestBlockHash", height = height) + } + whenever(lightningRepo.getStatus()).thenReturn(status) + } + + private fun confirmedCommand( + txid: String, + details: TransactionDetails, + blockHeight: UInt, + includeNotification: Boolean = false, + ) = NotifyPaymentReceived.Command.Onchain( + txid = txid, + details = details, + confirmedBlockHeight = blockHeight, + includeNotification = includeNotification, + ) } diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 26cfc992bc..dd2a1ecd0b 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -4470,6 +4470,61 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertEquals(sheet, sut.currentSheet.value) } + @Test + fun `confirmed-only onchain receive shows the sheet after updating the activity`() = test { + val sheetDetails = NewTransactionSheetDetails( + type = NewTransactionSheetType.ONCHAIN, + direction = NewTransactionSheetDirection.RECEIVED, + paymentHashOrTxId = "confirmed-txid", + sats = 1_000L, + ) + whenever(notifyPaymentReceivedHandler(any())) + .thenReturn(Result.success(NotifyPaymentReceived.Result.ShowSheet(sheetDetails))) + val details = TransactionDetails(amountSats = 1_000L, inputs = emptyList(), outputs = emptyList()) + + emitNodeEvent( + Event.OnchainTransactionConfirmed( + txid = "confirmed-txid", + blockHash = "block-hash", + blockHeight = 100u, + confirmationTime = 0uL, + details = details, + ), + ) + advanceUntilIdle() + + val expectedCommand = NotifyPaymentReceived.Command.Onchain( + txid = "confirmed-txid", + details = details, + confirmedBlockHeight = 100u, + ) + inOrder(activityRepo, notifyPaymentReceivedHandler) { + verify(activityRepo).handleOnchainTransactionConfirmed("confirmed-txid", details) + verify(notifyPaymentReceivedHandler).invoke(expectedCommand) + verify(notifyPaymentReceivedHandler).present(eq(expectedCommand), any(), any()) + } + assertEquals(sheetDetails, sut.transactionSheet.value) + } + + @Test + fun `confirmed-only onchain receive skips the handler during migration`() = test { + whenever(migrationService.needsPostMigrationSync()).thenReturn(true) + + emitNodeEvent( + Event.OnchainTransactionConfirmed( + txid = "confirmed-txid", + blockHash = "block-hash", + blockHeight = 100u, + confirmationTime = 0uL, + details = TransactionDetails(amountSats = 1_000L, inputs = emptyList(), outputs = emptyList()), + ), + ) + advanceUntilIdle() + + verify(notifyPaymentReceivedHandler, never()).invoke(any()) + assertEquals(NewTransactionSheetDetails.EMPTY, sut.transactionSheet.value) + } + @Test fun `received lightning payment is claimed by the UI while foregrounded`() = test { val details = NewTransactionSheetDetails( diff --git a/changelog.d/next/797.fixed.md b/changelog.d/next/797.fixed.md new file mode 100644 index 0000000000..0e4d1d53c6 --- /dev/null +++ b/changelog.d/next/797.fixed.md @@ -0,0 +1 @@ +Show the received transaction sheet or notification for on-chain deposits first seen already confirmed. From 7dce8e6ffdef354b323c437bc4977ad9181af35a Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 17 Sep 2026 06:31:47 -0300 Subject: [PATCH 02/10] fix: gate confirmed-only receives on block timestamp Co-Authored-By: Claude Opus 5 (1M context) --- .../domain/commands/NotifyPaymentReceived.kt | 11 +-- .../commands/NotifyPaymentReceivedHandler.kt | 30 +++++---- .../LightningNodeServiceTest.kt | 2 +- .../NotifyPaymentReceivedHandlerTest.kt | 67 +++++++++---------- .../viewmodels/AppViewModelSendFlowTest.kt | 2 +- 5 files changed, 56 insertions(+), 56 deletions(-) diff --git a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceived.kt b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceived.kt index a68ff4159c..61cb0737ac 100644 --- a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceived.kt +++ b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceived.kt @@ -16,16 +16,17 @@ sealed interface NotifyPaymentReceived { ) : Command /** - * An incoming onchain transaction. [confirmedBlockHeight] is set when the wallet first saw the - * transaction already confirmed, without a prior mempool event. + * An incoming onchain transaction. [confirmationTime] is the block timestamp in seconds since the + * UNIX epoch, set when the wallet first saw the transaction already confirmed without a prior + * mempool event. */ data class Onchain( val txid: String, val details: TransactionDetails, - val confirmedBlockHeight: UInt? = null, + val confirmationTime: ULong? = null, override val includeNotification: Boolean = false, ) : Command { - val isConfirmedOnly: Boolean get() = confirmedBlockHeight != null + val isConfirmedOnly: Boolean get() = confirmationTime != null } companion object { @@ -45,7 +46,7 @@ sealed interface NotifyPaymentReceived { is Event.OnchainTransactionConfirmed -> Onchain( txid = event.txid, details = event.details, - confirmedBlockHeight = event.blockHeight, + confirmationTime = event.confirmationTime, includeNotification = includeNotification, ) diff --git a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt index 12d46e145e..b2ecd1ef4d 100644 --- a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt +++ b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt @@ -4,6 +4,7 @@ import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.delay import kotlinx.coroutines.withContext import to.bitkit.di.IoDispatcher +import to.bitkit.ext.nowMillis import to.bitkit.ext.runSuspendCatching import to.bitkit.models.NewTransactionSheetDetails import to.bitkit.models.NewTransactionSheetDirection @@ -11,20 +12,24 @@ import to.bitkit.models.NewTransactionSheetType import to.bitkit.models.msatCeilOf import to.bitkit.repositories.ActivityRepo import to.bitkit.repositories.BackupRepo -import to.bitkit.repositories.LightningRepo import to.bitkit.services.MigrationService import to.bitkit.utils.Logger import javax.inject.Inject import javax.inject.Singleton -import kotlin.math.absoluteValue +import kotlin.time.Clock +import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.milliseconds +import kotlin.time.Duration.Companion.seconds +import kotlin.time.ExperimentalTime +@OptIn(ExperimentalTime::class) @Singleton class NotifyPaymentReceivedHandler @Inject constructor( @IoDispatcher private val ioDispatcher: CoroutineDispatcher, private val activityRepo: ActivityRepo, - private val lightningRepo: LightningRepo, private val backupRepo: BackupRepo, private val migrationService: MigrationService, + private val clock: Clock, private val receivedNotificationContent: ReceivedNotificationContent, ) { private val presentationClaimsLock = Any() @@ -121,7 +126,7 @@ class NotifyPaymentReceivedHandler @Inject constructor( } private suspend fun canShowConfirmedOnly(command: NotifyPaymentReceived.Command.Onchain): Boolean { - val blockHeight = command.confirmedBlockHeight ?: return false + val confirmationTime = command.confirmationTime ?: return false if (backupRepo.isRestoring.value) { Logger.debug("Skipping confirmed-only receive '${command.txid}' during restore", context = TAG) return false @@ -130,12 +135,10 @@ class NotifyPaymentReceivedHandler @Inject constructor( Logger.debug("Skipping confirmed-only receive '${command.txid}' during migration", context = TAG) return false } - val bestBlockHeight = lightningRepo.getStatus()?.currentBestBlock?.height - val depth = bestBlockHeight?.let { it.toLong() - blockHeight.toLong() } - if (depth == null || depth.absoluteValue > MAX_CONFIRMED_ONLY_BLOCK_DEPTH) { + val age = nowMillis(clock).milliseconds - confirmationTime.toLong().seconds + if (age.absoluteValue > MAX_CONFIRMED_ONLY_AGE) { Logger.debug( - "Skipping confirmed-only receive '${command.txid}' at height '$blockHeight' " + - "with best block '$bestBlockHeight'", + "Skipping confirmed-only receive '${command.txid}' confirmed at '$confirmationTime'", context = TAG, ) return false @@ -190,11 +193,12 @@ class NotifyPaymentReceivedHandler @Inject constructor( private const val MAX_RETRIES = 3 /** - * Max distance in blocks between a confirmed-only transaction and the node's best block for it to + * Max distance between a confirmed-only transaction's block timestamp and the device clock for it to * count as a new receive. Older confirmations, such as those replayed by a full wallet scan after a - * restore, stay silent. The distance is absolute because an unsynced best block (e.g. genesis on a - * fresh node) lags behind the wallet scan and must not let the replay through. + * restore, stay silent. The block timestamp is used instead of the node's best block height, which + * only advances with the lightning wallet sync and can lag the onchain sync that emits the event. + * The distance is absolute because block timestamps and device clocks can run ahead of each other. */ - private const val MAX_CONFIRMED_ONLY_BLOCK_DEPTH = 2L + private val MAX_CONFIRMED_ONLY_AGE = 1.hours } } diff --git a/app/src/test/java/to/bitkit/androidServices/LightningNodeServiceTest.kt b/app/src/test/java/to/bitkit/androidServices/LightningNodeServiceTest.kt index 2b6d47feaa..a8fc569efe 100644 --- a/app/src/test/java/to/bitkit/androidServices/LightningNodeServiceTest.kt +++ b/app/src/test/java/to/bitkit/androidServices/LightningNodeServiceTest.kt @@ -489,7 +489,7 @@ class LightningNodeServiceTest : BaseUnitTest() { val expectedCommand = NotifyPaymentReceived.Command.Onchain( txid = "confirmed_txid", details = details, - confirmedBlockHeight = 100u, + confirmationTime = 0uL, includeNotification = true, ) verify(notifyPaymentReceivedHandler).invoke(expectedCommand) diff --git a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt index 976d061dce..3119726da6 100644 --- a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt +++ b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt @@ -5,9 +5,7 @@ import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import org.junit.Before import org.junit.Test -import org.lightningdevkit.ldknode.BestBlock import org.lightningdevkit.ldknode.Event -import org.lightningdevkit.ldknode.NodeStatus import org.lightningdevkit.ldknode.TransactionDetails import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull @@ -28,7 +26,6 @@ import to.bitkit.models.WalletScope import to.bitkit.repositories.ActivityRepo import to.bitkit.repositories.BackupRepo import to.bitkit.repositories.CurrencyRepo -import to.bitkit.repositories.LightningRepo import to.bitkit.services.MigrationService import to.bitkit.test.BaseUnitTest import java.math.BigDecimal @@ -36,17 +33,24 @@ import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertTrue - +import kotlin.time.Clock +import kotlin.time.Duration +import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.minutes +import kotlin.time.ExperimentalTime +import kotlin.time.Instant + +@OptIn(ExperimentalTime::class) class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { companion object { - private const val BEST_BLOCK_HEIGHT = 1_000u + private val NOW = Instant.fromEpochSeconds(1_700_000_000L) } private val context: Context = mock() private val activityRepo: ActivityRepo = mock() private val currencyRepo: CurrencyRepo = mock() private val settingsStore: SettingsStore = mock() - private val lightningRepo: LightningRepo = mock() + private val clock: Clock = mock() private val backupRepo: BackupRepo = mock() private val migrationService: MigrationService = mock() private val isRestoring = MutableStateFlow(false) @@ -62,7 +66,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { whenever(backupRepo.isRestoring).thenReturn(isRestoring) whenever(migrationService.isShowingMigrationLoading).thenReturn(isShowingMigrationLoading) whenever { migrationService.needsPostMigrationSync() }.thenReturn(false) - givenBestBlockHeight(BEST_BLOCK_HEIGHT) + whenever(clock.now()).thenReturn(NOW) whenever(currencyRepo.convertSatsToFiat(any(), anyOrNull())).thenReturn( Result.success( ConvertedAmount( @@ -79,9 +83,9 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { sut = NotifyPaymentReceivedHandler( ioDispatcher = testDispatcher, activityRepo = activityRepo, - lightningRepo = lightningRepo, backupRepo = backupRepo, migrationService = migrationService, + clock = clock, receivedNotificationContent = ReceivedNotificationContent( context = context, currencyRepo = currencyRepo, @@ -336,7 +340,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { fun `confirmed-only recent onchain receive returns ShowSheet`() = test { val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = confirmedCommand(txid = "txidConfirmed", details = details, blockHeight = BEST_BLOCK_HEIGHT) + val command = confirmedCommand(txid = "txidConfirmed", details = details, age = Duration.ZERO) val result = sut(command).getOrThrow() @@ -362,7 +366,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { val command = confirmedCommand( txid = "txidConfirmed", details = details, - blockHeight = BEST_BLOCK_HEIGHT - 2u, + age = 59.minutes, includeNotification = true, ) @@ -374,10 +378,10 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { } @Test - fun `confirmed-only onchain receive ahead of an unsynced best block returns ShowSheet within depth`() = test { + fun `confirmed-only onchain receive slightly ahead of the device clock returns ShowSheet`() = test { val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = confirmedCommand(txid = "txidAhead", details = details, blockHeight = BEST_BLOCK_HEIGHT + 1u) + val command = confirmedCommand(txid = "txidAhead", details = details, age = (-5).minutes) val result = sut(command).getOrThrow() @@ -389,7 +393,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) val received = NotifyPaymentReceived.Command.Onchain(txid = "txidOnce", details = details) - val confirmed = confirmedCommand(txid = "txidOnce", details = details, blockHeight = BEST_BLOCK_HEIGHT) + val confirmed = confirmedCommand(txid = "txidOnce", details = details, age = Duration.ZERO) var presentationCount = 0 val receivedResult = sut(received).getOrThrow() @@ -405,10 +409,10 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { } @Test - fun `confirmed-only onchain receive at an old height returns Skip`() = test { + fun `confirmed-only onchain receive confirmed outside the recent window returns Skip`() = test { val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = confirmedCommand(txid = "txidOld", details = details, blockHeight = BEST_BLOCK_HEIGHT - 3u) + val command = confirmedCommand(txid = "txidOld", details = details, age = 61.minutes) val result = sut(command).getOrThrow() @@ -418,11 +422,10 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { } @Test - fun `confirmed-only onchain receive far ahead of the best block returns Skip`() = test { - givenBestBlockHeight(0u) + fun `confirmed-only onchain receive replayed from old history returns Skip`() = test { val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = confirmedCommand(txid = "txidReplay", details = details, blockHeight = BEST_BLOCK_HEIGHT) + val command = confirmedCommand(txid = "txidReplay", details = details, age = (24 * 365).hours) val result = sut(command).getOrThrow() @@ -431,11 +434,10 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { } @Test - fun `confirmed-only onchain receive returns Skip when the best block is unknown`() = test { - whenever(lightningRepo.getStatus()).thenReturn(null) + fun `confirmed-only onchain receive far ahead of the device clock returns Skip`() = test { val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = confirmedCommand(txid = "txidUnknown", details = details, blockHeight = BEST_BLOCK_HEIGHT) + val command = confirmedCommand(txid = "txidFuture", details = details, age = (-2).hours) val result = sut(command).getOrThrow() @@ -447,7 +449,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { fun `confirmed-only onchain send returns Skip`() = test { val details = TransactionDetails(amountSats = -5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = confirmedCommand(txid = "txidSent", details = details, blockHeight = BEST_BLOCK_HEIGHT) + val command = confirmedCommand(txid = "txidSent", details = details, age = Duration.ZERO) val result = sut(command).getOrThrow() @@ -461,7 +463,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { isRestoring.value = true val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = confirmedCommand(txid = "txidRestore", details = details, blockHeight = BEST_BLOCK_HEIGHT) + val command = confirmedCommand(txid = "txidRestore", details = details, age = Duration.ZERO) val result = sut(command).getOrThrow() @@ -475,7 +477,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { whenever(migrationService.needsPostMigrationSync()).thenReturn(true) val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) - val command = confirmedCommand(txid = "txidMigration", details = details, blockHeight = BEST_BLOCK_HEIGHT) + val command = confirmedCommand(txid = "txidMigration", details = details, age = Duration.ZERO) val result = sut(command).getOrThrow() @@ -489,8 +491,8 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { val event = Event.OnchainTransactionConfirmed( txid = "txidMapped", blockHash = "blockHash", - blockHeight = BEST_BLOCK_HEIGHT, - confirmationTime = 0uL, + blockHeight = 100u, + confirmationTime = 1_700_000_000uL, details = details, ) @@ -500,29 +502,22 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { NotifyPaymentReceived.Command.Onchain( txid = "txidMapped", details = details, - confirmedBlockHeight = BEST_BLOCK_HEIGHT, + confirmationTime = 1_700_000_000uL, includeNotification = true, ), command, ) } - private fun givenBestBlockHeight(height: UInt) { - val status = mock { - on { currentBestBlock } doReturn BestBlock(blockHash = "bestBlockHash", height = height) - } - whenever(lightningRepo.getStatus()).thenReturn(status) - } - private fun confirmedCommand( txid: String, details: TransactionDetails, - blockHeight: UInt, + age: Duration, includeNotification: Boolean = false, ) = NotifyPaymentReceived.Command.Onchain( txid = txid, details = details, - confirmedBlockHeight = blockHeight, + confirmationTime = (NOW - age).epochSeconds.toULong(), includeNotification = includeNotification, ) } diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index dd2a1ecd0b..c9aea0238d 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -4496,7 +4496,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { val expectedCommand = NotifyPaymentReceived.Command.Onchain( txid = "confirmed-txid", details = details, - confirmedBlockHeight = 100u, + confirmationTime = 0uL, ) inOrder(activityRepo, notifyPaymentReceivedHandler) { verify(activityRepo).handleOnchainTransactionConfirmed("confirmed-txid", details) From 9d3fb9bbe21d67312f1fbe7821e1abf13987edf1 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Thu, 17 Sep 2026 19:20:45 -0300 Subject: [PATCH 03/10] fix: skip redundant confirmed activity update Co-Authored-By: Claude Opus 5 (1M context) --- .../commands/NotifyPaymentReceivedHandler.kt | 10 ++++- .../NotifyPaymentReceivedHandlerTest.kt | 43 +++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt index b2ecd1ef4d..229faef218 100644 --- a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt +++ b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt @@ -111,7 +111,7 @@ class NotifyPaymentReceivedHandler @Inject constructor( if (command.isConfirmedOnly) { if (command.details.amountSats <= 0) return false if (!canShowConfirmedOnly(command)) return false - activityRepo.handleOnchainTransactionConfirmed(command.txid, command.details) + applyConfirmationIfMissing(command) } else { activityRepo.handleOnchainTransactionReceived(command.txid, command.details) if (command.details.amountSats <= 0) return false @@ -125,6 +125,14 @@ class NotifyPaymentReceivedHandler @Inject constructor( return shouldShowSheet } + private suspend fun applyConfirmationIfMissing(command: NotifyPaymentReceived.Command.Onchain) { + if (activityRepo.getOnchainActivityByTxId(command.txid)?.confirmed == true) { + Logger.debug("Skipping confirmed activity update for '${command.txid}', already applied", context = TAG) + return + } + activityRepo.handleOnchainTransactionConfirmed(command.txid, command.details) + } + private suspend fun canShowConfirmedOnly(command: NotifyPaymentReceived.Command.Onchain): Boolean { val confirmationTime = command.confirmationTime ?: return false if (backupRepo.isRestoring.value) { diff --git a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt index 3119726da6..ae617f2519 100644 --- a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt +++ b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt @@ -1,6 +1,8 @@ package to.bitkit.domain.commands import android.content.Context +import com.synonym.bitkitcore.OnchainActivity +import com.synonym.bitkitcore.PaymentType import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf import org.junit.Before @@ -19,6 +21,7 @@ import org.mockito.kotlin.whenever import to.bitkit.R import to.bitkit.data.SettingsData import to.bitkit.data.SettingsStore +import to.bitkit.ext.create import to.bitkit.models.ConvertedAmount import to.bitkit.models.NewTransactionSheetDirection import to.bitkit.models.NewTransactionSheetType @@ -359,6 +362,35 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { verify(activityRepo).markOnchainActivityAsSeen("txidConfirmed", WalletScope.default) } + @Test + fun `confirmed-only onchain receive does not reapply a confirmation already stored`() = test { + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + whenever(activityRepo.getOnchainActivityByTxId(eq("txidStored"), eq(WalletScope.default))) + .thenReturn(onchainActivity(txId = "txidStored", confirmed = true)) + val command = confirmedCommand(txid = "txidStored", details = details, age = Duration.ZERO) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.ShowSheet) + verify(activityRepo, never()).handleOnchainTransactionConfirmed(any(), any()) + verify(activityRepo).shouldShowReceivedSheet("txidStored", 5000uL) + } + + @Test + fun `confirmed-only onchain receive applies the confirmation when the activity is still unconfirmed`() = test { + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + whenever(activityRepo.getOnchainActivityByTxId(eq("txidUnconfirmed"), eq(WalletScope.default))) + .thenReturn(onchainActivity(txId = "txidUnconfirmed", confirmed = false)) + val command = confirmedCommand(txid = "txidUnconfirmed", details = details, age = Duration.ZERO) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.ShowSheet) + verify(activityRepo).handleOnchainTransactionConfirmed("txidUnconfirmed", details) + } + @Test fun `confirmed-only onchain receive returns ShowNotification when includeNotification is true`() = test { val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) @@ -509,6 +541,17 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { ) } + private fun onchainActivity(txId: String, confirmed: Boolean) = OnchainActivity.create( + id = txId, + txType = PaymentType.RECEIVED, + txId = txId, + value = 5000uL, + fee = 100uL, + address = "bc1test", + timestamp = 1_700_000_000uL, + confirmed = confirmed, + ) + private fun confirmedCommand( txid: String, details: TransactionDetails, From 3460f17c571821970bb7308183c09a0461b55547 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 18 Sep 2026 10:21:55 -0300 Subject: [PATCH 04/10] docs: add onchain receive journeys Co-Authored-By: Claude Opus 5 (1M context) --- journeys/README.md | 2 ++ journeys/onchain-receive/README.md | 25 +++++++++++++++++++ ...confirmed-only-background-notification.xml | 22 ++++++++++++++++ .../confirmed-only-received-sheet.xml | 25 +++++++++++++++++++ .../mempool-then-confirmed-single-sheet.xml | 21 ++++++++++++++++ 5 files changed, 95 insertions(+) create mode 100644 journeys/onchain-receive/README.md create mode 100644 journeys/onchain-receive/confirmed-only-background-notification.xml create mode 100644 journeys/onchain-receive/confirmed-only-received-sheet.xml create mode 100644 journeys/onchain-receive/mempool-then-confirmed-single-sheet.xml diff --git a/journeys/README.md b/journeys/README.md index f1d8ab2005..c577e0a7b1 100644 --- a/journeys/README.md +++ b/journeys/README.md @@ -126,6 +126,7 @@ fixtures, push notifications) live in each suite's README. | [lnurl](lnurl) | 1 | LNURL-pay comment kept as the activity note; needs an LNURL-pay endpoint that allows comments; no README | | [node-lifecycle](node-lifecycle) | 1 | Detached LDK restart completes; a cancelled RGS server change reconciles and recovers to Running; reads the app log; no README | | [notification-permission](notification-permission) | 4 | Background-setup toggles | +| [onchain-receive](onchain-receive) | 3 | Received sheet and notification for mempool-first and confirmed-only deposits | | [payment-requests](payment-requests) | 2 | Requires a linked fixture issuer; rejected shapes are unit fixtures | | [pubky-marketplace](pubky-marketplace) | 1 | Two-wallet Paykit marketplace payment; integration fixture required | | [receive](receive) | 1 | Receive sheet tab selection; needs a spending channel, no README | @@ -165,6 +166,7 @@ Known differences in the corpus, as of the iOS port (synonymdev/bitkit-ios#691): | `send/own-invoice-guard.xml` | not ported — iOS has no own-invoice guard | | `settings/electrum-server-error-toasts.xml` | not ported — iOS still shows one generic message for every manual Electrum connect failure | | `transfers/closed-channel-transfer-settles.xml` | not ported — the closed-channel and order-closure settle rules are an iOS follow-up | +| `onchain-receive/*` | not ported — bitkit-ios#455 tracks the same confirmed-only bug | | `deeplinks/*` | not ported — iOS registers the `bitkit` scheme but has no screen or sheet router | | `backup-restore/restore-keeps-tags-and-closed-channels.xml` | not ported yet — iOS already gates uploads across the whole restore (`AppScene.restoreFromMostRecentBackup` sets `BackupService.setRestoring(true)` before the timestamp probe), but still applies the three activity slices in one block (`BackupService.performFullRestoreFromLatestBackup`), which is the half this journey pins; port it with the iOS slice fix | | `shop/gift-card-category-titles.xml` | not ported — iOS still hardcodes the category names, and its route in has no screen deeplink | diff --git a/journeys/onchain-receive/README.md b/journeys/onchain-receive/README.md new file mode 100644 index 0000000000..6aa704af77 --- /dev/null +++ b/journeys/onchain-receive/README.md @@ -0,0 +1,25 @@ +# Onchain receive journeys + +These journeys cover the received sheet and notification for onchain deposits (issue #797). + +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. Both events go through +`NotifyPaymentReceivedHandler`, from `AppViewModel` in the foreground and `LightningNodeService` in +the background. + +A confirmed-only receive is shown only when its block timestamp is within one hour of the device +clock and no restore or migration is running. A full scan after a restore replays old confirmations +and stays silent; that case cannot be driven on a funded device and is covered by +`NotifyPaymentReceivedHandlerTest.kt`. + +## Preconditions + +- Onboarded regtest wallet with the node running. Fund and mine with the `lsp` helper at the repo root. +- Wallet sync runs every 10s. For the confirmed-only journeys, run the deposit and the mine in one + shell command, then check the log: an `OnchainTransactionReceived` line for the txid means the + sync saw the mempool first and the run tested the other path. +- The background journey needs background payments enabled (Settings > Notifications). +- Every event the node emits is logged by `LightningService` as `LDK event fired: ` under the + `APP` logcat tag, so `adb logcat -d -s APP:V` is enough to tell the two paths apart. +- The Receive sheet's tabs carry no test tag; select the Savings tab by its label. diff --git a/journeys/onchain-receive/confirmed-only-background-notification.xml b/journeys/onchain-receive/confirmed-only-background-notification.xml new file mode 100644 index 0000000000..58bf496a1d --- /dev/null +++ b/journeys/onchain-receive/confirmed-only-background-notification.xml @@ -0,0 +1,22 @@ + + + Covers issue #797 on the LightningNodeService path. With background payments enabled and the app + in the background, a deposit first seen already confirmed must post exactly one "Payment + Received" notification. + + Precondition: onboarded regtest wallet with background payments enabled, so the "Bitkit is + running in background" foreground service notification is present. + + + Tap Receive (testTag "Receive") and verify the Receive sheet opens (testTag "ReceiveScreen") + Tap the "Savings" receive tab (the tab row carries no test tag), tap "Show Details" (testTag "ShowDetails") and read the address from testTag "ReceiveOnchainAddress" + Press back, then send the app to the background: adb shell input keyevent KEYCODE_HOME + Run in one command: ./lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":7970}' && ./lsp POST /regtest/chain/mine '{"count":1}' + Wait 40s + Run: adb shell dumpsys notification --noredact | grep -A3 "pkg=to.bitkit.dev" + Verify exactly one notification titled "Payment Received" is posted, with body "Received <amount>" carrying the fiat and BTC amounts in the order set by the primary display setting + Open the notification shade and tap the "Payment Received" notification + Verify the app opens with the received sheet (testTag "ReceivedTransaction") showing the deposited amount + Tap the sheet button (testTag "ReceivedTransactionButton") and verify the home screen shows with no second sheet + + 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 0000000000..7f4dbc286f --- /dev/null +++ b/journeys/onchain-receive/confirmed-only-received-sheet.xml @@ -0,0 +1,25 @@ + + + Covers issue #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 OnchainTransactionReceived for the txid, the + run tested the mempool path instead; repeat with a new address. + + + Tap Receive (testTag "Receive") and verify the Receive sheet opens (testTag "ReceiveScreen") + Tap the "Savings" receive tab (the tab row carries no test tag), tap "Show Details" (testTag "ShowDetails") and read the address from testTag "ReceiveOnchainAddress" + Press back to return to the home screen + Run: adb logcat -c + Run in one command: ./lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":21797}' && ./lsp POST /regtest/chain/mine '{"count":1}' + Wait up to 30s for the next wallet sync + Run: adb logcat -d -s APP:V | grep <txid> + Verify the log shows an "LDK event fired" line with OnchainTransactionConfirmed for the txid and no OnchainTransactionReceived for it + Verify the received sheet (testTag "ReceivedTransaction") is visible with the deposited amount (testTag "MoneyText") + Tap the sheet button (testTag "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 0000000000..6f912d135a --- /dev/null +++ b/journeys/onchain-receive/mempool-then-confirmed-single-sheet.xml @@ -0,0 +1,21 @@ + + + Covers issue #797. Confirmed events now reach the received-payment handler, so a deposit seen + in the mempool first must not show a second sheet when it confirms. The handler dedupes on the + txid and on the persisted seen state. + + Precondition: onboarded regtest wallet, node running, app in the foreground on the home screen. + + + Tap Receive (testTag "Receive") and verify the Receive sheet opens (testTag "ReceiveScreen") + Tap the "Savings" receive tab (the tab row carries no test tag), tap "Show Details" (testTag "ShowDetails") and read the address from testTag "ReceiveOnchainAddress" + Press back to return to the home screen + Run: ./lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":14797}' + Wait up to 30s and verify the received sheet (testTag "ReceivedTransaction") is visible with the deposited amount + Tap the sheet button (testTag "ReceivedTransactionButton") + Run: ./lsp POST /regtest/chain/mine '{"count":1}' + Run: adb logcat -d -s APP:V | grep <txid> and wait until an "LDK event fired" line shows OnchainTransactionConfirmed + Wait 30s after that event and verify the received sheet does not appear again + Verify no "Payment Received" notification is posted: adb shell dumpsys notification --noredact | grep "Payment Received" + + From 97e4e50c33b0928e0f9f463c25d9742e56261b98 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 18 Sep 2026 11:06:37 -0300 Subject: [PATCH 05/10] docs: grep notification title in receive journey Co-Authored-By: Claude Opus 5 (1M context) --- .../confirmed-only-background-notification.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/journeys/onchain-receive/confirmed-only-background-notification.xml b/journeys/onchain-receive/confirmed-only-background-notification.xml index 58bf496a1d..a5834da7fb 100644 --- a/journeys/onchain-receive/confirmed-only-background-notification.xml +++ b/journeys/onchain-receive/confirmed-only-background-notification.xml @@ -13,8 +13,8 @@ Press back, then send the app to the background: adb shell input keyevent KEYCODE_HOME Run in one command: ./lsp POST /regtest/chain/deposit '{"address":"<savings addr>","amountSat":7970}' && ./lsp POST /regtest/chain/mine '{"count":1}' Wait 40s - Run: adb shell dumpsys notification --noredact | grep -A3 "pkg=to.bitkit.dev" - Verify exactly one notification titled "Payment Received" is posted, with body "Received <amount>" carrying the fiat and BTC amounts in the order set by the primary display setting + Run: adb shell dumpsys notification --noredact | grep -E "android\.title=String \(Payment Received\)|android\.text=String \(Received " + Verify exactly one "android.title=String (Payment Received)" line is printed, and an "android.text=String (Received <amount>)" line with it carrying the fiat and BTC amounts in the order set by the primary display setting. The foreground-service notification is titled "Bitkit Regtest" on the dev flavour, so it does not match either pattern Open the notification shade and tap the "Payment Received" notification Verify the app opens with the received sheet (testTag "ReceivedTransaction") showing the deposited amount Tap the sheet button (testTag "ReceivedTransactionButton") and verify the home screen shows with no second sheet From b40094ba9b742b58293685e1ce46837b842973bd Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Fri, 18 Sep 2026 11:07:23 -0300 Subject: [PATCH 06/10] docs: mark onchain receive ios port as pending Co-Authored-By: Claude Opus 5 (1M context) --- journeys/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/journeys/README.md b/journeys/README.md index c577e0a7b1..aad699bd2a 100644 --- a/journeys/README.md +++ b/journeys/README.md @@ -166,7 +166,7 @@ Known differences in the corpus, as of the iOS port (synonymdev/bitkit-ios#691): | `send/own-invoice-guard.xml` | not ported — iOS has no own-invoice guard | | `settings/electrum-server-error-toasts.xml` | not ported — iOS still shows one generic message for every manual Electrum connect failure | | `transfers/closed-channel-transfer-settles.xml` | not ported — the closed-channel and order-closure settle rules are an iOS follow-up | -| `onchain-receive/*` | not ported — bitkit-ios#455 tracks the same confirmed-only bug | +| `onchain-receive/*` | port pending in synonymdev/bitkit-ios#588, which wires `onchainTransactionConfirmed` into the same received-sheet flow but carries no `journeys/` files. Two adaptations when it lands: iOS suppresses replayed historical receives with a `pendingRestoreActivitySeen` flag cleared by the first post-restore on-chain sync, not the one-hour block-timestamp guard used here, so a stale-confirmation step has to drive a restore instead of a clock; and iOS has no foreground-service path, so `confirmed-only-background-notification.xml` has no counterpart | | `deeplinks/*` | not ported — iOS registers the `bitkit` scheme but has no screen or sheet router | | `backup-restore/restore-keeps-tags-and-closed-channels.xml` | not ported yet — iOS already gates uploads across the whole restore (`AppScene.restoreFromMostRecentBackup` sets `BackupService.setRestoring(true)` before the timestamp probe), but still applies the three activity slices in one block (`BackupService.performFullRestoreFromLatestBackup`), which is the half this journey pins; port it with the iOS slice fix | | `shop/gift-card-category-titles.xml` | not ported — iOS still hardcodes the category names, and its route in has no screen deeplink | From 6d3b0b5a07b2a8c54370a305b46453f17d741f07 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Mon, 21 Sep 2026 09:19:24 -0300 Subject: [PATCH 07/10] fix: hold onchain received sheets until the first sync after restore Co-Authored-By: Claude Opus 5 (1M context) --- .../main/java/to/bitkit/data/SettingsStore.kt | 6 ++ .../commands/NotifyPaymentReceivedHandler.kt | 9 +++ .../java/to/bitkit/viewmodels/AppViewModel.kt | 14 +++- .../to/bitkit/viewmodels/WalletViewModel.kt | 8 ++- .../NotifyPaymentReceivedHandlerTest.kt | 67 ++++++++++++++++++- .../java/to/bitkit/ui/WalletViewModelTest.kt | 35 ++++++++++ .../viewmodels/AppViewModelSendFlowTest.kt | 36 ++++++++++ journeys/onchain-receive/README.md | 10 ++- 8 files changed, 176 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/to/bitkit/data/SettingsStore.kt b/app/src/main/java/to/bitkit/data/SettingsStore.kt index c1d970ce05..6983abf59a 100644 --- a/app/src/main/java/to/bitkit/data/SettingsStore.kt +++ b/app/src/main/java/to/bitkit/data/SettingsStore.kt @@ -198,6 +198,12 @@ data class SettingsData( val selectedAddressType: String = DEFAULT_ADDRESS_TYPE_STRING, val addressTypesToMonitor: List = listOf(DEFAULT_ADDRESS_TYPE_STRING), val pendingRestoreAddressTypePrune: Boolean = false, + /** + * After a seed restore, suppresses the on-chain received sheet for historical transactions replayed by the + * post-restore sync. Set when the user taps Get Started on the restore success screen and cleared by the + * first on-chain sync completion after that, which marks the replayed activities as seen. + */ + val pendingRestoreActivitySeen: Boolean = false, ) data class BalanceUnitSwitch( diff --git a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt index 229faef218..26552f880b 100644 --- a/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt +++ b/app/src/main/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandler.kt @@ -2,7 +2,9 @@ package to.bitkit.domain.commands import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext +import to.bitkit.data.SettingsStore import to.bitkit.di.IoDispatcher import to.bitkit.ext.nowMillis import to.bitkit.ext.runSuspendCatching @@ -23,12 +25,14 @@ import kotlin.time.Duration.Companion.seconds import kotlin.time.ExperimentalTime @OptIn(ExperimentalTime::class) +@Suppress("LongParameterList") @Singleton class NotifyPaymentReceivedHandler @Inject constructor( @IoDispatcher private val ioDispatcher: CoroutineDispatcher, private val activityRepo: ActivityRepo, private val backupRepo: BackupRepo, private val migrationService: MigrationService, + private val settingsStore: SettingsStore, private val clock: Clock, private val receivedNotificationContent: ReceivedNotificationContent, ) { @@ -117,6 +121,11 @@ class NotifyPaymentReceivedHandler @Inject constructor( if (command.details.amountSats <= 0) return false } + if (settingsStore.data.first().pendingRestoreActivitySeen) { + Logger.debug("Skipping onchain receive '${command.txid}' until the first sync after restore", context = TAG) + return false + } + delay(DELAY_FOR_ACTIVITY_SYNC_MS) val shouldShowSheet = retryShouldShowReceivedSheet( command.txid, diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 3e0e7fd1ef..9719a577b0 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -74,6 +74,7 @@ import org.lightningdevkit.ldknode.NodeException import org.lightningdevkit.ldknode.PaymentFailureReason import org.lightningdevkit.ldknode.PaymentId import org.lightningdevkit.ldknode.SpendableUtxo +import org.lightningdevkit.ldknode.SyncType import org.lightningdevkit.ldknode.Txid import to.bitkit.BuildConfig import to.bitkit.R @@ -1336,7 +1337,7 @@ class AppViewModel @Inject constructor( is Event.ProbeSuccessful -> Unit is Event.SpliceFailed -> Unit is Event.SplicePending -> Unit - is Event.SyncCompleted -> handleSyncCompleted() + is Event.SyncCompleted -> handleSyncCompleted(event) is Event.SyncProgress -> Unit } }.onFailure { e -> @@ -1424,7 +1425,9 @@ class AppViewModel @Inject constructor( } } - private suspend fun handleSyncCompleted() { + private suspend fun handleSyncCompleted(event: Event.SyncCompleted) { + if (event.syncType == SyncType.ONCHAIN_WALLET) completePendingRestoreActivitySeen() + val isShowingLoading = migrationService.isShowingMigrationLoading.value val isRestoringRemote = migrationService.isRestoringFromRNRemoteBackup.value val needsPostMigrationSync = migrationService.needsPostMigrationSync() @@ -1454,6 +1457,13 @@ class AppViewModel @Inject constructor( } } + private suspend fun completePendingRestoreActivitySeen() { + if (!settingsStore.data.first().pendingRestoreActivitySeen) return + Logger.info("Marking activities replayed by the first sync after restore as seen", context = TAG) + activityRepo.markAllUnseenActivitiesAsSeen() + settingsStore.update { it.copy(pendingRestoreActivitySeen = false) } + } + private suspend fun completeRNRemoteBackupRestore() { val channelMigration = buildChannelMigrationIfAvailable() diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index 0315c8bbd2..99d3a53f64 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -254,8 +254,12 @@ class WalletViewModel @Inject constructor( fun onRestoreContinue() { viewModelScope.launch(bgDispatcher) { - if (!settingsStore.restoredMonitoredTypesFromBackup) { - settingsStore.update { it.copy(pendingRestoreAddressTypePrune = true) } + val shouldPrune = !settingsStore.restoredMonitoredTypesFromBackup + settingsStore.update { + it.copy( + pendingRestoreAddressTypePrune = it.pendingRestoreAddressTypePrune || shouldPrune, + pendingRestoreActivitySeen = true, + ) } } _restoreState.update { RestoreState.Settled } diff --git a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt index ae617f2519..0d78fcec74 100644 --- a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt +++ b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt @@ -4,7 +4,6 @@ import android.content.Context import com.synonym.bitkitcore.OnchainActivity import com.synonym.bitkitcore.PaymentType import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.flowOf import org.junit.Before import org.junit.Test import org.lightningdevkit.ldknode.Event @@ -58,6 +57,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { private val migrationService: MigrationService = mock() private val isRestoring = MutableStateFlow(false) private val isShowingMigrationLoading = MutableStateFlow(false) + private val settingsData = MutableStateFlow(SettingsData()) private lateinit var sut: NotifyPaymentReceivedHandler @@ -65,7 +65,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { fun setUp() { whenever(context.getString(R.string.notification__received__title)).thenReturn("Payment Received") whenever(context.getString(any(), any())).thenReturn("Received amount") - whenever(settingsStore.data).thenReturn(flowOf(SettingsData())) + whenever(settingsStore.data).thenReturn(settingsData) whenever(backupRepo.isRestoring).thenReturn(isRestoring) whenever(migrationService.isShowingMigrationLoading).thenReturn(isShowingMigrationLoading) whenever { migrationService.needsPostMigrationSync() }.thenReturn(false) @@ -88,6 +88,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { activityRepo = activityRepo, backupRepo = backupRepo, migrationService = migrationService, + settingsStore = settingsStore, clock = clock, receivedNotificationContent = ReceivedNotificationContent( context = context, @@ -517,6 +518,68 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { verify(activityRepo, never()).shouldShowReceivedSheet(any(), any()) } + @Test + fun `onchain mempool receive returns Skip while the first sync after restore is pending`() = test { + settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = NotifyPaymentReceived.Command.Onchain(txid = "txidRestored", details = details) + + val result = sut(command).getOrThrow() + + assertEquals(NotifyPaymentReceived.Result.Skip, result) + verify(activityRepo).handleOnchainTransactionReceived("txidRestored", details) + verify(activityRepo, never()).shouldShowReceivedSheet(any(), any()) + verify(activityRepo, never()).markOnchainActivityAsSeen(any(), any()) + } + + @Test + fun `confirmed-only onchain receive returns Skip while the first sync after restore is pending`() = test { + settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val command = confirmedCommand(txid = "txidRestored", details = details, age = Duration.ZERO) + + val result = sut(command).getOrThrow() + + assertEquals(NotifyPaymentReceived.Result.Skip, result) + verify(activityRepo).handleOnchainTransactionConfirmed("txidRestored", details) + verify(activityRepo, never()).shouldShowReceivedSheet(any(), any()) + verify(activityRepo, never()).markOnchainActivityAsSeen(any(), any()) + } + + @Test + fun `confirmed-only onchain receive already marked seen after restore returns Skip`() = test { + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet("txidHistorical", 5000uL)).thenReturn(false) + val command = confirmedCommand(txid = "txidHistorical", details = details, age = Duration.ZERO) + + val result = sut(command).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.Skip) + verify(activityRepo, never()).markOnchainActivityAsSeen(any(), any()) + } + + @Test + fun `onchain receive notifies again once the first sync after restore is done`() = test { + settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) + whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) + val historical = confirmedCommand(txid = "txidHistorical", details = details, age = Duration.ZERO) + val fresh = NotifyPaymentReceived.Command.Onchain(txid = "txidFresh", details = details) + + assertEquals(NotifyPaymentReceived.Result.Skip, sut(historical).getOrThrow()) + + settingsData.value = SettingsData(pendingRestoreActivitySeen = false) + val result = sut(fresh).getOrThrow() + + assertTrue(result is NotifyPaymentReceived.Result.ShowSheet) + assertEquals("txidFresh", result.sheet.paymentHashOrTxId) + assertTrue(sut.present(fresh) {}) + verify(activityRepo).markOnchainActivityAsSeen("txidFresh", WalletScope.default) + verify(activityRepo, never()).markOnchainActivityAsSeen("txidHistorical", WalletScope.default) + } + @Test fun `from maps a confirmed onchain event to a confirmed-only command`() { val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) diff --git a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt index 9cbf3af699..3240226099 100644 --- a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt @@ -42,6 +42,8 @@ import to.bitkit.utils.AppError import to.bitkit.viewmodels.RestoreState import to.bitkit.viewmodels.WalletViewModel import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue @OptIn(ExperimentalCoroutinesApi::class) class WalletViewModelTest : BaseUnitTest() { @@ -252,6 +254,29 @@ class WalletViewModelTest : BaseUnitTest() { assertEquals(RestoreState.Settled, sut.restoreState.value) } + @Test + fun `onRestoreContinue should defer marking restored activities seen until the first onchain sync`() = test { + val settingsData = stubSettingsUpdate() + + sut.onRestoreContinue() + advanceUntilIdle() + + assertTrue(settingsData.value.pendingRestoreActivitySeen) + assertTrue(settingsData.value.pendingRestoreAddressTypePrune) + } + + @Test + fun `onRestoreContinue should skip address type pruning when backup had monitored types`() = test { + whenever(settingsStore.restoredMonitoredTypesFromBackup).thenReturn(true) + val settingsData = stubSettingsUpdate() + + sut.onRestoreContinue() + advanceUntilIdle() + + assertTrue(settingsData.value.pendingRestoreActivitySeen) + assertFalse(settingsData.value.pendingRestoreAddressTypePrune) + } + @Test fun `onProceedWithoutRestore should exit restore flow`() = test { val testError = Exception("Test error") @@ -500,4 +525,14 @@ class WalletViewModelTest : BaseUnitTest() { verify(testWalletRepo, never()).refreshBip21() } + + private fun stubSettingsUpdate(): MutableStateFlow { + val settingsData = MutableStateFlow(SettingsData()) + whenever { settingsStore.update(any()) }.thenAnswer { + val transform = it.getArgument<(SettingsData) -> SettingsData>(0) + settingsData.value = transform(settingsData.value) + Unit + } + return settingsData + } } diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index c9aea0238d..b6180dcbe0 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -53,6 +53,7 @@ import org.lightningdevkit.ldknode.Event import org.lightningdevkit.ldknode.NodeException import org.lightningdevkit.ldknode.PaymentFailureReason import org.lightningdevkit.ldknode.SpendableUtxo +import org.lightningdevkit.ldknode.SyncType import org.lightningdevkit.ldknode.TransactionDetails import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull @@ -4506,6 +4507,41 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertEquals(sheetDetails, sut.transactionSheet.value) } + @Test + fun `first onchain sync after restore marks unseen activities seen and clears the pending flag`() = test { + settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + whenever { activityRepo.markAllUnseenActivitiesAsSeen() }.thenReturn(Result.success(Unit)) + + emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) + advanceUntilIdle() + + inOrder(activityRepo, settingsStore) { + verify(activityRepo).markAllUnseenActivitiesAsSeen() + verify(settingsStore).update(any()) + } + assertFalse(settingsData.value.pendingRestoreActivitySeen) + } + + @Test + fun `lightning sync after restore keeps the pending flag and activities untouched`() = test { + settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + + emitNodeEvent(Event.SyncCompleted(syncType = SyncType.LIGHTNING_WALLET, syncedBlockHeight = 100u)) + advanceUntilIdle() + + verify(activityRepo, never()).markAllUnseenActivitiesAsSeen() + assertTrue(settingsData.value.pendingRestoreActivitySeen) + } + + @Test + fun `onchain sync without a pending restore leaves unseen activities untouched`() = test { + emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) + advanceUntilIdle() + + verify(activityRepo, never()).markAllUnseenActivitiesAsSeen() + verify(settingsStore, never()).update(any()) + } + @Test fun `confirmed-only onchain receive skips the handler during migration`() = test { whenever(migrationService.needsPostMigrationSync()).thenReturn(true) diff --git a/journeys/onchain-receive/README.md b/journeys/onchain-receive/README.md index 6aa704af77..69476d5890 100644 --- a/journeys/onchain-receive/README.md +++ b/journeys/onchain-receive/README.md @@ -9,9 +9,13 @@ it in the mempool produces only the confirmed event. Both events go through the background. A confirmed-only receive is shown only when its block timestamp is within one hour of the device -clock and no restore or migration is running. A full scan after a restore replays old confirmations -and stays silent; that case cannot be driven on a funded device and is covered by -`NotifyPaymentReceivedHandlerTest.kt`. +clock and no restore or migration is running. After a seed restore, Get Started sets +`pendingRestoreActivitySeen`, which holds every onchain received sheet and notification until the +first onchain sync completes; that sync marks all unseen activities as seen and clears the flag, so +the transactions it discovered stay silent when they later confirm while new deposits notify again. +The same rule ships on iOS in bitkit-ios#588. A full scan after a restore also replays old +confirmations, which the one-hour window keeps silent. Neither case can be driven on a funded +device; both are covered by `NotifyPaymentReceivedHandlerTest.kt` and `AppViewModelSendFlowTest.kt`. ## Preconditions From 87f62575ac2fe8440282722f16e0404ab7dc6f1a Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Mon, 21 Sep 2026 09:23:27 -0300 Subject: [PATCH 08/10] fix: keep restore hold when marking activities seen fails Co-Authored-By: Claude Opus 5 (1M context) --- .../main/java/to/bitkit/viewmodels/AppViewModel.kt | 5 +++-- .../bitkit/viewmodels/AppViewModelSendFlowTest.kt | 13 +++++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 9719a577b0..415b7abdd6 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -1460,8 +1460,9 @@ class AppViewModel @Inject constructor( private suspend fun completePendingRestoreActivitySeen() { if (!settingsStore.data.first().pendingRestoreActivitySeen) return Logger.info("Marking activities replayed by the first sync after restore as seen", context = TAG) - activityRepo.markAllUnseenActivitiesAsSeen() - settingsStore.update { it.copy(pendingRestoreActivitySeen = false) } + activityRepo.markAllUnseenActivitiesAsSeen().onSuccess { + settingsStore.update { settings -> settings.copy(pendingRestoreActivitySeen = false) } + } } private suspend fun completeRNRemoteBackupRestore() { diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index b6180dcbe0..e59aaa7655 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -4522,6 +4522,19 @@ class AppViewModelSendFlowTest : BaseUnitTest() { assertFalse(settingsData.value.pendingRestoreActivitySeen) } + @Test + fun `first onchain sync after restore keeps the pending flag when marking activities seen fails`() = test { + settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + whenever { activityRepo.markAllUnseenActivitiesAsSeen() } + .thenReturn(Result.failure(AppError("mark seen failed"))) + + emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) + advanceUntilIdle() + + verify(activityRepo).markAllUnseenActivitiesAsSeen() + assertTrue(settingsData.value.pendingRestoreActivitySeen) + } + @Test fun `lightning sync after restore keeps the pending flag and activities untouched`() = test { settingsData.value = SettingsData(pendingRestoreActivitySeen = true) From 2926fd7132d448fcef95f7150dc05a30477096e2 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Tue, 22 Sep 2026 10:53:21 -0300 Subject: [PATCH 09/10] fix: persist activity seen state through the core call Co-Authored-By: Claude Opus 5 (1M context) --- .../java/to/bitkit/services/CoreService.kt | 10 +-- .../to/bitkit/services/ActivityServiceTest.kt | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+), 7 deletions(-) create mode 100644 app/src/test/java/to/bitkit/services/ActivityServiceTest.kt diff --git a/app/src/main/java/to/bitkit/services/CoreService.kt b/app/src/main/java/to/bitkit/services/CoreService.kt index 5093c0b269..1764cff5e8 100644 --- a/app/src/main/java/to/bitkit/services/CoreService.kt +++ b/app/src/main/java/to/bitkit/services/CoreService.kt @@ -113,6 +113,7 @@ import com.synonym.bitkitcore.TxInput as BitkitCoreTxInput import com.synonym.bitkitcore.TxOutput as BitkitCoreTxOutput import com.synonym.bitkitcore.getLnurlInvoiceForPayData as coreGetLnurlInvoiceForPayData import com.synonym.bitkitcore.getTransactionDetails as getBitkitCoreTransactionDetails +import com.synonym.bitkitcore.markActivityAsSeen as coreMarkActivityAsSeen // region Core @@ -1695,18 +1696,13 @@ class ActivityService( walletId: String = defaultWalletId, seenAt: ULong? = null, ) = ServiceQueue.CORE.background { - val activity = getActivityById(walletId = walletId, activityId = activityId) ?: run { + if (getActivityById(walletId = walletId, activityId = activityId) == null) { Logger.warn("Cannot mark activity as seen - activity not found: $activityId", context = TAG) return@background } val timestamp = seenAt ?: nowTimestamp().epochSecond.toULong() - val updatedActivity = when (activity) { - is Activity.Lightning -> Activity.Lightning(activity.v1.copy(seenAt = timestamp)) - is Activity.Onchain -> Activity.Onchain(activity.v1.copy(seenAt = timestamp)) - } - - updateActivity(activityId = activityId, activity = updatedActivity) + coreMarkActivityAsSeen(walletId = walletId, activityId = activityId, seenAt = timestamp) Logger.info("Marked activity $activityId as seen at $timestamp", context = TAG) } diff --git a/app/src/test/java/to/bitkit/services/ActivityServiceTest.kt b/app/src/test/java/to/bitkit/services/ActivityServiceTest.kt new file mode 100644 index 0000000000..0836287fd2 --- /dev/null +++ b/app/src/test/java/to/bitkit/services/ActivityServiceTest.kt @@ -0,0 +1,78 @@ +package to.bitkit.services + +import com.synonym.bitkitcore.Activity +import com.synonym.bitkitcore.OnchainActivity +import com.synonym.bitkitcore.PaymentType +import org.junit.Test +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyLong +import org.mockito.ArgumentMatchers.anyString +import org.mockito.Mockito.mockStatic +import org.mockito.Mockito.never +import org.mockito.kotlin.mock +import to.bitkit.async.ServiceQueue +import to.bitkit.ext.create +import to.bitkit.test.BaseUnitTest + +class ActivityServiceTest : BaseUnitTest() { + companion object { + private const val WALLET_ID = "bitkit" + private const val ACTIVITY_ID = "activity" + private const val SEEN_AT = 1_790_000_000uL + } + + private val binding = Class.forName("com.synonym.bitkitcore.Bitkitcore_androidKt") + private val getActivityById = binding.getMethod("getActivityById", String::class.java, String::class.java) + private val updateActivity = binding.getMethod("updateActivity", String::class.java, Activity::class.java) + private val markActivityAsSeen = binding.methods.single { it.name.startsWith("markActivityAsSeen") } + + private val sut by lazy { + ActivityService( + coreService = mock(), + cacheStore = mock(), + lightningService = mock(), + settingsStore = mock(), + privatePaykitContactResolver = mock(), + ) + } + + @Test + fun `markActivityAsSeen persists the timestamp through the core seen call`() = test { + ServiceQueue.CORE.background { + mockStatic(binding).use { native -> + native.`when` { getActivityById.invoke(null, WALLET_ID, ACTIVITY_ID) }.thenReturn(activity()) + + sut.markActivityAsSeen(ACTIVITY_ID, walletId = WALLET_ID, seenAt = SEEN_AT) + + native.verify { markActivityAsSeen.invoke(null, WALLET_ID, ACTIVITY_ID, SEEN_AT.toLong()) } + native.verify({ updateActivity.invoke(null, anyString(), any(Activity::class.java)) }, never()) + } + } + } + + @Test + fun `markActivityAsSeen skips the core seen call when the activity is missing`() = test { + ServiceQueue.CORE.background { + mockStatic(binding).use { native -> + native.`when` { getActivityById.invoke(null, WALLET_ID, ACTIVITY_ID) }.thenReturn(null) + + sut.markActivityAsSeen(ACTIVITY_ID, walletId = WALLET_ID, seenAt = SEEN_AT) + + native.verify({ markActivityAsSeen.invoke(null, anyString(), anyString(), anyLong()) }, never()) + } + } + } + + private fun activity() = Activity.Onchain( + OnchainActivity.create( + walletId = WALLET_ID, + id = ACTIVITY_ID, + txType = PaymentType.RECEIVED, + txId = ACTIVITY_ID, + value = 1uL, + fee = 0uL, + address = "address", + timestamp = 1uL, + ) + ) +} From 73bd3bbe5b586a76818f64aea5fcf283d4737df0 Mon Sep 17 00:00:00 2001 From: jvsena42 Date: Wed, 23 Sep 2026 14:07:16 -0300 Subject: [PATCH 10/10] fix: arm restore sheet hold before the node starts syncing Co-Authored-By: Claude Opus 5 (1M context) --- .../main/java/to/bitkit/data/SettingsStore.kt | 16 ++++++---- .../to/bitkit/repositories/ActivityRepo.kt | 4 +-- .../java/to/bitkit/services/CoreService.kt | 15 +++++++++- .../ui/screens/trezor/TrezorPreviewData.kt | 6 ++-- .../java/to/bitkit/viewmodels/AppViewModel.kt | 8 +++-- .../to/bitkit/viewmodels/WalletViewModel.kt | 9 +++--- .../NotifyPaymentReceivedHandlerTest.kt | 11 ++++--- .../java/to/bitkit/ui/WalletViewModelTest.kt | 29 +++++++++++++++++-- .../viewmodels/AppViewModelSendFlowTest.kt | 23 +++++++++------ 9 files changed, 88 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/to/bitkit/data/SettingsStore.kt b/app/src/main/java/to/bitkit/data/SettingsStore.kt index 6983abf59a..1218ff9a6b 100644 --- a/app/src/main/java/to/bitkit/data/SettingsStore.kt +++ b/app/src/main/java/to/bitkit/data/SettingsStore.kt @@ -199,12 +199,18 @@ data class SettingsData( val addressTypesToMonitor: List = listOf(DEFAULT_ADDRESS_TYPE_STRING), val pendingRestoreAddressTypePrune: Boolean = false, /** - * After a seed restore, suppresses the on-chain received sheet for historical transactions replayed by the - * post-restore sync. Set when the user taps Get Started on the restore success screen and cleared by the - * first on-chain sync completion after that, which marks the replayed activities as seen. + * When a seed restore began, as epoch seconds, or 0 when no restore is being suppressed. + * + * Suppresses the on-chain received sheet for the historical transactions the post-restore sync replays. Set as + * the restore starts, before the node is started, because the node syncs long before the backup is read - setting + * it on the Get Started tap left a window where replayed transactions could still raise a sheet. Doubles as the + * cutoff for the sweep that marks those transactions seen, so a payment arriving mid-restore is not swept up with + * them. Cleared by the first on-chain sync completion whose sweep succeeds. */ - val pendingRestoreActivitySeen: Boolean = false, -) + val pendingRestoreActivitySeenSince: Long = 0, +) { + val pendingRestoreActivitySeen: Boolean get() = pendingRestoreActivitySeenSince > 0 +} data class BalanceUnitSwitch( val previousDisplay: PrimaryDisplay, diff --git a/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt b/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt index 13de7430dd..4119de267f 100644 --- a/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/ActivityRepo.kt @@ -1059,9 +1059,9 @@ class ActivityRepo @Inject constructor( return@withContext Result.success(Unit) } - suspend fun markAllUnseenActivitiesAsSeen(): Result = withContext(bgDispatcher) { + suspend fun markAllUnseenActivitiesAsSeen(startedBefore: ULong? = null): Result = withContext(bgDispatcher) { runCatching { - coreService.activity.markAllUnseenActivitiesAsSeen() + coreService.activity.markAllUnseenActivitiesAsSeen(startedBefore) notifyActivitiesChanged() }.onFailure { Logger.error("Failed to mark all activities as seen: $it", it, context = TAG) diff --git a/app/src/main/java/to/bitkit/services/CoreService.kt b/app/src/main/java/to/bitkit/services/CoreService.kt index 1764cff5e8..9d4ab907c8 100644 --- a/app/src/main/java/to/bitkit/services/CoreService.kt +++ b/app/src/main/java/to/bitkit/services/CoreService.kt @@ -1720,7 +1720,14 @@ class ActivityService( markActivityAsSeen(activity.id, walletId = activity.walletId, seenAt = seenAt) } - suspend fun markAllUnseenActivitiesAsSeen() = ServiceQueue.CORE.background { + /** + * Marks every unseen activity as seen. + * + * [startedBefore] limits the pass to activity that already existed at that epoch second. 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. + */ + suspend fun markAllUnseenActivitiesAsSeen(startedBefore: ULong? = null) = ServiceQueue.CORE.background { val timestamp = nowTimestamp().epochSecond.toULong() val activities = getActivities( walletId = null, @@ -1739,6 +1746,12 @@ class ActivityService( is Activity.Onchain -> activity.v1.seenAt != null is Activity.Lightning -> activity.v1.seenAt != null } + val createdAt = when (activity) { + is Activity.Onchain -> activity.v1.timestamp + is Activity.Lightning -> activity.v1.timestamp + } + + if (startedBefore != null && createdAt > startedBefore) continue if (!isSeen) { markActivityAsSeen(activity.rawId(), walletId = activity.walletId(), seenAt = timestamp) diff --git a/app/src/main/java/to/bitkit/ui/screens/trezor/TrezorPreviewData.kt b/app/src/main/java/to/bitkit/ui/screens/trezor/TrezorPreviewData.kt index 76795a937b..c1d8a5c16b 100644 --- a/app/src/main/java/to/bitkit/ui/screens/trezor/TrezorPreviewData.kt +++ b/app/src/main/java/to/bitkit/ui/screens/trezor/TrezorPreviewData.kt @@ -316,7 +316,8 @@ internal object TrezorPreviewData { val sampleWatcherActivities = listOf( Activity.Onchain( - OnchainActivity.create(walletId = "wallet0", + OnchainActivity.create( + walletId = "wallet0", id = SAMPLE_TXID, txType = PaymentType.RECEIVED, txId = SAMPLE_TXID, @@ -328,7 +329,8 @@ internal object TrezorPreviewData { ), ), Activity.Onchain( - OnchainActivity.create(walletId = "wallet0", + OnchainActivity.create( + walletId = "wallet0", id = "b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3", txType = PaymentType.SENT, txId = "b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3", diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index 415b7abdd6..405a6c2798 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -1458,10 +1458,12 @@ class AppViewModel @Inject constructor( } private suspend fun completePendingRestoreActivitySeen() { - if (!settingsStore.data.first().pendingRestoreActivitySeen) return + val restoreStartedAt = settingsStore.data.first().pendingRestoreActivitySeenSince + if (restoreStartedAt <= 0) return Logger.info("Marking activities replayed by the first sync after restore as seen", context = TAG) - activityRepo.markAllUnseenActivitiesAsSeen().onSuccess { - settingsStore.update { settings -> settings.copy(pendingRestoreActivitySeen = false) } + // Bounded by the restore start so a payment arriving mid-restore keeps its unseen state. + activityRepo.markAllUnseenActivitiesAsSeen(startedBefore = restoreStartedAt.toULong()).onSuccess { + settingsStore.update { settings -> settings.copy(pendingRestoreActivitySeenSince = 0) } } } diff --git a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt index 99d3a53f64..7ed466d66d 100644 --- a/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt @@ -29,6 +29,7 @@ import org.lightningdevkit.ldknode.PeerDetails import to.bitkit.R import to.bitkit.data.SettingsStore import to.bitkit.di.BgDispatcher +import to.bitkit.ext.nowTimestamp import to.bitkit.ext.of import to.bitkit.ext.runSuspendCatching import to.bitkit.models.Toast @@ -256,10 +257,7 @@ class WalletViewModel @Inject constructor( viewModelScope.launch(bgDispatcher) { val shouldPrune = !settingsStore.restoredMonitoredTypesFromBackup settingsStore.update { - it.copy( - pendingRestoreAddressTypePrune = it.pendingRestoreAddressTypePrune || shouldPrune, - pendingRestoreActivitySeen = true, - ) + it.copy(pendingRestoreAddressTypePrune = it.pendingRestoreAddressTypePrune || shouldPrune) } } _restoreState.update { RestoreState.Settled } @@ -568,6 +566,9 @@ class WalletViewModel @Inject constructor( // The node starts and syncs long before the backup is read, so ordinary uploads are held from // here rather than from the restore itself, which would upload over the backup it has not read. backupRepo.setRestorePending(true) + // Same reason for the received-sheet hold: by the time the user taps continue the node has + // already been replaying historical transactions for a while. + settingsStore.update { it.copy(pendingRestoreActivitySeenSince = nowTimestamp().epochSecond) } walletRepo.restoreWallet( mnemonic = mnemonic, diff --git a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt index 0d78fcec74..533adde3bb 100644 --- a/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt +++ b/app/src/test/java/to/bitkit/domain/commands/NotifyPaymentReceivedHandlerTest.kt @@ -46,6 +46,9 @@ import kotlin.time.Instant class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { companion object { private val NOW = Instant.fromEpochSeconds(1_700_000_000L) + + /** Stands in for the epoch second a seed restore began. */ + private const val RESTORE_STARTED_AT = 1_700_000_000L } private val context: Context = mock() @@ -520,7 +523,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { @Test fun `onchain mempool receive returns Skip while the first sync after restore is pending`() = test { - settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + settingsData.value = SettingsData(pendingRestoreActivitySeenSince = RESTORE_STARTED_AT) val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) val command = NotifyPaymentReceived.Command.Onchain(txid = "txidRestored", details = details) @@ -535,7 +538,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { @Test fun `confirmed-only onchain receive returns Skip while the first sync after restore is pending`() = test { - settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + settingsData.value = SettingsData(pendingRestoreActivitySeenSince = RESTORE_STARTED_AT) val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) val command = confirmedCommand(txid = "txidRestored", details = details, age = Duration.ZERO) @@ -562,7 +565,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { @Test fun `onchain receive notifies again once the first sync after restore is done`() = test { - settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + settingsData.value = SettingsData(pendingRestoreActivitySeenSince = RESTORE_STARTED_AT) val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList()) whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true) val historical = confirmedCommand(txid = "txidHistorical", details = details, age = Duration.ZERO) @@ -570,7 +573,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() { assertEquals(NotifyPaymentReceived.Result.Skip, sut(historical).getOrThrow()) - settingsData.value = SettingsData(pendingRestoreActivitySeen = false) + settingsData.value = SettingsData(pendingRestoreActivitySeenSince = 0) val result = sut(fresh).getOrThrow() assertTrue(result is NotifyPaymentReceived.Result.ShowSheet) diff --git a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt index 3240226099..12838a406b 100644 --- a/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt +++ b/app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt @@ -211,6 +211,20 @@ class WalletViewModelTest : BaseUnitTest() { verify(lightningRepo).setInitNodeLifecycleState() } + @Test + fun `restoreWallet should arm the received sheet hold before the node syncs`() = test { + // The node starts replaying historical txs as soon as the restore begins, so the hold has to + // be armed here rather than when the user taps continue. + whenever(walletRepo.restoreWallet(any(), anyOrNull())).thenReturn(Result.success(Unit)) + val settingsData = stubSettingsUpdate() + + sut.restoreWallet("test_mnemonic", null) + advanceUntilIdle() + + assertTrue(settingsData.value.pendingRestoreActivitySeen) + assertTrue(settingsData.value.pendingRestoreActivitySeenSince > 0) + } + @Test fun `addTagToSelected should call walletRepo addTagToSelected`() = test { sut.addTagToSelected("test_tag") @@ -255,13 +269,12 @@ class WalletViewModelTest : BaseUnitTest() { } @Test - fun `onRestoreContinue should defer marking restored activities seen until the first onchain sync`() = test { + fun `onRestoreContinue should request address type pruning`() = test { val settingsData = stubSettingsUpdate() sut.onRestoreContinue() advanceUntilIdle() - assertTrue(settingsData.value.pendingRestoreActivitySeen) assertTrue(settingsData.value.pendingRestoreAddressTypePrune) } @@ -273,10 +286,20 @@ class WalletViewModelTest : BaseUnitTest() { sut.onRestoreContinue() advanceUntilIdle() - assertTrue(settingsData.value.pendingRestoreActivitySeen) assertFalse(settingsData.value.pendingRestoreAddressTypePrune) } + @Test + fun `onRestoreContinue should not arm the received sheet hold, the restore already did`() = test { + // Regression: arming it here left the node replaying historical txs before the hold existed. + val settingsData = stubSettingsUpdate() + + sut.onRestoreContinue() + advanceUntilIdle() + + assertFalse(settingsData.value.pendingRestoreActivitySeen) + } + @Test fun `onProceedWithoutRestore should exit restore flow`() = test { val testError = Exception("Test error") diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index e59aaa7655..cde3f32cb7 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -4509,14 +4509,16 @@ class AppViewModelSendFlowTest : BaseUnitTest() { @Test fun `first onchain sync after restore marks unseen activities seen and clears the pending flag`() = test { - settingsData.value = SettingsData(pendingRestoreActivitySeen = true) - whenever { activityRepo.markAllUnseenActivitiesAsSeen() }.thenReturn(Result.success(Unit)) + settingsData.value = SettingsData(pendingRestoreActivitySeenSince = RESTORE_STARTED_AT) + whenever { + activityRepo.markAllUnseenActivitiesAsSeen(eq(RESTORE_STARTED_AT.toULong())) + }.thenReturn(Result.success(Unit)) emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) advanceUntilIdle() inOrder(activityRepo, settingsStore) { - verify(activityRepo).markAllUnseenActivitiesAsSeen() + verify(activityRepo).markAllUnseenActivitiesAsSeen(eq(RESTORE_STARTED_AT.toULong())) verify(settingsStore).update(any()) } assertFalse(settingsData.value.pendingRestoreActivitySeen) @@ -4524,25 +4526,25 @@ class AppViewModelSendFlowTest : BaseUnitTest() { @Test fun `first onchain sync after restore keeps the pending flag when marking activities seen fails`() = test { - settingsData.value = SettingsData(pendingRestoreActivitySeen = true) - whenever { activityRepo.markAllUnseenActivitiesAsSeen() } + settingsData.value = SettingsData(pendingRestoreActivitySeenSince = RESTORE_STARTED_AT) + whenever { activityRepo.markAllUnseenActivitiesAsSeen(eq(RESTORE_STARTED_AT.toULong())) } .thenReturn(Result.failure(AppError("mark seen failed"))) emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) advanceUntilIdle() - verify(activityRepo).markAllUnseenActivitiesAsSeen() + verify(activityRepo).markAllUnseenActivitiesAsSeen(eq(RESTORE_STARTED_AT.toULong())) assertTrue(settingsData.value.pendingRestoreActivitySeen) } @Test fun `lightning sync after restore keeps the pending flag and activities untouched`() = test { - settingsData.value = SettingsData(pendingRestoreActivitySeen = true) + settingsData.value = SettingsData(pendingRestoreActivitySeenSince = RESTORE_STARTED_AT) emitNodeEvent(Event.SyncCompleted(syncType = SyncType.LIGHTNING_WALLET, syncedBlockHeight = 100u)) advanceUntilIdle() - verify(activityRepo, never()).markAllUnseenActivitiesAsSeen() + verify(activityRepo, never()).markAllUnseenActivitiesAsSeen(anyOrNull()) assertTrue(settingsData.value.pendingRestoreActivitySeen) } @@ -4551,7 +4553,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() { emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u)) advanceUntilIdle() - verify(activityRepo, never()).markAllUnseenActivitiesAsSeen() + verify(activityRepo, never()).markAllUnseenActivitiesAsSeen(anyOrNull()) verify(settingsStore, never()).update(any()) } @@ -7801,6 +7803,9 @@ class AppViewModelSendFlowTest : BaseUnitTest() { private const val SAMROCK_SETUP_URL = "https://btcpay.example.com/plugins/store/samrock/protocol?setup=btc-chain&otp=secret" + +/** Stands in for the epoch second a seed restore began. */ +private const val RESTORE_STARTED_AT = 1_700_000_000L private const val HARDWARE_WALLET_ID = "trezor:wallet" private const val REGTEST_ADDRESS = "bcrt1qs04g2ka4pr9s3mv73nu32tvfy7r3cxd27wkyu8" private const val OWN_NODE_ID = "02abababababababababababababababababababababababababababababababab"