fix: bip21 wallet state management on events

Author: ovitrif

app/src/main/java/to/bitkit/repositories/LightningRepo.kt

@@ -257,7 +257,7 @@ class LightningRepo @Inject constructor(
         _isRecoveryMode.value = enabled
     }
 
-    suspend fun updateGeoBlockState() {
+    suspend fun updateGeoBlockState() = withContext(bgDispatcher) {
         val (isGeoBlocked, shouldBlockLightning) = coreService.checkGeoBlock()
         _lightningState.update {
             it.copy(isGeoBlocked = isGeoBlocked, shouldBlockLightningReceive = shouldBlockLightning)

app/src/main/java/to/bitkit/repositories/WalletRepo.kt

@@ -98,39 +98,15 @@ class WalletRepo @Inject constructor(
         }
     }
 
-    suspend fun refreshBip21(force: Boolean = false): Result<Unit> = withContext(bgDispatcher) {
-        Logger.debug("Refreshing bip21 (force: $force)", context = TAG)
+    suspend fun refreshBip21(): Result<Unit> = withContext(bgDispatcher) {
+        Logger.debug("Refreshing bip21", context = TAG)
 
-        val shouldBlockLightningReceive = coreService.checkGeoBlock().second
+        val (_, shouldBlockLightningReceive) = coreService.checkGeoBlock()
         _walletState.update {
             it.copy(receiveOnSpendingBalance = !shouldBlockLightningReceive)
         }
-
-        // Reset invoice state
-        _walletState.update {
-            it.copy(
-                selectedTags = emptyList(),
-                bip21Description = "",
-                bip21 = "",
-                bip21AmountSats = null,
-            )
-        }
-
-        // Check current address or generate new one
-        val currentAddress = getOnchainAddress()
-        if (force || currentAddress.isEmpty()) {
-            newAddress()
-        } else {
-            // Check if current address has been used
-            checkAddressUsage(currentAddress)
-                .onSuccess { hasTransactions ->
-                    if (hasTransactions) {
-                        // Address has been used, generate a new one
-                        newAddress()
-                    }
-                }
-        }
-
+        clearBip21State()
+        refreshAddressIfNeeded()
         updateBip21Invoice()
         return@withContext Result.success(Unit)
     }
@@ -172,11 +148,66 @@ class WalletRepo @Inject constructor(
 
     suspend fun refreshBip21ForEvent(event: Event) {
         when (event) {
-            is Event.PaymentReceived, is Event.ChannelReady, is Event.ChannelClosed -> refreshBip21()
+            is Event.ChannelReady -> {
+                // Only refresh bolt11 if we can now receive on lightning
+                if (lightningRepo.canReceive()) {
+                    lightningRepo.createInvoice(
+                        amountSats = _walletState.value.bip21AmountSats,
+                        description = _walletState.value.bip21Description,
+                    ).onSuccess { bolt11 ->
+                        setBolt11(bolt11)
+                        updateBip21Url()
+                    }
+                }
+            }
+
+            is Event.ChannelClosed -> {
+                // Clear bolt11 if we can no longer receive on lightning
+                if (!lightningRepo.canReceive()) {
+                    setBolt11("")
+                    updateBip21Url()
+                }
+            }
+
+            is Event.PaymentReceived -> {
+                // Check if onchain address was used, generate new one if needed
+                refreshAddressIfNeeded()
+                updateBip21Url()
+            }
+
             else -> Unit
         }
     }
 
+    private suspend fun refreshAddressIfNeeded() = withContext(bgDispatcher) {
+        val address = getOnchainAddress()
+        if (address.isEmpty()) {
+            newAddress()
+        } else {
+            checkAddressUsage(address).onSuccess { wasUsed ->
+                if (wasUsed) {
+                    newAddress()
+                }
+            }
+        }
+    }
+
+    private suspend fun updateBip21Url(
+        amountSats: ULong? = _walletState.value.bip21AmountSats,
+        message: String = _walletState.value.bip21Description,
+    ): String {
+        val address = getOnchainAddress()
+        val newBip21 = buildBip21Url(
+            bitcoinAddress = address,
+            amountSats = amountSats,
+            message = message.ifBlank { Env.DEFAULT_INVOICE_MESSAGE },
+            lightningInvoice = getBolt11(),
+        )
+        setBip21(newBip21)
+
+        return newBip21
+    }
+
     suspend fun createWallet(bip39Passphrase: String?): Result<Unit> = withContext(bgDispatcher) {
         lightningRepo.setRecoveryMode(enabled = false)
         try {
@@ -310,12 +341,19 @@ class WalletRepo @Inject constructor(
     }
 
     // BIP21 state management
-    fun updateBip21AmountSats(amount: ULong?) {
-        _walletState.update { it.copy(bip21AmountSats = amount) }
-    }
+    fun setBip21AmountSats(amount: ULong?) = _walletState.update { it.copy(bip21AmountSats = amount) }
+
+    fun setBip21Description(description: String) = _walletState.update { it.copy(bip21Description = description) }
 
-    fun updateBip21Description(description: String) {
-        _walletState.update { it.copy(bip21Description = description) }
+    fun clearBip21State() {
+        _walletState.update {
+            it.copy(
+                bip21 = "",
+                selectedTags = emptyList(),
+                bip21AmountSats = null,
+                bip21Description = "",
+            )
+        }
     }
 
     suspend fun toggleReceiveOnSpendingBalance(): Result<Unit> = withContext(bgDispatcher) {
@@ -348,37 +386,23 @@ class WalletRepo @Inject constructor(
 
     // BIP21 invoice creation
     suspend fun updateBip21Invoice(
-        amountSats: ULong? = null,
-        description: String = "",
+        amountSats: ULong? = walletState.value.bip21AmountSats,
+        description: String = walletState.value.bip21Description,
     ): Result<Unit> = withContext(bgDispatcher) {
         try {
-            updateBip21AmountSats(amountSats)
-            updateBip21Description(description)
+            setBip21AmountSats(amountSats)
+            setBip21Description(description)
 
             val canReceive = lightningRepo.canReceive()
             if (canReceive && _walletState.value.receiveOnSpendingBalance) {
-                lightningRepo.createInvoice(
-                    amountSats = _walletState.value.bip21AmountSats,
-                    description = _walletState.value.bip21Description,
-                ).onSuccess { bolt11 ->
-                    setBolt11(bolt11)
+                lightningRepo.createInvoice(amountSats, description).onSuccess {
+                    setBolt11(it)
                 }
             } else {
                 setBolt11("")
             }
-            val address = getOnchainAddress()
-            val newBip21 = buildBip21Url(
-                bitcoinAddress = address,
-                amountSats = _walletState.value.bip21AmountSats,
-                message = description.ifBlank { Env.DEFAULT_INVOICE_MESSAGE },
-                lightningInvoice = getBolt11()
-            )
-            setBip21(newBip21)
-            saveInvoiceWithTags(
-                bip21Invoice = newBip21,
-                onChainAddress = address,
-                tags = _walletState.value.selectedTags
-            )
+            val newBip21Url = updateBip21Url(amountSats, description)
+            persistTagsMetadata(newBip21Url)
             Result.success(Unit)
         } catch (e: Throwable) {
             Logger.error("Update BIP21 invoice error", e, context = TAG)
@@ -404,14 +428,16 @@ class WalletRepo @Inject constructor(
         }
     }
 
-    suspend fun saveInvoiceWithTags(bip21Invoice: String, onChainAddress: String, tags: List<String>) =
+    private suspend fun persistTagsMetadata(bip21Url: String) =
         withContext(bgDispatcher) {
+            val tags = _walletState.value.selectedTags
             if (tags.isEmpty()) return@withContext
 
+            val onChainAddress = getOnchainAddress()
+
             try {
-                deleteExpiredInvoices()
-                val decoded = decode(bip21Invoice)
-                val paymentHash = when (decoded) {
+                deleteExpiredTagMetadata()
+                val paymentHash = when (val decoded = decode(bip21Url)) {
                     is Scanner.Lightning -> decoded.invoice.paymentHash.toHex()
                     is Scanner.OnChain -> decoded.extractLightningHash()
                     else -> null
@@ -432,15 +458,7 @@ class WalletRepo @Inject constructor(
             }
         }
 
-    suspend fun deleteAllInvoices() = withContext(bgDispatcher) {
-        try {
-            db.tagMetadataDao().deleteAll()
-        } catch (e: Throwable) {
-            Logger.error("deleteAllInvoices error", e, context = TAG)
-        }
-    }
-
-    suspend fun deleteExpiredInvoices() = withContext(bgDispatcher) {
+    private suspend fun deleteExpiredTagMetadata() = withContext(bgDispatcher) {
         try {
             val twoDaysAgoMillis = Clock.System.now().minus(2.days).toEpochMilliseconds()
             db.tagMetadataDao().deleteExpired(expirationTimeStamp = twoDaysAgoMillis)
@@ -451,9 +469,8 @@ class WalletRepo @Inject constructor(
 
     private suspend fun Scanner.OnChain.extractLightningHash(): String? {
         val lightningInvoice: String = this.invoice.params?.get("lightning") ?: return null
-        val decoded = decode(lightningInvoice)
 
-        return when (decoded) {
+        return when (val decoded = decode(lightningInvoice)) {
             is Scanner.Lightning -> decoded.invoice.paymentHash.toHex()
             else -> null
         }

app/src/main/java/to/bitkit/viewmodels/WalletViewModel.kt

@@ -202,13 +202,10 @@ class WalletViewModel @Inject constructor(
     }
 
     fun updateBip21Invoice(
-        amountSats: ULong? = null,
+        amountSats: ULong? = walletState.value.bip21AmountSats,
     ) {
         viewModelScope.launch {
-            walletRepo.updateBip21Invoice(
-                amountSats = amountSats,
-                description = walletState.value.bip21Description,
-            ).onFailure { error ->
+            walletRepo.updateBip21Invoice(amountSats).onFailure { error ->
                 ToastEventBus.send(
                     type = Toast.ToastType.ERROR,
                     title = "Error updating invoice",
@@ -220,33 +217,23 @@ class WalletViewModel @Inject constructor(
 
     fun toggleReceiveOnSpending() {
         viewModelScope.launch {
-            walletRepo.toggleReceiveOnSpendingBalance().onSuccess {
-                updateBip21Invoice(
-                    amountSats = walletState.value.bip21AmountSats,
-                )
-            }.onFailure { e ->
-                if (e is ServiceError.GeoBlocked) {
-                    walletEffect(WalletViewModelEffects.NavigateGeoBlockScreen)
-                    return@launch
+            walletRepo.toggleReceiveOnSpendingBalance()
+                .onSuccess {
+                    updateBip21Invoice()
+                }.onFailure { e ->
+                    if (e is ServiceError.GeoBlocked) {
+                        walletEffect(WalletViewModelEffects.NavigateGeoBlockScreen)
+                        return@launch
+                    }
+                    updateBip21Invoice()
                 }
-
-                updateBip21Invoice(
-                    amountSats = walletState.value.bip21AmountSats,
-                )
-            }
         }
     }
 
-    fun refreshReceiveState() = viewModelScope.launch(bgDispatcher) {
-        launch { lightningRepo.updateGeoBlockState() }
-        launch { walletRepo.refreshBip21() }
+    fun refreshReceiveState() = viewModelScope.launch {
         launch { blocktankRepo.refreshInfo() }
-    }
-
-    fun refreshBip21() {
-        viewModelScope.launch {
-            walletRepo.refreshBip21()
-        }
+        lightningRepo.updateGeoBlockState()
+        walletRepo.refreshBip21()
     }
 
     fun wipeWallet() {
@@ -290,7 +277,7 @@ class WalletViewModel @Inject constructor(
         if (newText.isEmpty()) {
             Logger.warn("Empty")
         }
-        walletRepo.updateBip21Description(newText)
+        walletRepo.setBip21Description(newText)
     }
 
     suspend fun handleHideBalanceOnOpen() {

app/src/test/java/to/bitkit/repositories/WalletRepoTest.kt

@@ -200,19 +200,6 @@ class WalletRepoTest : BaseUnitTest() {
         verify(lightningRepo, never()).newAddress()
     }
 
-    @Test
-    fun `refreshBip21 forced should always generate new address`() = test {
-        val existingAddress = "existingAddress"
-        whenever(cacheStore.data).thenReturn(flowOf(AppCacheData(onchainAddress = existingAddress)))
-        whenever(lightningRepo.newAddress()).thenReturn(Result.success("newAddress"))
-        whenever(addressChecker.getAddressInfo(any())).thenReturn(mockAddressInfo())
-
-        val result = sut.refreshBip21(force = true)
-
-        assertTrue(result.isSuccess)
-        verify(lightningRepo).newAddress()
-    }
-
     @Test
     fun `syncBalances should update balance cache and state`() = test {
         val expectedState = BalanceState(

app/src/test/java/to/bitkit/ui/WalletViewModelTest.kt

@@ -69,9 +69,9 @@ class WalletViewModelTest : BaseUnitTest() {
     fun `refreshReceiveState should refresh receive state`() = test {
         sut.refreshReceiveState()
 
+        verify(blocktankRepo).refreshInfo()
         verify(lightningRepo).updateGeoBlockState()
         verify(walletRepo).refreshBip21()
-        verify(blocktankRepo).refreshInfo()
     }
 
     @Test
@@ -104,23 +104,6 @@ class WalletViewModelTest : BaseUnitTest() {
         // Add verification for ToastEventBus.send if you have a way to capture those events
     }
 
-    @Test
-    fun `updateBip21Invoice should call walletRepo updateBip21Invoice and send failure toast`() = test {
-        val testError = Exception("Test error")
-        whenever(walletRepo.updateBip21Invoice(anyOrNull(), any())).thenReturn(Result.failure(testError))
-
-        sut.updateBip21Invoice()
-
-        verify(walletRepo).updateBip21Invoice(anyOrNull(), any())
-        // Add verification for ToastEventBus.send
-    }
-
-    @Test
-    fun `refreshBip21 should call walletRepo refreshBip21`() = test {
-        sut.refreshBip21()
-        verify(walletRepo).refreshBip21()
-    }
-
     @Test
     fun `wipeWallet should call walletRepo wipeWallet`() =
         test {
@@ -170,7 +153,7 @@ class WalletViewModelTest : BaseUnitTest() {
     fun `updateBip21Description should call walletRepo updateBip21Description`() = test {
         sut.updateBip21Description("test_description")
 
-        verify(walletRepo).updateBip21Description("test_description")
+        verify(walletRepo).setBip21Description("test_description")
     }
 
     @Test