Skip to content

Checkpoints: The hash of the genesis block it's the genesis checkpoint #6230

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Checkpoints: The hash of the genesis block it's the genesis checkpoint #6230

wants to merge 1 commit into from

Conversation

jtimon
Copy link
Contributor

@jtimon jtimon commented Jun 4, 2015

It's nicer to have all the hardcoded hashes of the genesis blocks of the respective supported chains together.
More importantly, all chains should have the genesis block as checkpoint. That way the checkpoint logic can always trust the first checkpoint and hopefully we will be able to save some if (block.GetHash() != chainparams.GetConsensus().hashGenesisBlock) here and there.
I also add some tests to checkpoints. They're not very important since the asserts in chainparams would already detect any change in the genesis block, but it makes sure that "the checkpoint logic can always trust the first checkpoint".
In fact, the genesis checkpoint is that is immune to reorgs, by definition.
The genesis block is the first rule and the only true chain ID (because one based in names would require a memorable-ID system [and those are centralized and/or vulnerable to squatting]).
(Shares base commit with #6229)

@jtimon jtimon force-pushed the chainparams-genesis branch from 15cabcd to 8b9ed6e Compare June 4, 2015 19:33
@jtimon jtimon changed the title Chainparams: The hash of the genesis block it's the genesis checkpoint and chain id Chainparams: The hash of the genesis block is the genesis checkpoint and chain id Jun 4, 2015
@jtimon jtimon force-pushed the chainparams-genesis branch 2 times, most recently from 408d2b4 to dc16119 Compare June 21, 2015 16:00
@jtimon
Copy link
Contributor Author

jtimon commented Jun 26, 2015

Added an alternative solution to the bug described in #6299. It may also solve #5984 (at least it shares some code with #5987) and maybe also other bugs related to not having a genesis checkpoint (the genesis block) in all chain modes.

@jtimon jtimon mentioned this pull request Jun 26, 2015
Merged
@jtimon jtimon force-pushed the chainparams-genesis branch from dbbb53f to 29f02d6 Compare June 26, 2015 15:45
@jtimon
Copy link
Contributor Author

jtimon commented Jun 26, 2015

Rebased after merging #6299. Apart from its original form, now this PR additionally has the a removal of the now-unnecessary check for the genesis hash block (and an additional big indentation commit).
Let's wait for testing, I think it was safer to just merge #6299, but I truly believe the existence of a fully-trustable genesis checkpoint is the necessary step for decoupling the checkpoint logic from any notion of chainparams or a genesis block hash. I think I'm finding enough prove that problems can spring by not doing so and - still - this PR would be minimal if the enum-to-string was merged somewhere else (like in the currently closed #6229 or an improved subset of it) and ignoring the trivially-verifiable indentation.

@jtimon jtimon force-pushed the chainparams-genesis branch 3 times, most recently from 3a42c88 to e9adf7b Compare June 30, 2015 23:27
@jtimon jtimon mentioned this pull request Jul 1, 2015
Closed
@jtimon jtimon force-pushed the chainparams-genesis branch 2 times, most recently from f904b87 to 8d940d2 Compare July 1, 2015 17:21
@jtimon
Copy link
Contributor Author

jtimon commented Jul 1, 2015

Updated without adding any tests or sharing a commit with #6229.
@sdaftuar Can you confirm that this also solves the problem you found, which was solved in #6299 ?

@sdaftuar
Copy link
Member

sdaftuar commented Jul 1, 2015

@jtimon Just looked at this, reindex.py fails, I think essentially for the same reason it failed before #6299 was merged.

@jtimon jtimon force-pushed the chainparams-genesis branch from 8d940d2 to bd6aafa Compare July 1, 2015 21:29
@jtimon
Copy link
Contributor Author

jtimon commented Jul 1, 2015

Yes, @sdaftuar , you're right, this is no alternative fix of #6299 .
I was testing it wrong, but fortunately qa/rpc-tests/reindex.py noticed, great!
I removed everything but the single commit introducing the "genesis checkpoints", which @TheBlueMatt should like because he did the equivalent for alpha-0.10.
If @luke-jr rebases #5987 on top of this I can close the PR.

In summary and to reiterate, there's no reason not to have the genesis block as a checkpoint: it's the best checkpoint you will ever have! by definition!

@jtimon jtimon changed the title Chainparams: The hash of the genesis block is the genesis checkpoint and chain id Checkpoints: The hash of the genesis block it's the genesis checkpoint Jul 1, 2015
@jtimon jtimon force-pushed the chainparams-genesis branch from bd6aafa to 0c576b2 Compare July 2, 2015 12:44
@jtimon
Copy link
Contributor Author

jtimon commented Jul 6, 2015

Closing for now. I will reopen again with the version with the tests. In the meantime @luke-jr can take this for his PR if he wants to.

@jtimon jtimon closed this Jul 6, 2015
@jtimon jtimon mentioned this pull request Aug 29, 2015
Closed
12 tasks
@jtimon jtimon mentioned this pull request Nov 8, 2016
Closed
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Refactoring
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jtimon @sdaftuar @laanwj