Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

add --show-xpub option #577

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

dan-da
Copy link

@dan-da dan-da commented Jun 17, 2016

note: I closed pr #575 and opened this instead based on develop branch instead of master because I don't know how to modify the original pr.

This pull request addresses issue #573.

User facing changes:

new flag --show-xpub "Display master xpub key for wallet and each mix level"
if flag is present:
    wallet xpub is displayed first with label: "wallet xpub:"
    mixlevel xpub is displayed before addresses with label: "xpub:".
if flag is not present, no xpub are displayed, not even for external addresses.

note: This pull request removes the display of xpub for external addresses (m/0/x/0). In my judgement, that info is mostly redundant to m/0/x xpub and clutters up the screen. A possible enhancement could be to add a verbosity level to the --show-xpub flag, in which case it could print xpub for both m/0/x/0 and m/0/x/1. Or we could simply keep the previous behavior.

note: This pull request intentionally does not address display of xpriv keys as there are potentially more privacy issues around that, and it could easily be implented with a separate flag, eg --show-xpriv.

Alternatively, the --show-xpub flag could be removed, so that xpubs are always displayed and there is is one less flag/option. ( per discussion in #496 )

Let me know your thoughts.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 78.069% when pulling 65b8cf5 on dan-da:xpub_pull_request2 into c2ec1dd on JoinMarket-Org:develop.

@dan-da
Copy link
Author

dan-da commented Jul 2, 2016

comments?

@AdamISZ AdamISZ force-pushed the develop branch 3 times, most recently from 97d2603 to 47479d5 Compare September 13, 2016 11:23
@AdamISZ
Copy link
Member

AdamISZ commented Sep 15, 2016

I hadn't looked at this at all, so this is additional to #493 right, i.e. add xpubs for whole mixdepth and for whole wallet?

AFAIU #493 was based on the idea that you can give out an xpub for people to send funds to your wallet, while this is motivated differently. Quoting @chris-belcher from #573 "I'll leave it to someone else to think about the privacy and security aspects and decide whether it's a good idea to do this or not." The problem is I don't have time to look at everything, and this one seemed to slip through the gaps. Does anyone else have an opinion about it?

@BlinkyStitt
Copy link
Contributor

Do we still need this? It looks like wallet-tool already has something to print xpubs

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants