Display the account xpub in QT interface #1266
Conversation
|
Should add "copy extended public key to clipboard" to the context menu for this new xpub too. |
|
Now that #1265 is merged, also "show QR code" should be added. |
BitcoinWukong
force-pushed
the
wukong--display-account-xpub
branch
from
e56313e
to
027bffb
Compare
PR updated, both issues are addressed, PTAL, thanks! |
BitcoinWukong
force-pushed
the
wukong--display-account-xpub
branch
from
027bffb
to
7d30028
Compare
|
Just made another update to address the display of a "fbond-xpub". |
| parsed = txt.split() | ||
| if len(parsed) > 1: | ||
| if parsed[-1].startswith("xpub") or parsed[-1].startswith("ypub") or parsed[-1].startswith("zpub"): | ||
| xpub = parsed[-1] |
There was a problem hiding this comment.
Both the pre-existing code (relying on "EXTERNAL" and list position of the xpub) and your new code (relying on having a list of items more than 1 and that the final one is where the xpub is stored, and also the repeated check for [x,y,z]pub which I think could be abstracted) are a bit brittle in how they assume the structure of the tree items we're creating.
I don't think there's anything actually wrong though. Also a lot of the code in this script is like that, anyway.
There was a problem hiding this comment.
(You could just get rid of repeated startswith with if any([parsed[-1].startswith[prefix] for prefix in ["xpub",...]]) as one simplifier, and/or push out the list of *pub strings somewhere else)
There was a problem hiding this comment.
I don't think there's anything actually wrong though. Also a lot of the code in this script is like that, anyway.
Going to eventually update the is_extended_public_key method to perform the base58 decoding and compare the check sum. Probably in a following up PR after this one.
|
Tested QR code read and copy-paste to clipboard successfully on Qt. So provisional tACK , just, as per comments, whitespace and also probably should handle tpub somehow. |
BitcoinWukong
force-pushed
the
wukong--display-account-xpub
branch
from
7d30028
to
d95e279
Compare
|
@AdamISZ , comments addressed, PTAL, thanks! |
| @@ -50,6 +55,12 @@ def detect_script_type(script_str): | |||
| raise EngineError("Unknown script type for script '{}'" | |||
| .format(bintohex(script_str))) | |||
|
|
|||
|
|
|||
| def is_extended_public_key(key_str): | |||
| return any([key_str.startswith(prefix) for prefix in [ | |||
There was a problem hiding this comment.
A more robust check would be doing a base58 decoding check here.
But I don' think that has to be part of this PR now.
|
I think after another round of testing, I'm happy to add tACK and will merge, however: you've removed the |
This I guess a better solution might be we give the user an option to show/hide this prefix base on their own need. |
Right, it probably seems like a silly question except: if a person did actually have any timelocked funds in that mixdepth, and they tried to import it somewhere else, or another Joinmarket read-only setup (if that were possible yet), the timelocked funds would not be shown.
Yeah I guess that could make sense. I'm just not sure exactly what should be done here. |
So for the time being, I'm only dropping the prefix and converting the xpub to ypub/zpub in QT interface. When the user run Not a perfect solution, but at least this way the user can have both format available to them. |
The account xpub is available when you run
python wallet-tool.py wallet.jmdat, but it's not available in the QT interface.This PR shows the account xpub in the QT interface as well so that users can also access it from joinmarket-qt.