Why change NodeSeconds{} to {}?

m_last_getheaders_timestamp is of type
NodeClock::time_point aka
std::chrono::time_point<NodeClock> aka
std::chrono::time_point<std::chrono::system_clock>

while NodeSeconds is of type
std::chrono::time_point<NodeClock, std::chrono::seconds> aka
std::chrono::time_point<std::chrono::system_clock, std::chrono::seconds>.

So, the type of m_last_getheaders_timestamp omits the second template parameter. It defaults to std::chrono::system_clock::duration. From the documentation, it is std::chrono::duration<rep, period>, but I can't find where it is specified that std::chrono::system_clock::period is std::ratio<1>. It only says:

period -- a std::ratio type representing the tick period of the clock, in seconds

Even if that is ok, there is now inconsistency wrt how m_last_getheaders_timestamp is set - in its declaration NodeSeconds{} is used and here {} is used.

NodeSeconds{}, as well as std::chrono::time_point<NodeClock>{} will construct a time point representing the clock's epoch, see https://en.cppreference.com/w/cpp/chrono/time_point/time_point. The duration (/time) since epoch is always zero, regardless of the durations period. So this should be fine as is.

Alright, zero is always zero even if the period is std::ratio<1> for one and something else for the other.

This means that it should be possible to use just {} in the declaration for consistency, right?

-NodeClock::time_point m_last_getheaders_timestamp GUARDED_BY(m_last_getheaders_mutex) {NodeSeconds{}};
+NodeClock::time_point m_last_getheaders_timestamp GUARDED_BY(m_last_getheaders_mutex) {};

This means that it should be possible to use just {} in the declaration for consistency, right?

For atomic that isn't possible due to compiler bugs, but without atomic, it may be possible.

A similar result can be achieved without a mutex, in a lock-free manner:

     // Only allow a new getheaders message to go out if we don't have a recent
     // one already in-flight
-    if (current_time - peer.m_last_getheaders_timestamp.load() > HEADERS_RESPONSE_TIME) {
+    auto last = peer.m_last_getheaders_timestamp.load();
+    if (current_time - last > HEADERS_RESPONSE_TIME &&
+        peer.m_last_getheaders_timestamp.compare_exchange_strong(last, current_time)) {
+
         m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETHEADERS, locator, uint256()));
-        peer.m_last_getheaders_timestamp = current_time;
         return true;
     }

Anyway, feel free to ignore. I think in this case using a mutex is also ok.

Is it not better to first lock the mutex and then acquire the current time, because locking the mutex can take "long" time and then we would operate on an outdated current_time?