From 33f0d971e028d70fbaf8015999b7576e8a0acc31 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Tue, 22 Sep 2020 11:38:31 +1000 Subject: [PATCH] Perform a self-review --- book/src/advanced_metrics.md | 4 +-- book/src/api-lighthouse.md | 3 +- common/eth2/src/lib.rs | 20 +++++++++++ common/eth2/src/lighthouse.rs | 2 ++ common/eth2/src/types.rs | 12 +++---- common/warp_utils/src/lib.rs | 3 ++ common/warp_utils/src/reject.rs | 4 +-- consensus/serde_utils/src/bytes_4_hex.rs | 4 +++ consensus/serde_utils/src/hex.rs | 4 +++ consensus/serde_utils/src/quoted.rs | 39 --------------------- consensus/serde_utils/src/quoted_int.rs | 6 ++++ consensus/serde_utils/src/quoted_u64_vec.rs | 6 ++++ consensus/serde_utils/src/u32_hex.rs | 4 +++ consensus/serde_utils/src/u8_hex.rs | 4 +++ consensus/types/src/graffiti.rs | 1 + validator_client/src/validator_duty.rs | 7 +++- 16 files changed, 69 insertions(+), 54 deletions(-) delete mode 100644 consensus/serde_utils/src/quoted.rs diff --git a/book/src/advanced_metrics.md b/book/src/advanced_metrics.md index 502e5ceb1f9..6c901862ee0 100644 --- a/book/src/advanced_metrics.md +++ b/book/src/advanced_metrics.md @@ -11,8 +11,8 @@ Grafana dashboard. These components are available in a docker-compose format at ## Beacon Node Metrics By default, these metrics are disabled but can be enabled with the `--metrics` -flag. Use the `--metrics-address` and `--metrics-port` flags to customize the -listening socket of the metrics server. +flag. Use the `--metrics-address`, `--metrics-port` and +`--metrics-allow-origin` flags to customize the metrics server. ### Example diff --git a/book/src/api-lighthouse.md b/book/src/api-lighthouse.md index 0f8dd593041..3f37673fa9d 100644 --- a/book/src/api-lighthouse.md +++ b/book/src/api-lighthouse.md @@ -1,5 +1,4 @@ -# Lighthouse APIs - +# Lighthouse Non-Standard APIs Lighthouse fully supports the standardization efforts at [github.com/ethereum/eth2.0-APIs](https://github.com/ethereum/eth2.0-APIs), diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 453326d7448..f5203b9a986 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1,3 +1,12 @@ +//! This crate provides two major things: +//! +//! 1. The types served by the `http_api` crate. +//! 2. A wrapper around `reqwest` that forms a HTTP client, able of consuming the endpoints served +//! by the `http_api` crate. +//! +//! Eventually it would be ideal to publish this crate on crates.io, however we have some local +//! dependencies preventing this presently. + #[cfg(feature = "lighthouse")] pub mod lighthouse; pub mod types; @@ -13,13 +22,18 @@ pub use reqwest::{StatusCode, Url}; #[derive(Debug)] pub enum Error { + /// The `reqwest` client raised an error. Reqwest(reqwest::Error), + /// The server returned an error message where the body was able to be parsed. ServerMessage(ErrorMessage), + /// The server returned an error message where the body was unable to be parsed. StatusCode(StatusCode), + /// The supplied URL is badly formatted. It should look something like `http://127.0.0.1:5052`. InvalidUrl(Url), } impl Error { + /// If the error has a HTTP status code, return it. pub fn status(&self) -> Option { match self { Error::Reqwest(error) => error.status(), @@ -36,6 +50,8 @@ impl fmt::Display for Error { } } +/// A wrapper around `reqwest::Client` which provides convenience methods for interfacing with a +/// Lighthouse Beacon Node HTTP server (`http_api`). #[derive(Clone)] pub struct BeaconNodeClient { client: reqwest::Client, @@ -56,6 +72,7 @@ impl BeaconNodeClient { Self { client, server } } + /// Return the path with the standard `/eth1/v1` prefix applied. fn eth_path(&self) -> Result { let mut path = self.server.clone(); @@ -67,6 +84,7 @@ impl BeaconNodeClient { Ok(path) } + /// Perform a HTTP GET request. async fn get(&self, url: U) -> Result { let response = self.client.get(url).send().await.map_err(Error::Reqwest)?; ok_or_error(response) @@ -76,6 +94,7 @@ impl BeaconNodeClient { .map_err(Error::Reqwest) } + /// Perform a HTTP GET request, returning `None` on a 404 error. async fn get_opt(&self, url: U) -> Result, Error> { let response = self.client.get(url).send().await.map_err(Error::Reqwest)?; match ok_or_error(response).await { @@ -90,6 +109,7 @@ impl BeaconNodeClient { } } + /// Perform a HTTP POST request. async fn post(&self, url: U, body: &T) -> Result<(), Error> { let response = self .client diff --git a/common/eth2/src/lighthouse.rs b/common/eth2/src/lighthouse.rs index b0fa85101a0..f9cbfe0bcdb 100644 --- a/common/eth2/src/lighthouse.rs +++ b/common/eth2/src/lighthouse.rs @@ -1,3 +1,5 @@ +//! This module contains endpoints that are non-standard and only available on Lighthouse servers. + use crate::{ types::{Epoch, EthSpec, GenericResponse, ValidatorId}, BeaconNodeClient, Error, diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 28c3b2d0cbe..9c0cf68bde2 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1,16 +1,12 @@ +//! This module exposes a superset of the `types` crate. It adds additional types that are only +//! required for the HTTP API. + use eth2_libp2p::{Enr, Multiaddr}; use serde::{Deserialize, Serialize}; use std::convert::TryFrom; use std::fmt; use std::str::FromStr; -use types::serde_utils; - -pub use types::{ - Address, Attestation, AttestationData, AttesterSlashing, BeaconBlock, BeaconBlockHeader, - BeaconState, Checkpoint, CommitteeIndex, Epoch, EthSpec, Fork, Graffiti, Hash256, - ProposerSlashing, PublicKeyBytes, SignatureBytes, SignedAggregateAndProof, SignedBeaconBlock, - SignedVoluntaryExit, Slot, Validator, ValidatorSubscription, YamlConfig, -}; +use types::*; /// An API error serializable to JSON. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/common/warp_utils/src/lib.rs b/common/warp_utils/src/lib.rs index 22ada7426aa..ec9cf3c3442 100644 --- a/common/warp_utils/src/lib.rs +++ b/common/warp_utils/src/lib.rs @@ -1,2 +1,5 @@ +//! This crate contains functions that are common across multiple `warp` HTTP servers in the +//! Lighthouse project. E.g., the `http_api` and `http_metrics` crates. + pub mod reject; pub mod reply; diff --git a/common/warp_utils/src/reject.rs b/common/warp_utils/src/reject.rs index a99c3ddd494..a48ad56a855 100644 --- a/common/warp_utils/src/reject.rs +++ b/common/warp_utils/src/reject.rs @@ -83,8 +83,8 @@ pub fn object_invalid(msg: String) -> warp::reject::Rejection { warp::reject::custom(ObjectInvalid(msg)) } -// This function receives a `Rejection` and tries to return a custom -// value, otherwise simply passes the rejection along. +/// This function receives a `Rejection` and tries to return a custom +/// value, otherwise simply passes the rejection along. pub async fn handle_rejection(err: warp::Rejection) -> Result { let code; let message; diff --git a/consensus/serde_utils/src/bytes_4_hex.rs b/consensus/serde_utils/src/bytes_4_hex.rs index 03da1c80c15..8e5ab915534 100644 --- a/consensus/serde_utils/src/bytes_4_hex.rs +++ b/consensus/serde_utils/src/bytes_4_hex.rs @@ -1,3 +1,7 @@ +//! Formats `[u8; 4]` as a 0x-prefixed hex string. +//! +//! E.g., `[0, 1, 2, 3]` serializes as `"0x00010203"`. + use serde::de::Error; use serde::{Deserialize, Deserializer, Serializer}; diff --git a/consensus/serde_utils/src/hex.rs b/consensus/serde_utils/src/hex.rs index 374583420a5..79dfaa506b8 100644 --- a/consensus/serde_utils/src/hex.rs +++ b/consensus/serde_utils/src/hex.rs @@ -1,6 +1,9 @@ +//! Provides utilities for parsing 0x-prefixed hex strings. + use serde::de::{self, Visitor}; use std::fmt; +/// Encode `data` as a 0x-prefixed hex string. pub fn encode>(data: T) -> String { let hex = hex::encode(data); let mut s = "0x".to_string(); @@ -8,6 +11,7 @@ pub fn encode>(data: T) -> String { s } +/// Decode `data` from a 0x-prefixed hex string. pub fn decode(s: &str) -> Result, String> { if s.starts_with("0x") { hex::decode(&s[2..]).map_err(|e| format!("invalid hex: {:?}", e)) diff --git a/consensus/serde_utils/src/quoted.rs b/consensus/serde_utils/src/quoted.rs deleted file mode 100644 index 67f14af6e88..00000000000 --- a/consensus/serde_utils/src/quoted.rs +++ /dev/null @@ -1,39 +0,0 @@ -use serde::{Deserializer, Serializer}; - -pub struct QuotedIntVisitor; -impl<'a> serde::de::Visitor<'a> for QuotedIntVisitor { - type Value = u64; - - fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { - write!( - formatter, - "a string containing digits or an int fitting into u64" - ) - } - - fn visit_str(self, s: &str) -> Result - where - E: serde::de::Error, - { - let s = if s.len() > 2 && s.starts_with("\"") && s.ends_with("\"") { - &s[1..s.len() - 1] - } else { - s - }; - s.parse().map_err(serde::de::Error::custom) - } -} - -pub fn serialize(value: &u64, serializer: S) -> Result -where - S: Serializer, -{ - serializer.serialize_str(&format!("{}", value)) -} - -pub fn deserialize<'de, D>(deserializer: D) -> Result -where - D: Deserializer<'de>, -{ - deserializer.deserialize_any(QuotedIntVisitor) -} diff --git a/consensus/serde_utils/src/quoted_int.rs b/consensus/serde_utils/src/quoted_int.rs index 9fd86c408b5..24edf1ebee2 100644 --- a/consensus/serde_utils/src/quoted_int.rs +++ b/consensus/serde_utils/src/quoted_int.rs @@ -1,3 +1,9 @@ +//! Formats some integer types using quotes. +//! +//! E.g., `1` serializes as `"1"`. +//! +//! Quotes can be optional during decoding. + use serde::{Deserializer, Serializer}; use serde_derive::{Deserialize, Serialize}; use std::convert::TryFrom; diff --git a/consensus/serde_utils/src/quoted_u64_vec.rs b/consensus/serde_utils/src/quoted_u64_vec.rs index 370155e53ad..f124c989092 100644 --- a/consensus/serde_utils/src/quoted_u64_vec.rs +++ b/consensus/serde_utils/src/quoted_u64_vec.rs @@ -1,3 +1,9 @@ +//! Formats `Vec` using quotes. +//! +//! E.g., `vec![0, 1, 2]` serializes as `["0", "1", "2"]`. +//! +//! Quotes can be optional during decoding. + use serde::ser::SerializeSeq; use serde::{Deserializer, Serializer}; use serde_derive::{Deserialize, Serialize}; diff --git a/consensus/serde_utils/src/u32_hex.rs b/consensus/serde_utils/src/u32_hex.rs index c5305c5e8cd..d39732ac903 100644 --- a/consensus/serde_utils/src/u32_hex.rs +++ b/consensus/serde_utils/src/u32_hex.rs @@ -1,3 +1,7 @@ +//! Formats `u32` as a 0x-prefixed, little-endian hex string. +//! +//! E.g., `0` serializes as `"0x00000000"`. + use serde::de::Error; use serde::{Deserialize, Deserializer, Serializer}; diff --git a/consensus/serde_utils/src/u8_hex.rs b/consensus/serde_utils/src/u8_hex.rs index aadb8a89c6f..7f8635579f4 100644 --- a/consensus/serde_utils/src/u8_hex.rs +++ b/consensus/serde_utils/src/u8_hex.rs @@ -1,3 +1,7 @@ +//! Formats `u8` as a 0x-prefixed hex string. +//! +//! E.g., `0` serializes as `"0x00"`. + use serde::de::Error; use serde::{Deserialize, Deserializer, Serializer}; diff --git a/consensus/types/src/graffiti.rs b/consensus/types/src/graffiti.rs index 5db5314090b..f35df93838b 100644 --- a/consensus/types/src/graffiti.rs +++ b/consensus/types/src/graffiti.rs @@ -10,6 +10,7 @@ use tree_hash::TreeHash; pub const GRAFFITI_BYTES_LEN: usize = 32; +/// The 32-byte `graffiti` field on a beacon block. #[derive(Default, Debug, PartialEq, Clone, Copy, Serialize, Deserialize)] #[serde(transparent)] #[cfg_attr(feature = "arbitrary-fuzz", derive(arbitrary::Arbitrary))] diff --git a/validator_client/src/validator_duty.rs b/validator_client/src/validator_duty.rs index 81cf6c49cbe..e83b6a84d09 100644 --- a/validator_client/src/validator_duty.rs +++ b/validator_client/src/validator_duty.rs @@ -7,7 +7,7 @@ use types::{CommitteeIndex, Epoch, PublicKey, PublicKeyBytes, Slot}; /// This struct is being used as a shim since we deprecated the `rest_api` in favour of `http_api`. /// -/// TODO: add an issue about this. +/// Tracking issue: https://github.com/sigp/lighthouse/issues/1643 // NOTE: if you add or remove fields, please adjust `eq_ignoring_proposal_slots` #[derive(PartialEq, Debug, Serialize, Deserialize, Clone)] pub struct ValidatorDuty { @@ -32,6 +32,7 @@ pub struct ValidatorDuty { } impl ValidatorDuty { + /// Instantiate `Self` as if there are no known dutes for `validator_pubkey`. fn no_duties(validator_pubkey: PublicKey) -> Self { ValidatorDuty { validator_pubkey, @@ -45,6 +46,9 @@ impl ValidatorDuty { } } + /// Instantiate `Self` by performing requests on the `beacon_node`. + /// + /// Will only request proposer duties if `current_epoch == request_epoch`. pub async fn download( beacon_node: &BeaconNodeClient, current_epoch: Epoch, @@ -113,6 +117,7 @@ impl ValidatorDuty { && self.committee_count_at_slot == other.committee_count_at_slot } + /// Generate a subscription for `self`, if `self` has appropriate attestation duties. pub fn subscription(&self, is_aggregator: bool) -> Option { Some(BeaconCommitteeSubscription { validator_index: self.validator_index?,