From eb11da8adffc522e07490da89d9a295410dc1436 Mon Sep 17 00:00:00 2001 From: Dan Cline <6798349+Rjected@users.noreply.github.com> Date: Fri, 20 Jan 2023 04:53:01 -0500 Subject: [PATCH] fix(net): set status, forkfilter from chainspec (#939) --- crates/net/eth-wire/src/types/status.rs | 19 ++++++- crates/net/network/Cargo.toml | 1 + crates/net/network/src/config.rs | 74 +++++++++++++------------ 3 files changed, 59 insertions(+), 35 deletions(-) diff --git a/crates/net/eth-wire/src/types/status.rs b/crates/net/eth-wire/src/types/status.rs index ddf0f79cf4..43b27f62de 100644 --- a/crates/net/eth-wire/src/types/status.rs +++ b/crates/net/eth-wire/src/types/status.rs @@ -1,4 +1,4 @@ -use crate::{EthVersion, StatusBuilder}; +use crate::{BlockHashNumber, EthVersion, StatusBuilder}; use ethers_core::utils::Genesis; use reth_codecs::derive_arbitrary; @@ -82,6 +82,23 @@ impl Status { pub fn builder() -> StatusBuilder { Default::default() } + + /// Create a [`StatusBuilder`] from the given [`ChainSpec`](reth_primitives::ChainSpec) and + /// head block number. + /// + /// Sets the `chain` and `genesis`, `blockhash`, and `forkid` fields based on the [`ChainSpec`] + /// and head. + /// + /// The user should set the `total_difficulty` field if desired. Otherwise, the total + /// difficulty will be set to the [`Default`](std::default::Default) implementation for + /// [`Status`]. + pub fn spec_builder(spec: &ChainSpec, head: &BlockHashNumber) -> StatusBuilder { + Self::builder() + .chain(spec.chain) + .genesis(spec.genesis_hash()) + .blockhash(head.hash) + .forkid(spec.fork_id(head.number)) + } } impl Display for Status { diff --git a/crates/net/network/Cargo.toml b/crates/net/network/Cargo.toml index 25f613f7dd..b344f13c17 100644 --- a/crates/net/network/Cargo.toml +++ b/crates/net/network/Cargo.toml @@ -94,5 +94,6 @@ tempfile = "3.3" serial_test = "0.10" [features] +default = ["serde"] serde = ["dep:serde", "dep:humantime-serde"] test-utils = ["reth-provider/test-utils", "dep:enr", "dep:ethers-core", "dep:tempfile"] diff --git a/crates/net/network/src/config.rs b/crates/net/network/src/config.rs index ce7fb5ae20..8d6e0802d4 100644 --- a/crates/net/network/src/config.rs +++ b/crates/net/network/src/config.rs @@ -28,7 +28,7 @@ mod __reexport { pub use __reexport::*; use reth_dns_discovery::DnsDiscoveryConfig; use reth_ecies::util::pk2id; -use reth_eth_wire::{HelloMessage, Status}; +use reth_eth_wire::{BlockHashNumber, HelloMessage, Status}; /// Convenience function to create a new random [`SecretKey`] pub fn rng_secret_key() -> SecretKey { @@ -146,14 +146,10 @@ pub struct NetworkConfigBuilder { /// The executor to use for spawning tasks. #[serde(skip)] executor: Option, - /// The `Status` message to send to peers at the beginning. - status: Option, /// Sets the hello message for the p2p handshake in RLPx hello_message: Option, - /// The [`ForkFilter`] to use at launch for authenticating sessions. - fork_filter: Option, - /// Head used to start set for the fork filter - head: Option, + /// Head used to start set for the fork filter and status. + head: Option, } // === impl NetworkConfigBuilder === @@ -173,9 +169,7 @@ impl NetworkConfigBuilder { chain_spec: MAINNET.clone(), network_mode: Default::default(), executor: None, - status: None, hello_message: None, - fork_filter: None, head: None, } } @@ -185,22 +179,6 @@ impl NetworkConfigBuilder { pk2id(&self.secret_key.public_key(SECP256K1)) } - /// Sets the `Status` message to send when connecting to peers. - /// - /// ``` - /// # use reth_eth_wire::Status; - /// # use reth_network::NetworkConfigBuilder; - /// # fn builder(builder: NetworkConfigBuilder) { - /// builder.status( - /// Status::builder().build() - /// ); - /// # } - /// ``` - pub fn status(mut self, status: Status) -> Self { - self.status = Some(status); - self - } - /// Sets the chain spec. pub fn chain_spec(mut self, chain_spec: ChainSpec) -> Self { self.chain_spec = chain_spec; @@ -301,9 +279,7 @@ impl NetworkConfigBuilder { chain_spec, network_mode, executor, - status, hello_message, - fork_filter, head, } = self; @@ -315,11 +291,13 @@ impl NetworkConfigBuilder { hello_message.unwrap_or_else(|| HelloMessage::builder(peer_id).build()); hello_message.port = listener_addr.port(); - // get the fork filter - let fork_filter = fork_filter.unwrap_or_else(|| { - let head = head.unwrap_or_default(); - chain_spec.fork_filter(head) - }); + let head = head.unwrap_or(BlockHashNumber { hash: chain_spec.genesis_hash(), number: 0 }); + + // set the status + let status = Status::spec_builder(&chain_spec, &head).build(); + + // set a fork filter based on the chain spec and head + let fork_filter = chain_spec.fork_filter(head.number); // If default DNS config is used then we add the known dns network to bootstrap from if let Some(dns_networks) = @@ -348,7 +326,7 @@ impl NetworkConfigBuilder { block_import: Box::::default(), network_mode, executor, - status: status.unwrap_or_default(), + status, hello_message, fork_filter, } @@ -383,8 +361,9 @@ mod tests { use super::*; use rand::thread_rng; use reth_dns_discovery::tree::LinkEntry; - use reth_primitives::Chain; + use reth_primitives::{Chain, ForkHash}; use reth_provider::test_utils::NoopProvider; + use std::collections::BTreeMap; fn builder() -> NetworkConfigBuilder { let secret_key = SecretKey::new(&mut thread_rng()); @@ -402,4 +381,31 @@ mod tests { assert!(bootstrap_nodes.contains(&mainnet_dns)); assert_eq!(bootstrap_nodes.len(), 1); } + + #[test] + fn test_network_fork_filter_default() { + let mut chain_spec = MAINNET.clone(); + + // remove any `next` fields we would have by removing all hardforks + chain_spec.hardforks = BTreeMap::new(); + + // check that the forkid is initialized with the genesis and no other forks + let genesis_fork_hash = ForkHash::from(chain_spec.genesis_hash()); + + // enforce that the fork_id set in the status is consistent with the generated fork filter + let config = builder().chain_spec(chain_spec).build(Arc::new(NoopProvider::default())); + + let status = config.status; + let fork_filter = config.fork_filter; + + // assert that there are no other forks + assert_eq!(status.forkid.next, 0); + + // assert the same thing for the fork_filter + assert_eq!(fork_filter.current().next, 0); + + // check status and fork_filter forkhash + assert_eq!(status.forkid.hash, genesis_fork_hash); + assert_eq!(fork_filter.current().hash, genesis_fork_hash); + } }