Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix typed message schema mistake #243

Merged
merged 3 commits into from
Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"@types/node": "^14.14.25",
"@typescript-eslint/eslint-plugin": "^4.28.2",
"@typescript-eslint/parser": "^4.28.2",
"ajv": "^8.11.0",
"eslint": "^7.30.0",
"eslint-config-prettier": "^8.3.0",
"eslint-plugin-import": "^2.23.4",
Expand Down
215 changes: 215 additions & 0 deletions src/sign-typed-data.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,232 @@
import * as ethUtil from 'ethereumjs-util';
import Ajv from 'ajv';
import {
recoverTypedSignature,
signTypedData,
TypedDataUtils,
typedSignatureHash,
SignTypedDataVersion,
TYPED_MESSAGE_SCHEMA,
} from './sign-typed-data';

const privateKey = Buffer.from(
'4af1bceebf7f3634ec3cff8a2c38e51178d5d4ce585c52d6043e5e2cc3418bb0',
'hex',
);

/**
* Get a list of all Solidity types supported by EIP-712.
*
* @returns A list of all supported Solidity types.
*/
function getEip712SolidityTypes() {
const types = ['bool', 'address', 'string', 'bytes'];
const ints = Array.from(new Array(32)).map(
(_, index) => `int${(index + 1) * 8}`,
);
const uints = Array.from(new Array(32)).map(
(_, index) => `uint${(index + 1) * 8}`,
);
const bytes = Array.from(new Array(32)).map(
(_, index) => `bytes${index + 1}`,
);

return [...types, ...ints, ...uints, ...bytes];
}

const eip712SolidityTypes = getEip712SolidityTypes();

describe('TYPED_MESSAGE_SCHEMA', () => {
it('should match valid typed message', () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] What do you think about extracting this validate function since it's the same for every test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Done in b5f0d87

const typedMessage = {
domain: {},
message: {},
primaryType: 'object',
types: {
EIP712Domain: [],
},
};

expect(validate(typedMessage)).toBe(true);
});

it('should allow custom types in addition to domain', () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: {},
message: {},
primaryType: 'Message',
types: {
EIP712Domain: [],
Message: [],
},
};

expect(validate(typedMessage)).toBe(true);
});

for (const solidityType of eip712SolidityTypes) {
// eslint-disable-next-line no-loop-func
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nit] If you use .forEach instead of a for loop, does it remove the need for this override?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but now that you've brought this up, I kinda want to get to the bottom of this. This ought to work. Something is messed up with the ESLint globals configuration. I'll look into this in a subsequent PR.

Copy link
Member Author

@Gudahtt Gudahtt Apr 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my goodness. I found the problem. The override for the test ESLint config is wrong, the file glob uses the js file extension rather than ts. There are a bunch of violations, I'll fix this separately.

Edit: Done in #245

it(`should allow custom type to have type of '${solidityType}'`, () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: {},
message: {},
primaryType: 'Message',
types: {
EIP712Domain: [],
Message: [{ name: 'data', type: solidityType }],
},
};

expect(validate(typedMessage)).toBe(true);
});
}

it('should allow custom type to have a custom type', () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: {},
message: {},
primaryType: 'Message',
types: {
CustomValue: [{ name: 'value', type: 'string' }],
EIP712Domain: [],
Message: [{ name: 'data', type: 'CustomValue' }],
},
};

expect(validate(typedMessage)).toBe(true);
});

const invalidStrings = [undefined, null, 0, 1, [], {}];

for (const invalidString of invalidStrings) {
// eslint-disable-next-line no-loop-func
it(`should disallow a primary type with value '${invalidString}'`, () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: {},
message: {},
primaryType: invalidString,
types: {
EIP712Domain: [],
},
};

expect(validate(typedMessage)).toBe(false);
});
}

const invalidObjects = [undefined, null, 0, 1, [], '', 'test'];
for (const invalidObject of invalidObjects) {
// eslint-disable-next-line no-loop-func
it(`should disallow a domain with value '${invalidObject}'`, () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: invalidObject,
message: {},
primaryType: 'object',
types: {
EIP712Domain: [],
},
};

expect(validate(typedMessage)).toBe(false);
});

