From edb7c5da91ce271688561364d867998b0f0675e3 Mon Sep 17 00:00:00 2001
From: Richard Moore <github@ricmoo.com>
Date: Tue, 4 Feb 2020 00:48:45 -0500
Subject: [PATCH] Safer transaction serialization, matching signature.v with
 chainId (#708).

---
 packages/transactions/src.ts/index.ts | 36 +++++++++++++++++++++------
 1 file changed, 28 insertions(+), 8 deletions(-)

diff --git a/packages/transactions/src.ts/index.ts b/packages/transactions/src.ts/index.ts
index 2fd113884a..8e954ad3eb 100644
--- a/packages/transactions/src.ts/index.ts
+++ b/packages/transactions/src.ts/index.ts
@@ -2,7 +2,7 @@
 
 import { getAddress } from "@ethersproject/address";
 import { BigNumber, BigNumberish } from "@ethersproject/bignumber";
-import { arrayify, BytesLike, DataOptions, hexDataSlice, hexlify, hexZeroPad, SignatureLike, splitSignature, stripZeros, } from "@ethersproject/bytes";
+import { arrayify, BytesLike, DataOptions, hexDataSlice, hexlify, hexZeroPad, isBytesLike, SignatureLike, splitSignature, stripZeros, } from "@ethersproject/bytes";
 import { Zero } from "@ethersproject/constants";
 import { keccak256 } from "@ethersproject/keccak256";
 import { checkProperties } from "@ethersproject/properties";
@@ -109,17 +109,30 @@ export function serialize(transaction: UnsignedTransaction, signature?: Signatur
         raw.push(hexlify(value));
     });
 
-    if (transaction.chainId != null && transaction.chainId !== 0) {
-        raw.push(hexlify(transaction.chainId));
+    let chainId = 0;
+    if (transaction.chainId != null) {
+        // A chainId was provided; if non-zero we'll use EIP-155
+        chainId = transaction.chainId;
+
+        if (typeof(chainId) !== "number") {
+            logger.throwArgumentError("invalid transaction.chainId", "transaction", transaction);
+        }
+
+    } else if (signature && !isBytesLike(signature) && signature.v > 28) {
+        // No chainId provided, but the signature is signing with EIP-155; derive chainId
+        chainId = Math.floor((signature.v - 35) / 2);
+    }
+
+    // We have an EIP-155 transaction (chainId was specified and non-zero)
+    if (chainId !== 0) {
+        raw.push(hexlify(chainId));
         raw.push("0x");
         raw.push("0x");
     }
 
-    const unsignedTransaction = RLP.encode(raw);
-
     // Requesting an unsigned transation
     if (!signature) {
-        return unsignedTransaction;
+        return RLP.encode(raw);
     }
 
     // The splitSignature will ensure the transaction has a recoveryParam in the
@@ -128,11 +141,18 @@ export function serialize(transaction: UnsignedTransaction, signature?: Signatur
 
     // We pushed a chainId and null r, s on for hashing only; remove those
     let v = 27 + sig.recoveryParam
-    if (raw.length === 9) {
+    if (chainId !== 0) {
         raw.pop();
         raw.pop();
         raw.pop();
-        v += transaction.chainId * 2 + 8;
+        v += chainId * 2 + 8;
+
+        // If an EIP-155 v (directly or indirectly; maybe _vs) was provided, check it!
+        if (sig.v > 28 && sig.v !== v) {
+             logger.throwArgumentError("transaction.chainId/signature.v mismatch", "signature", signature);
+        }
+    } else if (sig.v !== v) {
+         logger.throwArgumentError("transaction.chainId/signature.v mismatch", "signature", signature);
     }
 
     raw.push(hexlify(v));