Skip to content

qt: Use SynchronizationState enum for signals to GUI #18152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 9 additions & 8 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@
#include <validationinterface.h>
#include <walletinitinterface.h>

#include <functional>
#include <set>
#include <stdint.h>
#include <stdio.h>
#include <set>

#ifndef WIN32
#include <attributes.h>
Expand Down Expand Up @@ -350,13 +351,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");
}
Expand Down Expand Up @@ -604,9 +605,9 @@ std::string LicenseInfo()
}

#if HAVE_SYSTEM
static void BlockNotifyCallback(bool initialSync, const CBlockIndex *pBlockIndex)
static void BlockNotifyCallback(SynchronizationState sync_state, const CBlockIndex* pBlockIndex)
{
if (initialSync || !pBlockIndex)
if (sync_state != SynchronizationState::POST_INIT || !pBlockIndex)
return;

std::string strCmd = gArgs.GetArg("-blocknotify", "");
Expand All @@ -622,7 +623,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) {
{
Expand Down Expand Up @@ -1701,7 +1702,7 @@ bool AppInitMain(NodeContext& node)
}

const CBlockIndex* tip = chainstate->m_chain.Tip();
RPCNotifyBlockChange(true, 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. "
Expand Down Expand Up @@ -1826,7 +1827,7 @@ bool AppInitMain(NodeContext& node)
// 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;
}
Expand Down
8 changes: 4 additions & 4 deletions src/interfaces/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,16 +308,16 @@ class NodeImpl : public Node
}
std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) override
{
return MakeHandler(::uiInterface.NotifyBlockTip_connect([fn](bool initial_download, const CBlockIndex* block) {
fn(initial_download, block->nHeight, block->GetBlockTime(),
return MakeHandler(::uiInterface.NotifyBlockTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) {
fn(sync_state, block->nHeight, block->GetBlockTime(),
GuessVerificationProgress(Params().TxData(), block));
}));
}
std::unique_ptr<Handler> handleNotifyHeaderTip(NotifyHeaderTipFn fn) override
{
return MakeHandler(
::uiInterface.NotifyHeaderTip_connect([fn](bool initial_download, const CBlockIndex* block) {
fn(initial_download, block->nHeight, block->GetBlockTime(),
::uiInterface.NotifyHeaderTip_connect([fn](SynchronizationState sync_state, const CBlockIndex* block) {
fn(sync_state, block->nHeight, block->GetBlockTime(),
/* verification progress is unused when a header was received */ 0);
}));
}
Expand Down
5 changes: 3 additions & 2 deletions src/interfaces/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class Coin;
class RPCTimerInterface;
class UniValue;
class proxyType;
enum class SynchronizationState;
enum class WalletCreationStatus;
struct CNodeStateStats;
struct NodeContext;
Expand Down Expand Up @@ -249,12 +250,12 @@ class Node

//! Register handler for block tip messages.
using NotifyBlockTipFn =
std::function<void(bool initial_download, int height, int64_t block_time, double verification_progress)>;
std::function<void(SynchronizationState, int height, int64_t block_time, double verification_progress)>;
virtual std::unique_ptr<Handler> handleNotifyBlockTip(NotifyBlockTipFn fn) = 0;

//! Register handler for header tip messages.
using NotifyHeaderTipFn =
std::function<void(bool initial_download, int height, int64_t block_time, double verification_progress)>;
std::function<void(SynchronizationState, int height, int64_t block_time, double verification_progress)>;
virtual std::unique_ptr<Handler> handleNotifyHeaderTip(NotifyHeaderTipFn fn) = 0;

//! Return pointer to internal chain interface, useful for testing.
Expand Down
3 changes: 3 additions & 0 deletions src/qt/bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <uint256.h>
#include <util/system.h>
#include <util/threadnames.h>
#include <validation.h>

#include <memory>

Expand Down Expand Up @@ -61,6 +62,7 @@ Q_IMPORT_PLUGIN(QCocoaIntegrationPlugin);
// 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()
Expand Down Expand Up @@ -435,6 +437,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
Expand Down
11 changes: 8 additions & 3 deletions src/qt/bitcoingui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <ui_interface.h>
#include <util/system.h>
#include <util/translation.h>
#include <validation.h>

#include <QAction>
#include <QApplication>
Expand Down Expand Up @@ -567,7 +568,7 @@ void BitcoinGUI::setClientModel(ClientModel *_clientModel)
connect(_clientModel, &ClientModel::networkActiveChanged, this, &BitcoinGUI::setNetworkActive);

modalOverlay->setKnownBestHeight(_clientModel->getHeaderTipHeight(), QDateTime::fromTime_t(_clientModel->getHeaderTipTime()));
setNumBlocks(m_node.getNumBlocks(), QDateTime::fromTime_t(m_node.getLastBlockTime()), m_node.getVerificationProgress(), false);
setNumBlocks(m_node.getNumBlocks(), QDateTime::fromTime_t(m_node.getLastBlockTime()), m_node.getVerificationProgress(), false, SynchronizationState::INIT_DOWNLOAD);
connect(_clientModel, &ClientModel::numBlocksChanged, this, &BitcoinGUI::setNumBlocks);

// Receive and report messages from client model
Expand Down Expand Up @@ -926,11 +927,15 @@ void BitcoinGUI::openOptionsDialogWithTab(OptionsDialog::Tab tab)
dlg.exec();
}

void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool header)
void BitcoinGUI::setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool header, SynchronizationState sync_state)
{
// Disabling macOS App Nap on initial sync, disk and reindex operations.
#ifdef Q_OS_MAC
(m_node.isInitialBlockDownload() || m_node.getReindex() || m_node.getImporting()) ? m_app_nap_inhibitor->disableAppNap() : m_app_nap_inhibitor->enableAppNap();
if (sync_state == SynchronizationState::POST_INIT) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We drop the check on fImporting here? Is that a problem?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the (m_node.isInitialBlockDownload() || m_node.getReindex() || m_node.getImporting()) expression all but the first operand are redundant as in CChainState::IsInitialBlockDownload() we have:

bitcoin/src/validation.cpp

Lines 1302 to 1303 in 362f9c6

if (fImporting || fReindex)
return true;

Right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. It was unnecessary in the first place as it looks.

m_app_nap_inhibitor->enableAppNap();
} else {
m_app_nap_inhibitor->disableAppNap();
}
#endif

if (modalOverlay)
Expand Down
3 changes: 2 additions & 1 deletion src/qt/bitcoingui.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class WalletFrame;
class WalletModel;
class HelpMessageDialog;
class ModalOverlay;
enum class SynchronizationState;

namespace interfaces {
class Handler;
Expand Down Expand Up @@ -213,7 +214,7 @@ public Q_SLOTS:
/** Set network state shown in the UI */
void setNetworkActive(bool networkActive);
/** Set number of blocks and last block date shown in the UI */
void setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool headers);
void setNumBlocks(int count, const QDateTime& blockDate, double nVerificationProgress, bool headers, SynchronizationState sync_state);

/** Notify the user of an event from the core network or transaction handling code.
@param[in] title the message box / notification title
Expand Down
37 changes: 17 additions & 20 deletions src/qt/clientmodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <net.h>
#include <netbase.h>
#include <util/system.h>
#include <validation.h>

#include <stdint.h>

Expand Down Expand Up @@ -234,17 +235,8 @@ static void BannedListChanged(ClientModel *clientmodel)
assert(invoked);
}

static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, int height, int64_t blockTime, double verificationProgress, bool fHeader)
static void BlockTipChanged(ClientModel* clientmodel, SynchronizationState sync_state, int height, int64_t blockTime, 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 = height;
Expand All @@ -253,17 +245,22 @@ static void BlockTipChanged(ClientModel *clientmodel, bool initialSync, int heig
clientmodel->m_cached_num_blocks = height;
}

// 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
bool invoked = QMetaObject::invokeMethod(clientmodel, "numBlocksChanged", Qt::QueuedConnection,
Q_ARG(int, height),
Q_ARG(QDateTime, QDateTime::fromTime_t(blockTime)),
Q_ARG(double, verificationProgress),
Q_ARG(bool, fHeader));
assert(invoked);
nLastUpdateNotification = now;
// 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, height),
Q_ARG(QDateTime, QDateTime::fromTime_t(blockTime)),
Q_ARG(double, verificationProgress),
Q_ARG(bool, fHeader),
Q_ARG(SynchronizationState, sync_state));
assert(invoked);
nLastUpdateNotification = now;
}

void ClientModel::subscribeToCoreSignals()
Expand Down
6 changes: 3 additions & 3 deletions src/qt/clientmodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
#include <memory>

class BanTableModel;
class CBlockIndex;
class OptionsModel;
class PeerTableModel;

class CBlockIndex;
enum class SynchronizationState;

namespace interfaces {
class Handler;
Expand Down Expand Up @@ -100,7 +100,7 @@ class ClientModel : public QObject

Q_SIGNALS:
void numConnectionsChanged(int count);
void numBlocksChanged(int count, const QDateTime& blockDate, double nVerificationProgress, bool header);
void numBlocksChanged(int count, const QDateTime& blockDate, double nVerificationProgress, bool header, SynchronizationState sync_state);
void mempoolSizeChanged(long count, size_t mempoolSizeInBytes);
void networkActiveChanged(bool networkActive);
void alertsChanged(const QString &warnings);
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/blockchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ static UniValue getbestblockhash(const JSONRPCRequest& request)
return ::ChainActive().Tip()->GetBlockHash().GetHex();
}

void RPCNotifyBlockChange(bool ibd, const CBlockIndex * pindex)
void RPCNotifyBlockChange(const CBlockIndex* pindex)
{
if(pindex) {
std::lock_guard<std::mutex> lock(cs_blockchange);
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/blockchain.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,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, bool txDetails = false) LOCKS_EXCLUDED(cs_main);
Expand Down
4 changes: 2 additions & 2 deletions src/ui_interface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ 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::NotifyHeaderTip(bool b, const CBlockIndex* i) { return g_ui_signals.NotifyHeaderTip(b, i); }
void CClientUIInterface::NotifyBlockTip(SynchronizationState s, const CBlockIndex* i) { return g_ui_signals.NotifyBlockTip(s, i); }
void CClientUIInterface::NotifyHeaderTip(SynchronizationState s, const CBlockIndex* i) { return g_ui_signals.NotifyHeaderTip(s, i); }
void CClientUIInterface::BannedListChanged() { return g_ui_signals.BannedListChanged(); }

bool InitError(const bilingual_str& str)
Expand Down
5 changes: 3 additions & 2 deletions src/ui_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <string>

class CBlockIndex;
enum class SynchronizationState;
struct bilingual_str;

namespace boost {
Expand Down Expand Up @@ -110,10 +111,10 @@ class CClientUIInterface
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*);

/** Best header has changed */
ADD_SIGNALS_DECL_WRAPPER(NotifyHeaderTip, void, bool, const CBlockIndex*);
ADD_SIGNALS_DECL_WRAPPER(NotifyHeaderTip, void, SynchronizationState, const CBlockIndex*);

/** Banlist did change. */
ADD_SIGNALS_DECL_WRAPPER(BannedListChanged, void, void);
Expand Down
13 changes: 10 additions & 3 deletions src/validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2800,6 +2800,13 @@ bool CChainState::ActivateBestChainStep(BlockValidationState& state, const CChai
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() LOCKS_EXCLUDED(cs_main) {
bool fNotify = false;
bool fInitialBlockDownload = false;
Expand All @@ -2817,7 +2824,7 @@ static bool NotifyHeaderTip() LOCKS_EXCLUDED(cs_main) {
}
// Send block tip changed notifications without cs_main
if (fNotify) {
uiInterface.NotifyHeaderTip(fInitialBlockDownload, pindexHeader);
uiInterface.NotifyHeaderTip(GetSynchronizationState(fInitialBlockDownload), pindexHeader);
}
return fNotify;
}
Expand Down Expand Up @@ -2906,7 +2913,7 @@ bool CChainState::ActivateBestChain(BlockValidationState &state, const CChainPar
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).
Expand Down Expand Up @@ -3097,7 +3104,7 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, const CChainParam

// 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;
}
Expand Down
7 changes: 7 additions & 0 deletions src/validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ struct BlockHasher
size_t operator()(const uint256& hash) const { return ReadLE64(hash.begin()); }
};

/** Current sync state passed to tip changed callbacks. */
enum class SynchronizationState {
INIT_REINDEX,
INIT_DOWNLOAD,
POST_INIT
};

extern RecursiveMutex cs_main;
extern CBlockPolicyEstimator feeEstimator;
extern CTxMemPool mempool;
Expand Down