Add block ban flag --invalid-block-roots
#7042
Conversation
michaelsproul
force-pushed
the
block-ban-flag
branch
from
6f2b7ef to
f5b4e4a
Compare
|
I've cherry-picked all the block ban related changes to this PR and rebased on |
|
I reverted the |
michaelsproul
changed the title
--invalid-block-roots
| false | ||
| } | ||
| }; | ||
| if chain.config.invalid_block_roots.contains(&block_root) || invalid_holesky_block { |
There was a problem hiding this comment.
This flag can be exploited for censoring, you should restrict usage to a specific network
There was a problem hiding this comment.
ive constrained this flag to holesky only
There was a problem hiding this comment.
I disagree with both of Lion's suggested changes, sorry 😅
I think we need the flag to be not limited to Holesky so that we can potentially use it to get out of trouble on other catastrophically failing networks (without having to patch). We don't have another solution for preventing optimistic sync of the wrong chain at the moment.
I think this also means we should keep the hardcoded Holesky block as a separate check. It doesn't make sense as the flag default if the flag works on multiple networks.
There was a problem hiding this comment.
got it, I had assumed that this was strictly a holesky thing. If the plan is to keep this flag until we have a better solution for optimistic sync of the wrong chain, I'm cool with that
There was a problem hiding this comment.
i've reverted the changes back to what they were originally
There was a problem hiding this comment.
We can get rid of this when we implement checkpoint sync from an unfinalized checkpoint
There was a problem hiding this comment.
@realbigsean would that prevent us from importing an invalid block though? wouldn't sync still attempt to sync with the invalid chain?
There was a problem hiding this comment.
you’d have to use a checkpoint ahead of the invalid block, and since we’d backfill from there we’d avoid it. Part of the complexity of unfinalized checkpoint sync would be making sure you ignore chain tips older than the checkpoint
|
This pull request has merge conflicts. Could you please resolve them @eserilev? 🙏 |
eserilev
force-pushed
the
block-ban-flag
branch
from
b3395b0 to
05e0871
Compare
|
I'm going to move the invalid block check so it happens sooner. |
Add a new flag that allows users to ban block roots listed in a file. Also make sure the hardcoded invalid block root is only considered on holesky