diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -4661,12 +4661,6 @@ LegacyScriptPubKeyMan &spk_man = EnsureLegacyScriptPubKeyMan(*pwallet, true); - if (pwallet->chain().isInitialBlockDownload()) { - 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, diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -18,6 +18,8 @@ #include +#include + enum class OutputType; class CChainParams; struct bilingual_str; @@ -156,6 +158,13 @@ } }; +class KeyIDHasher { +public: + KeyIDHasher() {} + + size_t operator()(const CKeyID &id) const { return id.GetUint64(0); } +}; + /** * A class implementing ScriptPubKeyMan manages some (or all) scriptPubKeys used * in a wallet. It contains the scripts and keys related to the scriptPubKeys it @@ -369,11 +378,13 @@ const KeyOriginInfo &info); /* the HD chain data model (external chain counters) */ - CHDChain hdChain; + CHDChain m_hd_chain; + std::unordered_map m_inactive_hd_chains; /* HD derive new child key (on internal or external chain) */ void DeriveNewChildKey(WalletBatch &batch, CKeyMetadata &metadata, - CKey &secret, bool internal = false) + CKey &secret, CHDChain &hd_chain, + bool internal = false) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore); std::set setInternalKeyPool GUARDED_BY(cs_KeyStore); @@ -406,6 +417,22 @@ bool ReserveKeyFromKeyPool(int64_t &nIndex, CKeyPool &keypool, bool fRequestedInternal); + /** + * Like TopUp() but adds keys for inactive HD chains. + * Ensures that there are at least -keypool number of keys derived after the + * given index. + * + * @param seed_id the CKeyID for the HD seed. + * @param index the index to start generating keys from + * @param internal whether the internal chain should be used. true for + * internal chain, false for external chain. + * + * @return true if seed was found and keys were derived. false if unable to + * derive seeds + */ + bool TopUpInactiveHDChain(const CKeyID seed_id, int64_t index, + bool internal); + public: using ScriptPubKeyMan::ScriptPubKeyMan; @@ -501,12 +528,14 @@ void LoadScriptMetadata(const CScriptID &script_id, const CKeyMetadata &metadata); //! Generate a new key - CPubKey GenerateNewKey(WalletBatch &batch, bool internal = false) + CPubKey GenerateNewKey(WalletBatch &batch, CHDChain &hd_chain, + bool internal = false) EXCLUSIVE_LOCKS_REQUIRED(cs_KeyStore); /* Set the HD chain model (chain child index counters) */ void SetHDChain(const CHDChain &chain, bool memonly); - const CHDChain &GetHDChain() const { return hdChain; } + const CHDChain &GetHDChain() const { return m_hd_chain; } + void AddInactiveHDChain(const CHDChain &chain); //! Adds a watch-only address to the store, without saving it to disk (used //! by LoadWallet) diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -14,6 +14,10 @@ #include #include +//! Value for the first BIP 32 hardened derivation. Can be used as a bit mask +//! and as a value. See BIP 32 for more details. +const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; + bool LegacyScriptPubKeyMan::GetNewDestination(const OutputType type, CTxDestination &dest, std::string &error) { @@ -254,6 +258,51 @@ return true; } +bool LegacyScriptPubKeyMan::TopUpInactiveHDChain(const CKeyID seed_id, + int64_t index, bool internal) { + LOCK(cs_KeyStore); + + if (m_storage.IsLocked()) { + return false; + } + + auto it = m_inactive_hd_chains.find(seed_id); + if (it == m_inactive_hd_chains.end()) { + return false; + } + + CHDChain &chain = it->second; + + // Top up key pool + int64_t target_size = + std::max(gArgs.GetArg("-keypool", DEFAULT_KEYPOOL_SIZE), (int64_t)1); + + // "size" of the keypools. Not really the size, actually the difference + // between index and the chain counter Since chain counter is 1 based and + // index is 0 based, one of them needs to be offset by 1. + int64_t kp_size = + (internal ? chain.nInternalChainCounter : chain.nExternalChainCounter) - + (index + 1); + + // make sure the keypool fits the user-selected target (-keypool) + int64_t missing = std::max(target_size - kp_size, (int64_t)0); + + if (missing > 0) { + WalletBatch batch(m_storage.GetDatabase()); + for (int64_t i = missing; i > 0; --i) { + GenerateNewKey(batch, chain, internal); + } + if (internal) { + WalletLogPrintf("inactive seed with id %s added %d internal keys\n", + HexStr(seed_id), missing); + } else { + WalletLogPrintf("inactive seed with id %s added %d keys\n", + HexStr(seed_id), missing); + } + } + return true; +} + void LegacyScriptPubKeyMan::MarkUnusedAddresses(const CScript &script) { LOCK(cs_KeyStore); // extract addresses and check if they match with an unused keypool key @@ -262,7 +311,7 @@ m_pool_key_to_index.find(keyid); if (mi != m_pool_key_to_index.end()) { WalletLogPrintf("%s: Detected a used keypool key, mark all keypool " - "key up to this key as used\n", + "keys up to this key as used\n", __func__); MarkReserveKeysAsUsed(mi->second); @@ -272,6 +321,26 @@ __func__); } } + + // Find the key's metadata and check if it's seed id (if it has one) is + // inactive, i.e. it is not the current m_hd_chain seed id. If so, TopUp + // the inactive hd chain + auto it = mapKeyMetadata.find(keyid); + if (it != mapKeyMetadata.end()) { + CKeyMetadata meta = it->second; + if (!meta.hd_seed_id.IsNull() && + meta.hd_seed_id != m_hd_chain.seed_id) { + bool internal = + (meta.key_origin.path[1] & ~BIP32_HARDENED_KEY_LIMIT) != 0; + int64_t index = + meta.key_origin.path[2] & ~BIP32_HARDENED_KEY_LIMIT; + + if (!TopUpInactiveHDChain(meta.hd_seed_id, index, internal)) { + WalletLogPrintf("%s: Adding inactive seed keys failed\n", + __func__); + } + } + } } } @@ -327,7 +396,7 @@ } bool LegacyScriptPubKeyMan::IsHDEnabled() const { - return !hdChain.seed_id.IsNull(); + return !m_hd_chain.seed_id.IsNull(); } bool LegacyScriptPubKeyMan::CanGetAddresses(bool internal) const { @@ -811,12 +880,28 @@ void LegacyScriptPubKeyMan::SetHDChain(const CHDChain &chain, bool memonly) { LOCK(cs_KeyStore); - if (!memonly && !WalletBatch(m_storage.GetDatabase()).WriteHDChain(chain)) { - throw std::runtime_error(std::string(__func__) + - ": writing chain failed"); + // memonly == true means we are loading the wallet file + // memonly == false means that the chain is actually being changed + if (!memonly) { + // Store the new chain + if (!WalletBatch(m_storage.GetDatabase()).WriteHDChain(chain)) { + throw std::runtime_error(std::string(__func__) + + ": writing chain failed"); + } + // When there's an old chain, add it as an inactive chain as we are now + // rotating hd chains + if (!m_hd_chain.seed_id.IsNull()) { + AddInactiveHDChain(m_hd_chain); + } } - hdChain = chain; + m_hd_chain = chain; +} + +void LegacyScriptPubKeyMan::AddInactiveHDChain(const CHDChain &chain) { + LOCK(cs_KeyStore); + assert(!chain.seed_id.IsNull()); + m_inactive_hd_chains[chain.seed_id] = chain; } bool LegacyScriptPubKeyMan::HaveKey(const CKeyID &address) const { @@ -896,6 +981,7 @@ } CPubKey LegacyScriptPubKeyMan::GenerateNewKey(WalletBatch &batch, + CHDChain &hd_chain, bool internal) { assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); assert(!m_storage.IsWalletFlagSet(WALLET_FLAG_BLANK_WALLET)); @@ -913,7 +999,7 @@ // is present if (IsHDEnabled()) { DeriveNewChildKey( - batch, metadata, secret, + batch, metadata, secret, hd_chain, (m_storage.CanSupportFeature(FEATURE_HD_SPLIT) ? internal : false)); } else { secret.MakeNewKey(fCompressed); @@ -937,11 +1023,10 @@ return pubkey; } -const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; - void LegacyScriptPubKeyMan::DeriveNewChildKey(WalletBatch &batch, CKeyMetadata &metadata, - CKey &secret, bool internal) { + CKey &secret, CHDChain &hd_chain, + bool internal) { // for now we use a fixed keypath scheme of m/0'/0'/k // seed (256bit) CKey seed; @@ -955,7 +1040,7 @@ CExtKey childKey; // try to get the seed - if (!GetKey(hdChain.seed_id, seed)) { + if (!GetKey(hd_chain.seed_id, seed)) { throw std::runtime_error(std::string(__func__) + ": seed not found"); } @@ -978,37 +1063,38 @@ // child-index-range // example: 1 | BIP32_HARDENED_KEY_LIMIT == 0x80000001 == 2147483649 if (internal) { - chainChildKey.Derive(childKey, hdChain.nInternalChainCounter | + chainChildKey.Derive(childKey, hd_chain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT); metadata.hdKeypath = - "m/0'/1'/" + ToString(hdChain.nInternalChainCounter) + "'"; + "m/0'/1'/" + ToString(hd_chain.nInternalChainCounter) + "'"; metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(1 | BIP32_HARDENED_KEY_LIMIT); - metadata.key_origin.path.push_back(hdChain.nInternalChainCounter | + metadata.key_origin.path.push_back(hd_chain.nInternalChainCounter | BIP32_HARDENED_KEY_LIMIT); - hdChain.nInternalChainCounter++; + hd_chain.nInternalChainCounter++; } else { - chainChildKey.Derive(childKey, hdChain.nExternalChainCounter | + chainChildKey.Derive(childKey, hd_chain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT); metadata.hdKeypath = - "m/0'/0'/" + ToString(hdChain.nExternalChainCounter) + "'"; + "m/0'/0'/" + ToString(hd_chain.nExternalChainCounter) + "'"; metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); metadata.key_origin.path.push_back(0 | BIP32_HARDENED_KEY_LIMIT); - metadata.key_origin.path.push_back(hdChain.nExternalChainCounter | + metadata.key_origin.path.push_back(hd_chain.nExternalChainCounter | BIP32_HARDENED_KEY_LIMIT); - hdChain.nExternalChainCounter++; + hd_chain.nExternalChainCounter++; } } while (HaveKey(childKey.key.GetPubKey().GetID())); secret = childKey.key; - metadata.hd_seed_id = hdChain.seed_id; + metadata.hd_seed_id = hd_chain.seed_id; CKeyID master_id = masterKey.key.GetPubKey().GetID(); std::copy(master_id.begin(), master_id.begin() + 4, metadata.key_origin.fingerprint); metadata.has_key_origin = true; // update the chain model in the database - if (!batch.WriteHDChain(hdChain)) { + if (hd_chain.seed_id == m_hd_chain.seed_id && + !batch.WriteHDChain(hd_chain)) { throw std::runtime_error(std::string(__func__) + - ": Writing HD chain model failed"); + ": writing HD chain model failed"); } } @@ -1165,7 +1251,7 @@ internal = true; } - CPubKey pubkey(GenerateNewKey(batch, internal)); + CPubKey pubkey(GenerateNewKey(batch, m_hd_chain, internal)); AddKeypoolPubkeyWithDB(pubkey, internal, batch); } if (missingInternal + missingExternal > 0) { @@ -1250,7 +1336,7 @@ return false; } WalletBatch batch(m_storage.GetDatabase()); - result = GenerateNewKey(batch, internal); + result = GenerateNewKey(batch, m_hd_chain, internal); return true; } diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -118,6 +118,10 @@ nInternalChainCounter = 0; seed_id.SetNull(); } + + bool operator==(const CHDChain &chain) const { + return seed_id == chain.seed_id; + } }; class CKeyMetadata { diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -286,6 +287,7 @@ std::map, std::pair>> m_descriptor_crypt_keys; + std::map m_hd_chains; CWalletScanState() {} }; @@ -476,6 +478,83 @@ wss.nKeyMeta++; pwallet->GetOrCreateLegacyScriptPubKeyMan()->LoadKeyMetadata( vchPubKey.GetID(), keyMeta); + + // Extract some CHDChain info from this metadata if it has any + if (keyMeta.nVersion >= CKeyMetadata::VERSION_WITH_HDDATA && + !keyMeta.hd_seed_id.IsNull() && keyMeta.hdKeypath.size() > 0) { + // Get the path from the key origin or from the path string + // Not applicable when path is "s" as that indicates a seed + bool internal = false; + uint32_t index = 0; + if (keyMeta.hdKeypath != "s") { + std::vector path; + if (keyMeta.has_key_origin) { + // We have a key origin, so pull it from its path vector + path = keyMeta.key_origin.path; + } else { + // No key origin, have to parse the string + if (!ParseHDKeypath(keyMeta.hdKeypath, path)) { + strErr = "Error reading wallet database: keymeta " + "with invalid HD keypath"; + return false; + } + } + + // Extract the index and internal from the path + // Path string is m/0'/k'/i' + // Path vector is [0', k', i'] (but as ints OR'd with the + // hardened bit k == 0 for external, 1 for internal. i is + // the index + if (path.size() != 3) { + strErr = "Error reading wallet database: keymeta found " + "with unexpected path"; + return false; + } + if (path[0] != 0x80000000) { + strErr = strprintf( + "Unexpected path index of 0x%08x (expected " + "0x80000000) for the element at index 0", + path[0]); + return false; + } + if (path[1] != 0x80000000 && path[1] != (1 | 0x80000000)) { + strErr = + strprintf("Unexpected path index of 0x%08x " + "(expected 0x80000000 or 0x80000001) for " + "the element at index 1", + path[1]); + return false; + } + if ((path[2] & 0x80000000) == 0) { + strErr = strprintf( + "Unexpected path index of 0x%08x (expected to be " + "greater than or equal to 0x80000000)", + path[2]); + return false; + } + internal = path[1] == (1 | 0x80000000); + index = path[2] & ~0x80000000; + } + + // Insert a new CHDChain, or get the one that already exists + auto ins = + wss.m_hd_chains.emplace(keyMeta.hd_seed_id, CHDChain()); + CHDChain &chain = ins.first->second; + if (ins.second) { + // For new chains, we want to default to VERSION_HD_BASE + // until we see an internal + chain.nVersion = CHDChain::VERSION_HD_BASE; + chain.seed_id = keyMeta.hd_seed_id; + } + if (internal) { + chain.nVersion = CHDChain::VERSION_HD_CHAIN_SPLIT; + chain.nInternalChainCounter = + std::max(chain.nInternalChainCounter, index); + } else { + chain.nExternalChainCounter = + std::max(chain.nExternalChainCounter, index); + } + } } else if (strType == DBKeys::WATCHMETA) { CScript script; ssKey >> script; @@ -842,6 +921,24 @@ result = DBErrors::CORRUPT; } + // Set the inactive chain + if (wss.m_hd_chains.size() > 0) { + LegacyScriptPubKeyMan *legacy_spkm = + pwallet->GetLegacyScriptPubKeyMan(); + if (!legacy_spkm) { + pwallet->WalletLogPrintf( + "Inactive HD Chains found but no Legacy ScriptPubKeyMan\n"); + return DBErrors::CORRUPT; + } + for (const auto &chain_pair : wss.m_hd_chains) { + if (chain_pair.first != + pwallet->GetLegacyScriptPubKeyMan()->GetHDChain().seed_id) { + pwallet->GetLegacyScriptPubKeyMan()->AddInactiveHDChain( + chain_pair.second); + } + } + } + return result; } diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -153,7 +153,8 @@ # Make sure the new address is the first from the keypool assert_equal(self.nodes[1].getaddressinfo( addr)['hdkeypath'], 'm/0\'/0\'/0\'') - self.nodes[1].keypoolrefill(1) # Fill keypool with 1 key + # Fill keypool with 1 key + self.nodes[1].keypoolrefill(1) # Set a new HD seed on node 1 without flushing the keypool new_seed = self.nodes[0].dumpprivkey(self.nodes[0].getnewaddress()) @@ -192,6 +193,127 @@ assert_raises_rpc_error(-5, "Already have this key", self.nodes[1].sethdseed, False, self.nodes[1].dumpprivkey(self.nodes[1].getnewaddress())) + self.log.info( + 'Test sethdseed restoring with keys outside of the initial keypool') + self.nodes[0].generate(10) + # Restart node 1 with keypool of 3 and a different wallet + self.nodes[1].createwallet(wallet_name='origin', blank=True) + self.stop_node(1) + self.start_node(1, extra_args=['-keypool=3', '-wallet=origin']) + connect_nodes(self.nodes[0], self.nodes[1]) + + # sethdseed restoring and seeing txs to addresses out of the + # keypool + origin_rpc = self.nodes[1].get_wallet_rpc('origin') + seed = self.nodes[0].dumpprivkey(self.nodes[0].getnewaddress()) + origin_rpc.sethdseed(True, seed) + + self.nodes[1].createwallet(wallet_name='restore', blank=True) + restore_rpc = self.nodes[1].get_wallet_rpc('restore') + # Set to be the same seed as origin_rpc + restore_rpc.sethdseed(True, seed) + # Rotate to a new seed, making original `seed` inactive + restore_rpc.sethdseed(True) + + self.nodes[1].createwallet(wallet_name='restore2', blank=True) + restore2_rpc = self.nodes[1].get_wallet_rpc('restore2') + # Set to be the same seed as origin_rpc + restore2_rpc.sethdseed(True, seed) + # Rotate to a new seed, making original `seed` inactive + restore2_rpc.sethdseed(True) + + # Check persistence of inactive seed by reloading restore. restore2 + # is still loaded to test the case where the wallet is not reloaded + restore_rpc.unloadwallet() + self.nodes[1].loadwallet('restore') + restore_rpc = self.nodes[1].get_wallet_rpc('restore') + + # Empty origin keypool and get an address that is beyond the + # initial keypool + origin_rpc.getnewaddress() + origin_rpc.getnewaddress() + # Last address of initial keypool + last_addr = origin_rpc.getnewaddress() + # First address beyond initial keypool + addr = origin_rpc.getnewaddress() + + # Check that the restored seed has last_addr but does not have addr + info = restore_rpc.getaddressinfo(last_addr) + assert_equal(info['ismine'], True) + info = restore_rpc.getaddressinfo(addr) + assert_equal(info['ismine'], False) + info = restore2_rpc.getaddressinfo(last_addr) + assert_equal(info['ismine'], True) + info = restore2_rpc.getaddressinfo(addr) + assert_equal(info['ismine'], False) + # Check that the origin seed has addr + info = origin_rpc.getaddressinfo(addr) + assert_equal(info['ismine'], True) + + # Send a transaction to addr, which is out of the initial keypool. + # The wallet that has set a new seed (restore_rpc) should not + # detect this transaction. + txid = self.nodes[0].sendtoaddress(addr, 1) + origin_rpc.sendrawtransaction( + self.nodes[0].gettransaction(txid)['hex']) + self.nodes[0].generate(1) + origin_rpc.gettransaction(txid) + assert_raises_rpc_error(-5, + 'Invalid or non-wallet transaction id', + restore_rpc.gettransaction, + txid) + out_of_kp_txid = txid + + # Send a transaction to last_addr, which is in the initial keypool. + # The wallet that has set a new seed (restore_rpc) should detect this + # transaction and generate 3 new keys from the initial seed. + # The previous transaction (out_of_kp_txid) should still not be + # detected as a rescan is required. + txid = self.nodes[0].sendtoaddress(last_addr, 1) + origin_rpc.sendrawtransaction( + self.nodes[0].gettransaction(txid)['hex']) + self.nodes[0].generate(1) + origin_rpc.gettransaction(txid) + restore_rpc.gettransaction(txid) + assert_raises_rpc_error(-5, + 'Invalid or non-wallet transaction id', + restore_rpc.gettransaction, + out_of_kp_txid) + restore2_rpc.gettransaction(txid) + assert_raises_rpc_error(-5, + 'Invalid or non-wallet transaction id', + restore2_rpc.gettransaction, + out_of_kp_txid) + + # After rescanning, restore_rpc should now see out_of_kp_txid and generate an additional key. + # addr should now be part of restore_rpc and be ismine + restore_rpc.rescanblockchain() + restore_rpc.gettransaction(out_of_kp_txid) + info = restore_rpc.getaddressinfo(addr) + assert_equal(info['ismine'], True) + restore2_rpc.rescanblockchain() + restore2_rpc.gettransaction(out_of_kp_txid) + info = restore2_rpc.getaddressinfo(addr) + assert_equal(info['ismine'], True) + + # Check again that 3 keys were derived. + # Empty keypool and get an address that is beyond the initial + # keypool + origin_rpc.getnewaddress() + origin_rpc.getnewaddress() + last_addr = origin_rpc.getnewaddress() + addr = origin_rpc.getnewaddress() + + # Check that the restored seed has last_addr but does not have addr + info = restore_rpc.getaddressinfo(last_addr) + assert_equal(info['ismine'], True) + info = restore_rpc.getaddressinfo(addr) + assert_equal(info['ismine'], False) + info = restore2_rpc.getaddressinfo(last_addr) + assert_equal(info['ismine'], True) + info = restore2_rpc.getaddressinfo(addr) + assert_equal(info['ismine'], False) + if __name__ == '__main__': WalletHDTest().main()