From 943716b9a22c1c2589739f3f5af4725f69f48c3a Mon Sep 17 00:00:00 2001
From: Shayan Eskandari <shayanb@users.noreply.github.com>
Date: Fri, 13 Dec 2024 00:07:01 -0500
Subject: [PATCH] Fix for blank line in graffiti file (#6635)

* Fix for blank line in graffiti file

Fix as described in https://github.com/sigp/lighthouse/issues/5880

* add graffiti new line tests

* cargo fmt
---
 validator_client/graffiti_file/src/lib.rs | 52 +++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/validator_client/graffiti_file/src/lib.rs b/validator_client/graffiti_file/src/lib.rs
index 0328c14eeb5..9dab2e78272 100644
--- a/validator_client/graffiti_file/src/lib.rs
+++ b/validator_client/graffiti_file/src/lib.rs
@@ -66,6 +66,9 @@ impl GraffitiFile {
 
         for line in lines {
             let line = line.map_err(|e| Error::InvalidLine(e.to_string()))?;
+            if line.trim().is_empty() {
+                continue;
+            }
             let (pk_opt, graffiti) = read_line(&line)?;
             match pk_opt {
                 Some(pk) => {
@@ -133,9 +136,15 @@ mod tests {
     const CUSTOM_GRAFFITI1: &str = "custom-graffiti1";
     const CUSTOM_GRAFFITI2: &str = "graffitiwall:720:641:#ffff00";
     const EMPTY_GRAFFITI: &str = "";
+    // Newline test cases
+    const CUSTOM_GRAFFITI4: &str = "newlines-tests";
+
     const PK1: &str = "0x800012708dc03f611751aad7a43a082142832b5c1aceed07ff9b543cf836381861352aa923c70eeb02018b638aa306aa";
     const PK2: &str = "0x80001866ce324de7d80ec73be15e2d064dcf121adf1b34a0d679f2b9ecbab40ce021e03bb877e1a2fe72eaaf475e6e21";
     const PK3: &str = "0x9035d41a8bc11b08c17d0d93d876087958c9d055afe86fce558e3b988d92434769c8d50b0b463708db80c6aae1160c02";
+    const PK4: &str = "0x8c0fca2cc70f44188a4b79e5623ac85898f1df479e14a1f4ebb615907810b6fb939c3fb4ba2081b7a5b6e33dc73621d2";
+    const PK5: &str = "0x87998b0ea4a8826f03d1985e5a5ce7235bd3a56fb7559b02a55b737f4ebc69b0bf35444de5cf2680cb7eb2283eb62050";
+    const PK6: &str = "0xa2af9b128255568e2ee5c42af118cc4301198123d210dbdbf2ca7ec0222f8d491f308e85076b09a2f44a75875cd6fa0f";
 
     // Create a graffiti file in the required format and return a path to the file.
     fn create_graffiti_file() -> PathBuf {
@@ -143,6 +152,9 @@ mod tests {
         let pk1 = PublicKeyBytes::deserialize(&hex::decode(&PK1[2..]).unwrap()).unwrap();
         let pk2 = PublicKeyBytes::deserialize(&hex::decode(&PK2[2..]).unwrap()).unwrap();
         let pk3 = PublicKeyBytes::deserialize(&hex::decode(&PK3[2..]).unwrap()).unwrap();
+        let pk4 = PublicKeyBytes::deserialize(&hex::decode(&PK4[2..]).unwrap()).unwrap();
+        let pk5 = PublicKeyBytes::deserialize(&hex::decode(&PK5[2..]).unwrap()).unwrap();
+        let pk6 = PublicKeyBytes::deserialize(&hex::decode(&PK6[2..]).unwrap()).unwrap();
 
         let file_name = temp.into_path().join("graffiti.txt");
 
@@ -160,6 +172,29 @@ mod tests {
         graffiti_file
             .write_all(format!("{}:{}\n", pk3.as_hex_string(), EMPTY_GRAFFITI).as_bytes())
             .unwrap();
+
+        // Test Lines with leading newlines - these empty lines will be skipped
+        graffiti_file.write_all(b"\n").unwrap();
+        graffiti_file.write_all(b"   \n").unwrap();
+        graffiti_file
+            .write_all(format!("{}: {}\n", pk4.as_hex_string(), CUSTOM_GRAFFITI4).as_bytes())
+            .unwrap();
+
+        // Test Empty lines between entries - these will be skipped
+        graffiti_file.write_all(b"\n").unwrap();
+        graffiti_file.write_all(b"   \n").unwrap();
+        graffiti_file.write_all(b"\t\n").unwrap();
+        graffiti_file
+            .write_all(format!("{}: {}\n", pk5.as_hex_string(), CUSTOM_GRAFFITI4).as_bytes())
+            .unwrap();
+
+        // Test Trailing empty lines - these will be skipped
+        graffiti_file
+            .write_all(format!("{}: {}\n", pk6.as_hex_string(), CUSTOM_GRAFFITI4).as_bytes())
+            .unwrap();
+        graffiti_file.write_all(b"\n").unwrap();
+        graffiti_file.write_all(b"   \n").unwrap();
+
         graffiti_file.flush().unwrap();
         file_name
     }
@@ -172,6 +207,9 @@ mod tests {
         let pk1 = PublicKeyBytes::deserialize(&hex::decode(&PK1[2..]).unwrap()).unwrap();
         let pk2 = PublicKeyBytes::deserialize(&hex::decode(&PK2[2..]).unwrap()).unwrap();
         let pk3 = PublicKeyBytes::deserialize(&hex::decode(&PK3[2..]).unwrap()).unwrap();
+        let pk4 = PublicKeyBytes::deserialize(&hex::decode(&PK4[2..]).unwrap()).unwrap();
+        let pk5 = PublicKeyBytes::deserialize(&hex::decode(&PK5[2..]).unwrap()).unwrap();
+        let pk6 = PublicKeyBytes::deserialize(&hex::decode(&PK6[2..]).unwrap()).unwrap();
 
         // Read once
         gf.read_graffiti_file().unwrap();
@@ -190,6 +228,20 @@ mod tests {
             GraffitiString::from_str(EMPTY_GRAFFITI).unwrap().into()
         );
 
+        // Test newline cases - all empty lines should be skipped
+        assert_eq!(
+            gf.load_graffiti(&pk4).unwrap().unwrap(),
+            GraffitiString::from_str(CUSTOM_GRAFFITI4).unwrap().into()
+        );
+        assert_eq!(
+            gf.load_graffiti(&pk5).unwrap().unwrap(),
+            GraffitiString::from_str(CUSTOM_GRAFFITI4).unwrap().into()
+        );
+        assert_eq!(
+            gf.load_graffiti(&pk6).unwrap().unwrap(),
+            GraffitiString::from_str(CUSTOM_GRAFFITI4).unwrap().into()
+        );
+
         // Random pk should return the default graffiti
         let random_pk = Keypair::random().pk.compress();
         assert_eq!(