Skip to content
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

Prevent remaining dust when sending max BTC #4979

Open
314159265359879 opened this issue Feb 26, 2024 · 13 comments · Fixed by #5472
Open

Prevent remaining dust when sending max BTC #4979

314159265359879 opened this issue Feb 26, 2024 · 13 comments · Fixed by #5472
Labels
area:bitcoin area:fees area:send bug Functionality broken bug-p2 Critical functionality broken for few users, with no clear workarounds

Comments

@314159265359879
Copy link
Contributor

Affected transaction: https://mempool.space/tx/0a2abf93ada04e8e9335987ee762f5871adab50e083b42b60e774bae1c2925ce
When using send-max the wallet should prevent this.

The user was not 100% certain if they switched to custom fee and then back to medium. I thought we wanted the send amount to change when selecting low, medium or high, when in "send-max" mode, so that everything (economical is spend). And I see that happening when switching between low, medium, high or custom fee.

image

With a much lower fee the number of economical UTXO's increases so the total spend does increase. But that is not relevant for this case because it spends a single big UTXO.

image

The next step would be to try and reproduce this on a wallet that has a single UTXO, to see if there is any difference between the low, medium, high and custom options and if any dust is left. If it is related to this issue: #4777, I wouldn't see it because I am not transmitting these transactions unless I create four wallets with a single UTXO and try all four fee options. I will revisit after 4777 is resolved.

@314159265359879 314159265359879 added bug Functionality broken bug-p2 Critical functionality broken for few users, with no clear workarounds area:bitcoin area:fees area:send labels Feb 26, 2024
@markmhendrickson markmhendrickson added this to the Redesign send UX milestone Feb 26, 2024
@markmhendrickson markmhendrickson changed the title Sending BTC with send-max left dust amount Prevent remaining dust when sending max BTC Feb 26, 2024
@friedger
Copy link
Contributor

I was not able to send max BTC. It is really hard to empty an address!

I used custom fees so that the fees + transferred amount = current balance. Then I receive an error {"code":-26,"message":"dust"}
Looking at the txs it shows that an output with 0 btc was added.

The UI could show the remaining balance on the review page

@friedger
Copy link
Contributor

friedger commented May 29, 2024

My issue is probably more related to adding always a change address if not using sendMax: https://github.com/leather-wallet/extension/blob/d8e0088e97fbe8c195c6040c05764322c112faf4/src/app/common/transactions/bitcoin/utils.ts#L175

I created the tx with the full amount using custom fee because sendMax was not emptying the wallet. Some dust was left over.

@314159265359879
Copy link
Contributor Author

@friedger thanks for the added detail, SendMax is expected to leave dust amounts when those amounts would cost more to include in a transaction than they are worth.

Any chance that was the case? What was the size of the UTXO that were not getting included?

If adding a single input adds 70 vB... (depends on script type) at 20 sats/vB no UTXO would be included of 1400 sats or less because it is not economical.

@friedger
Copy link
Contributor

@314159265359879 I have three utxos above the dust limit, they are used, I see them in console log as coin selection. The dust amount changes with the fee that I choose. I suspect that the spendable balance is not updated/calculated correctly.

@friedger
Copy link
Contributor

#5472 addresses the issue when not using "send max", but still sending the max amount.

@friedger
Copy link
Contributor

When this issue is solved, then #3139 can be closed as well.

@314159265359879
Copy link
Contributor Author

Thanks for diving into this one and creating a PR!

@friedger
Copy link
Contributor

friedger commented Jun 5, 2024

I experienced that sending max does indeed send all btc , however, the display is wrong.

The "Sending" amount is correct, however, the "Total spend" is wrong. Also, the broadcasted tx uses a higher fee rate than shown.

With the higher fee rate, the broadcasted tx does not leave any dust in the account because there is only 1 output.

@314159265359879
Copy link
Contributor Author

@friedger did I understand correctly that your PR here #5472 addresses the concern so that the displayed amount is correct?

When sendMax is clicked, no fee has been selected, so the sats needed to cover the "high" fee option are reserved. Sending the transaction with the standard fee will leave the difference between medium and high fee, is that what you are seeing? Related:
#3080 and this comment about current behavior #3080 (comment)

I believe this will be addressed with the next UX redesign so fees are selected together with the amount being sent rather than in isolation. pinging @markmhendrickson to check me on that.

@friedger
Copy link
Contributor

I don't know where the higher fee comes from. Maybe from "high" fee option, however, I think it is more a problem of determine the size.

@markmhendrickson
Copy link
Collaborator

I believe this will be addressed with the next UX redesign so fees are selected together with the amount being sent rather than in isolation

The amount will continue to be set before the fee is determined, which is necessary given how Bitcoin transactions are constructed. We can't reliably calculate the fee before knowing the amount afaik.

kyranjamie pushed a commit that referenced this issue Jul 15, 2024
## [6.43.0](v6.42.2...v6.43.0) (2024-07-15)

### Features

* add Leather to WBIP004 array, closes [#5615](#5615) ([e38f6ab](e38f6ab))
* mock mainnet btc blockstream requests ([16d751c](16d751c))

### Bug Fixes

* choose account prevent bug, closes [#5509](#5509) ([89500a8](89500a8))
* collectible hover, closes [#4971](#4971) ([7728eeb](7728eeb))
* confine clickable area of account switcher to account name and chevron, closes [#5621](#5621) ([472c7e4](472c7e4))
* dust change amounts, closes [#4979](#4979) ([8b40ea7](8b40ea7))
* increase zIndex of tooltip to prevent it being obscured, closes [#5622](#5622) ([a1f86bb](a1f86bb))
* show account name in signPsbt and signBip322 header, ref [#4657](#4657) [#4859](#4859) ([71f2565](71f2565))

### Internal

* add new analytics events ([3f9548e](3f9548e))
* post-release merge back ([c1bbf89](c1bbf89))
* reenable swaps, closes leather-io/issues[#98](#98) ([5faba22](5faba22))
@friedger
Copy link
Contributor

friedger commented Jul 21, 2024

@markmhendrickson
Copy link
Collaborator

@friedger any idea why it wasn't handled with the merged PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:bitcoin area:fees area:send bug Functionality broken bug-p2 Critical functionality broken for few users, with no clear workarounds
Projects
None yet
3 participants