-
Notifications
You must be signed in to change notification settings - Fork 37.7k
[Wallet] add option for a custom extended master privat key (xpriv) #8735
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
… HD wallet creation
b9e0d93
to
d772d3b
Compare
As it stands this will leak the xpriv since the xpriv will be in the command history. The import of the xpriv should be interactive and should be encrypted before being stored. I know previously we would not recommend forcing encryption because it could lead to funds loss, here the user already has the xpriv so it's already backed up. |
@btcdrak We can change this to accept the seed only from stdin ( |
Yes. I'm aware of the security weakness of accepting a xpriv over a startup argument. I think accepting a xpriv over a configuration value is not extremely bad. If an attacker can access bitcoin.conf he might also be capable of grabbing your RPC password and dump your wallet to /tmp. |
Personally I prefer a separate interactive import tool and a Qt GUI option which I think would be used more in any case. FWIW, I think importing via a bitcoin.conf option is even worse. |
Sounds good, that relieves my concerns. |
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.
Needs rebase.
|
||
static const int VERSION_BASE = 1; | ||
static const int VERSION_WITH_CUSTOM_MASTER_KEY = 2; | ||
static const int CURRENT_VERSION = VERSION_WITH_CUSTOM_MASTER_KEY; |
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.
Why add a new version? Why not just overwrite more akin to the way #11085 works?
Concept ACK, but I think this needs significant reworking to interact with multiwallet. |
This would possibly be easier to implement using bitcoin-wallet-tool (#8745) |
I am confused. Is there any workaround that allows me importing xpriv or seed or mnemonic into |
There hasn't been much activity lately and the patch still needs rebase, so I am closing this for now. Please let me know when you want to continue working on this, so the pull request can be re-opened. |
At the moment, the HD seed cannot be set by the user. It will always be generated by Bitcoin-Core (
CKey::MakeNewKey()
).This PR adds a startup argument
-hdxpriv
where users can set their custom hd extended master private key. The argument takes only affect during the creation of a newwallet.dat
and there will be a warning if the argument is present during loading an exiting wallet.If a custom extended master private key has been set, it will be kept when encrypting the wallet (with log-printing a warning).