Use BCS for signature

fastpay/src/bench.rs

@@ -166,7 +166,7 @@ impl ClientServerBenchmark {
             };
             for i in 0..committee.quorum_threshold() {
                 let (pubx, secx) = keys.get(i).unwrap();
-                let sig = Signature::new(&certificate.value, secx);
+                let sig = Signature::new(&certificate.value.transfer, secx);
                 certificate.signatures.push((*pubx, sig));
             }
 

fastpay/src/client.rs

@@ -145,7 +145,7 @@ fn make_benchmark_certificates_from_orders_and_server_configs(
         };
         for i in 0..committee.quorum_threshold() {
             let (pubx, secx) = keys.get(i).unwrap();
-            let sig = Signature::new(&certificate.value, secx);
+            let sig = Signature::new(&certificate.value.transfer, secx);
             certificate.signatures.push((*pubx, sig));
         }
         let serialized_certificate = serialize_cert(&certificate);

fastpay_core/Cargo.toml

@@ -7,6 +7,7 @@ edition = "2018"
 
 [dependencies]
 base64 = "0.12.3"
+bcs = "0.1.3"
 bincode = "1.3.1"
 failure = "0.1.8"
 futures = "0.3.5"
@@ -16,6 +17,7 @@ tokio = { version = "0.2.22", features = ["full"] }
 ed25519 = { version = "1.0.1"}
 ed25519-dalek = { version = "1.0.1", features = ["batch", "serde"] }
 serde-reflection = "0.3.2"
+serde-name = "0.1.2"
 serde_yaml = "0.8.17"
 structopt = "0.3.21"
 

fastpay_core/src/base_types.rs

@@ -268,33 +268,37 @@ impl From<SequenceNumber> for usize {
     }
 }
 
