Skip to content

Commit 8c61374

Browse files
hebastohebasto
committed
Merge bitcoin-core/gui#581: refactor: Revamp ClientModel code to handle core signals
bcbf982 qt, doc: Remove unneeded comments (Hennadii Stepanov) 9bd1565 qt: Revamp ClientModel code to handle {Block|Header}Tip core signals (Hennadii Stepanov) 48f6d39 qt: Revamp ClientModel code to handle BannedListChanged core signal (Hennadii Stepanov) 36b12af qt: Revamp ClientModel code to handle AlertChanged core signal (Hennadii Stepanov) bfe5140 qt: Revamp ClientModel code to handle NetworkActiveChanged core signal (Hennadii Stepanov) 639563d qt: Revamp ClientModel code to handle NumConnectionsChanged core signal (Hennadii Stepanov) 508e2dc qt: Revamp ClientModel code to handle ShowProgress core signal (Hennadii Stepanov) Pull request description: This PR: - is a pure refactoring with no behavior change - gets rid of `QMetaObject::invokeMethod()` "dynamic" calls, i.e., without compile-time checks of a called function name and its parameters - replaces `std::bind`s with lambdas, making parameter permutation (including parameter omitting) explicit - makes code simpler, more concise, and easier to reason about Additionally, debug messages have been unified. ACKs for top commit: promag: Code review ACK bcbf982 w0xlt: tACK bitcoin-core/gui@bcbf982 on Ubuntu 21.10, Qt 5.15.2. Tree-SHA512: 35f62b84f916b3ad7442f0fea945d344b3c448878b33506ac7b81fdf5e49bd2a82e12a6927dc91f62c335487bf2305cc45e2f08985303eef31c3ed2dd39e1037
2 parents 8118970 + bcbf982 commit 8c61374

File tree

2 files changed

+42
-90
lines changed

2 files changed

+42
-90
lines changed

‎src/qt/clientmodel.cpp

Lines changed: 40 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@
2121
#include <validation.h>
2222

2323
#include <stdint.h>
24-
#include <functional>
2524

2625
#include <QDebug>
26+
#include <QMetaObject>
2727
#include <QThread>
2828
#include <QTimer>
2929

@@ -148,21 +148,6 @@ uint256 ClientModel::getBestBlockHash()
148148
return m_cached_tip_blocks;
149149
}
150150

151-
void ClientModel::updateNumConnections(int numConnections)
152-
{
153-
Q_EMIT numConnectionsChanged(numConnections);
154-
}
155-
156-
void ClientModel::updateNetworkActive(bool networkActive)
157-
{
158-
Q_EMIT networkActiveChanged(networkActive);
159-
}
160-
161-
void ClientModel::updateAlert()
162-
{
163-
Q_EMIT alertsChanged(getStatusBarWarnings());
164-
}
165-
166151
enum BlockSource ClientModel::getBlockSource() const
167152
{
168153
if (m_node.getReindex())
@@ -230,94 +215,65 @@ QString ClientModel::blocksDir() const
230215
return GUIUtil::PathToQString(gArgs.GetBlocksDirPath());
231216
}
232217

