From 115e623ae630b3c1841841dfe192a8cc63505786 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien?= Date: Wed, 18 Jan 2023 11:17:43 +0100 Subject: [PATCH] Ability to (de)serialize NetworkConfigBuilder (#897) --- Cargo.lock | 5 +++ bin/reth/src/config.rs | 6 +-- crates/net/discv4/Cargo.toml | 5 ++- crates/net/discv4/src/config.rs | 6 ++- crates/net/dns/Cargo.toml | 3 +- crates/net/dns/src/config.rs | 4 +- crates/net/dns/src/tree.rs | 3 +- crates/net/nat/Cargo.toml | 1 + crates/net/nat/src/lib.rs | 5 ++- crates/net/network/Cargo.toml | 1 + crates/net/network/src/config.rs | 42 ++++++++------------ crates/net/network/src/session/config.rs | 3 ++ crates/net/network/src/test_utils/testnet.rs | 8 ++-- crates/net/network/src/transactions.rs | 4 +- crates/net/network/tests/it/connect.rs | 26 ++++++------ crates/primitives/src/forkid.rs | 4 +- 16 files changed, 69 insertions(+), 57 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e2cf3c1d1..e3b5173a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4056,6 +4056,7 @@ dependencies = [ "reth-rlp-derive", "reth-tracing", "secp256k1 0.24.2", + "serde", "thiserror", "tokio", "tokio-stream", @@ -4078,6 +4079,8 @@ dependencies = [ "reth-rlp", "reth-tracing", "secp256k1 0.24.2", + "serde", + "serde_with", "thiserror", "tokio", "tokio-stream", @@ -4302,6 +4305,7 @@ dependencies = [ "pin-project-lite", "public-ip", "reth-tracing", + "serde_with", "thiserror", "tokio", "tracing", @@ -4919,6 +4923,7 @@ checksum = "d9512ffd81e3a3503ed401f79c33168b9148c75038956039166cd750eaa037c3" dependencies = [ "rand 0.8.5", "secp256k1-sys 0.6.1", + "serde", ] [[package]] diff --git a/bin/reth/src/config.rs b/bin/reth/src/config.rs index a024ad0e3..e59e48eaf 100644 --- a/bin/reth/src/config.rs +++ b/bin/reth/src/config.rs @@ -5,7 +5,7 @@ use reth_db::database::Database; use reth_discv4::Discv4Config; use reth_network::{ config::{mainnet_nodes, rng_secret_key}, - NetworkConfig, PeersConfig, + NetworkConfig, NetworkConfigBuilder, PeersConfig, }; use reth_primitives::{ChainSpec, NodeRecord}; use reth_provider::ProviderImpl; @@ -37,13 +37,13 @@ impl Config { .with_connect_trusted_nodes_only(self.peers.connect_trusted_nodes_only); let discv4 = Discv4Config::builder().external_ip_resolver(Some(nat_resolution_method)).clone(); - NetworkConfig::builder(Arc::new(ProviderImpl::new(db)), rng_secret_key()) + NetworkConfigBuilder::new(rng_secret_key()) .boot_nodes(bootnodes.unwrap_or_else(mainnet_nodes)) .peer_config(peer_config) .discovery(discv4) .chain_spec(chain_spec) .set_discovery(disable_discovery) - .build() + .build(Arc::new(ProviderImpl::new(db))) } } diff --git a/crates/net/discv4/Cargo.toml b/crates/net/discv4/Cargo.toml index 75a02c90d..9d2e9ec01 100644 --- a/crates/net/discv4/Cargo.toml +++ b/crates/net/discv4/Cargo.toml @@ -24,7 +24,9 @@ secp256k1 = { version = "0.24", features = [ "rand-std", "recovery", ] } -enr = { version = "0.7.0", default-features = false, features = ["rust-secp256k1"] } +enr = { version = "0.7.0", default-features = false, features = [ + "rust-secp256k1", +] } # async/futures tokio = { version = "1", features = ["io-util", "net", "time"] } @@ -37,6 +39,7 @@ thiserror = "1.0" hex = "0.4" rand = { version = "0.8", optional = true } generic-array = "0.14" +serde = "1.0" [dev-dependencies] rand = "0.8" diff --git a/crates/net/discv4/src/config.rs b/crates/net/discv4/src/config.rs index d186a4e9e..510ec4900 100644 --- a/crates/net/discv4/src/config.rs +++ b/crates/net/discv4/src/config.rs @@ -8,13 +8,14 @@ use reth_net_common::ban_list::BanList; use reth_net_nat::{NatResolver, ResolveNatInterval}; use reth_primitives::NodeRecord; use reth_rlp::Encodable; +use secp256k1::serde::{Deserialize, Serialize}; use std::{ collections::{HashMap, HashSet}, time::Duration, }; /// Configuration parameters that define the performance of the discovery network. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct Discv4Config { /// Whether to enable the incoming packet filter. Default: false. pub enable_packet_filter: bool, @@ -38,6 +39,7 @@ pub struct Discv4Config { /// The duration we set for neighbours responses pub neighbours_expiration: Duration, /// Provides a way to ban peers and ips. + #[serde(skip)] pub ban_list: BanList, /// Set the default duration for which nodes are banned for. This timeouts are checked every 5 /// minutes, so the precision will be to the nearest 5 minutes. If set to `None`, bans from @@ -135,7 +137,7 @@ impl Default for Discv4Config { } /// Builder type for [`Discv4Config`] -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, Serialize, Deserialize)] pub struct Discv4ConfigBuilder { config: Discv4Config, } diff --git a/crates/net/dns/Cargo.toml b/crates/net/dns/Cargo.toml index fe3e60805..dfefa5ebf 100644 --- a/crates/net/dns/Cargo.toml +++ b/crates/net/dns/Cargo.toml @@ -37,7 +37,8 @@ lru = "0.9" thiserror = "1.0" tracing = "0.1" parking_lot = "0.12" - +serde = "1.0" +serde_with = "2.1.0" [dev-dependencies] tokio = { version = "1", features = ["sync", "rt", "rt-multi-thread"] } diff --git a/crates/net/dns/src/config.rs b/crates/net/dns/src/config.rs index 7d39a6856..26df6886f 100644 --- a/crates/net/dns/src/config.rs +++ b/crates/net/dns/src/config.rs @@ -1,8 +1,10 @@ +use serde::{Deserialize, Serialize}; + use crate::tree::LinkEntry; use std::{collections::HashSet, num::NonZeroUsize, time::Duration}; /// Settings for the [DnsDiscoveryClient](crate::DnsDiscoveryClient). -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Serialize, Deserialize)] pub struct DnsDiscoveryConfig { /// Timeout for DNS lookups. /// diff --git a/crates/net/dns/src/tree.rs b/crates/net/dns/src/tree.rs index 36e59c878..870060337 100644 --- a/crates/net/dns/src/tree.rs +++ b/crates/net/dns/src/tree.rs @@ -28,6 +28,7 @@ use data_encoding::{BASE32_NOPAD, BASE64URL_NOPAD}; use enr::{Enr, EnrError, EnrKey, EnrKeyUnambiguous, EnrPublicKey}; use reth_primitives::hex; use secp256k1::SecretKey; +use serde_with::{DeserializeFromStr, SerializeDisplay}; use std::{fmt, str::FromStr}; const ROOT_V1_PREFIX: &str = "enrtree-root:v1"; @@ -202,7 +203,7 @@ impl fmt::Display for BranchEntry { } /// A link entry -#[derive(Debug, Clone, Hash, Eq, PartialEq)] +#[derive(Debug, Clone, Hash, Eq, PartialEq, SerializeDisplay, DeserializeFromStr)] pub struct LinkEntry { pub domain: String, pub pubkey: K::PublicKey, diff --git a/crates/net/nat/Cargo.toml b/crates/net/nat/Cargo.toml index 23e3ba12f..b8bd11be5 100644 --- a/crates/net/nat/Cargo.toml +++ b/crates/net/nat/Cargo.toml @@ -23,6 +23,7 @@ tracing = "0.1" pin-project-lite = "0.2.9" tokio = { version = "1", features = ["time"] } thiserror = "1.0" +serde_with = "2.1.0" [dev-dependencies] reth-tracing = { path = "../../tracing" } diff --git a/crates/net/nat/src/lib.rs b/crates/net/nat/src/lib.rs index 68e2a3b6b..09640d4db 100644 --- a/crates/net/nat/src/lib.rs +++ b/crates/net/nat/src/lib.rs @@ -9,6 +9,7 @@ use igd::aio::search_gateway; use pin_project_lite::pin_project; +use serde_with::{DeserializeFromStr, SerializeDisplay}; use std::{ fmt, future::{poll_fn, Future}, @@ -21,7 +22,9 @@ use std::{ use tracing::warn; /// All builtin resolvers. -#[derive(Debug, Clone, Copy, Eq, PartialEq, Default, Hash)] +#[derive( + Debug, Clone, Copy, Eq, PartialEq, Default, Hash, SerializeDisplay, DeserializeFromStr, +)] pub enum NatResolver { /// Resolve with any available resolver. #[default] diff --git a/crates/net/network/Cargo.toml b/crates/net/network/Cargo.toml index ccbdb3c2b..25f613f7d 100644 --- a/crates/net/network/Cargo.toml +++ b/crates/net/network/Cargo.toml @@ -61,6 +61,7 @@ secp256k1 = { version = "0.24", features = [ "global-context", "rand-std", "recovery", + "serde", ] } enr = { version = "0.7.0", features = ["serde", "rust-secp256k1"], optional = true } diff --git a/crates/net/network/src/config.rs b/crates/net/network/src/config.rs index 0956d74d8..ce7fb5ae2 100644 --- a/crates/net/network/src/config.rs +++ b/crates/net/network/src/config.rs @@ -12,6 +12,7 @@ use reth_primitives::{ChainSpec, ForkFilter, NodeRecord, PeerId, MAINNET}; use reth_provider::{BlockProvider, HeaderProvider}; use reth_tasks::TaskExecutor; use secp256k1::{SecretKey, SECP256K1}; +use serde::{Deserialize, Serialize}; use std::{ collections::HashSet, net::{Ipv4Addr, SocketAddr, SocketAddrV4}, @@ -80,12 +81,12 @@ pub struct NetworkConfig { impl NetworkConfig { /// Create a new instance with all mandatory fields set, rest is field with defaults. pub fn new(client: Arc, secret_key: SecretKey) -> Self { - Self::builder(client, secret_key).build() + Self::builder(secret_key).build(client) } /// Convenience method for creating the corresponding builder type - pub fn builder(client: Arc, secret_key: SecretKey) -> NetworkConfigBuilder { - NetworkConfigBuilder::new(client, secret_key) + pub fn builder(secret_key: SecretKey) -> NetworkConfigBuilder { + NetworkConfigBuilder::new(secret_key) } /// Sets the config to use for the discovery v4 protocol. @@ -119,10 +120,9 @@ where } /// Builder for [`NetworkConfig`](struct.NetworkConfig.html). +#[derive(Debug, Serialize, Deserialize)] #[allow(missing_docs)] -pub struct NetworkConfigBuilder { - /// The client type that can interact with the chain. - client: Arc, +pub struct NetworkConfigBuilder { /// The node's secret key, from which the node's identity is derived. secret_key: SecretKey, /// How to configure discovery over DNS. @@ -141,11 +141,10 @@ pub struct NetworkConfigBuilder { sessions_config: Option, /// The network's chain spec chain_spec: ChainSpec, - /// The block importer type. - block_import: Box, /// The default mode of the network. network_mode: NetworkMode, /// The executor to use for spawning tasks. + #[serde(skip)] executor: Option, /// The `Status` message to send to peers at the beginning. status: Option, @@ -160,10 +159,9 @@ pub struct NetworkConfigBuilder { // === impl NetworkConfigBuilder === #[allow(missing_docs)] -impl NetworkConfigBuilder { - pub fn new(client: Arc, secret_key: SecretKey) -> Self { +impl NetworkConfigBuilder { + pub fn new(secret_key: SecretKey) -> Self { Self { - client, secret_key, dns_discovery_config: Some(Default::default()), discovery_v4_builder: Some(Default::default()), @@ -173,7 +171,6 @@ impl NetworkConfigBuilder { peers_config: None, sessions_config: None, chain_spec: MAINNET.clone(), - block_import: Box::::default(), network_mode: Default::default(), executor: None, status: None, @@ -245,12 +242,6 @@ impl NetworkConfigBuilder { self } - /// Sets the [`BlockImport`] type to configure. - pub fn block_import(mut self, block_import: T) -> Self { - self.block_import = Box::new(block_import); - self - } - /// Sets the socket address the network will listen on pub fn listener_addr(mut self, listener_addr: SocketAddr) -> Self { self.listener_addr = Some(listener_addr); @@ -295,10 +286,10 @@ impl NetworkConfigBuilder { } /// Consumes the type and creates the actual [`NetworkConfig`] - pub fn build(self) -> NetworkConfig { + /// for the given client type that can interact with the chain. + pub fn build(self, client: Arc) -> NetworkConfig { let peer_id = self.get_peer_id(); let Self { - client, secret_key, mut dns_discovery_config, discovery_v4_builder, @@ -308,7 +299,6 @@ impl NetworkConfigBuilder { peers_config, sessions_config, chain_spec, - block_import, network_mode, executor, status, @@ -355,7 +345,7 @@ impl NetworkConfigBuilder { peers_config: peers_config.unwrap_or_default(), sessions_config: sessions_config.unwrap_or_default(), chain_spec, - block_import, + block_import: Box::::default(), network_mode, executor, status: status.unwrap_or_default(), @@ -370,7 +360,7 @@ impl NetworkConfigBuilder { /// This affects block propagation in the `eth` sub-protocol [EIP-3675](https://eips.ethereum.org/EIPS/eip-3675#devp2p) /// /// In POS `NewBlockHashes` and `NewBlock` messages become invalid. -#[derive(Debug, Clone, Copy, Eq, PartialEq, Default)] +#[derive(Debug, Clone, Copy, Eq, PartialEq, Default, Serialize, Deserialize)] pub enum NetworkMode { /// Network is in proof-of-work mode. Work, @@ -396,14 +386,14 @@ mod tests { use reth_primitives::Chain; use reth_provider::test_utils::NoopProvider; - fn builder() -> NetworkConfigBuilder { + fn builder() -> NetworkConfigBuilder { let secret_key = SecretKey::new(&mut thread_rng()); - NetworkConfig::builder(Arc::new(NoopProvider::default()), secret_key) + NetworkConfigBuilder::new(secret_key) } #[test] fn test_network_dns_defaults() { - let config = builder().build(); + let config = builder().build(Arc::new(NoopProvider::default())); let dns = config.dns_discovery_config.unwrap(); let bootstrap_nodes = dns.bootstrap_dns_networks.unwrap(); diff --git a/crates/net/network/src/session/config.rs b/crates/net/network/src/session/config.rs index 6a5a1ba4d..aa23dbfd6 100644 --- a/crates/net/network/src/session/config.rs +++ b/crates/net/network/src/session/config.rs @@ -9,6 +9,8 @@ use std::time::Duration; pub const INITIAL_REQUEST_TIMEOUT: Duration = Duration::from_secs(20); /// Configuration options when creating a [SessionManager](crate::session::SessionManager). +#[derive(Debug)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct SessionsConfig { /// Size of the session command buffer (per session task). pub session_command_buffer: usize, @@ -55,6 +57,7 @@ impl SessionsConfig { /// /// By default, no session limits will be enforced #[derive(Debug, Clone, Default)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct SessionLimits { max_pending_inbound: Option, max_pending_outbound: Option, diff --git a/crates/net/network/src/test_utils/testnet.rs b/crates/net/network/src/test_utils/testnet.rs index 3ba3bd82b..462c77b41 100644 --- a/crates/net/network/src/test_utils/testnet.rs +++ b/crates/net/network/src/test_utils/testnet.rs @@ -1,8 +1,8 @@ //! A network implementation for testing purposes. use crate::{ - error::NetworkError, eth_requests::EthRequestHandler, NetworkConfig, NetworkEvent, - NetworkHandle, NetworkManager, + error::NetworkError, eth_requests::EthRequestHandler, NetworkConfig, NetworkConfigBuilder, + NetworkEvent, NetworkHandle, NetworkManager, }; use futures::{FutureExt, StreamExt}; use pin_project::pin_project; @@ -287,10 +287,10 @@ where /// Initialize the network with a given secret key, allowing devp2p and discovery to bind any /// available IP and port. pub fn with_secret_key(client: Arc, secret_key: SecretKey) -> Self { - let config = NetworkConfig::builder(Arc::clone(&client), secret_key) + let config = NetworkConfigBuilder::new(secret_key) .listener_addr(SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, 0))) .discovery_addr(SocketAddr::V4(SocketAddrV4::new(Ipv4Addr::UNSPECIFIED, 0))) - .build(); + .build(Arc::clone(&client)); Self { config, client, secret_key } } } diff --git a/crates/net/network/src/transactions.rs b/crates/net/network/src/transactions.rs index d9d69d847..2baeadd38 100644 --- a/crates/net/network/src/transactions.rs +++ b/crates/net/network/src/transactions.rs @@ -558,7 +558,7 @@ pub enum NetworkTransactionEvent { #[cfg(test)] mod tests { use super::*; - use crate::{NetworkConfig, NetworkManager}; + use crate::{NetworkConfigBuilder, NetworkManager}; use reth_interfaces::sync::{SyncState, SyncStateUpdater}; use reth_provider::test_utils::NoopProvider; use reth_transaction_pool::test_utils::testing_pool; @@ -572,7 +572,7 @@ mod tests { let client = Arc::new(NoopProvider::default()); let pool = testing_pool(); - let config = NetworkConfig::builder(Arc::clone(&client), secret_key).build(); + let config = NetworkConfigBuilder::new(secret_key).build(Arc::clone(&client)); let (handle, network, mut transactions, _) = NetworkManager::new(config) .await .unwrap() diff --git a/crates/net/network/tests/it/connect.rs b/crates/net/network/tests/it/connect.rs index 061469385..52c57a1c6 100644 --- a/crates/net/network/tests/it/connect.rs +++ b/crates/net/network/tests/it/connect.rs @@ -16,7 +16,7 @@ use reth_network::{ test_utils::{ enr_to_peer_id, unused_tcp_udp, NetworkEventStream, PeerConfig, Testnet, GETH_TIMEOUT, }, - NetworkConfig, NetworkEvent, NetworkManager, PeersConfig, + NetworkConfigBuilder, NetworkEvent, NetworkManager, PeersConfig, }; use reth_network_api::{NetworkInfo, PeersInfo}; use reth_primitives::{HeadersDirection, NodeRecord, PeerId}; @@ -207,9 +207,9 @@ async fn test_connect_with_boot_nodes() { let mut discv4 = Discv4Config::builder(); discv4.add_boot_nodes(mainnet_nodes()); - let config = NetworkConfig::builder(Arc::new(NoopProvider::default()), secret_key) + let config = NetworkConfigBuilder::new(secret_key) .discovery(discv4) - .build(); + .build(Arc::new(NoopProvider::default())); let network = NetworkManager::new(config).await.unwrap(); let handle = network.handle().clone(); @@ -230,7 +230,7 @@ async fn test_connect_with_builder() { discv4.add_boot_nodes(mainnet_nodes()); let client = Arc::new(NoopProvider::default()); - let config = NetworkConfig::builder(Arc::clone(&client), secret_key).discovery(discv4).build(); + let config = NetworkConfigBuilder::new(secret_key).discovery(discv4).build(Arc::clone(&client)); let (handle, network, _, requests) = NetworkManager::new(config) .await .unwrap() @@ -266,7 +266,7 @@ async fn test_connect_to_trusted_peer() { let discv4 = Discv4Config::builder(); let client = Arc::new(NoopProvider::default()); - let config = NetworkConfig::builder(Arc::clone(&client), secret_key).discovery(discv4).build(); + let config = NetworkConfigBuilder::new(secret_key).discovery(discv4).build(Arc::clone(&client)); let (handle, network, transactions, requests) = NetworkManager::new(config) .await .unwrap() @@ -331,11 +331,11 @@ async fn test_incoming_node_id_blacklist() { let peer_config = PeersConfig::default().with_ban_list(ban_list); let (reth_p2p, reth_disc) = unused_tcp_udp(); - let config = NetworkConfig::builder(Arc::new(NoopProvider::default()), secret_key) + let config = NetworkConfigBuilder::new(secret_key) .listener_addr(reth_p2p) .discovery_addr(reth_disc) .peer_config(peer_config) - .build(); + .build(Arc::new(NoopProvider::default())); let network = NetworkManager::new(config).await.unwrap(); @@ -380,10 +380,10 @@ async fn test_incoming_connect_with_single_geth() { let geth_peer_id = enr_to_peer_id(provider.node_info().await.unwrap().enr); let (reth_p2p, reth_disc) = unused_tcp_udp(); - let config = NetworkConfig::builder(Arc::new(NoopProvider::default()), secret_key) + let config = NetworkConfigBuilder::new(secret_key) .listener_addr(reth_p2p) .discovery_addr(reth_disc) - .build(); + .build(Arc::new(NoopProvider::default())); let network = NetworkManager::new(config).await.unwrap(); @@ -414,10 +414,10 @@ async fn test_outgoing_connect_with_single_geth() { let secret_key = SecretKey::new(&mut rand::thread_rng()); let (reth_p2p, reth_disc) = unused_tcp_udp(); - let config = NetworkConfig::builder(Arc::new(NoopProvider::default()), secret_key) + let config = NetworkConfigBuilder::new(secret_key) .listener_addr(reth_p2p) .discovery_addr(reth_disc) - .build(); + .build(Arc::new(NoopProvider::default())); let network = NetworkManager::new(config).await.unwrap(); let handle = network.handle().clone(); @@ -459,10 +459,10 @@ async fn test_geth_disconnect() { let secret_key = SecretKey::new(&mut rand::thread_rng()); let (reth_p2p, reth_disc) = unused_tcp_udp(); - let config = NetworkConfig::builder(Arc::new(NoopProvider::default()), secret_key) + let config = NetworkConfigBuilder::new(secret_key) .listener_addr(reth_p2p) .discovery_addr(reth_disc) - .build(); + .build(Arc::new(NoopProvider::default())); let network = NetworkManager::new(config).await.unwrap(); let handle = network.handle().clone(); diff --git a/crates/primitives/src/forkid.rs b/crates/primitives/src/forkid.rs index 97ab5590e..94dbee840 100644 --- a/crates/primitives/src/forkid.rs +++ b/crates/primitives/src/forkid.rs @@ -106,7 +106,7 @@ pub enum ValidationError { /// Filter that describes the state of blockchain and can be used to check incoming `ForkId`s for /// compatibility. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub struct ForkFilter { forks: BTreeMap, @@ -256,7 +256,7 @@ pub struct ForkTransition { pub past: ForkId, } -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] struct Cache { // An epoch is a period between forks. // When we progress from one fork to the next one we move to the next epoch.