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.

Sign up for GitHub

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

Jump to bottom

When sending, have a max button to send the whole balance (sweep wallet) #546

Closed
pythcoiner opened this issue May 29, 2023 · 14 comments
Closed

When sending, have a max button to send the whole balance (sweep wallet) #546

pythcoiner opened this issue May 29, 2023 · 14 comments
Assignees
@jp1ac4
Labels
Daemon / Liana library This is about lianad or the liana library (not the GUI) Feature New feature or functionality. GUI gui related

Comments

@pythcoiner
Copy link
Collaborator

pythcoiner commented May 29, 2023 •
edited
Loading

In the send screen, it would be useful to have a MAX button that autofill the sum amount of all the wallet coins.
image
@darosior
Copy link
Member

Yes. It's something we've discussed. It's a bit intricate to make work as you change the feerate though.

@darosior darosior added GUI gui related Feature New feature or functionality. labels May 30, 2023
@darosior darosior changed the title Add a 'MAX' button for send whole coins When sending, have a max button to send the whole balance May 30, 2023
@pythcoiner
Copy link
Collaborator Author

Yes. It's something we've discussed. It's a bit intricate to make work as you change the feerate though.

yes, i think if max has been selected (and until send value not changed by the user) the output amount might be implicitly (total - fee)

@darosior darosior changed the title When sending, have a max button to send the whole balance When sending, have a max button to send the whole balance (sweep wallet) Jul 13, 2023
@darosior
Copy link
Member

This was brought up today again. A path forward could be to have the recovery command logic be moved to a "sweep" helper and add another command, sweep, that takes a single address and sweeps the whole balance there through the primary path.

The integration in the GUI would be more involved. If sweep was used, it should not let the user add more destinations. Nor select coins actually. @edouardparis wdyt?

@darosior darosior added the Daemon / Liana library This is about lianad or the liana library (not the GUI) label Jul 13, 2023
@darosior
Copy link
Member

Adding the daemon label as ideally i think the sweep logic should be in the daemon and tested using the functional tests.

@edouardparis
Copy link
Member

Recovery panel could be renamed sweep and the primary path added to the list of recovery paths.
Wording can be:

2 paths are accessible next block, please select one

  • The primary path
    • Signatures from Ed and Antoine can move 11 coins totalling 1.2334 BTC
  • Recovery path:
    • Signatures from Ed can move 4 coins totalling 0.6244 BTC

@darosior
Copy link
Member

darosior commented Jul 13, 2023 via email

@pythcoiner
Copy link
Collaborator Author

sweep, that takes a single address and sweeps the whole balance there through the primary path.

Do you mean to move in batch one to one or consolidate many to one?

@darosior
Copy link
Member

See also #821.

@darosior
Copy link
Member

Relevant: #863 (comment)

@jp1ac4
Copy link
Collaborator

jp1ac4 commented Feb 7, 2024

Is the requirement to rename the Recovery panel as in #546 (comment) instead of adding a max button as in #546 (comment)?

@darosior
Copy link
Member

darosior commented Feb 7, 2024

I'm not sure. Any opinion?

@jp1ac4
Copy link
Collaborator

jp1ac4 commented Feb 7, 2024

I think we could do both. Having the max button on the Send page is intuitive I think and users may expect to see that option there.

We could have the max button even for multiple recipients: for a given recipient, clicking the max button would return the max available for that recipient after taking into account the other recipients.

@darosior
Copy link
Member

darosior commented Feb 7, 2024

Sounds good. Then let's do the max button first. We can track changing the recovery screen for a sweep screen into another issue.

I think you have access to the beta of Liana Lite (/Online)? There is a max amount there if you want to play with it before starting with the implementation.

@jp1ac4 jp1ac4 mentioned this issue Feb 23, 2024
Merged
edouardparis added a commit that referenced this issue Mar 4, 2024
@edouardparis
Merge #974: gui: add max button for spend recipients
3420c54 
46cd0f4 gui: add max checkbox for spend recipients (jp1ac4)
83fa9a9 gui: unset amount left to select if form invalid (jp1ac4)
fba77a3 gui: check if feerate is already set (jp1ac4)
d9b8285 gui: fix cargo fmt (jp1ac4)

Pull request description:

  This adds the MAX button from #546.

  The max amount is calculated within the `redraft` function based on the change output. The recipient's amount is updated directly within this same function in order to avoid another call to `redraft` after updating the amount.

  The MAX button only takes effect once when the user clicks it. If further changes are made (e.g. to feerate or other recipients), the value will not update and the user will need to click MAX again.

  Currently, the MAX button is always clickable. It would be possible to determine for each recipient whether the MAX button should be clickable for that recipient (feerate is valid and all other recipients are valid and no duplicates), but it's not a trivial change so I'm not sure if it's worth doing as I think the behaviour is quite intuitive.

ACKs for top commit:
  edouardparis:
    ACK 46cd0f4

Tree-SHA512: 16f22804d630686427da196fd1627287e2e20f2e122602c385157064323113b27b9ec9c56ada162acb55d11b8084667ee3f1a9f992a2f4eef70f25c6f7a5b20b
@jp1ac4 jp1ac4 mentioned this issue Mar 5, 2024
Closed
@jp1ac4
Copy link
Collaborator

jp1ac4 commented Mar 5, 2024

We can track changing the recovery screen for a sweep screen into another issue.

This is now in issue #994.

@jp1ac4 jp1ac4 closed this as completed Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jp1ac4 jp1ac4

Labels
Daemon / Liana library This is about lianad or the liana library (not the GUI) Feature New feature or functionality. GUI gui related
Projects
No open projects
Liana v5
Status: Done
Milestone
No milestone
Development

No branches or pull requests
4 participants
@edouardparis @darosior @jp1ac4 @pythcoiner