Sanity assert GetAncestor() != nullptr where appropriate

Add sanity asserts for return value of `CBlockIndex::GetAncestor()` where appropriate.

In validation.cpp `CheckSequenceLocks`, check the return value of `tip->GetAncestor(maxInputHeight)` stored into `lp->maxInputBlock`. If it ever returns `nullptr` because the ancestor isn't found, it's going to be a bad bug to keep going, since a `LockPoints` object with the `maxInputBlock` member set to `nullptr` signifies no relative lock time.

In the other places, the added asserts would prevent accidental dereferencing of a null pointer which is undefined behavior.

Co-Authored-By: Aurèle Oulès <aurele@oules.com>
Co-Authored-By: danra <danra@users.noreply.github.com>

1 files changed, 13 insertions, 9 deletions

diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp
index f6c1d1efad..82a4257c76 100644
--- a/src/test/miner_tests.cpp
+++ b/src/test/miner_tests.cpp
@@ -446,16 +446,18 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
     BOOST_CHECK(CheckFinalTxAtTip(m_node.chainman->ActiveChain().Tip(), CTransaction{tx})); // Locktime passes
     BOOST_CHECK(!TestSequenceLocks(CTransaction{tx})); // Sequence locks fail
 
-    for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
-        m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast
-
+    const int SEQUENCE_LOCK_TIME = 512; // Sequence locks pass 512 seconds later
+    for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i)
+        m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i)->nTime += SEQUENCE_LOCK_TIME; // Trick the MedianTimePast
     {
         CBlockIndex* active_chain_tip = m_node.chainman->ActiveChain().Tip();
-        BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, prevheights, CreateBlockIndex(active_chain_tip->nHeight + 1, active_chain_tip))); // Sequence locks pass 512 seconds later
+        BOOST_CHECK(SequenceLocks(CTransaction(tx), flags, prevheights, CreateBlockIndex(active_chain_tip->nHeight + 1, active_chain_tip)));
     }
 
-    for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
-        m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i)->nTime -= 512; //undo tricked MTP
+    for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i) {
+        CBlockIndex* ancestor{Assert(m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i))};
+        ancestor->nTime -= SEQUENCE_LOCK_TIME; // undo tricked MTP
+    }
 
     // absolute height locked
     tx.vin[0].prevout.hash = txFirst[2]->GetHash();
@@ -500,9 +502,11 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity)
     // but relative locked txs will if inconsistently added to mempool.
     // For now these will still generate a valid template until BIP68 soft fork
     BOOST_CHECK_EQUAL(pblocktemplate->block.vtx.size(), 3U);
-    // However if we advance height by 1 and time by 512, all of them should be mined
-    for (int i = 0; i < CBlockIndex::nMedianTimeSpan; i++)
-        m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i)->nTime += 512; //Trick the MedianTimePast
+    // However if we advance height by 1 and time by SEQUENCE_LOCK_TIME, all of them should be mined
+    for (int i = 0; i < CBlockIndex::nMedianTimeSpan; ++i) {
+        CBlockIndex* ancestor{Assert(m_node.chainman->ActiveChain().Tip()->GetAncestor(m_node.chainman->ActiveChain().Tip()->nHeight - i))};
+        ancestor->nTime += SEQUENCE_LOCK_TIME; // Trick the MedianTimePast
+    }
     m_node.chainman->ActiveChain().Tip()->nHeight++;
     SetMockTime(m_node.chainman->ActiveChain().Tip()->GetMedianTimePast() + 1);