Skip to content

Commit 855d727

Browse files
committed
Added Transaction and Signature support for non-canonical S values (#4223, #5013).
1 parent f2269b8 commit 855d727

File tree

2 files changed

+102
-8
lines changed

2 files changed

+102
-8
lines changed

src.ts/crypto/signature.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ const BN_2 = BigInt(2);
1818
const BN_27 = BigInt(27);
1919
const BN_28 = BigInt(28);
2020
const BN_35 = BigInt(35);
21+
const BN_N = BigInt("0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141");
22+
const BN_N_2 = BN_N / BN_2; // Must be integer (floor) division; do NOT shifts
2123

24+
const inspect = Symbol.for("nodejs.util.inspect.custom");
2225

2326
const _guard = { };
2427

@@ -103,7 +106,8 @@ export class Signature {
103106
* Returns true if the Signature is valid for [[link-eip-2]] signatures.
104107
*/
105108
isValid(): boolean {
106-
return (parseInt(this.#s.substring(0, 3)) < 8);
109+
const s = BigInt(this.#s);
110+
return (s <= BN_N_2);
107111
}
108112

109113
/**
@@ -184,8 +188,25 @@ export class Signature {
184188
this.#networkV = null;
185189
}
186190

187-
[Symbol.for('nodejs.util.inspect.custom')](): string {
188-
return `Signature { r: "${ this.r }", s: "${ this._s }"${ this.isValid() ? "": ', valid: "false"'}, yParity: ${ this.yParity }, networkV: ${ this.networkV } }`;
191+
/**
192+
* Returns the canonical signature.
193+
*
194+
* This is only necessary when dealing with legacy transaction which
195+
* did not enforce canonical S values (i.e. [[link-eip-2]]. Most
196+
* developers should never require this.
197+
*/
198+
getCanonical(): Signature {
199+
if (this.isValid()) { return this; }
200+
201+
// Compute the canonical signature; s' = N - s, v = !v
202+
const s = BN_N - BigInt(this._s);
203+
const v = <27 | 28>(55 - this.v);
204+
const result = new Signature(_guard, this.r, toUint256(s), v);
205+
206+
// Populate the networkV if necessary
207+
if (this.networkV) { result.#networkV = this.networkV; }
208+
209+
return result;
189210
}
190211

191212
/**
@@ -209,6 +230,17 @@ export class Signature {
209230
};
210231
}
211232

233+
[inspect](): string {
234+
return this.toString();
235+
}
236+
237+
toString(): string {
238+
if (this.isValid()) {
239+
return `Signature { r: ${ this.r }, s: ${ this._s }, v: ${ this.v } }`;
240+
}
241+
return `Signature { r: ${ this.r }, s: ${ this._s }, v: ${ this.v }, valid: false }`;
242+
}
243+
212244
/**
213245
* Compute the chain ID from the ``v`` in a legacy EIP-155 transactions.
214246
*

src.ts/transaction/transaction.ts

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,15 @@ import type {
2020
AccessList, AccessListish, Authorization, AuthorizationLike
2121
} from "./index.js";
2222

23-
2423
const BN_0 = BigInt(0);
2524
const BN_2 = BigInt(2);
2625
const BN_27 = BigInt(27)
2726
const BN_28 = BigInt(28)
2827
const BN_35 = BigInt(35);
2928
const BN_MAX_UINT = BigInt("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff");
3029

30+
const inspect = Symbol.for("nodejs.util.inspect.custom");
31+
3132
const BLOB_SIZE = 4096 * 32;
3233

3334
// The BLS Modulo; each field within a BLOb must be less than this
@@ -318,7 +319,7 @@ function formatAuthorizationList(value: Array<Authorization>): Array<Array<strin
318319
formatNumber(a.nonce, "nonce"),
319320
formatNumber(a.signature.yParity, "yParity"),
320321
toBeArray(a.signature.r),
321-
toBeArray(a.signature.s)
322+
toBeArray(a.signature._s)
322323
];
323324
});
324325
}
@@ -434,7 +435,7 @@ function _serializeLegacy(tx: Transaction, sig: null | Signature): string {
434435
// Add the signature
435436
fields.push(toBeArray(v));
436437
fields.push(toBeArray(sig.r));
437-
fields.push(toBeArray(sig.s));
438+
fields.push(toBeArray(sig._s));
438439

439440
return encodeRlp(fields);
440441
}
@@ -895,6 +896,20 @@ export class Transaction implements TransactionLike<string> {
895896
this.#sig = (value == null) ? null: Signature.from(value);
896897
}
897898

899+
isValid(): boolean {
900+
const sig = this.signature;
901+
if (sig && !sig.isValid()) { return false; }
902+
903+
const auths = this.authorizationList;
904+
if (auths) {
905+
for (const auth of auths) {
906+
if (!auth.signature.isValid()) { return false; }
907+
}
908+
}
909+
910+
return true;
911+
}
912+
898913
/**
899914
* The access list.
900915
*
@@ -1104,15 +1119,15 @@ export class Transaction implements TransactionLike<string> {
11041119
*/
11051120
get from(): null | string {
11061121
if (this.signature == null) { return null; }
1107-
return recoverAddress(this.unsignedHash, this.signature);
1122+
return recoverAddress(this.unsignedHash, this.signature.getCanonical());
11081123
}
11091124

11101125
/**
11111126
* The public key of the sender, if signed. Otherwise, ``null``.
11121127
*/
11131128
get fromPublicKey(): null | string {
11141129
if (this.signature == null) { return null; }
1115-
return SigningKey.recoverPublicKey(this.unsignedHash, this.signature);
1130+
return SigningKey.recoverPublicKey(this.unsignedHash, this.signature.getCanonical());
11161131
}
11171132

11181133
/**
@@ -1315,6 +1330,53 @@ export class Transaction implements TransactionLike<string> {
13151330
};
13161331
}
13171332

1333+
[inspect](): string {
1334+
return this.toString();
1335+
}
1336+
1337+
toString(): string {
1338+
const output: Array<string> = [ ];
1339+
const add = (key: string) => {
1340+
let value = (<any>this)[key];
1341+
if (typeof(value) === "string") { value = JSON.stringify(value); }
1342+
output.push(`${ key }: ${ value }`);
1343+
};
1344+
1345+
if (this.type) { add("type"); }
1346+
add("to");
1347+
add("data");
1348+
add("nonce");
1349+
add("gasLimit");
1350+
add("value");
1351+
if (this.chainId != null) { add("chainId"); }
1352+
if (this.signature) {
1353+
add("from");
1354+
output.push(`signature: ${ this.signature.toString() }`);
1355+
}
1356+
1357+
// @TODO: accessList
1358+
1359+
// @TODO: blobs (might make output huge; maybe just include a flag?)
1360+
1361+
const auths = this.authorizationList;
1362+
if (auths) {
1363+
const outputAuths: Array<string> = [ ];
1364+
for (const auth of auths) {
1365+
const o: Array<string> = [ ];
1366+
o.push(`address: ${ JSON.stringify(auth.address) }`);
1367+
if (auth.nonce != null) { o.push(`nonce: ${ auth.nonce }`); }
1368+
if (auth.chainId != null) { o.push(`chainId: ${ auth.chainId }`); }
1369+
if (auth.signature) {
1370+
o.push(`signature: ${ auth.signature.toString() }`);
1371+
}
1372+
outputAuths.push(`Authorization { ${ o.join(", ") } }`);
1373+
}
1374+
output.push(`authorizations: [ ${ outputAuths.join(", ") } ]`);
1375+
}
1376+
1377+
return `Transaction { ${ output.join(", ") } }`;
1378+
}
1379+
13181380
/**
13191381
* Create a **Transaction** from a serialized transaction or a
13201382
* Transaction-like object.

0 commit comments

Comments
 (0)