Wrap BLS keypair, pubkey and privkey in newtypes

beacon_chain/attestation_validation/src/signature.rs

@@ -27,7 +27,7 @@ where
                 .get_public_key_by_index(validator)?
                 .ok_or(Error::NoPublicKeyForValidator)?;
             // Aggregate the public key.
-            agg_pub_key.add(&pub_key);
+            agg_pub_key.add(&pub_key.as_raw());
         }
     }
 
@@ -129,7 +129,8 @@ mod tests {
             &agg_sig,
             &attestation_indices,
             &store,
-        ).unwrap();
+        )
+        .unwrap();
         assert_eq!(outcome, Outcome::Valid);
 
         /*
@@ -143,7 +144,8 @@ mod tests {
             &agg_sig,
             &attestation_indices,
             &store,
-        ).unwrap();
+        )
+        .unwrap();
         assert_eq!(outcome, Outcome::Invalid(Invalid::SignatureInvalid));
     }
 }

beacon_chain/utils/bls/src/keypair.rs

@@ -0,0 +1,16 @@
+use super::{PublicKey, SecretKey};
+
+#[derive(Debug, Clone, PartialEq)]
+pub struct Keypair {
+    pub sk: SecretKey,
+    pub pk: PublicKey,
+}
+
+impl Keypair {
+    /// Instantiate a Keypair using SecretKey::random().
+    pub fn random() -> Self {
+        let sk = SecretKey::random();
+        let pk = PublicKey::from_secret_key(&sk);
+        Keypair { sk, pk }
+    }
+}

beacon_chain/utils/bls/src/lib.rs

@@ -3,16 +3,18 @@ extern crate hashing;
 extern crate ssz;
 
 mod aggregate_signature;
+mod keypair;
 mod public_key;
+mod secret_key;
 mod signature;
 
 pub use crate::aggregate_signature::AggregateSignature;
-pub use crate::signature::Signature;
+pub use crate::keypair::Keypair;
 pub use crate::public_key::PublicKey;
+pub use crate::secret_key::SecretKey;
+pub use crate::signature::Signature;
 
 pub use self::bls_aggregates::AggregatePublicKey;
-pub use self::bls_aggregates::Keypair;
-pub use self::bls_aggregates::SecretKey;
 
 pub const BLS_AGG_SIG_BYTE_SIZE: usize = 97;
 
@@ -34,7 +36,7 @@ pub fn verify_proof_of_possession(sig: &Signature, pubkey: &PublicKey) -> bool {
 }
 
 pub fn create_proof_of_possession(keypair: &Keypair) -> Signature {
-    let mut hash = canonical_hash(&keypair.pk.as_bytes());
+    let mut hash = canonical_hash(&ssz_encode(&keypair.pk));
     extend_if_needed(&mut hash);
     Signature::new_hashed(&hash, &keypair.sk)
 }

beacon_chain/utils/bls/src/public_key.rs

@@ -11,7 +11,7 @@ pub struct PublicKey(RawPublicKey);
 
 impl PublicKey {
     pub fn from_secret_key(secret_key: &SecretKey) -> Self {
-        PublicKey(RawPublicKey::from_secret_key(secret_key))
+        PublicKey(RawPublicKey::from_secret_key(secret_key.as_raw()))
     }
 
     /// Returns the underlying signature.

beacon_chain/utils/bls/src/secret_key.rs

@@ -0,0 +1,59 @@
+use bls_aggregates::{DecodeError as BlsDecodeError, SecretKey as RawSecretKey};
+use ssz::{decode_ssz_list, Decodable, DecodeError, Encodable, SszStream};
+
+/// A single BLS signature.
+///
+/// This struct is a wrapper upon a base type and provides helper functions (e.g., SSZ
+/// serialization).
+#[derive(Debug, PartialEq, Clone)]
+pub struct SecretKey(RawSecretKey);
+
+impl SecretKey {
+    pub fn random() -> Self {
+        SecretKey(RawSecretKey::random())
+    }
+
+    /// Instantiate a SecretKey from existing bytes.
+    ///
+    /// Note: this is _not_ SSZ decoding.
+    pub fn from_bytes(bytes: &[u8]) -> Result<SecretKey, BlsDecodeError> {
+        Ok(SecretKey(RawSecretKey::from_bytes(bytes)?))
+    }
+
+    /// Returns the underlying secret key.
+    pub fn as_raw(&self) -> &RawSecretKey {
+        &self.0
+    }
+}
+
+impl Encodable for SecretKey {
+    fn ssz_append(&self, s: &mut SszStream) {
+        s.append_vec(&self.0.as_bytes());
+    }
+}
+
+impl Decodable for SecretKey {
+    fn ssz_decode(bytes: &[u8], i: usize) -> Result<(Self, usize), DecodeError> {
+        let (sig_bytes, i) = decode_ssz_list(bytes, i)?;
+        let raw_sig = RawSecretKey::from_bytes(&sig_bytes).map_err(|_| DecodeError::TooShort)?;
+        Ok((SecretKey(raw_sig), i))
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::super::ssz::ssz_encode;
+    use super::*;
+
+    #[test]
+    pub fn test_ssz_round_trip() {
+        let original =
+            SecretKey::from_bytes("jzjxxgjajfjrmgodszzsgqccmhnyvetcuxobhtynojtpdtbj".as_bytes())
+                .unwrap();
+
+        let bytes = ssz_encode(&original);
+        let (decoded, _) = SecretKey::ssz_decode(&bytes, 0).unwrap();
+
+        assert_eq!(original, decoded);
+    }
+}

beacon_chain/utils/bls/src/signature.rs

@@ -1,6 +1,6 @@
 use super::ssz::{decode_ssz_list, Decodable, DecodeError, Encodable, SszStream};
-use super::PublicKey;
+use super::{PublicKey, SecretKey};
-use bls_aggregates::{SecretKey, Signature as RawSignature};
+use bls_aggregates::Signature as RawSignature;
 
 /// A single BLS signature.
 ///
@@ -12,13 +12,13 @@ pub struct Signature(RawSignature);
 impl Signature {
     /// Instantiate a new Signature from a message and a SecretKey.
     pub fn new(msg: &[u8], sk: &SecretKey) -> Self {
-        Signature(RawSignature::new(msg, sk))
+        Signature(RawSignature::new(msg, sk.as_raw()))
     }
 
     /// Instantiate a new Signature from a message and a SecretKey, where the message has already
     /// been hashed.
     pub fn new_hashed(msg_hashed: &[u8], sk: &SecretKey) -> Self {
-        Signature(RawSignature::new_hashed(msg_hashed, sk))
+        Signature(RawSignature::new_hashed(msg_hashed, sk.as_raw()))
     }
 
     /// Verify the Signature against a PublicKey.

beacon_chain/validator_change/Cargo.toml

@@ -7,4 +7,5 @@ edition = "2018"
 [dependencies]
 bytes = "0.4.10"
 hashing = { path = "../utils/hashing" }
+ssz = { path = "../utils/ssz" }
 types = { path = "../types" }

beacon_chain/validator_change/src/lib.rs

@@ -4,6 +4,7 @@ extern crate types;
 
 use bytes::{BufMut, BytesMut};
 use hashing::canonical_hash;
+use ssz::ssz_encode;
 use std::cmp::max;
 use types::{Hash256, ValidatorRecord, ValidatorStatus};
 
@@ -70,7 +71,7 @@ pub fn update_validator_set(
                  */
                 if new_total_changed <= max_allowable_change {
                     v.status = ValidatorStatus::Active;
-                    hasher.extend(i, &v.pubkey.as_bytes(), VALIDATOR_FLAG_ENTRY);
+                    hasher.extend(i, &ssz_encode(&v.pubkey), VALIDATOR_FLAG_ENTRY);
                     total_changed = new_total_changed;
                 } else {
                     // Entering the validator would exceed the balance delta.
@@ -91,7 +92,7 @@ pub fn update_validator_set(
                 if new_total_changed <= max_allowable_change {
                     v.status = ValidatorStatus::PendingWithdraw;
                     v.exit_slot = present_slot;
-                    hasher.extend(i, &v.pubkey.as_bytes(), VALIDATOR_FLAG_EXIT);
+                    hasher.extend(i, &ssz_encode(&v.pubkey), VALIDATOR_FLAG_EXIT);
                     total_changed = new_total_changed;
                 } else {
                     // Exiting the validator would exceed the balance delta.

lighthouse/db/src/stores/validator_store.rs

@@ -4,6 +4,7 @@ use self::bytes::{BufMut, BytesMut};
 use super::bls::PublicKey;
 use super::VALIDATOR_DB_COLUMN as DB_COLUMN;
 use super::{ClientDB, DBError};
+use ssz::{ssz_encode, Decodable};
 use std::sync::Arc;
 
 #[derive(Debug, PartialEq)]
@@ -54,7 +55,7 @@ impl<T: ClientDB> ValidatorStore<T> {
         public_key: &PublicKey,
     ) -> Result<(), ValidatorStoreError> {
         let key = self.get_db_key_for_index(&KeyPrefixes::PublicKey, index);
-        let val = public_key.as_bytes();
+        let val = ssz_encode(public_key);
         self.db
             .put(DB_COLUMN, &key[..], &val[..])
             .map_err(ValidatorStoreError::from)
@@ -68,8 +69,8 @@ impl<T: ClientDB> ValidatorStore<T> {
         let val = self.db.get(DB_COLUMN, &key[..])?;
         match val {
             None => Ok(None),
-            Some(val) => match PublicKey::from_bytes(&val) {
+            Some(val) => match PublicKey::ssz_decode(&val, 0) {
-                Ok(key) => Ok(Some(key)),
+                Ok((key, _)) => Ok(Some(key)),
                 Err(_) => Err(ValidatorStoreError::DecodeError),
             },
         }
@@ -87,7 +88,10 @@ mod tests {
         let db = Arc::new(MemoryDB::open());
         let store = ValidatorStore::new(db.clone());
 
-        assert_eq!(store.prefix_bytes(&KeyPrefixes::PublicKey), b"pubkey".to_vec());
+        assert_eq!(
+            store.prefix_bytes(&KeyPrefixes::PublicKey),
+            b"pubkey".to_vec()
+        );
     }
 
     #[test]
@@ -98,7 +102,10 @@ mod tests {
         let mut buf = BytesMut::with_capacity(6 + 8);
         buf.put(b"pubkey".to_vec());
         buf.put_u64_be(42);
-        assert_eq!(store.get_db_key_for_index(&KeyPrefixes::PublicKey, 42), buf.take().to_vec())
+        assert_eq!(
+            store.get_db_key_for_index(&KeyPrefixes::PublicKey, 42),
+            buf.take().to_vec()
+        )
     }
 
     #[test]
@@ -110,12 +117,15 @@ mod tests {
         let public_key = Keypair::random().pk;
 
         store.put_public_key_by_index(index, &public_key).unwrap();
-        let public_key_at_index = db.get(
+        let public_key_at_index = db
-            DB_COLUMN,
+            .get(
-            &store.get_db_key_for_index(&KeyPrefixes::PublicKey, index)[..]
+                DB_COLUMN,
-        ).unwrap().unwrap();
+                &store.get_db_key_for_index(&KeyPrefixes::PublicKey, index)[..],
+            )
+            .unwrap()
+            .unwrap();
 
-        assert_eq!(public_key_at_index, public_key.as_bytes());
+        assert_eq!(public_key_at_index, ssz_encode(&public_key));
     }
 
     #[test]
@@ -129,8 +139,9 @@ mod tests {
         db.put(
             DB_COLUMN,
             &store.get_db_key_for_index(&KeyPrefixes::PublicKey, index)[..],
-            &public_key.as_bytes()[..]
+            &ssz_encode(&public_key)[..],
-        ).unwrap();
+        )
+        .unwrap();
 
         let public_key_at_index = store.get_public_key_by_index(index).unwrap().unwrap();
         assert_eq!(public_key_at_index, public_key);
@@ -146,8 +157,9 @@ mod tests {
         db.put(
             DB_COLUMN,
             &store.get_db_key_for_index(&KeyPrefixes::PublicKey, 3)[..],
-            &public_key.as_bytes()[..]
+            &ssz_encode(&public_key)[..],
-        ).unwrap();
+        )
+        .unwrap();
 
         let public_key_at_index = store.get_public_key_by_index(4).unwrap();
         assert_eq!(public_key_at_index, None);
@@ -195,11 +207,9 @@ mod tests {
         /*
          * Check that an index that wasn't stored returns None.
          */
-        assert!(
+        assert!(store
-            store
+            .get_public_key_by_index(keys.len() + 1)
-                .get_public_key_by_index(keys.len() + 1)
+            .unwrap()
-                .unwrap()
+            .is_none());
-                .is_none()
-        );
     }
 }