bitcoin
consensus: Remove mainnet checkpoints
Once we have logic at the p2p layer to avoid permanently storing headers unless they lead to a sufficiently high work chain, we no longer need to use checkpoints to protect against spam from low-difficulty headers.
Waiting for #28043.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Code Coverage
For detailed information about the code coverage, see the test coverage report.
Reviews
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | michaelfolkson, Sjors, ariard, brunoerg, dergoegge |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #26177 (refactor / kernel: Move non-gArgs chainparams functionality to kernel by TheCharlatan)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Concept ACK (once the logic at the p2p layer is added).
It could be argued by theoretical sticklers that this change is technically a hard fork. But in practice removing checkpoints poses zero risk of a network split and checkpoints have caused much confusion in the past.
Concept ACK. I always tested the previous PR with this patch on top of it.
No longer needs to be draft.
I suggest holding off merge until after the v24 branch-off, perhaps a bit longer. We should give people a chance to come up with attacks that are still possible after #25717, yet are somehow thwarted by checkpoints. Also, if we find a structural problem with that PR and have to revert it, I'd rather not give CNN the headline "Bitcoin Core brings back checkpoints".
@michaelfolkson I think dropping checkpoints is in the same category as retroactively applying soft forks from genesis. With checkpoints an attacker can "only" wipe out eight years of transaction history (the last checkpoint is from 2014). Without checkpoints they can wipe out thirteen years. Those who run an older client will be "saved" in the case of the latter attack.
Interestingly, if the attacker violates P2SH rules before block 173,805 (see #11739) they would previously be rejected by all nodes, just based on headers, because that's before the most recent checkpoint. With this PR the blocks would be rejected based on violating P2SH rules. That's still not the deepest burried soft-fork, that honor either goes to the 1MB limit (which Satoshi buried in 172f006020965ae8763a0610845c051ed1e3b522 almost immediately after introducing it) or something older.
Note that the changes in #25717 do not prevent such a massive reorg: as long as the headers have valid proof-of-work, they'll get processed.
I suggest holding off merge until after the v24 branch-off, perhaps a bit longer. We should give people a chance to come up with attacks that are still possible after https://github.com/bitcoin/bitcoin/pull/25717, yet are somehow thwarted by checkpoints.
I agree with this -- it would be good for reviewers to consider whether there are any avenues to accepting headers that may have been overlooked in #25717, and generally how to ensure that our code is robust to bugs being introduced in the future that could open up an attack that would otherwise be mitigated by checkpoints.
Meanwhile developers and other volunteers could run their node with -nocheckpoints to see if any attacks show up.
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
- Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
- Is it no longer relevant? ➡️ Please close.
- Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
- Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
- Is it no longer relevant? ➡️ Please close.
- Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.
Concept ACK
For anyone interested in moving this forward, I opened #28043 to give us some more confidence in making this change.
Are you still working on this? Given that #25717 has been long since merged, should this still be a draft?
I think the question here is whether we're confident enough in the checks we have in place to prevent low-work-header DoS that we're ok with dropping the existing checkpoints. Philosophically, the change from #25717 should make us theoretically protected from these kinds of concerns. In practice, however, bugs are easy to inadvertently introduce, and if we're not confident that we have adequate test coverage or protections against future bugs that would allow for low-difficulty headers to be accepted in some edge cases, then the existing checkpoints at least raise the cost of exploiting such bugs by a material amount.
So I think this is worth further discussion. My view is that we should at least wait for #28043, and then have a discussion about the risk/reward for dropping the checkpoints.
In any case I prefer to punt this until at least after the upcoming release branch-off :-)
Meanwhile we should make hats and t-shirts with checkpoints=0, and also perhaps move that option out of --help-debug to --help (#28524).
Open to manually test and review more the removal of mainnet checkpoints when there is momentum on moving forward to remove this logic from consensus. Reviewed last year the low-work-header DoS mitigation, always valuable to check again to prevent future bugs or issues circling in.
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
- Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
- Is it no longer relevant? ➡️ Please close.
- Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.
Waiting for https://github.com/bitcoin/bitcoin/pull/28043.
This PR is now closed. If the want here is to move that PR forward, then people interested in this change, will want to review that PR.
