Change all ping times to std::chrono types

sipa · dhruv · commit 4d98b401fbd8 · 2021-03-03T09:48:07.000-08:00
diff --git a/src/net.cpp b/src/net.cpp
@@ -603,8 +603,8 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
         stats.minFeeFilter = 0;
     }
 
-    stats.m_ping_usec = m_last_ping_time;
-    stats.m_min_ping_usec = m_min_ping_time;
+    X(m_last_ping_time);
+    X(m_min_ping_time);
 
     // Leave string empty if addrLocal invalid (not filled in yet)
     CService addrLocalUnlocked = GetAddrLocal();
diff --git a/src/net.h b/src/net.h
@@ -261,8 +261,8 @@ class CNodeStats
     uint64_t nRecvBytes;
     mapMsgCmdSize mapRecvBytesPerMsgCmd;
     NetPermissionFlags m_permissionFlags;
-    int64_t m_ping_usec;
-    int64_t m_min_ping_usec;
+    std::chrono::microseconds m_last_ping_time;
+    std::chrono::microseconds m_min_ping_time;
     CAmount minFeeFilter;
     // Our address, as reported by the peer
     std::string addrLocal;
@@ -593,11 +593,11 @@ class CNode
     std::atomic<int64_t> nLastTXTime{0};
 
     /** Last measured round-trip time. Used only for RPC/GUI stats/debugging.*/
-    std::atomic<int64_t> m_last_ping_time{0};
+    std::atomic<std::chrono::microseconds> m_last_ping_time{0us};
 
     /** Lowest measured round-trip time. Used as an inbound peer eviction
      * criterium in CConnman::AttemptToEvictConnection. */
-    std::atomic<int64_t> m_min_ping_time{std::numeric_limits<int64_t>::max()};
+    std::atomic<std::chrono::microseconds> m_min_ping_time{std::chrono::microseconds::max()};
 
     CNode(NodeId id, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion);
     ~CNode();
@@ -719,8 +719,8 @@ class CNode
 
     /** A ping-pong round trip has completed successfully. Update latest and minimum ping times. */
     void PongReceived(std::chrono::microseconds ping_time) {
-        m_last_ping_time = count_microseconds(ping_time);
-        m_min_ping_time = std::min(m_min_ping_time.load(), count_microseconds(ping_time));
+        m_last_ping_time = ping_time;
+        m_min_ping_time = std::min(m_min_ping_time.load(), ping_time);
     }
 
 private:
@@ -1284,7 +1284,7 @@ struct NodeEvictionCandidate
 {
     NodeId id;
     int64_t nTimeConnected;
-    int64_t m_min_ping_time;
+    std::chrono::microseconds m_min_ping_time;
     int64_t nLastBlockTime;
     int64_t nLastTXTime;
     bool fRelevantServices;
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -1112,7 +1112,7 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats)
         ping_wait = GetTime<std::chrono::microseconds>() - peer->m_ping_start.load();
     }
 
-    stats.m_ping_wait_usec = count_microseconds(ping_wait);
+    stats.m_ping_wait = ping_wait;
 
     return true;
 }
diff --git a/src/net_processing.h b/src/net_processing.h
@@ -30,7 +30,7 @@ struct CNodeStateStats {
     int nSyncHeight = -1;
     int nCommonHeight = -1;
     int m_starting_height = -1;
-    int64_t m_ping_wait_usec;
+    std::chrono::microseconds m_ping_wait;
     std::vector<int> vHeightInFlight;
 };
 
diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp
@@ -59,6 +59,8 @@
 #include <QUrlQuery>
 #include <QtGlobal>
 
+#include <chrono>
+
 #if defined(Q_OS_MAC)
 
 #include <QProcess>
@@ -706,9 +708,11 @@ QString formatServicesStr(quint64 mask)
         return QObject::tr("None");
 }
 
-QString formatPingTime(int64_t ping_usec)
+QString formatPingTime(std::chrono::microseconds ping_time)
 {
-    return (ping_usec == std::numeric_limits<int64_t>::max() || ping_usec == 0) ? QObject::tr("N/A") : QString(QObject::tr("%1 ms")).arg(QString::number((int)(ping_usec / 1000), 10));
+    return (ping_time == std::chrono::microseconds::max() || ping_time == 0us) ?
+        QObject::tr("N/A") :
+        QString(QObject::tr("%1 ms")).arg(QString::number((int)(count_microseconds(ping_time) / 1000), 10));
 }
 
 QString formatTimeOffset(int64_t nTimeOffset)
diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h
@@ -20,6 +20,8 @@
 #include <QString>
 #include <QTableView>
 
+#include <chrono>
+
 class QValidatedLineEdit;
 class SendCoinsRecipient;
 
@@ -202,8 +204,8 @@ namespace GUIUtil
     /** Format CNodeStats.nServices bitmask into a user-readable string */
     QString formatServicesStr(quint64 mask);
 
-    /** Format a CNodeStats.m_ping_usec into a user-readable string or display N/A, if 0 */
-    QString formatPingTime(int64_t ping_usec);
+    /** Format a CNodeStats.m_last_ping_time into a user-readable string or display N/A, if 0 */
+    QString formatPingTime(std::chrono::microseconds ping_time);
 
     /** Format a CNodeCombinedStats.nTimeOffset into a user-readable string */
     QString formatTimeOffset(int64_t nTimeOffset);
diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp
@@ -34,7 +34,7 @@ bool NodeLessThan::operator()(const CNodeCombinedStats &left, const CNodeCombine
     case PeerTableModel::Network:
         return pLeft->m_network < pRight->m_network;
     case PeerTableModel::Ping:
-        return pLeft->m_min_ping_usec < pRight->m_min_ping_usec;
+        return pLeft->m_min_ping_time < pRight->m_min_ping_time;
     case PeerTableModel::Sent:
         return pLeft->nSendBytes < pRight->nSendBytes;
     case PeerTableModel::Received:
@@ -170,7 +170,7 @@ QVariant PeerTableModel::data(const QModelIndex &index, int role) const
         case Network:
             return GUIUtil::NetworkToQString(rec->nodeStats.m_network);
         case Ping:
-            return GUIUtil::formatPingTime(rec->nodeStats.m_min_ping_usec);
+            return GUIUtil::formatPingTime(rec->nodeStats.m_min_ping_time);
         case Sent:
             return GUIUtil::formatBytes(rec->nodeStats.nSendBytes);
         case Received:
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
@@ -1128,8 +1128,8 @@ void RPCConsole::updateDetailWidget()
     ui->peerLastRecv->setText(TimeDurationField(time_now, stats->nodeStats.nLastRecv));
     ui->peerBytesSent->setText(GUIUtil::formatBytes(stats->nodeStats.nSendBytes));
     ui->peerBytesRecv->setText(GUIUtil::formatBytes(stats->nodeStats.nRecvBytes));
-    ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_ping_usec));
-    ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_usec));
+    ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_last_ping_time));
+    ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_time));
     ui->timeoffset->setText(GUIUtil::formatTimeOffset(stats->nodeStats.nTimeOffset));
     ui->peerVersion->setText(QString::number(stats->nodeStats.nVersion));
     ui->peerSubversion->setText(QString::fromStdString(stats->nodeStats.cleanSubVer));
@@ -1162,7 +1162,7 @@ void RPCConsole::updateDetailWidget()
             ui->peerCommonHeight->setText(tr("Unknown"));
 
         ui->peerHeight->setText(QString::number(stats->nodeStateStats.m_starting_height));
-        ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStateStats.m_ping_wait_usec));
+        ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStateStats.m_ping_wait));
     }
 
     ui->peersTabRightPanel->show();
diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
@@ -202,14 +202,14 @@ static RPCHelpMan getpeerinfo()
         obj.pushKV("bytesrecv", stats.nRecvBytes);
         obj.pushKV("conntime", stats.nTimeConnected);
         obj.pushKV("timeoffset", stats.nTimeOffset);
