From 799660bd26fc49da08f306383dec7121545e7509 Mon Sep 17 00:00:00 2001
From: Dan Cline <dan@dancline.net>
Date: Wed, 25 May 2022 17:10:07 -0400
Subject: [PATCH] fix(signer): use wallet chainid for tx signing (#1308)

* add a test checking that the wallet outputs a signature with a
   correct v
---
 ethers-signers/src/wallet/mod.rs         |  9 ++++--
 ethers-signers/src/wallet/private_key.rs | 41 ++++++++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/ethers-signers/src/wallet/mod.rs b/ethers-signers/src/wallet/mod.rs
index 5f44ef6f..c9300601 100644
--- a/ethers-signers/src/wallet/mod.rs
+++ b/ethers-signers/src/wallet/mod.rs
@@ -119,10 +119,15 @@ impl<D: Sync + Send + DigestSigner<Sha256Proxy, RecoverableSignature>> Signer fo
 
 impl<D: DigestSigner<Sha256Proxy, RecoverableSignature>> Wallet<D> {
     /// Synchronously signs the provided transaction, normalizing the signature `v` value with
-    /// EIP-155 using the transaction's `chain_id`.
+    /// EIP-155 using the transaction's `chain_id`, or the signer's `chain_id` if the transaction
+    /// does not specify one.
     pub fn sign_transaction_sync(&self, tx: &TypedTransaction) -> Signature {
-        let sighash = tx.sighash();
+        // rlp (for sighash) must have the same chain id as v in the signature
         let chain_id = tx.chain_id().map(|id| id.as_u64()).unwrap_or(self.chain_id);
+        let mut tx = tx.clone();
+        tx.set_chain_id(chain_id);
+
+        let sighash = tx.sighash();
         let mut sig = self.sign_hash(sighash);
 
         // sign_hash sets `v` to recid + 27, so we need to subtract 27 before normalizing
diff --git a/ethers-signers/src/wallet/private_key.rs b/ethers-signers/src/wallet/private_key.rs
index d2bd75fa..e9871d92 100644
--- a/ethers-signers/src/wallet/private_key.rs
+++ b/ethers-signers/src/wallet/private_key.rs
@@ -252,6 +252,47 @@ mod tests {
         assert!(sig.verify(sighash, wallet.address).is_ok());
     }
 
+    #[test]
+    #[cfg(not(feature = "celo"))]
+    fn signs_tx_empty_chain_id_sync() {
+        use crate::TypedTransaction;
+        use ethers_core::types::TransactionRequest;
+
+        let chain_id = 1337u64;
+        // retrieved test vector from:
+        // https://web3js.readthedocs.io/en/v1.2.0/web3-eth-accounts.html#eth-accounts-signtransaction
+        let tx: TypedTransaction = TransactionRequest {
+            from: None,
+            to: Some("F0109fC8DF283027b6285cc889F5aA624EaC1F55".parse::<Address>().unwrap().into()),
+            value: Some(1_000_000_000u64.into()),
+            gas: Some(2_000_000u64.into()),
+            nonce: Some(0u64.into()),
+            gas_price: Some(21_000_000_000u128.into()),
+            data: None,
+            chain_id: None,
+        }
+        .into();
+        let wallet: Wallet<SigningKey> =
+            "4c0883a69102937d6231471b5dbb6204fe5129617082792ae468d01a3f362318".parse().unwrap();
+        let wallet = wallet.with_chain_id(chain_id);
+
+        // this should populate the tx chain_id as the signer's chain_id (1337) before signing and
+        // normalize the v
+        let sig = wallet.sign_transaction_sync(&tx);
+
+        // ensure correct v given the chain - first extract recid
+        let recid = (sig.v - 35) % 2;
+        // eip155 check
+        assert_eq!(sig.v, chain_id * 2 + 35 + recid);
+
+        // since we initialize with None we need to re-set the chain_id for the sighash to be
+        // correct
+        let mut tx = tx;
+        tx.set_chain_id(chain_id);
+        let sighash = tx.sighash();
+        assert!(sig.verify(sighash, wallet.address).is_ok());
+    }
+
     #[test]
     fn key_to_address() {
         let wallet: Wallet<SigningKey> =