Unify Graffiti structs

Cargo.toml

@@ -45,7 +45,6 @@ members = [
     "consensus/ssz",
     "consensus/ssz_derive",
     "consensus/ssz_types",
-    "consensus/serde_hex",
     "consensus/serde_utils",
     "consensus/state_processing",
     "consensus/swap_or_not_shuffle",

beacon_node/beacon_chain/Cargo.toml

@@ -58,4 +58,3 @@ environment = { path = "../../lighthouse/environment" }
 bus = "2.2.3"
 derivative = "2.1.1"
 itertools = "0.9.0"
-regex = "1.3.9"

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -31,7 +31,6 @@ use fork_choice::ForkChoice;
 use itertools::process_results;
 use operation_pool::{OperationPool, PersistedOperationPool};
 use parking_lot::RwLock;
-use regex::bytes::Regex;
 use slog::{crit, debug, error, info, trace, warn, Logger};
 use slot_clock::SlotClock;
 use state_processing::{
@@ -1358,11 +1357,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         block: SignedBeaconBlock<T::EthSpec>,
     ) -> Result<GossipVerifiedBlock<T>, BlockError<T::EthSpec>> {
         let slot = block.message.slot;
-        #[allow(clippy::invalid_regex)]
-        let re = Regex::new("\\p{C}").expect("regex is valid");
-        let graffiti_string =
-            String::from_utf8_lossy(&re.replace_all(&block.message.body.graffiti[..], &b""[..]))
-                .to_string();
+        let graffiti_string = block.message.body.graffiti.as_utf8_lossy();
 
         match GossipVerifiedBlock::new(block, self) {
             Ok(verified) => {

beacon_node/beacon_chain/src/test_utils.rs

@@ -399,7 +399,7 @@ where
         // If we produce two blocks for the same slot, they hash up to the same value and
         // BeaconChain errors out with `BlockIsAlreadyKnown`.  Vary the graffiti so that we produce
         // different blocks each time.
-        self.chain.set_graffiti(self.rng.gen::<[u8; 32]>());
+        self.chain.set_graffiti(self.rng.gen::<[u8; 32]>().into());
 
         let randao_reveal = {
             let epoch = slot.epoch(E::slots_per_epoch());

common/eth2/src/types.rs

@@ -10,9 +10,9 @@ pub use rest_types::ValidatorSubscription;
 
 pub use types::{
     Address, Attestation, AttestationData, AttesterSlashing, BeaconBlock, BeaconBlockHeader,
-    BeaconState, Checkpoint, CommitteeIndex, Epoch, EthSpec, Fork, Hash256, ProposerSlashing,
-    PublicKeyBytes, SignatureBytes, SignedAggregateAndProof, SignedBeaconBlock,
-    SignedVoluntaryExit, Slot, Validator, YamlConfig, GRAFFITI_BYTES_LEN,
+    BeaconState, Checkpoint, CommitteeIndex, Epoch, EthSpec, Fork, Graffiti, Hash256,
+    ProposerSlashing, PublicKeyBytes, SignatureBytes, SignedAggregateAndProof, SignedBeaconBlock,
+    SignedVoluntaryExit, Slot, Validator, YamlConfig,
 };
 
 /// An API error serializable to JSON.
@@ -396,28 +396,6 @@ pub struct ProposerData {
     pub slot: Slot,
 }
 
-#[derive(Clone, Copy, Serialize, Deserialize)]
-#[serde(transparent)]
-pub struct Graffiti(#[serde(with = "serde_utils::graffiti")] pub [u8; GRAFFITI_BYTES_LEN]);
-
-impl fmt::Display for Graffiti {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}", serde_utils::hex::encode(&self.0))
-    }
-}
-
-impl From<[u8; GRAFFITI_BYTES_LEN]> for Graffiti {
-    fn from(bytes: [u8; GRAFFITI_BYTES_LEN]) -> Self {
-        Self(bytes)
-    }
-}
-
-impl Into<[u8; GRAFFITI_BYTES_LEN]> for Graffiti {
-    fn into(self) -> [u8; GRAFFITI_BYTES_LEN] {
-        self.0
-    }
-}
-
 #[derive(Clone, Serialize, Deserialize)]
 pub struct ValidatorBlocksQuery {
     pub randao_reveal: SignatureBytes,

consensus/serde_utils/src/hex.rs

@@ -8,6 +8,14 @@ pub fn encode<T: AsRef<[u8]>>(data: T) -> String {
     s
 }
 
+pub fn decode(s: &str) -> Result<Vec<u8>, String> {
+    if s.starts_with("0x") {
+        hex::decode(&s[2..]).map_err(|e| format!("invalid hex: {:?}", e))
+    } else {
+        Err("hex must have 0x prefix".to_string())
+    }
+}
+
 pub struct PrefixedHexVisitor;
 
 impl<'de> Visitor<'de> for PrefixedHexVisitor {

consensus/types/Cargo.toml

@@ -41,6 +41,7 @@ derivative = "2.1.1"
 rusqlite = { version = "0.23.1", features = ["bundled"], optional = true }
 arbitrary = { version = "0.4.4", features = ["derive"], optional = true }
 serde_utils = { path = "../serde_utils" }
+regex = "1.3.9"
 
 [dev-dependencies]
 serde_json = "1.0.52"

consensus/types/src/beacon_block_body.rs

@@ -1,4 +1,3 @@
-use crate::serde_utils::Graffiti;
 use crate::test_utils::TestRandom;
 use crate::*;
 
@@ -17,7 +16,6 @@ use tree_hash_derive::TreeHash;
 pub struct BeaconBlockBody<T: EthSpec> {
     pub randao_reveal: Signature,
     pub eth1_data: Eth1Data,
-    #[serde(with = "crate::serde_utils::graffiti")]
     pub graffiti: Graffiti,
     pub proposer_slashings: VariableList<ProposerSlashing, T::MaxProposerSlashings>,
     pub attester_slashings: VariableList<AttesterSlashing<T>, T::MaxAttesterSlashings>,

consensus/types/src/graffiti.rs

@@ -0,0 +1,130 @@
+use crate::{
+    test_utils::{RngCore, TestRandom},
+    Hash256,
+};
+use regex::bytes::Regex;
+use serde::{de::Error, Deserialize, Deserializer, Serialize, Serializer};
+use ssz::{Decode, DecodeError, Encode};
+use std::fmt;
+use tree_hash::TreeHash;
+
+pub const GRAFFITI_BYTES_LEN: usize = 32;
+
+#[derive(Default, Debug, PartialEq, Clone, Copy, Serialize, Deserialize)]
+#[serde(transparent)]
+pub struct Graffiti(#[serde(with = "serde_graffiti")] pub [u8; GRAFFITI_BYTES_LEN]);
+
+impl Graffiti {
+    pub fn as_utf8_lossy(&self) -> String {
+        #[allow(clippy::invalid_regex)]
+        let re = Regex::new("\\p{C}").expect("graffiti regex is valid");
+        String::from_utf8_lossy(&re.replace_all(&self.0[..], &b""[..])).to_string()
+    }
+}
+
+impl fmt::Display for Graffiti {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", serde_utils::hex::encode(&self.0))
+    }
+}
+
+impl From<[u8; GRAFFITI_BYTES_LEN]> for Graffiti {
+    fn from(bytes: [u8; GRAFFITI_BYTES_LEN]) -> Self {
+        Self(bytes)
+    }
+}
+
+impl Into<[u8; GRAFFITI_BYTES_LEN]> for Graffiti {
+    fn into(self) -> [u8; GRAFFITI_BYTES_LEN] {
+        self.0
+    }
+}
+
+pub mod serde_graffiti {
+    use super::*;
+
+    pub fn serialize<S>(bytes: &[u8; GRAFFITI_BYTES_LEN], serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        serializer.serialize_str(&serde_utils::hex::encode(bytes))
+    }
+
+    pub fn deserialize<'de, D>(deserializer: D) -> Result<[u8; GRAFFITI_BYTES_LEN], D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let s: String = Deserialize::deserialize(deserializer)?;
+
+        let bytes = serde_utils::hex::decode(&s).map_err(D::Error::custom)?;
+
+        if bytes.len() != GRAFFITI_BYTES_LEN {
+            return Err(D::Error::custom(format!(
+                "incorrect byte length {}, expected {}",
+                bytes.len(),
+                GRAFFITI_BYTES_LEN
+            )));
+        }
+
+        let mut array = [0; GRAFFITI_BYTES_LEN];
+        array[..].copy_from_slice(&bytes);
+
+        Ok(array)
+    }
+}
+
+impl Encode for Graffiti {
+    fn is_ssz_fixed_len() -> bool {
+        <[u8; GRAFFITI_BYTES_LEN] as Encode>::is_ssz_fixed_len()
+    }
+
+    fn ssz_fixed_len() -> usize {
+        <[u8; GRAFFITI_BYTES_LEN] as Encode>::ssz_fixed_len()
+    }
+
+    fn ssz_bytes_len(&self) -> usize {
+        self.0.ssz_bytes_len()
+    }
+
+    fn ssz_append(&self, buf: &mut Vec<u8>) {
+        self.0.ssz_append(buf)
+    }
+}
+
+impl Decode for Graffiti {
+    fn is_ssz_fixed_len() -> bool {
+        <[u8; GRAFFITI_BYTES_LEN] as Decode>::is_ssz_fixed_len()
+    }
+
+    fn ssz_fixed_len() -> usize {
+        <[u8; GRAFFITI_BYTES_LEN] as Decode>::ssz_fixed_len()
+    }
+
+    fn from_ssz_bytes(bytes: &[u8]) -> Result<Self, DecodeError> {
+        <[u8; GRAFFITI_BYTES_LEN]>::from_ssz_bytes(bytes).map(Self)
+    }
+}
+
+impl TreeHash for Graffiti {
+    fn tree_hash_type() -> tree_hash::TreeHashType {
+        <[u8; GRAFFITI_BYTES_LEN]>::tree_hash_type()
+    }
+
+    fn tree_hash_packed_encoding(&self) -> Vec<u8> {
+        self.0.tree_hash_packed_encoding()
+    }
+
+    fn tree_hash_packing_factor() -> usize {
+        <[u8; GRAFFITI_BYTES_LEN]>::tree_hash_packing_factor()
+    }
+
+    fn tree_hash_root(&self) -> tree_hash::Hash256 {
+        self.0.tree_hash_root()
+    }
+}
+
+impl TestRandom for Graffiti {
+    fn random_for_test(rng: &mut impl RngCore) -> Self {
+        Self::from(Hash256::random_for_test(rng).to_fixed_bytes())
+    }
+}

consensus/types/src/lib.rs

@@ -29,6 +29,7 @@ pub mod eth_spec;
 pub mod fork;
 pub mod fork_data;
 pub mod free_attestation;
+pub mod graffiti;
 pub mod historical_batch;
 pub mod indexed_attestation;
 pub mod pending_attestation;
@@ -75,6 +76,7 @@ pub use crate::eth1_data::Eth1Data;
 pub use crate::fork::Fork;
 pub use crate::fork_data::ForkData;
 pub use crate::free_attestation::FreeAttestation;
+pub use crate::graffiti::{Graffiti, GRAFFITI_BYTES_LEN};
 pub use crate::historical_batch::HistoricalBatch;
 pub use crate::indexed_attestation::IndexedAttestation;
 pub use crate::pending_attestation::PendingAttestation;
@@ -97,7 +99,6 @@ pub type Hash256 = H256;
 pub type Address = H160;
 pub type ForkVersion = [u8; 4];
 
-pub use crate::serde_utils::{Graffiti, GRAFFITI_BYTES_LEN};
 pub use bls::{
     AggregateSignature, Keypair, PublicKey, PublicKeyBytes, SecretKey, Signature, SignatureBytes,
 };