Skip to content

Send-max computed at the default fee speed, but the drain happens at the selected speed (#1144) #1147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
coreyphillips wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Send-max computed at the default fee speed, but the drain happens at the selected speed (#1144) #1147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6780822
fix: verify max onchain send at selected speed
coreyphillips Aug 11, 2026
e00b553
fix: reuse send fee rates for drain check
coreyphillips Aug 11, 2026
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
15 changes: 15 additions & 0 deletions app/src/main/java/to/bitkit/repositories/LightningRepo.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1481,6 +1481,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<ULong> = 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()
}
}
Comment on lines +1484 to +1497

@jvsena42 jvsena42 Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This duplicates DeriveBalanceStateUseCase.getMaxSendAmount with different inputs, and the drain decision is exact equality between the two.

getMaxSendAmount (~L214) computes the same quantity but fetches fee rates fresh via blocktank.getFees() and applies the 1%-of-balance fallback; this one takes the send-sheet snapshot state.feeRates (captured in resetSendState) and has no fallback. Since shouldDrainOnchain requires amount == maxAtSelectedSpeed exactly, any blocktank rate refresh between the last balance derivation and confirm silently disables drain — same failure mode as the AppViewModel comments.

Having the use case delegate to this new repo method (same address, same rates) would make the two agree by construction rather than by coincidence.


suspend fun getFeeRateForSpeed(
speed: TransactionSpeed,
feeRates: FeeRates? = null,
Expand Down
34 changes: 30 additions & 4 deletions app/src/main/java/to/bitkit/viewmodels/AppViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2879,17 +2879,43 @@ class AppViewModel @Inject constructor(
amount: ULong,
tags: List<String> = emptyList(),
): Result<Txid> {
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 &&
shouldDrainOnchain(address, amount, state),
tags = tags,
)
}

private suspend fun shouldDrainOnchain(address: String, amount: ULong, state: SendUiState): Boolean {
// cached max is computed at the default speed, so drain only if it still holds for the selected one
if (amount != walletRepo.balanceState.value.maxSendOnchainSats) return false

val maxAtSelectedSpeed = lightningRepo.estimateMaxSendOnchain(
address = address,
speed = state.speed,
feeRates = state.feeRates,
Comment on lines +2899 to +2902

@jvsena42 jvsena42 Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address-type mismatch makes this comparison apples-to-oranges — max-send is broken for P2TR/P2SH/P2PKH recipients.

This recomputes the max using the recipient address, but the cached value it gets compared against on L2897 (walletRepo.balanceState.value.maxSendOnchainSats) comes from DeriveBalanceStateUseCase.getMaxSendAmount, which passes address = null and therefore falls back to cacheStore.onchainAddress — our own receive address. calculateSendAllFee depends on the output script size, so the two values differ for any recipient whose script type differs from selectedAddressType, with no speed change at all.

Reproduced on regtest (balance 1,000,000 sats, default speed):

  • P2TR recipient → Sending exact amount '999890' instead of draining, max at speed 'Medium' is '999878' → isMaxAmount = false → AppError='The available funds are insufficient to cover the transaction' and an "Error Sending" toast.
  • P2WPKH recipient, same wallet and speed → isMaxAmount = true, drain succeeds (txid 498ced33…).

So MAX now fails for P2TR/P2SH/P2PKH recipients, and symmetrically for P2WPKH recipients once the user switches selectedAddressType to Taproot. iOS avoids this because its MAX amount is itself derived from calculateMaxSendableAmount(address: recipient, rate: selected).

Suggest comparing like with like: either recompute with the same address the cached max used, or derive the MAX button amount from the recipient + selected rate.

).onFailure {
Logger.warn("Failed to recompute max send amount for speed '${state.speed}'", it, context = TAG)

@jvsena42 jvsena42 Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TransactionSpeed has no toString, so this logs Failed to recompute max send amount for speed 'to.bitkit.models.TransactionSpeed$Medium@987bb0b' on device. Use the existing state.speed.serialized() to keep the reference traceable.

}.getOrNull() ?: return false

if (amount != maxAtSelectedSpeed) {
Logger.info(
"Sending exact amount '$amount' instead of draining, " +
"max at speed '${state.speed}' is '$maxAtSelectedSpeed'",

@jvsena42 jvsena42 Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here — '${state.speed}' renders as to.bitkit.models.TransactionSpeed$Medium@987bb0b. Use state.speed.serialized().

context = TAG,
)
return false
}
Comment on lines +2907 to +2914

@jvsena42 jvsena42 Aug 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The intended "fall back to an exact-amount send" path can never succeed.

Whenever maxAtSelectedSpeed < amount, the headroom left over is exactly the 1-output send-all fee at the default rate, while an exact-amount send needs a 2-output tx (recipient + change) at the selected — higher — rate. That is always more, so returning false here doesn't degrade to an exact-amount send, it degrades to an insufficient-funds error. This is what the P2TR repro above ends in.

Consider clamping amount to maxAtSelectedSpeed (and updating the displayed amount/fee when the speed changes, as iOS does), or surfacing an actionable "reduce amount" error instead of the generic LDK failure.


return true
}

