Page MenuHomePhabricator

clarify GetBlockScriptFlags
ClosedPublic

Authored by markblundeberg on Jun 18 2019, 03:42.

Details

Summary

The effect of GetBlockScriptFlags is to return the "next block" flags
for a given block index, however the name, comments and parameter names
seem to contradict this. Fortunately, all actual code using this function
is doing the right thing. This Diff tries to make it all clear; besides
a change in an obscure error message, there is no functional change as
only comments and internal function/variable names are changed.

This may break backports but that is good since the function actually does
something different than in Core.

(Note: Calculating the next-block flags is clean & nice, since it means
that calling this function with the chain tip gives us exactly the
consensus flags that we should be applying to the mempool, even around
activation times. This can be contrasted to Core, where the function of
the same name returns flags for the given block instead; that creates a
weird off-by-one error for mempool acceptance around activation times.)

Test Plan

make check

Diff Detail

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

Event Timeline

src/validation.cpp
1778 ↗(On Diff #9511)

This is of course the most important callsite for this function, and from the code we can see that renaming to GetNextBlockScriptFlags makes a lot of sense.

This revision is now accepted and ready to land.Jun 18 2019, 13:39