-
Notifications
You must be signed in to change notification settings - Fork 2
fix: handle dust change in max transfer to spending #748
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
d35ffc3
f781d8b
0751c39
d14bf62
b9c94c8
2d76f28
3a7eefb
cc927b2
5822e8f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ import org.lightningdevkit.ldknode.ChannelDetails | |
| import to.bitkit.R | ||
| import to.bitkit.data.CacheStore | ||
| import to.bitkit.data.SettingsStore | ||
| import to.bitkit.env.Defaults | ||
| import to.bitkit.ext.amountOnClose | ||
| import to.bitkit.models.Toast | ||
| import to.bitkit.models.TransactionSpeed | ||
|
|
@@ -194,13 +195,26 @@ class TransferViewModel @Inject constructor( | |
| viewModelScope.launch { | ||
| val address = order.payment?.onchain?.address.orEmpty() | ||
|
|
||
| // Calculate if change would be dust and we should use sendAll | ||
| val spendableBalance = | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can be extracted to a private method |
||
| lightningRepo.lightningState.value.balances?.spendableOnchainBalanceSats ?: 0uL | ||
| val txFee = lightningRepo.calculateTotalFee( | ||
| amountSats = order.feeSat, | ||
| address = address, | ||
| speed = speed, | ||
| ).getOrElse { 0uL } | ||
|
|
||
| val expectedChange = spendableBalance.toLong() - order.feeSat.toLong() - txFee.toLong() | ||
| val shouldUseSendAll = expectedChange >= 0 && expectedChange < Defaults.dustLimit.toInt() | ||
|
|
||
| lightningRepo | ||
| .sendOnChain( | ||
| address = address, | ||
| sats = order.feeSat, | ||
| speed = speed, | ||
| isTransfer = true, | ||
| channelId = order.channel?.shortChannelId, | ||
| isMaxAmount = shouldUseSendAll, | ||
| ) | ||
| .onSuccess { txId -> | ||
| cacheStore.addPaidOrder(orderId = order.id, txId = txId) | ||
|
|
@@ -318,30 +332,53 @@ class TransferViewModel @Inject constructor( | |
| isNodeRunning.first { it } | ||
| } | ||
|
|
||
| // Calculate the LSP fee to the total balance | ||
| blocktankRepo.estimateOrderFee( | ||
| // Two-pass fee estimation to match actual order creation | ||
jvsena42 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // First pass: estimate with availableAmount to get approximate clientBalance | ||
| val values1 = blocktankRepo.calculateLiquidityOptions(availableAmount).getOrNull() | ||
| if (values1 == null) { | ||
| _spendingUiState.update { it.copy(isLoading = false) } | ||
| return@launch | ||
| } | ||
| val lspBalance1 = maxOf(values1.defaultLspBalanceSat, values1.minLspBalanceSat) | ||
| val feeEstimate1 = blocktankRepo.estimateOrderFee( | ||
| spendingBalanceSats = availableAmount, | ||
| receivingBalanceSats = _transferValues.value.maxLspBalance | ||
| receivingBalanceSats = lspBalance1, | ||
| ).getOrNull() | ||
|
|
||
| if (feeEstimate1 == null) { | ||
| _spendingUiState.update { it.copy(isLoading = false) } | ||
| return@launch | ||
| } | ||
|
|
||
| val lspFees1 = feeEstimate1.networkFeeSat.safe() + feeEstimate1.serviceFeeSat.safe() | ||
|
Comment on lines
+335
to
+353
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can also be extracted to a method |
||
| val approxClientBalance = availableAmount.safe() - lspFees1.safe() | ||
|
|
||
| // Second pass: recalculate with actual clientBalance that order creation will use | ||
| val values2 = blocktankRepo.calculateLiquidityOptions(approxClientBalance).getOrNull() | ||
| if (values2 == null || values2.maxLspBalanceSat == 0uL) { | ||
| _spendingUiState.update { it.copy(isLoading = false, maxAllowedToSend = 0) } | ||
| return@launch | ||
| } | ||
| val lspBalance2 = maxOf(values2.defaultLspBalanceSat, values2.minLspBalanceSat) | ||
|
Comment on lines
+357
to
+362
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. values1/ values2 is not descriptive enough; makes reader have to actually read implementation to understand what it means |
||
|
|
||
| blocktankRepo.estimateOrderFee( | ||
| spendingBalanceSats = approxClientBalance, | ||
| receivingBalanceSats = lspBalance2, | ||
| ).onSuccess { estimate -> | ||
| maxLspFee = estimate.feeSat | ||
|
|
||
| // Calculate the available balance to send after LSP fee | ||
| val balanceAfterLspFee = availableAmount.safe() - maxLspFee.safe() | ||
| val lspFees = estimate.networkFeeSat.safe() + estimate.serviceFeeSat.safe() | ||
| val maxClientBalance = availableAmount.safe() - lspFees.safe() | ||
|
|
||
| _spendingUiState.update { | ||
| // Calculate the max available to send considering the current balance and LSP policy | ||
| it.copy( | ||
| maxAllowedToSend = min( | ||
| _transferValues.value.maxClientBalance.toLong(), | ||
| balanceAfterLspFee.toLong() | ||
| ), | ||
| maxAllowedToSend = min(values2.maxClientBalanceSat.toLong(), maxClientBalance.toLong()), | ||
| isLoading = false, | ||
| balanceAfterFee = availableAmount.toLong() | ||
| balanceAfterFee = availableAmount.toLong(), | ||
| ) | ||
| } | ||
| }.onFailure { exception -> | ||
| _spendingUiState.update { it.copy(isLoading = false) } | ||
| Logger.error("Failure", exception) | ||
| Logger.error("Failure", exception, context = TAG) | ||
| setTransferEffect(TransferEffect.ToastException(exception)) | ||
| } | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.