Merge bitcoin/bitcoin#21009: Remove RewindBlockIndex logic

d831e711cab83c70bf2ded62fe33f484844e73dd [validation] RewindBlockIndex no longer needed (Dhruv Mehta)

Pull request description:

  Closes #17862

  Context from [original comment](https://github.com/bitcoin/bitcoin/issues/17862#issuecomment-744285188) (minor edits):

  `RewindBlockIndex()` is a mechanism to allow nodes to be upgraded after segwit activation, while still keeping their chainstate/datadir in a consistent state. It works as follows:

  - A pre-segwit (i.e. v0.13.0 or older) node is running.
  -  Segwit activates. The pre-segwit node remains sync'ed to the tip, but is not enforcing the new segwit rules.
  - The user upgrades the node to a segwit-aware version (v0.13.1 or newer).
  - On startup, in `AppInitMain()`, `RewindBlockIndex()` is called. This walks the chain backwards from the tip, disconnecting and erasing blocks that from after segwit activation that weren't validated with segwit rules.
  - those blocks are then redownloaded (with witness data) and validated with segwit rules.

  This logic probably isn't required any more since:

  - Segwit activated at height 481824, when the block chain was 130GB and the total number of txs was 250 million. Today, we're at height 667704, the blockchain is over 315GB and the total number of txs is over 600 million. Even if 20% of that added data is witness data (a high estimate), then around 150GB of transactions would need to be rewound to get back to segwit activation height. It'd probably be faster to simply validate from genesis, especially since we won't be validating any scripts before the assumevalid block. It's also unclear whether rewinding 150GB of transactions would even work. It's certainly never been tested.
  - Bitcoin Core v0.13 is hardly used any more. https://luke.dashjr.org/programs/bitcoin/files/charts/software.html shows less than 50 nodes running it. The software was EOL on Aug 1st 2018. It's very unlikely that anyone is running 0.13 and will want to upgrade to 0.22.

  This PR introduces `NeedsRedownload()` which merely checks for insufficiently validated segwit blocks and requests that the user restarts the node with `-reindex`. Reindexing the block files upon restart will make the node rebuild chain state and block index from the `blk*.dat` files on disk. The node won't be able to index the blocks with `BLOCK_OPT_WITNESS`, so they will be missing from the chain and be re-downloaded, with witness data.

  Removing this code allows the following (done in follow-up #21090):

  - removal of tests using `segwitheight=-1` in `p2p_segwit.py`.
  - in turn, that allows us to drop support for `-segwitheight=-1`, which is only supported for that test.
  - that allows us to always set `NODE_WITNESS` in our local services. The only reason we don't do that is to support `-segwitheight=-1`.
  - that in turn allows us to drop all of the `GetLocalServices() & NODE_WITNESS` checks inside `net_processing.cpp`, since our local services would always include `NODE_WITNESS`

ACKs for top commit:
  jnewbery:
    utACK d831e711cab83c70bf2ded62fe33f484844e73dd
  jamesob:
    ACK https://github.com/bitcoin/bitcoin/pull/21009/commits/d831e711cab83c70bf2ded62fe33f484844e73dd
  laanwj:
    Cursory code review ACK d831e711cab83c70bf2ded62fe33f484844e73dd. Agree with the direction of the change, thanks for simplifying the logic here.
  glozow:
    utACK d831e711cab83c70bf2ded62fe33f484844e73dd

Tree-SHA512: 3eddf5121ccd081ad7f15a5c6478ef867083edc8ba0bf1ee759e87bc070ee3d2f0698a3feba8db8dc087987c8452887b6f72cff05b3e178f41cb10a515fb8053

1 files changed, 22 insertions, 11 deletions

diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py
index 54891b07e1..14a4afc2d7 100755
--- a/test/functional/p2p_segwit.py
+++ b/test/functional/p2p_segwit.py
@@ -1956,22 +1956,33 @@ class SegWitTest(BitcoinTestFramework):
     def test_upgrade_after_activation(self):
         """Test the behavior of starting up a segwit-aware node after the softfork has activated."""
 
-        self.restart_node(2, extra_args=["-segwitheight={}".format(SEGWIT_HEIGHT)])
+        # All nodes are caught up and node 2 is a pre-segwit node that will soon upgrade.
+        for n in range(2):
+            assert_equal(self.nodes[n].getblockcount(), self.nodes[2].getblockcount())
+            assert softfork_active(self.nodes[n], "segwit")
+        assert SEGWIT_HEIGHT < self.nodes[2].getblockcount()
+        assert 'segwit' not in self.nodes[2].getblockchaininfo()['softforks']
+
+        # Restarting node 2 should result in a shutdown because the blockchain consists of
+        # insufficiently validated blocks per segwit consensus rules.
+        self.stop_node(2)
+        with self.nodes[2].assert_debug_log(expected_msgs=[
+                f"Witness data for blocks after height {SEGWIT_HEIGHT} requires validation. Please restart with -reindex."], timeout=10):
+            self.nodes[2].start([f"-segwitheight={SEGWIT_HEIGHT}"])
+
+        # As directed, the user restarts the node with -reindex
+        self.start_node(2, extra_args=["-reindex", f"-segwitheight={SEGWIT_HEIGHT}"])
+
+        # With the segwit consensus rules, the node is able to validate only up to SEGWIT_HEIGHT - 1
+        assert_equal(self.nodes[2].getblockcount(), SEGWIT_HEIGHT - 1)
         self.connect_nodes(0, 2)
 
         # We reconnect more than 100 blocks, give it plenty of time
+        # sync_blocks() also verifies the best block hash is the same for all nodes
         self.sync_blocks(timeout=240)
 
-        # Make sure that this peer thinks segwit has activated.
-        assert softfork_active(self.nodes[2], 'segwit')
-
-        # Make sure this peer's blocks match those of node0.
-        height = self.nodes[2].getblockcount()
-        while height >= 0:
-            block_hash = self.nodes[2].getblockhash(height)
-            assert_equal(block_hash, self.nodes[0].getblockhash(height))
-            assert_equal(self.nodes[0].getblock(block_hash), self.nodes[2].getblock(block_hash))
-            height -= 1
+        # The upgraded node should now have segwit activated
+        assert softfork_active(self.nodes[2], "segwit")
 
     @subtest  # type: ignore
     def test_witness_sigops(self):