-
Notifications
You must be signed in to change notification settings - Fork 38.7k
[Wallet] Add HD xpriv to dumpwallet #8206
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
Conversation
cea1e7d to
9109703
Compare
9109703 to
e4d6997
Compare
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the meaning of "the P2PKH address of the hd master key"? Why would someone need this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extended keys are mostly not P2PKH encoded (they have their "own encoding format"). But here we print the masterkeyid. A P2PKH address should be okay for this case, but we should label it as P2PKH.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonasschnelli I think it is good to split off this change to another pull, as it seems too controversial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is, we store the masterkeyid as a CKeyID. IMO to get the xpub (base58 check encoded extended public key) we need to have a unlocked wallet.
@sipa: What would you propose instead? Caching the xpub in a separate wallet.dat record type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
578b35d to
4ba20ac
Compare
|
Changed the masterkey-ID from P2PKH to a plain hex of the Hash160. |
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: comment still says addr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, could mention that this is the pubkey.
4ba20ac to
4cdac40
Compare
|
Fixed @MarcoFalke 's nits. |
|
@MarcoFalke: |
src/wallet/rpcwallet.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is already merged. Needs rebase
4cdac40 to
d2bcb23
Compare
|
Rebased and fixed nits. |
d2bcb23 to
d07513c
Compare
|
Fixed rebase issue ( |
|
"Implementation looks really clean and simple, and like it should compile ACK" d07513c |
d07513c to
0d17b08
Compare
|
Tested ACK 0d17b08
--- /tmp/hd_old 2016-07-21 22:08:29.063603489 +0200
+++ /tmp/hd_new 2016-07-21 22:28:20.219771942 +0200
@@ -1,9 +1,11 @@
-# Wallet dump created by Bitcoin v0.12.99.0-51a4ee8-dirty
-# * Created on 2016-07-21T20:08:29Z
+# Wallet dump created by Bitcoin v0.13.99.0-0d17b08
+# * Created on 2016-07-21T20:28:20Z
# * Best block at time of backup was 0 (0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206),
# mined on 2011-02-02T23:16:42Z
-cT5hkmWtB9JjGQ98pnfxyHokCKcht8MnGNgGtvhq4M5FvnbE3PT 2011-02-02T21:16:42Z change=1 # addr=moFcHwrWqkULwZ8yrCN8SkPgyBHzAq5WDF
+# extended private masterkey: tprv8ZgxMBicQKsPdriJqoSXGJgLwhkQVGzgr7CMvA79HSYtLYf4rYtwFkkNZAL1qBiE29bkwTF6Sf2Wjn3zUTw5LeWnZgToQq1NAhNDn3ot
+
+cT5hkmWtB9JjGQ98pnfxyHokCKcht8MnGNgGtvhq4M5FvnbE3PT 2011-02-02T21:16:42Z hdmaster=1 # addr=moFcHwrWqkULwZ8yrCN8SkPgyBHzAq5WDF
cVrL8K7UNJa3L78tsuAjdi51HCnQMTkWrDWhNb2dKtzozyReanc 2016-07-21T20:08:14Z label= # addr=mqaKs6gthQGVXWjaaWF9tmpcMrXCYRbKRb
cQajRw3Fnk9QovuXsbrj21SX3EJcF5trzRcrCi85cPzN62qJnMq 2016-07-21T20:08:14Z reserve=1 # addr=mySBbQzsTLBdEwJe6CKEL1HNnqsM9EZm68
--- /tmp/no_old 2016-07-21 22:07:55.770906223 +0200
+++ /tmp/no_new 2016-07-21 22:28:42.492569385 +0200
@@ -1,5 +1,5 @@
-# Wallet dump created by Bitcoin v0.12.99.0-51a4ee8-dirty
-# * Created on 2016-07-21T20:07:55Z
+# Wallet dump created by Bitcoin v0.13.99.0-0d17b08
+# * Created on 2016-07-21T20:28:42Z
# * Best block at time of backup was 0 (0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206),
# mined on 2011-02-02T23:16:42Z
|
3c30f5b to
aa5487b
Compare
|
Slightly updated to allow identifying old, non active master keys (currently useful after encrypting the wallet). Example of an encrypted wallet done with #8389: Though, this PR does not directly depend on #8389. |
src/wallet/rpcdump.cpp
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this code simplification
aa5487b to
df29fe0
Compare
|
Force push changed two typo/text-only changes |
df29fe0 to
c74d640
Compare
c74d640 to
77c912d
Compare
77c912d [Wallet] add HD xpriv to dumpwallet (Jonas Schnelli)
|
Tested ACK 77c912d |
|
Removed "Needs backport" |
P2PKH addressHASH160 of the master key ingetwalletinfoBefore:
After: