Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion app/src/main/java/to/bitkit/data/SettingsStore.kt
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@ class SettingsStore @Inject constructor(
store.updateData { current ->
// The received-sheet hold is armed before the backup is read, so it has to survive the
// settings the backup brings with it - otherwise the replayed history raises sheets.
data.copy(pendingRestoreActivitySeenSince = current.pendingRestoreActivitySeenSince)
data.copy(
pendingRestoreActivitySeenSince = current.pendingRestoreActivitySeenSince,
restoreSyncedBlockHeight = current.restoreSyncedBlockHeight,
)
}

val monitored = data.addressTypesToMonitor
Expand Down Expand Up @@ -212,6 +215,14 @@ data class SettingsData(
* them. Cleared by the first on-chain sync completion whose sweep succeeds.
*/
val pendingRestoreActivitySeenSince: Long = 0,
/**
* Chain tip of the first on-chain sync after the latest seed restore, or 0 when none completed.
*
* Everything confirmed at or below it was already on chain when the restore scanned the wallet, so it outlives
* [pendingRestoreActivitySeenSince]: LDK events are handled concurrently, and a later rescan replays those
* confirmations too, so their received sheets must stay silent however late they are handled.
*/
val restoreSyncedBlockHeight: Long = 0,
) {
val pendingRestoreActivitySeen: Boolean get() = pendingRestoreActivitySeenSince > 0
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ sealed interface NotifyPaymentReceived {

/**
* 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.
* UNIX epoch and [blockHeight] the confirming block, both 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 confirmationTime: ULong? = null,
val blockHeight: UInt? = null,
override val includeNotification: Boolean = false,
) : Command {
val isConfirmedOnly: Boolean get() = confirmationTime != null
Expand All @@ -47,6 +48,7 @@ sealed interface NotifyPaymentReceived {
txid = event.txid,
details = event.details,
confirmationTime = event.confirmationTime,
blockHeight = event.blockHeight,
includeNotification = includeNotification,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.first
import kotlinx.coroutines.withContext
import to.bitkit.data.SettingsData
import to.bitkit.data.SettingsStore
import to.bitkit.di.IoDispatcher
import to.bitkit.ext.nowMillis
Expand Down Expand Up @@ -112,16 +113,19 @@ class NotifyPaymentReceivedHandler @Inject constructor(
}

private suspend fun shouldShowOnchain(command: NotifyPaymentReceived.Command.Onchain): Boolean {
// One snapshot for both restore checks: the sweep lifts the hold and records the restore tip in the same
// update, so a snapshot either still holds the receive or already knows which blocks are history.
val settings = settingsStore.data.first()
if (command.isConfirmedOnly) {
if (command.details.amountSats <= 0) return false
if (!canShowConfirmedOnly(command)) return false
if (!canShowConfirmedOnly(command, settings)) return false
applyConfirmationIfMissing(command)
} else {
activityRepo.handleOnchainTransactionReceived(command.txid, command.details)
if (command.details.amountSats <= 0) return false
}

if (settingsStore.data.first().pendingRestoreActivitySeen) {
if (settings.pendingRestoreActivitySeen) {
Logger.debug("Skipping onchain receive '${command.txid}' until the first sync after restore", context = TAG)
return false
}
Expand All @@ -142,25 +146,31 @@ class NotifyPaymentReceivedHandler @Inject constructor(
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) {
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
}
private suspend fun canShowConfirmedOnly(
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed
command: NotifyPaymentReceived.Command.Onchain,
settings: SettingsData,
): Boolean {
val skipReason = confirmedOnlySkipReason(command, settings) ?: return true
Logger.debug("Skipping confirmed-only receive '${command.txid}' $skipReason", context = TAG)
return false
}

private suspend fun confirmedOnlySkipReason(
command: NotifyPaymentReceived.Command.Onchain,
settings: SettingsData,
): String? {
val confirmationTime = command.confirmationTime ?: return "without a confirmation time"
val blockHeight = command.blockHeight ?: return "without a block height"
val age = nowMillis(clock).milliseconds - confirmationTime.toLong().seconds
if (age.absoluteValue > MAX_CONFIRMED_ONLY_AGE) {
Logger.debug(
"Skipping confirmed-only receive '${command.txid}' confirmed at '$confirmationTime'",
context = TAG,
)
return false
return when {
blockHeight.toLong() <= settings.restoreSyncedBlockHeight ->
"at height '$blockHeight', scanned by the restore"
backupRepo.isRestoring.value -> "during restore"
migrationService.isShowingMigrationLoading.value || migrationService.needsPostMigrationSync() ->
"during migration"
age.absoluteValue > MAX_CONFIRMED_ONLY_AGE -> "confirmed at '$confirmationTime'"
else -> null
}
return true
}

private suspend fun markAsSeen(command: NotifyPaymentReceived.Command) {
Expand Down
31 changes: 23 additions & 8 deletions app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ class AppViewModel @Inject constructor(
registerSheet(highBalanceSheet)
}
private var isCompletingMigration = false
private var isCompletingRestoreHold = false
private var addressValidationJob: Job? = null
private var lastPrivatePaykitContactKeys: Set<String> = emptySet()
private val isPaykitEnabled = settingsStore.isPaykitEnabled
Expand Down Expand Up @@ -1452,7 +1453,7 @@ class AppViewModel @Inject constructor(
}

private suspend fun handleSyncCompleted(event: Event.SyncCompleted) {
if (event.syncType == SyncType.ONCHAIN_WALLET) completePendingRestoreActivitySeen()
if (event.syncType == SyncType.ONCHAIN_WALLET) completePendingRestoreActivitySeen(event.syncedBlockHeight)

val isShowingLoading = migrationService.isShowingMigrationLoading.value
val isRestoringRemote = migrationService.isRestoringFromRNRemoteBackup.value
Expand Down Expand Up @@ -1483,13 +1484,27 @@ class AppViewModel @Inject constructor(
}
}

private suspend fun completePendingRestoreActivitySeen() {
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)
// 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) }
private suspend fun completePendingRestoreActivitySeen(syncedBlockHeight: UInt) {
// Claimed before the first suspension, so a later sync completing while this sweep runs cannot record its
// higher tip and silence a new receive confirmed in between.
if (isCompletingRestoreHold) return
isCompletingRestoreHold = true
try {
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)
// Bounded by the restore start so a payment arriving mid-restore keeps its unseen state.
activityRepo.markAllUnseenActivitiesAsSeen(startedBefore = restoreStartedAt.toULong()).onSuccess {
settingsStore.update { settings ->
if (!settings.pendingRestoreActivitySeen) return@update settings
settings.copy(
pendingRestoreActivitySeenSince = 0,
restoreSyncedBlockHeight = syncedBlockHeight.toLong(),
)
}
}
} finally {
isCompletingRestoreHold = false
}
}

Expand Down
6 changes: 5 additions & 1 deletion app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -568,14 +568,18 @@ class WalletViewModel @Inject constructor(
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) }
settingsStore.update {
it.copy(pendingRestoreActivitySeenSince = nowTimestamp().epochSecond, restoreSyncedBlockHeight = 0)
}

walletRepo.restoreWallet(
mnemonic = mnemonic,
bip39Passphrase = bip39Passphrase,
).onFailure {
// Nothing reaches restoreFromBackup when the wallet was never created, so release here.
backupRepo.setRestorePending(false)
// No node starts, so no sync would ever lift the received-sheet hold.
settingsStore.update { it.copy(pendingRestoreActivitySeenSince = 0) }
ToastEventBus.send(it)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,7 @@ class LightningNodeServiceTest : BaseUnitTest() {
txid = "confirmed_txid",
details = details,
confirmationTime = 0uL,
blockHeight = 100u,
includeNotification = true,
)
verify(notifyPaymentReceivedHandler).invoke(expectedCommand)
Expand Down
10 changes: 10 additions & 0 deletions app/src/test/java/to/bitkit/data/SettingsStoreTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -97,4 +97,14 @@ class SettingsStoreTest : BaseUnitTest() {
assertEquals(restoreStartedAt, sut.data.first().pendingRestoreActivitySeenSince)
assertTrue(sut.data.first().pendingRestoreActivitySeen)
}

@Test
fun `restoring settings keeps the restore tip recorded on this device`() = test {
sut.update { it.copy(restoreSyncedBlockHeight = 900L) }
val backup = SettingsBackupV1(createdAt = 0L, settings = SettingsData(restoreSyncedBlockHeight = 5L))

assertTrue(sut.restoreFromBackup(backup).isSuccess)

assertEquals(900L, sut.data.first().restoreSyncedBlockHeight)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() {

/** Stands in for the epoch second a seed restore began. */
private const val RESTORE_STARTED_AT = 1_700_000_000L
private const val RESTORE_SYNCED_HEIGHT = 900u
}

private val context: Context = mock()
Expand Down Expand Up @@ -583,6 +584,43 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() {
verify(activityRepo, never()).markOnchainActivityAsSeen("txidHistorical", WalletScope.default)
}

@Test
fun `confirmed-only onchain receive scanned by the restore returns Skip after the hold lifts`() = test {
// #1342: LDK events are handled concurrently, so a replayed confirmation can reach the handler after the
// first sync already lifted the hold, and a later rescan replays it again.
settingsData.value = SettingsData(restoreSyncedBlockHeight = RESTORE_SYNCED_HEIGHT.toLong())
val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList())
whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true)
val command = confirmedCommand(
txid = "txidHistorical",
details = details,
age = Duration.ZERO,
blockHeight = RESTORE_SYNCED_HEIGHT,
)

val result = sut(command).getOrThrow()

assertEquals(NotifyPaymentReceived.Result.Skip, result)
verify(activityRepo, never()).shouldShowReceivedSheet(any(), any())
}

