Merge bitcoin#18660: test: Verify findCommonAncestor always initializes outputs

9986608 test: Verify findCommonAncestor always initializes outputs (Russell Yanofsky)

Pull request description:

  Also add code comment to clarify surprising code noted by practicalswift
  bitcoin#18657 (comment)

ACKs for top commit:
  MarcoFalke:
    ACK 9986608
  jonatack:
    ACK 9986608 modulo @practicalswift's bitcoin#18660 (comment)

Tree-SHA512: d79c910291d68b770ef4b09564d274c0e19a6acf43ef1a6691dc889e3944ab3462b86056eeb794fd0c6f2464cfad6cc00711a833f84b32079c69ef9b3c8da24c

Author: MarcoFalke

src/interfaces/chain.cpp

@@ -275,6 +275,8 @@ class ChainImpl : public Chain
         const CBlockIndex* block1 = LookupBlockIndex(block_hash1);
         const CBlockIndex* block2 = LookupBlockIndex(block_hash2);
         const CBlockIndex* ancestor = block1 && block2 ? LastCommonAncestor(block1, block2) : nullptr;
+        // Using & instead of && below to avoid short circuiting and leaving
+        // output uninitialized.
         return FillBlock(ancestor, ancestor_out, lock) & FillBlock(block1, block1_out, lock) & FillBlock(block2, block2_out, lock);
     }
     void findCoins(std::map<COutPoint, Coin>& coins) override { return FindCoins(m_node, coins); }

src/test/interfaces_tests.cpp

@@ -116,6 +116,12 @@ BOOST_AUTO_TEST_CASE(findCommonAncestor)
     BOOST_CHECK_EQUAL(orig_height, orig_tip->nHeight);
     BOOST_CHECK_EQUAL(fork_height, orig_tip->nHeight - 10);
     BOOST_CHECK_EQUAL(fork_hash, active[fork_height]->GetBlockHash());
+
+    uint256 active_hash, orig_hash;
+    BOOST_CHECK(!chain->findCommonAncestor(active.Tip()->GetBlockHash(), {}, {}, FoundBlock().hash(active_hash), {}));
+    BOOST_CHECK(!chain->findCommonAncestor({}, orig_tip->GetBlockHash(), {}, {}, FoundBlock().hash(orig_hash)));
+    BOOST_CHECK_EQUAL(active_hash, active.Tip()->GetBlockHash());
+    BOOST_CHECK_EQUAL(orig_hash, orig_tip->GetBlockHash());
 }
 
 BOOST_AUTO_TEST_CASE(hasBlocks)