diff --git a/Cargo.lock b/Cargo.lock index aefd51a9501..610f48d593b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2851,6 +2851,7 @@ dependencies = [ "kzg", "logging", "milhouse", + "proto_array", "rayon", "serde", "serde_json", diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index f618cf63217..0bf01281204 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -107,7 +107,7 @@ use operation_pool::{ CompactAttestationRef, OperationPool, PersistedOperationPool, ReceivedPreCapella, }; use parking_lot::{Mutex, RwLock, RwLockWriteGuard}; -use proto_array::{DoNotReOrg, ProposerHeadError}; +use proto_array::{DoNotReOrg, ProposerHeadError, ReOrgThreshold}; use rand::RngCore; use safe_arith::SafeArith; use slasher::Slasher; @@ -5113,15 +5113,14 @@ impl BeaconChain { let _timer = metrics::start_timer(&metrics::FORK_CHOICE_OVERRIDE_FCU_TIMES); // Never override if proposer re-orgs are disabled. - let re_org_head_threshold = self - .config - .re_org_head_threshold - .ok_or(Box::new(DoNotReOrg::ReOrgsDisabled.into()))?; + if self.config.disable_proposer_reorg { + return Err(Box::new(DoNotReOrg::ReOrgsDisabled.into())); + }; - let re_org_parent_threshold = self - .config - .re_org_parent_threshold - .ok_or(Box::new(DoNotReOrg::ReOrgsDisabled.into()))?; + let re_org_head_threshold = ReOrgThreshold(self.spec.reorg_head_weight_threshold); + let re_org_parent_threshold = ReOrgThreshold(self.spec.reorg_parent_weight_threshold); + let re_org_max_epochs_since_finalization = + Epoch::new(self.spec.reorg_max_epochs_since_finalization); let head_block_root = canonical_forkchoice_params.head_root; @@ -5134,7 +5133,7 @@ impl BeaconChain { re_org_head_threshold, re_org_parent_threshold, &self.config.re_org_disallowed_offsets, - self.config.re_org_max_epochs_since_finalization, + re_org_max_epochs_since_finalization, ) .map_err(|e| e.map_inner_error(Error::ProposerHeadForkChoiceError))?; @@ -5155,7 +5154,12 @@ impl BeaconChain { .and_then(|slot_start| { let now = self.slot_clock.now_duration()?; let slot_delay = now.saturating_sub(slot_start); - Some(slot_delay <= self.config.re_org_cutoff(self.spec.get_slot_duration())) + let re_org_cutoff_duration = self + .spec + .compute_slot_component_duration(self.spec.proposer_reorg_cutoff_bps) + .ok()?; + + Some(slot_delay <= re_org_cutoff_duration) }) .unwrap_or(false) } else { diff --git a/beacon_node/beacon_chain/src/block_production/mod.rs b/beacon_node/beacon_chain/src/block_production/mod.rs index fd5e3810232..a94bc697b94 100644 --- a/beacon_node/beacon_chain/src/block_production/mod.rs +++ b/beacon_node/beacon_chain/src/block_production/mod.rs @@ -1,10 +1,10 @@ use std::{sync::Arc, time::Duration}; use fork_choice::PayloadStatus; -use proto_array::ProposerHeadError; +use proto_array::{ProposerHeadError, ReOrgThreshold}; use slot_clock::SlotClock; use tracing::{debug, error, info, instrument, warn}; -use types::{BeaconState, Hash256, SignedExecutionPayloadEnvelope, Slot}; +use types::{BeaconState, Epoch, Hash256, SignedExecutionPayloadEnvelope, Slot}; use crate::{ BeaconChain, BeaconChainTypes, BlockProductionError, StateSkipConfig, @@ -174,8 +174,10 @@ impl BeaconChain { head_slot: Slot, canonical_head: Hash256, ) -> Option<(BeaconState, Hash256)> { - let re_org_head_threshold = self.config.re_org_head_threshold?; - let re_org_parent_threshold = self.config.re_org_parent_threshold?; + let re_org_head_threshold = ReOrgThreshold(self.spec.reorg_head_weight_threshold); + let re_org_parent_threshold = ReOrgThreshold(self.spec.reorg_parent_weight_threshold); + let re_org_max_epochs_since_finalization = + Epoch::new(self.spec.reorg_max_epochs_since_finalization); if self.spec.proposer_score_boost.is_none() { warn!( @@ -198,8 +200,12 @@ impl BeaconChain { // 1. It seems we have time to propagate and still receive the proposer boost. // 2. The current head block was seen late. // 3. The `get_proposer_head` conditions from fork choice pass. - let proposing_on_time = - slot_delay < self.config.re_org_cutoff(self.spec.get_slot_duration()); + let re_org_cutoff_duration = self + .spec + .compute_slot_component_duration(self.spec.proposer_reorg_cutoff_bps) + .ok()?; + + let proposing_on_time = slot_delay < re_org_cutoff_duration; if !proposing_on_time { debug!(reason = "not proposing on time", "Not attempting re-org"); return None; @@ -223,7 +229,7 @@ impl BeaconChain { re_org_head_threshold, re_org_parent_threshold, &self.config.re_org_disallowed_offsets, - self.config.re_org_max_epochs_since_finalization, + re_org_max_epochs_since_finalization, ) .map_err(|e| match e { ProposerHeadError::DoNotReOrg(reason) => { diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index d70561db9ba..1fdd03c04a7 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -29,7 +29,7 @@ use kzg::Kzg; use logging::crit; use operation_pool::{OperationPool, PersistedOperationPool}; use parking_lot::{Mutex, RwLock}; -use proto_array::{DisallowedReOrgOffsets, ReOrgThreshold}; +use proto_array::DisallowedReOrgOffsets; use rand::RngCore; use rayon::prelude::*; use slasher::Slasher; @@ -46,8 +46,8 @@ use tracing::{debug, error, info, warn}; use tree_hash::TreeHash; use types::data::CustodyIndex; use types::{ - BeaconState, BlobSidecarList, ChainSpec, ColumnIndex, DataColumnSidecarList, Epoch, EthSpec, - Hash256, SignedBeaconBlock, Slot, + BeaconState, BlobSidecarList, ChainSpec, ColumnIndex, DataColumnSidecarList, EthSpec, Hash256, + SignedBeaconBlock, Slot, }; /// An empty struct used to "witness" all the `BeaconChainTypes` traits. It has no user-facing @@ -175,21 +175,6 @@ where self } - /// Sets the proposer re-org threshold. - pub fn proposer_re_org_head_threshold(mut self, threshold: Option) -> Self { - self.chain_config.re_org_head_threshold = threshold; - self - } - - /// Sets the proposer re-org max epochs since finalization. - pub fn proposer_re_org_max_epochs_since_finalization( - mut self, - epochs_since_finalization: Epoch, - ) -> Self { - self.chain_config.re_org_max_epochs_since_finalization = epochs_since_finalization; - self - } - /// Sets the proposer re-org disallowed offsets list. pub fn proposer_re_org_disallowed_offsets( mut self, diff --git a/beacon_node/beacon_chain/src/chain_config.rs b/beacon_node/beacon_chain/src/chain_config.rs index b2c017a469d..dde09bf1057 100644 --- a/beacon_node/beacon_chain/src/chain_config.rs +++ b/beacon_node/beacon_chain/src/chain_config.rs @@ -1,15 +1,10 @@ use crate::custody_context::NodeCustodyType; -pub use proto_array::{DisallowedReOrgOffsets, ReOrgThreshold}; +pub use proto_array::DisallowedReOrgOffsets; use serde::{Deserialize, Serialize}; use std::str::FromStr; use std::{collections::HashSet, sync::LazyLock, time::Duration}; -use types::{Checkpoint, Epoch, Hash256}; +use types::{Checkpoint, Hash256}; -pub const DEFAULT_RE_ORG_HEAD_THRESHOLD: ReOrgThreshold = ReOrgThreshold(20); -pub const DEFAULT_RE_ORG_PARENT_THRESHOLD: ReOrgThreshold = ReOrgThreshold(160); -pub const DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION: Epoch = Epoch::new(2); -/// Default to 1/12th of the slot, which is 1 second on mainnet. -pub const DEFAULT_RE_ORG_CUTOFF_DENOMINATOR: u32 = 12; pub const DEFAULT_FORK_CHOICE_BEFORE_PROPOSAL_TIMEOUT: u64 = 250; /// Default fraction of a slot lookahead for payload preparation (12/3 = 4 seconds on mainnet). @@ -41,14 +36,6 @@ pub struct ChainConfig { pub archive: bool, /// The max size of a message that can be sent over the network. pub max_network_size: usize, - /// Maximum percentage of the head committee weight at which to attempt re-orging the canonical head. - pub re_org_head_threshold: Option, - /// Minimum percentage of the parent committee weight at which to attempt re-orging the canonical head. - pub re_org_parent_threshold: Option, - /// Maximum number of epochs since finalization for attempting a proposer re-org. - pub re_org_max_epochs_since_finalization: Epoch, - /// Maximum delay after the start of the slot at which to propose a reorging block. - pub re_org_cutoff_millis: Option, /// Additional epoch offsets at which re-orging block proposals are not permitted. /// /// By default this list is empty, but it can be useful for reacting to network conditions, e.g. @@ -125,6 +112,8 @@ pub struct ChainConfig { pub enable_partial_columns: bool, /// The node's custody type, determining how many data columns to custody and sample. pub node_custody_type: NodeCustodyType, + /// Disable proposer re-org + pub disable_proposer_reorg: bool, } impl Default for ChainConfig { @@ -134,10 +123,6 @@ impl Default for ChainConfig { weak_subjectivity_checkpoint: None, archive: false, max_network_size: 10 * 1_048_576, // 10M - re_org_head_threshold: Some(DEFAULT_RE_ORG_HEAD_THRESHOLD), - re_org_parent_threshold: Some(DEFAULT_RE_ORG_PARENT_THRESHOLD), - re_org_max_epochs_since_finalization: DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION, - re_org_cutoff_millis: None, re_org_disallowed_offsets: DisallowedReOrgOffsets::default(), fork_choice_before_proposal_timeout_ms: DEFAULT_FORK_CHOICE_BEFORE_PROPOSAL_TIMEOUT, // Builder fallback configs that are set in `clap` will override these. @@ -168,15 +153,7 @@ impl Default for ChainConfig { disable_get_blobs: false, enable_partial_columns: false, node_custody_type: NodeCustodyType::Fullnode, + disable_proposer_reorg: false, } } } - -impl ChainConfig { - /// The latest delay from the start of the slot at which to attempt a 1-slot re-org. - pub fn re_org_cutoff(&self, slot_duration: Duration) -> Duration { - self.re_org_cutoff_millis - .map(Duration::from_millis) - .unwrap_or_else(|| slot_duration / DEFAULT_RE_ORG_CUTOFF_DENOMINATOR) - } -} diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index 184bfffc9ad..31ada721516 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/beacon_node/http_api/tests/interactive_tests.rs @@ -2,7 +2,7 @@ use beacon_chain::custody_context::NodeCustodyType; use beacon_chain::{ ChainConfig, - chain_config::{DisallowedReOrgOffsets, ReOrgThreshold}, + chain_config::DisallowedReOrgOffsets, test_utils::{ AttestationStrategy, BlockStrategy, LightClientStrategy, SyncCommitteeStrategy, test_spec, }, @@ -23,7 +23,7 @@ use std::sync::Arc; use std::time::Duration; use types::{ Address, Epoch, EthSpec, ExecPayload, ExecutionBlockHash, ForkName, Hash256, MainnetEthSpec, - MinimalEthSpec, ProposerPreparationData, Slot, Uint256, + MinimalEthSpec, ProposerPreparationData, Slot, }; type E = MainnetEthSpec; @@ -181,8 +181,6 @@ pub struct ReOrgTest { parent_distance: u64, /// Number of slots between head block and block proposal slot. head_distance: u64, - re_org_threshold: u64, - max_epochs_since_finalization: u64, percent_parent_votes: usize, percent_empty_votes: usize, percent_head_votes: usize, @@ -201,8 +199,6 @@ impl Default for ReOrgTest { head_slot: Slot::new(E::slots_per_epoch() - 2), parent_distance: 1, head_distance: 1, - re_org_threshold: 20, - max_epochs_since_finalization: 2, percent_parent_votes: 100, percent_empty_votes: 100, percent_head_votes: 0, @@ -387,8 +383,6 @@ pub async fn proposer_boost_re_org_test( head_slot, parent_distance, head_distance, - re_org_threshold, - max_epochs_since_finalization, percent_parent_votes, percent_empty_votes, percent_head_votes, @@ -402,8 +396,7 @@ pub async fn proposer_boost_re_org_test( // TODO(EIP-7732): extend test for Gloas — `get_validator_blocks_v3` is missing the // `Eth-Execution-Payload-Blinded` header for Gloas block production responses. - let mut spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); - spec.terminal_total_difficulty = Uint256::from(1); + let spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); // Ensure there are enough validators to have `attesters_per_slot`. let attesters_per_slot = 10; @@ -426,14 +419,9 @@ pub async fn proposer_boost_re_org_test( validator_count, None, Some(Box::new(move |builder| { - builder - .proposer_re_org_head_threshold(Some(ReOrgThreshold(re_org_threshold))) - .proposer_re_org_max_epochs_since_finalization(Epoch::new( - max_epochs_since_finalization, - )) - .proposer_re_org_disallowed_offsets( - DisallowedReOrgOffsets::new::(disallowed_offsets).unwrap(), - ) + builder.proposer_re_org_disallowed_offsets( + DisallowedReOrgOffsets::new::(disallowed_offsets).unwrap(), + ) })), Default::default(), false, diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 51cda0fac3b..39b8efa31be 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -1320,8 +1320,7 @@ pub fn cli_app() -> Command { .long("proposer-reorg-threshold") .action(ArgAction::Set) .value_name("PERCENT") - .help("Percentage of head vote weight below which to attempt a proposer reorg. \ - Default: 20%") + .help("DEPRECATED. This flag has no effect.") .conflicts_with("disable-proposer-reorgs") .display_order(0) ) @@ -1329,8 +1328,7 @@ pub fn cli_app() -> Command { Arg::new("proposer-reorg-parent-threshold") .long("proposer-reorg-parent-threshold") .value_name("PERCENT") - .help("Percentage of parent vote weight above which to attempt a proposer reorg. \ - Default: 160%") + .help("DEPRECATED. This flag has no effect.") .conflicts_with("disable-proposer-reorgs") .action(ArgAction::Set) .display_order(0) @@ -1340,8 +1338,7 @@ pub fn cli_app() -> Command { .long("proposer-reorg-epochs-since-finalization") .action(ArgAction::Set) .value_name("EPOCHS") - .help("Maximum number of epochs since finalization at which proposer reorgs are \ - allowed. Default: 2") + .help("DEPRECATED. This flag has no effect.") .conflicts_with("disable-proposer-reorgs") .display_order(0) ) @@ -1350,10 +1347,7 @@ pub fn cli_app() -> Command { .long("proposer-reorg-cutoff") .value_name("MILLISECONDS") .action(ArgAction::Set) - .help("Maximum delay after the start of the slot at which to propose a reorging \ - block. Lower values can prevent failed reorgs by ensuring the block has \ - ample time to propagate and be processed by the network. The default is \ - 1/12th of a slot (1 second on mainnet)") + .help("DEPRECATED. This flag has no effect.") .conflicts_with("disable-proposer-reorgs") .display_order(0) ) diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 8ba2c0f3214..e00afffb38d 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -1,8 +1,6 @@ use account_utils::{STDIN_INPUTS_FLAG, read_input_from_user}; use beacon_chain::chain_config::{ - DEFAULT_PREPARE_PAYLOAD_LOOKAHEAD_FACTOR, DEFAULT_RE_ORG_HEAD_THRESHOLD, - DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION, DEFAULT_RE_ORG_PARENT_THRESHOLD, - DisallowedReOrgOffsets, INVALID_HOLESKY_BLOCK_ROOT, ReOrgThreshold, + DEFAULT_PREPARE_PAYLOAD_LOOKAHEAD_FACTOR, DisallowedReOrgOffsets, INVALID_HOLESKY_BLOCK_ROOT, }; use beacon_chain::custody_context::NodeCustodyType; use beacon_chain::graffiti_calculator::GraffitiOrigin; @@ -741,41 +739,21 @@ pub fn get_config( .individual_tracking_threshold = count; } - if cli_args.get_flag("disable-proposer-reorgs") { - client_config.chain.re_org_head_threshold = None; - client_config.chain.re_org_parent_threshold = None; - } else { - client_config.chain.re_org_head_threshold = Some( - clap_utils::parse_optional(cli_args, "proposer-reorg-threshold")? - .map(ReOrgThreshold) - .unwrap_or(DEFAULT_RE_ORG_HEAD_THRESHOLD), - ); - client_config.chain.re_org_max_epochs_since_finalization = - clap_utils::parse_optional(cli_args, "proposer-reorg-epochs-since-finalization")? - .unwrap_or(DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION); - client_config.chain.re_org_cutoff_millis = - clap_utils::parse_optional(cli_args, "proposer-reorg-cutoff")?; - - client_config.chain.re_org_parent_threshold = Some( - clap_utils::parse_optional(cli_args, "proposer-reorg-parent-threshold")? - .map(ReOrgThreshold) - .unwrap_or(DEFAULT_RE_ORG_PARENT_THRESHOLD), - ); + client_config.chain.disable_proposer_reorg = cli_args.get_flag("disable-proposer-reorgs"); - if let Some(disallowed_offsets_str) = - clap_utils::parse_optional::(cli_args, "proposer-reorg-disallowed-offsets")? - { - let disallowed_offsets = disallowed_offsets_str - .split(',') - .map(|s| { - s.parse() - .map_err(|e| format!("invalid disallowed-offsets: {e:?}")) - }) - .collect::, _>>()?; - client_config.chain.re_org_disallowed_offsets = - DisallowedReOrgOffsets::new::(disallowed_offsets) - .map_err(|e| format!("invalid disallowed-offsets: {e:?}"))?; - } + if let Some(disallowed_offsets_str) = + clap_utils::parse_optional::(cli_args, "proposer-reorg-disallowed-offsets")? + { + let disallowed_offsets = disallowed_offsets_str + .split(',') + .map(|s| { + s.parse() + .map_err(|e| format!("invalid disallowed-offsets: {e:?}")) + }) + .collect::, _>>()?; + client_config.chain.re_org_disallowed_offsets = + DisallowedReOrgOffsets::new::(disallowed_offsets) + .map_err(|e| format!("invalid disallowed-offsets: {e:?}"))?; } client_config.chain.prepare_payload_lookahead = diff --git a/book/src/advanced_re-orgs.md b/book/src/advanced_re-orgs.md index 3a31778786d..71751f354fb 100644 --- a/book/src/advanced_re-orgs.md +++ b/book/src/advanced_re-orgs.md @@ -14,14 +14,6 @@ attestations and transactions that can be included. There are three flags which control the re-orging behaviour: * `--disable-proposer-reorgs`: turn re-orging off (it's on by default). -* `--proposer-reorg-threshold N`: attempt to orphan blocks with less than N% of the committee vote. If this parameter isn't set then N defaults to 20% when the feature is enabled. -* `--proposer-reorg-epochs-since-finalization N`: only attempt to re-org late blocks when the number of epochs since finalization is less than or equal to N. The default is 2 epochs, - meaning re-orgs will only be attempted when the chain is finalizing optimally. -* `--proposer-reorg-cutoff T`: only attempt to re-org late blocks when the proposal is being made - before T milliseconds into the slot. Delays between the validator client and the beacon node can - cause some blocks to be requested later than the start of the slot, which makes them more likely - to fail. The default cutoff is 1000ms on mainnet, which gives blocks 3000ms to be signed and - propagated before the attestation deadline at 4000ms. * `--proposer-reorg-disallowed-offsets N1,N2,N3...`: Prohibit Lighthouse from attempting to reorg at specific offsets in each epoch. A disallowed offset `N` prevents reorging blocks from being proposed at any `slot` such that `slot % SLOTS_PER_EPOCH == N`. The value to this flag is a diff --git a/book/src/help_bn.md b/book/src/help_bn.md index b580bcae528..a69d26dc0d0 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -306,10 +306,7 @@ Options: values are useful for ensuring the EL is given ample notice. Default: 1/3 of a slot. --proposer-reorg-cutoff - Maximum delay after the start of the slot at which to propose a - reorging block. Lower values can prevent failed reorgs by ensuring the - block has ample time to propagate and be processed by the network. The - default is 1/12th of a slot (1 second on mainnet) + DEPRECATED. This flag has no effect. --proposer-reorg-disallowed-offsets Comma-separated list of integer offsets which can be used to avoid proposing reorging blocks at certain slots. An offset of N means that @@ -318,14 +315,11 @@ Options: avoided. Any offsets supplied with this flag will impose additional restrictions. --proposer-reorg-epochs-since-finalization - Maximum number of epochs since finalization at which proposer reorgs - are allowed. Default: 2 + DEPRECATED. This flag has no effect. --proposer-reorg-parent-threshold - Percentage of parent vote weight above which to attempt a proposer - reorg. Default: 160% + DEPRECATED. This flag has no effect. --proposer-reorg-threshold - Percentage of head vote weight below which to attempt a proposer - reorg. Default: 20% + DEPRECATED. This flag has no effect. --prune-blobs Prune blobs from Lighthouse's database when they are older than the data data availability boundary relative to the current epoch. diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index 78f5026689a..d07f63d89ad 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -667,11 +667,9 @@ impl ProtoArray { justified_balances: &JustifiedBalances, spec: &ChainSpec, ) -> bool { - let reorg_threshold = calculate_committee_fraction::( - justified_balances, - spec.reorg_head_weight_threshold.unwrap_or(20), - ) - .unwrap_or(0); + let reorg_threshold = + calculate_committee_fraction::(justified_balances, spec.reorg_head_weight_threshold) + .unwrap_or(0); let head_weight = head_node .attestation_score(PayloadStatus::Pending) diff --git a/consensus/types/src/core/chain_spec.rs b/consensus/types/src/core/chain_spec.rs index c54d032891a..7f7dd0ea79a 100644 --- a/consensus/types/src/core/chain_spec.rs +++ b/consensus/types/src/core/chain_spec.rs @@ -150,9 +150,9 @@ pub struct ChainSpec { * Fork choice */ pub proposer_score_boost: Option, - pub reorg_head_weight_threshold: Option, - pub reorg_parent_weight_threshold: Option, - pub reorg_max_epochs_since_finalization: Option, + pub reorg_head_weight_threshold: u64, + pub reorg_parent_weight_threshold: u64, + pub reorg_max_epochs_since_finalization: u64, /* * Eth1 @@ -918,7 +918,7 @@ impl ChainSpec { } /// Calculate the duration into a slot for a given slot component - fn compute_slot_component_duration( + pub fn compute_slot_component_duration( &self, component_basis_points: u64, ) -> Result { @@ -1151,9 +1151,9 @@ impl ChainSpec { * Fork choice */ proposer_score_boost: Some(40), - reorg_head_weight_threshold: Some(20), - reorg_parent_weight_threshold: Some(160), - reorg_max_epochs_since_finalization: Some(2), + reorg_head_weight_threshold: 20, + reorg_parent_weight_threshold: 160, + reorg_max_epochs_since_finalization: 2, /* * Eth1 @@ -1573,9 +1573,9 @@ impl ChainSpec { * Fork choice */ proposer_score_boost: Some(40), - reorg_head_weight_threshold: Some(20), - reorg_parent_weight_threshold: Some(160), - reorg_max_epochs_since_finalization: Some(2), + reorg_head_weight_threshold: 20, + reorg_parent_weight_threshold: 160, + reorg_max_epochs_since_finalization: 2, /* * Eth1 @@ -2013,12 +2013,15 @@ pub struct Config { #[serde(skip_serializing_if = "Option::is_none")] proposer_score_boost: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - reorg_head_weight_threshold: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - reorg_parent_weight_threshold: Option>, - #[serde(skip_serializing_if = "Option::is_none")] - reorg_max_epochs_since_finalization: Option>, + #[serde(default = "default_reorg_head_weight_threshold")] + #[serde(with = "serde_utils::quoted_u64")] + reorg_head_weight_threshold: u64, + #[serde(default = "default_reorg_parent_weight_threshold")] + #[serde(with = "serde_utils::quoted_u64")] + reorg_parent_weight_threshold: u64, + #[serde(default = "default_reorg_max_epochs_since_finalization")] + #[serde(with = "serde_utils::quoted_u64")] + reorg_max_epochs_since_finalization: u64, #[serde(with = "serde_utils::quoted_u64")] deposit_chain_id: u64, @@ -2411,6 +2414,18 @@ const fn default_max_per_epoch_activation_churn_limit_gloas() -> u64 { 256_000_000_000 } +const fn default_reorg_head_weight_threshold() -> u64 { + 20 +} + +const fn default_reorg_parent_weight_threshold() -> u64 { + 160 +} + +const fn default_reorg_max_epochs_since_finalization() -> u64 { + 2 +} + fn max_blocks_by_root_request_common(max_request_blocks: u64) -> usize { let max_request_blocks = max_request_blocks as usize; RuntimeVariableList::::new( @@ -2604,15 +2619,9 @@ impl Config { max_per_epoch_activation_churn_limit: spec.max_per_epoch_activation_churn_limit, proposer_score_boost: spec.proposer_score_boost.map(|value| MaybeQuoted { value }), - reorg_head_weight_threshold: spec - .reorg_head_weight_threshold - .map(|value| MaybeQuoted { value }), - reorg_parent_weight_threshold: spec - .reorg_parent_weight_threshold - .map(|value| MaybeQuoted { value }), - reorg_max_epochs_since_finalization: spec - .reorg_max_epochs_since_finalization - .map(|value| MaybeQuoted { value }), + reorg_head_weight_threshold: spec.reorg_head_weight_threshold, + reorg_parent_weight_threshold: spec.reorg_parent_weight_threshold, + reorg_max_epochs_since_finalization: spec.reorg_max_epochs_since_finalization, deposit_chain_id: spec.deposit_chain_id, deposit_network_id: spec.deposit_network_id, @@ -2822,10 +2831,9 @@ impl Config { max_per_epoch_activation_churn_limit, churn_limit_quotient, proposer_score_boost: proposer_score_boost.map(|q| q.value), - reorg_head_weight_threshold: reorg_head_weight_threshold.map(|q| q.value), - reorg_parent_weight_threshold: reorg_parent_weight_threshold.map(|q| q.value), - reorg_max_epochs_since_finalization: reorg_max_epochs_since_finalization - .map(|q| q.value), + reorg_head_weight_threshold, + reorg_parent_weight_threshold, + reorg_max_epochs_since_finalization, deposit_chain_id, deposit_network_id, deposit_contract_address, diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 0c5d9a59334..d1978454fda 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -1,8 +1,6 @@ use crate::exec::{CommandLineTestExec, CompletedTest}; use beacon_node::beacon_chain::chain_config::{ - DEFAULT_RE_ORG_CUTOFF_DENOMINATOR, DEFAULT_RE_ORG_HEAD_THRESHOLD, - DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION, DEFAULT_SYNC_TOLERANCE_EPOCHS, - DisallowedReOrgOffsets, + DEFAULT_SYNC_TOLERANCE_EPOCHS, DisallowedReOrgOffsets, }; use beacon_node::beacon_chain::custody_context::NodeCustodyType; use beacon_node::{ @@ -2344,73 +2342,29 @@ fn ensure_panic_on_failed_launch() { fn enable_proposer_re_orgs_default() { CommandLineTest::new() .run_with_zero_port() - .with_config(|config| { - assert_eq!( - config.chain.re_org_head_threshold, - Some(DEFAULT_RE_ORG_HEAD_THRESHOLD) - ); - assert_eq!( - config.chain.re_org_max_epochs_since_finalization, - DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION, - ); - assert_eq!( - config.chain.re_org_cutoff(Duration::from_secs(12)), - Duration::from_secs(12) / DEFAULT_RE_ORG_CUTOFF_DENOMINATOR - ); - }); -} - -#[test] -fn disable_proposer_re_orgs() { - CommandLineTest::new() - .flag("disable-proposer-reorgs", None) - .run_with_zero_port() - .with_config(|config| { - assert_eq!(config.chain.re_org_head_threshold, None); - assert_eq!(config.chain.re_org_parent_threshold, None) + .with_config_and_spec::(|_config, spec| { + assert_eq!(spec.reorg_head_weight_threshold, 20); + assert_eq!(spec.reorg_parent_weight_threshold, 160); + assert_eq!(spec.reorg_max_epochs_since_finalization, 2); + assert_eq!(spec.proposer_reorg_cutoff_bps, 1667); }); } #[test] -fn proposer_re_org_parent_threshold() { - CommandLineTest::new() - .flag("proposer-reorg-parent-threshold", Some("90")) - .run_with_zero_port() - .with_config(|config| assert_eq!(config.chain.re_org_parent_threshold.unwrap().0, 90)); -} - -#[test] -fn proposer_re_org_head_threshold() { +fn enable_proposer_re_orgs() { CommandLineTest::new() - .flag("proposer-reorg-threshold", Some("90")) .run_with_zero_port() - .with_config(|config| assert_eq!(config.chain.re_org_head_threshold.unwrap().0, 90)); -} - -#[test] -fn proposer_re_org_max_epochs_since_finalization() { - CommandLineTest::new() - .flag("proposer-reorg-epochs-since-finalization", Some("8")) - .run_with_zero_port() - .with_config(|config| { - assert_eq!( - config.chain.re_org_max_epochs_since_finalization.as_u64(), - 8 - ) - }); + // Default disable_proposer_reorg when the flag is not used = false + .with_config(|config| assert!(!config.chain.disable_proposer_reorg)); } #[test] -fn proposer_re_org_cutoff() { +fn disable_proposer_re_orgs() { CommandLineTest::new() - .flag("proposer-reorg-cutoff", Some("500")) + .flag("disable-proposer-reorgs", None) .run_with_zero_port() - .with_config(|config| { - assert_eq!( - config.chain.re_org_cutoff(Duration::from_secs(12)), - Duration::from_millis(500) - ) - }); + // When --disable-proposer-reorg is used, the field in ChainConfig should become true + .with_config(|config| assert!(config.chain.disable_proposer_reorg)); } #[test] diff --git a/lighthouse/tests/exec.rs b/lighthouse/tests/exec.rs index a25558bc2f0..696cf2f40a4 100644 --- a/lighthouse/tests/exec.rs +++ b/lighthouse/tests/exec.rs @@ -144,7 +144,6 @@ impl CompletedTest { func(&self.config, &self.dir); } - #[allow(dead_code)] pub fn with_config_and_spec(self, func: F) { let spec = ChainSpec::from_config::(&self.chain_config).unwrap(); func(&self.config, spec); diff --git a/testing/ef_tests/Cargo.toml b/testing/ef_tests/Cargo.toml index 9d09c3dfe68..ac51e827ad6 100644 --- a/testing/ef_tests/Cargo.toml +++ b/testing/ef_tests/Cargo.toml @@ -28,6 +28,7 @@ hex = { workspace = true } kzg = { workspace = true } logging = { workspace = true } milhouse = { workspace = true } +proto_array = { workspace = true } rayon = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 8b0b74d2564..4f84525d9d0 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -4,10 +4,7 @@ use ::fork_choice::{PayloadVerificationStatus, ProposerHeadError}; use beacon_chain::beacon_proposer_cache::compute_proposer_duties_from_head; use beacon_chain::blob_verification::GossipBlobError; use beacon_chain::block_verification_types::LookupBlock; -use beacon_chain::chain_config::{ - DEFAULT_RE_ORG_HEAD_THRESHOLD, DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION, - DEFAULT_RE_ORG_PARENT_THRESHOLD, DisallowedReOrgOffsets, -}; +use beacon_chain::chain_config::DisallowedReOrgOffsets; use beacon_chain::data_column_verification::GossipVerifiedDataColumn; use beacon_chain::slot_clock::SlotClock; use beacon_chain::{ @@ -22,6 +19,7 @@ use beacon_chain::{ use execution_layer::{ PayloadStatusV1, PayloadStatusV1Status, json_structures::JsonPayloadStatusV1Status, }; +use proto_array::ReOrgThreshold; use serde::Deserialize; use ssz_derive::Decode; use state_processing::VerifySignatures; @@ -33,9 +31,9 @@ use std::time::Duration; use types::{ Attestation, AttestationRef, AttesterSlashing, AttesterSlashingRef, BeaconBlock, BeaconState, BlobSidecar, BlobsList, BlockImportSource, Checkpoint, DataColumnSidecar, - DataColumnSidecarList, DataColumnSubnetId, ExecutionBlockHash, Hash256, IndexedAttestation, - KzgProof, ProposerPreparationData, SignedBeaconBlock, SignedExecutionPayloadEnvelope, Slot, - Uint256, + DataColumnSidecarList, DataColumnSubnetId, Epoch, ExecutionBlockHash, Hash256, + IndexedAttestation, KzgProof, ProposerPreparationData, SignedBeaconBlock, + SignedExecutionPayloadEnvelope, Slot, Uint256, }; // When set to true, cache any states fetched from the db. @@ -971,10 +969,10 @@ impl Tester { let proposer_head_result = fc.get_proposer_head( slot, canonical_head, - DEFAULT_RE_ORG_HEAD_THRESHOLD, - DEFAULT_RE_ORG_PARENT_THRESHOLD, + ReOrgThreshold(self.spec.reorg_head_weight_threshold), + ReOrgThreshold(self.spec.reorg_parent_weight_threshold), &DisallowedReOrgOffsets::default(), - DEFAULT_RE_ORG_MAX_EPOCHS_SINCE_FINALIZATION, + Epoch::new(self.spec.reorg_max_epochs_since_finalization), ); let proposer_head = match proposer_head_result { Ok(head) => head.parent_node.root(),