Simplify Phase 2: deduplicate signing logic and deser_pubkey_hex

Author: pablodeymo

bin/ethlambda/src/main.rs

@@ -23,7 +23,7 @@ use ethlambda_network_api::{InitBlockChain, InitP2P, ToBlockChainToP2PRef, ToP2P
 use ethlambda_p2p::{Bootnode, P2P, SwarmConfig, build_swarm, parse_enrs};
 use ethlambda_types::primitives::H256;
 use ethlambda_types::{
-    genesis::GenesisConfig,
+    genesis::{GenesisConfig, deser_pubkey_hex},
     signature::ValidatorSecretKey,
     state::{State, ValidatorPubkeyBytes},
 };
@@ -236,20 +236,6 @@ struct AnnotatedValidator {
     proposal_privkey_file: PathBuf,
 }
 
-pub fn deser_pubkey_hex<'de, D>(d: D) -> Result<ValidatorPubkeyBytes, D::Error>
-where
-    D: serde::Deserializer<'de>,
-{
-    use serde::de::Error;
-
-    let value = String::deserialize(d)?;
-    let pubkey: ValidatorPubkeyBytes = hex::decode(&value)
-        .map_err(|_| D::Error::custom("ValidatorPubkey value is not valid hex"))?
-        .try_into()
-        .map_err(|_| D::Error::custom("ValidatorPubkey length != 52"))?;
-    Ok(pubkey)
-}
-
 fn read_validator_keys(
     validators_path: impl AsRef<Path>,
     validator_keys_dir: impl AsRef<Path>,

crates/blockchain/src/key_manager.rs

@@ -19,20 +19,11 @@ pub enum KeyManagerError {
     SignatureConversionError(String),
 }
 
-/// A validator's dual XMSS key pair for attestation and block proposal signing.
-///
-/// Each key is independent and advances its OTS preparation separately,
-/// allowing the validator to sign both an attestation and a block proposal
-/// within the same slot.
 pub struct ValidatorKeyPair {
     pub attestation_key: ValidatorSecretKey,
     pub proposal_key: ValidatorSecretKey,
 }
 
-/// Manages validator secret keys for signing attestations and block proposals.
-///
-/// Each validator has two independent XMSS keys: one for attestation signing
-/// and one for block proposal signing.
 pub struct KeyManager {
     keys: HashMap<u64, ValidatorKeyPair>,
 }
@@ -42,78 +33,50 @@ impl KeyManager {
         Self { keys }
     }
 
-    /// Returns a list of all registered validator IDs.
     pub fn validator_ids(&self) -> Vec<u64> {
         self.keys.keys().copied().collect()
     }
 
-    /// Signs an attestation using the validator's attestation key.
     pub fn sign_attestation(
         &mut self,
         validator_id: u64,
         attestation_data: &AttestationData,
     ) -> Result<XmssSignature, KeyManagerError> {
-        let message_hash = attestation_data.tree_hash_root();
+        let message = attestation_data.tree_hash_root();
         let slot = attestation_data.slot as u32;
-        self.sign_with_attestation_key(validator_id, slot, &message_hash)
+        let key_pair = self
+            .keys
+            .get_mut(&validator_id)
+            .ok_or(KeyManagerError::ValidatorKeyNotFound(validator_id))?;
+        let sig = Self::sign_with_key(&mut key_pair.attestation_key, slot, &message)?;
+        metrics::inc_pq_sig_attestation_signatures();
+        Ok(sig)
     }
 
-    /// Signs a block root using the validator's proposal key.
     pub fn sign_block_root(
         &mut self,
         validator_id: u64,
         slot: u32,
         block_root: &H256,
-    ) -> Result<XmssSignature, KeyManagerError> {
-        self.sign_with_proposal_key(validator_id, slot, block_root)
-    }
-
-    fn sign_with_attestation_key(
-        &mut self,
-        validator_id: u64,
-        slot: u32,
-        message: &H256,
     ) -> Result<XmssSignature, KeyManagerError> {
         let key_pair = self
             .keys
             .get_mut(&validator_id)
             .ok_or(KeyManagerError::ValidatorKeyNotFound(validator_id))?;
-
-        let signature: ValidatorSignature = {
-            let _timing = metrics::time_pq_sig_attestation_signing();
-            key_pair
-                .attestation_key
-                .sign(slot, message)
-                .map_err(|e| KeyManagerError::SigningError(e.to_string()))
-        }?;
-        metrics::inc_pq_sig_attestation_signatures();
-
-        let sig_bytes = signature.to_bytes();
-        XmssSignature::try_from(sig_bytes)
-            .map_err(|e| KeyManagerError::SignatureConversionError(e.to_string()))
+        Self::sign_with_key(&mut key_pair.proposal_key, slot, block_root)
     }
 
-    fn sign_with_proposal_key(
-        &mut self,
-        validator_id: u64,
+    fn sign_with_key(
+        key: &mut ValidatorSecretKey,
         slot: u32,
         message: &H256,
     ) -> Result<XmssSignature, KeyManagerError> {
-        let key_pair = self
-            .keys
-            .get_mut(&validator_id)
-            .ok_or(KeyManagerError::ValidatorKeyNotFound(validator_id))?;
-
         let signature: ValidatorSignature = {
             let _timing = metrics::time_pq_sig_attestation_signing();
-            key_pair
-                .proposal_key
-                .sign(slot, message)
+            key.sign(slot, message)
                 .map_err(|e| KeyManagerError::SigningError(e.to_string()))
         }?;
-
-        let sig_bytes = signature.to_bytes();
-        XmssSignature::try_from(sig_bytes)
+        XmssSignature::try_from(signature.to_bytes())
             .map_err(|e| KeyManagerError::SignatureConversionError(e.to_string()))
     }
 }
@@ -133,9 +96,14 @@ mod tests {
     fn test_sign_attestation_validator_not_found() {
         let keys = HashMap::new();
         let mut key_manager = KeyManager::new(keys);
-        let message = H256::default();
-
-        let result = key_manager.sign_with_attestation_key(123, 0, &message);
+        let data = AttestationData {
+            slot: 0,
+            head: Default::default(),
+            target: Default::default(),
+            source: Default::default(),
+        };
+
+        let result = key_manager.sign_attestation(123, &data);
         assert!(matches!(
             result,
             Err(KeyManagerError::ValidatorKeyNotFound(123))