Merge bitcoin/bitcoin#29524: p2p: Don't consider blocks mutated if they don't connect to known prev block

a1fbde0ef7cf6c94d4a5181f8ceb327096713160 p2p: Don't consider blocks mutated if they don't connect to known prev block (Greg Sanders)

Pull request description:

  Followup to https://github.com/bitcoin/bitcoin/pull/29412 to revert some of the behavior change that was likely unintentional.

  Based on comments from https://github.com/bitcoin/bitcoin/pull/29412#discussion_r1507499192

ACKs for top commit:
  0xB10C:
    utACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160
  dergoegge:
    Code review ACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160
  Sjors:
    ACK a1fbde0ef7cf6c94d4a5181f8ceb327096713160
  sr-gi:
    tACK https://github.com/bitcoin/bitcoin/commit/a1fbde0ef7cf6c94d4a5181f8ceb327096713160

Tree-SHA512: be6204c8cc57b271d55c1d02a5c77d03a37738d91cb5ac164f483b0bab3991c24679c5ff02fbaa52bf37ee625874b63f4c9f7b39ad6fd5f3a25386567a0942e4

2 files changed, 23 insertions, 2 deletions

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 5c3ec5f700..99ae0e8fa1 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -4721,7 +4721,8 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
 
         const CBlockIndex* prev_block{WITH_LOCK(m_chainman.GetMutex(), return m_chainman.m_blockman.LookupBlockIndex(pblock->hashPrevBlock))};
 
-        if (IsBlockMutated(/*block=*/*pblock,
+        // Check for possible mutation if it connects to something we know so we can check for DEPLOYMENT_SEGWIT being active
+        if (prev_block && IsBlockMutated(/*block=*/*pblock,
                            /*check_witness_root=*/DeploymentActiveAfter(prev_block, m_chainman, Consensus::DEPLOYMENT_SEGWIT))) {
             LogDebug(BCLog::NET, "Received mutated block from peer=%d\n", peer->m_id);
             Misbehaving(*peer, 100, "mutated block");
diff --git a/test/functional/p2p_mutated_blocks.py b/test/functional/p2p_mutated_blocks.py
index 20f618dc6e..737edaf5bf 100755
--- a/test/functional/p2p_mutated_blocks.py
+++ b/test/functional/p2p_mutated_blocks.py
@@ -31,6 +31,11 @@ class MutatedBlocksTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 1
+        self.extra_args = [
+            [
+                "-testactivationheight=segwit@1", # causes unconnected headers/blocks to not have segwit considered deployed
+            ],
+        ]
 
     def run_test(self):
         self.wallet = MiniWallet(self.nodes[0])
@@ -74,7 +79,7 @@ class MutatedBlocksTest(BitcoinTestFramework):
 
         # Attempt to clear the honest relayer's download request by sending the
         # mutated block (as the attacker).
-        with self.nodes[0].assert_debug_log(expected_msgs=["bad-txnmrklroot, hashMerkleRoot mismatch"]):
+        with self.nodes[0].assert_debug_log(expected_msgs=["Block mutated: bad-txnmrklroot, hashMerkleRoot mismatch"]):
             attacker.send_message(msg_block(mutated_block))
         # Attacker should get disconnected for sending a mutated block
         attacker.wait_for_disconnect(timeout=5)
@@ -92,5 +97,20 @@ class MutatedBlocksTest(BitcoinTestFramework):
         honest_relayer.send_and_ping(block_txn)
         assert_equal(self.nodes[0].getbestblockhash(), block.hash)
 
+        # Check that unexpected-witness mutation check doesn't trigger on a header that doesn't connect to anything
+        assert_equal(len(self.nodes[0].getpeerinfo()), 1)
+        attacker = self.nodes[0].add_p2p_connection(P2PInterface())
+        block_missing_prev = copy.deepcopy(block)
+        block_missing_prev.hashPrevBlock = 123
+        block_missing_prev.solve()
+
+        # Attacker gets a DoS score of 10, not immediately disconnected, so we do it 10 times to get to 100
+        for _ in range(10):
+            assert_equal(len(self.nodes[0].getpeerinfo()), 2)
+            with self.nodes[0].assert_debug_log(expected_msgs=["AcceptBlock FAILED (prev-blk-not-found)"]):
+                attacker.send_message(msg_block(block_missing_prev))
+        attacker.wait_for_disconnect(timeout=5)
+
+
 if __name__ == '__main__':
     MutatedBlocksTest().main()