Skip to content

Commit

Permalink
Add test flag to override SYNC_TOLERANCE_EPOCHS for range sync test…
Browse files Browse the repository at this point in the history
…ing (#7030)

Related to #6880, an issue that's usually observed on local devnets with small number of nodes.

When testing range sync, I usually shutdown a node for some period of time and restart it again. However, if it's within `SYNC_TOLERANCE_EPOCHS` (8), Lighthouse would consider the node as synced, and if it may attempt to produce a block if requested by a validator - on a local devnet, nodes frequently produce blocks - when this happens, the node ends up producing a block that would revert finality and would get disconnected from peers immediately.

### Usage

Run Lighthouse BN with this flag to override:

```
--sync-tolerance--epoch 0
```
  • Loading branch information
jimmygchen authored Feb 24, 2025
1 parent 454c7d0 commit 54b4150
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 3 deletions.
9 changes: 7 additions & 2 deletions beacon_node/http_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ const API_PREFIX: &str = "eth";
///
/// This helps prevent attacks where nodes can convince us that we're syncing some non-existent
/// finalized head.
const SYNC_TOLERANCE_EPOCHS: u64 = 8;
const DEFAULT_SYNC_TOLERANCE_EPOCHS: u64 = 8;

/// A custom type which allows for both unsecured and TLS-enabled HTTP servers.
type HttpServer = (SocketAddr, Pin<Box<dyn Future<Output = ()> + Send>>);
Expand Down Expand Up @@ -156,6 +156,7 @@ pub struct Config {
#[serde(with = "eth2::types::serde_status_code")]
pub duplicate_block_status_code: StatusCode,
pub target_peers: usize,
pub sync_tolerance_epochs: Option<u64>,
}

impl Default for Config {
Expand All @@ -171,6 +172,7 @@ impl Default for Config {
enable_beacon_processor: true,
duplicate_block_status_code: StatusCode::ACCEPTED,
target_peers: 100,
sync_tolerance_epochs: None,
}
}
}
Expand Down Expand Up @@ -459,7 +461,10 @@ pub fn serve<T: BeaconChainTypes>(
)
})?;

let tolerance = SYNC_TOLERANCE_EPOCHS * T::EthSpec::slots_per_epoch();
let sync_tolerance_epochs = config
.sync_tolerance_epochs
.unwrap_or(DEFAULT_SYNC_TOLERANCE_EPOCHS);
let tolerance = sync_tolerance_epochs * T::EthSpec::slots_per_epoch();

if head_slot + tolerance >= current_slot {
Ok(())
Expand Down
13 changes: 13 additions & 0 deletions beacon_node/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1509,6 +1509,19 @@ pub fn cli_app() -> Command {
.help_heading(FLAG_HEADER)
.display_order(0)
)
.arg(
Arg::new("sync-tolerance-epochs")
.long("sync-tolerance-epochs")
.help("Overrides the default SYNC_TOLERANCE_EPOCHS. This flag is not intended \
for production and MUST only be used in TESTING only. This is primarily used \
for testing range sync, to prevent the node from producing a block before the \
node is synced with the network which may result in the node getting \
disconnected from peers immediately.")
.hide(true)
.requires("enable_http")
.action(ArgAction::Set)
.display_order(0)
)
.arg(
Arg::new("gui")
.long("gui")
Expand Down
5 changes: 4 additions & 1 deletion beacon_node/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use beacon_chain::graffiti_calculator::GraffitiOrigin;
use beacon_chain::TrustedSetup;
use clap::{parser::ValueSource, ArgMatches, Id};
use clap_utils::flags::DISABLE_MALLOC_TUNING_FLAG;
use clap_utils::{parse_flag, parse_required};
use clap_utils::{parse_flag, parse_optional, parse_required};
use client::{ClientConfig, ClientGenesis};
use directory::{DEFAULT_BEACON_NODE_DIR, DEFAULT_NETWORK_DIR, DEFAULT_ROOT_DIR};
use environment::RuntimeContext;
Expand Down Expand Up @@ -174,6 +174,9 @@ pub fn get_config<E: EthSpec>(

client_config.http_api.duplicate_block_status_code =
parse_required(cli_args, "http-duplicate-block-status")?;

client_config.http_api.sync_tolerance_epochs =
parse_optional(cli_args, "sync-tolerance-epochs")?;
}

if cli_args.get_flag("light-client-server") {
Expand Down
11 changes: 11 additions & 0 deletions lighthouse/tests/beacon_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2543,6 +2543,17 @@ fn light_client_http_server_disabled() {
});
}

#[test]
fn sync_tolerance_epochs() {
CommandLineTest::new()
.flag("http", None)
.flag("sync-tolerance-epochs", Some("0"))
.run_with_zero_port()
.with_config(|config| {
assert_eq!(config.http_api.sync_tolerance_epochs, Some(0));
});
}

#[test]
fn gui_flag() {
CommandLineTest::new()
Expand Down
4 changes: 4 additions & 0 deletions scripts/local_testnet/network_params_das.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,15 @@ participants:
cl_extra_params:
- --subscribe-all-data-column-subnets
- --subscribe-all-subnets
# Note: useful for testing range sync (only produce block if node is in sync to prevent forking)
- --sync-tolerance-epochs=0
- --target-peers=3
count: 2
- cl_type: lighthouse
cl_image: lighthouse:local
cl_extra_params:
# Note: useful for testing range sync (only produce block if node is in sync to prevent forking)
- --sync-tolerance-epochs=0
- --target-peers=3
count: 2
network_params:
Expand Down

0 comments on commit 54b4150

Please sign in to comment.