Skip to content

Commit

Permalink
rpc: make sure upgradetohd always has the passphrase for UpgradeToHD
Browse files Browse the repository at this point in the history
earlier it was possible to make it all the way to `EncryptSecret`
without actually having the passphrase in hand until being told off
by `CCrypter::SetKey`, we should avoid that.

also, let's get rid of checks that `UpgradeToHD` is now taking
responsibility for. no point in checking if the wallet is unlocked
as it has no bearing on your ability to upgrade the wallet.
  • Loading branch information
kwvg committed Jul 15, 2024
1 parent bd168da commit e7a9fc0
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
24 changes: 14 additions & 10 deletions src/wallet/rpcwallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2779,11 +2779,14 @@ static RPCHelpMan upgradetohd()
{
return RPCHelpMan{"upgradetohd",
"\nUpgrades non-HD wallets to HD.\n"
"\nWarning: You will need to make a new backup of your wallet after setting the HD wallet mnemonic.\n",
"\nThe wallet passphrase must be supplied as it is used to encrypt the HD chain. Unlocking the wallet beforehand\n"
"\nhas no bearing on the encryption process and cannot be used in place of supplying the passphrase.\n"
"\nSupplying an incorrect passphrase may result in your wallet getting locked.\n"
"\n\nWarning: You will need to make a new backup of your wallet after setting the HD wallet mnemonic.\n",
{
{"mnemonic", RPCArg::Type::STR, /* default */ "", "Mnemonic as defined in BIP39 to use for the new HD wallet. Use an empty string \"\" to generate a new random mnemonic."},
{"mnemonicpassphrase", RPCArg::Type::STR, /* default */ "", "Optional mnemonic passphrase as defined in BIP39"},
{"walletpassphrase", RPCArg::Type::STR, /* default */ "", "If your wallet is encrypted you must have your wallet passphrase here. If your wallet is not encrypted specifying wallet passphrase will trigger wallet encryption."},
{"walletpassphrase", RPCArg::Type::STR, /* default */ "", "If your wallet is encrypted you must have your wallet passphrase here. If your wallet is not encrypted, specifying wallet passphrase will trigger wallet encryption."},
{"rescan", RPCArg::Type::BOOL, /* default */ "false if mnemonic is empty", "Whether to rescan the blockchain for missing transactions or not"},
},
RPCResult{
Expand All @@ -2803,17 +2806,17 @@ static RPCHelpMan upgradetohd()
bool generate_mnemonic = request.params[0].isNull() || request.params[0].get_str().empty();
SecureString secureWalletPassphrase;
secureWalletPassphrase.reserve(100);
// TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string)
// Alternately, find a way to make request.params[0] mlock()'d to begin with.
if (!request.params[2].isNull()) {
secureWalletPassphrase = request.params[2].get_str().c_str();
if (!pwallet->Unlock(secureWalletPassphrase)) {
throw JSONRPCError(RPC_WALLET_PASSPHRASE_INCORRECT, "The wallet passphrase entered was incorrect");

if (request.params[2].isNull()) {
if (pwallet->IsCrypted()) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet encrypted but passphrase not supplied. Please refer to \"help upgradetohd\" for guidance.");
}
} else {
// TODO: get rid of this .c_str() by implementing SecureString::operator=(std::string)
// Alternately, find a way to make request.params[0] mlock()'d to begin with.
secureWalletPassphrase = request.params[2].get_str().c_str();
}

EnsureWalletIsUnlocked(pwallet.get());

SecureString secureMnemonic;
secureMnemonic.reserve(256);
if (!generate_mnemonic) {
Expand All @@ -2825,6 +2828,7 @@ static RPCHelpMan upgradetohd()
if (!request.params[1].isNull()) {
secureMnemonicPassphrase = request.params[1].get_str().c_str();
}

// TODO: breaking changes kept for v21!
// instead upgradetohd let's use more straightforward 'sethdseed'
constexpr bool is_v21 = false;
Expand Down
4 changes: 2 additions & 2 deletions test/functional/wallet_upgradetohd.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ def run_test(self):
node.stop()
node.wait_until_stopped()
self.start_node(0, extra_args=['-rescan'])
assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.upgradetohd, mnemonic)
assert_raises_rpc_error(-14, "The wallet passphrase entered was incorrect", node.upgradetohd, mnemonic, "", "wrongpass")
assert_raises_rpc_error(-4, "Wallet encrypted but passphrase not supplied. Please refer to \"help upgradetohd\" for guidance.", node.upgradetohd, mnemonic)
assert_raises_rpc_error(-1, "GenerateNewHDChain: supplied incorrect passphrase", node.upgradetohd, mnemonic, "", "wrongpass")
assert node.upgradetohd(mnemonic, "", walletpass)
assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.dumphdinfo)
node.walletpassphrase(walletpass, 100)
Expand Down

0 comments on commit e7a9fc0

Please sign in to comment.