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

a0d0f1c6c3 refactor: Remove Node:: queries from GUI (Hennadii Stepanov)
06d519f0b4 qt: Add SynchronizationState enum to signal parameter (Hennadii Stepanov)
3c709aa69d refactor: Remove Node::getReindex() call from GUI (Hennadii Stepanov)
1dab574edf refactor: Pass SynchronizationState enum to GUI (Hennadii Stepanov)
2bec309ad6 refactor: Remove unused bool parameter in RPCNotifyBlockChange() (Hennadii Stepanov)
1df77014d8 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 a0d0f1c6c3 (modulo [question](https://github.com/bitcoin/bitcoin/pull/18152#pullrequestreview-414140601)).
  ryanofsky:
    Code review ACK a0d0f1c6c3. 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
commit d44dd51322
No known key found for this signature in database
GPG key ID: 1EB776BB03C7922D
14 changed files with 73 additions and 50 deletions

View file

@ -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>
@ -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");
}
@ -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", "");
@ -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) {
{
@ -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. "
@ -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;
}

View file

@ -308,16 +308,16 @@ public:
}
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);
}));
}

View file

@ -27,6 +27,7 @@ class Coin;
class RPCTimerInterface;
class UniValue;
class proxyType;
enum class SynchronizationState;
enum class WalletCreationStatus;
struct CNodeStateStats;
struct NodeContext;
@ -249,12 +250,12 @@ public:
//! 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.

View file

@ -34,6 +34,7 @@
#include <uint256.h>
#include <util/system.h>
#include <util/threadnames.h>
#include <validation.h>
#include <memory>
@ -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()
@ -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

View file

@ -37,6 +37,7 @@
#include <ui_interface.h>
#include <util/system.h>
#include <util/translation.h>
#include <validation.h>
#include <QAction>
#include <QApplication>
@ -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
@ -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) {
m_app_nap_inhibitor->enableAppNap();
} else {
m_app_nap_inhibitor->disableAppNap();
}
#endif
if (modalOverlay)

View file

@ -38,6 +38,7 @@ class WalletFrame;
class WalletModel;
class HelpMessageDialog;
class ModalOverlay;
enum class SynchronizationState;
namespace interfaces {
class Handler;
@ -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

View file

@ -15,6 +15,7 @@
#include <net.h>
#include <netbase.h>
#include <util/system.h>
#include <validation.h>
#include <stdint.h>
@ -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;
@ -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()

View file

@ -12,10 +12,10 @@
#include <memory>
class BanTableModel;
class CBlockIndex;
class OptionsModel;
class PeerTableModel;
class CBlockIndex;
enum class SynchronizationState;
namespace interfaces {
class Handler;
@ -100,7 +100,7 @@ private:
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);

View file

@ -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);

View file

@ -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);

View file

@ -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)

View file

@ -11,6 +11,7 @@
#include <string>
class CBlockIndex;
enum class SynchronizationState;
struct bilingual_str;
namespace boost {
@ -110,10 +111,10 @@ 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*);
/** 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);

View file

@ -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;
@ -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;
}
@ -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).
@ -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;
}

View file

@ -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;