Details
- Reviewers
jasonbcox deadalnix - Group Reviewers
Restricted Project Restricted Owners Package (Owns No Changed Paths) - Commits
- rSTAGINGa57aebfd30e6: [Part 4 of 5] Add a CChainState class to clarify internal interfaces
rABCa57aebfd30e6: [Part 4 of 5] Add a CChainState class to clarify internal interfaces
make VERBOSE=1 check && ./test/functional/test_runner.py
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- arcpatch-D1971 (branched from master)
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 5399 Build 8860: Bitcoin ABC Buildbot (legacy) Build 8859: arc lint + arc unit
Event Timeline
| 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 |
| 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.. |
| src/validation.cpp | ||
|---|---|---|
| 3188 ↗ | (On Diff #7886) | this function was changed from void to bool as it's return type. was that intended? |
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 |
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... |