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

Gui taproot #986

Merged
merged 6 commits into from
Mar 20, 2024
Merged

Gui taproot #986

merged 6 commits into from
Mar 20, 2024

Conversation

edouardparis
Copy link
Member

@edouardparis edouardparis commented Feb 27, 2024

based on #985

@darosior
Copy link
Member

If not too complicated i think it's worth hiding the network and Taproot selectors under a collapsible "Advanced settings" options at the bottom right before the buttons.

As @kloaec rightly points out 99% people want mainnet, not signet or regtest, and the 1% can use advanced settings. Same goes for Taproot for now.

gui/src/installer/view.rs Outdated Show resolved Hide resolved
gui/src/installer/view.rs Outdated Show resolved Hide resolved
@edouardparis
Copy link
Member Author

Tried to put the pick_list behind an 'advanced settings' that would open a modal, but iced does not support the nesting of overlay iced-rs/iced#940
I am going back for the dropdown as suggested.

@edouardparis edouardparis force-pushed the gui-taproot branch 4 times, most recently from 205c0a4 to 1aca594 Compare March 14, 2024 15:42
@edouardparis edouardparis marked this pull request as ready for review March 14, 2024 15:46
@darosior
Copy link
Member

  • Why are the advanced settings after the "Next" button?
  • I get an error when signing with a simple "1 - 1" descriptor (signing using the primary key on the Ledger which is the internal Taproot key). Both for refresh and external payments.
  • It seems change is not properly detected. Cf the screenshot.
    image

My descriptor:

tr([b0822927/48'/1'/0'/2']tpubDEvZxV86Br8Knbm9tWcr5Hvmg5cYTYsg92vinqH6Bie6U8ix8CsoN9W11NQygdqVwmHUJpsHXxNsi5gXn36g4xNfLWkMqPuFhRZAmMQ7jjQ/<0;1>/*,and_v(v:pk([cafb6853/48'/1'/0'/2']tpubDF5GEQ4diRqBUs3hSq1CZ1eveX1yjHptGVEDH383xMJv52cwn2fzJcr3CDnDCRLrdBdRqA2jbggFgy7u9VBkmYW5qwVT8gzLQ4cYamqhMRr/<0;1>/*),older(65535)))#9w9v5fzt

@darosior
Copy link
Member

nit: i think the error message can just be "Taproot is not supported by this device". I think it would be clearer to a user.
image

@darosior
Copy link
Member

I can't click on next anymore after removing the Bitbox from the path though.

@darosior
Copy link
Member

When manually selecting coins, i can't click on "Next" when creating a spend.

@darosior
Copy link
Member

I don't think the bug above is related to this PR. I also found #1024 which i don't think is related to this PR.

@darosior
Copy link
Member

tACK b7f35c0 -- i've lightly tested this a couple times. It's good enough to get in and get tested along with the other changes.

@darosior darosior merged commit 4f78d3b into wizardsardine:master Mar 20, 2024
20 of 21 checks passed
@edouardparis edouardparis deleted the gui-taproot branch March 20, 2024 18:26
darosior added a commit that referenced this pull request Mar 22, 2024
a7f2bcc gui: handle daemon stop error (edouardparis)
924df8e bump liana:master (edouardparis)

Pull request description:

  based on #986

ACKs for top commit:
  darosior:
    ACK a7f2bcc

Tree-SHA512: 24c48948f10eed7f04dcab0779b75d89c5fd40441c04dffb6f4e5b3e682ca2ca36de51cdc69e7679ed262c4e92691d789c29822972f5c99290c54d13c7dc3472
@darosior darosior mentioned this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants