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 genwallet.py script #688

Merged
merged 2 commits into from
Sep 27, 2020
Merged

Conversation

nixbitcoin
Copy link
Contributor

This PR adds my version of @AdamISZ's genwallet.py from #683.

I'm not very familiar with the intricacies of JoinMarket's testing infrastructure, so maybe somebody else could add a simple test for this script.

Closes #683

@AdamISZ
Copy link
Member

AdamISZ commented Sep 17, 2020

Nice, thanks. I guess it makes sense to add an option for wallet type; in particular considering #656 and some other use cases for bech32 already (e.g. payjoin).

@AdamISZ
Copy link
Member

AdamISZ commented Sep 17, 2020

In 2f53c5f I added (admittedly fairly trivial) tests to check that when a wallet file is created with create_wallet, it can be opened with the specified password, and has matching secrets. Feel free to edit if you like.

@kristapsk
Copy link
Member

This duplicates functionality of wallet-tool.py generate, I'm wondering can't we figure out a way to just do a non-interactive way to do it using wallet-tool instead of having two scripts.

@AdamISZ
Copy link
Member

AdamISZ commented Sep 18, 2020

This duplicates functionality of wallet-tool.py generate, I'm wondering can't we figure out a way to just do a non-interactive way to do it using wallet-tool instead of having two scripts.

Very fair point yeah. This same code could be trivially repurposed as just another method in wallet-tool (if anyone is so inclined they could open another PR with that alternative). Another angle might be: make a subdirectory under scripts/ for "advanced/developer tool" scripts.

Personally I'm quite focused on moving in the direction in #670 (an RPC API), so I'm a bit less interested in exactly how to add more functionality into the scripts.

@AdamISZ
Copy link
Member

AdamISZ commented Sep 25, 2020

I feel like it's fine to just add this script as-is, even if there's an argument for folding it into the wallet-tool script (it could always be done later). Ordinary users won't need this anyway. Agreed @kristapsk ?

@kristapsk
Copy link
Member

@AdamISZ Agree

Copy link
Member

@kristapsk kristapsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 2f53c5f

@AdamISZ AdamISZ merged commit 6545f36 into JoinMarket-Org:master Sep 27, 2020
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.

Allow non-interactive usage of wallet-tool.py generate
3 participants