Skip to content

Conversation

jonasschnelli
Copy link
Contributor

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 new wallet.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).

@btcdrak
Copy link
Contributor

btcdrak commented Sep 15, 2016

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.

@paveljanik
Copy link
Contributor

@btcdrak We can change this to accept the seed only from stdin (-stdin).

@jonasschnelli
Copy link
Contributor Author

Yes. I'm aware of the security weakness of accepting a xpriv over a startup argument.
A previous proposed solution would be to support a tool called bitcoin-wallet which could create, alter and encrypt wallet without the need of bitcoind (this would by my preferred solution but can be added later).

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.

@btcdrak
Copy link
Contributor

btcdrak commented Sep 15, 2016

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.

@btcdrak
Copy link
Contributor

btcdrak commented Sep 15, 2016

@btcdrak We can change this to accept the seed only from stdin (-stdin).

Sounds good, that relieves my concerns.

Copy link
Contributor

@TheBlueMatt TheBlueMatt left a 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;
Copy link
Contributor

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?

@sipa
Copy link
Member

sipa commented Mar 6, 2018

Concept ACK, but I think this needs significant reworking to interact with multiwallet.

@jnewbery
Copy link
Contributor

jnewbery commented Apr 3, 2018

This would possibly be easier to implement using bitcoin-wallet-tool (#8745)

@dancju
Copy link

dancju commented May 28, 2018

I am confused. Is there any workaround that allows me importing xpriv or seed or mnemonic into bitcoind?

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 8, 2018

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.

@DrahtBot DrahtBot closed this Nov 8, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants