Skip to content
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.

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

Add block ban flag --invalid-block-roots #7042

Open
wants to merge 2 commits into
base: release-v7.0.0
Choose a base branch
from

Conversation

eserilev
Copy link
Collaborator

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

Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM!

@michaelsproul michaelsproul changed the base branch from holesky-rescue to release-v7.0.0 February 26, 2025 07:06
@michaelsproul michaelsproul requested a review from jxs as a code owner February 26, 2025 07:06
@michaelsproul
Copy link
Member

I've cherry-picked all the block ban related changes to this PR and rebased on release-v7.0.0 so we can have a clean history with each feature in its own PR.

@michaelsproul
Copy link
Member

michaelsproul commented Feb 26, 2025

I reverted the ignore for the tests, because they were caused by the fork choice invalidation changes, which we do not want to keep.

@michaelsproul michaelsproul changed the title Add Block ban flag Add block ban flag --invalid-block-roots Feb 26, 2025
@michaelsproul michaelsproul added the ready-for-review The code is ready for review label Feb 26, 2025
Copy link
Member

@jimmygchen jimmygchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

ParentExecutionPayloadInvalid {
parent_root: Hash256,
},
KnownInvalidExecutionPayload(Hash256),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment on top of this variant to have same format

@@ -1326,6 +1343,19 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
chain: &Arc<BeaconChain<T>>,
notify_execution_layer: NotifyExecutionLayer,
) -> Result<Self, BlockError> {
let invalid_holesky_block = {
if let Ok(invalid_block_root) = Hash256::from_str(
"2db899881ed8546476d0b92c6aa9110bea9a4cd0dbeb5519eb0ea69575f1f359",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it intentional to hardcode a block here? You can add this block root in the default value of the CLI flag invalid_block_roots

false
}
};
if chain.config.invalid_block_roots.contains(&block_root) || invalid_holesky_block {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This flag can be exploited for censoring, you should restrict usage to a specific network

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-review The code is ready for review v7.0.0-beta.2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants