-
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 3 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,20 @@ class TransferViewModel @Inject constructor( | |
| viewModelScope.launch { | ||
| _spendingUiState.update { it.copy(isLoading = true) } | ||
|
|
||
| val availableAmount = walletRepo.balanceState.value.maxSendOnchainSats | ||
|
|
||
| awaitNodeRunning() | ||
|
|
||
| // Match iOS: size the order against spendable minus fast send-all mining fee so | ||
| // max transfer leaves room to fund the order on-chain. | ||
| val spendable = walletRepo.balanceState.value.maxSendOnchainSats | ||
| val miningFee = lightningRepo.estimateSendAllFee( | ||
| speed = TransactionSpeed.Fast, | ||
| ).getOrElse { | ||
| Logger.warn("Failed to estimate transfer mining fee reserve", it, context = TAG) | ||
| 0uL | ||
| } | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| val availableAmount = | ||
| if (miningFee >= spendable) 0uL else spendable - miningFee | ||
|
greptile-apps[bot] marked this conversation as resolved.
Outdated
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) } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Savings transfers to spending now use a faster on-chain fee rate, reserve mining fees when sizing max transfers, and only drain the wallet when leftover change would be dust or a retry can still fully fund the order. |
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