Skip to content

Use sync_tolerance_epochs flag to control the proposer prep routines #7044

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

Merged
Merged
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
14 changes: 4 additions & 10 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,6 @@ pub const FORK_CHOICE_DB_KEY: Hash256 = Hash256::ZERO;
/// Defines how old a block can be before it's no longer a candidate for the early attester cache.
const EARLY_ATTESTER_CACHE_HISTORIC_SLOTS: u64 = 4;

/// Defines a distance between the head block slot and the current slot.
///
/// If the head block is older than this value, don't bother preparing beacon proposers.
const PREPARE_PROPOSER_HISTORIC_EPOCHS: u64 = 4;

/// If the head is more than `MAX_PER_SLOT_FORK_CHOICE_DISTANCE` slots behind the wall-clock slot, DO NOT
/// run the per-slot tasks (primarily fork choice).
///
Expand Down Expand Up @@ -4848,7 +4843,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
let proposer_index = if let Some(proposer) = cached_proposer {
proposer.index as u64
} else {
if head_epoch + 2 < proposal_epoch {
if head_epoch + self.config.sync_tolerance_epochs < proposal_epoch {
warn!(
self.log,
"Skipping proposer preparation";
Expand Down Expand Up @@ -6079,19 +6074,18 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
// Use a blocking task since blocking the core executor on the canonical head read lock can
// block the core tokio executor.
let chain = self.clone();
let tolerance_slots = self.config.sync_tolerance_epochs * T::EthSpec::slots_per_epoch();
let maybe_prep_data = self
.spawn_blocking_handle(
move || {
let cached_head = chain.canonical_head.cached_head();

// Don't bother with proposer prep if the head is more than
// `PREPARE_PROPOSER_HISTORIC_EPOCHS` prior to the current slot.
// `sync_tolerance_epochs` prior to the current slot.
//
// This prevents the routine from running during sync.
let head_slot = cached_head.head_slot();
if head_slot + T::EthSpec::slots_per_epoch() * PREPARE_PROPOSER_HISTORIC_EPOCHS
< current_slot
{
if head_slot + tolerance_slots < current_slot {
debug!(
chain.log,
"Head too old for proposer prep";
Expand Down
7 changes: 7 additions & 0 deletions beacon_node/beacon_chain/src/chain_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ pub const DEFAULT_PREPARE_PAYLOAD_LOOKAHEAD_FACTOR: u32 = 3;
/// Fraction of a slot lookahead for fork choice in the state advance timer (500ms on mainnet).
pub const FORK_CHOICE_LOOKAHEAD_FACTOR: u32 = 24;

/// Default sync tolerance epochs.
pub const DEFAULT_SYNC_TOLERANCE_EPOCHS: u64 = 2;

#[derive(Debug, PartialEq, Eq, Clone, Deserialize, Serialize)]
pub struct ChainConfig {
/// Maximum number of slots to skip when importing an attestation.
Expand Down Expand Up @@ -94,6 +97,9 @@ pub struct ChainConfig {
/// The delay in milliseconds applied by the node between sending each blob or data column batch.
/// This doesn't apply if the node is the block proposer.
pub blob_publication_batch_interval: Duration,
/// The max distance between the head block and the current slot at which Lighthouse will
/// consider itself synced and still serve validator-related requests.
pub sync_tolerance_epochs: u64,
}

impl Default for ChainConfig {
Expand Down Expand Up @@ -129,6 +135,7 @@ impl Default for ChainConfig {
enable_sampling: false,
blob_publication_batches: 4,
blob_publication_batch_interval: Duration::from_millis(300),
sync_tolerance_epochs: DEFAULT_SYNC_TOLERANCE_EPOCHS,
}
}
}
Expand Down
15 changes: 2 additions & 13 deletions beacon_node/http_api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,6 @@ use warp_utils::{query::multi_key_query, reject::convert_rejection, uor::Unifyin

const API_PREFIX: &str = "eth";

/// If the node is within this many epochs from the head, we declare it to be synced regardless of
/// the network sync state.
///
/// This helps prevent attacks where nodes can convince us that we're syncing some non-existent
/// finalized head.
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 @@ -157,7 +150,6 @@ pub struct Config {
pub duplicate_block_status_code: StatusCode,
pub enable_light_client_server: bool,
pub target_peers: usize,
pub sync_tolerance_epochs: Option<u64>,
}

impl Default for Config {
Expand All @@ -174,7 +166,6 @@ impl Default for Config {
duplicate_block_status_code: StatusCode::ACCEPTED,
enable_light_client_server: true,
target_peers: 100,
sync_tolerance_epochs: None,
}
}
}
Expand Down Expand Up @@ -475,10 +466,8 @@ pub fn serve<T: BeaconChainTypes>(
)
})?;

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

if head_slot + tolerance >= current_slot {
Ok(())
Expand Down
11 changes: 7 additions & 4 deletions 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_optional, parse_required};
use clap_utils::{parse_flag, 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 @@ -177,9 +177,6 @@ pub fn get_config<E: EthSpec>(

client_config.http_api.enable_light_client_server =
!cli_args.get_flag("disable-light-client-server");

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

if cli_args.get_flag("light-client-server") {
Expand All @@ -194,6 +191,12 @@ pub fn get_config<E: EthSpec>(
client_config.chain.enable_light_client_server = false;
}

if let Some(sync_tolerance_epochs) =
clap_utils::parse_optional(cli_args, "sync-tolerance-epochs")?
{
client_config.chain.sync_tolerance_epochs = sync_tolerance_epochs;
}

if let Some(cache_size) = clap_utils::parse_optional(cli_args, "shuffling-cache-size")? {
client_config.chain.shuffling_cache_size = cache_size;
}
Expand Down
17 changes: 15 additions & 2 deletions lighthouse/tests/beacon_node.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::exec::{CommandLineTestExec, CompletedTest};
use beacon_node::beacon_chain::chain_config::{
DisallowedReOrgOffsets, DEFAULT_RE_ORG_CUTOFF_DENOMINATOR, DEFAULT_RE_ORG_HEAD_THRESHOLD,
DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION,
DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION, DEFAULT_SYNC_TOLERANCE_EPOCHS,
};
use beacon_node::{
beacon_chain::graffiti_calculator::GraffitiOrigin,
Expand Down Expand Up @@ -2586,7 +2586,20 @@ fn sync_tolerance_epochs() {
.flag("sync-tolerance-epochs", Some("0"))
.run_with_zero_port()
.with_config(|config| {
assert_eq!(config.http_api.sync_tolerance_epochs, Some(0));
assert_eq!(config.chain.sync_tolerance_epochs, 0);
});
}

#[test]
fn sync_tolerance_epochs_default() {
CommandLineTest::new()
.flag("http", None)
.run_with_zero_port()
.with_config(|config| {
assert_eq!(
config.chain.sync_tolerance_epochs,
DEFAULT_SYNC_TOLERANCE_EPOCHS
);
});
}

Expand Down