[0.13 Backport] IBD using chainwork instead of height and not using header timestamp (#9053) #9293

Merged
merged 4 commits into from Dec 8, 2016

Projects

None yet

5 participants

@gmaxwell
Member
gmaxwell commented Dec 6, 2016

This is a backport of PR #9053 -- we passed on backporting it right after it was merged, but it's been in master for over a month now with no issue.

This resolves some frequent annoying misbehavior on testnet (in theory the problems it solves are also possible on mainnet but aren't likely there without miner funny business).

@MarcoFalke
Member

GetTotalBlocksEstimate() is still used in another place in main.

Style nit: Please use the backport script by @luke-jr, so that there is a reference to the original pull and commit hash? (@luke-jr would you mind to put that script in our repo?)

TheBlueMatt and others added some commits Sep 30, 2016
@TheBlueMatt @gmaxwell TheBlueMatt Remove duplicate nBlocksEstimate cmp (we already checked IsIBD())
Github-Pull: #8865
Rebased-From: 0278fb5
4c71fc4
@gmaxwell gmaxwell IBD check uses minimumchain work instead of checkpoints.
This introduces a 'minimum chain work' chainparam which is intended
 to be the known amount of work in the chain for the network at the
 time of software release.  If you don't have this much work, you're
 not yet caught up.

This is used instead of the count of blocks test from checkpoints.

This criteria is trivial to keep updated as there is no element of
subjectivity, trust, or position dependence to it. It is also a more
reliable metric of sync status than a block count.

Github-Pull: #9053
Rebased-From: fd46136
ad20cdd
@gmaxwell gmaxwell Remove GetTotalBlocksEstimate and checkpoint tests that test nothing.
GetTotalBlocksEstimate is no longer used and it was the only thing
 the checkpoint tests were testing.

Since checkpoints are on their way out it makes more sense to remove
 the test file than to cook up a new pointless test.

Github-Pull: #9053
Rebased-From: 2082b55
5b93eee
@gmaxwell gmaxwell IsInitialBlockDownload no longer uses header-only timestamps.
This avoids a corner case (mostly visible on testnet) where bogus
 headers can keep nodes in IsInitialBlockDownload.

Github-Pull: #9053
Rebased-From: e141beb
5998a09
@gmaxwell
Member
gmaxwell commented Dec 6, 2016

@MarcoFalke K. I believe I addressed your comments (I just imitated the style by hand.)

@MarcoFalke
Member

utACK 5998a09

@fanquake fanquake added this to the 0.13.2 milestone Dec 8, 2016
@laanwj
Member
laanwj commented Dec 8, 2016

I believe I addressed your comments (I just imitated the style by hand.)

LGTM

@laanwj laanwj merged commit 5998a09 into bitcoin:0.13 Dec 8, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@laanwj laanwj added a commit that referenced this pull request Dec 8, 2016
@laanwj laanwj Merge #9293: [0.13 Backport] IBD using chainwork instead of height an…
…d not using header timestamp (#9053)


5998a09 IsInitialBlockDownload no longer uses header-only timestamps. (Gregory Maxwell)
5b93eee Remove GetTotalBlocksEstimate and checkpoint tests that test nothing. (Gregory Maxwell)
ad20cdd IBD check uses minimumchain work instead of checkpoints. (Gregory Maxwell)
4c71fc4 Remove duplicate nBlocksEstimate cmp (we already checked IsIBD()) (Matt Corallo)
e591c10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment