Merge bitcoin/bitcoin#24804: Sanity assert GetAncestor() != nullptr where appropriate

308dd2e93e92f4cac4e7d75478316af9bb2b77b8 Sanity assert GetAncestor() != nullptr where appropriate (Adam Jonas)

Pull request description:

  Re-opening #17232. I have rebased the PR and addressed jonatack's nit suggestions.

  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: Adam Jonas <jonas@chaincode.com>
  Co-Authored-By: danra <danra@users.noreply.github.com>

ACKs for top commit:
  jonatack:
    ACK 308dd2e93e92f4cac4e7d75478316af9bb2b77b8

Tree-SHA512: 5bfdaab1499607ae2c3cd3e2e9e8c37850bfd0e327e680f4e36c81f9c6d98a543af78ecfac1ab0e06325d264412615a04d52005875780c7db2a4d81bd2d2259a

1 files changed, 10 insertions, 4 deletions

diff --git a/src/validation.cpp b/src/validation.cpp
index 4614fc1402..b5d6a66088 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -250,7 +250,12 @@ bool CheckSequenceLocksAtTip(CBlockIndex* tip,
                     maxInputHeight = std::max(maxInputHeight, height);
                 }
             }
-            lp->maxInputBlock = tip->GetAncestor(maxInputHeight);
+            // tip->GetAncestor(maxInputHeight) should never return a nullptr
+            // because maxInputHeight is always less than the tip height.
+            // It would, however, be a bad bug to continue execution, since a
+            // LockPoints object with the maxInputBlock member set to nullptr
+            // signifies no relative lock time.
+            lp->maxInputBlock = Assert(tip->GetAncestor(maxInputHeight));
         }
     }
     return EvaluateSequenceLocks(index, lockPair);
@@ -4108,10 +4113,11 @@ bool CChainState::ReplayBlocks()
     // Roll forward from the forking point to the new tip.
     int nForkHeight = pindexFork ? pindexFork->nHeight : 0;
     for (int nHeight = nForkHeight + 1; nHeight <= pindexNew->nHeight; ++nHeight) {
-        const CBlockIndex* pindex = pindexNew->GetAncestor(nHeight);
-        LogPrintf("Rolling forward %s (%i)\n", pindex->GetBlockHash().ToString(), nHeight);
+        const CBlockIndex& pindex{*Assert(pindexNew->GetAncestor(nHeight))};
+
+        LogPrintf("Rolling forward %s (%i)\n", pindex.GetBlockHash().ToString(), nHeight);
         uiInterface.ShowProgress(_("Replaying blocks…").translated, (int) ((nHeight - nForkHeight) * 100.0 / (pindexNew->nHeight - nForkHeight)) , false);
-        if (!RollforwardBlock(pindex, cache)) return false;
+        if (!RollforwardBlock(&pindex, cache)) return false;
     }
 
     cache.SetBestBlock(pindexNew->GetBlockHash());