Fix ecrecover and verify methods

crypto/src/signature.rs

@@ -21,7 +21,7 @@ pub const BLS_PUB_LEN: usize = 48;
 #[derive(Clone, Debug, PartialEq, FromPrimitive, Copy, Eq, Serialize_repr, Deserialize_repr)]
 #[repr(u8)]
 pub enum SignatureType {
-    Secp256 = 1,
+    Secp256k1 = 1,
     BLS = 2,
 }
 
@@ -81,7 +81,7 @@ impl Signature {
     /// Creates a SECP Signature given the raw bytes
     pub fn new_secp256k1(bytes: Vec<u8>) -> Self {
         Self {
-            sig_type: SignatureType::Secp256,
+            sig_type: SignatureType::Secp256k1,
             bytes,
         }
     }
@@ -178,21 +178,16 @@ pub fn verify_bls_aggregate(data: &[&[u8]], pub_keys: &[&[u8]], aggregate_sig: &
     verify(&sig, &hashed_data[..], &pks[..])
 }
 
-// TODO: verify signature data format after signing implemented
-fn ecrecover(hash: &[u8; 32], signature: &[u8; 65]) -> Result<Address, Error> {
-    /* Recovery id is the last big-endian byte. */
-    let v = (signature[64] as i8 - 27) as u8;
-    if v != 0 && v != 1 {
-        return Err(Error::InvalidRecovery("invalid recovery byte".to_owned()));
-    }
+/// Return Address for a message given it's hash and signature
+pub fn ecrecover(hash: &[u8; 32], signature: &[u8; 65]) -> Result<Address, Error> {
+    // generate types to recover key from
+    let rec_id = RecoveryId::parse(signature[64])?;
+    let message = Message::parse(&hash);
 
     // Signature value without recovery byte
     let mut s = [0u8; 64];
-    s[..64].clone_from_slice(signature.as_ref());
-
-    // generate types to recover key from
-    let message = Message::parse(&hash);
-    let rec_id = RecoveryId::parse(signature[64])?;
+    s.clone_from_slice(signature[..64].as_ref());
+    // generate Signature
     let sig = EcsdaSignature::parse(&s);
 
     let key = recover(&message, &sig, &rec_id)?;
@@ -204,36 +199,10 @@ fn ecrecover(hash: &[u8; 32], signature: &[u8; 65]) -> Result<Address, Error> {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use address::Address;
     use bls_signatures::{PrivateKey, Serialize, Signature as BlsSignature};
     use rand::rngs::mock::StepRng;
     use rand::Rng;
 
-    #[test]
-    fn bls_verify() {
-        let rng = &mut StepRng::new(8, 3);
-        let sk = PrivateKey::generate(rng);
-
-        let msg = (0..64).map(|_| rng.gen()).collect::<Vec<u8>>();
-        let signature = sk.sign(&msg);
-
-        let signature_bytes = signature.as_bytes();
-        assert_eq!(signature_bytes.len(), 96);
-        assert_eq!(
-            BlsSignature::from_bytes(&signature_bytes).unwrap(),
-            signature
-        );
-
-        let pk = sk.public_key();
-        let addr = Address::new_bls(&pk.as_bytes()).unwrap();
-
-        Signature::new_bls(signature_bytes.clone())
-            .verify(&msg, &addr)
-            .unwrap();
-        Signature::new_bls(signature_bytes.clone())
-            .verify_bls_sig(&msg, &addr)
-            .unwrap();
-    }
     #[test]
     fn bls_agg_verify() {
         // The number of signatures in aggregate

key_management/Cargo.toml

@@ -10,4 +10,5 @@ address = { package = "forest_address", path = "../vm/address", version = "0.2"
 crypto = { package = "forest_crypto", path = "../crypto" }
 bls-signatures = "0.6.0"
 libsecp256k1 = "0.3.4"
-rand = "0.7.3"
+rand = "0.7.3"
+encoding = { package = "forest_encoding", path = "../encoding", version = "0.1" }

key_management/src/wallet.rs

@@ -131,8 +131,8 @@ where
         Ok(())
     }
 
-    /// Generate a new Key that fits the requirement of the given SignatureType
-    pub fn generate_key(&mut self, typ: SignatureType) -> Result<Address, Error> {
+    /// Generate a new Address that fits the requirement of the given SignatureType
+    pub fn generate_addr(&mut self, typ: SignatureType) -> Result<Address, Error> {
         let key = generate_key(typ)?;
         let addr = format!("wallet-{}", key.address.to_string());
         self.keystore.put(addr, key.key_info.clone())?;
@@ -164,14 +164,15 @@ fn generate_key(typ: SignatureType) -> Result<Key, Error> {
 mod tests {
     use super::*;
     use crate::{generate, MemKeyStore};
+    use encoding::blake2b_256;
     use secp256k1::{Message as SecpMessage, SecretKey as SecpPrivate};
 
     fn construct_priv_keys() -> Vec<Key> {
         let mut secp_keys = Vec::new();
         let mut bls_keys = Vec::new();
         for _ in 1..5 {
-            let secp_priv_key = generate(SignatureType::Secp256).unwrap();
-            let secp_key_info = KeyInfo::new(SignatureType::Secp256, secp_priv_key);
+            let secp_priv_key = generate(SignatureType::Secp256k1).unwrap();
+            let secp_key_info = KeyInfo::new(SignatureType::Secp256k1, secp_priv_key);
             let secp_key = Key::try_from(secp_key_info).unwrap();
             secp_keys.push(secp_key);
 
@@ -224,17 +225,18 @@ mod tests {
         let priv_key_bytes = key_vec[2].key_info.private_key().clone();
         let addr = key_vec[2].address.clone();
         let mut wallet = Wallet::new_from_keys(MemKeyStore::new(), key_vec);
-        let msg: [u8; 32] = [
-            1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1,
-            1, 1, 1,
-        ];
+        let msg = [0u8; 64];
 
         let msg_sig = wallet.sign(&addr, &msg).unwrap();
 
+        let msg_complete = blake2b_256(&msg);
+        let message = SecpMessage::parse(&msg_complete);
         let priv_key = SecpPrivate::parse_slice(&priv_key_bytes).unwrap();
-        let message = SecpMessage::parse_slice(&msg).unwrap();
-        let (sig, _) = secp256k1::sign(&message, &priv_key);
-        let actual = Signature::new_secp256k1(sig.serialize().to_vec());
+        let (sig, recovery_id) = secp256k1::sign(&message, &priv_key);
+        let mut new_bytes = [0; 65];
+        new_bytes[..64].copy_from_slice(&sig.serialize());
+        new_bytes[64] = recovery_id.serialize();
+        let actual = Signature::new_secp256k1(new_bytes.to_vec());
         assert_eq!(msg_sig, actual)
     }
 
@@ -248,15 +250,15 @@ mod tests {
         // test to see if export returns the correct key_info
         assert_eq!(key_info, key.key_info.clone());
 
-        let new_priv_key = generate(SignatureType::Secp256).unwrap();
+        let new_priv_key = generate(SignatureType::Secp256k1).unwrap();
         let pub_key =
-            wallet_helpers::to_public(SignatureType::Secp256, new_priv_key.as_slice()).unwrap();
+            wallet_helpers::to_public(SignatureType::Secp256k1, new_priv_key.as_slice()).unwrap();
         let test_addr = Address::new_secp256k1(pub_key.as_slice()).unwrap();
         let key_info_err = wallet.export(&test_addr).unwrap_err();
         // test to make sure that an error is raised when an incorrect address is added
         assert_eq!(key_info_err, Error::KeyInfo);
 
-        let test_key_info = KeyInfo::new(SignatureType::Secp256, new_priv_key);
+        let test_key_info = KeyInfo::new(SignatureType::Secp256k1, new_priv_key);
         // make sure that key_info has been imported to wallet
         assert!(wallet.import(test_key_info.clone()).is_ok());
 
@@ -299,7 +301,7 @@ mod tests {
     #[test]
     fn generate_new_key() {
         let mut wallet = generate_wallet();
-        let addr = wallet.generate_key(SignatureType::BLS).unwrap();
+        let addr = wallet.generate_addr(SignatureType::BLS).unwrap();
         let key = wallet.keystore.get("default").unwrap();
         // make sure that the newly generated key is the default key - checking by key type
         assert_eq!(&SignatureType::BLS, key.key_type());
@@ -320,12 +322,12 @@ mod tests {
         // check to make sure that there is no default
         assert_eq!(wallet.get_default().unwrap_err(), Error::KeyInfo);
 
-        let new_priv_key = generate(SignatureType::Secp256).unwrap();
+        let new_priv_key = generate(SignatureType::Secp256k1).unwrap();
         let pub_key =
-            wallet_helpers::to_public(SignatureType::Secp256, new_priv_key.as_slice()).unwrap();
+            wallet_helpers::to_public(SignatureType::Secp256k1, new_priv_key.as_slice()).unwrap();
         let test_addr = Address::new_secp256k1(pub_key.as_slice()).unwrap();
 
-        let key_info = KeyInfo::new(SignatureType::Secp256, new_priv_key);
+        let key_info = KeyInfo::new(SignatureType::Secp256k1, new_priv_key);
         let test_addr_string = format!("wallet-{}", test_addr.to_string());
 
         wallet.keystore.put(test_addr_string, key_info).unwrap();
@@ -336,4 +338,40 @@ mod tests {
         // check to make sure that the test_addr is actually the default addr for the wallet
         assert_eq!(wallet.get_default().unwrap(), test_addr);
     }
+
+    #[test]
+    fn secp_verify() {
+        let secp_priv_key = generate(SignatureType::Secp256k1).unwrap();
+        let secp_key_info = KeyInfo::new(SignatureType::Secp256k1, secp_priv_key);
+        let secp_key = Key::try_from(secp_key_info).unwrap();
+        let addr = secp_key.address.clone();
+        let mut wallet = Wallet::new_from_keys(MemKeyStore::new(), vec![secp_key]);
+
+        let msg = [0u8; 64];
+
+        let sig = wallet.sign(&addr, &msg).unwrap();
+        sig.verify(&msg, &addr).unwrap();
+
+        // invalid verify check
+        let invalid_addr = wallet.generate_addr(SignatureType::Secp256k1).unwrap();
+        assert!(sig.verify(&msg, &invalid_addr).is_err())
+    }
+
+    #[test]
+    fn bls_verify_test() {
+        let bls_priv_key = generate(SignatureType::BLS).unwrap();
+        let bls_key_info = KeyInfo::new(SignatureType::BLS, bls_priv_key);
+        let bls_key = Key::try_from(bls_key_info).unwrap();
+        let addr = bls_key.address.clone();
+        let mut wallet = Wallet::new_from_keys(MemKeyStore::new(), vec![bls_key]);
+
+        let msg = [0u8; 64];
+
+        let sig = wallet.sign(&addr, &msg).unwrap();
+        sig.verify(&msg, &addr).unwrap();
+
+        // invalid verify check
+        let invalid_addr = wallet.generate_addr(SignatureType::BLS).unwrap();
+        assert!(sig.verify(&msg, &invalid_addr).is_err())
+    }
 }

key_management/src/wallet_helpers.rs

@@ -5,6 +5,7 @@ use super::errors::Error;
 use address::Address;
 use bls_signatures::{PrivateKey as BlsPrivate, Serialize};
 use crypto::{Signature, SignatureType};
+use encoding::blake2b_256;
 use rand::rngs::OsRng;
 use secp256k1::{Message as SecpMessage, PublicKey as SecpPublic, SecretKey as SecpPrivate};
 
@@ -15,7 +16,7 @@ pub fn to_public(sig_type: SignatureType, private_key: &[u8]) -> Result<Vec<u8>,
             .map_err(|err| Error::Other(err.to_string()))?
             .public_key()
             .as_bytes()),
-        SignatureType::Secp256 => {
+        SignatureType::Secp256k1 => {
             let private_key = SecpPrivate::parse_slice(private_key)
                 .map_err(|err| Error::Other(err.to_string()))?;
             let public_key = SecpPublic::from_secret_key(&private_key);
@@ -31,7 +32,7 @@ pub fn new_address(sig_type: SignatureType, public_key: &[u8]) -> Result<Address
             let addr = Address::new_bls(public_key).map_err(|err| Error::Other(err.to_string()))?;
             Ok(addr)
         }
-        SignatureType::Secp256 => {
+        SignatureType::Secp256k1 => {
             let addr =
                 Address::new_secp256k1(public_key).map_err(|err| Error::Other(err.to_string()))?;
             Ok(addr)
@@ -50,14 +51,16 @@ pub fn sign(sig_type: SignatureType, private_key: &[u8], msg: &[u8]) -> Result<S
             let crypto_sig = Signature::new_bls(sig.as_bytes());
             Ok(crypto_sig)
         }
-        SignatureType::Secp256 => {
+        SignatureType::Secp256k1 => {
             let priv_key = SecpPrivate::parse_slice(private_key)
                 .map_err(|err| Error::Other(err.to_string()))?;
-            let message =
-                SecpMessage::parse_slice(msg).map_err(|err| Error::Other(err.to_string()))?;
-            // this returns a signature of secp256k1 type, next lines convert this sig to crypto signature type
-            let (sig, _) = secp256k1::sign(&message, &priv_key);
-            let crypto_sig = Signature::new_secp256k1(sig.serialize().to_vec());
+            let msg_complete = blake2b_256(msg);
+            let message = SecpMessage::parse(&msg_complete);
+            let (sig, recovery_id) = secp256k1::sign(&message, &priv_key);
+            let mut new_bytes = [0; 65];
+            new_bytes[..64].copy_from_slice(&sig.serialize());
+            new_bytes[64] = recovery_id.serialize();
+            let crypto_sig = Signature::new_secp256k1(new_bytes.to_vec());
             Ok(crypto_sig)
         }
     }
@@ -71,7 +74,7 @@ pub fn generate(sig_type: SignatureType) -> Result<Vec<u8>, Error> {
             let key = BlsPrivate::generate(rng);
             Ok(key.as_bytes())
         }
-        SignatureType::Secp256 => {
+        SignatureType::Secp256k1 => {
             let key = SecpPrivate::random(rng);
             Ok(key.serialize().to_vec())
         }

vm/interpreter/src/gas_tracker/price_list.rs

@@ -122,7 +122,7 @@ impl PriceList {
     pub fn on_verify_signature(&self, sig_type: SignatureType, plain_text_size: usize) -> i64 {
         match sig_type {
             SignatureType::BLS => (3 * plain_text_size + 2) as i64,
-            SignatureType::Secp256 => (3 * plain_text_size + 2) as i64,
+            SignatureType::Secp256k1 => (3 * plain_text_size + 2) as i64,
         }
     }
     /// Returns gas required for hashing data