@Test
fun `confirmed-only onchain receive above the restore tip shows the sheet`() = test {
settingsData.value = SettingsData(restoreSyncedBlockHeight = RESTORE_SYNCED_HEIGHT.toLong())
val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList())
whenever(activityRepo.shouldShowReceivedSheet(any(), any())).thenReturn(true)
val command = confirmedCommand(
txid = "txidNew",
details = details,
age = Duration.ZERO,
blockHeight = RESTORE_SYNCED_HEIGHT + 1u,
)

val result = sut(command).getOrThrow()

assertTrue(result is NotifyPaymentReceived.Result.ShowSheet)
}

@Test
fun `from maps a confirmed onchain event to a confirmed-only command`() {
val details = TransactionDetails(amountSats = 5000L, inputs = emptyList(), outputs = emptyList())
Expand All @@ -601,6 +639,7 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() {
txid = "txidMapped",
details = details,
confirmationTime = 1_700_000_000uL,
blockHeight = 100u,
includeNotification = true,
),
command,
Expand All @@ -623,10 +662,12 @@ class NotifyPaymentReceivedHandlerTest : BaseUnitTest() {
details: TransactionDetails,
age: Duration,
includeNotification: Boolean = false,
blockHeight: UInt = RESTORE_SYNCED_HEIGHT + 1u,
) = NotifyPaymentReceived.Command.Onchain(
txid = txid,
details = details,
confirmationTime = (NOW - age).epochSeconds.toULong(),
blockHeight = blockHeight,
includeNotification = includeNotification,
)
}
11 changes: 11 additions & 0 deletions app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,17 @@ class WalletViewModelTest : BaseUnitTest() {
assertTrue(settingsData.value.pendingRestoreActivitySeenSince > 0)
}

