New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InitialBlockDownload upstream changes #3263

Merged
merged 5 commits into from Jul 17, 2018

Conversation

8 participants
@str4d
Copy link
Contributor

str4d commented May 14, 2018

Cherry-picked from the following upstream PRs:

@str4d str4d added this to the v1.1.1 milestone May 14, 2018

@str4d str4d added this to In Review in Zcashd Team May 14, 2018

@mdr0id mdr0id added this to Complete in User Support May 14, 2018

@bitcartel bitcartel moved this from Complete to Merged in User Support May 14, 2018

@mdr0id mdr0id removed this from Merged in User Support May 14, 2018

laanwj and others added some commits Dec 14, 2015

Make max tip age an option instead of chainparam
After discussion in #7164 I think this is better.

Max tip age was introduced in #5987 to make it possible to run
testnet-in-a-box. But associating this behavior with the testnet chain
is wrong conceptually, as it is not needed in normal usage.
Should aim to make testnet test the software as-is.

Replace it with a (debug) option `-maxtipage`, which can be
specified only in the specific case.
IsInitialBlockDownload: usually avoid locking
Optimistically test the latch bool before taking the lock.
For all IsInitialBlockDownload calls after the first to return false,
this avoids the need to lock cs_main.
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.

@str4d str4d force-pushed the str4d:ibd-upstream-changes branch from a9a8b2d to be360e8 May 15, 2018

gmaxwell and others added some commits Nov 1, 2016

IsInitialBlockDownload no longer uses header-only timestamps.
This avoids a corner case (mostly visible on testnet) where bogus
 headers can keep nodes in IsInitialBlockDownload.

@str4d str4d force-pushed the str4d:ibd-upstream-changes branch from be360e8 to bbff863 May 15, 2018

@@ -472,6 +472,7 @@ std::string HelpMessage(HelpMessageMode mode)
strUsage += HelpMessageOpt("-limitfreerelay=<n>", strprintf("Continuously rate-limit free transactions to <n>*1000 bytes per minute (default: %u)", 15));
strUsage += HelpMessageOpt("-relaypriority", strprintf("Require high priority for relaying free or low-fee transactions (default: %u)", 0));
strUsage += HelpMessageOpt("-maxsigcachesize=<n>", strprintf("Limit size of signature cache to <n> entries (default: %u)", 50000));
strUsage += HelpMessageOpt("-maxtipage=<n>", strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", DEFAULT_MAX_TIP_AGE));

This comment has been minimized.

@str4d

str4d May 16, 2018

Contributor

This will enable testnet users to configure their nodes to allow mining (if the testnet ever happens to get to a point where the last block was mined beyond the default) without needing to modify the source code.

This comment has been minimized.

@bitcartel

bitcartel Jul 17, 2018

Contributor

Yes, useful and related to #1609

@str4d

This comment has been minimized.

Copy link
Contributor

str4d commented May 17, 2018

@zkbot treeclosed=10

@mdr0id

This comment has been minimized.

Copy link
Contributor

mdr0id commented May 23, 2018

ACK

@str4d str4d modified the milestones: v1.1.1, v2.0.0 May 25, 2018

@bitcartel bitcartel moved this from In Review to Blocked in Zcashd Team Jun 6, 2018

@str4d str4d moved this from Blocked to Review Backlog in Zcashd Team Jun 22, 2018

@str4d str4d modified the milestones: v1.1.2, v2.0.0 Jul 2, 2018

@str4d str4d moved this from Review Backlog to In Review in Zcashd Team Jul 2, 2018

@str4d str4d moved this from In Review to Review Backlog in Zcashd Team Jul 3, 2018

@str4d str4d moved this from Review Backlog to In Review in Zcashd Team Jul 5, 2018

@str4d str4d moved this from In Review to Review Backlog in Zcashd Team Jul 12, 2018

@mdr0id mdr0id moved this from Review Backlog to In Review in Zcashd Team Jul 13, 2018

@@ -106,6 +106,9 @@ class CMainParams : public CChainParams {
consensus.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight =
Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT;

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

This comment has been minimized.

@bitcartel

bitcartel Jul 16, 2018

Contributor

@str4d Double checking - you retrieved this via getblockchaininfo?

This comment has been minimized.

@bitcartel

bitcartel Jul 17, 2018

Contributor

I verified that mainnet has more work than this.

@@ -268,6 +271,9 @@ class CTestNetParams : public CChainParams {
consensus.vUpgrades[Consensus::UPGRADE_SAPLING].nActivationHeight =
Consensus::NetworkUpgrade::NO_ACTIVATION_HEIGHT;

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

This comment has been minimized.

@bitcartel

bitcartel Jul 16, 2018

Contributor

@str4d Double checking - you retrieved this via getblockchaininfo?

This comment has been minimized.

@bitcartel

bitcartel Jul 17, 2018

Contributor

I verified that testnet as more work than this.

@bitcartel

This comment has been minimized.

Copy link
Contributor

bitcartel commented Jul 16, 2018

@zkbot try

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jul 16, 2018

⌛️ Trying commit bbff863 with merge ebd25d1...

zkbot added a commit that referenced this pull request Jul 16, 2018

Auto merge of #3263 - str4d:ibd-upstream-changes, r=<try>
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
@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jul 17, 2018

☀️ Test successful - pr-try
State: approved= try=True

@bitcartel
Copy link
Contributor

bitcartel left a comment

ACK. Synced up fresh chain and also ran test with -maxtipage to verify the InitialBlockDownload latch is set correctly.

@bitcartel

This comment has been minimized.

Copy link
Contributor

bitcartel commented Jul 17, 2018

Two ACKs and self-answered questions above to @str4d.
@zkbot r+

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jul 17, 2018

📌 Commit bbff863 has been approved by bitcartel

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jul 17, 2018

⌛️ Testing commit bbff863 with merge 6d605ff...

zkbot added a commit that referenced this pull request Jul 17, 2018

Auto merge of #3263 - str4d:ibd-upstream-changes, r=bitcartel
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

@bitcartel bitcartel moved this from In Review to Released (Merged in Master) in Zcashd Team Jul 17, 2018

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jul 17, 2018

💔 Test failed - pr-merge

@str4d

This comment has been minimized.

Copy link
Contributor

str4d commented Jul 17, 2018

@bitcartel bitcartel moved this from Released (Merged in Master) to In Review in Zcashd Team Jul 17, 2018

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jul 17, 2018

⌛️ Testing commit bbff863 with merge 3835cbb...

zkbot added a commit that referenced this pull request Jul 17, 2018

Auto merge of #3263 - str4d:ibd-upstream-changes, r=bitcartel
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
@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jul 17, 2018

☀️ Test successful - pr-merge
Approved by: bitcartel
Pushing 3835cbb to master...

@zkbot zkbot merged commit bbff863 into zcash:master Jul 17, 2018

1 check passed

homu Test successful
Details

Zcashd Team automation moved this from In Review to Released (Merged in Master) Jul 17, 2018

zclassic-instance added a commit to ZclassicDev/zclassic that referenced this pull request Oct 5, 2018

comment src/chainparams.* changes
double upstream zcash#3263

Changes to be committed:
modified:   src/chainparams.cpp
modified:   src/chainparams.h
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment