Skip to content

Commit

Permalink
Fix EIP-712 type aliases for uint and int (#4541).
Browse files Browse the repository at this point in the history
  • Loading branch information
ricmoo committed Jan 16, 2024
1 parent 7882905 commit 43fb9c2
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 13 deletions.
83 changes: 82 additions & 1 deletion src.ts/_tests/test-hash-typeddata.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import assert from "assert";
import { loadTests } from "./utils.js";
import type { TestCaseTypedData } from "./types.js";

import { TypedDataEncoder } from "../index.js";


Expand All @@ -18,3 +17,85 @@ describe("Tests Typed Data (EIP-712)", function() {
});
}
});

interface TestAlias {
name: string;
types: Record<string, Array<{ name: string, type: string }>>;
typesAlias: Record<string, Array<{ name: string, type: string }>>;
data: Record<string, any>;
encoded: string;
}

describe("Tests Typed Data (EIP-712) aliases", function() {
const tests: Array<TestAlias> = [
{
name: "uint",
types: {
foo: [
{ name: "a", type: "uint256" },
{ name: "b", type: "string" },
],
},
typesAlias: {
foo: [
{ name: "a", type: "uint" },
{ name: "b", type: "string" },
],
},
data: {
a: 35,
b: "hello"
},
encoded: "0x859b6b4a5d436f85a809f6383b4b35a153aa6fe9c95946c366d9dfd634b89f4700000000000000000000000000000000000000000000000000000000000000231c8aff950685c2ed4bc3174f3472287b56d9517b9c948127319a09a7a36deac8"
},
{
name: "int",
types: {
foo: [
{ name: "a", type: "int256" },
{ name: "b", type: "string" },
],
},
typesAlias: {
foo: [
{ name: "a", type: "int" },
{ name: "b", type: "string" },
],
},
data: {
a: 35,
b: "hello"
},
encoded: "0xa272ada5f88085e4cb18acdb87bd057a8cbfec249fee53de0149409080947cf500000000000000000000000000000000000000000000000000000000000000231c8aff950685c2ed4bc3174f3472287b56d9517b9c948127319a09a7a36deac8"
},
];

for (const test of tests) {
it(`tests encoding typed-data: ${ test.name }`, function() {
const encoder = TypedDataEncoder.from(test.types);
assert.equal(encoder.primaryType, "foo", "primaryType");
assert.equal(encoder.encodeData("foo", test.data), test.encoded, "encoded");
});
}

it(`tests overriding an alias as a type`, function() {
const encoder = TypedDataEncoder.from({
uint: [
{ name: "value", type: "uint256" }
],
foo: [
{ name: "a", type: "uint" },
{ name: "b", type: "string" },
]
});
assert.equal(encoder.primaryType, "foo", "primaryType");

const data = encoder.encodeData("foo", {
a: { value: 42 },
b: "hello"
});

const encoded = "0x87a4bfff36f1a2ecde6468d6acd51ecc5ef8f3a15d8115a412c686d82d3fdbe4628fc3080b86a044fb60153bb7dc3f904e9ed1cebadf35c17099a060ba4df90b1c8aff950685c2ed4bc3174f3472287b56d9517b9c948127319a09a7a36deac8";
assert.equal(data, encoded, "encoded");
});
});
32 changes: 20 additions & 12 deletions src.ts/hash/typed-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,12 @@ const domainChecks: Record<string, (value: any) => any> = {
function getBaseEncoder(type: string): null | ((value: any) => string) {
// intXX and uintXX
{
const match = type.match(/^(u?)int(\d*)$/);
const match = type.match(/^(u?)int(\d+)$/);
if (match) {
const signed = (match[1] === "");

const width = parseInt(match[2] || "256");
assertArgument(width % 8 === 0 && width !== 0 && width <= 256 && (match[2] == null || match[2] === String(width)), "invalid numeric width", "type", type);
const width = parseInt(match[2]);
assertArgument(width % 8 === 0 && width !== 0 && width <= 256 && match[2] === String(width), "invalid numeric width", "type", type);

const boundsUpper = mask(BN_MAX_UINT256, signed ? (width - 1): width);
const boundsLower = signed ? ((boundsUpper + BN_1) * BN__1): BN_0;
Expand Down Expand Up @@ -220,8 +220,7 @@ export class TypedDataEncoder {
* do not violate the [[link-eip-712]] structural constraints as
* well as computes the [[primaryType]].
*/
constructor(types: Record<string, Array<TypedDataField>>) {
this.#types = JSON.stringify(types);
constructor(_types: Record<string, Array<TypedDataField>>) {
this.#fullTypes = new Map();
this.#encoderCache = new Map();

Expand All @@ -234,30 +233,39 @@ export class TypedDataEncoder {
// Link all subtypes within a given struct
const subtypes: Map<string, Set<string>> = new Map();

Object.keys(types).forEach((type) => {
const types: Record<string, Array<TypedDataField>> = { };
Object.keys(_types).forEach((type) => {
// Normalize int/uint unless they are a complex type themselves
types[type] = _types[type].map(({ name, type }) => {
if (type === "int" && !_types["int"]) { type = "int256"; }
if (type === "uint" && !_types["uint"]) { type = "uint256"; }

This comment has been minimized.

Copy link
@charlie-kim

charlie-kim Jan 16, 2024

@ricmoo What is _types["int"] for? The key for _types is for signature type not data type?

This comment has been minimized.

Copy link
@ricmoo

ricmoo Jan 16, 2024

Author Member

The specification explicitly says that there is no type alias for int or uint, which means those labels are candidates for a complex type. Ethers wants to provide the aliases, but cannot interfere with the specification.

So, if someone really wants a type named "uint", they can, but that will disable the alias. There is an example in the tests. :)

For example:

myTypes = {
  message: [
    { name: "myUint", type: "uint" },
    { name: "to", type: "address" },
    { name: "subject", type: "string" },
  ],
  uint: [  // <--- this craziness
    { name: "value", type: "uint256" },
    { name: "iHateAliasesSoIWillBorkThem", type: "string" }
  ]
};

This comment has been minimized.

Copy link
@charlie-kim

charlie-kim Jan 16, 2024

Ah, I feel the craziness. Thanks for your explanation.

return { name, type };
});

links.set(type, new Set());
parents.set(type, [ ]);
subtypes.set(type, new Set());
});
this.#types = JSON.stringify(types);

for (const name in types) {
const uniqueNames: Set<string> = new Set();

for (const field of types[name]) {

// Check each field has a unique name
assertArgument(!uniqueNames.has(field.name), `duplicate variable name ${ JSON.stringify(field.name) } in ${ JSON.stringify(name) }`, "types", types);
assertArgument(!uniqueNames.has(field.name), `duplicate variable name ${ JSON.stringify(field.name) } in ${ JSON.stringify(name) }`, "types", _types);
uniqueNames.add(field.name);

// Get the base type (drop any array specifiers)
const baseType = (<any>(field.type.match(/^([^\x5b]*)(\x5b|$)/)))[1] || null;
assertArgument(baseType !== name, `circular type reference to ${ JSON.stringify(baseType) }`, "types", types);
assertArgument(baseType !== name, `circular type reference to ${ JSON.stringify(baseType) }`, "types", _types);

// Is this a base encoding type?
const encoder = getBaseEncoder(baseType);
if (encoder) { continue; }

assertArgument(parents.has(baseType), `unknown type ${ JSON.stringify(baseType) }`, "types", types);
assertArgument(parents.has(baseType), `unknown type ${ JSON.stringify(baseType) }`, "types", _types);

// Add linkage
(parents.get(baseType) as Array<string>).push(name);
Expand All @@ -267,14 +275,14 @@ export class TypedDataEncoder {

// Deduce the primary type
const primaryTypes = Array.from(parents.keys()).filter((n) => ((parents.get(n) as Array<string>).length === 0));
assertArgument(primaryTypes.length !== 0, "missing primary type", "types", types);
assertArgument(primaryTypes.length === 1, `ambiguous primary types or unused types: ${ primaryTypes.map((t) => (JSON.stringify(t))).join(", ") }`, "types", types);
assertArgument(primaryTypes.length !== 0, "missing primary type", "types", _types);
assertArgument(primaryTypes.length === 1, `ambiguous primary types or unused types: ${ primaryTypes.map((t) => (JSON.stringify(t))).join(", ") }`, "types", _types);

defineProperties<TypedDataEncoder>(this, { primaryType: primaryTypes[0] });

// Check for circular type references
function checkCircular(type: string, found: Set<string>) {
assertArgument(!found.has(type), `circular type reference to ${ JSON.stringify(type) }`, "types", types);
assertArgument(!found.has(type), `circular type reference to ${ JSON.stringify(type) }`, "types", _types);

found.add(type);

Expand Down

0 comments on commit 43fb9c2

Please sign in to comment.