diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a743f04dd15..ffff81d9f96 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -466,6 +466,8 @@ void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vectorpindexBestKnownBlock == NULL || state->pindexBestKnownBlock->nChainWork < chainActive.Tip()->nChainWork || state->pindexBestKnownBlock->nChainWork < UintToArith256(consensusParams.nMinimumChainWork)) { // This peer has nothing interesting. return; @@ -730,7 +732,7 @@ PeerLogicValidation::PeerLogicValidation(CConnman* connmanIn) : connman(connmanI recentRejects.reset(new CRollingBloomFilter(120000, 0.000001)); } -void PeerLogicValidation::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex, const std::vector& vtxConflicted) { +void PeerLogicValidation::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindex, const std::vector& vtxConflicted, bool &requestPause) { LOCK(cs_main); std::vector vOrphanErase; diff --git a/src/net_processing.h b/src/net_processing.h index db6d81e6b67..eb5cccf2688 100644 --- a/src/net_processing.h +++ b/src/net_processing.h @@ -34,7 +34,7 @@ class PeerLogicValidation : public CValidationInterface { public: PeerLogicValidation(CConnman* connmanIn); - void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) override; + void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted, bool &requestPause) override; void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; void BlockChecked(const CBlock& block, const CValidationState& state) override; void NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr& pblock) override; diff --git a/src/validation.cpp b/src/validation.cpp index ddc8b1b964a..8cbec08e14f 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -93,6 +93,14 @@ CScript COINBASE_FLAGS; const std::string strMessageMagic = "Bitcoin Signed Message:\n"; +/** if enable, blocks will not be requested automatically */ +static const bool DEFAULT_BLOCK_REQUESTS_PAUSED = false; +std::atomic fPauseBlockRequests(DEFAULT_BLOCK_REQUESTS_PAUSED); + +/** if enable, ActiveBestChain will ignore/refuse to do tip updates */ +static const bool DEFAULT_TIP_UPDATE_PAUSED = false; +std::atomic fPauseTipUpdates(DEFAULT_TIP_UPDATE_PAUSED); + // Internal stuff namespace { @@ -2417,6 +2425,12 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, // us in the middle of ProcessNewBlock - do not assume pblock is set // sanely for performance or correctness! + if (isTipUpdatesPaused()) { + LogPrintf("%s: ignore ActivateBestChain, tip update are disabled\n", std::string(__func__)); + + // we will abort with a return value of true to not trigger error routines/shutdown + return true; + } CBlockIndex *pindexMostWork = NULL; CBlockIndex *pindexNewTip = NULL; int nStopAtHeight = GetArg("-stopatheight", DEFAULT_STOPATHEIGHT); @@ -2453,9 +2467,18 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, pindexFork = chainActive.FindFork(pindexOldTip); fInitialDownload = IsInitialBlockDownload(); + bool requestPause = false; for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) { assert(trace.pblock && trace.pindex); - GetMainSignals().BlockConnected(trace.pblock, trace.pindex, *trace.conflictedTxs); + GetMainSignals().BlockConnected(trace.pblock, trace.pindex, *trace.conflictedTxs, requestPause); + if (requestPause) { + // in case we are in pruned mode, we have to halt verification and block requests + // to ensure the signal listener can keep up with the updates + if (fPruneMode) { + setBlockRequestsPaused(true); + setTipUpdatesPaused(true); + } + } } } // When we reach this point, we switched to a new tip (stored in pindexNewTip). @@ -2471,7 +2494,7 @@ bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, } if (nStopAtHeight && pindexNewTip && pindexNewTip->nHeight >= nStopAtHeight) StartShutdown(); - } while (pindexNewTip != pindexMostWork); + } while (pindexNewTip != pindexMostWork && !isTipUpdatesPaused()); CheckBlockIndex(chainparams.GetConsensus()); // Write changes periodically to disk, after relay. @@ -4353,6 +4376,24 @@ void DumpMempool(void) } } +bool isBlockRequestsPaused() { + return fPauseBlockRequests; +} + +void setBlockRequestsPaused(bool state) { + LogPrintf("%s Block Requests\n", (state ? "Pause" : "Resume")); + fPauseBlockRequests = state; +} + +bool isTipUpdatesPaused() { + return fPauseTipUpdates; +} + +void setTipUpdatesPaused(bool state) { + LogPrintf("%s Tip Updates\n", (state ? "Pause" : "Resume")); + fPauseTipUpdates = state; +} + //! Guess how far we are in the verification process at the given block index double GuessVerificationProgress(const ChainTxData& data, CBlockIndex *pindex) { if (pindex == NULL) diff --git a/src/validation.h b/src/validation.h index a9f995abb88..00573feae21 100644 --- a/src/validation.h +++ b/src/validation.h @@ -286,6 +286,20 @@ CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); double GuessVerificationProgress(const ChainTxData& data, CBlockIndex* pindex); /** + * Pausing block requests will temporarily pause the verification process. + * Pausing won't prevent ActivateBestChain from connecting blocks (that are in flight or already on disk) + */ +bool isBlockRequestsPaused(); +void setBlockRequestsPaused(bool state); + +/** + * Pausing tip updates temporarily pauses connecting new blocks + * Pausing won't prevent the net logic from requesting more blocks for download (up to BLOCK_DOWNLOAD_WINDOW) + */ +bool isTipUpdatesPaused(); +void setTipUpdatesPaused(bool state); + +/** * Mark one block file as pruned. */ void PruneOneBlockFile(const int fileNumber); diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index bf20d606f83..c1b30c08008 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -17,7 +17,7 @@ struct MainSignalsInstance { boost::signals2::signal UpdatedBlockTip; boost::signals2::signal TransactionAddedToMempool; - boost::signals2::signal &, const CBlockIndex *pindex, const std::vector&)> BlockConnected; + boost::signals2::signal &, const CBlockIndex *pindex, const std::vector&, bool &)> BlockConnected; boost::signals2::signal &)> BlockDisconnected; boost::signals2::signal SetBestChain; boost::signals2::signal Inventory; @@ -56,7 +56,7 @@ CMainSignals& GetMainSignals() void RegisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.m_internals->UpdatedBlockTip.connect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1, _2, _3)); g_signals.m_internals->TransactionAddedToMempool.connect(boost::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, _1)); - g_signals.m_internals->BlockConnected.connect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3)); + g_signals.m_internals->BlockConnected.connect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3, _4)); g_signals.m_internals->BlockDisconnected.connect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1)); g_signals.m_internals->SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.m_internals->Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); @@ -71,7 +71,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.m_internals->Inventory.disconnect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.m_internals->SetBestChain.disconnect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); g_signals.m_internals->TransactionAddedToMempool.disconnect(boost::bind(&CValidationInterface::TransactionAddedToMempool, pwalletIn, _1)); - g_signals.m_internals->BlockConnected.disconnect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3)); + g_signals.m_internals->BlockConnected.disconnect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3, _4)); g_signals.m_internals->BlockDisconnected.disconnect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1)); g_signals.m_internals->UpdatedBlockTip.disconnect(boost::bind(&CValidationInterface::UpdatedBlockTip, pwalletIn, _1, _2, _3)); g_signals.m_internals->NewPoWValidBlock.disconnect(boost::bind(&CValidationInterface::NewPoWValidBlock, pwalletIn, _1, _2)); @@ -97,8 +97,8 @@ void CMainSignals::TransactionAddedToMempool(const CTransactionRef &ptx) { m_internals->TransactionAddedToMempool(ptx); } -void CMainSignals::BlockConnected(const std::shared_ptr &pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) { - m_internals->BlockConnected(pblock, pindex, vtxConflicted); +void CMainSignals::BlockConnected(const std::shared_ptr &pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted, bool &requestPause) { + m_internals->BlockConnected(pblock, pindex, vtxConflicted, requestPause); } void CMainSignals::BlockDisconnected(const std::shared_ptr &pblock) { diff --git a/src/validationinterface.h b/src/validationinterface.h index 568da66df28..828dd48f4b8 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -40,7 +40,7 @@ class CValidationInterface { * Notifies listeners of a block being connected. * Provides a vector of transactions evicted from the mempool as a result. */ - virtual void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::vector &txnConflicted) {} + virtual void BlockConnected(const std::shared_ptr &block, const CBlockIndex *pindex, const std::vector &txnConflicted, bool &requestPause) {} /** Notifies listeners of a block being disconnected */ virtual void BlockDisconnected(const std::shared_ptr &block) {} /** Notifies listeners of the new active block chain on-disk. */ @@ -84,7 +84,7 @@ class CMainSignals { void UpdatedBlockTip(const CBlockIndex *, const CBlockIndex *, bool fInitialDownload); void TransactionAddedToMempool(const CTransactionRef &); - void BlockConnected(const std::shared_ptr &, const CBlockIndex *pindex, const std::vector &); + void BlockConnected(const std::shared_ptr &, const CBlockIndex *pindex, const std::vector &, bool &); void BlockDisconnected(const std::shared_ptr &); void UpdatedTransaction(const uint256 &); void SetBestChain(const CBlockLocator &); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 5f72e3b6f59..72d48f88967 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -2089,6 +2089,10 @@ UniValue walletpassphrase(const JSONRPCRequest& request) pwallet->TopUpKeyPool(); + // Give a hint to the wallet in case we have paused sync (we may have fall bellow the keypool min size limit). + // This runs synchronous, at least during the resync, we can be sure the keypool can be topped up. + pwallet->EventuallyRescanAfterKeypoolTopUp(); + int64_t nSleepTime = request.params[1].get_int64(); pwallet->nRelockTime = GetTime() + nSleepTime; RPCRunLater(strprintf("lockwallet(%s)", pwallet->GetName()), boost::bind(LockWallet, pwallet), nSleepTime); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5e9701c71cd..48730ffa5ac 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" //for StartShutdown() #include "key.h" #include "keystore.h" #include "validation.h" @@ -78,6 +79,38 @@ std::string COutput::ToString() const return strprintf("COutput(%s, %d, %d) [%s]", tx->GetHash().ToString(), i, nDepth, FormatMoney(tx->tx->vout[i].nValue)); } +class CAffectedKeysVisitor : public boost::static_visitor { +private: + const CKeyStore &keystore; + std::vector &vKeys; + +public: + CAffectedKeysVisitor(const CKeyStore &keystoreIn, std::vector &vKeysIn) : keystore(keystoreIn), vKeys(vKeysIn) {} + + void Process(const CScript &script) { + txnouttype type; + std::vector vDest; + int nRequired; + if (ExtractDestinations(script, type, vDest, nRequired)) { + for (const CTxDestination &dest : vDest) + boost::apply_visitor(*this, dest); + } + } + + void operator()(const CKeyID &keyId) { + if (keystore.HaveKey(keyId)) + vKeys.push_back(keyId); + } + + void operator()(const CScriptID &scriptId) { + CScript script; + if (keystore.GetCScript(scriptId, script)) + Process(script); + } + + void operator()(const CNoDestination &none) {} +}; + const CWalletTx* CWallet::GetWalletTx(const uint256& hash) const { LOCK(cs_wallet); @@ -361,6 +394,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, void CWallet::SetBestChain(const CBlockLocator& loc) { + if (m_sync_paused_for_keypool) return; CWalletDB walletdb(*dbw); walletdb.WriteBestBlock(loc); } @@ -983,6 +1017,26 @@ 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. + */ + std::set keyPool; + GetAllReserveKeys(keyPool); + // 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) { + if (keyPool.count(keyid)) { + LogPrintf("%s: Detected a used Keypool key, mark all keypool key up to this key as used\n", __func__); + MarkReserveKeysAsUsed(keyid); + } + } + } + CWalletTx wtx(this, ptx); // Get merkle branch if transaction was found in a block @@ -1135,29 +1189,41 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pin } void CWallet::TransactionAddedToMempool(const CTransactionRef& ptx) { + if (m_sync_paused_for_keypool) return; LOCK2(cs_main, cs_wallet); SyncTransaction(ptx); } -void CWallet::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) { - LOCK2(cs_main, cs_wallet); - // TODO: Temporarily ensure that mempool removals are notified before - // connected transactions. This shouldn't matter, but the abandoned - // state of transactions in our wallet is currently cleared when we - // receive another notification and there is a race condition where - // notification of a connected conflict might cause an outside process - // to abandon a transaction and then have it inadvertently cleared by - // the notification that the conflicted transaction was evicted. - - for (const CTransactionRef& ptx : vtxConflicted) { - SyncTransaction(ptx); +void CWallet::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted, bool &requestPause) { + if (m_sync_paused_for_keypool) { + requestPause = true; + return; } - for (size_t i = 0; i < pblock->vtx.size(); i++) { - SyncTransaction(pblock->vtx[i], pindex, i); + + { + LOCK2(cs_main, cs_wallet); + // TODO: Temporarily ensure that mempool removals are notified before + // connected transactions. This shouldn't matter, but the abandoned + // state of transactions in our wallet is currently cleared when we + // receive another notification and there is a race condition where + // notification of a connected conflict might cause an outside process + // to abandon a transaction and then have it inadvertently cleared by + // the notification that the conflicted transaction was evicted. + + for (const CTransactionRef& ptx : vtxConflicted) { + SyncTransaction(ptx); + } + for (size_t i = 0; i < pblock->vtx.size(); i++) { + SyncTransaction(pblock->vtx[i], pindex, i); + } + if (m_sync_paused_for_keypool) { + requestPause = true; + } } } void CWallet::BlockDisconnected(const std::shared_ptr& pblock) { + if (m_sync_paused_for_keypool) return; LOCK2(cs_main, cs_wallet); for (const CTransactionRef& ptx : pblock->vtx) { @@ -1370,6 +1436,57 @@ bool CWallet::IsHDEnabled() const return !hdChain.masterKeyID.IsNull(); } +void CWallet::EventuallyRescanAfterKeypoolTopUp() { + if (m_sync_paused_for_keypool) { + + // for now, enable block requests and tip updates via the states + // this will only be sufficient as long as only a single wallet adjusts these stats + // switch to counters (instead of a single boolean state) could solve this + ::setBlockRequestsPaused(false); + ::setTipUpdatesPaused(false); + + // disabled the per-wallet pause + m_sync_paused_for_keypool = false; + + const CChainParams& chainparams = Params(); + CValidationState state; + if (!ActivateBestChain(state, chainparams)) { + LogPrintf("Failed to connect best block"); + StartShutdown(); + } + + CBlockIndex *pindexRescan = chainActive.Genesis(); + CWalletDB walletdb(*dbw); + CBlockLocator locator; + if (walletdb.ReadBestBlock(locator)) { + pindexRescan = FindForkInGlobalIndex(chainActive, locator); + } + + if (chainActive.Tip() && chainActive.Tip() != pindexRescan) { + //We can't rescan beyond non-pruned blocks, stop and throw an error + //this might happen if a user uses a old wallet within a pruned node + // or if he ran -disablewallet for a longer time, then decided to re-enable + if (fPruneMode) { + CBlockIndex *block = chainActive.Tip(); + while (block && block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA) && block->pprev->nTx > 0 && pindexRescan != block) { + block = block->pprev; + } + + if (pindexRescan != block) { + const static std::string pruneError = "Prune: last wallet synchronisation goes beyond pruned data. You need to -reindex (download the whole blockchain again in case of pruned node)"; + uiInterface.ThreadSafeMessageBox(pruneError, "", CClientUIInterface::MSG_ERROR); + StartShutdown(); + throw std::runtime_error(pruneError); + } + } + if (pindexRescan) { + LogPrintf("Rescanning from height: %d\n", pindexRescan->nHeight); + } + ScanForWalletTransactions(pindexRescan, true); + } + } +} + int64_t CWalletTx::GetTxTime() const { int64_t n = nTimeSmart; @@ -1515,6 +1632,7 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool f CBlockIndex* pindex = pindexStart; CBlockIndex* ret = nullptr; + CBlockIndex* pindex_prev = NULL; { LOCK2(cs_main, cs_wallet); fAbortRescan = false; @@ -1537,6 +1655,11 @@ CBlockIndex* CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool f for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) { AddToWalletIfInvolvingMe(block.vtx[posInBlock], pindex, posInBlock, fUpdate); } + if (m_sync_paused_for_keypool) { + LogPrintf("Aborting rescanning at block %d. Please extend the keypool first.\n", (pindex ? pindex->nHeight: 0)); + return pindex_prev; + } + pindex_prev = pindex; } else { ret = pindex; } @@ -3225,6 +3348,7 @@ void CWallet::ReserveKeyFromKeyPool(int64_t& nIndex, CKeyPool& keypool, bool int setKeyPool.erase(id); assert(keypool.vchPubKey.IsValid()); LogPrintf("keypool reserve %d\n", nIndex); + CheckKeypoolMinSize(); return; } } @@ -3490,6 +3614,71 @@ void CReserveKey::ReturnKey() vchPubKey = CPubKey(); } +bool CWallet::CheckKeypoolMinSize() { + LOCK(cs_wallet); + size_t extKeypoolSize = KeypoolCountExternalKeys(); + if (IsHDEnabled() && (extKeypoolSize < GetArg("-keypoolrestoremin", DEFAULT_KEYPOOL_RESTORE_MIN) || (setKeyPool.size()-extKeypoolSize) < GetArg("-keypoolrestoremin", DEFAULT_KEYPOOL_RESTORE_MIN))) { + // if the remaining keypool size is below the gap limit, refuse to continue with the sync + m_sync_paused_for_keypool = true; + LogPrintf("%s: Keypool ran below min size, pause wallet sync\n", __func__); + return false; + } + return true; +} + +void CWallet::MarkReserveKeysAsUsed(const CKeyID& keyId) +{ + LOCK(cs_wallet); + CWalletDB walletdb(*dbw); + auto it = std::begin(setKeyPool); + + bool foundInternal = false; + int64_t foundIndex = -1; + for (const int64_t& id : setKeyPool) { + CKeyPool keypool; + if (!walletdb.ReadPool(id, keypool)) { + throw std::runtime_error(std::string(__func__) + ": read failed"); + } + + if (keypool.vchPubKey.GetID() == keyId) { + foundInternal = keypool.fInternal; + foundIndex = id; + if (!keypool.fInternal) { + SetAddressBook(keyId, "", "receive"); + } + break; + } + } + + // mark all keys up to the found key as used + if (foundIndex >= 0) { + while (it != std::end(setKeyPool)) { + const int64_t& id = *(it); + if (id > foundIndex) break; // setKeyPool is ordered + + CKeyPool keypool; + if (!walletdb.ReadPool(id, keypool)) { + throw std::runtime_error(std::string(__func__) + ": read failed"); + } + + // only mark keys on the corresponding chain + if (keypool.fInternal == foundInternal) { + KeepKey(id); + it = setKeyPool.erase(it); + continue; + } + + ++it; + } + } + + if (IsHDEnabled() && !TopUpKeyPool()) { + LogPrintf("%s: Topping up keypool failed (locked wallet)\n", __func__); + } + + CheckKeypoolMinSize(); +} + void CWallet::GetAllReserveKeys(std::set& setAddress) const { setAddress.clear(); @@ -3559,38 +3748,6 @@ void CWallet::ListLockedCoins(std::vector& vOutpts) const /** @} */ // end of Actions -class CAffectedKeysVisitor : public boost::static_visitor { -private: - const CKeyStore &keystore; - std::vector &vKeys; - -public: - CAffectedKeysVisitor(const CKeyStore &keystoreIn, std::vector &vKeysIn) : keystore(keystoreIn), vKeys(vKeysIn) {} - - void Process(const CScript &script) { - txnouttype type; - std::vector vDest; - int nRequired; - if (ExtractDestinations(script, type, vDest, nRequired)) { - for (const CTxDestination &dest : vDest) - boost::apply_visitor(*this, dest); - } - } - - void operator()(const CKeyID &keyId) { - if (keystore.HaveKey(keyId)) - vKeys.push_back(keyId); - } - - void operator()(const CScriptID &scriptId) { - CScript script; - if (keystore.GetCScript(scriptId, script)) - Process(script); - } - - void operator()(const CNoDestination &none) {} -}; - void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) const { AssertLockHeld(cs_wallet); // mapKeyMetadata mapKeyBirth.clear(); @@ -3792,6 +3949,7 @@ std::string CWallet::GetWalletHelpString(bool showDebug) strUsage += HelpMessageOpt("-dblogsize=", strprintf("Flush wallet database activity from memory to disk log every megabytes (default: %u)", DEFAULT_WALLET_DBLOGSIZE)); strUsage += HelpMessageOpt("-flushwallet", strprintf("Run a thread to flush wallet periodically (default: %u)", DEFAULT_FLUSHWALLET)); + strUsage += HelpMessageOpt("-keypoolrestore min", strprintf(_("If the keypool drops below this number of keys, generate new keys, or pause sync (default: %u)"), DEFAULT_KEYPOOL_RESTORE_MIN)); strUsage += HelpMessageOpt("-privdb", strprintf("Sets the DB_PRIVATE flag in the wallet db environment (default: %u)", DEFAULT_WALLET_PRIVDB)); strUsage += HelpMessageOpt("-walletrejectlongchains", strprintf(_("Wallet will not create transactions that violate mempool chain limits (default: %u)"), DEFAULT_WALLET_REJECT_LONG_CHAINS)); } @@ -3912,6 +4070,23 @@ CWallet* CWallet::CreateWalletFromFile(const std::string walletFile) RegisterValidationInterface(walletInstance); + // Keypool Restore: Make sure we always have a reasonable keypool size if HD is enabled + unsigned int keypool_size = GetArg("-keypool", DEFAULT_KEYPOOL_SIZE); + unsigned int keypool_restore_min = GetArg("-keypoolrestoremin", DEFAULT_KEYPOOL_RESTORE_MIN); + if (walletInstance->IsHDEnabled()) { + if (walletInstance->IsCrypted()) { + if (keypool_size < DEFAULT_KEYPOOL_RESTORE_MIN && keypool_size < keypool_restore_min) { + LogPrintf("Parameter Interaction: keypool size (%d) must be larger than keypool restore minimum size for encrypted wallets (%d)\n", keypool_size, keypool_restore_min); + SoftSetArg("-keypool", std::to_string(keypool_restore_min)); + } + } else { + walletInstance->TopUpKeyPool(); + } + + if (!walletInstance->CheckKeypoolMinSize()) { + InitWarning(_("Your keypool size is below the required limit for HD rescans. Wallet synchronisation is now paused until you have refilled the keypool.")); + } + } CBlockIndex *pindexRescan = chainActive.Genesis(); if (!GetBoolArg("-rescan", false)) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index e3715cdf376..8341732ef0c 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -41,6 +41,9 @@ extern bool bSpendZeroConfChange; extern bool fWalletRbf; static const unsigned int DEFAULT_KEYPOOL_SIZE = 100; +//! If the keypool drops below this value, generate new keys (or pause sync if +// unable to generate new keys) +static const unsigned int DEFAULT_KEYPOOL_RESTORE_MIN = 20; //! -paytxfee default static const CAmount DEFAULT_TRANSACTION_FEE = 0; //! -fallbackfee default @@ -656,6 +659,13 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface std::atomic fScanningWallet; /** + * m_sync_paused_for_keypool temporarily pauses the transaction syncing process until + * the keypool has been refilled (manual topup may be required for encrypted and locked wallets). + * Not doing so may result in missing transactions in case of a HD recovery (or shared HD wallet). + */ + std::atomic m_sync_paused_for_keypool; + + /** * Select a set of coins such that nValueRet >= nTargetValue and at least * all coins from coinControl are selected; Never select unconfirmed coins * if they are not ours @@ -795,6 +805,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface nRelockTime = 0; fAbortRescan = false; fScanningWallet = false; + m_sync_paused_for_keypool = false; } std::map mapWallet; @@ -919,7 +930,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); bool LoadToWallet(const CWalletTx& wtxIn); void TransactionAddedToMempool(const CTransactionRef& tx) override; - void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted) override; + void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex *pindex, const std::vector& vtxConflicted, bool &requestPause) override; void BlockDisconnected(const std::shared_ptr& pblock) override; bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate); int64_t RescanFromTime(int64_t startTime, bool update); @@ -979,6 +990,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void ReturnKey(int64_t nIndex); bool GetKeyFromPool(CPubKey &key, bool internal = false); int64_t GetOldestKeyPoolTime(); + bool CheckKeypoolMinSize(); + void MarkReserveKeysAsUsed(const CKeyID& keyId); void GetAllReserveKeys(std::set& setAddress) const; std::set< std::set > GetAddressGroupings(); @@ -1126,6 +1139,8 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface caller must ensure the current wallet version is correct before calling this function). */ bool SetHDMasterKey(const CPubKey& key); + + void EventuallyRescanAfterKeypoolTopUp(); }; /** A key allocated from the key pool. */ diff --git a/src/zmq/zmqnotificationinterface.cpp b/src/zmq/zmqnotificationinterface.cpp index c0638980568..6c10b32bc02 100644 --- a/src/zmq/zmqnotificationinterface.cpp +++ b/src/zmq/zmqnotificationinterface.cpp @@ -165,7 +165,7 @@ void CZMQNotificationInterface::TransactionAddedToMempool(const CTransactionRef& } } -void CZMQNotificationInterface::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) +void CZMQNotificationInterface::BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted, bool &requestPause) { for (const CTransactionRef& ptx : pblock->vtx) { // Do a normal notify for each transaction added in the block diff --git a/src/zmq/zmqnotificationinterface.h b/src/zmq/zmqnotificationinterface.h index eec6f7bc645..d5adbe14164 100644 --- a/src/zmq/zmqnotificationinterface.h +++ b/src/zmq/zmqnotificationinterface.h @@ -26,7 +26,7 @@ class CZMQNotificationInterface : public CValidationInterface // CValidationInterface void TransactionAddedToMempool(const CTransactionRef& tx) override; - void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted) override; + void BlockConnected(const std::shared_ptr& pblock, const CBlockIndex* pindexConnected, const std::vector& vtxConflicted, bool &requestPause) override; void BlockDisconnected(const std::shared_ptr& pblock) override; void UpdatedBlockTip(const CBlockIndex *pindexNew, const CBlockIndex *pindexFork, bool fInitialDownload) override; diff --git a/test/functional/keypool-restore.py b/test/functional/keypool-restore.py new file mode 100755 index 00000000000..bfd97ce3f62 --- /dev/null +++ b/test/functional/keypool-restore.py @@ -0,0 +1,153 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017 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 HD Wallet key restore function.""" +import shutil + +from test_framework.test_framework import BitcoinTestFramework, BITCOIND_PROC_WAIT_TIMEOUT +from test_framework.util import assert_equal, connect_nodes_bi + +class KeypoolRestoreTest(BitcoinTestFramework): + def __init__(self): + super().__init__() + self.setup_clean_chain = True + self.num_nodes = 2 + self.extra_args = [['-usehd=0'], ['-usehd=1', '-keypool=100', '-keypoolrestoremin=20']] + + def run_test(self): + tmpdir = self.options.tmpdir + + self.log.info("Initialize wallet including backups of unencrypted and encrypted wallet") + # stop and backup original wallet (only keypool has been initialized) + self.stop_node(1) + shutil.copyfile(tmpdir + "/node1/regtest/wallet.dat", tmpdir + "/hd.bak") + + # start again and encrypt wallet + self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1]) + self.nodes[1].encryptwallet('test') + self.bitcoind_processes[1].wait(timeout=BITCOIND_PROC_WAIT_TIMEOUT) + # node will be stopped during encryption, now do a backup + shutil.copyfile(tmpdir + "/node1/regtest/wallet.dat", tmpdir + "/hd.enc.bak") + + # start the node with encrypted wallet, get address in new pool at pos 50 (over the gap limit) + self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1]) + for _ in range(50): + addr_enc_oldpool = self.nodes[1].getnewaddress() + + # now make sure we retrive an address in the extended pool + self.nodes[1].walletpassphrase("test", 10) + for _ in range(80): + addr_enc_extpool = self.nodes[1].getnewaddress() + + # stop and load initial backup of the unencrypted wallet + self.stop_node(1) + 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) + for _ in range(10): + addr = self.nodes[1].getnewaddress() + + self.nodes[0].generate(101) + addr = self.nodes[1].getnewaddress() + assert_equal(self.nodes[1].validateaddress(addr)['hdkeypath'], "m/0'/0'/11'") + + rawch = self.nodes[1].getrawchangeaddress() + self.nodes[0].sendtoaddress(addr, 1) + n0addr = self.nodes[0].getnewaddress() + txdata = self.nodes[0].createrawtransaction([], {rawch: 2.0, n0addr: 3.0}) + + txdata_f = self.nodes[0].fundrawtransaction(txdata) + txdata_s = self.nodes[0].signrawtransaction(txdata_f['hex']) + self.nodes[0].sendrawtransaction(txdata_s['hex']) + + self.sync_all() + self.nodes[0].generate(1) + self.sync_all() + + assert_equal(self.nodes[1].getbalance(), 3) + assert_equal(self.nodes[1].listtransactions()[0]['category'], "receive") + + self.log.info("Testing with unencrypted wallet") + self.stop_node(1) + shutil.rmtree(tmpdir + "/node1/regtest/blocks") + shutil.rmtree(tmpdir + "/node1/regtest/chainstate") + shutil.copyfile(tmpdir + "/hd.bak", tmpdir + "/node1/regtest/wallet.dat") + self.nodes[0].generate(1) + self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1]) + connect_nodes_bi(self.nodes, 0, 1) + self.sync_all() + assert_equal(self.nodes[1].getbalance(), 3) # make sure we have reconstructed the transaction + assert_equal(self.nodes[1].listtransactions()[0]['category'], "receive") + + # now check if we have marked all keys up to the used keypool key as used + assert_equal(self.nodes[1].validateaddress(self.nodes[1].getnewaddress())['hdkeypath'], "m/0'/0'/12'") + + # make sure the key on the internal chain is also marked as used + assert_equal(self.nodes[1].validateaddress(self.nodes[1].getrawchangeaddress())['hdkeypath'], "m/0'/1'/1'") + + # continue send funds (one in the main keypool over the gap limit, the other in the extended pool space) + self.nodes[0].sendtoaddress(addr_enc_oldpool, 10) + self.nodes[0].generate(1) + stop_height = self.nodes[0].getblockchaininfo()['blocks'] + self.nodes[0].sendtoaddress(addr_enc_extpool, 5) + self.nodes[0].generate(1) + + ######################################################### + ######################################################### + + self.log.info("Testing with encrypted wallet") + # Try with the encrypted wallet (non pruning) + self.stop_node(1) + shutil.rmtree(tmpdir + "/node1/regtest/chainstate") + shutil.rmtree(tmpdir + "/node1/regtest/blocks") + shutil.copyfile(tmpdir + "/hd.enc.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) + + # Sync must be possible, though the wallet bestblock should lack behind + self.sync_all() + + # The balance should cover everything expect the very last tx of 5 BTC + assert_equal(self.nodes[1].getbalance(), 13) + + # unlock the wallet, the sync can continue then + self.nodes[1].walletpassphrase("test", 100) + self.sync_all() # sync is now possible + + # we should now have restored all funds + assert_equal(self.nodes[1].getbalance(), 18) # all funds recovered + assert_equal(self.nodes[1].listtransactions()[0]['category'], "receive") + + ######################################################### + ######################################################### + + self.log.info("Testing with encrypted wallet in prune mode") + # Now try with the encrypted wallet in prune mode + self.stop_node(1) + shutil.rmtree(tmpdir + "/node1/regtest/blocks") + shutil.rmtree(tmpdir + "/node1/regtest/chainstate") + shutil.copyfile(tmpdir + "/hd.enc.bak", tmpdir + "/node1/regtest/wallet.dat") + self.nodes[1] = self.start_node(1, self.options.tmpdir, self.extra_args[1] + ['-prune=550']) + connect_nodes_bi(self.nodes, 0, 1) + + # now we should only be capable to sync up to the second last block (pruned mode, sync will be paused) + assert_equal(self.nodes[1].waitforblockheight(stop_height, 10 * 1000)['height'], stop_height) # must be possible + + # This must timeout now, we can't sync up to stop_height+1 (== most recent block) + # Sync must be paused at this point + assert_equal(self.nodes[1].waitforblockheight(stop_height + 1, 3 * 1000)['height'], stop_height) + + # The balance should cover everything expect the very last tx of 5 BTC + assert_equal(self.nodes[1].getbalance(), 13) + + # unlock the wallet, the sync can continue then + self.nodes[1].walletpassphrase("test", 100) + self.sync_all() # sync is now possible + + # we should now have restored all funds + assert_equal(self.nodes[1].getbalance(), 18) + assert_equal(self.nodes[1].listtransactions()[0]['category'], "receive") + +if __name__ == '__main__': + KeypoolRestoreTest().main() diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 8d698a73276..9db4af17b39 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -209,7 +209,7 @@ def start_node(self, i, dirname, extra_args=None, rpchost=None, timewait=None, b datadir = os.path.join(dirname, "node" + str(i)) if binary is None: binary = os.getenv("BITCOIND", "bitcoind") - args = [binary, "-datadir=" + datadir, "-server", "-keypool=1", "-discover=0", "-rest", "-logtimemicros", "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", "-mocktime=" + str(self.mocktime), "-uacomment=testnode%d" % i] + args = [binary, "-datadir=" + datadir, "-server", "-keypool=1", "-keypoolrestoremin=0", "-discover=0", "-rest", "-logtimemicros", "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", "-mocktime=" + str(self.mocktime), "-uacomment=testnode%d" % i] if extra_args is not None: args.extend(extra_args) self.bitcoind_processes[i] = subprocess.Popen(args, stderr=stderr) @@ -379,7 +379,7 @@ def _initialize_chain(self, test_dir, num_nodes, cachedir): # Create cache directories, run bitcoinds: for i in range(MAX_NODES): datadir = initialize_datadir(cachedir, i) - args = [os.getenv("BITCOIND", "bitcoind"), "-server", "-keypool=1", "-datadir=" + datadir, "-discover=0"] + args = [os.getenv("BITCOIND", "bitcoind"), "-server", "-keypool=1", "-keypoolrestoremin=0", "-datadir=" + datadir, "-discover=0"] if i > 0: args.append("-connect=127.0.0.1:" + str(p2p_port(0))) self.bitcoind_processes[i] = subprocess.Popen(args) diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py old mode 100644 new mode 100755 diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index b7bc6e841b0..e7b55fa74f8 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -55,6 +55,7 @@ # Scripts that are run by the travis build process. # Longest test should go first, to favor running tests in parallel 'wallet-hd.py', + 'keypool-restore.py', 'walletbackup.py', # vv Tests less than 5m vv 'p2p-fullblocktest.py', diff --git a/test/functional/wallet-hd.py b/test/functional/wallet-hd.py index dfd3dc83c52..a2d41866b59 100755 --- a/test/functional/wallet-hd.py +++ b/test/functional/wallet-hd.py @@ -9,7 +9,6 @@ assert_equal, connect_nodes_bi, ) -import os import shutil @@ -72,10 +71,12 @@ def run_test (self): 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 @@ def run_test (self): 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