Merge bitcoin/bitcoin#23636: Remove GetAdjustedTime from init.cpp

fa551b3bdd Remove GetAdjustedTime from init.cpp (MarcoFalke)
fa815f8473 Replace addrman.h include with forward decl in net.h (MarcoFalke)

Pull request description:

  It seems confusing to call `GetAdjustedTime` there, because no offset could have been retrieved from the network at this point. Even if connman was started, `timedata` needs at least 5 peer connections to calculate an offset.

  Fix the confusion by replacing `GetAdjustedTime` with `GetTime`, which does not change behavior.

  Also:
  * Replace magic number with `MAX_FUTURE_BLOCK_TIME` to clarify the context
  * Add test, which passes both on current master and this pull request
  * An unrelated refactoring commit, happy to drop

ACKs for top commit:
  dongcarl:
    Code Review ACK fa551b3bdd, noticed the exact same thing here: e073634c37
  mzumsande:
    Code Review ACK fa551b3bdd
  jnewbery:
    Code review ACK fa551b3bdd
  shaavan:
    ACK fa551b3bdd
  theStack:
    Code-review ACK fa551b3bdd

Tree-SHA512: 15807a0e943e3e8d8c5250c8f6d7b56afb26002b1e290bf93636a2c747f27e78f01f1de04ce1a83d6339e27284c69c43e077a8467545c4078746f4c1ecb1164d

src/init.cpp

@@ -1561,7 +1561,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
 
                         const CBlockIndex* tip = chainstate->m_chain.Tip();
                         RPCNotifyBlockChange(tip);
-                        if (tip && tip->nTime > GetAdjustedTime() + 2 * 60 * 60) {
+                        if (tip && tip->nTime > GetTime() + MAX_FUTURE_BLOCK_TIME) {
                             strLoadError = _("The block database contains a block which appears to be from the future. "
                                     "This may be due to your computer's date and time being set incorrectly. "
                                     "Only rebuild the block database if you are sure that your computer's date and time are correct");

src/net.cpp

@@ -10,6 +10,7 @@
 #include <net.h>
 
 #include <addrdb.h>
+#include <addrman.h>
 #include <banman.h>
 #include <clientversion.h>
 #include <compat.h>

src/net.h

@@ -6,7 +6,6 @@
 #ifndef BITCOIN_NET_H
 #define BITCOIN_NET_H
 
-#include <addrman.h>
 #include <chainparams.h>
 #include <common/bloom.h>
 #include <compat.h>
@@ -37,9 +36,10 @@
 #include <thread>
 #include <vector>
 
-class CScheduler;
+class AddrMan;
-class CNode;
 class BanMan;
+class CNode;
+class CScheduler;
 struct bilingual_str;
 
 /** Default for -whitelistrelay. */

src/rpc/net.cpp

@@ -4,6 +4,7 @@
 
 #include <rpc/server.h>
 
+#include <addrman.h>
 #include <banman.h>
 #include <chainparams.h>
 #include <clientversion.h>

src/test/fuzz/connman.cpp

@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <addrman.h>
 #include <chainparams.h>
 #include <chainparamsbase.h>
 #include <net.h>

test/functional/p2p_invalid_block.py

@@ -16,6 +16,7 @@ import copy
 import time
 
 from test_framework.blocktools import (
+    MAX_FUTURE_BLOCK_TIME,
     create_block,
     create_coinbase,
     create_tx_with_script,
@@ -26,8 +27,6 @@ from test_framework.script import OP_TRUE
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 
-MAX_FUTURE_BLOCK_TIME = 2 * 60 * 60
-
 
 class InvalidBlockRequestTest(BitcoinTestFramework):
     def set_test_params(self):

test/functional/rpc_blockchain.py

@@ -26,9 +26,10 @@ import os
 import subprocess
 
 from test_framework.blocktools import (
+    MAX_FUTURE_BLOCK_TIME,
+    TIME_GENESIS_BLOCK,
     create_block,
     create_coinbase,
-    TIME_GENESIS_BLOCK,
 )
 from test_framework.messages import (
     CBlockHeader,
@@ -53,6 +54,7 @@ from test_framework.wallet import MiniWallet
 HEIGHT = 200  # blocks mined
 TIME_RANGE_STEP = 600  # ten-minute steps
 TIME_RANGE_MTP = TIME_GENESIS_BLOCK + (HEIGHT - 6) * TIME_RANGE_STEP
+TIME_RANGE_TIP = TIME_GENESIS_BLOCK + (HEIGHT - 1) * TIME_RANGE_STEP
 TIME_RANGE_END = TIME_GENESIS_BLOCK + HEIGHT * TIME_RANGE_STEP
 
 
@@ -65,6 +67,7 @@ class BlockchainTest(BitcoinTestFramework):
     def run_test(self):
         self.wallet = MiniWallet(self.nodes[0])
         self.mine_chain()
+        self._test_max_future_block_time()
         self.restart_node(0, extra_args=['-stopatheight=207', '-prune=1'])  # Set extra args with pruning after rescan is complete
 
         self._test_getblockchaininfo()
@@ -85,6 +88,19 @@ class BlockchainTest(BitcoinTestFramework):
             self.generate(self.wallet, 1)
         assert_equal(self.nodes[0].getblockchaininfo()['blocks'], HEIGHT)
 
+    def _test_max_future_block_time(self):
+        self.stop_node(0)
+        self.log.info("A block tip of more than MAX_FUTURE_BLOCK_TIME in the future raises an error")
+        self.nodes[0].assert_start_raises_init_error(
+            extra_args=[f"-mocktime={TIME_RANGE_TIP - MAX_FUTURE_BLOCK_TIME - 1}"],
+            expected_msg=": The block database contains a block which appears to be from the future."
+            " This may be due to your computer's date and time being set incorrectly."
+            f" Only rebuild the block database if you are sure that your computer's date and time are correct.{os.linesep}"
+            "Please restart with -reindex or -reindex-chainstate to recover.",
+        )
+        self.log.info("A block tip of MAX_FUTURE_BLOCK_TIME in the future is fine")
+        self.start_node(0, extra_args=[f"-mocktime={TIME_RANGE_TIP - MAX_FUTURE_BLOCK_TIME}"])
+
     def _test_getblockchaininfo(self):
         self.log.info("Test getblockchaininfo")
 

test/functional/test_framework/blocktools.py

@@ -50,6 +50,8 @@ MAX_BLOCK_SIGOPS_WEIGHT = MAX_BLOCK_SIGOPS * WITNESS_SCALE_FACTOR
 # Genesis block time (regtest)
 TIME_GENESIS_BLOCK = 1296688602
 
+MAX_FUTURE_BLOCK_TIME = 2 * 60 * 60
+
 # Coinbase transaction outputs can only be spent after this number of new blocks (network rule)
 COINBASE_MATURITY = 100