Merge #16046: util: Add type safe GetTime

fa013664ae23d0682a195b9bded85bc19c99536e util: Add type safe GetTime (MarcoFalke)

Pull request description:

  There are basically two ways to get the time in Bitcoin Core:
  * get the system time (via `GetSystemTimeInSeconds` or `GetTime{Millis,Micros}`)
  * get the mockable time (via `GetTime`)

  Both return the same type (a plain int). This can lead to (test-only) bugs such as 99464bc38e9575ff47f8e33223b252dcea2055e3.

  Fix that by deprecating `GetTime` and adding a `GetTime<>` that returns the mockable time in a non-int type. The new util function is currently unused, but new code should it where possible.

ACKs for commit fa0136:
  promag:
    utACK fa013664.

Tree-SHA512: efab9c463f079fd8fd3030c479637c7b1e8be567a881234bd0f555c8f87e518e3b43ef2466128103db8fc40295aaf24e87ad76d91f338c631246fc703477e95c

2 files changed, 32 insertions, 11 deletions

diff --git a/src/util/time.cpp b/src/util/time.cpp
index c0ede9870..2b202ae95 100644
--- a/src/util/time.cpp
+++ b/src/util/time.cpp
@@ -1,5 +1,5 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
-// Copyright (c) 2009-2018 The Bitcoin Core developers
+// Copyright (c) 2009-2019 The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -27,6 +27,20 @@ int64_t GetTime()
     return now;
 }
 
+template <typename T>
+T GetTime()
+{
+    const std::chrono::seconds mocktime{nMockTime.load(std::memory_order_relaxed)};
+
+    return std::chrono::duration_cast<T>(
+        mocktime.count() ?
+            mocktime :
+            std::chrono::microseconds{GetTimeMicros()});
+}
+template std::chrono::seconds GetTime();
+template std::chrono::milliseconds GetTime();
+template std::chrono::microseconds GetTime();
+
 void SetMockTime(int64_t nMockTimeIn)
 {
     nMockTime.store(nMockTimeIn, std::memory_order_relaxed);
diff --git a/src/util/time.h b/src/util/time.h
index 68de1c156..e4f999677 100644
--- a/src/util/time.h
+++ b/src/util/time.h
@@ -1,5 +1,5 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
-// Copyright (c) 2009-2018 The Bitcoin Core developers
+// Copyright (c) 2009-2019 The Bitcoin Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
@@ -8,27 +8,34 @@
 
 #include <stdint.h>
 #include <string>
+#include <chrono>
 
 /**
- * GetTimeMicros() and GetTimeMillis() both return the system time, but in
- * different units. GetTime() returns the system time in seconds, but also
- * supports mocktime, where the time can be specified by the user, eg for
- * testing (eg with the setmocktime rpc, or -mocktime argument).
- *
- * TODO: Rework these functions to be type-safe (so that we don't inadvertently
- * compare numbers with different units, or compare a mocktime to system time).
+ * DEPRECATED
+ * Use either GetSystemTimeInSeconds (not mockable) or GetTime<T> (mockable)
  */
-
 int64_t GetTime();
+
+/** Returns the system time (not mockable) */
 int64_t GetTimeMillis();
+/** Returns the system time (not mockable) */
 int64_t GetTimeMicros();
+/** Returns the system time (not mockable) */
 int64_t GetSystemTimeInSeconds(); // Like GetTime(), but not mockable
+
+/** For testing. Set e.g. with the setmocktime rpc, or -mocktime argument */
 void SetMockTime(int64_t nMockTimeIn);
+/** For testing */
 int64_t GetMockTime();
+
 void MilliSleep(int64_t n);
 
+/** Return system time (or mocked time, if set) */
+template <typename T>
+T GetTime();
+
 /**
- * ISO 8601 formatting is preferred. Use the FormatISO8601{DateTime,Date,Time}
+ * ISO 8601 formatting is preferred. Use the FormatISO8601{DateTime,Date}
  * helper functions if possible.
  */
 std::string FormatISO8601DateTime(int64_t nTime);