@Test
fun `restoreWallet should release the received sheet hold when the restore fails`() = test {
whenever(walletRepo.restoreWallet(any(), anyOrNull())).thenReturn(Result.failure(AppError("restore failed")))
val settingsData = stubSettingsUpdate()

sut.restoreWallet("test_mnemonic", null)
advanceUntilIdle()

assertFalse(settingsData.value.pendingRestoreActivitySeen)
}

@Test
fun `addTagToSelected should call walletRepo addTagToSelected`() = test {
sut.addTagToSelected("test_tag")
Expand Down
25 changes: 25 additions & 0 deletions app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4501,6 +4501,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() {
txid = "confirmed-txid",
details = details,
confirmationTime = 0uL,
blockHeight = 100u,
)
inOrder(activityRepo, notifyPaymentReceivedHandler) {
verify(activityRepo).handleOnchainTransactionConfirmed("confirmed-txid", details)
Expand All @@ -4525,6 +4526,7 @@ class AppViewModelSendFlowTest : BaseUnitTest() {
verify(settingsStore).update(any())
}
assertFalse(settingsData.value.pendingRestoreActivitySeen)
assertEquals(100L, settingsData.value.restoreSyncedBlockHeight)
}

@Test
Expand All @@ -4538,6 +4540,29 @@ class AppViewModelSendFlowTest : BaseUnitTest() {

verify(activityRepo).markAllUnseenActivitiesAsSeen(eq(RESTORE_STARTED_AT.toULong()))
assertTrue(settingsData.value.pendingRestoreActivitySeen)
assertEquals(0L, settingsData.value.restoreSyncedBlockHeight)
}

@Test
fun `a later onchain sync during the restore sweep does not overwrite the restore tip`() = test {
settingsData.value = SettingsData(pendingRestoreActivitySeenSince = RESTORE_STARTED_AT)
val sweep = CompletableDeferred<Unit>()
whenever { activityRepo.markAllUnseenActivitiesAsSeen(eq(RESTORE_STARTED_AT.toULong())) }
.doSuspendableAnswer {
sweep.await()
Result.success(Unit)
}

emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 100u))
runCurrent()
emitNodeEvent(Event.SyncCompleted(syncType = SyncType.ONCHAIN_WALLET, syncedBlockHeight = 101u))
runCurrent()
sweep.complete(Unit)
advanceUntilIdle()

verify(activityRepo).markAllUnseenActivitiesAsSeen(eq(RESTORE_STARTED_AT.toULong()))
assertEquals(100L, settingsData.value.restoreSyncedBlockHeight)
assertFalse(settingsData.value.pendingRestoreActivitySeen)
}

@Test
Expand Down
1 change: 1 addition & 0 deletions changelog.d/next/1342.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A historical on-chain payment no longer shows a Received sheet after restoring a wallet from its recovery phrase.
10 changes: 6 additions & 4 deletions journeys/onchain-receive/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ 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. After a seed restore, Get Started sets
`pendingRestoreActivitySeen`, which holds every onchain received sheet and notification until the
clock and no restore or migration is running. A seed restore sets
`pendingRestoreActivitySeen` as it starts, 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`.
confirmations, which the one-hour window keeps silent. Because LDK events are handled concurrently, a
replayed confirmation can reach the handler after the hold is lifted, so that sync also records its
chain tip and confirmed-only receives at or below it stay silent (#1342). The restore journey needs a
throwaway emulator, since it wipes the app and restores a public test seed.

## Preconditions

Expand Down
Loading
Loading