diff options
| -rw-r--r-- | src/wallet/rpcdump.cpp | 102 | ||||
| -rw-r--r-- | src/wallet/rpcwallet.cpp | 4 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 3 | ||||
| -rwxr-xr-x | test/functional/wallet_abandonconflict.py | 43 | ||||
| -rwxr-xr-x | test/functional/wallet_basic.py | 26 | ||||
| -rwxr-xr-x | test/functional/wallet_disableprivatekeys.py | 11 |
6 files changed, 126 insertions, 63 deletions
diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 0daaf94726..32c36ceaeb 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -134,6 +134,9 @@ UniValue importprivkey(const JSONRPCRequest& request) }, }.ToString()); + 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; @@ -587,8 +590,10 @@ UniValue importwallet(const JSONRPCRequest& request) // Use uiInterface.ShowProgress instead of pwallet.ShowProgress because pwallet.ShowProgress has a cancel button tied to AbortRescan which // we don't want for this progress bar showing the import progress. uiInterface.ShowProgress does not have a cancel button. uiInterface.ShowProgress(strprintf("%s " + _("Importing..."), pwallet->GetDisplayName()), 0, false); // show progress dialog in GUI + std::vector<std::tuple<CKey, int64_t, bool, std::string>> keys; + std::vector<std::pair<CScript, int64_t>> scripts; while (file.good()) { - uiInterface.ShowProgress("", std::max(1, std::min(99, (int)(((double)file.tellg() / (double)nFilesize) * 100))), false); + uiInterface.ShowProgress("", std::max(1, std::min(50, (int)(((double)file.tellg() / (double)nFilesize) * 100))), false); std::string line; std::getline(file, line); if (line.empty() || line[0] == '#') @@ -600,13 +605,6 @@ UniValue importwallet(const JSONRPCRequest& request) continue; 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)); - continue; - } int64_t nTime = DecodeDumpTime(vstr[1]); std::string strLabel; bool fLabel = true; @@ -622,36 +620,67 @@ UniValue importwallet(const JSONRPCRequest& request) fLabel = true; } } - pwallet->WalletLogPrintf("Importing %s...\n", EncodeDestination(keyid)); - 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<unsigned char> vData(ParseHex(vstr[0])); - CScript script = CScript(vData.begin(), vData.end()); - CScriptID id(script); - if (pwallet->HaveCScript(id)) { - 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[id].nCreateTime = birth_time; - nTimeBegin = std::min(nTimeBegin, birth_time); - } + std::vector<unsigned char> vData(ParseHex(vstr[0])); + CScript script = CScript(vData.begin(), vData.end()); + int64_t birth_time = DecodeDumpTime(vstr[1]); + scripts.push_back(std::pair<CScript, int64_t>(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)) { + uiInterface.ShowProgress("", 100, false); // hide progress dialog in GUI + 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, (int)((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)); + continue; + } + pwallet->WalletLogPrintf("Importing %s...\n", EncodeDestination(keyid)); + 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, (int)((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++; + } uiInterface.ShowProgress("", 100, false); // hide progress dialog in GUI pwallet->UpdateTimeFirstKey(nTimeBegin); } @@ -958,6 +987,11 @@ static UniValue ProcessImport(CWallet * const pwallet, const UniValue& data, con const bool watchOnly = data.exists("watchonly") ? data["watchonly"].get_bool() : false; const std::string& label = data.exists("label") ? 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"); + } + // Generate the script and destination for the scriptPubKey provided CScript script; CTxDestination dest; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 213765209c..c96a9b0aff 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3872,6 +3872,10 @@ UniValue sethdseed(const JSONRPCRequest& request) throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "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"); + } + auto locked_chain = pwallet->chain().lock(); LOCK(pwallet->cs_wallet); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 3805f46e1a..9b643be69a 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -251,6 +251,9 @@ bool CWallet::AddKeyPubKeyWithDB(WalletBatch &batch, const CKey& secret, const C { AssertLockHeld(cs_wallet); // mapKeyMetadata + // 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. diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 2b684349ce..0c3c247694 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2014-2018 The Bitcoin Core developers +# Copyright (c) 2014-2019 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the abandontransaction RPC. @@ -13,7 +13,15 @@ from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_raises_rpc_error, connect_nodes, disconnect_nodes, sync_blocks, sync_mempools +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, + connect_nodes, + disconnect_nodes, + sync_blocks, + sync_mempools, +) + class AbandonConflictTest(BitcoinTestFramework): def set_test_params(self): @@ -41,21 +49,21 @@ class AbandonConflictTest(BitcoinTestFramework): sync_blocks(self.nodes) newbalance = self.nodes[0].getbalance() - assert(balance - newbalance < Decimal("0.001")) #no more than fees lost + assert balance - newbalance < Decimal("0.001") #no more than fees lost balance = newbalance # Disconnect nodes so node0's transactions don't get into node1's mempool disconnect_nodes(self.nodes[0], 1) # Identify the 10btc outputs - nA = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txA, 1)["vout"]) if vout["value"] == Decimal("10")) - nB = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txB, 1)["vout"]) if vout["value"] == Decimal("10")) - nC = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txC, 1)["vout"]) if vout["value"] == Decimal("10")) + nA = next(tx_out["vout"] for tx_out in self.nodes[0].gettransaction(txA)["details"] if tx_out["amount"] == Decimal("10")) + nB = next(tx_out["vout"] for tx_out in self.nodes[0].gettransaction(txB)["details"] if tx_out["amount"] == Decimal("10")) + nC = next(tx_out["vout"] for tx_out in self.nodes[0].gettransaction(txC)["details"] if tx_out["amount"] == Decimal("10")) - inputs =[] + inputs = [] # spend 10btc outputs from txA and txB - inputs.append({"txid":txA, "vout":nA}) - inputs.append({"txid":txB, "vout":nB}) + inputs.append({"txid": txA, "vout": nA}) + inputs.append({"txid": txB, "vout": nB}) outputs = {} outputs[self.nodes[0].getnewaddress()] = Decimal("14.99998") @@ -64,12 +72,12 @@ class AbandonConflictTest(BitcoinTestFramework): txAB1 = self.nodes[0].sendrawtransaction(signed["hex"]) # Identify the 14.99998btc output - nAB = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txAB1, 1)["vout"]) if vout["value"] == Decimal("14.99998")) + nAB = next(tx_out["vout"] for tx_out in self.nodes[0].gettransaction(txAB1)["details"] if tx_out["amount"] == Decimal("14.99998")) #Create a child tx spending AB1 and C inputs = [] - inputs.append({"txid":txAB1, "vout":nAB}) - inputs.append({"txid":txC, "vout":nC}) + inputs.append({"txid": txAB1, "vout": nAB}) + inputs.append({"txid": txC, "vout": nC}) outputs = {} outputs[self.nodes[0].getnewaddress()] = Decimal("24.9996") signed2 = self.nodes[0].signrawtransactionwithwallet(self.nodes[0].createrawtransaction(inputs, outputs)) @@ -77,8 +85,8 @@ class AbandonConflictTest(BitcoinTestFramework): # Create a child tx spending ABC2 signed3_change = Decimal("24.999") - inputs = [ {"txid":txABC2, "vout":0} ] - outputs = { self.nodes[0].getnewaddress(): signed3_change } + inputs = [{"txid": txABC2, "vout": 0}] + outputs = {self.nodes[0].getnewaddress(): signed3_change} signed3 = self.nodes[0].signrawtransactionwithwallet(self.nodes[0].createrawtransaction(inputs, outputs)) # note tx is never directly referenced, only abandoned as a child of the above self.nodes[0].sendrawtransaction(signed3["hex"]) @@ -106,7 +114,7 @@ class AbandonConflictTest(BitcoinTestFramework): unconfbalance = self.nodes[0].getunconfirmedbalance() + self.nodes[0].getbalance() assert_equal(unconfbalance, newbalance) # Also shouldn't show up in listunspent - assert(not txABC2 in [utxo["txid"] for utxo in self.nodes[0].listunspent(0)]) + assert not txABC2 in [utxo["txid"] for utxo in self.nodes[0].listunspent(0)] balance = newbalance # Abandon original transaction and verify inputs are available again @@ -146,8 +154,8 @@ class AbandonConflictTest(BitcoinTestFramework): # Create a double spend of AB1 by spending again from only A's 10 output # Mine double spend from node 1 - inputs =[] - inputs.append({"txid":txA, "vout":nA}) + inputs = [] + inputs.append({"txid": txA, "vout": nA}) outputs = {} outputs[self.nodes[1].getnewaddress()] = Decimal("9.9999") tx = self.nodes[0].createrawtransaction(inputs, outputs) @@ -173,5 +181,6 @@ class AbandonConflictTest(BitcoinTestFramework): self.log.info("conflicted has not resumed causing its inputs to be seen as spent. See Issue #7315") self.log.info(str(balance) + " -> " + str(newbalance) + " ?") + if __name__ == '__main__': AbandonConflictTest().main() diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index daae2ed3c0..673eb718e6 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2014-2018 The Bitcoin Core developers +# Copyright (c) 2014-2019 The Bitcoin Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the wallet.""" @@ -19,6 +19,7 @@ from test_framework.util import ( wait_until, ) + class WalletTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 4 @@ -196,7 +197,7 @@ class WalletTest(BitcoinTestFramework): txid = self.nodes[2].sendtoaddress(address, 10, "", "", False) self.nodes[2].generate(1) self.sync_all([self.nodes[0:3]]) - node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), Decimal('84'), fee_per_byte, self.get_vsize(self.nodes[2].getrawtransaction(txid))) + node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), Decimal('84'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) assert_equal(self.nodes[0].getbalance(), Decimal('10')) # Send 10 BTC with subtract fee from amount @@ -205,14 +206,14 @@ class WalletTest(BitcoinTestFramework): self.sync_all([self.nodes[0:3]]) node_2_bal -= Decimal('10') assert_equal(self.nodes[2].getbalance(), node_2_bal) - node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), Decimal('20'), fee_per_byte, self.get_vsize(self.nodes[2].getrawtransaction(txid))) + node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), Decimal('20'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) # Sendmany 10 BTC txid = self.nodes[2].sendmany('', {address: 10}, 0, "", []) self.nodes[2].generate(1) self.sync_all([self.nodes[0:3]]) node_0_bal += Decimal('10') - node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), node_2_bal - Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].getrawtransaction(txid))) + node_2_bal = self.check_fee_amount(self.nodes[2].getbalance(), node_2_bal - Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) assert_equal(self.nodes[0].getbalance(), node_0_bal) # Sendmany 10 BTC with subtract fee from amount @@ -221,7 +222,7 @@ class WalletTest(BitcoinTestFramework): self.sync_all([self.nodes[0:3]]) node_2_bal -= Decimal('10') assert_equal(self.nodes[2].getbalance(), node_2_bal) - node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].getrawtransaction(txid))) + node_0_bal = self.check_fee_amount(self.nodes[0].getbalance(), node_0_bal + Decimal('10'), fee_per_byte, self.get_vsize(self.nodes[2].gettransaction(txid)['hex'])) # Test ResendWalletTransactions: # Create a couple of transactions, then start up a fourth @@ -239,7 +240,7 @@ class WalletTest(BitcoinTestFramework): assert_equal(set(relayed), {txid1, txid2}) sync_mempools(self.nodes) - assert(txid1 in self.nodes[3].getrawmempool()) + assert txid1 in self.nodes[3].getrawmempool() # check if we can list zero value tx as available coins # 1. create raw_tx @@ -266,7 +267,7 @@ class WalletTest(BitcoinTestFramework): if uTx['txid'] == zero_value_txid: found = True assert_equal(uTx['amount'], Decimal('0')) - assert(found) + assert found # do some -walletbroadcast tests self.stop_nodes() @@ -343,7 +344,7 @@ class WalletTest(BitcoinTestFramework): self.nodes[1].importaddress(address_to_import) # 3. Validate that the imported address is watch-only on node1 - assert(self.nodes[1].getaddressinfo(address_to_import)["iswatchonly"]) + assert self.nodes[1].getaddressinfo(address_to_import)["iswatchonly"] # 4. Check that the unspents after import are not spendable assert_array_result(self.nodes[1].listunspent(), @@ -385,7 +386,7 @@ class WalletTest(BitcoinTestFramework): addr = self.nodes[0].getnewaddress() self.nodes[0].setlabel(addr, label) assert_equal(self.nodes[0].getaddressinfo(addr)['label'], label) - assert(label in self.nodes[0].listlabels()) + assert label in self.nodes[0].listlabels() self.nodes[0].rpc.ensure_ascii = True # restore to default # maintenance tests @@ -444,8 +445,8 @@ class WalletTest(BitcoinTestFramework): # Without walletrejectlongchains, we will still generate a txid # The tx will be stored in the wallet but not accepted to the mempool extra_txid = self.nodes[0].sendtoaddress(sending_addr, Decimal('0.0001')) - assert(extra_txid not in self.nodes[0].getrawmempool()) - assert(extra_txid in [tx["txid"] for tx in self.nodes[0].listtransactions()]) + assert extra_txid not in self.nodes[0].getrawmempool() + assert extra_txid in [tx["txid"] for tx in self.nodes[0].listtransactions()] self.nodes[0].abandontransaction(extra_txid) total_txs = len(self.nodes[0].listtransactions("*", 99999)) @@ -482,7 +483,7 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].generate(1) destination = self.nodes[1].getnewaddress() txid = self.nodes[0].sendtoaddress(destination, 0.123) - tx = self.nodes[0].decoderawtransaction(self.nodes[0].getrawtransaction(txid)) + tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex']) output_addresses = [vout['scriptPubKey']['addresses'][0] for vout in tx["vout"]] assert len(output_addresses) > 1 for address in output_addresses: @@ -493,5 +494,6 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].setlabel(change, 'foobar') assert_equal(self.nodes[0].getaddressinfo(change)['ischange'], False) + if __name__ == '__main__': WalletTest().main() diff --git a/test/functional/wallet_disableprivatekeys.py b/test/functional/wallet_disableprivatekeys.py index 34ff525255..e55bb82e76 100755 --- 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, ) @@ -31,5 +32,15 @@ class DisablePrivateKeysTest(BitcoinTestFramework): assert_raises_rpc_error(-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() |