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

Disallow genesis sync outside blob pruning window #5038

Merged
merged 13 commits into from
Jan 9, 2024
45 changes: 45 additions & 0 deletions beacon_node/client/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,23 @@ use std::net::TcpListener;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use std::time::Duration;
use std::time::{SystemTime, UNIX_EPOCH};
use timer::spawn_timer;
use tokio::sync::oneshot;
use types::{
consts::deneb::MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS,
test_utils::generate_deterministic_keypairs, BeaconState, ChainSpec, EthSpec,
ExecutionBlockHash, Hash256, SignedBeaconBlock,
};

/// Interval between polling the eth1 node for genesis information.
pub const ETH1_GENESIS_UPDATE_INTERVAL_MILLIS: u64 = 7_000;

/// Reduces the blob availability period by some epochs. Helps prevent the user
/// from starting a genesis sync so near to the blob pruning window that blobs
/// have been pruned before they can manage to sync the chain.
const BLOB_AVAILABILITY_REDUCTION_EPOCHS: u64 = 2;

/// Builds a `Client` instance.
///
/// ## Notes
Expand Down Expand Up @@ -252,6 +259,44 @@ where

let genesis_state = genesis_state(&runtime_context, &config, log).await?;

// If the user has not explicitly allowed genesis sync, prevent
// them from trying to sync from genesis if we're outside of the
// blob P2P availability window.
//
// It doesn't make sense to try and sync the chain if we can't
// verify blob availability by downloading blobs from the P2P
// network. The user should do a checkpoint sync instead.
if !config.allow_insecure_genesis_sync {
if let Some(deneb_fork_epoch) = spec.deneb_fork_epoch {
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.map_err(|e| format!("Unable to read system time: {e:}"))?
.as_secs();
let genesis_time = genesis_state.genesis_time();
let deneb_time =
genesis_time + (deneb_fork_epoch.as_u64() * spec.seconds_per_slot);

// Shrink the blob availability window so users don't start
// a sync right before blobs start to disappear from the P2P
// network.
let reduced_p2p_availability_epochs = MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS
.as_u64()
.saturating_sub(BLOB_AVAILABILITY_REDUCTION_EPOCHS);
let blob_availability_window = reduced_p2p_availability_epochs
* TEthSpec::slots_per_epoch()
* spec.seconds_per_slot;

if now > deneb_time + blob_availability_window {
return Err(
"Syncing from genesis is insecure and incompatible with data availability checks. \
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should also suggest --allow-insecure-genesis-sync if the risks are understood

You should instead perform a checkpoint sync from a trusted node using the --checkpoint-sync-url option. \
For a list of public endpoints, see:\nhttps://eth-clients.github.io/checkpoint-sync-endpoints/"
.to_string(),
);
}
}
}

builder.genesis_state(genesis_state).map(|v| (v, None))?
}
ClientGenesis::WeakSubjSszBytes {
Expand Down
2 changes: 2 additions & 0 deletions beacon_node/client/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ pub struct Config {
pub beacon_processor: BeaconProcessorConfig,
pub genesis_state_url: Option<String>,
pub genesis_state_url_timeout: Duration,
pub allow_insecure_genesis_sync: bool,
}

impl Default for Config {
Expand Down Expand Up @@ -108,6 +109,7 @@ impl Default for Config {
genesis_state_url: <_>::default(),
// This default value should always be overwritten by the CLI default value.
genesis_state_url_timeout: Duration::from_secs(60),
allow_insecure_genesis_sync: false,
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions beacon_node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -955,6 +955,16 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.takes_value(true)
.default_value("180")
)
.arg(
Arg::with_name("allow-insecure-genesis-sync")
.long("allow-insecure-genesis-sync")
.help("Enable syncing from genesis, which is generally insecure and incompatible with data availability checks. \
Checkpoint syncing is the preferred method for syncing a node. \
Only use this flag when testing. DO NOT use on mainnet!")
.conflicts_with("checkpoint-sync-url")
.conflicts_with("checkpoint-state")
.takes_value(false)
)
.arg(
Arg::with_name("reconstruct-historic-states")
.long("reconstruct-historic-states")
Expand Down
2 changes: 2 additions & 0 deletions beacon_node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,8 @@ pub fn get_config<E: EthSpec>(
None
};

client_config.allow_insecure_genesis_sync = cli_args.is_present("allow-insecure-genesis-sync");

client_config.genesis = if eth2_network_config.genesis_state_is_known() {
// Set up weak subjectivity sync, or start from the hardcoded genesis state.
if let (Some(initial_state_path), Some(initial_block_path)) = (
Expand Down
4 changes: 4 additions & 0 deletions book/src/help_bn.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ USAGE:
lighthouse beacon_node [FLAGS] [OPTIONS]

FLAGS:
--allow-insecure-genesis-sync Enable syncing from genesis, which is generally insecure and incompatible
with data availability checks. Checkpoint syncing is the preferred method
for syncing a node. Only use this flag when testing. DO NOT use on
mainnet!
--always-prefer-builder-payload This flag is deprecated and has no effect.
--always-prepare-payload Send payload attributes with every fork choice update. This is intended
for use by block builders, relays and developers. You should set a fee
Expand Down
16 changes: 16 additions & 0 deletions lighthouse/tests/beacon_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,22 @@ fn staking_flag() {
});
}

#[test]
fn allow_insecure_genesis_sync() {
CommandLineTest::new()
.run_with_zero_port()
.with_config(|config| {
assert_eq!(config.allow_insecure_genesis_sync, false);
});

CommandLineTest::new()
.flag("allow-insecure-genesis-sync", None)
.run_with_zero_port()
.with_config(|config| {
assert_eq!(config.allow_insecure_genesis_sync, true);
});
}

#[test]
fn wss_checkpoint_flag() {
let state = Some(Checkpoint {
Expand Down