-        if (stats.m_ping_usec > 0) {
-            obj.pushKV("pingtime", ((double)stats.m_ping_usec) / 1e6);
+        if (stats.m_last_ping_time > 0us) {
+            obj.pushKV("pingtime", CountSecondsDouble(stats.m_last_ping_time));
         }
-        if (stats.m_min_ping_usec < std::numeric_limits<int64_t>::max()) {
-            obj.pushKV("minping", ((double)stats.m_min_ping_usec) / 1e6);
+        if (stats.m_min_ping_time < std::chrono::microseconds::max()) {
+            obj.pushKV("minping", CountSecondsDouble(stats.m_min_ping_time));
         }
-        if (fStateStats && statestats.m_ping_wait_usec > 0) {
-            obj.pushKV("pingwait", ((double)statestats.m_ping_wait_usec) / 1e6);
+        if (fStateStats && statestats.m_ping_wait > 0s) {
+            obj.pushKV("pingwait", CountSecondsDouble(statestats.m_ping_wait));
         }
         obj.pushKV("version", stats.nVersion);
         // Use the sanitized form of subver here, to avoid tricksy remote peers from
diff --git a/src/test/fuzz/node_eviction.cpp b/src/test/fuzz/node_eviction.cpp
@@ -21,17 +21,17 @@ FUZZ_TARGET(node_eviction)
     std::vector<NodeEvictionCandidate> eviction_candidates;
     while (fuzzed_data_provider.ConsumeBool()) {
         eviction_candidates.push_back({
-            fuzzed_data_provider.ConsumeIntegral<NodeId>(),
-            fuzzed_data_provider.ConsumeIntegral<int64_t>(),
-            fuzzed_data_provider.ConsumeIntegral<int64_t>(),
-            fuzzed_data_provider.ConsumeIntegral<int64_t>(),
-            fuzzed_data_provider.ConsumeIntegral<int64_t>(),
-            fuzzed_data_provider.ConsumeBool(),
-            fuzzed_data_provider.ConsumeBool(),
-            fuzzed_data_provider.ConsumeBool(),
-            fuzzed_data_provider.ConsumeIntegral<uint64_t>(),
-            fuzzed_data_provider.ConsumeBool(),
-            fuzzed_data_provider.ConsumeBool(),
+            /* id */ fuzzed_data_provider.ConsumeIntegral<NodeId>(),
+            /* nTimeConnected */ fuzzed_data_provider.ConsumeIntegral<int64_t>(),
+            /* m_min_ping_time */ std::chrono::microseconds{fuzzed_data_provider.ConsumeIntegral<int64_t>()},
+            /* nLastBlockTime */ fuzzed_data_provider.ConsumeIntegral<int64_t>(),
+            /* nLastTXTime */ fuzzed_data_provider.ConsumeIntegral<int64_t>(),
+            /* fRelevantServices */ fuzzed_data_provider.ConsumeBool(),
+            /* fRelayTxes */ fuzzed_data_provider.ConsumeBool(),
+            /* fBloomFilter */ fuzzed_data_provider.ConsumeBool(),
+            /* nKeyedNetGroup */ fuzzed_data_provider.ConsumeIntegral<uint64_t>(),
+            /* prefer_evict */ fuzzed_data_provider.ConsumeBool(),
+            /* m_is_local */ fuzzed_data_provider.ConsumeBool(),
         });
     }
     // Make a copy since eviction_candidates may be in some valid but otherwise
diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp
@@ -825,7 +825,7 @@ std::vector<NodeEvictionCandidate> GetRandomNodeEvictionCandidates(const int n_c
         candidates.push_back({
             /* id */ id,
             /* nTimeConnected */ static_cast<int64_t>(random_context.randrange(100)),
-            /* m_min_ping_time */ static_cast<int64_t>(random_context.randrange(100)),
+            /* m_min_ping_time */ std::chrono::microseconds{random_context.randrange(100)},
             /* nLastBlockTime */ static_cast<int64_t>(random_context.randrange(100)),
             /* nLastTXTime */ static_cast<int64_t>(random_context.randrange(100)),
             /* fRelevantServices */ random_context.randbool(),
@@ -885,7 +885,7 @@ BOOST_AUTO_TEST_CASE(node_eviction_test)
             // from eviction.
             BOOST_CHECK(!IsEvicted(
                 number_of_nodes, [](NodeEvictionCandidate& candidate) {
-                    candidate.m_min_ping_time = candidate.id;
+                    candidate.m_min_ping_time = std::chrono::microseconds{candidate.id};
                 },
                 {0, 1, 2, 3, 4, 5, 6, 7}, random_context));
 
@@ -931,10 +931,10 @@ BOOST_AUTO_TEST_CASE(node_eviction_test)
             // Combination of all tests above.
             BOOST_CHECK(!IsEvicted(
                 number_of_nodes, [number_of_nodes](NodeEvictionCandidate& candidate) {
-                    candidate.nKeyedNetGroup = number_of_nodes - candidate.id; // 4 protected
-                    candidate.m_min_ping_time = candidate.id;                 // 8 protected
-                    candidate.nLastTXTime = number_of_nodes - candidate.id;    // 4 protected
-                    candidate.nLastBlockTime = number_of_nodes - candidate.id; // 4 protected
+                    candidate.nKeyedNetGroup = number_of_nodes - candidate.id;           // 4 protected
+                    candidate.m_min_ping_time = std::chrono::microseconds{candidate.id}; // 8 protected
+                    candidate.nLastTXTime = number_of_nodes - candidate.id;              // 4 protected
+                    candidate.nLastBlockTime = number_of_nodes - candidate.id;           // 4 protected
                 },
                 {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}, random_context));
 
diff --git a/src/util/time.h b/src/util/time.h
@@ -30,6 +30,13 @@ inline int64_t count_seconds(std::chrono::seconds t) { return t.count(); }
 inline int64_t count_milliseconds(std::chrono::milliseconds t) { return t.count(); }
 inline int64_t count_microseconds(std::chrono::microseconds t) { return t.count(); }
 
+using SecondsDouble = std::chrono::duration<double, std::chrono::seconds::period>;
+
+/**
+ * Helper to count the seconds in any std::chrono::duration type
+ */
+inline double CountSecondsDouble(SecondsDouble t) { return t.count(); }
+
 /**
  * DEPRECATED
  * Use either GetSystemTimeInSeconds (not mockable) or GetTime<T> (mockable)

Original file line number	Diff line number	Diff line change
`@@ -603,8 +603,8 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)`
`603`	`603`	`stats.minFeeFilter = 0;`
`604`	`604`	`}`
`605`	`605`
`606`		`- stats.m_ping_usec = m_last_ping_time;`
`607`		`- stats.m_min_ping_usec = m_min_ping_time;`
	`606`	`+ X(m_last_ping_time);`
	`607`	`+ X(m_min_ping_time);`
`608`	`608`
`609`	`609`	`// Leave string empty if addrLocal invalid (not filled in yet)`
`610`	`610`	`CService addrLocalUnlocked = GetAddrLocal();`
Original file line number	Diff line number	Diff line change
`@@ -1112,7 +1112,7 @@ bool PeerManagerImpl::GetNodeStateStats(NodeId nodeid, CNodeStateStats &stats)`
`1112`	`1112`	`ping_wait = GetTime<std::chrono::microseconds>() - peer->m_ping_start.load();`
`1113`	`1113`	`}`
`1114`	`1114`
`1115`		`- stats.m_ping_wait_usec = count_microseconds(ping_wait);`
	`1115`	`+ stats.m_ping_wait = ping_wait;`
`1116`	`1116`
`1117`	`1117`	`return true;`
`1118`	`1118`	`}`
Original file line number	Diff line number	Diff line change
`@@ -59,6 +59,8 @@`
`59`	`59`	`#include <QUrlQuery>`
`60`	`60`	`#include <QtGlobal>`
`61`	`61`
	`62`	`+#include <chrono>`
	`63`	`+`
`62`	`64`	`#if defined(Q_OS_MAC)`
`63`	`65`
`64`	`66`	`#include <QProcess>`
`@@ -706,9 +708,11 @@ QString formatServicesStr(quint64 mask)`
`706`	`708`	`return QObject::tr("None");`
`707`	`709`	`}`
`708`	`710`
`709`		`-QString formatPingTime(int64_t ping_usec)`
	`711`	`+QString formatPingTime(std::chrono::microseconds ping_time)`
`710`	`712`	`{`
`711`		`- return (ping_usec == std::numeric_limits<int64_t>::max() \|\| ping_usec == 0) ? QObject::tr("N/A") : QString(QObject::tr("%1 ms")).arg(QString::number((int)(ping_usec / 1000), 10));`
	`713`	`+ return (ping_time == std::chrono::microseconds::max() \|\| ping_time == 0us) ?`
	`714`	`+ QObject::tr("N/A") :`
	`715`	`+ QString(QObject::tr("%1 ms")).arg(QString::number((int)(count_microseconds(ping_time) / 1000), 10));`
`712`	`716`	`}`
`713`	`717`
`714`	`718`	`QString formatTimeOffset(int64_t nTimeOffset)`