Ensure difficulty/hash/epoch overrides change the ChainSpec (#2798)

* Unify loading of eth2_network_config

* Apply overrides at lighthouse binary level

* Remove duplicate override values

* Add merge values to existing net configs

* Make override flags global

* Add merge fields to testing config

* Add one to TTD

* Fix failing engine tests

* Fix test compile error

* Remove TTD flags

* Move get_eth2_network_config

* Fix warn

* Address review comments
This commit is contained in:
Paul Hauner
2021-12-02 14:29:18 +11:00
parent 47db682d7e
commit afe59afacd
25 changed files with 391 additions and 267 deletions
+3 -1
View File
@@ -2893,7 +2893,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
let parent_hash;
if !is_merge_complete(&state) {
let terminal_pow_block_hash = execution_layer
.block_on(|execution_layer| execution_layer.get_terminal_pow_block_hash())
.block_on(|execution_layer| {
execution_layer.get_terminal_pow_block_hash(&self.spec)
})
.map_err(BlockProductionError::TerminalPoWBlockLookupFailed)?;
if let Some(terminal_pow_block_hash) = terminal_pow_block_hash {
@@ -1119,7 +1119,10 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> {
let is_valid_terminal_pow_block = execution_layer
.block_on(|execution_layer| {
execution_layer.is_valid_terminal_pow_block_hash(execution_payload.parent_hash)
execution_layer.is_valid_terminal_pow_block_hash(
execution_payload.parent_hash,
&chain.spec,
)
})
.map_err(ExecutionPayloadError::from)?;
+1 -3
View File
@@ -330,7 +330,6 @@ where
}
pub fn execution_layer(mut self, urls: &[&str]) -> Self {
let spec = self.spec.clone().expect("cannot build without spec");
assert!(
self.execution_layer.is_none(),
"execution layer already defined"
@@ -345,8 +344,6 @@ where
.unwrap();
let execution_layer = ExecutionLayer::from_urls(
urls,
spec.terminal_total_difficulty,
spec.terminal_block_hash,
Some(Address::repeat_byte(42)),
el_runtime.task_executor.clone(),
el_runtime.log.clone(),
@@ -364,6 +361,7 @@ where
spec.terminal_total_difficulty,
DEFAULT_TERMINAL_BLOCK,
spec.terminal_block_hash,
spec.terminal_block_hash_activation_epoch,
);
self.execution_layer = Some(mock.el.clone());
self.mock_execution_layer = Some(mock);
-9
View File
@@ -148,19 +148,10 @@ where
None
};
let terminal_total_difficulty = config
.terminal_total_difficulty_override
.unwrap_or(spec.terminal_total_difficulty);
let terminal_block_hash = config
.terminal_block_hash_override
.unwrap_or(spec.terminal_block_hash);
let execution_layer = if let Some(execution_endpoints) = config.execution_endpoints {
let context = runtime_context.service_context("exec".into());
let execution_layer = ExecutionLayer::from_urls(
execution_endpoints,
terminal_total_difficulty,
terminal_block_hash,
config.fee_recipient,
context.executor.clone(),
context.log().clone(),
+1 -8
View File
@@ -1,11 +1,10 @@
use beacon_chain::types::Epoch;
use directory::DEFAULT_ROOT_DIR;
use network::NetworkConfig;
use sensitive_url::SensitiveUrl;
use serde_derive::{Deserialize, Serialize};
use std::fs;
use std::path::PathBuf;
use types::{Address, Graffiti, Hash256, PublicKeyBytes, Uint256};
use types::{Address, Graffiti, PublicKeyBytes};
/// Default directory name for the freezer database under the top-level data dir.
const DEFAULT_FREEZER_DB_DIR: &str = "freezer_db";
@@ -76,9 +75,6 @@ pub struct Config {
pub chain: beacon_chain::ChainConfig,
pub eth1: eth1::Config,
pub execution_endpoints: Option<Vec<SensitiveUrl>>,
pub terminal_total_difficulty_override: Option<Uint256>,
pub terminal_block_hash_override: Option<Hash256>,
pub terminal_block_hash_epoch_override: Option<Epoch>,
pub fee_recipient: Option<Address>,
pub http_api: http_api::Config,
pub http_metrics: http_metrics::Config,
@@ -101,9 +97,6 @@ impl Default for Config {
sync_eth1_chain: false,
eth1: <_>::default(),
execution_endpoints: None,
terminal_total_difficulty_override: None,
terminal_block_hash_override: None,
terminal_block_hash_epoch_override: None,
fee_recipient: None,
disabled_forks: Vec::new(),
graffiti: Graffiti::default(),
+34 -37
View File
@@ -18,6 +18,7 @@ use tokio::{
sync::{Mutex, MutexGuard},
time::{sleep, sleep_until, Instant},
};
use types::ChainSpec;
pub use engine_api::{http::HttpJsonRpc, ExecutePayloadResponseStatus};
@@ -47,8 +48,6 @@ impl From<ApiError> for Error {
struct Inner {
engines: Engines<HttpJsonRpc>,
terminal_total_difficulty: Uint256,
terminal_block_hash: Hash256,
fee_recipient: Option<Address>,
execution_blocks: Mutex<LruCache<Hash256, ExecutionBlock>>,
executor: TaskExecutor,
@@ -73,8 +72,6 @@ impl ExecutionLayer {
/// Instantiate `Self` with `urls.len()` engines, all using the JSON-RPC via HTTP.
pub fn from_urls(
urls: Vec<SensitiveUrl>,
terminal_total_difficulty: Uint256,
terminal_block_hash: Hash256,
fee_recipient: Option<Address>,
executor: TaskExecutor,
log: Logger,
@@ -98,8 +95,6 @@ impl ExecutionLayer {
latest_forkchoice_state: <_>::default(),
log: log.clone(),
},
terminal_total_difficulty,
terminal_block_hash,
fee_recipient,
execution_blocks: Mutex::new(LruCache::new(EXECUTION_BLOCKS_LRU_CACHE_SIZE)),
executor,
@@ -121,14 +116,6 @@ impl ExecutionLayer {
&self.inner.executor
}
fn terminal_total_difficulty(&self) -> Uint256 {
self.inner.terminal_total_difficulty
}
fn terminal_block_hash(&self) -> Hash256 {
self.inner.terminal_block_hash
}
fn fee_recipient(&self) -> Result<Address, Error> {
self.inner
.fee_recipient
@@ -455,11 +442,14 @@ impl ExecutionLayer {
/// `get_terminal_pow_block_hash`
///
/// https://github.com/ethereum/consensus-specs/blob/v1.1.0/specs/merge/validator.md
pub async fn get_terminal_pow_block_hash(&self) -> Result<Option<Hash256>, Error> {
pub async fn get_terminal_pow_block_hash(
&self,
spec: &ChainSpec,
) -> Result<Option<Hash256>, Error> {
let hash_opt = self
.engines()
.first_success(|engine| async move {
if self.terminal_block_hash() != Hash256::zero() {
if spec.terminal_block_hash != Hash256::zero() {
// Note: the specification is written such that if there are multiple blocks in
// the PoW chain with the terminal block hash, then to select 0'th one.
//
@@ -468,11 +458,12 @@ impl ExecutionLayer {
// hash. Such a scenario would be a devestating hash collision with external
// implications far outweighing those here.
Ok(self
.get_pow_block(engine, self.terminal_block_hash())
.get_pow_block(engine, spec.terminal_block_hash)
.await?
.map(|block| block.block_hash))
} else {
self.get_pow_block_hash_at_total_difficulty(engine).await
self.get_pow_block_hash_at_total_difficulty(engine, spec)
.await
}
})
.await
@@ -482,8 +473,8 @@ impl ExecutionLayer {
info!(
self.log(),
"Found terminal block hash";
"terminal_block_hash_override" => ?self.terminal_block_hash(),
"terminal_total_difficulty" => ?self.terminal_total_difficulty(),
"terminal_block_hash_override" => ?spec.terminal_block_hash,
"terminal_total_difficulty" => ?spec.terminal_total_difficulty,
"block_hash" => ?hash,
);
}
@@ -503,6 +494,7 @@ impl ExecutionLayer {
async fn get_pow_block_hash_at_total_difficulty(
&self,
engine: &Engine<HttpJsonRpc>,
spec: &ChainSpec,
) -> Result<Option<Hash256>, ApiError> {
let mut ttd_exceeding_block = None;
let mut block = engine
@@ -518,7 +510,7 @@ impl ExecutionLayer {
//
// https://github.com/ethereum/consensus-specs/issues/2636
loop {
if block.total_difficulty >= self.terminal_total_difficulty() {
if block.total_difficulty >= spec.terminal_total_difficulty {
ttd_exceeding_block = Some(block.block_hash);
// Try to prevent infinite loops.
@@ -565,6 +557,7 @@ impl ExecutionLayer {
pub async fn is_valid_terminal_pow_block_hash(
&self,
block_hash: Hash256,
spec: &ChainSpec,
) -> Result<Option<bool>, Error> {
let broadcast_results = self
.engines()
@@ -574,7 +567,7 @@ impl ExecutionLayer {
self.get_pow_block(engine, pow_block.parent_hash).await?
{
return Ok(Some(
self.is_valid_terminal_pow_block(pow_block, pow_parent),
self.is_valid_terminal_pow_block(pow_block, pow_parent, spec),
));
}
}
@@ -618,15 +611,19 @@ impl ExecutionLayer {
/// This function should remain internal.
///
/// External users should use `self.is_valid_terminal_pow_block_hash`.
fn is_valid_terminal_pow_block(&self, block: ExecutionBlock, parent: ExecutionBlock) -> bool {
if block.block_hash == self.terminal_block_hash() {
fn is_valid_terminal_pow_block(
&self,
block: ExecutionBlock,
parent: ExecutionBlock,
spec: &ChainSpec,
) -> bool {
if block.block_hash == spec.terminal_block_hash {
return true;
}
let is_total_difficulty_reached =
block.total_difficulty >= self.terminal_total_difficulty();
let is_total_difficulty_reached = block.total_difficulty >= spec.terminal_total_difficulty;
let is_parent_total_difficulty_valid =
parent.total_difficulty < self.terminal_total_difficulty();
parent.total_difficulty < spec.terminal_total_difficulty;
is_total_difficulty_reached && is_parent_total_difficulty_valid
}
@@ -685,14 +682,14 @@ mod test {
async fn finds_valid_terminal_block_hash() {
MockExecutionLayer::default_params()
.move_to_block_prior_to_terminal_block()
.with_terminal_block(|el, _| async move {
assert_eq!(el.get_terminal_pow_block_hash().await.unwrap(), None)
.with_terminal_block(|spec, el, _| async move {
assert_eq!(el.get_terminal_pow_block_hash(&spec).await.unwrap(), None)
})
.await
.move_to_terminal_block()
.with_terminal_block(|el, terminal_block| async move {
.with_terminal_block(|spec, el, terminal_block| async move {
assert_eq!(
el.get_terminal_pow_block_hash().await.unwrap(),
el.get_terminal_pow_block_hash(&spec).await.unwrap(),
Some(terminal_block.unwrap().block_hash)
)
})
@@ -703,9 +700,9 @@ mod test {
async fn verifies_valid_terminal_block_hash() {
MockExecutionLayer::default_params()
.move_to_terminal_block()
.with_terminal_block(|el, terminal_block| async move {
.with_terminal_block(|spec, el, terminal_block| async move {
assert_eq!(
el.is_valid_terminal_pow_block_hash(terminal_block.unwrap().block_hash)
el.is_valid_terminal_pow_block_hash(terminal_block.unwrap().block_hash, &spec)
.await
.unwrap(),
Some(true)
@@ -718,11 +715,11 @@ mod test {
async fn rejects_invalid_terminal_block_hash() {
MockExecutionLayer::default_params()
.move_to_terminal_block()
.with_terminal_block(|el, terminal_block| async move {
.with_terminal_block(|spec, el, terminal_block| async move {
let invalid_terminal_block = terminal_block.unwrap().parent_hash;
assert_eq!(
el.is_valid_terminal_pow_block_hash(invalid_terminal_block)
el.is_valid_terminal_pow_block_hash(invalid_terminal_block, &spec)
.await
.unwrap(),
Some(false)
@@ -735,11 +732,11 @@ mod test {
async fn rejects_unknown_terminal_block_hash() {
MockExecutionLayer::default_params()
.move_to_terminal_block()
.with_terminal_block(|el, _| async move {
.with_terminal_block(|spec, el, _| async move {
let missing_terminal_block = Hash256::repeat_byte(42);
assert_eq!(
el.is_valid_terminal_pow_block_hash(missing_terminal_block)
el.is_valid_terminal_pow_block_hash(missing_terminal_block, &spec)
.await
.unwrap(),
None
@@ -6,7 +6,7 @@ use environment::null_logger;
use sensitive_url::SensitiveUrl;
use std::sync::Arc;
use task_executor::TaskExecutor;
use types::{Address, EthSpec, Hash256, Uint256};
use types::{Address, ChainSpec, Epoch, EthSpec, Hash256, Uint256};
pub struct ExecutionLayerRuntime {
pub runtime: Option<Arc<tokio::runtime::Runtime>>,
@@ -50,6 +50,7 @@ pub struct MockExecutionLayer<T: EthSpec> {
pub server: MockServer<T>,
pub el: ExecutionLayer,
pub el_runtime: ExecutionLayerRuntime,
pub spec: ChainSpec,
}
impl<T: EthSpec> MockExecutionLayer<T> {
@@ -58,6 +59,7 @@ impl<T: EthSpec> MockExecutionLayer<T> {
DEFAULT_TERMINAL_DIFFICULTY.into(),
DEFAULT_TERMINAL_BLOCK,
Hash256::zero(),
Epoch::new(0),
)
}
@@ -65,10 +67,16 @@ impl<T: EthSpec> MockExecutionLayer<T> {
terminal_total_difficulty: Uint256,
terminal_block: u64,
terminal_block_hash: Hash256,
terminal_block_hash_activation_epoch: Epoch,
) -> Self {
let el_runtime = ExecutionLayerRuntime::default();
let handle = el_runtime.runtime.as_ref().unwrap().handle();
let mut spec = T::default_spec();
spec.terminal_total_difficulty = terminal_total_difficulty;
spec.terminal_block_hash = terminal_block_hash;
spec.terminal_block_hash_activation_epoch = terminal_block_hash_activation_epoch;
let server = MockServer::new(
handle,
terminal_total_difficulty,
@@ -80,8 +88,6 @@ impl<T: EthSpec> MockExecutionLayer<T> {
let el = ExecutionLayer::from_urls(
vec![url],
terminal_total_difficulty,
Hash256::zero(),
Some(Address::repeat_byte(42)),
el_runtime.task_executor.clone(),
el_runtime.log.clone(),
@@ -92,6 +98,7 @@ impl<T: EthSpec> MockExecutionLayer<T> {
server,
el,
el_runtime,
spec,
}
}
@@ -171,7 +178,7 @@ impl<T: EthSpec> MockExecutionLayer<T> {
pub async fn with_terminal_block<'a, U, V>(self, func: U) -> Self
where
U: Fn(ExecutionLayer, Option<ExecutionBlock>) -> V,
U: Fn(ChainSpec, ExecutionLayer, Option<ExecutionBlock>) -> V,
V: Future<Output = ()>,
{
let terminal_block_number = self
@@ -183,7 +190,7 @@ impl<T: EthSpec> MockExecutionLayer<T> {
.execution_block_generator()
.execution_block_by_number(terminal_block_number);
func(self.el.clone(), terminal_block).await;
func(self.spec.clone(), self.el.clone(), terminal_block).await;
self
}
}
-36
View File
@@ -399,42 +399,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
will be used. Defaults to http://127.0.0.1:8545.")
.takes_value(true)
)
.arg(
Arg::with_name("terminal-total-difficulty-override")
.long("terminal-total-difficulty-override")
.value_name("INTEGER")
.help("Used to coordinate manual overrides to the TERMINAL_TOTAL_DIFFICULTY parameter. \
This flag should only be used if the user has a clear understanding that \
the broad Ethereum community has elected to override the terminal difficulty. \
Incorrect use of this flag will cause your node to experience a consensus
failure. Be extremely careful with this flag.")
.takes_value(true)
)
.arg(
Arg::with_name("terminal-block-hash-override")
.long("terminal-block-hash-override")
.value_name("TERMINAL_BLOCK_HASH")
.help("Used to coordinate manual overrides to the TERMINAL_BLOCK_HASH parameter. \
Accepts a 256-bit decimal integer (not a hex value). \
This flag should only be used if the user has a clear understanding that \
the broad Ethereum community has elected to override the terminal PoW block. \
Incorrect use of this flag will cause your node to experience a consensus
failure. Be extremely careful with this flag.")
.requires("terminal-block-hash-epoch-override")
.takes_value(true)
)
.arg(
Arg::with_name("terminal-block-hash-epoch-override")
.long("terminal-block-hash-epoch-override")
.value_name("EPOCH")
.help("Used to coordinate manual overrides to the TERMINAL_BLOCK_HASH_ACTIVATION_EPOCH \
parameter. This flag should only be used if the user has a clear understanding \
that the broad Ethereum community has elected to override the terminal PoW block. \
Incorrect use of this flag will cause your node to experience a consensus
failure. Be extremely careful with this flag.")
.requires("terminal-block-hash-override")
.takes_value(true)
)
.arg(
Arg::with_name("fee-recipient")
.long("fee-recipient")
+18 -51
View File
@@ -1,8 +1,8 @@
use clap::ArgMatches;
use clap_utils::{flags::DISABLE_MALLOC_TUNING_FLAG, BAD_TESTNET_DIR_MESSAGE};
use clap_utils::flags::DISABLE_MALLOC_TUNING_FLAG;
use client::{ClientConfig, ClientGenesis};
use directory::{DEFAULT_BEACON_NODE_DIR, DEFAULT_NETWORK_DIR, DEFAULT_ROOT_DIR};
use eth2_network_config::{Eth2NetworkConfig, DEFAULT_HARDCODED_NETWORK};
use environment::RuntimeContext;
use http_api::TlsConfig;
use lighthouse_network::{multiaddr::Protocol, Enr, Multiaddr, NetworkConfig, PeerIdSerialized};
use sensitive_url::SensitiveUrl;
@@ -14,9 +14,7 @@ use std::net::{IpAddr, Ipv4Addr, ToSocketAddrs};
use std::net::{TcpListener, UdpSocket};
use std::path::{Path, PathBuf};
use std::str::FromStr;
use types::{
ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, PublicKeyBytes, Uint256, GRAFFITI_BYTES_LEN,
};
use types::{Checkpoint, Epoch, EthSpec, Hash256, PublicKeyBytes, GRAFFITI_BYTES_LEN};
/// Gets the fully-initialized global client.
///
@@ -27,9 +25,11 @@ use types::{
/// response of some remote server.
pub fn get_config<E: EthSpec>(
cli_args: &ArgMatches,
spec: &ChainSpec,
log: Logger,
context: &RuntimeContext<E>,
) -> Result<ClientConfig, String> {
let spec = &context.eth2_config.spec;
let log = context.log();
let mut client_config = ClientConfig {
data_dir: get_data_dir(cli_args),
..Default::default()
@@ -63,7 +63,7 @@ pub fn get_config<E: EthSpec>(
&mut client_config.network,
cli_args,
&client_config.data_dir,
&log,
log,
false,
)?;
@@ -242,32 +242,7 @@ pub fn get_config<E: EthSpec>(
client_config.execution_endpoints = Some(client_config.eth1.endpoints.clone());
}
if let Some(string) =
clap_utils::parse_optional::<String>(cli_args, "terminal-total-difficulty-override")?
{
let stripped = string.replace(",", "");
let terminal_total_difficulty = Uint256::from_dec_str(&stripped).map_err(|e| {
format!(
"Could not parse --terminal-total-difficulty-override as decimal value: {:?}",
e
)
})?;
if client_config.execution_endpoints.is_none() {
return Err(
"The --merge flag must be provided when using --terminal-total-difficulty-override"
.into(),
);
}
client_config.terminal_total_difficulty_override = Some(terminal_total_difficulty);
}
client_config.fee_recipient = clap_utils::parse_optional(cli_args, "fee-recipient")?;
client_config.terminal_block_hash_override =
clap_utils::parse_optional(cli_args, "terminal-block-hash-override")?;
client_config.terminal_block_hash_epoch_override =
clap_utils::parse_optional(cli_args, "terminal-block-hash-epoch-override")?;
if let Some(freezer_dir) = cli_args.value_of("freezer-dir") {
client_config.freezer_db_path = Some(PathBuf::from(freezer_dir));
@@ -321,7 +296,10 @@ pub fn get_config<E: EthSpec>(
/*
* Load the eth2 network dir to obtain some additional config values.
*/
let eth2_network_config = get_eth2_network_config(cli_args)?;
let eth2_network_config = context
.eth2_network_config
.as_ref()
.ok_or("Context is missing eth2 network config")?;
client_config.eth1.deposit_contract_address = format!("{:?}", spec.deposit_contract_address);
client_config.eth1.deposit_contract_deploy_block =
@@ -344,13 +322,16 @@ pub fn get_config<E: EthSpec>(
// Only append network config bootnodes if discovery is not disabled
if !client_config.network.disable_discovery {
if let Some(mut boot_nodes) = eth2_network_config.boot_enr {
client_config.network.boot_nodes_enr.append(&mut boot_nodes)
if let Some(boot_nodes) = &eth2_network_config.boot_enr {
client_config
.network
.boot_nodes_enr
.extend_from_slice(boot_nodes)
}
}
client_config.genesis = if let Some(genesis_state_bytes) =
eth2_network_config.genesis_state_bytes
eth2_network_config.genesis_state_bytes.clone()
{
// Set up weak subjectivity sync, or start from the hardcoded genesis state.
if let (Some(initial_state_path), Some(initial_block_path)) = (
@@ -782,20 +763,6 @@ pub fn get_data_dir(cli_args: &ArgMatches) -> PathBuf {
.unwrap_or_else(|| PathBuf::from("."))
}
/// Try to parse the eth2 network config from the `network`, `testnet-dir` flags in that order.
/// Returns the default hardcoded testnet if neither flags are set.
pub fn get_eth2_network_config(cli_args: &ArgMatches) -> Result<Eth2NetworkConfig, String> {
let optional_network_config = if cli_args.is_present("network") {
clap_utils::parse_hardcoded_network(cli_args, "network")?
} else if cli_args.is_present("testnet-dir") {
clap_utils::parse_testnet_dir(cli_args, "testnet-dir")?
} else {
// if neither is present, assume the default network
Eth2NetworkConfig::constant(DEFAULT_HARDCODED_NETWORK)?
};
optional_network_config.ok_or_else(|| BAD_TESTNET_DIR_MESSAGE.to_string())
}
/// A bit of hack to find an unused port.
///
/// Does not guarantee that the given port is unused after the function exits, just that it was
+2 -3
View File
@@ -13,7 +13,7 @@ use beacon_chain::{
use clap::ArgMatches;
pub use cli::cli_app;
pub use client::{Client, ClientBuilder, ClientConfig, ClientGenesis};
pub use config::{get_config, get_data_dir, get_eth2_network_config, set_network_config};
pub use config::{get_config, get_data_dir, set_network_config};
use environment::RuntimeContext;
pub use eth2_config::Eth2Config;
use slasher::Slasher;
@@ -46,8 +46,7 @@ impl<E: EthSpec> ProductionBeaconNode<E> {
context: RuntimeContext<E>,
matches: ArgMatches<'static>,
) -> Result<Self, String> {
let client_config =
get_config::<E>(&matches, &context.eth2_config().spec, context.log().clone())?;
let client_config = get_config::<E>(&matches, &context)?;
Self::new(context, client_config).await
}