Page MenuHomePhabricator

[avalanche] pass the processor to Chainstate::AvalancheFinalizeBlock
ClosedPublic

Authored by PiRK on Jun 4 2024, 10:15.

Details

Summary

Pass a reference rather than a pointer, so that this function only ever gets called if there is an avalanche processor initialized.

This concludes the removal of g_avalanche access in Chainstate functions.

Depends on D16313

Test Plan

ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

PiRK requested review of this revision.Jun 4 2024, 10:15
Fabien requested changes to this revision.Jun 4 2024, 11:58
Fabien added a subscriber: Fabien.
Fabien added inline comments.
src/validation.cpp
2916 ↗(On Diff #48083)

You need to update isAvalancheEnabled to include the nullptr check. This is actually never null but this will prevent introducing bugs as we go because of that assumption

This revision now requires changes to proceed.Jun 4 2024, 11:58
src/validation.cpp
2916 ↗(On Diff #48083)

OK. But in this place nothing is being done with g_avalanche, so this particular callsite does not lead to potential bugs.

PiRK edited the summary of this revision. (Show Details)

rebase on D16313 (now we can rely on the processor pointer to determine if avalanche is enabled)

Fabien requested changes to this revision.Jun 7 2024, 08:45
Fabien added inline comments.
src/validation.cpp
3695 ↗(On Diff #48179)

This looks like a place where you want to use a reference. Calling this function makes no sense if avalanche is disabled, so you want to check at callsite and not call this at all if avalanche is disabled.

3702 ↗(On Diff #48179)

If avalanche is null this log is useless

This revision now requires changes to proceed.Jun 7 2024, 08:45
PiRK edited the summary of this revision. (Show Details)

pass a reference. This happens in a scope in which we have the guarantee that avalanche is initialized.

Initialize a couple of processors in tests, where this function is called.

This revision is now accepted and ready to land.Jun 7 2024, 15:38