Page MenuHomePhabricator

Do not unpark children of blocks outside of the RPC
ClosedPublic

Authored by deadalnix on Nov 13 2018, 02:42.

Details

Summary

As per title. When unparking chain automatically, we do not want to unpark all children by default. Only the RPC should do this.

Depends on D2004

Test Plan
make check
./test/functional/test_runner.py --extended

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Nov 13 2018, 02:42
Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 13 2018, 02:42
deadalnix edited the summary of this revision. (Show Details)Nov 13 2018, 02:42
deadalnix changed the edit policy from "All Users" to "Restricted Project (Project)".
jasonbcox edited the summary of this revision. (Show Details)Nov 13 2018, 02:42
deadalnix edited the summary of this revision. (Show Details)Nov 13 2018, 02:43
deadalnix changed the visibility from "Public (No Login Required)" to "Restricted Project (Project)".
jasonbcox edited the summary of this revision. (Show Details)Nov 13 2018, 02:43
jasonbcox requested changes to this revision.Nov 13 2018, 02:55
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/validation.cpp
2923 ↗(On Diff #5788)

Seeing that both UnparkBlockAndChildren() and UnparkBlock() return true, it's probably better design-wise to make UnparkBlockImpl return bool type and have it return true at the end of the function. Then those two functions just return UnparkBlockImpl(...);

This revision now requires changes to proceed.Nov 13 2018, 02:55
deadalnix updated this revision to Diff 5790.Nov 13 2018, 03:07

Have UnparkBlockImpl return true

jasonbcox requested changes to this revision.Nov 13 2018, 03:18

Ideally this would come with a test. Something like node0: A -> B (parked) --------> C -> D (chaintip) B -> E (parked-parent and parked). node1 goes through IBD and E remains parked.

This revision now requires changes to proceed.Nov 13 2018, 03:18
deadalnix added a child revision: Restricted Differential Revision.Nov 13 2018, 16:11
jasonbcox accepted this revision.Nov 18 2018, 01:19

Created a task to cover the test in the future: T484 (task has limited visibility)

This revision is now accepted and ready to land.Nov 18 2018, 01:19
schancel accepted this revision.Nov 18 2018, 01:27
deadalnix changed the visibility from "Restricted Project (Project)" to "Public (No Login Required)".Nov 20 2018, 16:28
This revision was automatically updated to reflect the committed changes.