IBD using chainwork instead of height and not using header timestamps #9053

gmaxwell · 2016-11-01T00:47:10Z

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.

This is the first step in a larger change to eliminate checkpoints
completely, but it's independently a good idea.

This PR makes IsInitialBlockDownload no longer use header-only timestamps.

This avoids a corner case (mostly visible on testnet) where bogus
headers can keep nodes in IsInitialBlockDownload.

maflcko · 2016-11-01T10:05:53Z

utACK f3f8d2e

Nit: GetTotalBlocksEstimate is no longer used (only in tests). Feel free to remove that now.

maflcko · 2016-11-01T10:06:34Z

doc/release-process.md

@@ -47,6 +47,7 @@ Update the following:
 - `doc/README.md` and `doc/README_windows.txt`
 - `doc/Doxyfile`: `PROJECT_NUMBER` contains the full version
 - `contrib/gitian-descriptors/*.yml`: usually one'd want to do this on master after branching off the release - but be sure to at least do it before a new major release
+- `src/chainparams.cpp`: periodically update nMinimumChainWork with information from the getblockchaininfo rpc.


Nit: I think you want to move this up to the section " Before every minor and major release:"

Leviathn · 2016-11-02T00:57:18Z

utACK - certainly a more reliable metric.

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.

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.

This avoids a corner case (mostly visible on testnet) where bogus headers can keep nodes in IsInitialBlockDownload.

gmaxwell · 2016-11-02T04:10:44Z

@MarcoFalke nits addressed, I think.

rebroad · 2016-11-02T05:25:52Z

src/chainparams.cpp

@@ -96,6 +96,9 @@ class CMainParams : public CChainParams {
        consensus.vDeployments[Consensus::DEPLOYMENT_SEGWIT].nStartTime = 1479168000; // November 15th, 2016.
        consensus.vDeployments[Consensus::DEPLOYMENT_SEGWIT].nTimeout = 1510704000; // November 15th, 2017.

+        // The best chain should have at least this much work.
+        consensus.nMinimumChainWork = uint256S("0x0000000000000000000000000000000000000000002cb971dd56d1c583c20f90");


Where is this number from please?

@rebroad It is mentioned in the doc change in this very pull...

You can just run the getblockchaininfo rpc per the instructions and observe that its a number equal to or greater than that in order to verify it. :)

maflcko · 2016-11-02T09:07:53Z

re-utACK e141beb

laanwj · 2016-11-02T19:53:34Z

utACK e141beb

sipa · 2016-11-03T07:04:04Z

utACK e141beb

sipa · 2016-11-03T07:06:19Z

I wonder if this should be backported to 0.13.2, as it fixes a visible issue on testnet.

…er timestamps e141beb IsInitialBlockDownload no longer uses header-only timestamps. (Gregory Maxwell) 2082b55 Remove GetTotalBlocksEstimate and checkpoint tests that test nothing. (Gregory Maxwell) fd46136 IBD check uses minimumchain work instead of checkpoints. (Gregory Maxwell)

jtimon · 2016-11-03T22:11:37Z

utACK e141beb very clean to read commit by commit.

mruddy · 2016-11-17T12:58:40Z

src/main.cpp

-    return state;
+    if (chainActive.Tip()->nChainWork < UintToArith256(chainParams.GetConsensus().nMinimumChainWork))
+        return true;
+    if (chainActive.Tip()->GetBlockTime() < (GetTime() - nMaxTipAge))


Any particular reason to use GetTime here over GetAdjustedTime?

…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)

InitialBlockDownload upstream changes Cherry-picked from the following upstream PRs: - bitcoin/bitcoin#7208 - bitcoin/bitcoin#8007 - bitcoin/bitcoin#9053 - Excluding second commit (requires bitcoin/bitcoin#8865) - bitcoin/bitcoin#10388

maflcko reviewed Nov 1, 2016

View reviewed changes

maflcko added Refactoring Consensus labels Nov 1, 2016

dcousens approved these changes Nov 2, 2016

View reviewed changes

gmaxwell added 3 commits November 2, 2016 01:47

IsInitialBlockDownload no longer uses header-only timestamps.

e141beb

This avoids a corner case (mostly visible on testnet) where bogus headers can keep nodes in IsInitialBlockDownload.

dcousens mentioned this pull request Nov 2, 2016

Ignore getheaders prior to passing all checkpoints. #9061

Closed

rebroad reviewed Nov 2, 2016

View reviewed changes

sipa merged commit e141beb into bitcoin:master Nov 3, 2016

laanwj added this to the 0.13.2 milestone Nov 3, 2016

laanwj added Needs backport and removed Needs backport labels Nov 3, 2016

laanwj removed this from the 0.13.2 milestone Nov 3, 2016

jtimon mentioned this pull request Nov 3, 2016

Use a proper factory for creating chainparams #8855

Merged

6 tasks

jtimon mentioned this pull request Nov 3, 2016

Testchains: Introduce custom chain whose constructor... #8994

Closed

12 tasks

mruddy reviewed Nov 17, 2016

View reviewed changes

gmaxwell mentioned this pull request Dec 6, 2016

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

Merged

codablock mentioned this pull request Aug 21, 2017

Backport "assumed valid blocks" feature from Bitcoin 0.13 dashpay/dash#1582

Merged

str4d mentioned this pull request Jan 16, 2018

Bitcoin 0.12 chainparams cleanups zcash/zcash#2217

Merged

str4d mentioned this pull request May 14, 2018

InitialBlockDownload upstream changes zcash/zcash#3263

Merged

scravy mentioned this pull request Nov 13, 2018

WIP: Work in Progress dtr-org/unit-e#212

Closed

92 tasks

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IBD using chainwork instead of height and not using header timestamps #9053

IBD using chainwork instead of height and not using header timestamps #9053

gmaxwell commented Nov 1, 2016 •

edited

Loading

maflcko commented Nov 1, 2016

maflcko Nov 1, 2016

Leviathn commented Nov 2, 2016

gmaxwell commented Nov 2, 2016

rebroad Nov 2, 2016

maflcko Nov 2, 2016

gmaxwell Nov 3, 2016

maflcko commented Nov 2, 2016

laanwj commented Nov 2, 2016

sipa commented Nov 3, 2016

sipa commented Nov 3, 2016

jtimon commented Nov 3, 2016 •

edited

Loading

mruddy Nov 17, 2016

IBD using chainwork instead of height and not using header timestamps #9053

IBD using chainwork instead of height and not using header timestamps #9053

Conversation

gmaxwell commented Nov 1, 2016 • edited Loading

maflcko commented Nov 1, 2016

maflcko Nov 1, 2016

Choose a reason for hiding this comment

Leviathn commented Nov 2, 2016

gmaxwell commented Nov 2, 2016

rebroad Nov 2, 2016

Choose a reason for hiding this comment

maflcko Nov 2, 2016

Choose a reason for hiding this comment

gmaxwell Nov 3, 2016

Choose a reason for hiding this comment

maflcko commented Nov 2, 2016

laanwj commented Nov 2, 2016

sipa commented Nov 3, 2016

sipa commented Nov 3, 2016

jtimon commented Nov 3, 2016 • edited Loading

mruddy Nov 17, 2016

Choose a reason for hiding this comment

gmaxwell commented Nov 1, 2016 •

edited

Loading

jtimon commented Nov 3, 2016 •

edited

Loading