Skip to content
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

[RPC] dumpmasterprivkey command #9504

Closed
wants to merge 1 commit into from

Conversation

achow101
Copy link
Member

RPC command to export the master private key from a wallet.

@luke-jr
Copy link
Member

luke-jr commented Jan 10, 2017

dumpwallet already does this...?

@achow101
Copy link
Member Author

dumpwallet requires that you dump to a file then find that file and open it to get the master private key. This just gives it to you with the command.

Copy link
Contributor

@jonasschnelli jonasschnelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we should add more private key exporting features because it increases the risk of doing something wrong with that.

If we are going to add this, it would require...
... add dumpmasterprivkey to the UI's sensitiv data filter
... add a test somewhere

@@ -2602,6 +2603,7 @@ static const CRPCCommand commands[] =
{ "wallet", "addwitnessaddress", &addwitnessaddress, true, {"address"} },
{ "wallet", "backupwallet", &backupwallet, true, {"destination"} },
{ "wallet", "dumpprivkey", &dumpprivkey, true, {"address"} },
{ "wallet", "dumpmasterprivkey", &dumpmasterprivkey, true, {} },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: column alignment.


EnsureWalletIsUnlocked();

CKeyID masterKeyID = pwalletMain->GetHDChain().masterKeyID;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use CWallet::IsHDEnabled()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well,... you need the masterKeys CKeyID anyway,... but I guess it would still make sense to use CWallet::IsHDEnabled()

@achow101
Copy link
Member Author

@jonasschnelli there is no sensitive data filter for the dump* commands. The user is not entering sensitive data, only sensitive data being output.

@achow101
Copy link
Member Author

Addressed nits and added a test.

It may need more tests, but I'm not sure what actually needs to be tested.

@jonasschnelli
Copy link
Contributor

@jonasschnelli there is no sensitive data filter for the dump* commands. The user is not entering sensitive data, only sensitive data being output.

Argh. Right. I was confusing the sensitiv history filter with the idea of having a filter that disables certain sensitive commands.

@achow101 achow101 force-pushed the dumpmasterprivkey branch 2 times, most recently from c74e66b to dfa8648 Compare August 17, 2017 01:15
RPC command to export the master private key from a wallet.
@achow101
Copy link
Member Author

achow101 commented Feb 8, 2018

Closing this for now

@achow101 achow101 closed this Feb 8, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 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.

None yet

4 participants