Disallow genesis sync outside blob pruning window (sigp#5038)

commit 63b09d1 Merge: ce8b614 db05d37 Author: Paul Hauner <[email protected]> Date: Mon Jan 8 14:05:39 2024 +1100 Merge branch 'unstable' into disallow-genesis-sync commit ce8b614 Author: Paul Hauner <[email protected]> Date: Mon Jan 8 13:59:11 2024 +1100 Add missing CLI flag commit b8e11ca Author: Paul Hauner <[email protected]> Date: Mon Jan 8 13:59:04 2024 +1100 Fix failing test commit 02a369e Author: Paul Hauner <[email protected]> Date: Mon Jan 8 13:58:57 2024 +1100 Fix typos commit 2dfc4b1 Author: Paul Hauner <[email protected]> Date: Mon Jan 8 12:10:34 2024 +1100 Return an error based on the Deneb fork commit 0153572 Author: Paul Hauner <[email protected]> Date: Mon Jan 8 11:56:11 2024 +1100 Tidy, fix tests commit 5033bfa Author: Paul Hauner <[email protected]> Date: Mon Jan 8 11:51:13 2024 +1100 Perform checks in the `ClientBuilder` commit 328ad6c Author: Mark Mackey <[email protected]> Date: Wed Jan 3 17:43:44 2024 +0100 Fix CLI Tests commit b7ff1c1 Author: Mark Mackey <[email protected]> Date: Wed Jan 3 15:21:52 2024 +0100 Disallow Syncing From Genesis By Default
paulhauner · Jan 8, 2024 · f05143b · f05143b
1 parent db05d37
commit f05143b
Show file tree

Hide file tree

Showing 6 changed files with 94 additions and 3 deletions.
diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs
@@ -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
@@ -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. \
+                                    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 {

diff --git a/beacon_node/client/src/config.rs b/beacon_node/client/src/config.rs
@@ -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 {
@@ -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,
         }
     }
 }

diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs
@@ -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")

diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs
@@ -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)) = (

diff --git a/book/src/help_bn.md b/book/src/help_bn.md
@@ -9,7 +9,11 @@ USAGE:
     lighthouse beacon_node [FLAGS] [OPTIONS]
 
 FLAGS:
-        --always-prefer-builder-payload        This flag is deprecated and has no effect.
+        --allow-insecure-genesis-sync          Enable syncing from genesis. This is insecure after Capella due to long-
+                                               range attacks. This should only be used for testing. DO NOT use on
+                                               mainnet!
+        --always-prefer-builder-payload        If set, the beacon node always uses the payload from the builder instead
+                                               of the local payload.
         --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
                                                recipient on this BN and also consider adjusting the --prepare-payload-
@@ -174,8 +178,12 @@ OPTIONS:
             `SLOTS_PER_EPOCH`, it will NOT query any connected builders, and will use the local execution engine for
             payload construction. [default: 8]
         --builder-profit-threshold <WEI_VALUE>
-            This flag is deprecated and has no effect.
-
+            The minimum reward in wei provided to the proposer by a block builder for an external payload to be
+            considered for inclusion in a proposal. If this threshold is not met, the local EE's payload will be used.
+            This is currently *NOT* in comparison to the value of the local EE's payload. It simply checks whether the
+            total proposer reward from an external payload is equal to or greater than this value. In the future, a
+            comparison to a local payload is likely to be added. Example: Use 250000000000000000 to set the threshold to
+            0.25 ETH. [default: 0]
         --builder-user-agent <STRING>
             The HTTP user agent to send alongside requests to the builder URL. The default is Lighthouse's version
             string.
@@ -308,6 +316,14 @@ OPTIONS:
         --http-tls-key <http-tls-key>
             The path of the private key to be used when serving the HTTP API server over TLS. Must not be password-
             protected.
+        --ignore-builder-override-suggestion-threshold <PERCENTAGE>
+            When the EE advises Lighthouse to ignore the builder payload, this flag specifies a percentage threshold for
+            the difference between the reward from the builder payload and the local EE's payload. This threshold must
+            be met for Lighthouse to consider ignoring the EE's suggestion. If the reward from the builder's payload
+            doesn't exceed the local payload by at least this percentage, the local payload will be used. The conditions
+            under which the EE may make this suggestion depend on the EE's implementation, with the primary intent being
+            to safeguard against potential censorship attacks from builders. Setting this flag to 0 will cause
+            Lighthouse to always ignore the EE's suggestion. Default: 10.0 (equivalent to 10%). [default: 10.0]
         --invalid-gossip-verified-blocks-path <PATH>
             If a block succeeds gossip validation whilst failing full validation, store the block SSZ as a file at this
             path. This feature is only recommended for developers. This directory is not pruned, users should be careful

diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs
@@ -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 {