From ea36b58cb20c6257bb7c7289620b1ea1568def6a Mon Sep 17 00:00:00 2001 From: Thomas Coratger <60488569+tcoratger@users.noreply.github.com> Date: Mon, 22 Jan 2024 11:26:47 +0100 Subject: [PATCH] add missing documentation and minor refactor (#6152) --- .../consensus/auto-seal/tests/it/auto_mine.rs | 3 +- crates/consensus/beacon/src/engine/message.rs | 1 - crates/net/discv4/src/proto.rs | 39 +++++++++++-- crates/net/discv4/src/test_utils.rs | 47 +++++++++++++--- crates/net/dns/src/tree.rs | 56 ++++++++++--------- 5 files changed, 104 insertions(+), 42 deletions(-) diff --git a/crates/consensus/auto-seal/tests/it/auto_mine.rs b/crates/consensus/auto-seal/tests/it/auto_mine.rs index 6b4bf1544..36c9213ef 100644 --- a/crates/consensus/auto-seal/tests/it/auto_mine.rs +++ b/crates/consensus/auto-seal/tests/it/auto_mine.rs @@ -1,4 +1,3 @@ -#![allow(unreachable_pub)] //! auto-mine consensus integration test use clap::Parser; @@ -58,7 +57,7 @@ impl RethNodeCommandConfig for AutoMineConfig { /// process transactions. #[test] #[cfg_attr(feature = "optimism", ignore)] -pub fn test_auto_mine() { +pub(crate) fn test_auto_mine() { // create temp path for test let temp_path = tempfile::TempDir::new().expect("tempdir is okay").into_path(); let datadir = temp_path.to_str().expect("temp path is okay"); diff --git a/crates/consensus/beacon/src/engine/message.rs b/crates/consensus/beacon/src/engine/message.rs index 16f43ea16..5e1826722 100644 --- a/crates/consensus/beacon/src/engine/message.rs +++ b/crates/consensus/beacon/src/engine/message.rs @@ -141,7 +141,6 @@ impl Future for PendingPayloadId { /// A message for the beacon engine from other components of the node (engine RPC API invoked by the /// consensus layer). #[derive(Debug)] -#[allow(clippy::large_enum_variant)] pub enum BeaconEngineMessage { /// Message with new payload. NewPayload { diff --git a/crates/net/discv4/src/proto.rs b/crates/net/discv4/src/proto.rs index 1716cca03..9f00749a7 100644 --- a/crates/net/discv4/src/proto.rs +++ b/crates/net/discv4/src/proto.rs @@ -1,7 +1,5 @@ //! Discovery v4 protocol implementation. -#![allow(missing_docs)] - use crate::{error::DecodePacketError, EnrForkIdEntry, PeerId, MAX_PACKET_SIZE, MIN_PACKET_SIZE}; use alloy_rlp::{ length_of_length, Decodable, Encodable, Error as RlpError, Header, RlpDecodable, RlpEncodable, @@ -19,15 +17,23 @@ use std::net::IpAddr; // Note: this is adapted from https://github.com/vorot93/discv4 -/// Id for message variants. +/// Represents the identifier for message variants. +/// +/// This enumeration assigns unique identifiers (u8 values) to different message types. #[derive(Debug)] #[repr(u8)] pub enum MessageId { + /// Ping message identifier. Ping = 1, + /// Pong message identifier. Pong = 2, + /// Find node message identifier. FindNode = 3, + /// Neighbours message identifier. Neighbours = 4, + /// ENR request message identifier. EnrRequest = 5, + /// ENR response message identifier. EnrResponse = 6, } @@ -173,11 +179,16 @@ impl Message { } } -/// Decoded packet +/// Represents a decoded packet. +/// +/// This struct holds information about a decoded packet, including the message, node ID, and hash. #[derive(Debug)] pub struct Packet { + /// The decoded message from the packet. pub msg: Message, + /// The ID of the peer that sent the packet. pub node_id: PeerId, + /// The hash of the packet. pub hash: B256, } @@ -231,6 +242,7 @@ pub struct Neighbours { pub struct EnrWrapper(Enr); impl EnrWrapper { + /// Creates a new instance of [`EnrWrapper`]. pub fn new(enr: Enr) -> Self { EnrWrapper(enr) } @@ -302,15 +314,24 @@ impl Decodable for EnrWrapper { } /// A [ENRRequest packet](https://github.com/ethereum/devp2p/blob/master/discv4.md#enrrequest-packet-0x05). +/// +/// This packet is used to request the current version of a node's Ethereum Node Record (ENR). #[derive(Clone, Copy, Debug, Eq, PartialEq, RlpEncodable, RlpDecodable)] pub struct EnrRequest { + /// The expiration timestamp for the request. No reply should be sent if it refers to a time in + /// the past. pub expire: u64, } /// A [ENRResponse packet](https://github.com/ethereum/devp2p/blob/master/discv4.md#enrresponse-packet-0x06). +/// +/// This packet is used to respond to an ENRRequest packet and includes the requested ENR along with +/// the hash of the original request. #[derive(Clone, Debug, Eq, PartialEq, RlpEncodable)] pub struct EnrResponse { + /// The hash of the ENRRequest packet being replied to. pub request_hash: B256, + /// The ENR (Ethereum Node Record) for the responding node. pub enr: EnrWrapper, } @@ -352,11 +373,16 @@ impl Decodable for EnrResponse { } } +/// Represents a Ping packet. +/// /// A [Ping packet](https://github.com/ethereum/devp2p/blob/master/discv4.md#ping-packet-0x01). #[derive(Debug, Clone, Eq, PartialEq)] pub struct Ping { + /// The sender's endpoint. pub from: NodeEndpoint, + /// The recipient's endpoint. pub to: NodeEndpoint, + /// The expiration timestamp. pub expire: u64, /// Optional enr_seq for pub enr_sq: Option, @@ -440,11 +466,16 @@ impl Decodable for Ping { } } +/// Represents a Pong packet. +/// /// A [Pong packet](https://github.com/ethereum/devp2p/blob/master/discv4.md#pong-packet-0x02). #[derive(Clone, Debug, Eq, PartialEq)] pub struct Pong { + /// The recipient's endpoint. pub to: NodeEndpoint, + /// The hash of the corresponding ping packet. pub echo: B256, + /// The expiration timestamp. pub expire: u64, /// Optional enr_seq for pub enr_sq: Option, diff --git a/crates/net/discv4/src/test_utils.rs b/crates/net/discv4/src/test_utils.rs index a1b05d9d2..c56a75a18 100644 --- a/crates/net/discv4/src/test_utils.rs +++ b/crates/net/discv4/src/test_utils.rs @@ -1,7 +1,5 @@ //! Mock discovery support -#![allow(missing_docs)] - use crate::{ proto::{FindNode, Message, Neighbours, NodeEndpoint, Packet, Ping, Pong}, receive_loop, send_loop, Discv4, Discv4Config, Discv4Service, EgressSender, IngressEvent, @@ -104,10 +102,12 @@ impl MockDiscovery { self.pending_neighbours.insert(target, nodes); } + /// Returns the local socket address associated with the service. pub fn local_addr(&self) -> SocketAddr { self.local_addr } + /// Returns the local [`NodeRecord`] associated with the service. pub fn local_enr(&self) -> NodeRecord { self.local_enr } @@ -190,18 +190,44 @@ impl Stream for MockDiscovery { } } -/// The event type the mock service produces +/// Represents the event types produced by the mock service. #[derive(Debug)] pub enum MockEvent { - Pong { ping: Ping, pong: Pong, to: SocketAddr }, - Neighbours { nodes: Vec, to: SocketAddr }, + /// A Pong event, consisting of the original Ping packet, the corresponding Pong packet, + /// and the recipient's socket address. + Pong { + /// The original Ping packet. + ping: Ping, + /// The corresponding Pong packet. + pong: Pong, + /// The recipient's socket address. + to: SocketAddr, + }, + /// A Neighbours event, containing a list of node records and the recipient's socket address. + Neighbours { + /// The list of node records. + nodes: Vec, + /// The recipient's socket address. + to: SocketAddr, + }, } -/// Command for interacting with the `MockDiscovery` service +/// Represents commands for interacting with the `MockDiscovery` service. #[derive(Debug)] pub enum MockCommand { - MockPong { node_id: PeerId }, - MockNeighbours { target: PeerId, nodes: Vec }, + /// A command to simulate a Pong event, including the node ID of the recipient. + MockPong { + /// The node ID of the recipient. + node_id: PeerId, + }, + /// A command to simulate a Neighbours event, including the target node ID and a list of node + /// records. + MockNeighbours { + /// The target node ID. + target: PeerId, + /// The list of node records. + nodes: Vec, + }, } /// Creates a new testing instance for [`Discv4`] and its service @@ -221,6 +247,7 @@ pub async fn create_discv4_with_config(config: Discv4Config) -> (Discv4, Discv4S Discv4::bind(socket, local_enr, secret_key, config).await.unwrap() } +/// Generates a random [`NodeEndpoint`] using the provided random number generator. pub fn rng_endpoint(rng: &mut impl Rng) -> NodeEndpoint { let address = if rng.gen() { let mut ip = [0u8; 4]; @@ -234,11 +261,13 @@ pub fn rng_endpoint(rng: &mut impl Rng) -> NodeEndpoint { NodeEndpoint { address, tcp_port: rng.gen(), udp_port: rng.gen() } } +/// Generates a random [`NodeRecord`] using the provided random number generator. pub fn rng_record(rng: &mut impl RngCore) -> NodeRecord { let NodeEndpoint { address, udp_port, tcp_port } = rng_endpoint(rng); NodeRecord { address, tcp_port, udp_port, id: rng.gen() } } +/// Generates a random IPv6 [`NodeRecord`] using the provided random number generator. pub fn rng_ipv6_record(rng: &mut impl RngCore) -> NodeRecord { let mut ip = [0u8; 16]; rng.fill_bytes(&mut ip); @@ -246,6 +275,7 @@ pub fn rng_ipv6_record(rng: &mut impl RngCore) -> NodeRecord { NodeRecord { address, tcp_port: rng.gen(), udp_port: rng.gen(), id: rng.gen() } } +/// Generates a random IPv4 [`NodeRecord`] using the provided random number generator. pub fn rng_ipv4_record(rng: &mut impl RngCore) -> NodeRecord { let mut ip = [0u8; 4]; rng.fill_bytes(&mut ip); @@ -253,6 +283,7 @@ pub fn rng_ipv4_record(rng: &mut impl RngCore) -> NodeRecord { NodeRecord { address, tcp_port: rng.gen(), udp_port: rng.gen(), id: rng.gen() } } +/// Generates a random [`Message`] using the provided random number generator. pub fn rng_message(rng: &mut impl RngCore) -> Message { match rng.gen_range(1..=4) { 1 => Message::Ping(Ping { diff --git a/crates/net/dns/src/tree.rs b/crates/net/dns/src/tree.rs index b1b2c263b..cc9efeb7d 100644 --- a/crates/net/dns/src/tree.rs +++ b/crates/net/dns/src/tree.rs @@ -16,8 +16,6 @@ //! `signature` is a 65-byte secp256k1 EC signature over the keccak256 hash of the record //! content, excluding the sig= part, encoded as URL-safe base64 (RFC-4648). -#![allow(missing_docs)] - use crate::error::{ ParseDnsEntryError, ParseDnsEntryError::{FieldNotFound, UnknownEntry}, @@ -27,27 +25,33 @@ use data_encoding::{BASE32_NOPAD, BASE64URL_NOPAD}; use enr::{Enr, EnrError, EnrKey, EnrKeyUnambiguous, EnrPublicKey}; use reth_primitives::{hex, Bytes}; use secp256k1::SecretKey; +#[cfg(feature = "serde")] +use serde_with::{DeserializeFromStr, SerializeDisplay}; use std::{ fmt, hash::{Hash, Hasher}, str::FromStr, }; -#[cfg(feature = "serde")] -use serde_with::{DeserializeFromStr, SerializeDisplay}; - +/// Prefix used for root entries in the ENR tree. const ROOT_V1_PREFIX: &str = "enrtree-root:v1"; +/// Prefix used for link entries in the ENR tree. const LINK_PREFIX: &str = "enrtree://"; +/// Prefix used for branch entries in the ENR tree. const BRANCH_PREFIX: &str = "enrtree-branch:"; +/// Prefix used for ENR entries in the ENR tree. const ENR_PREFIX: &str = "enr:"; -/// Represents all variants +/// Represents all variants of DNS entries for Ethereum node lists. #[derive(Debug, Clone)] -#[allow(missing_docs)] pub enum DnsEntry { + /// Represents a root entry in the DNS tree containing node records. Root(TreeRootEntry), + /// Represents a link entry in the DNS tree pointing to another node list. Link(LinkEntry), + /// Represents a branch entry in the DNS tree containing hashes of subtree entries. Branch(BranchEntry), + /// Represents a leaf entry in the DNS tree containing a node record. Node(NodeEntry), } @@ -83,9 +87,13 @@ impl FromStr for DnsEntry { /// Represents an `enr-root` hash of subtrees containing nodes and links. #[derive(Clone, Eq, PartialEq)] pub struct TreeRootEntry { + /// The `enr-root` hash. pub enr_root: String, + /// The root hash of the links. pub link_root: String, + /// The sequence number associated with the entry. pub sequence_number: u64, + /// The signature of the entry. pub signature: Bytes, } @@ -171,9 +179,10 @@ impl fmt::Display for TreeRootEntry { } } -/// A branch entry with base32 hashes +/// Represents a branch entry in the DNS tree, containing base32 hashes of subtree entries. #[derive(Debug, Clone)] pub struct BranchEntry { + /// The list of base32-encoded hashes of subtree entries in the branch. pub children: Vec, } @@ -211,11 +220,8 @@ impl FromStr for BranchEntry { type Err = ParseDnsEntryError; fn from_str(s: &str) -> Result { - if let Some(s) = s.strip_prefix(BRANCH_PREFIX) { - Self::parse_value(s) - } else { - Err(UnknownEntry(s.to_string())) - } + s.strip_prefix(BRANCH_PREFIX) + .map_or_else(|| Err(UnknownEntry(s.to_string())), Self::parse_value) } } @@ -225,11 +231,13 @@ impl fmt::Display for BranchEntry { } } -/// A link entry +/// Represents a link entry in the DNS tree, facilitating federation and web-of-trust functionality. #[derive(Debug, Clone)] #[cfg_attr(feature = "serde", derive(SerializeDisplay, DeserializeFromStr))] pub struct LinkEntry { + /// The domain associated with the link entry. pub domain: String, + /// The public key corresponding to the Ethereum Node Record (ENR) used for the link entry. pub pubkey: K::PublicKey, } @@ -283,11 +291,8 @@ impl FromStr for LinkEntry { type Err = ParseDnsEntryError; fn from_str(s: &str) -> Result { - if let Some(s) = s.strip_prefix(LINK_PREFIX) { - Self::parse_value(s) - } else { - Err(UnknownEntry(s.to_string())) - } + s.strip_prefix(LINK_PREFIX) + .map_or_else(|| Err(UnknownEntry(s.to_string())), Self::parse_value) } } @@ -303,9 +308,10 @@ impl fmt::Display for LinkEntry { } } -/// The actual [Enr] entry. +/// Represents the actual Ethereum Node Record (ENR) entry in the DNS tree. #[derive(Debug, Clone)] pub struct NodeEntry { + /// The Ethereum Node Record (ENR) associated with the node entry. pub enr: Enr, } @@ -316,8 +322,7 @@ impl NodeEntry { /// /// Caution: This assumes the prefix is already removed. fn parse_value(s: &str) -> ParseEntryResult { - let enr: Enr = s.parse().map_err(ParseDnsEntryError::Other)?; - Ok(Self { enr }) + Ok(Self { enr: s.parse().map_err(ParseDnsEntryError::Other)? }) } } @@ -325,11 +330,8 @@ impl FromStr for NodeEntry { type Err = ParseDnsEntryError; fn from_str(s: &str) -> Result { - if let Some(s) = s.strip_prefix(ENR_PREFIX) { - Self::parse_value(s) - } else { - Err(UnknownEntry(s.to_string())) - } + s.strip_prefix(ENR_PREFIX) + .map_or_else(|| Err(UnknownEntry(s.to_string())), Self::parse_value) } }