From 095142d1f93f39ad2b88dbe8d40140223a1b3900 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Tue, 18 Jul 2017 15:49:56 -0400 Subject: [PATCH] [wallet] keypool mark-used and topup This commit adds basic keypool mark-used and topup: - try to topup the keypool on initial load - if a key in the keypool is used, mark all keys before that as used and try to top up --- src/wallet/wallet.cpp | 50 ++++++++++++++++++++++++++++++++++++ src/wallet/wallet.h | 4 +++ test/functional/wallet-hd.py | 10 +++++--- 3 files changed, 60 insertions(+), 4 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ce345804e..b40dcceda 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -12,6 +12,7 @@ #include "consensus/consensus.h" #include "consensus/validation.h" #include "fs.h" +#include "init.h" #include "key.h" #include "keystore.h" #include "validation.h" @@ -1053,6 +1054,30 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const CBlockI if (fExisted && !fUpdate) return false; if (fExisted || IsMine(tx) || IsFromMe(tx)) { + /* Check if any keys in the wallet keypool that were supposed to be unused + * have appeared in a new transaction. If so, remove those keys from the keypool. + * This can happen when restoring an old wallet backup that does not contain + * the mostly recently created transactions from newer versions of the wallet. + */ + + // loop though all outputs + for (const CTxOut& txout: tx.vout) { + // extract addresses and check if they match with an unused keypool key + std::vector vAffected; + CAffectedKeysVisitor(*this, vAffected).Process(txout.scriptPubKey); + for (const CKeyID &keyid : vAffected) { + std::map::const_iterator mi = m_pool_key_to_index.find(keyid); + if (mi != m_pool_key_to_index.end()) { + LogPrintf("%s: Detected a used keypool key, mark all keypool key up to this key as used\n", __func__); + MarkReserveKeysAsUsed(mi->second); + + if (!TopUpKeyPool()) { + LogPrintf("%s: Topping up keypool failed (locked wallet)\n", __func__); + } + } + } + } + CWalletTx wtx(this, ptx); // Get merkle branch if transaction was found in a block @@ -3611,6 +3636,28 @@ void CReserveKey::ReturnKey() vchPubKey = CPubKey(); } +void CWallet::MarkReserveKeysAsUsed(int64_t keypool_id) +{ + AssertLockHeld(cs_wallet); + bool internal = setInternalKeyPool.count(keypool_id); + if (!internal) assert(setExternalKeyPool.count(keypool_id)); + std::set *setKeyPool = internal ? &setInternalKeyPool : &setExternalKeyPool; + auto it = setKeyPool->begin(); + + CWalletDB walletdb(*dbw); + while (it != std::end(*setKeyPool)) { + const int64_t& index = *(it); + if (index > keypool_id) break; // set*KeyPool is ordered + + CKeyPool keypool; + if (walletdb.ReadPool(index, keypool)) { //TODO: This should be unnecessary + m_pool_key_to_index.erase(keypool.vchPubKey.GetID()); + } + walletdb.ErasePool(index); + it = setKeyPool->erase(it); + } +} + bool CWallet::HasUnusedKeys(int min_keys) const { return setExternalKeyPool.size() >= min_keys && (setInternalKeyPool.size() >= min_keys || !CanSupportFeature(FEATURE_HD_SPLIT)); @@ -3989,6 +4036,9 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) RegisterValidationInterface(walletInstance); + // Try to top up keypool. No-op if the wallet is locked. + walletInstance->TopUpKeyPool(); + CBlockIndex *pindexRescan = chainActive.Genesis(); if (!GetBoolArg("-rescan", false)) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 310300126..a9b90fe9a 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -977,6 +977,10 @@ public: void ReturnKey(int64_t nIndex, bool fInternal, const CPubKey& pubkey); bool GetKeyFromPool(CPubKey &key, bool internal = false); int64_t GetOldestKeyPoolTime(); + /** + * Marks all keys in the keypool up to and including reserve_key as used. + */ + void MarkReserveKeysAsUsed(int64_t keypool_id); const std::map& GetAllReserveKeys() const { return m_pool_key_to_index; } /** Does the wallet have at least min_keys in the keypool? */ bool HasUnusedKeys(int min_keys) const; diff --git a/test/functional/wallet-hd.py b/test/functional/wallet-hd.py index dfd3dc83c..821575ed1 100755 --- a/test/functional/wallet-hd.py +++ b/test/functional/wallet-hd.py @@ -9,7 +9,6 @@ from test_framework.util import ( assert_equal, connect_nodes_bi, ) -import os import shutil @@ -72,10 +71,12 @@ class WalletHDTest(BitcoinTestFramework): self.log.info("Restore backup ...") self.stop_node(1) - os.remove(self.options.tmpdir + "/node1/regtest/wallet.dat") + # we need to delete the complete regtest directory + # otherwise node1 would auto-recover all funds in flag the keypool keys as used + shutil.rmtree(tmpdir + "/node1/regtest/blocks") + shutil.rmtree(tmpdir + "/node1/regtest/chainstate") shutil.copyfile(tmpdir + "/hd.bak", tmpdir + "/node1/regtest/wallet.dat") self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1]) - #connect_nodes_bi(self.nodes, 0, 1) # Assert that derivation is deterministic hd_add_2 = None @@ -85,11 +86,12 @@ class WalletHDTest(BitcoinTestFramework): assert_equal(hd_info_2["hdkeypath"], "m/0'/0'/"+str(_+1)+"'") assert_equal(hd_info_2["hdmasterkeyid"], masterkeyid) assert_equal(hd_add, hd_add_2) + connect_nodes_bi(self.nodes, 0, 1) + self.sync_all() # Needs rescan self.stop_node(1) self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1] + ['-rescan']) - #connect_nodes_bi(self.nodes, 0, 1) assert_equal(self.nodes[1].getbalance(), num_hd_adds + 1) # send a tx and make sure its using the internal chain for the changeoutput