This fix an issue where the auto-finalization behaviour could cause
unexpected rewind of the finalization chain by an amount of
maxreorgdepth blocks.
Depends on D2130
Details
Details
- Reviewers
deadalnix - Group Reviewers
Restricted Project - Commits
- rSTAGINGa51d18fc5566: Prevent auto-finalization from moving backward the finalization point
rABCa51d18fc5566: Prevent auto-finalization from moving backward the finalization point
./test/functional/abc-finalize-block.py
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- fix_finalization_block
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 4267 Build 6599: Bitcoin ABC Buildbot (legacy) Build 6598: arc lint + arc unit
Event Timeline
| src/validation.cpp | ||
|---|---|---|
| 2325 ↗ | (On Diff #6201) | if (pindexFinalized && pindexFinalized ->GetAncestor(pindex->nHeight) == pindex) {
// The block is already finalized.
return true;
}
// We have a new block to finalize.
pindexFinalized = pindex;
retunr true; |
| test/functional/abc-finalize-block.py | ||
| 47 ↗ | (On Diff #6201) | It would be preferable to designate this block by something that actually identify it. There is something special about this block, and it is not that it is at height 209. |
| 124 ↗ | (On Diff #6201) | I commented on these tests a million times. They do not check anything very interesting. The node could be on any branch the test would pass, but if we add another test case that shift the hight, then we got to modify everything in the test. Please test what's relevant. |
| 158 ↗ | (On Diff #6201) | dito |
| 181 ↗ | (On Diff #6201) | Reformat so it fit on one line. |
| 212 ↗ | (On Diff #6201) | dito |
| src/validation.cpp | ||
|---|---|---|
| 2325 ↗ | (On Diff #6201) | Incidentally, this is what IsBlockFinalized already does. |
| test/functional/abc-finalize-block.py | ||
|---|---|---|
| 171 ↗ | (On Diff #6232) | Maybe change this into assert_equal(node.getfinalizedblockhash(), tip) instead of removing it. |
| test/functional/abc-finalize-block.py | ||
|---|---|---|
| 171 ↗ | (On Diff #6232) | It has been removed because it is a duplicate, already tested at line 162 (164 in new version) |