233-
void ClientModel::updateBanlist()
234-
{
235-
banTableModel->refresh();
236-
}
237-
238-
// Handlers for core signals
239-
static void ShowProgress(ClientModel *clientmodel, const std::string &title, int nProgress)
240-
{
241-
// emits signal "showProgress"
242-
bool invoked = QMetaObject::invokeMethod(clientmodel, "showProgress", Qt::QueuedConnection,
243-
Q_ARG(QString, QString::fromStdString(title)),
244-
Q_ARG(int, nProgress));
245-
assert(invoked);
246-
}
247-
248-
static void NotifyNumConnectionsChanged(ClientModel *clientmodel, int newNumConnections)
249-
{
250-
// Too noisy: qDebug() << "NotifyNumConnectionsChanged: " + QString::number(newNumConnections);
251-
bool invoked = QMetaObject::invokeMethod(clientmodel, "updateNumConnections", Qt::QueuedConnection,
252-
Q_ARG(int, newNumConnections));
253-
assert(invoked);
254-
}
255-
256-
static void NotifyNetworkActiveChanged(ClientModel *clientmodel, bool networkActive)
257-
{
258-
bool invoked = QMetaObject::invokeMethod(clientmodel, "updateNetworkActive", Qt::QueuedConnection,
259-
Q_ARG(bool, networkActive));
260-
assert(invoked);
261-
}
262-
263-
static void NotifyAlertChanged(ClientModel *clientmodel)
264-
{
265-
qDebug() << "NotifyAlertChanged";
266-
bool invoked = QMetaObject::invokeMethod(clientmodel, "updateAlert", Qt::QueuedConnection);
267-
assert(invoked);
268-
}
269-
270-
static void BannedListChanged(ClientModel *clientmodel)
271-
{
272-
qDebug() << QString("%1: Requesting update for peer banlist").arg(__func__);
273-
bool invoked = QMetaObject::invokeMethod(clientmodel, "updateBanlist", Qt::QueuedConnection);
274-
assert(invoked);
275-
}
276-
277-
static void BlockTipChanged(ClientModel* clientmodel, SynchronizationState sync_state, interfaces::BlockTip tip, double verificationProgress, bool fHeader)
218+
void ClientModel::TipChanged(SynchronizationState sync_state, interfaces::BlockTip tip, double verification_progress, bool header)
278219
{
279-
if (fHeader) {
220+
if (header) {
280221
// cache best headers time and height to reduce future cs_main locks
281-
clientmodel->cachedBestHeaderHeight = tip.block_height;
282-
clientmodel->cachedBestHeaderTime = tip.block_time;
222+
cachedBestHeaderHeight = tip.block_height;
223+
cachedBestHeaderTime = tip.block_time;
283224
} else {
284-
clientmodel->m_cached_num_blocks = tip.block_height;
285-
WITH_LOCK(clientmodel->m_cached_tip_mutex, clientmodel->m_cached_tip_blocks = tip.block_hash;);
225+
m_cached_num_blocks = tip.block_height;
226+
WITH_LOCK(m_cached_tip_mutex, m_cached_tip_blocks = tip.block_hash;);
286227
}
287228

288229
// Throttle GUI notifications about (a) blocks during initial sync, and (b) both blocks and headers during reindex.
289-
const bool throttle = (sync_state != SynchronizationState::POST_INIT && !fHeader) || sync_state == SynchronizationState::INIT_REINDEX;
230+
const bool throttle = (sync_state != SynchronizationState::POST_INIT && !header) || sync_state == SynchronizationState::INIT_REINDEX;
290231
const int64_t now = throttle ? GetTimeMillis() : 0;
291-
int64_t& nLastUpdateNotification = fHeader ? nLastHeaderTipUpdateNotification : nLastBlockTipUpdateNotification;
232+
int64_t& nLastUpdateNotification = header ? nLastHeaderTipUpdateNotification : nLastBlockTipUpdateNotification;
292233
if (throttle && now < nLastUpdateNotification + count_milliseconds(MODEL_UPDATE_DELAY)) {
293234
return;
294235
}
295236

296-
bool invoked = QMetaObject::invokeMethod(clientmodel, "numBlocksChanged", Qt::QueuedConnection,
297-
Q_ARG(int, tip.block_height),
298-
Q_ARG(QDateTime, QDateTime::fromSecsSinceEpoch(tip.block_time)),
299-
Q_ARG(double, verificationProgress),
300-
Q_ARG(bool, fHeader),
301-
Q_ARG(SynchronizationState, sync_state));
302-
assert(invoked);
237+
Q_EMIT numBlocksChanged(tip.block_height, QDateTime::fromSecsSinceEpoch(tip.block_time), verification_progress, header, sync_state);
303238
nLastUpdateNotification = now;
304239
}
305240

306241
void ClientModel::subscribeToCoreSignals()
307242
{
308-
// Connect signals to client
309-
m_handler_show_progress = m_node.handleShowProgress(std::bind(ShowProgress, this, std::placeholders::_1, std::placeholders::_2));
310-
m_handler_notify_num_connections_changed = m_node.handleNotifyNumConnectionsChanged(std::bind(NotifyNumConnectionsChanged, this, std::placeholders::_1));
311-
m_handler_notify_network_active_changed = m_node.handleNotifyNetworkActiveChanged(std::bind(NotifyNetworkActiveChanged, this, std::placeholders::_1));
312-
m_handler_notify_alert_changed = m_node.handleNotifyAlertChanged(std::bind(NotifyAlertChanged, this));
313-
m_handler_banned_list_changed = m_node.handleBannedListChanged(std::bind(BannedListChanged, this));
314-
m_handler_notify_block_tip = m_node.handleNotifyBlockTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, false));
315-
m_handler_notify_header_tip = m_node.handleNotifyHeaderTip(std::bind(BlockTipChanged, this, std::placeholders::_1, std::placeholders::_2, std::placeholders::_3, true));
243+
m_handler_show_progress = m_node.handleShowProgress(
244+
[this](const std::string& title, int progress, [[maybe_unused]] bool resume_possible) {
245+
Q_EMIT showProgress(QString::fromStdString(title), progress);
246+
});
247+
m_handler_notify_num_connections_changed = m_node.handleNotifyNumConnectionsChanged(
248+
[this](int new_num_connections) {
249+
Q_EMIT numConnectionsChanged(new_num_connections);
250+
});
251+
m_handler_notify_network_active_changed = m_node.handleNotifyNetworkActiveChanged(
252+
[this](bool network_active) {
253+
Q_EMIT networkActiveChanged(network_active);
254+
});
255+
m_handler_notify_alert_changed = m_node.handleNotifyAlertChanged(
256+
[this]() {
257+
qDebug() << "ClientModel: NotifyAlertChanged";
258+
Q_EMIT alertsChanged(getStatusBarWarnings());
259+
});
260+
m_handler_banned_list_changed = m_node.handleBannedListChanged(
261+
[this]() {
262+
qDebug() << "ClienModel: Requesting update for peer banlist";
263+
QMetaObject::invokeMethod(banTableModel, [this] { banTableModel->refresh(); });
264+
});
265+
m_handler_notify_block_tip = m_node.handleNotifyBlockTip(
266+
[this](SynchronizationState sync_state, interfaces::BlockTip tip, double verification_progress) {
267+
TipChanged(sync_state, tip, verification_progress, /*header=*/false);
268+
});
269+
m_handler_notify_header_tip = m_node.handleNotifyHeaderTip(
270+
[this](SynchronizationState sync_state, interfaces::BlockTip tip, double verification_progress) {
271+
TipChanged(sync_state, tip, verification_progress, /*header=*/true);
272+
});
316273
}
317274

