Skip to content

[Merged by Bors] - Remove legacy max-skip-slots checks #4403

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 0 additions & 35 deletions beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,6 @@ pub enum BlockError<T: EthSpec> {
/// It's unclear if this block is valid, but it cannot be processed without already knowing
/// its parent.
ParentUnknown(Arc<SignedBeaconBlock<T>>),
/// The block skips too many slots and is a DoS risk.
TooManySkippedSlots { parent_slot: Slot, block_slot: Slot },
/// The block slot is greater than the present slot.
///
/// ## Peer scoring
Expand Down Expand Up @@ -786,9 +784,6 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
parent_block.root
};

// Reject any block that exceeds our limit on skipped slots.
check_block_skip_slots(chain, parent_block.slot, block.message())?;

// We assign to a variable instead of using `if let Some` directly to ensure we drop the
// write lock before trying to acquire it again in the `else` clause.
let proposer_opt = chain
Expand Down Expand Up @@ -942,9 +937,6 @@ impl<T: BeaconChainTypes> SignatureVerifiedBlock<T> {

let (mut parent, block) = load_parent(block_root, block, chain)?;

// Reject any block that exceeds our limit on skipped slots.
check_block_skip_slots(chain, parent.beacon_block.slot(), block.message())?;

let state = cheap_state_advance_to_obtain_committees(
&mut parent.pre_state,
parent.beacon_state_root,
Expand Down Expand Up @@ -1135,9 +1127,6 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
return Err(BlockError::ParentUnknown(block));
}

// Reject any block that exceeds our limit on skipped slots.
check_block_skip_slots(chain, parent.beacon_block.slot(), block.message())?;

/*
* Perform cursory checks to see if the block is even worth processing.
*/
Expand Down Expand Up @@ -1492,30 +1481,6 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
}
}

/// Check that the count of skip slots between the block and its parent does not exceed our maximum
/// value.
///
/// Whilst this is not part of the specification, we include this to help prevent us from DoS
/// attacks. In times of dire network circumstance, the user can configure the
/// `import_max_skip_slots` value.
fn check_block_skip_slots<T: BeaconChainTypes>(
chain: &BeaconChain<T>,
parent_slot: Slot,
block: BeaconBlockRef<'_, T::EthSpec>,
) -> Result<(), BlockError<T::EthSpec>> {
// Reject any block that exceeds our limit on skipped slots.
if let Some(max_skip_slots) = chain.config.import_max_skip_slots {
if block.slot() > parent_slot + max_skip_slots {
return Err(BlockError::TooManySkippedSlots {
parent_slot,
block_slot: block.slot(),
});
}
}

Ok(())
}

/// Returns `Ok(())` if the block's slot is greater than the anchor block's slot (if any).
fn check_block_against_anchor_slot<T: BeaconChainTypes>(
block: BeaconBlockRef<'_, T::EthSpec>,
Expand Down
3 changes: 1 addition & 2 deletions beacon_node/beacon_chain/src/chain_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ pub const FORK_CHOICE_LOOKAHEAD_FACTOR: u32 = 24;

#[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
pub struct ChainConfig {
/// Maximum number of slots to skip when importing a consensus message (e.g., block,
/// attestation, etc).
/// Maximum number of slots to skip when importing an attestation.
///
/// If `None`, there is no limit.
pub import_max_skip_slots: Option<u64>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,6 @@ impl<T: BeaconChainTypes> Worker<T> {
| Err(e @ BlockError::NonLinearParentRoots)
| Err(e @ BlockError::BlockIsNotLaterThanParent { .. })
| Err(e @ BlockError::InvalidSignature)
| Err(e @ BlockError::TooManySkippedSlots { .. })
| Err(e @ BlockError::WeakSubjectivityConflict)
| Err(e @ BlockError::InconsistentFork(_))
| Err(e @ BlockError::ExecutionPayloadError(_))
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -685,7 +685,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
Arg::with_name("max-skip-slots")
.long("max-skip-slots")
.help(
"Refuse to skip more than this many slots when processing a block or attestation. \
"Refuse to skip more than this many slots when processing an attestation. \
This prevents nodes on minority forks from wasting our time and disk space, \
but could also cause unnecessary consensus failures, so is disabled by default."
)
Expand Down