From 50b6533aa2a9ccbc758aaf5a9f6dfa1c9433bff1 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 27 Apr 2018 14:01:02 -0400 Subject: scripted-diff: Rename SetBestChain callback ChainStateFlushed This much more accurately captures the meaning of the callback. -BEGIN VERIFY SCRIPT- sed -i 's/SetBestChain/ChainStateFlushed/g' src/validationinterface.h src/validationinterface.cpp src/wallet/wallet.h src/wallet/wallet.cpp src/validation.cpp src/index/txindex.h src/index/txindex.cpp -END VERIFY SCRIPT- --- src/index/txindex.cpp | 4 ++-- src/index/txindex.h | 2 +- src/validation.cpp | 2 +- src/validationinterface.cpp | 12 ++++++------ src/validationinterface.h | 4 ++-- src/wallet/wallet.cpp | 6 +++--- src/wallet/wallet.h | 2 +- 7 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/index/txindex.cpp b/src/index/txindex.cpp index 2a661f0330..0bb553ee6a 100644 --- a/src/index/txindex.cpp +++ b/src/index/txindex.cpp @@ -192,7 +192,7 @@ void TxIndex::BlockConnected(const std::shared_ptr& block, const C } } -void TxIndex::SetBestChain(const CBlockLocator& locator) +void TxIndex::ChainStateFlushed(const CBlockLocator& locator) { if (!m_synced) { return; @@ -211,7 +211,7 @@ void TxIndex::SetBestChain(const CBlockLocator& locator) return; } - // This checks that SetBestChain callbacks are received after BlockConnected. The check may fail + // This checks that ChainStateFlushed callbacks are received after BlockConnected. The check may fail // immediately after the the sync thread catches up and sets m_synced. Consider the case where // there is a reorg and the blocks on the stale branch are in the ValidationInterface queue // backlog even after the sync thread has caught up to the new chain tip. In this unlikely diff --git a/src/index/txindex.h b/src/index/txindex.h index ac746de05b..4937bd64e9 100644 --- a/src/index/txindex.h +++ b/src/index/txindex.h @@ -55,7 +55,7 @@ protected: void BlockConnected(const std::shared_ptr& block, const CBlockIndex* pindex, const std::vector& txn_conflicted) override; - void SetBestChain(const CBlockLocator& locator) override; + void ChainStateFlushed(const CBlockLocator& locator) override; public: /// Constructs the TxIndex, which becomes available to be queried. diff --git a/src/validation.cpp b/src/validation.cpp index 14257d78f9..6ab99f1bd6 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2160,7 +2160,7 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState & } if (fDoFullFlush || ((mode == FlushStateMode::ALWAYS || mode == FlushStateMode::PERIODIC) && nNow > nLastSetChain + (int64_t)DATABASE_WRITE_INTERVAL * 1000000)) { // Update best block in wallet (so we can detect restored wallets). - GetMainSignals().SetBestChain(chainActive.GetLocator()); + GetMainSignals().ChainStateFlushed(chainActive.GetLocator()); nLastSetChain = nNow; } } catch (const std::runtime_error& e) { diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 746263f113..f328d2d14b 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -25,7 +25,7 @@ struct MainSignalsInstance { boost::signals2::signal &, const CBlockIndex *pindex, const std::vector&)> BlockConnected; boost::signals2::signal &)> BlockDisconnected; boost::signals2::signal TransactionRemovedFromMempool; - boost::signals2::signal SetBestChain; + boost::signals2::signal ChainStateFlushed; boost::signals2::signal Inventory; boost::signals2::signal Broadcast; boost::signals2::signal BlockChecked; @@ -80,7 +80,7 @@ void RegisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.m_internals->BlockConnected.connect(boost::bind(&CValidationInterface::BlockConnected, pwalletIn, _1, _2, _3)); g_signals.m_internals->BlockDisconnected.connect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1)); g_signals.m_internals->TransactionRemovedFromMempool.connect(boost::bind(&CValidationInterface::TransactionRemovedFromMempool, pwalletIn, _1)); - g_signals.m_internals->SetBestChain.connect(boost::bind(&CValidationInterface::SetBestChain, pwalletIn, _1)); + g_signals.m_internals->ChainStateFlushed.connect(boost::bind(&CValidationInterface::ChainStateFlushed, pwalletIn, _1)); g_signals.m_internals->Inventory.connect(boost::bind(&CValidationInterface::Inventory, pwalletIn, _1)); g_signals.m_internals->Broadcast.connect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2)); g_signals.m_internals->BlockChecked.connect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2)); @@ -91,7 +91,7 @@ void UnregisterValidationInterface(CValidationInterface* pwalletIn) { g_signals.m_internals->BlockChecked.disconnect(boost::bind(&CValidationInterface::BlockChecked, pwalletIn, _1, _2)); g_signals.m_internals->Broadcast.disconnect(boost::bind(&CValidationInterface::ResendWalletTransactions, pwalletIn, _1, _2)); 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->ChainStateFlushed.disconnect(boost::bind(&CValidationInterface::ChainStateFlushed, 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->BlockDisconnected.disconnect(boost::bind(&CValidationInterface::BlockDisconnected, pwalletIn, _1)); @@ -107,7 +107,7 @@ void UnregisterAllValidationInterfaces() { g_signals.m_internals->BlockChecked.disconnect_all_slots(); g_signals.m_internals->Broadcast.disconnect_all_slots(); g_signals.m_internals->Inventory.disconnect_all_slots(); - g_signals.m_internals->SetBestChain.disconnect_all_slots(); + g_signals.m_internals->ChainStateFlushed.disconnect_all_slots(); g_signals.m_internals->TransactionAddedToMempool.disconnect_all_slots(); g_signals.m_internals->BlockConnected.disconnect_all_slots(); g_signals.m_internals->BlockDisconnected.disconnect_all_slots(); @@ -166,9 +166,9 @@ void CMainSignals::BlockDisconnected(const std::shared_ptr &pblock }); } -void CMainSignals::SetBestChain(const CBlockLocator &locator) { +void CMainSignals::ChainStateFlushed(const CBlockLocator &locator) { m_internals->m_schedulerClient.AddToProcessQueue([locator, this] { - m_internals->SetBestChain(locator); + m_internals->ChainStateFlushed(locator); }); } diff --git a/src/validationinterface.h b/src/validationinterface.h index 63097166af..6089bc2b3c 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -101,7 +101,7 @@ protected: * * Called on a background thread. */ - virtual void SetBestChain(const CBlockLocator &locator) {} + virtual void ChainStateFlushed(const CBlockLocator &locator) {} /** * Notifies listeners about an inventory item being seen on the network. * @@ -157,7 +157,7 @@ public: void TransactionAddedToMempool(const CTransactionRef &); void BlockConnected(const std::shared_ptr &, const CBlockIndex *pindex, const std::shared_ptr> &); void BlockDisconnected(const std::shared_ptr &); - void SetBestChain(const CBlockLocator &); + void ChainStateFlushed(const CBlockLocator &); void Inventory(const uint256 &); void Broadcast(int64_t nBestBlockTime, CConnman* connman); void BlockChecked(const CBlock&, const CValidationState&); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index ad3dd4cd2c..123252fdce 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -447,7 +447,7 @@ bool CWallet::ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, return false; } -void CWallet::SetBestChain(const CBlockLocator& loc) +void CWallet::ChainStateFlushed(const CBlockLocator& loc) { WalletBatch batch(*database); batch.WriteBestBlock(loc); @@ -4032,7 +4032,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& return nullptr; } - walletInstance->SetBestChain(chainActive.GetLocator()); + walletInstance->ChainStateFlushed(chainActive.GetLocator()); } else if (gArgs.IsArgSet("-usehd")) { bool useHD = gArgs.GetBoolArg("-usehd", true); if (walletInstance->IsHDEnabled() && !useHD) { @@ -4174,7 +4174,7 @@ CWallet* CWallet::CreateWalletFromFile(const std::string& name, const fs::path& walletInstance->ScanForWalletTransactions(pindexRescan, nullptr, reserver, true); } LogPrintf(" rescan %15dms\n", GetTimeMillis() - nStart); - walletInstance->SetBestChain(chainActive.GetLocator()); + walletInstance->ChainStateFlushed(chainActive.GetLocator()); walletInstance->database->IncrementUpdateCounter(); // Restore wallet transaction metadata after -zapwallettxes=1 diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 780c82ac36..d24ae1186f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1013,7 +1013,7 @@ public: bool IsAllFromMe(const CTransaction& tx, const isminefilter& filter) const; CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const; CAmount GetChange(const CTransaction& tx) const; - void SetBestChain(const CBlockLocator& loc) override; + void ChainStateFlushed(const CBlockLocator& loc) override; DBErrors LoadWallet(bool& fFirstRunRet); DBErrors ZapWalletTx(std::vector& vWtx); -- cgit v1.2.3 From 9cb6cdc59f9eb826b70ebbb6353a5bcee74996e3 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 27 Apr 2018 14:08:39 -0400 Subject: Simplify semantics of ChainStateFlushed callback Previously, ChainStateFlushed would fire either if a full flush completed (which can happen due to memory limits, forced flush, or on its own DATABASE_WRITE_INTERVAL timer) *or* on a ChainStateFlushed-specific DATABASE_WRITE_INTERVAL timer. This is both less clear for clients (as there are no guarantees about a flush having actually happened prior to the call), and reults in extra flushes not clearly intended by the code. We drop the second case, providing a strong guarantee without removing the periodit timer-based flushing. --- src/validation.cpp | 16 ++++++---------- src/validationinterface.h | 11 +++++++++++ 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 6ab99f1bd6..1b9e982753 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2066,13 +2066,12 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState & LOCK(cs_main); static int64_t nLastWrite = 0; static int64_t nLastFlush = 0; - static int64_t nLastSetChain = 0; std::set setFilesToPrune; - bool fFlushForPrune = false; - bool fDoFullFlush = false; - int64_t nNow = 0; + bool full_flush_completed = false; try { { + bool fFlushForPrune = false; + bool fDoFullFlush = false; LOCK(cs_LastBlockFile); if (fPruneMode && (fCheckForPruning || nManualPruneHeight > 0) && !fReindex) { if (nManualPruneHeight > 0) { @@ -2089,7 +2088,7 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState & } } } - nNow = GetTimeMicros(); + int64_t nNow = GetTimeMicros(); // Avoid writing/flushing immediately after startup. if (nLastWrite == 0) { nLastWrite = nNow; @@ -2097,9 +2096,6 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState & if (nLastFlush == 0) { nLastFlush = nNow; } - if (nLastSetChain == 0) { - nLastSetChain = nNow; - } int64_t nMempoolSizeMax = gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; int64_t cacheSize = pcoinsTip->DynamicMemoryUsage(); int64_t nTotalSpace = nCoinCacheUsage + std::max(nMempoolSizeMax - nMempoolUsage, 0); @@ -2156,12 +2152,12 @@ bool static FlushStateToDisk(const CChainParams& chainparams, CValidationState & if (!pcoinsTip->Flush()) return AbortNode(state, "Failed to write to coin database"); nLastFlush = nNow; + full_flush_completed = true; } } - if (fDoFullFlush || ((mode == FlushStateMode::ALWAYS || mode == FlushStateMode::PERIODIC) && nNow > nLastSetChain + (int64_t)DATABASE_WRITE_INTERVAL * 1000000)) { + if (full_flush_completed) { // Update best block in wallet (so we can detect restored wallets). GetMainSignals().ChainStateFlushed(chainActive.GetLocator()); - nLastSetChain = nNow; } } catch (const std::runtime_error& e) { return AbortNode(state, std::string("System error while flushing: ") + e.what()); diff --git a/src/validationinterface.h b/src/validationinterface.h index 6089bc2b3c..3a5fed0106 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -99,6 +99,17 @@ protected: /** * Notifies listeners of the new active block chain on-disk. * + * Prior to this callback, any updates are not guaranteed to persist on disk + * (ie clients need to handle shutdown/restart safety by being able to + * understand when some updates were lost due to unclean shutdown). + * + * When this callback is invoked, the validation changes done by any prior + * callback are guaranteed to exist on disk and survive a restart, including + * an unclean shutdown. + * + * Provides a locator describing the best chain, which is likely useful for + * storing current state on disk in client DBs. + * * Called on a background thread. */ virtual void ChainStateFlushed(const CBlockLocator &locator) {} -- cgit v1.2.3