From e16437108366225e9c18b184ba1c2a1e83d2e38e Mon Sep 17 00:00:00 2001 From: pscott Date: Tue, 14 Jul 2020 08:05:02 +0000 Subject: [PATCH] Set Graffiti via CLI (#1320) ## Issue Addressed Closes #1319 ## Proposed Changes This issue: 1. Allows users to edit their Graffiti via the cli option `--graffiti`. If the graffiti is too long, lighthouse will not start and throw an error message. Otherwise, it will set the Graffiti to be the one provided by the user, right-padded with 0s. 2. Create a new `Graffiti` type and unify the code around it. With this type, everything is enforced at compile-time, and the code can be (I think...) panic-free! :) ## Additional info Currently, only `&str` are supported, as this is the returned type by `.arg("graffiti")`. Since this is user-input, I tried being as careful as I could. This is also why I created the `Graffiti` type, to make sure I could check as much as possible at compile time. --- beacon_node/beacon_chain/src/beacon_chain.rs | 13 +++---------- beacon_node/beacon_chain/src/builder.rs | 12 +++++++++++- beacon_node/client/src/builder.rs | 4 +++- beacon_node/client/src/config.rs | 4 ++++ beacon_node/src/cli.rs | 18 ++++++++++++++++++ beacon_node/src/config.rs | 18 +++++++++++++++++- consensus/types/src/beacon_block.rs | 2 +- consensus/types/src/beacon_block_body.rs | 4 ++-- consensus/types/src/lib.rs | 1 + consensus/types/src/utils/serde_utils.rs | 14 ++++++++------ 10 files changed, 68 insertions(+), 22 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 1dac8b296a..452ad82297 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -49,12 +49,6 @@ use types::*; pub type ForkChoiceError = fork_choice::Error; -// Text included in blocks. -// Must be 32-bytes or panic. -// -// |-------must be this long------| -pub const GRAFFITI: &str = "sigp/lighthouse-0.1.2-prerelease"; - /// The time-out before failure during an operation to take a read/write RwLock on the canonical /// head. pub const HEAD_LOCK_TIMEOUT: Duration = Duration::from_secs(1); @@ -224,6 +218,8 @@ pub struct BeaconChain { pub disabled_forks: Vec, /// Logging to CLI, etc. pub(crate) log: Logger, + /// Arbitrary bytes included in the blocks. + pub(crate) graffiti: Graffiti, } type BeaconBlockAndState = (BeaconBlock, BeaconState); @@ -1615,9 +1611,6 @@ impl BeaconChain { state.latest_block_header.canonical_root() }; - let mut graffiti: [u8; 32] = [0; 32]; - graffiti.copy_from_slice(GRAFFITI.as_bytes()); - let (proposer_slashings, attester_slashings) = self.op_pool.get_slashings(&state); let eth1_data = eth1_chain.eth1_data_for_block_production(&state, &self.spec)?; @@ -1649,7 +1642,7 @@ impl BeaconChain { body: BeaconBlockBody { randao_reveal, eth1_data, - graffiti, + graffiti: self.graffiti.clone(), proposer_slashings: proposer_slashings.into(), attester_slashings: attester_slashings.into(), attestations: self diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index c9deca146a..edfb4a8243 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -27,7 +27,8 @@ use std::sync::Arc; use std::time::Duration; use store::{HotColdDB, ItemStore}; use types::{ - BeaconBlock, BeaconState, ChainSpec, EthSpec, Hash256, Signature, SignedBeaconBlock, Slot, + BeaconBlock, BeaconState, ChainSpec, EthSpec, Graffiti, Hash256, Signature, SignedBeaconBlock, + Slot, }; pub const PUBKEY_CACHE_FILENAME: &str = "pubkey_cache.ssz"; @@ -110,6 +111,7 @@ pub struct BeaconChainBuilder { spec: ChainSpec, disabled_forks: Vec, log: Option, + graffiti: Graffiti, } impl @@ -155,6 +157,7 @@ where validator_pubkey_cache: None, spec: TEthSpec::default_spec(), log: None, + graffiti: Graffiti::default(), } } @@ -396,6 +399,12 @@ where self } + /// Sets the `graffiti` field. + pub fn graffiti(mut self, graffiti: Graffiti) -> Self { + self.graffiti = graffiti; + self + } + /// Consumes `self`, returning a `BeaconChain` if all required parameters have been supplied. /// /// An error will be returned at runtime if all required parameters have not been configured. @@ -523,6 +532,7 @@ where validator_pubkey_cache: TimeoutRwLock::new(validator_pubkey_cache), disabled_forks: self.disabled_forks, log: log.clone(), + graffiti: self.graffiti, }; let head = beacon_chain diff --git a/beacon_node/client/src/builder.rs b/beacon_node/client/src/builder.rs index 561b8019b8..31eb4dfea3 100644 --- a/beacon_node/client/src/builder.rs +++ b/beacon_node/client/src/builder.rs @@ -134,6 +134,7 @@ where let eth_spec_instance = self.eth_spec_instance.clone(); let data_dir = config.data_dir.clone(); let disabled_forks = config.disabled_forks.clone(); + let graffiti = config.graffiti.clone(); let store = store.ok_or_else(|| "beacon_chain_start_method requires a store".to_string())?; @@ -151,7 +152,8 @@ where .store_migrator(store_migrator) .data_dir(data_dir) .custom_spec(spec.clone()) - .disabled_forks(disabled_forks); + .disabled_forks(disabled_forks) + .graffiti(graffiti); let chain_exists = builder .store_contains_beacon_chain() diff --git a/beacon_node/client/src/config.rs b/beacon_node/client/src/config.rs index 067005a4a9..9311e33f19 100644 --- a/beacon_node/client/src/config.rs +++ b/beacon_node/client/src/config.rs @@ -2,6 +2,7 @@ use network::NetworkConfig; use serde_derive::{Deserialize, Serialize}; use std::fs; use std::path::PathBuf; +use types::Graffiti; pub const DEFAULT_DATADIR: &str = ".lighthouse"; @@ -55,6 +56,8 @@ pub struct Config { pub sync_eth1_chain: bool, /// A list of hard-coded forks that will be disabled. pub disabled_forks: Vec, + /// Graffiti to be inserted everytime we create a block. + pub graffiti: Graffiti, #[serde(skip)] /// The `genesis` field is not serialized or deserialized by `serde` to ensure it is defined /// via the CLI at runtime, instead of from a configuration file saved to disk. @@ -84,6 +87,7 @@ impl Default for Config { sync_eth1_chain: false, eth1: <_>::default(), disabled_forks: Vec::new(), + graffiti: Graffiti::default(), } } } diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 07cc44fabc..6e319e8490 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -1,5 +1,11 @@ use clap::{App, Arg}; +// Default text included in blocks. +// Must be 32-bytes or will not build. +// +// |-------must be this long------| +const DEFAULT_GRAFFITI: &str = "sigp/lighthouse-0.1.2-prerelease"; + pub fn cli_app<'a, 'b>() -> App<'a, 'b> { App::new("beacon_node") .visible_aliases(&["b", "bn", "beacon"]) @@ -227,4 +233,16 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .long("purge-db") .help("If present, the chain database will be deleted. Use with caution.") ) + + /* + * Misc. + */ + .arg( + Arg::with_name("graffiti") + .long("graffiti") + .help("Specify your custom graffiti to be included in blocks.") + .value_name("GRAFFITI") + .default_value(DEFAULT_GRAFFITI) + .takes_value(true) + ) } diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 5cb029a363..dd8bb9928d 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -10,7 +10,7 @@ use std::fs; use std::net::{IpAddr, Ipv4Addr, ToSocketAddrs}; use std::net::{TcpListener, UdpSocket}; use std::path::PathBuf; -use types::{ChainSpec, EthSpec}; +use types::{ChainSpec, EthSpec, GRAFFITI_BYTES_LEN}; pub const BEACON_NODE_DIR: &str = "beacon"; pub const NETWORK_DIR: &str = "network"; @@ -343,6 +343,22 @@ pub fn get_config( client_config.genesis = ClientGenesis::DepositContract; } + if let Some(graffiti) = cli_args.value_of("graffiti") { + let graffiti_bytes = graffiti.as_bytes(); + if graffiti_bytes.len() > GRAFFITI_BYTES_LEN { + return Err(format!( + "Your graffiti is too long! {} bytes maximum!", + GRAFFITI_BYTES_LEN + )); + } else { + // `client_config.graffiti` is initialized by default to be all 0. + // We simply copy the bytes from `graffiti_bytes` in there. + // + // Panic-free because `graffiti_bytes.len()` <= `GRAFFITI_BYTES_LEN`. + client_config.graffiti[..graffiti_bytes.len()].copy_from_slice(graffiti_bytes); + } + } + Ok(client_config) } diff --git a/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index eda2bc5910..1a31da8752 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -41,7 +41,7 @@ impl BeaconBlock { block_hash: Hash256::zero(), deposit_count: 0, }, - graffiti: [0; 32], + graffiti: Graffiti::default(), proposer_slashings: VariableList::empty(), attester_slashings: VariableList::empty(), attestations: VariableList::empty(), diff --git a/consensus/types/src/beacon_block_body.rs b/consensus/types/src/beacon_block_body.rs index 51de605465..489c5bc9d7 100644 --- a/consensus/types/src/beacon_block_body.rs +++ b/consensus/types/src/beacon_block_body.rs @@ -1,5 +1,5 @@ use crate::test_utils::TestRandom; -use crate::utils::{graffiti_from_hex_str, graffiti_to_hex_str}; +use crate::utils::{graffiti_from_hex_str, graffiti_to_hex_str, Graffiti}; use crate::*; use serde_derive::{Deserialize, Serialize}; @@ -21,7 +21,7 @@ pub struct BeaconBlockBody { serialize_with = "graffiti_to_hex_str", deserialize_with = "graffiti_from_hex_str" )] - pub graffiti: [u8; 32], + pub graffiti: Graffiti, pub proposer_slashings: VariableList, pub attester_slashings: VariableList, T::MaxAttesterSlashings>, pub attestations: VariableList, T::MaxAttestations>, diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index c941052dc6..515b93005d 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -100,3 +100,4 @@ pub use bls::{ Signature, SignatureBytes, }; pub use ssz_types::{typenum, typenum::Unsigned, BitList, BitVector, FixedVector, VariableList}; +pub use utils::{Graffiti, GRAFFITI_BYTES_LEN}; diff --git a/consensus/types/src/utils/serde_utils.rs b/consensus/types/src/utils/serde_utils.rs index d2d3e5655c..36b719646b 100644 --- a/consensus/types/src/utils/serde_utils.rs +++ b/consensus/types/src/utils/serde_utils.rs @@ -4,6 +4,11 @@ use serde::{Deserialize, Deserializer, Serializer}; pub const FORK_BYTES_LEN: usize = 4; pub const GRAFFITI_BYTES_LEN: usize = 32; +/// Type for a slice of `GRAFFITI_BYTES_LEN` bytes. +/// +/// Gets included inside each `BeaconBlockBody`. +pub type Graffiti = [u8; GRAFFITI_BYTES_LEN]; + pub fn u8_from_hex_str<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, @@ -92,10 +97,7 @@ where serializer.serialize_str(&hex_string) } -pub fn graffiti_to_hex_str( - bytes: &[u8; GRAFFITI_BYTES_LEN], - serializer: S, -) -> Result +pub fn graffiti_to_hex_str(bytes: &Graffiti, serializer: S) -> Result where S: Serializer, { @@ -105,12 +107,12 @@ where serializer.serialize_str(&hex_string) } -pub fn graffiti_from_hex_str<'de, D>(deserializer: D) -> Result<[u8; GRAFFITI_BYTES_LEN], D::Error> +pub fn graffiti_from_hex_str<'de, D>(deserializer: D) -> Result where D: Deserializer<'de>, { let s: String = Deserialize::deserialize(deserializer)?; - let mut array = [0 as u8; GRAFFITI_BYTES_LEN]; + let mut array = Graffiti::default(); let start = s .as_str()