Fixed legacy serialization for implicit chainId transactions (#3898, #…

…3899).
ethers-io · Mar 16, 2023 · fcf6c8f · fcf6c8f
1 parent 3ad4273
commit fcf6c8f
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 7 deletions.
diff --git a/src.ts/crypto/signing-key.ts b/src.ts/crypto/signing-key.ts
@@ -181,9 +181,9 @@ export class SigningKey {
         const der = secp256k1.Signature.fromCompact(getBytesCopy(concat([ sig.r, sig.s ]))).toDERRawBytes();
 
         const pubKey = secp256k1.recoverPublicKey(getBytesCopy(digest), der, sig.yParity);
-        if (pubKey != null) { return hexlify(pubKey); }
+        assertArgument(pubKey != null, "invalid signautre for digest", "signature", signature);
 
-        assertArgument(false, "invalid signautre for digest", "signature", signature);
+        return hexlify(pubKey);
     }
 
     /**

diff --git a/src.ts/transaction/transaction.ts b/src.ts/transaction/transaction.ts
@@ -191,7 +191,7 @@ function _serializeLegacy(tx: Transaction, sig?: Signature): string {
     ];
 
     let chainId = BN_0;
-    if (tx.chainId != null) {
+    if (tx.chainId != BN_0) {
         // A chainId was provided; if non-zero we'll use EIP-155
         chainId = getBigInt(tx.chainId, "tx.chainId");
 
@@ -200,9 +200,9 @@ function _serializeLegacy(tx: Transaction, sig?: Signature): string {
         assertArgument(!sig || sig.networkV == null || sig.legacyChainId === chainId,
              "tx.chainId/sig.v mismatch", "sig", sig);
 
-    } else if (sig) {
-        // No chainId provided, but the signature is signing with EIP-155; derive chainId
-        const legacy = sig.legacyChainId;
+    } else if (tx.signature) {
+        // No explicit chainId, but EIP-155 have a derived implicit chainId
+        const legacy = tx.signature.legacyChainId;
         if (legacy != null) { chainId = legacy; }
     }
 
@@ -218,14 +218,19 @@ function _serializeLegacy(tx: Transaction, sig?: Signature): string {
         return encodeRlp(fields);
     }
 
-    // We pushed a chainId and null r, s on for hashing only; remove those
+    // @TODO: We should probably check that tx.signature, chainId, and sig
+    //        match but that logic could break existing code, so schedule
+    //        this for the next major bump.
+
+    // Compute the EIP-155 v
     let v = BigInt(27 + sig.yParity);
     if (chainId !== BN_0) {
         v = Signature.getChainIdV(chainId, sig.v);
     } else if (BigInt(sig.v) !== v) {
         assertArgument(false, "tx.chainId/sig.v mismatch", "sig", sig);
     }
 
+    // Add the signature
     fields.push(toBeArray(v));
     fields.push(toBeArray(sig.r));
     fields.push(toBeArray(sig.s));