// eslint-disable-next-line no-loop-func
it(`should disallow a message with value '${invalidObject}'`, () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: {},
message: invalidObject,
primaryType: 'object',
types: {
EIP712Domain: [],
},
};

expect(validate(typedMessage)).toBe(false);
});

// eslint-disable-next-line no-loop-func
it(`should disallow types with value '${invalidObject}'`, () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: {},
message: {},
primaryType: 'object',
types: invalidObject,
};

expect(validate(typedMessage)).toBe(false);
});
}

it('should require custom type properties to have a name', () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: {},
message: {},
primaryType: 'Message',
types: {
EIP712Domain: [],
Message: [{ type: 'string' }],
},
};

expect(validate(typedMessage)).toBe(false);
});

it('should require custom type properties to have a type', () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: {},
message: {},
primaryType: 'Message',
types: {
EIP712Domain: [],
Message: [{ name: 'name' }],
},
};

expect(validate(typedMessage)).toBe(false);
});

const invalidTypes = [undefined, null, 0, 1, [], {}];

for (const invalidType of invalidTypes) {
// eslint-disable-next-line no-loop-func
it(`should disallow a type of '${invalidType}'`, () => {
const ajv = new Ajv();
const validate = ajv.compile(TYPED_MESSAGE_SCHEMA);
const typedMessage = {
domain: {},
message: {},
primaryType: 'Message',
types: {
EIP712Domain: [],
Message: [{ name: 'name', type: invalidType }],
},
};

expect(validate(typedMessage)).toBe(false);
});
}
});

const encodeDataExamples = {
// dynamic types supported by EIP-712:
bytes: [10, '10', '0x10', Buffer.from('10', 'utf8')],
Expand Down
22 changes: 1 addition & 21 deletions src/sign-typed-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const TYPED_MESSAGE_SCHEMA = {
type: 'object',
properties: {
name: { type: 'string' },
type: { type: 'string', enum: getSolidityTypes() },
type: { type: 'string' },
},
required: ['name', 'type'],
},
Expand All @@ -113,26 +113,6 @@ export const TYPED_MESSAGE_SCHEMA = {
required: ['types', 'primaryType', 'domain', 'message'],
};

/**
* Get a list of all Solidity types.
*
* @returns A list of all Solidity types.
*/
function getSolidityTypes() {
const types = ['bool', 'address', 'string', 'bytes'];
const ints = Array.from(new Array(32)).map(
(_, index) => `int${(index + 1) * 8}`,
);
const uints = Array.from(new Array(32)).map(
(_, index) => `uint${(index + 1) * 8}`,
);
const bytes = Array.from(new Array(32)).map(
(_, index) => `bytes${index + 1}`,
);

return [...types, ...ints, ...uints, ...bytes];
}

/**
* Validate that the given value is a valid version string.
*
Expand Down
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1005,6 +1005,16 @@ ajv@^8.0.1:
require-from-string "^2.0.2"
uri-js "^4.2.2"

ajv@^8.11.0:
version "8.11.0"
resolved "https://registry.yarnpkg.com/ajv/-/ajv-8.11.0.tgz#977e91dd96ca669f54a11e23e378e33b884a565f"
integrity sha512-wGgprdCvMalC0BztXvitD2hC04YffAvtsUn93JbGXYLAtCUO4xd17mCCZQxUOItiBwZvJScWo8NIvQMQ71rdpg==
dependencies:
fast-deep-equal "^3.1.1"
json-schema-traverse "^1.0.0"
require-from-string "^2.0.2"
uri-js "^4.2.2"

ansi-colors@^4.1.1:
version "4.1.1"
resolved "https://registry.yarnpkg.com/ansi-colors/-/ansi-colors-4.1.1.tgz#cbb9ae256bf750af1eab344f229aa27fe94ba348"
Expand Down