Page MenuHomePhabricator

Sanity assert GetAncestor() != nullptr where appropriate
ClosedPublic

Authored by PiRK on Mar 6 2023, 09:36.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC70912f9a54fd: Sanity assert GetAncestor() != nullptr where appropriate
Summary

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>

This is a backport of core#24804

Test Plan

ninja all check-all

Event Timeline

PiRK requested review of this revision.Mar 6 2023, 09:36
Fabien requested changes to this revision.Mar 6 2023, 12:46
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/test/miner_tests.cpp
489 ↗(On Diff #38308)

How come the test passes ???

583 ↗(On Diff #38308)
This revision now requires changes to proceed.Mar 6 2023, 12:46
src/test/miner_tests.cpp
489 ↗(On Diff #38308)

The following test either don't set the tx ntime or set it relative to the tip's MTP. This is actually useless.
I will fix it anyway, it doesn't hurt either to undo the change.

fix wrong sign in miner_tests.cpp and fix a comment

This revision is now accepted and ready to land.Mar 6 2023, 13:49