diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1142,13 +1142,11 @@ } } -static UniValue ProcessImportLegacy(CWallet *const pwallet, - ImportData &import_data, - std::map &pubkey_map, - std::map &privkey_map, - std::set &script_pub_keys, - bool &have_solving_data, - const UniValue &data) { +static UniValue ProcessImportLegacy( + CWallet *const pwallet, ImportData &import_data, + std::map &pubkey_map, std::map &privkey_map, + std::set &script_pub_keys, bool &have_solving_data, + const UniValue &data, std::vector &ordered_pubkeys) { UniValue warnings(UniValue::VARR); // First ensure scriptPubKey has either a script or JSON with "address" @@ -1232,6 +1230,7 @@ "\" is not a valid public key"); } pubkey_map.emplace(pubkey.GetID(), pubkey); + ordered_pubkeys.push_back(pubkey.GetID()); } for (size_t i = 0; i < keys.size(); ++i) { const auto &str = keys[i].get_str(); @@ -1334,7 +1333,8 @@ std::map &privkey_map, std::set &script_pub_keys, bool &have_solving_data, - const UniValue &data) { + const UniValue &data, + std::vector &ordered_pubkeys) { UniValue warnings(UniValue::VARR); const std::string &descriptor = data["desc"].get_str(); @@ -1375,25 +1375,28 @@ const UniValue &priv_keys = data.exists("keys") ? data["keys"].get_array() : UniValue(); - FlatSigningProvider out_keys; - // Expand all descriptors to get public keys and scripts. // TODO: get private keys from descriptors too for (int i = range_start; i <= range_end; ++i) { + FlatSigningProvider out_keys; std::vector scripts_temp; parsed_desc->Expand(i, keys, scripts_temp, out_keys); std::copy(scripts_temp.begin(), scripts_temp.end(), std::inserter(script_pub_keys, script_pub_keys.end())); - } + for (const auto &key_pair : out_keys.pubkeys) { + ordered_pubkeys.push_back(key_pair.first); + } - for (const auto &x : out_keys.scripts) { - import_data.import_scripts.emplace(x.second); + for (const auto &x : out_keys.scripts) { + import_data.import_scripts.emplace(x.second); + } + + std::copy(out_keys.pubkeys.begin(), out_keys.pubkeys.end(), + std::inserter(pubkey_map, pubkey_map.end())); + import_data.key_origins.insert(out_keys.origins.begin(), + out_keys.origins.end()); } - std::copy(out_keys.pubkeys.begin(), out_keys.pubkeys.end(), - std::inserter(pubkey_map, pubkey_map.end())); - import_data.key_origins.insert(out_keys.origins.begin(), - out_keys.origins.end()); for (size_t i = 0; i < priv_keys.size(); ++i) { const auto &str = priv_keys[i].get_str(); CKey key = DecodeSecret(str); @@ -1454,11 +1457,22 @@ } const std::string &label = data.exists("label") ? data["label"].get_str() : ""; + const bool add_keypool = + data.exists("keypool") ? data["keypool"].get_bool() : false; + + // Add to keypool only works with privkeys disabled + if (add_keypool && + !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + "Keys can only be imported to the keypool when " + "private keys are disabled"); + } ImportData import_data; std::map pubkey_map; std::map privkey_map; std::set script_pub_keys; + std::vector ordered_pubkeys; bool have_solving_data; if (data.exists("scriptPubKey") && data.exists("desc")) { @@ -1466,13 +1480,13 @@ RPC_INVALID_PARAMETER, "Both a descriptor and a scriptPubKey should not be provided."); } else if (data.exists("scriptPubKey")) { - warnings = ProcessImportLegacy(pwallet, import_data, pubkey_map, - privkey_map, script_pub_keys, - have_solving_data, data); + warnings = ProcessImportLegacy( + pwallet, import_data, pubkey_map, privkey_map, script_pub_keys, + have_solving_data, data, ordered_pubkeys); } else if (data.exists("desc")) { - warnings = ProcessImportDescriptor(import_data, pubkey_map, - privkey_map, script_pub_keys, - have_solving_data, data); + warnings = ProcessImportDescriptor( + import_data, pubkey_map, privkey_map, script_pub_keys, + have_solving_data, data, ordered_pubkeys); } else { throw JSONRPCError( RPC_INVALID_PARAMETER, @@ -1501,7 +1515,6 @@ // All good, time to import pwallet->MarkDirty(); - for (const auto &entry : import_data.import_scripts) { if (!pwallet->HaveCScript(CScriptID(entry)) && !pwallet->AddCScript(entry)) { @@ -1522,9 +1535,12 @@ } pwallet->UpdateTimeFirstKey(timestamp); } - for (const auto &entry : pubkey_map) { - const CPubKey &pubkey = entry.second; - const CKeyID &id = entry.first; + for (const CKeyID &id : ordered_pubkeys) { + auto entry = pubkey_map.find(id); + if (entry == pubkey_map.end()) { + continue; + } + const CPubKey &pubkey = entry->second; CPubKey temp; if (!pwallet->GetPubKey(id, temp) && !pwallet->AddWatchOnly(GetScriptForRawPubKey(pubkey), @@ -1537,6 +1553,11 @@ pwallet->AddKeyOrigin(pubkey, key_orig_it->second); } pwallet->mapKeyMetadata[id].nCreateTime = timestamp; + + // Add to keypool only works with pubkeys + if (add_keypool) { + pwallet->AddKeypoolPubkey(pubkey, internal); + } } for (const CScript &script : script_pub_keys) { diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -178,12 +178,6 @@ .ToString()); } - // Belt and suspenders check for disabled private keys - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { - throw JSONRPCError(RPC_WALLET_ERROR, - "Error: Private keys are disabled for this wallet"); - } - LOCK(pwallet->cs_wallet); if (!pwallet->CanGetAddresses()) { @@ -247,12 +241,6 @@ .ToString()); } - // Belt and suspenders check for disabled private keys - if (pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { - throw JSONRPCError(RPC_WALLET_ERROR, - "Error: Private keys are disabled for this wallet"); - } - LOCK(pwallet->cs_wallet); if (!pwallet->CanGetAddresses(true)) { @@ -2960,7 +2948,7 @@ obj.pushKV("keypoololdest", pwallet->GetOldestKeyPoolTime()); obj.pushKV("keypoolsize", (int64_t)kpExternalSize); CKeyID seed_id = pwallet->GetHDChain().seed_id; - if (!seed_id.IsNull() && pwallet->CanSupportFeature(FEATURE_HD_SPLIT)) { + if (pwallet->CanSupportFeature(FEATURE_HD_SPLIT)) { obj.pushKV("keypoolsize_hd_internal", int64_t(pwallet->GetKeyPoolSize() - kpExternalSize)); } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1176,6 +1176,9 @@ bool NewKeyPool(); size_t KeypoolCountExternalKeys() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); bool TopUpKeyPool(unsigned int kpSize = 0); + void AddKeypoolPubkey(const CPubKey &pubkey, const bool internal); + void AddKeypoolPubkeyWithDB(const CPubKey &pubkey, const bool internal, + WalletBatch &batch); /** * Reserves a key from the keypool and sets nIndex to its index diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3059,10 +3059,10 @@ // with keys of ours to recover post-backup change. // Reserve a new key pair from key pool - if (IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { + if (!CanGetAddresses(true)) { strFailReason = - _("Can't generate a change-address key. Private keys " - "are disabled for this wallet.") + _("Can't generate a change-address key. No keys in the " + "internal keypool and can't generate any keys.") .translated; return false; } @@ -3687,22 +3687,8 @@ internal = true; } - // How in the hell did you use so many keys? - assert(m_max_keypool_index < std::numeric_limits::max()); - int64_t index = ++m_max_keypool_index; - CPubKey pubkey(GenerateNewKey(batch, internal)); - if (!batch.WritePool(index, CKeyPool(pubkey, internal))) { - throw std::runtime_error(std::string(__func__) + - ": writing generated key failed"); - } - - if (internal) { - setInternalKeyPool.insert(index); - } else { - setExternalKeyPool.insert(index); - } - m_pool_key_to_index[pubkey.GetID()] = index; + AddKeypoolPubkeyWithDB(pubkey, internal, batch); } if (missingInternal + missingExternal > 0) { WalletLogPrintf( @@ -3717,6 +3703,30 @@ return true; } +void CWallet::AddKeypoolPubkey(const CPubKey &pubkey, const bool internal) { + WalletBatch batch(*database); + AddKeypoolPubkeyWithDB(pubkey, internal, batch); + NotifyCanGetAddressesChanged(); +} + +void CWallet::AddKeypoolPubkeyWithDB(const CPubKey &pubkey, const bool internal, + WalletBatch &batch) { + LOCK(cs_wallet); + // How in the hell did you use so many keys? + assert(m_max_keypool_index < std::numeric_limits::max()); + int64_t index = ++m_max_keypool_index; + if (!batch.WritePool(index, CKeyPool(pubkey, internal))) { + throw std::runtime_error(std::string(__func__) + + ": writing imported pubkey failed"); + } + if (internal) { + setInternalKeyPool.insert(index); + } else { + setExternalKeyPool.insert(index); + } + m_pool_key_to_index[pubkey.GetID()] = index; +} + bool CWallet::ReserveKeyFromKeyPool(int64_t &nIndex, CKeyPool &keypool, bool fRequestedInternal) { nIndex = -1; @@ -3728,9 +3738,10 @@ TopUpKeyPool(); } - bool fReturningInternal = IsHDEnabled() && - CanSupportFeature(FEATURE_HD_SPLIT) && - fRequestedInternal; + bool fReturningInternal = fRequestedInternal; + fReturningInternal &= + (IsHDEnabled() && CanSupportFeature(FEATURE_HD_SPLIT)) || + IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS); bool use_split_keypool = set_pre_split_keypool.empty(); std::set &setKeyPool = use_split_keypool @@ -3750,7 +3761,8 @@ if (!batch.ReadPool(nIndex, keypool)) { throw std::runtime_error(std::string(__func__) + ": read failed"); } - if (!HaveKey(keypool.vchPubKey.GetID())) { + CPubKey pk; + if (!GetPubKey(keypool.vchPubKey.GetID(), pk)) { throw std::runtime_error(std::string(__func__) + ": unknown key in key pool"); } @@ -3804,7 +3816,8 @@ CKeyPool keypool; LOCK(cs_wallet); int64_t nIndex; - if (!ReserveKeyFromKeyPool(nIndex, keypool, internal)) { + if (!ReserveKeyFromKeyPool(nIndex, keypool, internal) && + !IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)) { if (IsLocked()) { return false; } diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py --- a/test/functional/wallet_createwallet.py +++ b/test/functional/wallet_createwallet.py @@ -33,10 +33,10 @@ self.log.info("Test disableprivatekeys creation.") self.nodes[0].createwallet(wallet_name='w1', disable_private_keys=True) w1 = node.get_wallet_rpc('w1') - assert_raises_rpc_error( - -4, "Error: Private keys are disabled for this wallet", w1.getnewaddress) - assert_raises_rpc_error( - -4, "Error: Private keys are disabled for this wallet", w1.getrawchangeaddress) + assert_raises_rpc_error(-4, + "Error: This wallet has no available keys", w1.getnewaddress) + assert_raises_rpc_error(-4, "Error: This wallet has no available keys", + w1.getrawchangeaddress) w1.importpubkey(w0.getaddressinfo(address1)['pubkey']) self.log.info('Test that private keys cannot be imported') @@ -56,10 +56,10 @@ self.nodes[0].createwallet( wallet_name='w2', disable_private_keys=True, blank=True) w2 = node.get_wallet_rpc('w2') - assert_raises_rpc_error( - -4, "Error: Private keys are disabled for this wallet", w2.getnewaddress) - assert_raises_rpc_error( - -4, "Error: Private keys are disabled for this wallet", w2.getrawchangeaddress) + assert_raises_rpc_error(-4, + "Error: This wallet has no available keys", w2.getnewaddress) + assert_raises_rpc_error(-4, "Error: This wallet has no available keys", + w2.getrawchangeaddress) w2.importpubkey(w0.getaddressinfo(address1)['pubkey']) self.log.info("Test blank creation with private keys enabled.") @@ -112,16 +112,16 @@ w5 = node.get_wallet_rpc('w5') assert_equal(w5.getwalletinfo()['keypoolsize'], 0) - assert_raises_rpc_error( - -4, "Error: Private keys are disabled for this wallet", w5.getnewaddress) - assert_raises_rpc_error( - -4, "Error: Private keys are disabled for this wallet", w5.getrawchangeaddress) + assert_raises_rpc_error(-4, + "Error: This wallet has no available keys", w5.getnewaddress) + assert_raises_rpc_error(-4, "Error: This wallet has no available keys", + w5.getrawchangeaddress) # Encrypt the wallet w5.encryptwallet('pass') - assert_raises_rpc_error( - -4, "Error: Private keys are disabled for this wallet", w5.getnewaddress) - assert_raises_rpc_error( - -4, "Error: Private keys are disabled for this wallet", w5.getrawchangeaddress) + assert_raises_rpc_error(-4, + "Error: This wallet has no available keys", w5.getnewaddress) + assert_raises_rpc_error(-4, "Error: This wallet has no available keys", + w5.getrawchangeaddress) if __name__ == '__main__': diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -580,6 +580,127 @@ assert 'hdmasterfingerprint' not in pub_import_info assert 'hdkeypath' not in pub_import_info + # Import some public keys to the keypool of a no privkey wallet + self.log.info("Adding pubkey to keypool of disableprivkey wallet") + self.nodes[1].createwallet( + wallet_name="noprivkeys", + disable_private_keys=True) + wrpc = self.nodes[1].get_wallet_rpc("noprivkeys") + + addr1 = self.nodes[0].getnewaddress() + addr2 = self.nodes[0].getnewaddress() + pub1 = self.nodes[0].getaddressinfo(addr1)['pubkey'] + pub2 = self.nodes[0].getaddressinfo(addr2)['pubkey'] + result = wrpc.importmulti( + [{ + 'desc': 'pkh(' + pub1 + ')', + 'keypool': True, + "timestamp": "now", + }, + { + 'desc': 'pkh(' + pub2 + ')', + 'keypool': True, + "timestamp": "now", + }] + ) + assert result[0]['success'] + assert result[1]['success'] + assert_equal(wrpc.getwalletinfo()["keypoolsize"], 2) + newaddr1 = wrpc.getnewaddress() + assert_equal(addr1, newaddr1) + newaddr2 = wrpc.getnewaddress() + assert_equal(addr2, newaddr2) + + # Import some public keys to the internal keypool of a no privkey + # wallet + self.log.info( + "Adding pubkey to internal keypool of disableprivkey wallet") + addr1 = self.nodes[0].getnewaddress() + addr2 = self.nodes[0].getnewaddress() + pub1 = self.nodes[0].getaddressinfo(addr1)['pubkey'] + pub2 = self.nodes[0].getaddressinfo(addr2)['pubkey'] + result = wrpc.importmulti( + [{ + 'desc': 'pkh(' + pub1 + ')', + 'keypool': True, + 'internal': True, + "timestamp": "now", + }, + { + 'desc': 'pkh(' + pub2 + ')', + 'keypool': True, + 'internal': True, + "timestamp": "now", + }] + ) + assert result[0]['success'] + assert result[1]['success'] + assert_equal(wrpc.getwalletinfo()["keypoolsize_hd_internal"], 2) + newaddr1 = wrpc.getrawchangeaddress() + assert_equal(addr1, newaddr1) + newaddr2 = wrpc.getrawchangeaddress() + assert_equal(addr2, newaddr2) + + # Import a multisig and make sure the keys don't go into the keypool + self.log.info( + 'Imported scripts with pubkeys shoud not have their pubkeys go into the keypool') + addr1 = self.nodes[0].getnewaddress() + addr2 = self.nodes[0].getnewaddress() + pub1 = self.nodes[0].getaddressinfo(addr1)['pubkey'] + pub2 = self.nodes[0].getaddressinfo(addr2)['pubkey'] + result = wrpc.importmulti( + [{ + 'desc': 'sh(multi(2,' + pub1 + ',' + pub2 + '))', + 'keypool': True, + "timestamp": "now", + }] + ) + assert result[0]['success'] + assert_equal(wrpc.getwalletinfo()["keypoolsize"], 0) + + # Cannot import those pubkeys to keypool of wallet with privkeys + self.log.info( + "Pubkeys cannot be added to the keypool of a wallet with private keys") + wrpc = self.nodes[1].get_wallet_rpc("") + assert wrpc.getwalletinfo()['private_keys_enabled'] + result = wrpc.importmulti( + [{ + 'desc': 'pkh(' + pub1 + ')', + 'keypool': True, + "timestamp": "now", + }] + ) + assert_equal(result[0]['error']['code'], -8) + assert_equal( + result[0]['error']['message'], + "Keys can only be imported to the keypool when private keys are disabled") + + # Make sure ranged imports import keys in order + self.log.info('Key ranges should be imported in order') + wrpc = self.nodes[1].get_wallet_rpc("noprivkeys") + assert_equal(wrpc.getwalletinfo()["keypoolsize"], 0) + assert_equal(wrpc.getwalletinfo()["private_keys_enabled"], False) + xpub = "tpubDAXcJ7s7ZwicqjprRaEWdPoHKrCS215qxGYxpusRLLmJuT69ZSicuGdSfyvyKpvUNYBW1s2U3NSrT6vrCYB9e6nZUEvrqnwXPF8ArTCRXMY" + addresses = [ + 'bchreg:qp0v86h53rc92hjrlpwzpjtdlgzsxu25svryj39hul', # m/0'/0'/0 + 'bchreg:qqasy0zlkdleqt4pkn8fs4ehm5gnnz6qpgzxm0035q', # m/0'/0'/1 + 'bchreg:qp0sp4wlhctvprqvdt2dgvqcfdjssu04xgk64mmwew', # m/0'/0'/2 + 'bchreg:qrhn24tegn04cptfv4ldhtkduxq55zcwryhvnfcm3r', # m/0'/0'/3 + 'bchreg:qzpqhett2uwltq803vrxv7zkqhft5vsnmca8ds9jjp', # m/0'/0'/4 + ] + result = wrpc.importmulti( + [{ + 'desc': 'pkh([80002067/0h/0h]' + xpub + '/*)', + 'keypool': True, + 'timestamp': 'now', + 'range': {'start': 0, 'end': 4} + }] + ) + self.log.info(result) + for i in range(0, 5): + addr = wrpc.getnewaddress('') + assert_equal(addr, addresses[i]) + if __name__ == '__main__': ImportMultiTest().main()