Move block finalization out of ConnectTip
AbandonedPublic
Actions

Authored by sdulfari on Jan 18 2023, 00:42.

Details

Reviewers

Group Reviewers

Restricted Project

Summary

The plan is to introduce block parking policies after ConnectTip since we can
be certain the block is valid. This patch moves block finalization logic so
that we do not advance finalization on a possibly parked chain.

Constness of FindBlockToFinalize was changed to better indicate that the
returned block index may be modified via InvalidChainFound but pindexNew
is not expected to change.

Test Plan

ninja check-extended

Diff Detail

Repository

rABC Bitcoin ABC

Branch

mv-block-finalization

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 21764
Build 43163: Build Diff	build-diff · build-debug · build-without-wallet · lint-circular-dependencies · build-clang · build-clang-tidy
Build 43162: arc lint + arc unit

Event Timeline

sdulfari created this revision.Jan 18 2023, 00:42

Herald added a reviewer: Restricted Project. · View Herald TranscriptJan 18 2023, 00:42

sdulfari requested review of this revision.Jan 18 2023, 00:42

Harbormaster completed remote builds in B21764: Diff 37560.Jan 18 2023, 00:56

Fabien requested changes to this revision.Jan 18 2023, 09:03

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/validation.cpp
2655	revert
3124	pindexToFinalize should still be const...
3128	A few remarks: The error cases in this function can only happen from weird RPC calls One of these error case is untested... And the most important, if your actual tip (and not a not-yet-connected candidate at this point) is on an invalid chain, you have a much bigger issue An assert is a better solution imo. If this fails shutting down the node is probably the best move, and reindex.
3129	... because InvalidChainFound doesn't touch pindexToFinalize and should be changed to take a const CBlockIndex * instead