diff --git a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt index 406e16127..b8c03ba5b 100644 --- a/app/src/main/java/to/bitkit/repositories/LightningRepo.kt +++ b/app/src/main/java/to/bitkit/repositories/LightningRepo.kt @@ -1514,6 +1514,21 @@ class LightningRepo @Inject constructor( } } + /** Max onchain amount sendable at [speed], i.e. the spendable balance minus the send-all mining fee */ + suspend fun estimateMaxSendOnchain( + address: Address? = null, + speed: TransactionSpeed? = null, + feeRates: FeeRates? = null, + ): Result = withContext(bgDispatcher) { + runSuspendCatching { + val spendableSats = getBalancesAsync().getOrThrow().spendableOnchainBalanceSats + if (spendableSats == 0uL) return@runSuspendCatching 0uL + + val fee = estimateSendAllFee(address = address, speed = speed, feeRates = feeRates).getOrThrow() + spendableSats.safe() - fee.safe() + } + } + suspend fun getFeeRateForSpeed( speed: TransactionSpeed, feeRates: FeeRates? = null, diff --git a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt index dae74658d..8184b984e 100644 --- a/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt +++ b/app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt @@ -2082,6 +2082,7 @@ class AppViewModel @Inject constructor( it.copy( amount = amount, isAmountInputValid = validateAmount(amount), + isMaxAmount = false, confirmedWarnings = persistentListOf(), ) } @@ -2358,6 +2359,7 @@ class AppViewModel @Inject constructor( _sendUiState.update { it.copy(selectedUtxos = utxos.toImmutableList()) } + refreshMaxSendOnchain() refreshOnchainFeeUi() setSendEffect(SendEffect.NavigateToConfirm) } @@ -3240,6 +3242,7 @@ class AppViewModel @Inject constructor( state.copy( amount = 0u, isAmountInputValid = false, + isMaxAmount = false, ) } } @@ -3355,6 +3358,9 @@ class AppViewModel @Inject constructor( return } + // pay the amount & drain flag the refresh settled on, not the ones it is about to replace + onchainSendRefreshJob?.join() + val amount = _sendUiState.value.amount val lnurl = _sendUiState.value.lnurl @@ -3729,13 +3735,14 @@ class AppViewModel @Inject constructor( amount: ULong, tags: List = emptyList(), ): Result { + val state = _sendUiState.value return lightningRepo.sendOnChain( address = address, sats = amount, - speed = _sendUiState.value.speed, - utxosToSpend = _sendUiState.value.selectedUtxos, - isMaxAmount = _sendUiState.value.payMethod == SendMethod.ONCHAIN && - amount == walletRepo.balanceState.value.maxSendOnchainSats, + speed = state.speed, + utxosToSpend = state.selectedUtxos, + feeRates = state.feeRates, + isMaxAmount = state.payMethod == SendMethod.ONCHAIN && state.isMaxAmount, tags = tags, ) } @@ -3787,12 +3794,12 @@ class AppViewModel @Inject constructor( } } - /** Reselect utxos for current amount & speed then refresh fees using updated utxos */ + /** Recheck the max sendable, reselect utxos for current amount & speed, then refresh fees using updated utxos */ private fun refreshOnchainSendIfNeeded(): Job? { - val currentState = _sendUiState.value - if (currentState.payMethod != SendMethod.ONCHAIN || - currentState.amount == 0uL || - currentState.address.isEmpty() + val state = _sendUiState.value + if (state.payMethod != SendMethod.ONCHAIN || + state.amount == 0uL || + state.address.isEmpty() ) { return null } @@ -3800,6 +3807,8 @@ class AppViewModel @Inject constructor( updateOnchainFeeUi { it.copy(isLoading = true) } onchainSendRefreshJob?.cancel() val job = viewModelScope.launch(bgDispatcher, start = CoroutineStart.LAZY) { + refreshMaxSendOnchain() + val currentState = _sendUiState.value // preselect utxos for deterministic fee estimation if ( currentState.hardwareWalletId == null && @@ -3827,6 +3836,48 @@ class AppViewModel @Inject constructor( return job } + /** + * Flags the send as a drain when the amount reaches the max sendable to this recipient at the selected speed, + * lowering the amount to that max so the confirmed figure matches what the drain delivers. + */ + private suspend fun refreshMaxSendOnchain() { + val state = _sendUiState.value + if (state.payMethod != SendMethod.ONCHAIN || state.hardwareWalletId != null) return + if (state.amount == 0uL || state.address.isEmpty()) return + + val max = lightningRepo.estimateMaxSendOnchain( + address = state.address, + speed = state.speed, + feeRates = state.feeRates, + ).getOrNull()?.takeIf { it > 0uL } + + if (max == null) { + // without an estimate the cached max is the only max-send signal left + _sendUiState.update { + if (it.divergedFrom(state)) return@update it + it.copy(isMaxAmount = it.amount == walletRepo.balanceState.value.maxSendOnchainSats) + } + return + } + + val isMaxAmount = state.amount >= max + if (isMaxAmount && state.amount != max) { + Logger.info( + "Lowering amount '${state.amount}' to max '$max' at speed '${state.speed.serialized()}'", + context = TAG, + ) + } + _sendUiState.update { + if (it.divergedFrom(state)) return@update it + it.copy(amount = if (isMaxAmount) max else it.amount, isMaxAmount = isMaxAmount) + } + } + + private fun SendUiState.divergedFrom(snapshot: SendUiState) = amount != snapshot.amount || + address != snapshot.address || + speed != snapshot.speed || + hardwareWalletId != snapshot.hardwareWalletId + private suspend fun refreshOnchainFeeUi() = withContext(bgDispatcher) { val currentState = _sendUiState.value updateOnchainFeeUi { it.copy(isLoading = true) } @@ -4809,6 +4860,7 @@ data class SendUiState( val isAddressInputValid: Boolean = false, val amount: ULong = 0u, val isAmountInputValid: Boolean = false, + val isMaxAmount: Boolean = false, val isUnified: Boolean = false, val canSwitchWallet: Boolean = false, val canSwitchFundingSource: Boolean = false, diff --git a/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt b/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt index 76c9c99ad..a1d8015ab 100644 --- a/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt +++ b/app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt @@ -1386,6 +1386,55 @@ class LightningRepoTest : BaseUnitTest() { assertEquals(80_000uL, result) } + @Test + fun `estimateMaxSendOnchain should subtract the send-all fee for the given speed`() = test { + startNodeForTesting() + whenever(lightningService.balances).thenReturn( + BalanceDetails( + totalOnchainBalanceSats = 100_000uL, + spendableOnchainBalanceSats = 80_000uL, + totalAnchorChannelsReserveSats = 0uL, + totalLightningBalanceSats = 0uL, + lightningBalances = emptyList(), + pendingBalancesFromChannelClosures = emptyList(), + ), + ) + whenever { lightningService.estimateSendAllFee(any(), any()) }.thenReturn(2_000uL) + + val result = sut.estimateMaxSendOnchain( + address = "bcrt1qtest", + speed = TransactionSpeed.Fast, + feeRates = FeeRates(fast = 20u, mid = 10u, slow = 5u), + ) + + assertEquals(78_000uL, result.getOrNull()) + verify(lightningService).estimateSendAllFee(address = "bcrt1qtest", satsPerVByte = 20uL) + } + + @Test + fun `estimateMaxSendOnchain should return zero when nothing is spendable`() = test { + startNodeForTesting() + whenever(lightningService.balances).thenReturn( + BalanceDetails( + totalOnchainBalanceSats = 100_000uL, + spendableOnchainBalanceSats = 0uL, + totalAnchorChannelsReserveSats = 0uL, + totalLightningBalanceSats = 0uL, + lightningBalances = emptyList(), + pendingBalancesFromChannelClosures = emptyList(), + ), + ) + + val result = sut.estimateMaxSendOnchain( + address = "bcrt1qtest", + speed = TransactionSpeed.Fast, + feeRates = FeeRates(fast = 20u, mid = 10u, slow = 5u), + ) + + assertEquals(0uL, result.getOrNull()) + verify(lightningService, never()).estimateSendAllFee(any(), any()) + } + @Test fun `updateAddressType should fail when already in progress`() = test { startNodeForTesting() diff --git a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt index 76c59f2e2..9dfece134 100644 --- a/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt +++ b/app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt @@ -4516,6 +4516,309 @@ class AppViewModelSendFlowTest : BaseUnitTest() { confirmCurrentPayment() } + @Test + fun `max onchain send drains at the max for the newly selected speed`() = test { + val address = "bcrt1qmaxsend" + val cachedMax = 100_000uL + val maxAtFast = 99_500uL + val feeRates = FeeRates(fast = 20u, mid = 10u, slow = 5u) + balanceState.value = BalanceState(maxSendOnchainSats = cachedMax) + whenever { + lightningRepo.estimateMaxSendOnchain( + address = address, + speed = TransactionSpeed.Fast, + feeRates = feeRates, + ) + }.thenReturn(Result.success(maxAtFast)) + whenever { lightningRepo.getFeeRateForSpeed(any(), anyOrNull()) }.thenReturn(Result.success(20uL)) + whenever { + lightningRepo.sendOnChain( + address = address, + sats = maxAtFast, + speed = TransactionSpeed.Fast, + utxosToSpend = null, + feeRates = feeRates, + isMaxAmount = true, + tags = emptyList(), + ) + }.thenReturn(Result.success("txid")) + setSendState( + SendUiState( + address = address, + amount = cachedMax, + payMethod = SendMethod.ONCHAIN, + speed = TransactionSpeed.Medium, + feeRates = feeRates, + ), + ) + + sut.setTransactionSpeed(TransactionSpeed.Fast) + advanceUntilIdle() + + assertEquals(maxAtFast, sut.sendUiState.value.amount) + assertTrue(sut.sendUiState.value.isMaxAmount) + + sut.setSendEvent(SendEvent.PayConfirmed) + advanceUntilIdle() + + // same rates must back both the max estimate and the send + verify(lightningRepo).estimateMaxSendOnchain( + address = address, + speed = TransactionSpeed.Fast, + feeRates = feeRates, + ) + verify(lightningRepo).sendOnChain( + address = address, + sats = maxAtFast, + speed = TransactionSpeed.Fast, + utxosToSpend = null, + feeRates = feeRates, + isMaxAmount = true, + tags = emptyList(), + ) + } + + @Test + fun `onchain send below the max is not a drain`() = test { + val address = "bcrt1qbelowmax" + val amount = 50_000uL + val feeRates = FeeRates(fast = 20u, mid = 10u, slow = 5u) + balanceState.value = BalanceState(maxSendOnchainSats = 100_000uL) + whenever { + lightningRepo.estimateMaxSendOnchain( + address = address, + speed = TransactionSpeed.Fast, + feeRates = feeRates, + ) + }.thenReturn(Result.success(99_500uL)) + whenever { lightningRepo.getFeeRateForSpeed(any(), anyOrNull()) }.thenReturn(Result.success(20uL)) + whenever { + lightningRepo.sendOnChain( + address = address, + sats = amount, + speed = TransactionSpeed.Fast, + utxosToSpend = null, + feeRates = feeRates, + isMaxAmount = false, + tags = emptyList(), + ) + }.thenReturn(Result.success("txid")) + setSendState( + SendUiState( + address = address, + amount = amount, + payMethod = SendMethod.ONCHAIN, + speed = TransactionSpeed.Medium, + feeRates = feeRates, + ), + ) + + sut.setTransactionSpeed(TransactionSpeed.Fast) + advanceUntilIdle() + + assertEquals(amount, sut.sendUiState.value.amount) + assertFalse(sut.sendUiState.value.isMaxAmount) + + sut.setSendEvent(SendEvent.PayConfirmed) + advanceUntilIdle() + + verify(lightningRepo).sendOnChain( + address = address, + sats = amount, + speed = TransactionSpeed.Fast, + utxosToSpend = null, + feeRates = feeRates, + isMaxAmount = false, + tags = emptyList(), + ) + } + + @Test + fun `max onchain send still drains when the max cannot be estimated`() = test { + val address = "bcrt1qmaxsendfailure" + val cachedMax = 100_000uL + val feeRates = FeeRates(fast = 20u, mid = 10u, slow = 5u) + balanceState.value = BalanceState(maxSendOnchainSats = cachedMax) + whenever { + lightningRepo.estimateMaxSendOnchain( + address = address, + speed = TransactionSpeed.Fast, + feeRates = feeRates, + ) + }.thenReturn(Result.failure(AppError("no estimate"))) + whenever { lightningRepo.getFeeRateForSpeed(any(), anyOrNull()) }.thenReturn(Result.success(20uL)) + whenever { + lightningRepo.sendOnChain( + address = address, + sats = cachedMax, + speed = TransactionSpeed.Fast, + utxosToSpend = null, + feeRates = feeRates, + isMaxAmount = true, + tags = emptyList(), + ) + }.thenReturn(Result.success("txid")) + setSendState( + SendUiState( + address = address, + amount = cachedMax, + payMethod = SendMethod.ONCHAIN, + speed = TransactionSpeed.Medium, + feeRates = feeRates, + ), + ) + + sut.setTransactionSpeed(TransactionSpeed.Fast) + advanceUntilIdle() + + assertEquals(cachedMax, sut.sendUiState.value.amount) + assertTrue(sut.sendUiState.value.isMaxAmount) + + sut.setSendEvent(SendEvent.PayConfirmed) + advanceUntilIdle() + + verify(lightningRepo).sendOnChain( + address = address, + sats = cachedMax, + speed = TransactionSpeed.Fast, + utxosToSpend = null, + feeRates = feeRates, + isMaxAmount = true, + tags = emptyList(), + ) + } + + @Test + fun `max onchain send waits for the in-flight max refresh before paying`() = test { + val address = "bcrt1qmaxsendrace" + val cachedMax = 100_000uL + val maxAtFast = 99_500uL + val feeRates = FeeRates(fast = 20u, mid = 10u, slow = 5u) + val finishEstimate = CompletableDeferred() + balanceState.value = BalanceState(maxSendOnchainSats = cachedMax) + whenever { + lightningRepo.estimateMaxSendOnchain( + address = address, + speed = TransactionSpeed.Fast, + feeRates = feeRates, + ) + }.doSuspendableAnswer { + finishEstimate.await() + Result.success(maxAtFast) + } + whenever { lightningRepo.getFeeRateForSpeed(any(), anyOrNull()) }.thenReturn(Result.success(20uL)) + whenever { + lightningRepo.sendOnChain( + address = address, + sats = maxAtFast, + speed = TransactionSpeed.Fast, + utxosToSpend = null, + feeRates = feeRates, + isMaxAmount = true, + tags = emptyList(), + ) + }.thenReturn(Result.success("txid")) + setSendState( + SendUiState( + address = address, + amount = cachedMax, + isMaxAmount = true, + payMethod = SendMethod.ONCHAIN, + speed = TransactionSpeed.Medium, + feeRates = feeRates, + ), + ) + + sut.setTransactionSpeed(TransactionSpeed.Fast) + sut.setSendEvent(SendEvent.PayConfirmed) + advanceUntilIdle() + + // the send must not go out while the max for the new speed is still being estimated + verifyNoOnchainSend() + + finishEstimate.complete(Unit) + advanceUntilIdle() + + verify(lightningRepo).sendOnChain( + address = address, + sats = maxAtFast, + speed = TransactionSpeed.Fast, + utxosToSpend = null, + feeRates = feeRates, + isMaxAmount = true, + tags = emptyList(), + ) + } + + @Test + fun `max refresh does not restore the amount after it was edited`() = test { + val address = "bcrt1qmaxsendedited" + val cachedMax = 100_000uL + val feeRates = FeeRates(fast = 20u, mid = 10u, slow = 5u) + val finishEstimate = CompletableDeferred() + balanceState.value = BalanceState(maxSendOnchainSats = cachedMax) + whenever { + lightningRepo.estimateMaxSendOnchain( + address = address, + speed = TransactionSpeed.Fast, + feeRates = feeRates, + ) + }.doSuspendableAnswer { + finishEstimate.await() + Result.success(99_500uL) + } + whenever { lightningRepo.getFeeRateForSpeed(any(), anyOrNull()) }.thenReturn(Result.success(20uL)) + setSendState( + SendUiState( + address = address, + amount = cachedMax, + isMaxAmount = true, + payMethod = SendMethod.ONCHAIN, + speed = TransactionSpeed.Medium, + feeRates = feeRates, + ), + ) + + sut.setTransactionSpeed(TransactionSpeed.Fast) + sut.setSendEvent(SendEvent.AmountChange(1_000uL)) + advanceUntilIdle() + finishEstimate.complete(Unit) + advanceUntilIdle() + + assertEquals(1_000uL, sut.sendUiState.value.amount) + assertFalse(sut.sendUiState.value.isMaxAmount) + } + + @Test + fun `hardware max send keeps its available amount and skips the onchain max estimate`() = test { + val available = 50_000uL + hwWallets.value = persistentListOf(hardwareWallet(fundingBalanceSats = available)) + whenever { hwWalletRepo.maxSpendableFunding(any(), any(), any()) }.thenReturn(Result.success(available)) + balanceState.value = BalanceState(maxSendOnchainSats = 100_000uL) + setSendState( + SendUiState( + address = REGTEST_ADDRESS, + amount = available, + isAmountInputValid = true, + isMaxAmount = true, + hardwareWalletId = HARDWARE_WALLET_ID, + hardwareWalletName = "Trezor", + hardwareAvailableSats = available, + payMethod = SendMethod.ONCHAIN, + speed = TransactionSpeed.Medium, + feeRates = FeeRates(fast = 20u, mid = 10u, slow = 5u), + ), + ) + + sut.setTransactionSpeed(TransactionSpeed.Fast) + advanceUntilIdle() + + assertEquals(available, sut.sendUiState.value.amount) + assertEquals(available, sut.sendUiState.value.hardwareAvailableSats) + assertTrue(sut.sendUiState.value.isMaxAmount) + verify(lightningRepo, never()).estimateMaxSendOnchain(anyOrNull(), anyOrNull(), anyOrNull()) + } + @Test fun `private lightning contact payment consumes private list before send`() = test { val bolt11 = "lnbcrt1privatecontact" @@ -5144,6 +5447,18 @@ class AppViewModelSendFlowTest : BaseUnitTest() { method.invoke(sut) } + private suspend fun verifyNoOnchainSend() = verify(lightningRepo, never()).sendOnChain( + address = any(), + sats = any(), + speed = anyOrNull(), + utxosToSpend = anyOrNull(), + feeRates = anyOrNull(), + isTransfer = any(), + channelId = anyOrNull(), + isMaxAmount = any(), + tags = any(), + ) + private fun hardwareWallet(fundingBalanceSats: ULong) = HwWallet( id = HARDWARE_WALLET_ID, name = "Trezor", diff --git a/changelog.d/next/1144.fixed.md b/changelog.d/next/1144.fixed.md new file mode 100644 index 000000000..9fc4447d2 --- /dev/null +++ b/changelog.d/next/1144.fixed.md @@ -0,0 +1 @@ +Fixed max on-chain sends so the amount always matches the fee speed and recipient it is sent at.