refactor: Use NodeClock::time_point in more places #34882

maflcko commented at 3:28 PM on March 20, 2026: member

It is a bit confusing to have some code use the deprecated GetTime, which returns a duration and not a time point, and other code to use NodeClock time points.

Fix a few more places to properly use time_point types.

DrahtBot renamed this:
~~refactor: Use NodeClock::time_point in more places~~
refactor: Use NodeClock::time_point in more places
on Mar 20, 2026

DrahtBot added the label Refactoring on Mar 20, 2026

DrahtBot commented at 3:29 PM on March 20, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	seduless, stickies-v, naiyoma, sedited

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

maflcko force-pushed on Mar 20, 2026

DrahtBot added the label CI failed on Mar 20, 2026

DrahtBot commented at 3:46 PM on March 20, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task 32 bit ARM: https://github.com/bitcoin/bitcoin/actions/runs/23349904881/job/67925442030</sub> <sub>LLM reason (✨ experimental): CI failed because the C++ build broke with a std::chrono::duration_cast/time conversion compile error in src/util/time.h (triggering net.cpp/bitcoin_node compilation to fail).</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

</details>

maflcko force-pushed on Mar 20, 2026

fanquake commented at 7:01 AM on March 23, 2026: member

https://github.com/bitcoin/bitcoin/actions/runs/23350972073/job/67929332277?pr=34882#step:11:2765:

