Page MenuHomePhabricator

validation: change UpdateTip for multiple chainstates
ClosedPublic

Authored by PiRK on Oct 17 2022, 15:03.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCa1093fa5f579: validation: change UpdateTip for multiple chainstates
Summary

Only perform certain behavior (namely that related to servicing
the getblocktemplate RPC call) for the active chainstate when
calling UpdateTip.

Co-authored-by: Jon Atack <jon@atack.com>

This is a backport of core#21526 [3/12]
https://github.com/bitcoin/bitcoin/pull/21526/commits/b217020df78bc981d221fe04497c831120ef969f
Depends on D12277

Notes:

  • the main difference from the source material is that we do not have the code handling BIP9 deployments in UpdateTip. As a result, there is no need for the warning_messages arguments in UpdateTipLog
  • the const qualifier for the CblockIndex *pindexNew argument in UpdateTip was missed in D1972
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.Oct 17 2022, 15:03
Fabien added inline comments.
src/validation.cpp
2138 ↗(On Diff #35758)
2147 ↗(On Diff #35758)

Does that make sense to always log (with no category for filtering) when this is not the active chain ? What's the result in the log ?

src/validation.h
1175 ↗(On Diff #35758)

This was missed in D1972

this->CoinsTip();

src/validation.cpp
2147 ↗(On Diff #35758)

I think it makes sense. The philosophy here is that the background chainstate is the important one, the one that goes through the complete IBD and verifies everything. And the assumed valid chain in foreground is just temporary, until the IBD caches up with the current tip.

It is going to be verbose for the whole duration of the IBD, and the background chain will print most of the messages, so it will probably make the new blocks added by the active chain every 10 minutes a bit hard to spot, but that's not a problem with a negative grep : tail -f debug.log | grep -v [background validation] "

I vote for sticking with the upstream choice on this one. In the current master branch of Core this is still logged nconditionally.

This revision is now accepted and ready to land.Oct 18 2022, 09:06