Set graffiti per validator (#2044)

## Issue Addressed Resolves #1944 ## Proposed Changes Adds a "graffiti" key to the `validator_definitions.yml`. Setting the key will override anything passed through the validator `--graffiti` flag. Returns an error if the value for the graffiti key is > 32 bytes instead of silently truncating.
sigp · Mar 2, 2021 · da8791a · da8791a
1 parent 1c507c5
commit da8791a
Show file tree

Hide file tree

Showing 18 changed files with 428 additions and 14 deletions.
diff --git a/account_manager/src/validator/import.rs b/account_manager/src/validator/import.rs
@@ -223,7 +223,7 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin
         num_imported_keystores += 1;
 
         let validator_def =
-            ValidatorDefinition::new_keystore_with_password(&dest_keystore, password_opt)
+            ValidatorDefinition::new_keystore_with_password(&dest_keystore, password_opt, None)
                 .map_err(|e| format!("Unable to create new validator definition: {:?}", e))?;
 
         defs.push(validator_def);

diff --git a/book/src/SUMMARY.md b/book/src/SUMMARY.md
@@ -30,6 +30,7 @@
 	* [Prometheus Metrics](./advanced_metrics.md)
 * [Advanced Usage](./advanced.md)
     * [Custom Data Directories](./advanced-datadir.md)
+    * [Validator Graffiti](./graffiti.md)
     * [Database Configuration](./advanced_database.md)
 	* [Local Testnets](./local-testnets.md)
     * [Advanced Networking](./advanced_networking.md)

diff --git a/book/src/api-vc-endpoints.md b/book/src/api-vc-endpoints.md
@@ -279,7 +279,8 @@ Typical Responses | 200
     {
         "enable": true,
         "description": "validator_one",
-        "deposit_gwei": "32000000000"
+        "deposit_gwei": "32000000000",
+        "graffiti": "Mr F was here"
     },
     {
         "enable": false,

diff --git a/book/src/graffiti.md b/book/src/graffiti.md
@@ -0,0 +1,62 @@
+# Validator Graffiti
+
+Lighthouse provides four options for setting validator graffiti.
+
+### 1. Using the "--graffiti-file" flag on the validator client
+Users can specify a file with the `--graffiti-file` flag. This option is useful for dynamically changing graffitis for various use cases (e.g. drawing on the beaconcha.in graffiti wall). This file is loaded once on startup and reloaded everytime a validator is chosen to propose a block.
+
+Usage:
+`lighthouse vc --graffiti-file graffiti_file.txt`
+
+The file should contain key value pairs corresponding to validator public keys and their associated graffiti. The file can also contain a `default` key for the default case.
+```
+default: default_graffiti
+public_key1: graffiti1
+public_key2: graffiti2
+...
+```
+
+Below is an example of a graffiti file:
+
+```
+default: Lighthouse
+0x87a580d31d7bc69069b55f5a01995a610dd391a26dc9e36e81057a17211983a79266800ab8531f21f1083d7d84085007: mr f was here
+0xa5566f9ec3c6e1fdf362634ebec9ef7aceb0e460e5079714808388e5d48f4ae1e12897fed1bea951c17fa389d511e477: mr v was here
+```
+
+Lighthouse will first search for the graffiti corresponding to the public key of the proposing validator, if there are no matches for the public key, then it uses the graffiti corresponding to the default key if present.
+
+### 2. Setting the graffiti in the `validator_definitions.yml`
+Users can set validator specific graffitis in `validator_definitions.yml` with the `graffiti` key. This option is recommended for static setups where the graffitis won't change on every new block proposal.
+
+Below is an example of the validator_definitions.yml with validator specific graffitis:
+```
+---
+- enabled: true
+  voting_public_key: "0x87a580d31d7bc69069b55f5a01995a610dd391a26dc9e36e81057a17211983a79266800ab8531f21f1083d7d84085007"
+  type: local_keystore
+  voting_keystore_path: /home/paul/.lighthouse/validators/0x87a580d31d7bc69069b55f5a01995a610dd391a26dc9e36e81057a17211983a79266800ab8531f21f1083d7d84085007/voting-keystore.json
+  voting_keystore_password_path: /home/paul/.lighthouse/secrets/0x87a580d31d7bc69069b55f5a01995a610dd391a26dc9e36e81057a17211983a79266800ab8531f21f1083d7d84085007
+  graffiti: "mr f was here"
+- enabled: false
+  voting_public_key: "0xa5566f9ec3c6e1fdf362634ebec9ef7aceb0e460e5079714808388e5d48f4ae1e12897fed1bea951c17fa389d511e477"
+  type: local_keystore
+  voting_keystore_path: /home/paul/.lighthouse/validators/0xa5566f9ec3c6e1fdf362634ebec9ef7aceb0e460e5079714808388e5d48f4ae1e12897fed1bea951c17fa389d511e477/voting-keystore.json
+  voting_keystore_password: myStrongpa55word123&$
+  graffiti: "somethingprofound"
+```
+
+### 3. Using the "--graffiti" flag on the validator client
+Users can specify a common graffiti for all their validators using the `--graffiti` flag on the validator client.
+
+### 4. Using the "--graffiti" flag on the beacon node
+Users can also specify a common graffiti using the `--graffiti` flag on the beacon node as a common  graffiti for all validators.
+
+Usage: `lighthouse vc --graffiti fortytwo`
+
+> Note: The order of preference for loading the graffiti is as follows:
+> 1. Read from `--graffiti-file` if provided.
+> 2. If `--graffiti-file` is not provided or errors, read graffiti from `validator_definitions.yml`.
+> 3. If graffiti is not specified in `validator_definitions.yml`, load the graffiti passed in the `--graffiti` flag on the validator client.
+> 4. If the `--graffiti` flag on the validator client is not passed, load the graffiti passed in the `--graffiti` flag on the beacon node.
+> 4. If the `--graffiti` flag is not passed, load the default Lighthouse graffiti.
diff --git a/common/account_utils/src/validator_definitions.rs b/common/account_utils/src/validator_definitions.rs
@@ -13,7 +13,7 @@ use std::collections::HashSet;
 use std::fs::{self, OpenOptions};
 use std::io;
 use std::path::{Path, PathBuf};
-use types::PublicKey;
+use types::{graffiti::GraffitiString, PublicKey};
 use validator_dir::VOTING_KEYSTORE_FILE;
 
 /// The file name for the serialized `ValidatorDefinitions` struct.
@@ -66,6 +66,9 @@ pub struct ValidatorDefinition {
     pub enabled: bool,
     pub voting_public_key: PublicKey,
     #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub graffiti: Option<GraffitiString>,
+    #[serde(default)]
     pub description: String,
     #[serde(flatten)]
     pub signing_definition: SigningDefinition,
@@ -81,6 +84,7 @@ impl ValidatorDefinition {
     pub fn new_keystore_with_password<P: AsRef<Path>>(
         voting_keystore_path: P,
         voting_keystore_password: Option<ZeroizeString>,
+        graffiti: Option<GraffitiString>,
     ) -> Result<Self, Error> {
         let voting_keystore_path = voting_keystore_path.as_ref().into();
         let keystore =
@@ -91,6 +95,7 @@ impl ValidatorDefinition {
             enabled: true,
             voting_public_key,
             description: keystore.description().unwrap_or("").to_string(),
+            graffiti,
             signing_definition: SigningDefinition::LocalKeystore {
                 voting_keystore_path,
                 voting_keystore_password_path: None,
@@ -227,6 +232,7 @@ impl ValidatorDefinitions {
                     enabled: true,
                     voting_public_key,
                     description: keystore.description().unwrap_or("").to_string(),
+                    graffiti: None,
                     signing_definition: SigningDefinition::LocalKeystore {
                         voting_keystore_path,
                         voting_keystore_password_path,
@@ -347,6 +353,7 @@ pub fn is_voting_keystore(file_name: &str) -> bool {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::str::FromStr;
 
     #[test]
     fn voting_keystore_filename_lighthouse() {
@@ -382,4 +389,44 @@ mod tests {
         assert!(!is_voting_keystore("keystore-0a.json"));
         assert!(!is_voting_keystore("keystore-cats.json"));
     }
+
+    #[test]
+    fn graffiti_checks() {
+        let no_graffiti = r#"--- 
+        description: ""
+        enabled: true
+        type: local_keystore
+        voting_keystore_path: ""
+        voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7"
+        "#;
+        let def: ValidatorDefinition = serde_yaml::from_str(&no_graffiti).unwrap();
+        assert!(def.graffiti.is_none());
+
+        let invalid_graffiti = r#"--- 
+        description: ""
+        enabled: true
+        type: local_keystore
+        graffiti: "mrfwasheremrfwasheremrfwasheremrf"
+        voting_keystore_path: ""
+        voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7"
+        "#;
+
+        let def: Result<ValidatorDefinition, _> = serde_yaml::from_str(&invalid_graffiti);
+        assert!(def.is_err());
+
+        let valid_graffiti = r#"--- 
+        description: ""
+        enabled: true
+        type: local_keystore
+        graffiti: "mrfwashere"
+        voting_keystore_path: ""
+        voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7"
+        "#;
+
+        let def: ValidatorDefinition = serde_yaml::from_str(&valid_graffiti).unwrap();
+        assert_eq!(
+            def.graffiti,
+            Some(GraffitiString::from_str("mrfwashere").unwrap())
+        );
+    }
 }
diff --git a/common/eth2/src/lighthouse_vc/types.rs b/common/eth2/src/lighthouse_vc/types.rs
@@ -1,5 +1,6 @@
 use account_utils::ZeroizeString;
 use eth2_keystore::Keystore;
+use graffiti::GraffitiString;
 use serde::{Deserialize, Serialize};
 
 pub use crate::lighthouse::Health;
@@ -17,6 +18,9 @@ pub struct ValidatorData {
 pub struct ValidatorRequest {
     pub enable: bool,
     pub description: String,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub graffiti: Option<GraffitiString>,
     #[serde(with = "serde_utils::quoted_u64")]
     pub deposit_gwei: u64,
 }
@@ -34,6 +38,9 @@ pub struct CreatedValidator {
     pub enabled: bool,
     pub description: String,
     pub voting_pubkey: PublicKeyBytes,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub graffiti: Option<GraffitiString>,
     pub eth1_deposit_tx_data: String,
     #[serde(with = "serde_utils::quoted_u64")]
     pub deposit_gwei: u64,
@@ -55,4 +62,5 @@ pub struct KeystoreValidatorsPostRequest {
     pub password: ZeroizeString,
     pub enable: bool,
     pub keystore: Keystore,
+    pub graffiti: Option<GraffitiString>,
 }
diff --git a/consensus/types/src/graffiti.rs b/consensus/types/src/graffiti.rs
@@ -6,6 +6,7 @@ use regex::bytes::Regex;
 use serde::{de::Error, Deserialize, Deserializer, Serialize, Serializer};
 use ssz::{Decode, DecodeError, Encode};
 use std::fmt;
+use std::str::FromStr;
 use tree_hash::TreeHash;
 
 pub const GRAFFITI_BYTES_LEN: usize = 32;
@@ -42,6 +43,49 @@ impl Into<[u8; GRAFFITI_BYTES_LEN]> for Graffiti {
     }
 }
 
+#[derive(Debug, Clone, PartialEq, Serialize, Default)]
+#[serde(transparent)]
+pub struct GraffitiString(String);
+
+impl FromStr for GraffitiString {
+    type Err = String;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        if s.as_bytes().len() > GRAFFITI_BYTES_LEN {
+            return Err(format!(
+                "Graffiti exceeds max length {}",
+                GRAFFITI_BYTES_LEN
+            ));
+        }
+        Ok(Self(s.to_string()))
+    }
+}
+
+impl<'de> Deserialize<'de> for GraffitiString {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let s: String = serde::Deserialize::deserialize(deserializer)?;
+        GraffitiString::from_str(&s).map_err(serde::de::Error::custom)
+    }
+}
+
+impl Into<Graffiti> for GraffitiString {
+    fn into(self) -> Graffiti {
+        let graffiti_bytes = self.0.as_bytes();
+        let mut graffiti = [0; 32];
+
+        let graffiti_len = std::cmp::min(graffiti_bytes.len(), 32);
+
+        // Copy the provided bytes over.
+        //
+        // Panic-free because `graffiti_bytes.len()` <= `GRAFFITI_BYTES_LEN`.
+        graffiti[..graffiti_len].copy_from_slice(&graffiti_bytes);
+        graffiti.into()
+    }
+}
+
 pub mod serde_graffiti {
     use super::*;
 

diff --git a/lighthouse/tests/account_manager.rs b/lighthouse/tests/account_manager.rs
@@ -482,6 +482,7 @@ fn validator_import_launchpad() {
     let expected_def = ValidatorDefinition {
         enabled: true,
         description: "".into(),
+        graffiti: None,
         voting_public_key: keystore.public_key().unwrap(),
         signing_definition: SigningDefinition::LocalKeystore {
             voting_keystore_path,

diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs
@@ -1,4 +1,7 @@
-use crate::beacon_node_fallback::{BeaconNodeFallback, RequireSynced};
+use crate::{
+    beacon_node_fallback::{BeaconNodeFallback, RequireSynced},
+    graffiti_file::GraffitiFile,
+};
 use crate::{http_metrics::metrics, validator_store::ValidatorStore};
 use environment::RuntimeContext;
 use eth2::types::Graffiti;
@@ -17,6 +20,7 @@ pub struct BlockServiceBuilder<T, E: EthSpec> {
     beacon_nodes: Option<Arc<BeaconNodeFallback<T, E>>>,
     context: Option<RuntimeContext<E>>,
     graffiti: Option<Graffiti>,
+    graffiti_file: Option<GraffitiFile>,
 }
 
 impl<T: SlotClock + 'static, E: EthSpec> BlockServiceBuilder<T, E> {
@@ -27,6 +31,7 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockServiceBuilder<T, E> {
             beacon_nodes: None,
             context: None,
             graffiti: None,
+            graffiti_file: None,
         }
     }
 
@@ -55,6 +60,11 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockServiceBuilder<T, E> {
         self
     }
 
+    pub fn graffiti_file(mut self, graffiti_file: Option<GraffitiFile>) -> Self {
+        self.graffiti_file = graffiti_file;
+        self
+    }
+
     pub fn build(self) -> Result<BlockService<T, E>, String> {
         Ok(BlockService {
             inner: Arc::new(Inner {
@@ -71,6 +81,7 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockServiceBuilder<T, E> {
                     .context
                     .ok_or("Cannot build BlockService without runtime_context")?,
                 graffiti: self.graffiti,
+                graffiti_file: self.graffiti_file,
             }),
         })
     }
@@ -83,6 +94,7 @@ pub struct Inner<T, E: EthSpec> {
     beacon_nodes: Arc<BeaconNodeFallback<T, E>>,
     context: RuntimeContext<E>,
     graffiti: Option<Graffiti>,
+    graffiti_file: Option<GraffitiFile>,
 }
 
 /// Attempts to produce attestations for any block producer(s) at the start of the epoch.
@@ -226,14 +238,27 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {
             .ok_or("Unable to produce randao reveal")?
             .into();
 
+        let graffiti = self
+            .graffiti_file
+            .clone()
+            .and_then(|mut g| match g.load_graffiti(&validator_pubkey) {
+                Ok(g) => g,
+                Err(e) => {
+                    warn!(log, "Failed to read graffiti file"; "error" => ?e);
+                    None
+                }
+            })
+            .or_else(|| self.validator_store.graffiti(&validator_pubkey))
+            .or(self.graffiti);
+
         let randao_reveal_ref = &randao_reveal;
         let self_ref = &self;
         let validator_pubkey_ref = &validator_pubkey;
         let signed_block = self
             .beacon_nodes
             .first_success(RequireSynced::No, |beacon_node| async move {
                 let block = beacon_node
-                    .get_validator_blocks(slot, randao_reveal_ref, self_ref.graffiti.as_ref())
+                    .get_validator_blocks(slot, randao_reveal_ref, graffiti.as_ref())
                     .await
                     .map_err(|e| format!("Error from beacon node when producing block: {:?}", e))?
                     .data;
@@ -260,6 +285,7 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {
             "Successfully published block";
             "deposits" => signed_block.message.body.deposits.len(),
             "attestations" => signed_block.message.body.attestations.len(),
+            "graffiti" => ?graffiti.map(|g| g.as_utf8_lossy()),
             "slot" => signed_block.slot().as_u64(),
         );
 

diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs
@@ -102,6 +102,14 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                 .value_name("GRAFFITI")
                 .takes_value(true)
         )
+        .arg(
+            Arg::with_name("graffiti-file")
+                .long("graffiti-file")
+                .help("Specify a graffiti file to load validator graffitis from.")
+                .value_name("GRAFFITI-FILE")
+                .takes_value(true)
+                .conflicts_with("graffiti")
+        )
         /* REST API related arguments */
         .arg(
             Arg::with_name("http")