318275
void ClientModel::unsubscribeFromCoreSignals()
319276
{
320-
// Disconnect signals from client
321277
m_handler_show_progress->disconnect();
322278
m_handler_notify_num_connections_changed->disconnect();
323279
m_handler_notify_network_active_changed->disconnect();

‎src/qt/clientmodel.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ enum class SynchronizationState;
2323
namespace interfaces {
2424
class Handler;
2525
class Node;
26+
struct BlockTip;
2627
}
2728

2829
QT_BEGIN_NAMESPACE
@@ -104,6 +105,7 @@ class ClientModel : public QObject
104105
//! A thread to interact with m_node asynchronously
105106
QThread* const m_thread;
106107

108+
void TipChanged(SynchronizationState sync_state, interfaces::BlockTip tip, double verification_progress, bool header);
107109
void subscribeToCoreSignals();
108110
void unsubscribeFromCoreSignals();
109111

@@ -120,12 +122,6 @@ class ClientModel : public QObject
120122

121123
// Show progress dialog e.g. for verifychain
122124
void showProgress(const QString &title, int nProgress);
123-
124-
public Q_SLOTS:
125-
void updateNumConnections(int numConnections);
126-
void updateNetworkActive(bool networkActive);
127-
void updateAlert();
128-
void updateBanlist();
129125
};
130126

131127
#endif // BITCOIN_QT_CLIENTMODEL_H

0 commit comments

Comments
 (0)