Merge #18152: qt: Use SynchronizationState enum for signals to GUI

a0d0f1c6c3d736bc0ee076b7f27a0ef59fd260bc refactor: Remove Node:: queries from GUI (Hennadii Stepanov)
06d519f0b43ed16252428e935d3aeb5a38f582e0 qt: Add SynchronizationState enum to signal parameter (Hennadii Stepanov)
3c709aa69d5bb5a1564c339a0e6a16bac8f02c98 refactor: Remove Node::getReindex() call from GUI (Hennadii Stepanov)
1dab574edf57ccd6cdf5ec706ac328c62142d7a2 refactor: Pass SynchronizationState enum to GUI (Hennadii Stepanov)
2bec309ad6d0f2543948d64ed26f7d9a903f67e5 refactor: Remove unused bool parameter in RPCNotifyBlockChange() (Hennadii Stepanov)
1df77014d8bb733d7d89e36b28671cb47f436292 refactor: Remove unused bool parameter in BlockNotifyGenesisWait() (Hennadii Stepanov)

Pull request description:

  This PR is a followup of #18121 and:
  - addresses confusion about GUI notification throttling conditions (**luke-jr**'s [comment](https://github.com/bitcoin/bitcoin/pull/18121#discussion_r378552386), **ryanofsky**'s [comment](https://github.com/bitcoin/bitcoin/pull/18121#discussion_r378975960))
  - removes `isInitialBlockDownload()` call from the GUI back to the node (on macOS). See:  **ryanofsky**'s [comment](https://github.com/bitcoin/bitcoin/pull/18121#pullrequestreview-357730284)

ACKs for top commit:
  jonasschnelli:
    Core Review ACK a0d0f1c6c3d736bc0ee076b7f27a0ef59fd260bc (modulo [question](https://github.com/bitcoin/bitcoin/pull/18152#pullrequestreview-414140601)).
  ryanofsky:
    Code review ACK a0d0f1c6c3d736bc0ee076b7f27a0ef59fd260bc. Only changes since last review were rebase and tweaking SynchronizationState enum declaration as suggested (thanks!)

Tree-SHA512: b6a712a710666e763aeee0d5440de1391a4c6c8f7fa661888773e1ba59e9e0f83654ee384d4edc704031be7eb25616e5eca2a6e26058d3efb7f64c47f9ed7316
This commit is contained in:
Jonas Schnelli 2020-05-19 14:34:54 +02:00 committed by pasta
parent 95fad525e9
commit 5758a3368d
No known key found for this signature in database
GPG Key ID: 52527BEDABE87984
14 changed files with 73 additions and 55 deletions

View File

@ -104,11 +104,12 @@
#include <statsd_client.h>
#include <functional>
#include <set>
#include <stdint.h>
#include <stdio.h>
#include <memory>
#include <optional>
#include <set>
#include <thread>
#include <vector>
@ -458,13 +459,13 @@ static void registerSignalHandler(int signal, void(*handler)(int))
static boost::signals2::connection rpc_notify_block_change_connection;
static void OnRPCStarted()
{
rpc_notify_block_change_connection = uiInterface.NotifyBlockTip_connect(&RPCNotifyBlockChange);
rpc_notify_block_change_connection = uiInterface.NotifyBlockTip_connect(std::bind(RPCNotifyBlockChange, std::placeholders::_2));
}
static void OnRPCStopped()
{
rpc_notify_block_change_connection.disconnect();
RPCNotifyBlockChange(false, nullptr);
RPCNotifyBlockChange(nullptr);
g_best_block_cv.notify_all();
LogPrint(BCLog::RPC, "RPC stopped.\n");
}
@ -815,7 +816,7 @@ static bool fHaveGenesis = false;
static Mutex g_genesis_wait_mutex;
static std::condition_variable g_genesis_wait_cv;
static void BlockNotifyGenesisWait(bool, const CBlockIndex *pBlockIndex)
static void BlockNotifyGenesisWait(const CBlockIndex* pBlockIndex)
{
if (pBlockIndex != nullptr) {
{
@ -2108,8 +2109,8 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
MIN_BLOCKS_TO_KEEP);
}
CBlockIndex* tip = chainstate->m_chain.Tip();
RPCNotifyBlockChange(true, tip);
const CBlockIndex* tip = chainstate->m_chain.Tip();
RPCNotifyBlockChange(tip);
if (tip && tip->nTime > GetAdjustedTime() + 2 * 60 * 60) {
strLoadError = _("The block database contains a block which appears to be from the future. "
"This may be due to your computer's date and time being set incorrectly. "
@ -2345,7 +2346,7 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
// No locking, as this happens before any background thread is started.
boost::signals2::connection block_notify_genesis_wait_connection;
if (::ChainActive().Tip() == nullptr) {
block_notify_genesis_wait_connection = uiInterface.NotifyBlockTip_connect(BlockNotifyGenesisWait);
block_notify_genesis_wait_connection = uiInterface.NotifyBlockTip_connect(std::bind(BlockNotifyGenesisWait, std::placeholders::_2));
} else {
fHaveGenesis = true;
}
@ -2353,8 +2354,8 @@ bool AppInitMain(const CoreContext& context, NodeContext& node, interfaces::Bloc
#if HAVE_SYSTEM
if (args.IsArgSet("-blocknotify")) {
const std::string block_notify = args.GetArg("-blocknotify", "");
const auto BlockNotifyCallback = [block_notify](bool initialSync, const CBlockIndex* pBlockIndex) {
if (initialSync || !pBlockIndex)
const auto BlockNotifyCallback = [block_notify](SynchronizationState sync_state, const CBlockIndex* pBlockIndex) {
if (sync_state != SynchronizationState::POST_INIT || !pBlockIndex)
return;
std::string strCmd = block_notify;

View File

@ -347,7 +347,7 @@ public:
//! Register handler for block tip messages.
using NotifyBlockTipFn =
std::function<void(bool initial_download, interfaces::BlockTip tip, double verification_progress)>;
std::function<void(SynchronizationState, interfaces::BlockTip tip, double verification_progress)>;
virtual std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) = 0;
//! Register handler for chainlock messages.
@ -357,7 +357,7 @@ public:
//! Register handler for header tip messages.
using NotifyHeaderTipFn =
std::function<void(bool initial_download, interfaces::BlockTip tip, double verification_progress)>;
std::function<void(SynchronizationState, interfaces::BlockTip tip, double verification_progress)>;
virtual std::unique_ptr<Handler> handleNotifyHeaderTip(NotifyHeaderTipFn fn) = 0;
//! Register handler for masternode list update messages.

View File

@ -488,8 +488,8 @@ public:
}
std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) override
{
return MakeHandler(::uiInterface.NotifyBlockTip_connect([fn](bool initial_download, const CBlockIndex* block) {
fn(initial_download, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()},
return MakeHandler(::uiInterface.NotifyBlockTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) {
fn(sync_state, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()},
GuessVerificationProgress(Params().TxData(), block));
}));
}
@ -502,8 +502,8 @@ public:
std::unique_ptr<Handler> handleNotifyHeaderTip(NotifyHeaderTipFn fn) override
{
return MakeHandler(
::uiInterface.NotifyHeaderTip_connect([fn](bool initial_download, const CBlockIndex* block) {
fn(initial_download, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()},
::uiInterface.NotifyHeaderTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) {
fn(sync_state, BlockTip{block->nHeight, block->GetBlockTime(), block->GetBlockHash()},
/* verification progress is unused when a header was received */ 0);
}));
}

View File

@ -55,9 +55,9 @@ void CClientUIInterface::NotifyNumConnectionsChanged(int newNumConnections) { re
void CClientUIInterface::NotifyNetworkActiveChanged(bool networkActive) { return g_ui_signals.NotifyNetworkActiveChanged(networkActive); }
void CClientUIInterface::NotifyAlertChanged() { return g_ui_signals.NotifyAlertChanged(); }
void CClientUIInterface::ShowProgress(const std::string& title, int nProgress, bool resume_possible) { return g_ui_signals.ShowProgress(title, nProgress, resume_possible); }
void CClientUIInterface::NotifyBlockTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyBlockTip(b, i); }
void CClientUIInterface::NotifyBlockTip(SynchronizationState s, const CBlockIndex* i) { return g_ui_signals.NotifyBlockTip(s, i); }
void CClientUIInterface::NotifyChainLock(const std::string& bestChainLockHash, int bestChainLockHeight) { return g_ui_signals.NotifyChainLock(bestChainLockHash, bestChainLockHeight); }
void CClientUIInterface::NotifyHeaderTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyHeaderTip(b, i); }
void CClientUIInterface::NotifyHeaderTip(SynchronizationState s, const CBlockIndex* i) { return g_ui_signals.NotifyHeaderTip(s, i); }
void CClientUIInterface::NotifyMasternodeListChanged(const CDeterministicMNList& list, const CBlockIndex* i) { return g_ui_signals.NotifyMasternodeListChanged(list, i); }
void CClientUIInterface::NotifyAdditionalDataSyncProgressChanged(double nSyncProgress) { return g_ui_signals.NotifyAdditionalDataSyncProgressChanged(nSyncProgress); }
void CClientUIInterface::BannedListChanged() { return g_ui_signals.BannedListChanged(); }

View File

@ -11,6 +11,7 @@
#include <string>
class CBlockIndex;
enum class SynchronizationState;
struct bilingual_str;
class CDeterministicMNList;
@ -101,13 +102,13 @@ public:
ADD_SIGNALS_DECL_WRAPPER(ShowProgress, void, const std::string& title, int nProgress, bool resume_possible);
/** New block has been accepted */
ADD_SIGNALS_DECL_WRAPPER(NotifyBlockTip, void, bool, const CBlockIndex*);
ADD_SIGNALS_DECL_WRAPPER(NotifyBlockTip, void, SynchronizationState, const CBlockIndex*);
/** New chainlock block has been accepted */
ADD_SIGNALS_DECL_WRAPPER(NotifyChainLock, void, const std::string& bestChainLockHash, int bestChainLockHeight);
/** Best header has changed */
ADD_SIGNALS_DECL_WRAPPER(NotifyHeaderTip, void, bool, const CBlockIndex*);
ADD_SIGNALS_DECL_WRAPPER(NotifyHeaderTip, void, SynchronizationState, const CBlockIndex*);
/** Masternode list has changed */
ADD_SIGNALS_DECL_WRAPPER(NotifyMasternodeListChanged, void, const CDeterministicMNList&, const CBlockIndex*);

View File

@ -38,6 +38,7 @@
#include <util/system.h>
#include <util/threadnames.h>
#include <util/translation.h>
#include <validation.h>
#include <memory>
@ -67,6 +68,7 @@ Q_IMPORT_PLUGIN(QMacStylePlugin);
// Declare meta types used for QMetaObject::invokeMethod
Q_DECLARE_METATYPE(bool*)
Q_DECLARE_METATYPE(CAmount)
Q_DECLARE_METATYPE(SynchronizationState)
Q_DECLARE_METATYPE(uint256)
static QString GetLangTerritory()
@ -483,6 +485,7 @@ int GuiMain(int argc, char* argv[])
// Register meta types used for QMetaObject::invokeMethod and Qt::QueuedConnection
qRegisterMetaType<bool*>();
qRegisterMetaType<SynchronizationState>();
#ifdef ENABLE_WALLET
qRegisterMetaType<WalletModel*>();
#endif

View File

@ -41,6 +41,7 @@
#include <qt/masternodelist.h>
#include <util/system.h>
#include <util/translation.h>
#include <validation.h>
#include <QAction>
#include <QApplication>
@ -809,7 +810,7 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel, interfaces::BlockAndH
connect(_clientModel, &ClientModel::networkActiveChanged, this, &BitcoinGUI::setNetworkActive);
modalOverlay->setKnownBestHeight(tip_info->header_height, QDateTime::fromTime_t(tip_info->header_time));
setNumBlocks(tip_info->block_height, QDateTime::fromTime_t(tip_info->block_time), QString::fromStdString(tip_info->block_hash.ToString()), tip_info->verification_progress, false);
setNumBlocks(tip_info->block_height, QDateTime::fromTime_t(tip_info->block_time), QString::fromStdString(tip_info->block_hash.ToString()), tip_info->verification_progress, false, SynchronizationState::INIT_DOWNLOAD);
connect(_clientModel, &ClientModel::numBlocksChanged, this, &BitcoinGUI::setNumBlocks);
connect(_clientModel, &ClientModel::additionalDataSyncProgressChanged, this, &BitcoinGUI::setAdditionalDataSyncProgress);
@ -1254,7 +1255,7 @@ void BitcoinGUI::updateNetworkState()
}
if (fNetworkBecameActive || fNetworkBecameInactive) {
setNumBlocks(m_node.getNumBlocks(), QDateTime::fromTime_t(m_node.getLastBlockTime()), QString::fromStdString(m_node.getLastBlockHash()), m_node.getVerificationProgress(), false);
setNumBlocks(m_node.getNumBlocks(), QDateTime::fromTime_t(m_node.getLastBlockTime()), QString::fromStdString(m_node.getLastBlockHash()), m_node.getVerificationProgress(), false, SynchronizationState::INIT_DOWNLOAD);
}
nCountPrev = count;
@ -1372,11 +1373,11 @@ void BitcoinGUI::updateWidth()
resize(nWidth, height());
}
void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, const QString& blockHash, double nVerificationProgress, bool header)
void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, const QString& blockHash, double nVerificationProgress, bool header, SynchronizationState sync_state)
{
#ifdef Q_OS_MAC
// Disabling macOS App Nap on initial sync, disk, reindex operations and mixing.
bool disableAppNap = !m_node.masternodeSync().isSynced();
bool disableAppNap = !m_node.masternodeSync().isSynced() || sync_state != SynchronizationState::POST_INIT;
#ifdef ENABLE_WALLET
if (enableWallet) {
for (const auto& wallet : m_node.walletLoader().getWallets()) {
@ -1495,7 +1496,7 @@ void BitcoinGUI::setAdditionalDataSyncProgress(double nSyncProgress)
// If masternodeSync->Reset() has been called make sure status bar shows the correct information.
if (nSyncProgress == -1) {
setNumBlocks(m_node.getNumBlocks(), QDateTime::fromTime_t(m_node.getLastBlockTime()), QString::fromStdString(m_node.getLastBlockHash()), m_node.getVerificationProgress(), false);
setNumBlocks(m_node.getNumBlocks(), QDateTime::fromTime_t(m_node.getLastBlockTime()), QString::fromStdString(m_node.getLastBlockHash()), m_node.getVerificationProgress(), false, SynchronizationState::INIT_DOWNLOAD);
if (clientModel->getNumConnections()) {
labelBlocksIcon->show();
startSpinner();

View File

@ -38,6 +38,7 @@ class WalletFrame;
class WalletModel;
class HelpMessageDialog;
class ModalOverlay;
enum class SynchronizationState;
namespace interfaces {
class Handler;
@ -266,7 +267,7 @@ public Q_SLOTS:
/** Get restart command-line parameters and request restart */
void handleRestart(QStringList args);
/** Set number of blocks and last block date shown in the UI */
void setNumBlocks(int count, const QDateTime& blockDate, const QString& blockHash, double nVerificationProgress, bool headers);
void setNumBlocks(int count, const QDateTime& blockDate, const QString& blockHash, double nVerificationProgress, bool headers, SynchronizationState sync_state);
/** Set additional data sync status shown in the UI */
void setAdditionalDataSyncProgress(double nSyncProgress);

View File

@ -19,6 +19,7 @@
#include <net.h>
#include <netbase.h>
#include <util/system.h>
#include <validation.h>
#include <stdint.h>
#include <functional>
@ -293,17 +294,8 @@ static void BannedListChanged(ClientModel *clientmodel)
assert(invoked);
}
static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, interfaces::BlockTip tip, double verificationProgress, bool fHeader)
static void BlockTipChanged(ClientModel* clientmodel, SynchronizationState sync_state, interfaces::BlockTip tip, double verificationProgress, bool fHeader)
{
// lock free async UI updates in case we have a new block tip
// during initial sync, only update the UI if the last update
// was > 250ms (MODEL_UPDATE_DELAY) ago
int64_t now = 0;
if (initialSync)
now = GetTimeMillis();
int64_t& nLastUpdateNotification = fHeader ? nLastHeaderTipUpdateNotification : nLastBlockTipUpdateNotification;
if (fHeader) {
// cache best headers time and height to reduce future cs_main locks
clientmodel->cachedBestHeaderHeight = tip.block_height;
@ -313,18 +305,23 @@ static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, interfac
WITH_LOCK(clientmodel->m_cached_tip_mutex, clientmodel->m_cached_tip_blocks = tip.block_hash;);
}
// During initial sync, block notifications, and header notifications from reindexing are both throttled.
if (!initialSync || (fHeader && !clientmodel->node().getReindex()) || now - nLastUpdateNotification > MODEL_UPDATE_DELAY) {
//pass an async signal to the UI thread
// Throttle GUI notifications about (a) blocks during initial sync, and (b) both blocks and headers during reindex.
const bool throttle = (sync_state != SynchronizationState::POST_INIT && !fHeader) || sync_state == SynchronizationState::INIT_REINDEX;
const int64_t now = throttle ? GetTimeMillis() : 0;
int64_t& nLastUpdateNotification = fHeader ? nLastHeaderTipUpdateNotification : nLastBlockTipUpdateNotification;
if (throttle && now < nLastUpdateNotification + MODEL_UPDATE_DELAY) {
return;
}
bool invoked = QMetaObject::invokeMethod(clientmodel, "numBlocksChanged", Qt::QueuedConnection,
Q_ARG(int, tip.block_height),
Q_ARG(QDateTime, QDateTime::fromTime_t(tip.block_time)),
Q_ARG(QString, QString::fromStdString(tip.block_hash.ToString())),
Q_ARG(double, verificationProgress),
Q_ARG(bool, fHeader));
Q_ARG(bool, fHeader),
Q_ARG(SynchronizationState, sync_state));
assert(invoked);
nLastUpdateNotification = now;
}
}
static void NotifyChainLock(ClientModel *clientmodel, const std::string& bestChainLockHash, int bestChainLockHeight)

View File

@ -17,10 +17,10 @@
#include <uint256.h>
class BanTableModel;
class CBlockIndex;
class OptionsModel;
class PeerTableModel;
class CBlockIndex;
enum class SynchronizationState;
QT_BEGIN_NAMESPACE
class QTimer;
@ -128,7 +128,7 @@ Q_SIGNALS:
void numConnectionsChanged(int count);
void masternodeListChanged() const;
void chainLockChanged(const QString& bestChainLockHash, int bestChainLockHeight);
void numBlocksChanged(int count, const QDateTime& blockDate, const QString& blockHash, double nVerificationProgress, bool header);
void numBlocksChanged(int count, const QDateTime& blockDate, const QString& blockHash, double nVerificationProgress, bool header, SynchronizationState sync_state);
void additionalDataSyncProgressChanged(double nSyncProgress);
void mempoolSizeChanged(long count, size_t mempoolSizeInBytes);
void islockCountChanged(size_t count);

View File

@ -340,7 +340,7 @@ static UniValue getbestchainlock(const JSONRPCRequest& request)
return result;
}
void RPCNotifyBlockChange(bool ibd, const CBlockIndex * pindex)
void RPCNotifyBlockChange(const CBlockIndex* pindex)
{
if(pindex) {
LOCK(cs_blockchange);

View File

@ -40,7 +40,7 @@ static constexpr int NUM_GETBLOCKSTATS_PERCENTILES = 5;
double GetDifficulty(const CBlockIndex* blockindex);
/** Callback for when block tip changed. */
void RPCNotifyBlockChange(bool ibd, const CBlockIndex *);
void RPCNotifyBlockChange(const CBlockIndex*);
/** Block description to JSON */
UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, llmq::CChainLocksHandler& clhandler, llmq::CInstantSendManager& isman, bool txDetails = false) LOCKS_EXCLUDED(cs_main);

View File

@ -3065,6 +3065,13 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, CBlockIndex
return true;
}
static SynchronizationState GetSynchronizationState(bool init)
{
if (!init) return SynchronizationState::POST_INIT;
if (::fReindex) return SynchronizationState::INIT_REINDEX;
return SynchronizationState::INIT_DOWNLOAD;
}
static bool NotifyHeaderTip(CChainState& chainstate) LOCKS_EXCLUDED(cs_main) {
bool fNotify = false;
bool fInitialBlockDownload = false;
@ -3083,7 +3090,7 @@ static bool NotifyHeaderTip(CChainState& chainstate) LOCKS_EXCLUDED(cs_main) {
}
// Send block tip changed notifications without cs_main
if (fNotify) {
uiInterface.NotifyHeaderTip(fInitialBlockDownload, pindexHeader);
uiInterface.NotifyHeaderTip(GetSynchronizationState(fInitialBlockDownload), pindexHeader);
GetMainSignals().NotifyHeaderTip(pindexHeader, fInitialBlockDownload);
}
return fNotify;
@ -3177,7 +3184,7 @@ bool CChainState::ActivateBestChain(BlockValidationState& state, std::shared_ptr
GetMainSignals().UpdatedBlockTip(pindexNewTip, pindexFork, fInitialDownload);
// Always notify the UI if a new block tip was connected
uiInterface.NotifyBlockTip(fInitialDownload, pindexNewTip);
uiInterface.NotifyBlockTip(GetSynchronizationState(fInitialDownload), pindexNewTip);
}
}
// When we reach this point, we switched to a new tip (stored in pindexNewTip).
@ -3384,7 +3391,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind
// Only notify about a new block tip if the active chain was modified.
if (pindex_was_in_chain) {
uiInterface.NotifyBlockTip(IsInitialBlockDownload(), to_mark_failed->pprev);
uiInterface.NotifyBlockTip(GetSynchronizationState(IsInitialBlockDownload()), to_mark_failed->pprev);
}
return true;
}
@ -3487,7 +3494,7 @@ bool CChainState::MarkConflictingBlock(BlockValidationState& state, CBlockIndex
// Only notify about a new block tip if the active chain was modified.
if (pindex_was_in_chain) {
uiInterface.NotifyBlockTip(IsInitialBlockDownload(), pindex->pprev);
uiInterface.NotifyBlockTip(GetSynchronizationState(IsInitialBlockDownload()), pindex->pprev);
}
return true;
}

View File

@ -127,6 +127,13 @@ static const unsigned int DEFAULT_CHECKLEVEL = 3;
// Setting the target to > than 945 MiB will make it likely we can respect the target.
static const uint64_t MIN_DISK_SPACE_FOR_BLOCK_FILES = 945 * 1024 * 1024;
/** Current sync state passed to tip changed callbacks. */
enum class SynchronizationState {
INIT_REINDEX,
INIT_DOWNLOAD,
POST_INIT
};
extern RecursiveMutex cs_main;
typedef std::unordered_map<uint256, CBlockIndex*, BlockHasher> BlockMap;
typedef std::unordered_multimap<uint256, CBlockIndex*, BlockHasher> PrevBlockMap;