ethereum
Better handling of persist_checkpoint_header if header is already present.
What is wrong?
Citing @carver from https://github.com/ethereum/py-evm/pull/1830#pullrequestreview-278043892
Yeah, the only issue I can think of here is if the header was already present and some headers were persisted on top of it. In that case, the persist_checkpoint_header could either be ignored (so it's an idempotent call), or it could raise an exception. It should probably do one of those things, since rewinding the chain is probably not what we want.
Let's assume we try to persist a checkpoint header and notice the header already exists, should we check the score of it? And what if it has a different score than the one we are trying to set? Should that trigger an exception? Or would that mean that we do in fact want to rewind the chain even though the header already exists (potentially with headers building on top of it)
How can it be fixed
First need to define how exactly to handle the edge cases.
My instinct:
Let's assume we try to persist a checkpoint header and notice the header already exists, should we check the score of it?
Yes
And what if it has a different score than the one we are trying to set?
If we have a locally computed score that's different, that's a fatal problem.
Should that trigger an exception?
Yes
Or would that mean that we do in fact want to rewind the chain even though the header already exists (potentially with headers building on top of it)
I don't think we should try to automatically recover from a situation that is so broken.
If at some point we can determine that the local score is fully computed from genesis, maybe we could just emit a warning that the user entered something wrong, and continue on. But that's a big if, and not worth adding the machinery to answer it now, I think. So we should just loudly die with as much information as we have.
