From 07b9ebccf950a1948535e83beb9959671ef9dab3 Mon Sep 17 00:00:00 2001
From: fanquake <fanquake@gmail.com>
Date: Tue, 19 Apr 2022 13:36:43 +0100
Subject: [PATCH] Merge bitcoin/bitcoin#24871: refactor: Simplify GetTime

0000a63689036dc4368d04c0648a55fdf507932f Simplify GetTime (MarcoFalke)

Pull request description:

  The implementation of `GetTime` is confusing:
  * The value returned by `GetTime` is assumed to be equal to `GetTime<std::chrono::seconds>()`. Both are mockable and the only difference is return type, the value itself is equal. However, the implementation does not support this assumption.
  * On some systems, `time_t` might be a signed 32-bit integer (https://en.cppreference.com/w/c/chrono/time), thus breaking in the year 2038, whereas `GetTime<std::chrono::seconds>` does not. Also, `time_t` might be `-1` "on error", where "error" is unspecified.
  * `GetTime<std::chrono::seconds>` calls `GetTimeMicros`, which calls `GetSystemTime`, which calls `std::chrono::system_clock::now`, which doesn't have the above issues. See https://en.cppreference.com/w/cpp/chrono/system_clock/now
  * `GetTimeMicros` and the internal-only `GetSystemTime` will likely be renamed (to clarify they are the non-mockable non-monotonic system time) or removed in the future to be replaced by appropriate `std::chrono::time_point<Clock>` getters.

  Fix all issues by:
  * making `GetTime()` an alias for `GetTime<std::chrono::seconds>().count()`.
  * inlining the needed parts of `GetSystemTime` directly instead of needlessly increasing the function call stack with functions that are likely to be removed in the future.

ACKs for top commit:
  martinus:
    Code review, untested ACK 0000a63689036dc4368d04c0648a55fdf507932f. By the way strictly speaking `std::chrono::system_clock` is only guaranteed to be based on the unix epoch starting with C++20: https://en.cppreference.com/w/cpp/chrono/system_clock
  theStack:
    Code-review ACK 0000a63689036dc4368d04c0648a55fdf507932f

Tree-SHA512: f751ba740e0da65537be800e9414dd02282d9f04c0b0fb986a36546f257d0b888d8688653cdda5d355ec832c0e09d866922d9161b1ccd33485c1c92c5d1e802f
---
 src/util/time.cpp | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/src/util/time.cpp b/src/util/time.cpp
index 0c69ed729e..d65d4f2594 100644
--- a/src/util/time.cpp
+++ b/src/util/time.cpp
@@ -23,16 +23,6 @@ void UninterruptibleSleep(const std::chrono::microseconds& n) { std::this_thread
 
 static std::atomic<int64_t> nMockTime(0); //!< For testing
 
-int64_t GetTime()
-{
-    int64_t mocktime = nMockTime.load(std::memory_order_relaxed);
-    if (mocktime) return mocktime;
-
-    time_t now = time(nullptr);
-    assert(now > 0);
-    return now;
-}
-
 bool ChronoSanityCheck()
 {
     // std::chrono::system_clock.time_since_epoch and time_t(0) are not guaranteed
@@ -80,11 +70,12 @@ template <typename T>
 T GetTime()
 {
     const std::chrono::seconds mocktime{nMockTime.load(std::memory_order_relaxed)};
-
-    return std::chrono::duration_cast<T>(
+    const auto ret{
         mocktime.count() ?
             mocktime :
-            std::chrono::microseconds{GetTimeMicros()});
+            std::chrono::duration_cast<T>(std::chrono::system_clock::now().time_since_epoch())};
+    assert(ret > 0s);
+    return ret;
 }
 template std::chrono::seconds GetTime();
 template std::chrono::milliseconds GetTime();
@@ -124,6 +115,8 @@ int64_t GetSystemTimeInSeconds()
     return int64_t{GetSystemTime<std::chrono::seconds>().count()};
 }
 
+int64_t GetTime() { return GetTime<std::chrono::seconds>().count(); }
+
 std::string FormatISO8601DateTime(int64_t nTime) {
     struct tm ts;
     time_t time_val = nTime;