-pub trait Digestible {
-    fn digest(&self) -> [u8; 32];
+/// Something that we know how to hash and sign.
+pub trait Signable<Hasher> {
+    fn write(&self, hasher: &mut Hasher);
 }
 
-#[cfg(test)]
-impl Digestible for [u8; 5] {
-    fn digest(self: &[u8; 5]) -> [u8; 32] {
-        use dalek::Digest;
-
-        let mut h = dalek::Sha512::new();
-        let mut hash = [0u8; 64];
-        let mut digest = [0u8; 32];
-        h.update(&self);
-        hash.copy_from_slice(h.finalize().as_slice());
-        digest.copy_from_slice(&hash[..32]);
-        digest
+/// Activate the blanket implementation of `Signable` based on serde and BCS.
+/// * We use `serde_name` to extract a seed from the name of structs and enums.
+/// * We use `BCS` to generate canonical bytes suitable for hashing and signing.
+pub trait BcsSignable: Serialize + serde::de::DeserializeOwned {}
+
+impl<T, Hasher> Signable<Hasher> for T
+where
+    T: BcsSignable,
+    Hasher: std::io::Write,
+{
+    fn write(&self, hasher: &mut Hasher) {
+        let name = serde_name::trace_name::<Self>().expect("Self must be a struct or an enum");
+        // Note: This assumes that names never contain the separator `::`.
+        write!(hasher, "{}::", name).expect("Hasher should not fail");
+        bcs::serialize_into(hasher, &self).expect("Message serialization should not fail");
     }
 }
 
 impl Signature {
     pub fn new<T>(value: &T, secret: &SecretKey) -> Self
     where
-        T: Digestible,
+        T: Signable<Vec<u8>>,
     {
-        let message = value.digest();
-        let key_pair = &secret.0;
-        let signature = key_pair.sign(&message);
+        let mut message = Vec::new();
+        value.write(&mut message);
+        let signature = secret.0.sign(&message);
         Signature(signature)
     }
 
@@ -304,16 +308,17 @@ impl Signature {
         author: FastPayAddress,
     ) -> Result<(), dalek::SignatureError>
     where
-        T: Digestible,
+        T: Signable<Vec<u8>>,
     {
-        let message = value.digest();
+        let mut message = Vec::new();
+        value.write(&mut message);
         let public_key = dalek::PublicKey::from_bytes(&author.0)?;
         public_key.verify(&message, &self.0)
     }
 
     pub fn check<T>(&self, value: &T, author: FastPayAddress) -> Result<(), FastPayError>
     where
-        T: Digestible,
+        T: Signable<Vec<u8>>,
     {
         self.check_internal(value, author)
             .map_err(|error| FastPayError::InvalidSignature {
@@ -323,15 +328,16 @@ impl Signature {
 
     fn verify_batch_internal<'a, T, I>(value: &'a T, votes: I) -> Result<(), dalek::SignatureError>
     where
-        T: Digestible,
+        T: Signable<Vec<u8>>,
         I: IntoIterator<Item = &'a (FastPayAddress, Signature)>,
     {
-        let msg: &[u8] = &value.digest();
+        let mut msg = Vec::new();
+        value.write(&mut msg);
         let mut messages: Vec<&[u8]> = Vec::new();
         let mut signatures: Vec<dalek::Signature> = Vec::new();
         let mut public_keys: Vec<dalek::PublicKey> = Vec::new();
         for (addr, sig) in votes.into_iter() {
-            messages.push(msg);
+            messages.push(&msg);
             signatures.push(sig.0);
             public_keys.push(dalek::PublicKey::from_bytes(&addr.0)?);
         }
@@ -340,7 +346,7 @@ impl Signature {
 
     pub fn verify_batch<'a, T, I>(value: &'a T, votes: I) -> Result<(), FastPayError>
     where
-        T: Digestible,
+        T: Signable<Vec<u8>>,
         I: IntoIterator<Item = &'a (FastPayAddress, Signature)>,
     {
         Signature::verify_batch_internal(value, votes).map_err(|error| {

fastpay_core/src/messages.rs

@@ -3,8 +3,6 @@
 
 use super::{base_types::*, committee::Committee, error::*};
 
-use ed25519_dalek::{Digest, Sha512};
-
 #[cfg(test)]
 #[path = "unit_tests/messages_tests.rs"]
 mod messages_tests;
@@ -163,48 +161,10 @@ impl TransferOrder {
     }
 }
 
-impl Digestible for Transfer {
-    fn digest(self: &Transfer) -> [u8; 32] {
-        let mut h: Sha512 = Sha512::new();
-        let mut hash: [u8; 64] = [0u8; 64];
-        let mut digest: [u8; 32] = [0u8; 32];
-
-        h.update(&self.sender.0);
-        match self.recipient {
-            Address::Primary(addr) => {
-                h.update([0x03]);
-                h.update(&addr.0);
-            }
-            Address::FastPay(addr) => {
-                h.update([0x04]);
-                h.update(&addr.0);
-            }
-        }
-        h.update(u64::from(self.amount).to_le_bytes());
-        h.update(u64::from(self.sequence_number).to_le_bytes());
-        match self.user_data.0 {
-            None => h.update([0x00]),
-            Some(data) => {
-                h.update([0x01]);
-                h.update(data);
-            }
-        }
-        hash.copy_from_slice(h.finalize().as_slice());
-        digest.copy_from_slice(&hash[..32]);
-        digest
-    }
-}
-
-impl Digestible for TransferOrder {
-    fn digest(self: &TransferOrder) -> [u8; 32] {
-        self.transfer.digest()
-    }
-}
-
 impl SignedTransferOrder {
     /// Use signing key to create a signed object.
     pub fn new(value: TransferOrder, authority: AuthorityName, secret: &SecretKey) -> Self {
-        let signature = Signature::new(&value, secret);
+        let signature = Signature::new(&value.transfer, secret);
         Self {
             value,
             authority,
@@ -217,7 +177,7 @@ impl SignedTransferOrder {
         self.value.check_signature()?;
         let weight = committee.weight(&self.authority);
         fp_ensure!(weight > 0, FastPayError::UnknownSigner);
-        self.signature.check(&self.value, self.authority)?;
+        self.signature.check(&self.value.transfer, self.authority)?;
         Ok(weight)
     }
 }
@@ -257,7 +217,7 @@ impl<'a> SignatureAggregator<'a> {
         authority: AuthorityName,
         signature: Signature,
     ) -> Result<Option<CertifiedTransferOrder>, FastPayError> {
-        signature.check(&self.partial.value, authority)?;
+        signature.check(&self.partial.value.transfer, authority)?;
         // Check that each authority only appears once.
         fp_ensure!(
             !self.used_authorities.contains(&authority),
@@ -309,7 +269,7 @@ impl CertifiedTransferOrder {
         // All what is left is checking signatures!
         let inner_sig = (self.value.transfer.sender, self.value.signature);
         Signature::verify_batch(
-            &self.value,
+            &self.value.transfer,
             std::iter::once(&inner_sig).chain(&self.signatures),
         )
     }
@@ -330,3 +290,5 @@ impl ConfirmationOrder {
         }
     }
 }
+
+impl BcsSignable for Transfer {}

fastpay_core/src/unit_tests/base_types_tests.rs

@@ -1,17 +1,34 @@
 // Copyright (c) Facebook Inc.
 // SPDX-License-Identifier: Apache-2.0
 
+#![allow(clippy::blacklisted_name)]
+
 use super::*;
 
+#[derive(Serialize, Deserialize)]
+struct Foo(String);
+
+impl BcsSignable for Foo {}
+
+#[derive(Serialize, Deserialize)]
+struct Bar(String);
+
+impl BcsSignable for Bar {}
+
 #[test]
-fn test_fake_signatures() {
+fn test_signatures() {
     let (addr1, sec1) = get_key_pair();
     let (addr2, _sec2) = get_key_pair();
 
-    let s = Signature::new(b"hello", &sec1);
-    assert!(s.check(b"hello", addr1).is_ok());
-    assert!(s.check(b"hello", addr2).is_err());
-    assert!(s.check(b"hellx", addr1).is_err());
+    let foo = Foo("hello".into());
+    let foox = Foo("hellox".into());
+    let bar = Bar("hello".into());
+
+    let s = Signature::new(&foo, &sec1);
+    assert!(s.check(&foo, addr1).is_ok());
+    assert!(s.check(&foo, addr2).is_err());
+    assert!(s.check(&foox, addr1).is_err());
+    assert!(s.check(&bar, addr1).is_err());
 }
 
 #[test]

fastpay_core/src/unit_tests/serialize_tests.rs

@@ -137,7 +137,7 @@ fn test_cert() {
 
     for _ in 0..3 {
         let (authority_name, authority_key) = get_key_pair();
-        let sig = Signature::new(&cert.value, &authority_key);
+        let sig = Signature::new(&cert.value.transfer, &authority_key);
 
         cert.signatures.push((authority_name, sig));
     }
@@ -174,7 +174,7 @@ fn test_info_response() {
 
     for _ in 0..3 {
         let (authority_name, authority_key) = get_key_pair();
-        let sig = Signature::new(&cert.value, &authority_key);
+        let sig = Signature::new(&cert.value.transfer, &authority_key);
 
         cert.signatures.push((authority_name, sig));
     }
@@ -283,7 +283,9 @@ fn test_time_vote() {
     let now = Instant::now();
     for _ in 0..100 {
         if let SerializedMessage::Vote(vote) = deserialize_message(&mut buf2).unwrap() {
-            vote.signature.check(&vote.value, vote.authority).unwrap();
+            vote.signature
+                .check(&vote.value.transfer, vote.authority)
+                .unwrap();
         }
     }
     assert!(deserialize_message(&mut buf2).is_err());
@@ -312,7 +314,7 @@ fn test_time_cert() {
 
     for _ in 0..7 {
         let (authority_name, authority_key) = get_key_pair();
-        let sig = Signature::new(&cert.value, &authority_key);
+        let sig = Signature::new(&cert.value.transfer, &authority_key);
         cert.signatures.push((authority_name, sig));
     }
 
@@ -328,7 +330,7 @@ fn test_time_cert() {
     let mut buf2 = buf.as_slice();
     for _ in 0..count {
         if let SerializedMessage::Cert(cert) = deserialize_message(&mut buf2).unwrap() {
-            Signature::verify_batch(&cert.value, &cert.signatures).unwrap();
+            Signature::verify_batch(&cert.value.transfer, &cert.signatures).unwrap();
         }
     }
     assert!(deserialize_message(buf2).is_err());