This is incomplete.
Progress estimation is broken. (Any suggestions?)
Also I'm not sure about "int nBlockEstimate = pindexBestHeader->nHeight;"
This is incomplete.
Progress estimation is broken. (Any suggestions?)
Also I'm not sure about "int nBlockEstimate = pindexBestHeader->nHeight;"
Please use the number of blocks on top of a block, rather than the height difference with the best known tip. This shouldn't be hard, as ActivateBestChain knows what tip we're working towards, so it could be passed down.
Also, I think you need a check to have a reasonable guess that pindexBestHeader is reasonably accurate/recent/valid, so you can't be cheated into skipping validation while being sybilled during IBD.
Furthermore, just height can probably be skewed by an attacker. I think it's safer to look at difference in nChainWork, and divide that by best difficulty seen.
Also, removing the checkpoint verification before accepting a header means you're vulnerable to a memory usage DOS attack during IBD (a peer can give you a arbitrarily long low-difficulty headers).
I'm generally in favor of reducing the relevance of checkpoints, but I think this patch is a way too large step at once.
EDIT: fixed wrong code example
I think in general we can't get rid of checkpoints at this point. But we can reduce them to a performance/security tradeoff, without being a lock-in to a particular chain.
I was combing through the code recently with this in mind.
My idea was to disable checkpoints for accepting blocks once you are done totally catching up with the main chain(IsInitialBlockDownload() ?). You get the security benefits of the checkpoints during spinup, but removes the "but Bitcoin uses checkpoints!" idiocy in practice.
Would anyone be interested in this?
@instagibbs with headers first we do not need them during initial sync the benefit there can just be replaced by a work threshold before showing confirmations, or the like.
@gmaxwell Just looking for small discrete steps that can be taken that answer 90% of the misconception without too much logic/code change.
Conflicts:
src/chainparams.cpp
src/init.cpp
src/test/Checkpoints_tests.cpp
What did sipa say? It's not in the pull request. I would be for the patch if disabled script was constrained to initial block download only. On Mar 18, 2015 2:20 AM, "Wladimir J. van der Laan" < notifications@github.com> wrote:
Agree with @sipa https://github.com/sipa. Going to close this for now. Brainstorming can continue, but we can rule out 'cut out checkpoints completely' for the forseeable future.
— Reply to this email directly or view it on GitHub #5842 (comment).
Can we get broad agreement about what specific attacks/issues checkpoints solve today in a post-headers first era? As a relative outsider to the codebase I'd make a pull request attempt but don't want to waste my time with inadequate fixes.
@maaku @sipa I was referring to this:
I'm generally in favor of reducing the relevance of checkpoints, but I think this patch is a way too large step at once.
The way forward would be to split this up up, to remove or replace the individual uses of checkpoints one by one, so the advantages and disadvantages can be discussed on a case by case basis. Just ripping it out completely is an interesting experiment but doesn't pass the risk/benefits test.
Currently checkpoints do two things:
Principally they allow for skipping script checks for blocks which are covered by the checkpoint.
They also prevent a re-org that would change a checkpoint.
The re-org locking behavior is really a consequence of the first; it is not a useful attack mitigation.
On 03/18/2015 09:36 AM, instagibbs wrote:
Can we get broad agreement about what specific attacks/issues checkpoints solve today in a post-headers first era? As a relative outsider to the codebase I'd make a pull request attempt but don't want to waste my time with inadequate fixes.
— Reply to this email directly or view it on GitHub #5842 (comment).
Forcing the consensus with centrally administrated data is not acceptable in my view. It's set far enough back that it has no effect on the network consensus, in practice, but it causes a lot of confusion.
I think it's fundamentally dishonest: we intentionally do it in a way that doesn't influence consensus in practice, but let people who want it to matter because they can't wrap their head around bitcoin think it does; but if they go to far and believe its where the system consensus comes from and start suggesting other "optimizations" like 'checkpointing' blocks minute by minute only then do we correct them... and I think shouldn't be continued.
The script checking skipping is a historical accident (driven primarily by a huge slowdown that was caused by a coding glitch: use of mlocking secureallocator in places it shouldn't be used) that has turned into a crutch that we're now dependent on. We shouldn't let the dependance for one thing cause a lot of other baggage: We don't need checkpoints for initialization short cuts. The primary concern raised with doing short cutting based on work burrying a block was incentives with reorg attacks. I believe they can be completely mitigated by making the shortcutting an initialization time one-shot (even saving to the shortcut criteria to the block index, so reindexing doesn't expose you to it again).
I agree with laanwj that we should just one by one remove the unnecessary use, then we would be free to handle whatever remains.