private suspend fun sendLightning(
bolt11: String,
amount: ULong? = null,
Expand Down
49 changes: 49 additions & 0 deletions app/src/test/java/to/bitkit/repositories/LightningRepoTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
143 changes: 143 additions & 0 deletions app/src/test/java/to/bitkit/viewmodels/AppViewModelSendFlowTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import android.net.Uri
import android.nfc.NfcAdapter
import androidx.core.net.toUri
import app.cash.turbine.test
import com.synonym.bitkitcore.FeeRates
import com.synonym.bitkitcore.LightningInvoice
import com.synonym.bitkitcore.NetworkType
import com.synonym.bitkitcore.Scanner
Expand Down Expand Up @@ -2148,6 +2149,148 @@ class AppViewModelSendFlowTest : BaseUnitTest() {
confirmCurrentPayment()
}

@Test
fun `max onchain send drains when max still matches the selected speed`() = test {
val address = "bcrt1qmaxsend"
val maxAmount = 100_000uL
val feeRates = FeeRates(fast = 20u, mid = 10u, slow = 5u)
balanceState.value = BalanceState(maxSendOnchainSats = maxAmount)
whenever {
lightningRepo.estimateMaxSendOnchain(
address = address,
speed = TransactionSpeed.Fast,
feeRates = feeRates,
)
}.thenReturn(Result.success(maxAmount))
whenever {
lightningRepo.sendOnChain(
address = address,
sats = maxAmount,
speed = TransactionSpeed.Fast,
utxosToSpend = null,
feeRates = feeRates,
isMaxAmount = true,
tags = emptyList(),
)
}.thenReturn(Result.success("txid"))
setSendState(
SendUiState(
address = address,
amount = maxAmount,
payMethod = SendMethod.ONCHAIN,
speed = TransactionSpeed.Fast,
feeRates = feeRates,
),
)

sut.setSendEvent(SendEvent.PayConfirmed)
advanceUntilIdle()

// same rates must back both the drain check and the send
verify(lightningRepo).estimateMaxSendOnchain(
address = address,
speed = TransactionSpeed.Fast,
feeRates = feeRates,
)
verify(lightningRepo).sendOnChain(
address = address,
sats = maxAmount,
speed = TransactionSpeed.Fast,
utxosToSpend = null,
feeRates = feeRates,
isMaxAmount = true,
tags = emptyList(),
)
}

@Test
fun `max onchain send falls back to exact amount when selected speed changes the max`() = test {
val address = "bcrt1qmaxsendstale"
val maxAmount = 100_000uL
balanceState.value = BalanceState(maxSendOnchainSats = maxAmount)
whenever {
lightningRepo.estimateMaxSendOnchain(
address = address,
speed = TransactionSpeed.Fast,
feeRates = null,
)
}.thenReturn(Result.success(maxAmount - 500uL))
whenever {
lightningRepo.sendOnChain(
address = address,
sats = maxAmount,
speed = TransactionSpeed.Fast,
utxosToSpend = null,
isMaxAmount = false,
tags = emptyList(),
)
}.thenReturn(Result.success("txid"))
setSendState(
SendUiState(
address = address,
amount = maxAmount,
payMethod = SendMethod.ONCHAIN,
speed = TransactionSpeed.Fast,
),
)

sut.setSendEvent(SendEvent.PayConfirmed)
advanceUntilIdle()

verify(lightningRepo).sendOnChain(
address = address,
sats = maxAmount,
speed = TransactionSpeed.Fast,
utxosToSpend = null,
isMaxAmount = false,
tags = emptyList(),
)
}

@Test
fun `max onchain send falls back to exact amount when max cannot be recomputed`() = test {
val address = "bcrt1qmaxsendfailure"
val maxAmount = 100_000uL
balanceState.value = BalanceState(maxSendOnchainSats = maxAmount)
whenever {
lightningRepo.estimateMaxSendOnchain(
address = address,
speed = TransactionSpeed.Medium,
feeRates = null,
)
}.thenReturn(Result.failure(AppError("no estimate")))
whenever {
lightningRepo.sendOnChain(
address = address,
sats = maxAmount,
speed = TransactionSpeed.Medium,
utxosToSpend = null,
isMaxAmount = false,
tags = emptyList(),
)
}.thenReturn(Result.success("txid"))
setSendState(
SendUiState(
address = address,
amount = maxAmount,
payMethod = SendMethod.ONCHAIN,
speed = TransactionSpeed.Medium,
),
)

sut.setSendEvent(SendEvent.PayConfirmed)
advanceUntilIdle()

verify(lightningRepo).sendOnChain(
address = address,
sats = maxAmount,
speed = TransactionSpeed.Medium,
utxosToSpend = null,
isMaxAmount = false,
tags = emptyList(),
)
}

@Test
fun `private lightning contact payment consumes private list before send`() = test {
val bolt11 = "lnbcrt1privatecontact"
Expand Down
1 change: 1 addition & 0 deletions changelog.d/next/1144.fixed.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed max on-chain sends so the wallet no longer drains at a fee speed the confirmed amount did not account for.
Loading