-
Notifications
You must be signed in to change notification settings - Fork 3
fix: use fast fee for savings transfer #1091
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
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 |
|---|---|---|
|
|
@@ -212,7 +212,8 @@ class TransferViewModel @Inject constructor( | |
| } | ||
|
|
||
| /** Pays for the order and start watching it for state updates */ | ||
| fun onTransferToSpendingConfirm(order: IBtOrder, speed: TransactionSpeed? = null) { | ||
| @Suppress("LongMethod") | ||
| fun onTransferToSpendingConfirm(order: IBtOrder, speed: TransactionSpeed = TransactionSpeed.Fast) { | ||
| viewModelScope.launch { | ||
| val address = order.payment?.onchain?.address.orEmpty() | ||
|
|
||
|
|
@@ -230,53 +231,70 @@ class TransferViewModel @Inject constructor( | |
|
|
||
| val expectedChange = | ||
| spendableBalance.toLong() - order.feeSat.toLong() - sendAllFee.toLong() | ||
| // Match iOS: proactive drain only for dust change. Negative change means the drain | ||
| // output would underpay the order — never send-all in that case. | ||
| val shouldUseSendAll = | ||
| expectedChange >= 0 && expectedChange < TRANSFER_SEND_ALL_THRESHOLD_SATS | ||
|
Comment on lines
236
to
237
Member
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: could apply a small refactor converting this to a range check |
||
|
|
||
| val miningFee = if (shouldUseSendAll) { | ||
| sendAllFee | ||
| } else { | ||
| lightningRepo.calculateTotalFee( | ||
| amountSats = order.feeSat, | ||
| address = address, | ||
| speed = speed, | ||
| ).getOrElse { | ||
| Logger.warn("Failed to estimate transfer funding fee", it, context = TAG) | ||
| 0uL | ||
| } | ||
| } | ||
| val txTotalSats = if (shouldUseSendAll) { | ||
| spendableBalance | ||
| } else { | ||
| order.feeSat.safe() + miningFee.safe() | ||
| } | ||
| val maxSendable = | ||
| if (sendAllFee >= spendableBalance) 0uL else spendableBalance - sendAllFee | ||
|
Comment on lines
+238
to
+239
Member
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. use |
||
|
|
||
| Logger.debug( | ||
| "BT confirm: spendable=$spendableBalance, feeSat=${order.feeSat}, " + | ||
| "sendAllFee=$sendAllFee, expectedChange=$expectedChange, sendAll=$shouldUseSendAll", | ||
| context = TAG, | ||
| ) | ||
|
|
||
| lightningRepo | ||
| suspend fun fund(isMaxAmount: Boolean, txTotalSats: ULong) = lightningRepo | ||
| .sendOnChain( | ||
| address = address, | ||
| sats = order.feeSat, | ||
| speed = speed, | ||
| isTransfer = true, | ||
| channelId = order.channel?.shortChannelId, | ||
| isMaxAmount = shouldUseSendAll, | ||
| isMaxAmount = isMaxAmount, | ||
| ) | ||
| .onSuccess { txId -> | ||
| fundPaidOrder( | ||
| order = order, | ||
| txId = txId, | ||
| txTotalSats = txTotalSats, | ||
| preTransferOnchainSats = balanceDetails?.totalOnchainBalanceSats ?: spendableBalance, | ||
| preTransferOnchainSats = balanceDetails?.totalOnchainBalanceSats | ||
| ?: spendableBalance, | ||
| ) | ||
| } | ||
| .onFailure { error -> | ||
|
|
||
| if (shouldUseSendAll) { | ||
| fund(isMaxAmount = true, txTotalSats = spendableBalance) | ||
| .onFailure { ToastEventBus.send(it) } | ||
| return@launch | ||
| } | ||
|
|
||
| val miningFee = lightningRepo.calculateTotalFee( | ||
| amountSats = order.feeSat, | ||
| address = address, | ||
| speed = speed, | ||
| ).getOrElse { | ||
| Logger.warn("Failed to estimate transfer funding fee", it, context = TAG) | ||
| 0uL | ||
| } | ||
| fund( | ||
| isMaxAmount = false, | ||
| txTotalSats = order.feeSat.safe() + miningFee.safe(), | ||
| ).onFailure { error -> | ||
| // Match iOS: if fixed send fails (e.g. coin selection), drain only when the | ||
| // drain output still covers order.feeSat — never underpay by emptying the wallet. | ||
| if (maxSendable >= order.feeSat) { | ||
|
Member
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. I'm still checking this logic |
||
| Logger.warn( | ||
| "Normal transfer funding failed, retrying send-all", | ||
| error, | ||
| context = TAG, | ||
| ) | ||
| fund(isMaxAmount = true, txTotalSats = spendableBalance) | ||
| .onFailure { ToastEventBus.send(it) } | ||
| } else { | ||
| ToastEventBus.send(error) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -415,10 +433,21 @@ class TransferViewModel @Inject constructor( | |
| viewModelScope.launch { | ||
| _spendingUiState.update { it.copy(isLoading = true) } | ||
|
|
||
| val availableAmount = walletRepo.balanceState.value.maxSendOnchainSats | ||
|
|
||
| awaitNodeRunning() | ||
|
|
||
| // Match iOS: start from raw spendable (not maxSendOnchainSats — that already reserved | ||
| // a default-tier send-all fee), then subtract exactly one fast mining fee. | ||
| val spendable = lightningRepo.getBalancesAsync().getOrNull()?.spendableOnchainBalanceSats | ||
| ?: 0uL | ||
| val miningFee = lightningRepo.estimateSendAllFee( | ||
| speed = TransactionSpeed.Fast, | ||
| ).getOrElse { | ||
| Logger.warn("Failed to estimate transfer mining fee reserve", it, context = TAG) | ||
| (spendable.toDouble() * Defaults.fallbackFeePercent).toULong() | ||
| } | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| val availableAmount = | ||
| if (miningFee >= spendable) 0uL else spendable - miningFee | ||
|
Comment on lines
+448
to
+449
Member
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. use .safe() here too |
||
|
|
||
| val initialLspFees = estimateInitialLspFees(availableAmount) | ||
| if (initialLspFees == null) { | ||
| _spendingUiState.update { it.copy(isLoading = false) } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A custom speed is never set anywhere, could remove this parameter