Merge #21676: test: Use mocktime to avoid intermittent failure in rpc_tests

fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1 test: Reset mocktime in the common setup (MarcoFalke)
fa78590a8fffdfc7e98ddb1f81218f05b1935a0a test: Use mocktime to avoid intermittent failure (MarcoFalke)

Pull request description:

  See https://github.com/bitcoin/bitcoin/pull/21602#discussion_r611176103

ACKs for top commit:
  jonatack:
    Code review ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1
  jarolrod:
    ACK fa40d6a1c47ac7f3dc6c11a2e6642cfef95422c1

Tree-SHA512: 4967e006f3d2c4eb92f03c9086a6abe3190ad54755d251c30d20422c574bb1a154c06f3d5bcb0d4deaa3c4abfd3864d743b71d84897edd358e829bb42233ad12

6 files changed, 7 insertions, 16 deletions

diff --git a/src/test/denialofservice_tests.cpp b/src/test/denialofservice_tests.cpp
index e7cb12dbba..599e628685 100644
--- a/src/test/denialofservice_tests.cpp
+++ b/src/test/denialofservice_tests.cpp
@@ -116,7 +116,6 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
         BOOST_CHECK(peerLogic->SendMessages(&dummyNode1)); // should result in disconnect
     }
     BOOST_CHECK(dummyNode1.fDisconnect == true);
-    SetMockTime(0);
 
     peerLogic->FinalizeNode(dummyNode1);
 }
diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp
index 25655b8894..e99c6e0fc8 100644
--- a/src/test/logging_tests.cpp
+++ b/src/test/logging_tests.cpp
@@ -14,7 +14,6 @@ BOOST_FIXTURE_TEST_SUITE(logging_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(logging_timer)
 {
-
     SetMockTime(1);
     auto sec_timer = BCLog::Timer<std::chrono::seconds>("tests", "end_msg");
     SetMockTime(2);
@@ -29,8 +28,6 @@ BOOST_AUTO_TEST_CASE(logging_timer)
     auto micro_timer = BCLog::Timer<std::chrono::microseconds>("tests", "end_msg");
     SetMockTime(2);
     BOOST_CHECK_EQUAL(micro_timer.LogMsg("test micros"), "tests: test micros (1000000.00μs)");
-
-    SetMockTime(0);
 }
 
 BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp
index 38fed51af2..bf36f8a6c9 100644
--- a/src/test/mempool_tests.cpp
+++ b/src/test/mempool_tests.cpp
@@ -571,8 +571,6 @@ BOOST_AUTO_TEST_CASE(MempoolSizeLimitTest)
     SetMockTime(42 + 8*CTxMemPool::ROLLING_FEE_HALFLIFE + CTxMemPool::ROLLING_FEE_HALFLIFE/2 + CTxMemPool::ROLLING_FEE_HALFLIFE/4);
     BOOST_CHECK_EQUAL(pool.GetMinFee(1).GetFeePerK(), 0);
     // ... unless it has gone all the way to 0 (after getting past 1000/2)
-
-    SetMockTime(0);
 }
 
 inline CTransactionRef make_tx(std::vector<CAmount>&& output_values, std::vector<CTransactionRef>&& inputs=std::vector<CTransactionRef>(), std::vector<uint32_t>&& input_indices=std::vector<uint32_t>())
diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp
index 3b6faf7bbb..67fbc9f8a2 100644
--- a/src/test/rpc_tests.cpp
+++ b/src/test/rpc_tests.cpp
@@ -275,7 +275,11 @@ BOOST_AUTO_TEST_CASE(rpc_ban)
 
     BOOST_CHECK_NO_THROW(CallRPC(std::string("clearbanned")));
 
+    auto now = 10'000s;
+    SetMockTime(now);
     BOOST_CHECK_NO_THROW(r = CallRPC(std::string("setban 127.0.0.0/24 add 200")));
+    SetMockTime(now += 2s);
+    const int64_t time_remaining_expected{198};
     BOOST_CHECK_NO_THROW(r = CallRPC(std::string("listbanned")));
     ar = r.get_array();
     o1 = ar[0].get_obj();
@@ -284,12 +288,10 @@ BOOST_AUTO_TEST_CASE(rpc_ban)
     const int64_t ban_created{find_value(o1, "ban_created").get_int64()};
     const int64_t ban_duration{find_value(o1, "ban_duration").get_int64()};
     const int64_t time_remaining{find_value(o1, "time_remaining").get_int64()};
-    const int64_t now{GetTime()};
     BOOST_CHECK_EQUAL(adr.get_str(), "127.0.0.0/24");
-    BOOST_CHECK(banned_until > now);
-    BOOST_CHECK(banned_until - now <= 200);
+    BOOST_CHECK_EQUAL(banned_until, time_remaining_expected + now.count());
     BOOST_CHECK_EQUAL(ban_duration, banned_until - ban_created);
-    BOOST_CHECK_EQUAL(time_remaining, banned_until - now);
+    BOOST_CHECK_EQUAL(time_remaining, time_remaining_expected);
 
     // must throw an exception because 127.0.0.1 is in already banned subnet range
     BOOST_CHECK_THROW(r = CallRPC(std::string("setban 127.0.0.1 add")), std::runtime_error);
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
index 2d7f335a04..d644dd2327 100644
--- a/src/test/util/setup_common.cpp
+++ b/src/test/util/setup_common.cpp
@@ -120,6 +120,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName, const std::ve
 
 BasicTestingSetup::~BasicTestingSetup()
 {
+    SetMockTime(0s); // Reset mocktime for following tests
     LogInstance().DisconnectTestLogger();
     fs::remove_all(m_path_root);
     gArgs.ClearArgs();
@@ -303,7 +304,6 @@ CMutableTransaction TestChain100Setup::CreateValidMempoolTransaction(CTransactio
 TestChain100Setup::~TestChain100Setup()
 {
     gArgs.ForceSetArg("-segwitheight", "0");
-    SetMockTime(0);
 }
 
 CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) const
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index e201cc1eb4..011d59ecaf 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -295,8 +295,6 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
             BOOST_CHECK_EQUAL(found, expected);
         }
     }
-
-    SetMockTime(0);
 }
 
 // Check that GetImmatureCredit() returns a newly calculated value instead of
@@ -377,9 +375,6 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart)
     // If there are future entries, new transaction should use time of the
     // newest entry that is no more than 300 seconds ahead of the clock time.
     BOOST_CHECK_EQUAL(AddTx(*m_node.chainman, m_wallet, 5, 50, 600), 300);
-
-    // Reset mock time for other tests.
-    SetMockTime(0);
 }
 
 BOOST_AUTO_TEST_CASE(LoadReceiveRequests)