gui: Add automated coin selection for spend #563
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Would it be possible to not require a button to trigger the coin selection? Or would that be too involved?
I was thinking of displaying the selecting coins once they've set the feerate and (at least) one address. If they tick any coin before or after filling up the feerate/addresses, we don't do any automated selection.
…-------- Original Message --------
On Jun 30, 2023, 4:33 PM, jp1ac4 wrote:
This adds automated coin selection from [#560](#560) to the GUI.
The new "Auto-select" button uses automated coin selection to select coins for a spend, which the user can then modify as required.
The button can only be clicked once the recipients and feerate have been validated, and it does not appear when creating a self-send.
I haven't added any validation on the amount before making the button clickable, but it may be hard to anticipate all coin selection errors once fees are taken into consideration.
---------------------------------------------------------------
You can view, comment on, or merge this pull request online at:
#563
Commit Summary
- [f031b96](f031b96) temporarily point to auto-select branch
- [203eeda](203eeda) gui: add auto-select button for spend
File Changes
([5 files](https://github.com/wizardsardine/liana/pull/563/files))
- M [gui/Cargo.lock](https://github.com/wizardsardine/liana/pull/563/files#diff-d383f325c462e8a7ca7a279871fd0d92b019c26127a864cb779ef3334a8bc750) (10)
- M [gui/Cargo.toml](https://github.com/wizardsardine/liana/pull/563/files#diff-6ec93b2f00bf881b5541dd9ebd0611a05738fcf970422f3ae38f13c3a5414650) (2)
- M [gui/src/app/state/spend/step.rs](https://github.com/wizardsardine/liana/pull/563/files#diff-ce4cd7a013849448f4d903275aef628ad57141dbfac93155dc01ef531b386c0d) (53)
- M [gui/src/app/view/message.rs](https://github.com/wizardsardine/liana/pull/563/files#diff-da831dcbb7d9b0ca3dd0f41c1e520c7f907c4c59cada6f50728a86a3632afddb) (1)
- M [gui/src/app/view/spend/mod.rs](https://github.com/wizardsardine/liana/pull/563/files#diff-c4434684b1245ad9323130176fac36f59f0068c550f0d7b9039ef14a6d0c75b8) (11)
Patch Links:
- https://github.com/wizardsardine/liana/pull/563.patch
- https://github.com/wizardsardine/liana/pull/563.diff
—
Reply to this email directly, [view it on GitHub](#563), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AFLK3F3F3XJNY2S3I3JI3HTXN3PSHANCNFSM6AAAAAAZZ53EZM).
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
|
|
I think that should be fine. I've made an initial attempt at that now, which seems to work as you describe. It may need some refactoring and better handling of errors from coin selection. |
jp1ac4
force-pushed
the
add-auto-coin-selection-to-gui
branch
from
454535b
to
25cf513
Compare
jp1ac4
force-pushed
the
add-auto-coin-selection-to-gui
branch
5 times, most recently
from
9dbcb41
to
c890e56
Compare
|
This is ready for initial testing. When creating a new spend, coins are selected automatically unless the user edits the selection, after which point only manual selection will be used. In case auto-selection fails due to insufficient funds, the amount left to select will show a positive value. If the user then edits the recipient amounts and/or feerate, coin selection will run again. A spend created using auto-selection should pay the same fee and give the same change amount as another spend created using manual selection with the same auto-selected coins. |
jp1ac4
force-pushed
the
add-auto-coin-selection-to-gui
branch
from
c890e56
to
3ef2e9b
Compare
|
Updated |
|
Got a crash on my first try :p. It might be because i used a receive address as destination address? logsEDIT: this was reported upstream (bitcoindevkit/bdk#1072 (comment)). |
|
Due to how the feerate will be converted from Perhaps we should modify the GUI to use |
|
The latter seems preferable, the GUI shouldn't crash just because an error is returned by a command. EDIT: arf, i read too quickly. It implies we tackle #800 first.. |
jp1ac4
force-pushed
the
add-auto-coin-selection-to-gui
branch
from
3ef2e9b
to
fc9bd8b
Compare
|
Updated Cargo.lock with latest commit hashes for With the latest changes from #560, the GUI will no longer crash if a feerate larger than |
jp1ac4
force-pushed
the
add-auto-coin-selection-to-gui
branch
from
fc9bd8b
to
9e2407e
Compare
|
Reverted |
|
I think the `[WIP]` label and draft status can be removed?
…-------- Original Message --------
On Nov 15, 2023, 9:30 PM, jp1ac4 wrote:
Reverted liana dependency to master 🎉
—
Reply to this email directly, [view it on GitHub](#563 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AFLK3F7WPCXX4Q53QYM5YJ3YEURAHAVCNFSM6AAAAAAZZ53EZOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMJTGIYTKOJWGY).
You are receiving this because you commented.Message ID: ***@***.***>
|
jp1ac4
changed the title
|
Have now set as ready for review. |
darosior
reviewed
Nov 17, 2023
|
|
||
| fn check_valid(&mut self) { | ||
| self.is_valid = | ||
| self.form_values_are_valid() && self.coins.iter().any(|(_, selected)| *selected); |
| } | ||
| let feerate_vb = self.feerate.value.parse::<u64>().unwrap_or(0); | ||
| // Create a spend with empty inputs in order to use auto-selection. | ||
| match daemon.create_spend_tx(&[], &outputs, feerate_vb) { |
There was a problem hiding this comment.
One issue with creating a spend on every update is how the change address derivation index will keep being increased over and over. I think that's fine for now.
There was a problem hiding this comment.
After #821, we could call create_spend here with a fixed change address just for coin selection purposes, and then increment the change index only when the user clicks "Next" and the PSBT is generated on screen.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds automated coin selection from #560 to the GUI.
The new "Auto-select" button uses automated coin selection to select coins for a spend, which the user can then modify as required.
The button can only be clicked once the recipients and feerate have been validated, and it does not appear when creating a self-send.
I haven't added any validation on the amount before making the button clickable, but it may be hard to anticipate all coin selection errors once fees are taken into consideration.