Page MenuHomePhabricator

[Part 4 of 5] Add a CChainState class to clarify internal interfaces
ClosedPublic

Authored by schancel on Oct 24 2018, 18:51.

Details

Summary

Create initial CChainState to hold chain state information

Backport of Core PR 10279 Commit fd4d80a

Depends on D1970
T572

Test Plan
make VERBOSE=1 check && ./test/functional/test_runner.py

Diff Detail

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

Event Timeline

Owners added a reviewer: Restricted Owners Package.Mar 31 2019, 20:07

Remove GlobalState errorenous comment.

src/validation.cpp
3189 ↗(On Diff #7886)

Kind of unsure if some of these other functions should be rolled up into CChainState. Can be refactored in a later commit for clarity if there's a good reason to put them there. For now, only this function needs internal state of setBlockIndexCandidates

jasonbcox requested changes to this revision.Apr 1 2019, 22:58
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/validation.cpp
3208 ↗(On Diff #7886)

Rather than add global access here, UpdateFlags() should be added to CChainState.

3213 ↗(On Diff #7886)

ditto

3228 ↗(On Diff #7886)

This should also be added to CChainState. It appears to not be used outside of it's implementation.

5192 ↗(On Diff #7886)

This line isn't actually applied in the original commit. In the interest of fewer merge conflicts, lets revert this. It's fine otherwise though..

This revision now requires changes to proceed.Apr 1 2019, 22:58
src/validation.cpp
3188 ↗(On Diff #7886)

this function was changed from void to bool as it's return type. was that intended?

Move UpdateFlags items into CChainState, rebase, and address other minor nit.

deadalnix requested changes to this revision.Apr 4 2019, 15:30
deadalnix added a subscriber: deadalnix.

Overall it looks good, but there are discrepencies with the original code that need to be explained and possibly missing backports.

Also pindexFinalized should really be moved in CChainState, but that can be done later on.

src/chain.h
242 ↗(On Diff #7936)

How come this is here and not in validation.h ? Are there some backports missing?

src/validation.cpp
62 ↗(On Diff #7936)

What happened to enum DisconnectResult ?

219 ↗(On Diff #7936)

Fix comment. Also, clearly that comment doesn't match core source code, so there is something missing here as well.

2952 ↗(On Diff #7936)

dito

This revision now requires changes to proceed.Apr 4 2019, 15:30

Fix the comments and this change should be good.

src/chain.h
242 ↗(On Diff #7936)

It was moved to chain.h in this diff: D712

src/validation.cpp
62 ↗(On Diff #7936)

Moved into undo.h: D364

2952 ↗(On Diff #7936)

This comment was effectively removed in this diff: D1182 It's probably best to add it back for backporting purposes...

Rebase and fix comments.

This revision is now accepted and ready to land.Apr 8 2019, 20:07
This revision was automatically updated to reflect the committed changes.