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.
This commit is contained in:
pscott 2020-07-14 08:05:02 +00:00
parent 00c89c51c8
commit e164371083
10 changed files with 68 additions and 22 deletions

View File

@ -49,12 +49,6 @@ use types::*;
pub type ForkChoiceError = fork_choice::Error<crate::ForkChoiceStoreError>;
// 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<T: BeaconChainTypes> {
pub disabled_forks: Vec<String>,
/// Logging to CLI, etc.
pub(crate) log: Logger,
/// Arbitrary bytes included in the blocks.
pub(crate) graffiti: Graffiti,
}
type BeaconBlockAndState<T> = (BeaconBlock<T>, BeaconState<T>);
@ -1615,9 +1611,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
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<T: BeaconChainTypes> BeaconChain<T> {
body: BeaconBlockBody {
randao_reveal,
eth1_data,
graffiti,
graffiti: self.graffiti.clone(),
proposer_slashings: proposer_slashings.into(),
attester_slashings: attester_slashings.into(),
attestations: self

View File

@ -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<T: BeaconChainTypes> {
spec: ChainSpec,
disabled_forks: Vec<String>,
log: Option<Logger>,
graffiti: Graffiti,
}
impl<TStoreMigrator, TSlotClock, TEth1Backend, TEthSpec, TEventHandler, THotStore, TColdStore>
@ -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

View File

@ -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()

View File

@ -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<String>,
/// 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(),
}
}
}

View File

@ -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)
)
}

View File

@ -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<E: EthSpec>(
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)
}

View File

@ -41,7 +41,7 @@ impl<T: EthSpec> BeaconBlock<T> {
block_hash: Hash256::zero(),
deposit_count: 0,
},
graffiti: [0; 32],
graffiti: Graffiti::default(),
proposer_slashings: VariableList::empty(),
attester_slashings: VariableList::empty(),
attestations: VariableList::empty(),

View File

@ -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<T: EthSpec> {
serialize_with = "graffiti_to_hex_str",
deserialize_with = "graffiti_from_hex_str"
)]
pub graffiti: [u8; 32],
pub graffiti: Graffiti,
pub proposer_slashings: VariableList<ProposerSlashing, T::MaxProposerSlashings>,
pub attester_slashings: VariableList<AttesterSlashing<T>, T::MaxAttesterSlashings>,
pub attestations: VariableList<Attestation<T>, T::MaxAttestations>,

View File

@ -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};

View File

@ -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<u8, D::Error>
where
D: Deserializer<'de>,
@ -92,10 +97,7 @@ where
serializer.serialize_str(&hex_string)
}
pub fn graffiti_to_hex_str<S>(
bytes: &[u8; GRAFFITI_BYTES_LEN],
serializer: S,
) -> Result<S::Ok, S::Error>
pub fn graffiti_to_hex_str<S>(bytes: &Graffiti, serializer: S) -> Result<S::Ok, S::Error>
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<Graffiti, D::Error>
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()