Run node_eviction with args ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/node_eviction')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1814253240
INFO: Loaded 1 modules   (629291 inline 8-bit counters): 629291 [0x613d409b64f8, 0x613d40a4ff23), 
INFO: Loaded 1 PC tables (629291 PCs): 629291 [0x613d40a4ff28,0x613d413ea1d8), 
INFO:      281 files found in /home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/node_eviction
INFO: -max_len is not provided; libFuzzer will not generate inputs larger than 572950 bytes
INFO: seed corpus: files: 281 min: 1b max: 572950b total: 15456954b rss: 99Mb
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:225:38: runtime error: signed integer overflow: -9223372036854775808 * 1000000000 cannot be represented in type 'long'
    [#0](/bitcoin-bitcoin/0/) 0x613d3da2fc9b in std::chrono::duration<long, std::ratio<1l, 1000000000l>> std::chrono::__duration_cast_impl<std::chrono::duration<long, std::ratio<1l, 1000000000l>>, std::ratio<1000000000l, 1l>, long, false, true>::__cast<long, std::ratio<1l, 1l>>(std::chrono::duration<long, std::ratio<1l, 1l>> const&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:225:38
    [#1](/bitcoin-bitcoin/1/) 0x613d3da2fc9b in std::enable_if<__is_duration<std::chrono::duration<long, std::ratio<1l, 1000000000l>>>::value, std::chrono::duration<long, std::ratio<1l, 1000000000l>>>::type std::chrono::duration_cast<std::chrono::duration<long, std::ratio<1l, 1000000000l>>, long, std::ratio<1l, 1l>>(std::chrono::duration<long, std::ratio<1l, 1l>> const&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:287:11
    [#2](/bitcoin-bitcoin/2/) 0x613d3da2fc9b in std::chrono::duration<long, std::ratio<1l, 1000000000l>>::duration<long, std::ratio<1l, 1l>, void>(std::chrono::duration<long, std::ratio<1l, 1l>> const&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:582:10
    [#3](/bitcoin-bitcoin/3/) 0x613d3da2fc9b in std::chrono::time_point<NodeClock, std::chrono::duration<long, std::ratio<1l, 1000000000l>>>::time_point<std::chrono::duration<long, std::ratio<1l, 1l>>, void>(std::chrono::time_point<NodeClock, std::chrono::duration<long, std::ratio<1l, 1l>>> const&) /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:944:6
    [#4](/bitcoin-bitcoin/4/) 0x613d3da2fc9b in node_eviction_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) /home/admin/actions-runner/_work/_temp/src/test/fuzz/node_eviction.cpp:25:29
    [#5](/bitcoin-bitcoin/5/) 0x613d3de6372b in std::function<void (std::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::span<unsigned char const, 18446744073709551615ul>) const /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/std_function.h:591:9
    [#6](/bitcoin-bitcoin/6/) 0x613d3de6372b in test_one_input(std::span<unsigned char const, 18446744073709551615ul>) /home/admin/actions-runner/_work/_temp/src/test/fuzz/fuzz.cpp:88:5
    [#7](/bitcoin-bitcoin/7/) 0x613d3de6372b in LLVMFuzzerTestOneInput /home/admin/actions-runner/_work/_temp/src/test/fuzz/fuzz.cpp:216:5
    [#8](/bitcoin-bitcoin/8/) 0x613d3d40aa19 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1b35a19) (BuildId: f2fdc24d7e3ad46fa0430c8bd8ff9f3177fcf93d)
    [#9](/bitcoin-bitcoin/9/) 0x613d3d40a029 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1b35029) (BuildId: f2fdc24d7e3ad46fa0430c8bd8ff9f3177fcf93d)
    [#10](/bitcoin-bitcoin/10/) 0x613d3d40bda2 in fuzzer::Fuzzer::ReadAndExecuteSeedCorpora(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1b36da2) (BuildId: f2fdc24d7e3ad46fa0430c8bd8ff9f3177fcf93d)
    [#11](/bitcoin-bitcoin/11/) 0x613d3d40c2c0 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1b372c0) (BuildId: f2fdc24d7e3ad46fa0430c8bd8ff9f3177fcf93d)
    [#12](/bitcoin-bitcoin/12/) 0x613d3d3f8b3d in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1b23b3d) (BuildId: f2fdc24d7e3ad46fa0430c8bd8ff9f3177fcf93d)
    [#13](/bitcoin-bitcoin/13/) 0x613d3d424d06 in main (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1b4fd06) (BuildId: f2fdc24d7e3ad46fa0430c8bd8ff9f3177fcf93d)
    [#14](/bitcoin-bitcoin/14/) 0x78751bbc81c9  (/lib/x86_64-linux-gnu/libc.so.6+0x2a1c9) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    [#15](/bitcoin-bitcoin/15/) 0x78751bbc828a in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2a28a) (BuildId: 8e9fd827446c24067541ac5390e6f527fb5947bb)
    [#16](/bitcoin-bitcoin/16/) 0x613d3d3ed124 in _start (/home/admin/actions-runner/_work/_temp/build/bin/fuzz+0x1b18124) (BuildId: f2fdc24d7e3ad46fa0430c8bd8ff9f3177fcf93d)

SUMMARY: UndefinedBehaviorSanitizer: signed-integer-overflow /usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/chrono.h:225:38

maflcko marked this as a draft on Mar 24, 2026

maflcko commented at 2:06 PM on March 24, 2026: member

This happens in line 25, but I can see the same error in line 26 (src/test/fuzz/node_eviction.cpp:26:33). This is simply due to the fact that this assumption is violated:

each of the predefined duration types up to hours covers a range of at least ±292 years.

Source: https://en.cppreference.com/w/cpp/chrono/duration.html

For larger ranges, usually everything falls apart in the stdlib: https://godbolt.org/z/er3E5PhEE

This is a bug in the fuzz target. I'll try to fix it in a separate pull.

edit: done in https://github.com/bitcoin/bitcoin/pull/34913

maflcko force-pushed on Mar 24, 2026

DrahtBot removed the label CI failed on Mar 24, 2026

maflcko force-pushed on Mar 25, 2026

maflcko marked this as ready for review on Mar 27, 2026

refactor: Use NodeClock alias over deprecated GetTime

GetTime returns a duration, but a time point is the correct type to use
here.

This refactor does not change any behavior.

fa41e072b3

util: Add NodeClock::epoch alias

A default constructed time_point is the epoch, by definition.

Existing code uses a default constructed (or explicitly constructed with
a zero duration) chrono type to mean epoch. New code can now use
NodeClock::epoch as an alias.

facfce37f6

refactor: Avoid manual chrono casts with * or /

Manual chrono casts, using multiplication or division is confusing and
brittle.

Also, when calling ShouldRunInactivityChecks remove a confusing and
useless std::chrono::duration_cast<std::chrono::seconds>.

fab88884b7

refactor: gui: Accept up to nanoseconds in formatDurationStr, but clarify they are ignored

This refactor does not change any behavior. However, it helps future
commits to avoid having to place manual
std::chrono::duration_cast<std::chrono::seconds> when calling this
function.

fa54fb0129

doc: Fix typo "eviction criterium" -> "eviction criterion"

Also, clarify round-trip time to mean round-trip duration.

333316f6be

refactor: Use NodeClock::duration for m_last_ping_time/m_min_ping_time/m_ping_wait

This refactor does not change any behavior and is needed for a future
commit, to avoid having to add duration casts.

It also improves the docs to better document that this is not a time
point, but a duration.

Also, it uses decltype to explain where the _::max() is coming from.

fa644e625b

maflcko force-pushed on Mar 27, 2026

stickies-v commented at 3:59 PM on March 30, 2026: contributor

Concept ACK for increased type safety.

in src/qt/guiutil.h:241 in fa81cc0494 outdated

 241 |      QString formatServicesStr(quint64 mask);
 242 |  
 243 | -    /** Format a CNodeStats.m_last_ping_time into a user-readable string or display N/A, if 0 */
 244 | -    QString formatPingTime(std::chrono::microseconds ping_time);
 245 | +    /// Format a CNodeStats.m_last_ping_time/m_min_ping_time/m_ping_wait into a user-readable string if it exists, or display N/A
 246 | +    QString formatPingTime(NodeClock::duration ping_time);

stickies-v commented at 6:25 PM on March 30, 2026:

This PR doesn't make it worse, but the doc change highlights that formatPingTime's implementation is problematic. An minimal alternative could be to just have formatPingTime format as ms, and inline the rest of the logic, similar to how getpeerinfo does it? It's a bit more verbose, but imo not prohibitively so?

E.g.:

diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp
index e0966b9c38..9f333d15cf 100644
--- a/src/qt/guiutil.cpp
+++ b/src/qt/guiutil.cpp
@@ -768,9 +768,7 @@ QString formatServicesStr(quint64 mask)
 
 QString formatPingTime(NodeClock::duration ping_time)
 {
-    return (ping_time == decltype(CNode::m_min_ping_time.load())::max() || ping_time == 0us) ?
-        QObject::tr("N/A") :
-        QObject::tr("%1 ms").arg(QString::number(Ticks<std::chrono::milliseconds>(ping_time)));
+    return QObject::tr("%1 ms").arg(QString::number(Ticks<std::chrono::milliseconds>(ping_time)));
 }
 
 QString formatTimeOffset(int64_t time_offset)
diff --git a/src/qt/guiutil.h b/src/qt/guiutil.h
index 57d042f6c8..7658eab4ed 100644
--- a/src/qt/guiutil.h
+++ b/src/qt/guiutil.h
@@ -237,7 +237,7 @@ namespace GUIUtil
     /** Format CNodeStats.nServices bitmask into a user-readable string */
     QString formatServicesStr(quint64 mask);
 
-    /// Format a CNodeStats.m_last_ping_time/m_min_ping_time/m_ping_wait into a user-readable string if it exists, or display N/A
+    /// Format a duration as a ping time in milliseconds.
     QString formatPingTime(NodeClock::duration ping_time);
 
     /** Format a CNodeStateStats.time_offset into a user-readable string */
diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp
index 3c0dedfcc7..d3caa8554f 100644
--- a/src/qt/peertablemodel.cpp
+++ b/src/qt/peertablemodel.cpp
@@ -82,7 +82,10 @@ 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_time);
+            if (rec->nodeStats.m_min_ping_time < decltype(CNode::m_min_ping_time.load())::max()) {
+                return GUIUtil::formatPingTime(rec->nodeStats.m_min_ping_time);
+            }
+            return QObject::tr("N/A");
         case Sent:
             return GUIUtil::formatBytes(rec->nodeStats.nSendBytes);
         case Received:
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index 4c69ec29dd..4e385d6a78 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -1174,8 +1174,16 @@ void RPCConsole::updateDetailWidget()
     ui->peerLastRecv->setText(TimeDurationField(now, stats->nodeStats.m_last_recv));
     ui->peerBytesSent->setText(GUIUtil::formatBytes(stats->nodeStats.nSendBytes));
     ui->peerBytesRecv->setText(GUIUtil::formatBytes(stats->nodeStats.nRecvBytes));
-    ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_last_ping_time));
-    ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_time));
+    if (stats->nodeStats.m_last_ping_time > 0us) {
+        ui->peerPingTime->setText(GUIUtil::formatPingTime(stats->nodeStats.m_last_ping_time));
+    } else {
+        ui->peerPingTime->setText(ts.na);
+    }
+    if (stats->nodeStats.m_min_ping_time < decltype(CNode::m_min_ping_time.load())::max()) {
+        ui->peerMinPing->setText(GUIUtil::formatPingTime(stats->nodeStats.m_min_ping_time));
+    } else {
+        ui->peerMinPing->setText(ts.na);
+    }
     ui->peerVersion->setText(stats->nodeStats.nVersion ? QString::number(stats->nodeStats.nVersion) : ts.na);
     ui->peerSubversion->setText(!stats->nodeStats.cleanSubVer.empty() ? QString::fromStdString(stats->nodeStats.cleanSubVer) : ts.na);
     ui->peerConnectionType->setText(GUIUtil::ConnectionTypeToQString(stats->nodeStats.m_conn_type, /*prepend_direction=*/true));
@@ -1217,7 +1225,11 @@ void RPCConsole::updateDetailWidget()
         } else {
             ui->peerCommonHeight->setText(ts.unknown);
         }
-        ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStateStats.m_ping_wait));
+        if (stats->nodeStateStats.m_ping_wait > 0s) {
+            ui->peerPingWait->setText(GUIUtil::formatPingTime(stats->nodeStateStats.m_ping_wait));
+        } else {
+            ui->peerPingWait->setText(ts.na);
+        }
         ui->peerAddrRelayEnabled->setText(stats->nodeStateStats.m_addr_relay_enabled ? ts.yes : ts.no);
         ui->peerAddrProcessed->setText(QString::number(stats->nodeStateStats.m_addr_processed));
         ui->peerAddrRateLimited->setText(QString::number(stats->nodeStateStats.m_addr_rate_limited));

</details>

maflcko commented at 7:51 AM on March 31, 2026:

I guess in that case, it could be called formatMs (or so). Though, I guess the gui developers wrote it on purpose this way, so I think I'll leave as-is for now.

in src/net.h:240 in fa81cc0494 outdated

 236 | @@ -237,7 +237,8 @@ class CNetMessage
 237 |  {
 238 |  public:
 239 |      DataStream m_recv;                   //!< received message data
 240 | -    std::chrono::microseconds m_time{0}; //!< time of message receipt
 241 | +    /// time of message receipt

stickies-v commented at 6:29 PM on March 30, 2026:

nit: does this comment need to be move?

maflcko commented at 7:50 AM on March 31, 2026:

Yes, because I don't like those trailing comments:

Devs often get them wrong, and forget the !, in which case the doxygen html output will be misleading
Changing the length of any type of the name requires reflowing the whole struct
They do not really support multi-line comments

So they may appear nice on a first glance, but they are painful for any code that has to be touched more than once, so I'll try to avoid using them and I'll leave this as-is for now.

stickies-v commented at 11:05 AM on March 31, 2026:

I agree with the rationale, and I'll try to adhere with that in my own code going forward. I'm happy for this to stay as-is, it's just a nit, but I generally don't think it's productive to make these kinds of stylistic changes when they're not documented in developer-notes (or ideally enforced), as I think it quite likely someone else working on this code in the future will find the one differently-styled comment odd and inline it again.

But what's also not productive is spending a lot of time talking about docstrings so I'll leave it at that 😄

maflcko commented at 4:46 PM on March 31, 2026:

Fixed in #34967, as it is unrelated to this pull

in src/test/fuzz/p2p_transport_serialization.cpp:81 in fa81cc0494

  77 | @@ -78,7 +78,7 @@ FUZZ_TARGET(p2p_transport_serialization, .init = initialize_p2p_transport_serial
  78 |              break;
  79 |          }
  80 |          if (recv_transport.ReceivedMessageComplete()) {
  81 | -            const std::chrono::microseconds m_time{std::numeric_limits<int64_t>::max()};
  82 | +            const auto m_time{NodeClock::time_point::max()};

stickies-v commented at 6:33 PM on March 30, 2026:

nit: increases the amount of lines touched, but could be nice to fix name to time or max_time

maflcko commented at 7:50 AM on March 31, 2026:

thx, done

in src/net_processing.cpp:4903 in fa81cc0494 outdated

4899 | @@ -4900,7 +4900,7 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
4900 |      }
4901 |  
4902 |      if (msg_type == NetMsgType::PONG) {
4903 | -        const auto ping_end = time_received;
4904 | +        const auto ping_end{time_received};

stickies-v commented at 6:38 PM on March 30, 2026:

nit: ping_end can just be removed:

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 16d3f6112d..162b86449d 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -4900,7 +4900,6 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
     }
 
     if (msg_type == NetMsgType::PONG) {
-        const auto ping_end{time_received};
         uint64_t nonce = 0;
         size_t nAvail = vRecv.in_avail();
         bool bPingFinished = false;
@@ -4914,7 +4913,7 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
                 if (nonce == peer.m_ping_nonce_sent) {
                     // Matching pong received, this ping is no longer outstanding
                     bPingFinished = true;
-                    const auto ping_time = ping_end - peer.m_ping_start.load();
+                    const auto ping_time = time_received - peer.m_ping_start.load();
                     if (ping_time.count() >= 0) {
                         // Let connman know about this successful ping-pong
                         pfrom.PongReceived(ping_time);

</details>

maflcko commented at 7:50 AM on March 31, 2026:

right, but I have a follow-up, which takes care of this in a different way :)

in src/qt/rpcconsole.h:185 in fa81cc0494

 181 | @@ -182,6 +182,11 @@ public Q_SLOTS:
 182 |      /** Update UI with latest network info from model. */
 183 |      void updateNetworkState();
 184 |  
 185 | +    /// Helper for the output of a time point field.

stickies-v commented at 7:02 PM on March 30, 2026:

    /// Format the duration between now and event as a string.

stickies-v commented at 7:03 PM on March 30, 2026:

The TimeDurationField(std::chrono::seconds time_now, std::chrono::seconds time_at_event) overload should probably be marked as deprecated?

maflcko commented at 7:50 AM on March 31, 2026:

thx, done both nits

in src/net_processing.cpp:5738 in fa81cc0494 outdated

5733 | @@ -5733,27 +5734,28 @@ bool PeerManagerImpl::SendMessages(CNode& node)
5734 |      if (!node.fSuccessfullyConnected || node.fDisconnect)
5735 |          return true;
5736 |  
5737 | +    const auto now{NodeClock::now()};
5738 |      const auto current_time{GetTime<std::chrono::microseconds>()};

stickies-v commented at 7:15 PM on March 30, 2026:

I don't think it matters practically, but for sanity might be better to maintain a single source of truth for now?

    const auto now{NodeClock::now()};
    const auto current_time{now.time_since_epoch()};

SImilar changes in diff:

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 16d3f6112d..e824a3e63b 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -5374,7 +5374,7 @@ void PeerManagerImpl::CheckForStaleTipAndEvictPeers()
     LOCK(cs_main);
 
     const auto current_time{NodeClock::now()};
-    auto now{GetTime<std::chrono::seconds>()};
+    auto now{std::chrono::duration_cast<std::chrono::seconds>(current_time.time_since_epoch())};
 
     EvictExtraOutboundPeers(current_time);
 
@@ -5735,7 +5735,7 @@ bool PeerManagerImpl::SendMessages(CNode& node)
         return true;
 
     const auto now{NodeClock::now()};
-    const auto current_time{GetTime<std::chrono::microseconds>()};
+    const auto current_time{now.time_since_epoch()};
 
     // The logic below does not apply to private broadcast peers, so skip it.
     // Also in CConnman::PushMessage() we make sure that unwanted messages are
diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
index 4c69ec29dd..1c522e225b 100644
--- a/src/qt/rpcconsole.cpp
+++ b/src/qt/rpcconsole.cpp
@@ -1166,7 +1166,7 @@ void RPCConsole::updateDetailWidget()
     if (bip152_hb_settings.isEmpty()) bip152_hb_settings = ts.no;
     ui->peerHighBandwidth->setText(bip152_hb_settings);
     const auto now{NodeClock::now()};
-    const auto time_now{GetTime<std::chrono::seconds>()};
+    const auto time_now{std::chrono::duration_cast<std::chrono::seconds>(now.time_since_epoch())};
     ui->peerConnTime->setText(GUIUtil::formatDurationStr(now - stats->nodeStats.m_connected));
     ui->peerLastBlock->setText(TimeDurationField(time_now, stats->nodeStats.m_last_block_time));
     ui->peerLastTx->setText(TimeDurationField(time_now, stats->nodeStats.m_last_tx_time));

</details>

maflcko commented at 7:50 AM on March 31, 2026:

Thx, but I think it is better to keep GetTime for now, so that it is easier to grep for them and fix them.

stickies-v approved

stickies-v commented at 7:33 PM on March 30, 2026: contributor

ACK fa81cc04943c2422736a0c4115bdef19077982aa

Left a few nits and suggestions, but nothing blocking. Safer and more readable code. Couldn't find any behaviour change (except for potentially miniscule mismatches between different measurements of the current time, as commented).

refactor: Use NodeClock::time_point for CNetMessage::m_time

The field is not a duration, but a time point.

This will add two temporary calls to time_since_epoch(), which are fixed
in the next commit.

fa2605b204

refactor: Use NodeClock::time_point for m_last_send/recv and m_ping_start

The two fields represent a time point, not a duration. Also, it is
unclear why they use second precision.

Fix both issues by using NodeClock::time_point.

This refactor should not change any behavior.

This resolves the two temporary calls to time_since_epoch() added in the
previous commit. However, it adds one new call to time_since_epoch(),
which is resolved in the next commit.

fa244b984c

maflcko force-pushed on Mar 31, 2026

stickies-v commented at 11:09 AM on March 31, 2026: contributor

re-ACK c22222fe68f706f11f06ad7dfd13e360ddda95d1

seduless commented at 5:05 PM on April 11, 2026: contributor

Tested ACK c22222fe68f706f11f06ad7dfd13e360ddda95d1

Ran fuzz targets node_eviction, p2p_transport_serialization, and process_message for 24h with no issues.

In commit "refactor: Use NodeClock::time_point for m_connected" (c22222fe68f706f11f06ad7dfd13e360ddda95d1)

Not blocking, but this could also update the initialization of m_connected to NodeClock::now(), which is consistent with how the other time_point fields in this PR were updated at migration time.

diff --git a/src/net.cpp b/src/net.cpp
index dd6e5a6680..d046c2a272 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -3989,3 +3989,3 @@ CNode::CNode(NodeId idIn,
       m_sock{sock},
-      m_connected{GetTime<std::chrono::seconds>()},
+      m_connected{NodeClock::now()},
       addr{addrIn},

refactor: Use NodeClock::time_point for m_connected

Also, increase the precision to the native one, over prescribing second
precision.

fa1015bbcb

maflcko force-pushed on Apr 13, 2026

maflcko commented at 9:14 AM on April 13, 2026: member

thx, pushed your diff into the last commit

seduless commented at 5:43 PM on April 13, 2026: contributor

re-ACK fa1015bbcb15453a4a47be51490d1d28c837151e

Verified via git range-diff bitcoin-core/master c22222fe68 fa1015bbcb. Only the last commit is amended, updating the m_connected initialization to NodeClock::now().

DrahtBot requested review from stickies-v on Apr 13, 2026

stickies-v commented at 4:48 PM on April 14, 2026: contributor

re-ACK fa1015bbcb15453a4a47be51490d1d28c837151e

naiyoma commented at 11:46 AM on April 15, 2026: contributor

concept ACK

Reviewed up to commit fa644e6

m_headers_sync_timeout seems like it would also benefit from this migration unless its out of scope.

https://github.com/bitcoin/bitcoin/blob/01651324f4e540f6b96cff31e89752c3f9417293/src/net_processing.cpp#L5787 https://github.com/bitcoin/bitcoin/blob/01651324f4e540f6b96cff31e89752c3f9417293/src/net_processing.cpp#L6126

maflcko commented at 11:58 AM on April 15, 2026: member

m_headers_sync_timeout

Sure, happy to do this in a follow-up. This pull already has 9 commits and changes more than 100 lines. Albeit it has no code-conflicts, I think the size is neither to small, nor too large, so my preference would be to keep this as-is for now.

naiyoma commented at 11:37 AM on April 20, 2026: contributor

ACK fa1015bbcb15453a4a47be51490d1d28c837151e

Reviewed the last 3 remaining commits.

Changes LGTM. I have not observed any behavior changes, deprecated GetTime is replaced correctly. NodeClock::duration is used where a duration is semantically correct, and NodeClock::time_point is used where a specific moment in time.

sedited approved

sedited commented at 8:37 PM on April 21, 2026: contributor

ACK fa1015bbcb15453a4a47be51490d1d28c837151e

I'm not sure about the exact split into multiple commits and had an easier time just looking at the complete diff, but the changes seem fine to me.

sedited merged this on Apr 21, 2026

sedited closed this on Apr 21, 2026