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

add faucet and exfund extensions #1820

Merged
merged 13 commits into from
Aug 25, 2022
Merged

Conversation

stepansnigirev
Copy link
Collaborator

I wrote two extensions that might be useful for people. This PR adds them to the default Specter distribution.

Faucet extension

It helps to control your regtest node - generate blocks and fund wallets directly from Specter, without going to command line. Useful for development. As it's a development extension it is available with --config DevelopmentConfig flag.

Repository

Exfund extension

Helps to do transactions with large number of outputs - you can select a CSV file with addresses and amounts, extension will parse it and display outputs in a convenient table form. Then you can tweak the fee rate, select wallet to use etc, and then get to a normal signing flow of Specter-Desktop. Amounts can be changed on all inputs at the same time as well - so if funding multiple addresses with the same amount you don't need to have it in the CSV. Also supports liquid.

Repository

@netlify
Copy link

netlify bot commented Jul 23, 2022

Deploy Preview for specter-desktop-docs ready!

Name Link
🔨 Latest commit 3ec03ac
🔍 Latest deploy log https://app.netlify.com/sites/specter-desktop-docs/deploys/630741e7777adc000903a62e
😎 Deploy Preview https://deploy-preview-1820--specter-desktop-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@relativisticelectron
Copy link
Collaborator

relativisticelectron commented Jul 26, 2022

The Exfund extension looks like a great replacement for the current txt based input (which lacks labels):
image

I doubt there are users, which would prefer the current text based input over a csv based input.

@k9ert
Copy link
Collaborator

k9ert commented Jul 26, 2022

It would work on v1.10.5 but also would break if we would merge #1782 . Needs some rework:
#1782 (comment)

The Exfund extension looks like a great replacement for the current txt based input (which lacks labels):

This might also be a good opportunity for me to look how extensions can replace core UI components. Not in this PR, though.

@stepansnigirev
Copy link
Collaborator Author

I will migrate the extension to json interface of the PSBT creator, it makes more sense than hacking into ui. I fixed json interface for liquid in #1831

@stepansnigirev
Copy link
Collaborator Author

Updated to use json api, will not work until #1831 is merged

@moneymanolis
Copy link
Collaborator

moneymanolis commented Aug 5, 2022

UX suggestions for the Exfund extension

  1. Make those buttons smaller (via max-width etc.)

grafik

  1. More width for the tooltip (just adding width=XXX)

grafik

  1. Rather use sticky and the overall scrollbar than a scrollable table for this table:

grafik

See the discussion and end result in this PR

  1. Perhaps a toggle for the fee options?

grafik

  1. And perhaps also one for settings?
    Takes a lot of space for this simple decision and stays there forever.

grafik

@moneymanolis
Copy link
Collaborator

Parsing of the recipients / destinations from CSV works, but I get this error when trying to continue after that:

grafik

@moneymanolis
Copy link
Collaborator

moneymanolis commented Aug 5, 2022

The Exfund extension looks like a great replacement for the current txt based input (which lacks labels)
I doubt there are users, which would prefer the current text based input over a csv based input.

I agree but before removing the text based stuff in the UI we have to ensure that the exfund extension works solidly.

@stepansnigirev
Copy link
Collaborator Author

Parsing of the recipients / destinations from CSV works, but I get this error when trying to continue after that:

grafik

Can you provide test data you used to get this error? Then I could reproduce it.

@moneymanolis
Copy link
Collaborator

Can you provide test data you used to get this error? Then I could reproduce it.

I used your test data: https://github.com/stepansnigirev/specterext-exfund#example-csv

@moneymanolis
Copy link
Collaborator

If I use get_by_alias() to get the wallet instance it works!

grafik

@moneymanolis
Copy link
Collaborator

UX suggestions for the Faucet extension

  • "Get some funds from your default wallet" - I think most people (even devs) will not understand that the default wallet in Core ("") is meant here. Either add / explain this or just skip the default wallet part. I thought first there that some sort of default wallet in Specter was set through the extension.
  • <input type="number" value="0.1" name="amount" id="amount"> the value attribute seems to only allow (because of some browser side validation) amounts with 0.1 as decimal, so only 15.1 not 15:

grafik

Just replace value with a placeholder?

  • Why show "Send back to bcrt1qsrts3q6xwae9whxx8sn957d6pvpj2924uxt5ju"? Doesn't really matter to which address of the default wallet the generatetoaddress is transferring to.

General questions

  • How is the Core default wallet getting funds? Clicking 100 times generate block doesn't seem great.

@stepansnigirev
Copy link
Collaborator Author

I addressed all your comments and bumped pip versions.

Why show "Send back to bcrt1qsrts3q6xwae9whxx8sn957d6pvpj2924uxt5ju"? Doesn't really matter to which address of the default wallet the generatetoaddress is transferring to.

It's sometimes useful to get a random destination address to test send functionality, so I decided to add this line with a fresh address of the default wallet, so people can grab it and send money to it while testing.

@moneymanolis
Copy link
Collaborator

It's sometimes useful to get a random destination address to test send functionality, so I decided to add this line with a fresh address of the default wallet, so people can grab it and send money to it while testing.

Ah okay, I can now see the benefit of that!

@moneymanolis
Copy link
Collaborator

@stepansnigirev I've tested the exfund plugin again. Looks nice. I think we should add the @login_required decorator to the index route otherwise you can run into this error (if you have authentication setup up and enter the URL directly, e.g. if you have it bookmarked etc.):
AttributeError: 'AnonymousUserMixin' object has no attribute 'wallet_manager'
The alternative would be to replace the flask User object current_user with userfrom the user_manager but then there would be no check whether the user is logged in which is currently not a problem for the plugin but could be an issue later down the line if there are further changes to the plugin.

@moneymanolis
Copy link
Collaborator

@relativisticelectron I made a PR in the extension repo, in case you want to check it out and test: stepansnigirev/specterext-exfund#4

@k9ert k9ert merged commit 4f0139c into cryptoadvance:master Aug 25, 2022
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.

4 participants