From 2c734f0d61b8b0c94b8217b030cfb3d7dbb8153e Mon Sep 17 00:00:00 2001
From: Georgios Konstantopoulos <me@gakonst.com>
Date: Wed, 17 Jun 2020 09:45:15 +0300
Subject: [PATCH] fix(core): make Signature.v a u64 instead of a u8 and expose
 `verify` (#21)

u8 would not work if a large chain_id was used
---
 ethers-core/src/types/crypto/keys.rs      |  2 +-
 ethers-core/src/types/crypto/signature.rs | 66 ++++++++++++++++++-----
 ethers/examples/sign.rs                   |  6 +--
 3 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/ethers-core/src/types/crypto/keys.rs b/ethers-core/src/types/crypto/keys.rs
index 370163e7..5dafc41b 100644
--- a/ethers-core/src/types/crypto/keys.rs
+++ b/ethers-core/src/types/crypto/keys.rs
@@ -165,7 +165,7 @@ impl PrivateKey {
         let r = H256::from_slice(&signature.r.b32());
         let s = H256::from_slice(&signature.s.b32());
 
-        Signature { v: v as u8, r, s }
+        Signature { v, r, s }
     }
 }
 
diff --git a/ethers-core/src/types/crypto/signature.rs b/ethers-core/src/types/crypto/signature.rs
index a5f420fc..9a70d7ec 100644
--- a/ethers-core/src/types/crypto/signature.rs
+++ b/ethers-core/src/types/crypto/signature.rs
@@ -4,13 +4,14 @@ use crate::{
     utils::hash_message,
 };
 
-use rustc_hex::ToHex;
+use rustc_hex::{FromHex, ToHex};
 use secp256k1::{
     self as Secp256k1, Error as Secp256k1Error, Message, RecoveryId,
     Signature as RecoverableSignature,
 };
 use serde::{Deserialize, Serialize};
-use std::{convert::TryFrom, fmt};
+use std::{convert::TryFrom, fmt, str::FromStr};
+
 use thiserror::Error;
 
 /// An error involving a signature.
@@ -22,6 +23,13 @@ pub enum SignatureError {
     /// Invalid length, secp256k1 signatures are 65 bytes
     #[error("invalid signature length, got {0}, expected 65")]
     InvalidLength(usize),
+    /// When parsing a signature from string to hex
+    #[error(transparent)]
+    DecodingError(#[from] rustc_hex::FromHexError),
+    /// Thrown when signature verification failed (i.e. when the address that
+    /// produced the signature did not match the expected address)
+    #[error("Signature verification failed. Expected {0}, got {0}")]
+    VerificationError(Address, Address),
 }
 
 /// Recovery message data.
@@ -45,7 +53,7 @@ pub struct Signature {
     /// S Value
     pub s: H256,
     /// V value in 'Electrum' notation.
-    pub v: u8,
+    pub v: u64,
 }
 
 impl fmt::Display for Signature {
@@ -56,6 +64,21 @@ impl fmt::Display for Signature {
 }
 
 impl Signature {
+    /// Verifies that signature on `message` was produced by `address`
+    pub fn verify<M, A>(&self, message: M, address: A) -> Result<(), SignatureError>
+    where
+        M: Into<RecoveryMessage>,
+        A: Into<Address>,
+    {
+        let address = address.into();
+        let recovered = self.recover(message)?;
+        if recovered != address {
+            return Err(SignatureError::VerificationError(address, recovered));
+        }
+
+        Ok(())
+    }
+
     /// Recovers the Ethereum address which was used to sign the given message.
     ///
     /// Recovery signature data uses 'Electrum' notation, this means the `v`
@@ -92,13 +115,7 @@ impl Signature {
 
     /// Retrieve the recovery ID.
     fn recovery_id(&self) -> Result<RecoveryId, SignatureError> {
-        let standard_v = match self.v {
-            27 => 0,
-            28 => 1,
-            v if v >= 35 => ((v - 1) % 2) as _,
-            _ => 4,
-        };
-
+        let standard_v = normalize_recovery_id(self.v);
         Ok(RecoveryId::parse(standard_v)?)
     }
 
@@ -108,6 +125,17 @@ impl Signature {
     }
 }
 
+fn normalize_recovery_id(v: u64) -> u8 {
+    match v {
+        0 => 0,
+        1 => 1,
+        27 => 0,
+        28 => 1,
+        v if v >= 35 => ((v - 1) % 2) as _,
+        _ => 4,
+    }
+}
+
 impl<'a> TryFrom<&'a [u8]> for Signature {
     type Error = SignatureError;
 
@@ -123,7 +151,16 @@ impl<'a> TryFrom<&'a [u8]> for Signature {
         let r = H256::from_slice(&bytes[0..32]);
         let s = H256::from_slice(&bytes[32..64]);
 
-        Ok(Signature { r, s, v })
+        Ok(Signature { r, s, v: v.into() })
+    }
+}
+
+impl FromStr for Signature {
+    type Err = SignatureError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let bytes = s.from_hex::<Vec<u8>>()?;
+        Signature::try_from(&bytes[..])
     }
 }
 
@@ -132,7 +169,9 @@ impl From<&Signature> for [u8; 65] {
         let mut sig = [0u8; 65];
         sig[..32].copy_from_slice(src.r.as_bytes());
         sig[32..64].copy_from_slice(src.s.as_bytes());
-        sig[64] = src.v;
+        // TODO: What if we try to serialize a signature where
+        // the `v` is not normalized?
+        sig[64] = src.v as u8;
         sig
     }
 }
@@ -212,6 +251,9 @@ mod tests {
         // if provided with a hash, it will skip hashing
         let recovered2 = signature.recover(hash).unwrap();
 
+        // verifies the signature is produced by `address`
+        signature.verify(message, address).unwrap();
+
         assert_eq!(recovered, address);
         assert_eq!(recovered2, address);
     }
diff --git a/ethers/examples/sign.rs b/ethers/examples/sign.rs
index 953b8a1b..df3171c0 100644
--- a/ethers/examples/sign.rs
+++ b/ethers/examples/sign.rs
@@ -8,10 +8,8 @@ fn main() {
     let signature = wallet.sign_message(message);
     println!("Produced signature {}", signature);
 
-    // recover the address that signed it
-    let recovered = signature.recover(message).unwrap();
-
-    assert_eq!(recovered, wallet.address());
+    // verify the signature
+    signature.verify(message, wallet.address()).unwrap();
 
     println!("Verified signature produced by {:?}!", wallet.address());
 }