From 9986608ba93de040490ee0d5584ea33e605f1df0 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Wed, 15 Apr 2020 17:07:44 -0400 Subject: [PATCH] test: Verify findCommonAncestor always initializes outputs Also add code comment to clarify surprising code noted by practicalswift https://github.com/bitcoin/bitcoin/pull/18657#issuecomment-614278450 --- src/interfaces/chain.cpp | 2 ++ src/test/interfaces_tests.cpp | 6 ++++++ 2 files changed, 8 insertions(+) diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index c8311b298..0e7641ae3 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -275,6 +275,8 @@ public: 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& coins) override { return FindCoins(m_node, coins); } diff --git a/src/test/interfaces_tests.cpp b/src/test/interfaces_tests.cpp index fab357175..b0d4de89f 100644 --- a/src/test/interfaces_tests.cpp +++ b/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)