Sanity assert GetAncestor() != nullptr where appropriate
ClosedPublic
Actions

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

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Mar 6 2023, 09:36

Herald added a reviewer: Restricted Project. · View Herald TranscriptMar 6 2023, 09:36

PiRK requested review of this revision.Mar 6 2023, 09:36

teamcity edited the summary of this revision. (Show Details)Mar 6 2023, 09:36

Harbormaster completed remote builds in B22268: Diff 38308.Mar 6 2023, 09:53

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

PiRK added inline comments.Mar 6 2023, 13:31

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.