diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -113,7 +113,8 @@ return NullUniValue; } - if (request.fHelp || request.params.size() < 1 || request.params.size() > 3) + if (request.fHelp || request.params.size() < 1 || + request.params.size() > 3) { throw std::runtime_error( "importprivkey \"privkey\" ( \"label\" ) ( rescan )\n" "\nAdds a private key (as returned by dumpprivkey) to your wallet. " @@ -142,6 +143,13 @@ HelpExampleCli("importprivkey", "\"mykey\" \"\" false") + "\nAs a JSON-RPC call\n" + HelpExampleRpc("importprivkey", "\"mykey\", \"testing\", false")); + } + + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_WALLET_ERROR, + "Cannot import private keys to a wallet with " + "private keys disabled"); + } WalletRescanReserver reserver(pwallet); bool fRescan = true; @@ -661,12 +669,13 @@ uiInterface.ShowProgress( strprintf("%s " + _("Importing..."), pwallet->GetDisplayName()), 0, false); + std::vector> keys; + std::vector> scripts; while (file.good()) { uiInterface.ShowProgress( "", - std::max(1, std::min(99, (int)(((double)file.tellg() / - (double)nFilesize) * - 100))), + std::max(1, std::min(50, 100 * double(file.tellg()) / + double(nFilesize))), false); std::string line; std::getline(file, line); @@ -681,19 +690,10 @@ } CKey key = DecodeSecret(vstr[0]); if (key.IsValid()) { - CPubKey pubkey = key.GetPubKey(); - assert(key.VerifyPubKey(pubkey)); - CKeyID keyid = pubkey.GetID(); - if (pwallet->HaveKey(keyid)) { - pwallet->WalletLogPrintf( - "Skipping import of %s (key already present)\n", - EncodeDestination(keyid, config)); - continue; - } int64_t nTime = DecodeDumpTime(vstr[1]); std::string strLabel; bool fLabel = true; - for (unsigned int nStr = 2; nStr < vstr.size(); nStr++) { + for (size_t nStr = 2; nStr < vstr.size(); nStr++) { if (vstr[nStr].front() == '#') { break; } @@ -708,42 +708,86 @@ fLabel = true; } } - pwallet->WalletLogPrintf("Importing %s...\n", - EncodeDestination(keyid, config)); - if (!pwallet->AddKeyPubKey(key, pubkey)) { - fGood = false; - continue; - } - pwallet->mapKeyMetadata[keyid].nCreateTime = nTime; - if (fLabel) { - pwallet->SetAddressBook(keyid, strLabel, "receive"); - } - nTimeBegin = std::min(nTimeBegin, nTime); + keys.push_back(std::make_tuple(key, nTime, fLabel, strLabel)); } else if (IsHex(vstr[0])) { std::vector vData(ParseHex(vstr[0])); CScript script = CScript(vData.begin(), vData.end()); - if (pwallet->HaveCScript(script)) { - pwallet->WalletLogPrintf( - "Skipping import of %s (script already present)\n", - vstr[0]); - continue; - } - if (!pwallet->AddCScript(script)) { - pwallet->WalletLogPrintf("Error importing script %s\n", - vstr[0]); - fGood = false; - continue; - } int64_t birth_time = DecodeDumpTime(vstr[1]); - if (birth_time > 0) { - pwallet->m_script_metadata[CScriptID(script)].nCreateTime = - birth_time; - nTimeBegin = std::min(nTimeBegin, birth_time); - } + scripts.push_back( + std::pair(script, birth_time)); } } file.close(); - + // We now know whether we are importing private keys, so we can error if + // private keys are disabled + if (keys.size() > 0 && + pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + // hide progress dialog in GUI + uiInterface.ShowProgress("", 100, false); + throw JSONRPCError( + RPC_WALLET_ERROR, + "Importing wallets is disabled when private keys are disabled"); + } + double total = double(keys.size() + scripts.size()); + double progress = 0; + for (const auto &key_tuple : keys) { + uiInterface.ShowProgress( + "", + std::max(50, std::min(75, (progress / total) * 100) + 50), + false); + const CKey &key = std::get<0>(key_tuple); + int64_t time = std::get<1>(key_tuple); + bool has_label = std::get<2>(key_tuple); + std::string label = std::get<3>(key_tuple); + + CPubKey pubkey = key.GetPubKey(); + assert(key.VerifyPubKey(pubkey)); + CKeyID keyid = pubkey.GetID(); + if (pwallet->HaveKey(keyid)) { + pwallet->WalletLogPrintf( + "Skipping import of %s (key already present)\n", + EncodeDestination(keyid, config)); + continue; + } + pwallet->WalletLogPrintf("Importing %s...\n", + EncodeDestination(keyid, config)); + if (!pwallet->AddKeyPubKey(key, pubkey)) { + fGood = false; + continue; + } + pwallet->mapKeyMetadata[keyid].nCreateTime = time; + if (has_label) { + pwallet->SetAddressBook(keyid, label, "receive"); + } + nTimeBegin = std::min(nTimeBegin, time); + progress++; + } + for (const auto &script_pair : scripts) { + uiInterface.ShowProgress( + "", + std::max(50, std::min(75, (progress / total) * 100) + 50), + false); + const CScript &script = script_pair.first; + int64_t time = script_pair.second; + CScriptID id(script); + if (pwallet->HaveCScript(id)) { + pwallet->WalletLogPrintf( + "Skipping import of %s (script already present)\n", + HexStr(script)); + continue; + } + if (!pwallet->AddCScript(script)) { + pwallet->WalletLogPrintf("Error importing script %s\n", + HexStr(script)); + fGood = false; + continue; + } + if (time > 0) { + pwallet->m_script_metadata[id].nCreateTime = time; + nTimeBegin = std::min(nTimeBegin, time); + } + progress++; + } // hide progress dialog in GUI uiInterface.ShowProgress("", 100, false); pwallet->UpdateTimeFirstKey(nTimeBegin); @@ -991,6 +1035,15 @@ const std::string &label = data.exists("label") && !internal ? data["label"].get_str() : ""; + // If private keys are disabled, abort if private keys are being + // imported + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS) && + !keys.isNull()) { + throw JSONRPCError(RPC_WALLET_ERROR, + "Cannot import private keys to a wallet with " + "private keys disabled"); + } + bool isScript = scriptPubKey.getType() == UniValue::VSTR; bool isP2SH = strRedeemScript.length() > 0; const std::string &output = isScript diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -5046,6 +5046,12 @@ "Cannot set a new HD seed while still in Initial Block Download"); } + if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError( + RPC_WALLET_ERROR, + "Cannot set a HD seed to a wallet with private keys disabled"); + } + LOCK2(cs_main, pwallet->cs_wallet); // Do not do anything to non-HD wallets diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -266,10 +266,12 @@ // mapKeyMetadata AssertLockHeld(cs_wallet); + // Make sure we aren't adding private keys to private key disabled wallets + assert(!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); + // CCryptoKeyStore has no concept of wallet databases, but calls - // AddCryptedKey - // which is overridden below. To avoid flushes, the database handle is - // tunneled through to it. + // AddCryptedKey which is overridden below. To avoid flushes, the database + // handle is tunneled through to it. bool needsDB = !encrypted_batch; if (needsDB) { encrypted_batch = &batch; diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -427,6 +427,15 @@ return lambda: self(*args, **kwargs) +def arg_to_cli(arg): + if isinstance(arg, bool): + return str(arg).lower() + elif isinstance(arg, dict) or isinstance(arg, list): + return json.dumps(arg) + else: + return str(arg) + + class TestNodeCLI(): """Interface to bitcoin-cli for an individual node""" @@ -458,14 +467,11 @@ def send_cli(self, command=None, *args, **kwargs): """Run bitcoin-cli command. Deserializes returned string as python object.""" - - pos_args = [str(arg).lower() if type( - arg) is bool else str(arg) for arg in args] - named_args = [str(key) + "=" + str(value) + pos_args = [arg_to_cli(arg) for arg in args] + named_args = [str(key) + "=" + arg_to_cli(value) for (key, value) in kwargs.items()] assert not ( pos_args and named_args), "Cannot use positional arguments and named arguments in the same bitcoin-cli call" - p_args = [self.binary, "-datadir=" + self.datadir] + self.options if named_args: p_args += ["-named"] diff --git a/test/functional/wallet_disableprivatekeys.py b/test/functional/wallet_disableprivatekeys.py --- a/test/functional/wallet_disableprivatekeys.py +++ b/test/functional/wallet_disableprivatekeys.py @@ -7,6 +7,7 @@ from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( + assert_equal, assert_raises_rpc_error, ) @@ -30,6 +31,19 @@ -4, "Error: Private keys are disabled for this wallet", w1.getrawchangeaddress) w1.importpubkey(w2.getaddressinfo(w2.getnewaddress())['pubkey']) + self.log.info('Test that private keys cannot be imported') + addr = w2.getnewaddress('', 'legacy') + privkey = w2.dumpprivkey(addr) + assert_raises_rpc_error( + -4, 'Cannot import private keys to a wallet with private keys disabled', w1.importprivkey, privkey) + result = w1.importmulti( + [{'scriptPubKey': {'address': addr}, 'timestamp': 'now', 'keys': [privkey]}]) + assert(not result[0]['success']) + assert('warning' not in result[0]) + assert_equal(result[0]['error']['code'], -4) + assert_equal(result[0]['error']['message'], + 'Cannot import private keys to a wallet with private keys disabled') + if __name__ == '__main__': DisablePrivateKeysTest().main()