Conversation
The exported JSON schema `TYPED_MESSAGE_SCHEMA` was changed in v4 to specify all supported Solidity types, but this accidentally prevented the use of reference types. This change was undone, so that the schema no longer validates that the types used are valid. I could not find a way to preserve this validation without disallowing reference types. JSON schema did not make this possible. Tests were added for the typed message schema to ensure this doesn't happen again. Fixes #242
mcmire
left a comment
There was a problem hiding this comment.
That's very interesting that JSON Schema didn't allow you to do what you wanted to do.
A couple of nits below but looks good to me regardless.
| describe('TYPED_MESSAGE_SCHEMA', () => { | ||
| it('should match valid typed message', () => { | ||
| const ajv = new Ajv(); | ||
| const validate = ajv.compile(TYPED_MESSAGE_SCHEMA); |
There was a problem hiding this comment.
[nit] What do you think about extracting this validate function since it's the same for every test?
| }); | ||
|
|
||
| for (const solidityType of eip712SolidityTypes) { | ||
| // eslint-disable-next-line no-loop-func |
There was a problem hiding this comment.
[nit] If you use .forEach instead of a for loop, does it remove the need for this override?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
The exported JSON schema
TYPED_MESSAGE_SCHEMAwas changed in v4 to specify all supported Solidity types, but this accidentally prevented the use of reference types.This change was undone, so that the schema no longer validates that the types used are valid. I could not find a way to preserve this validation without disallowing reference types. JSON schema did not make this possible.
Tests were added for the typed message schema to ensure this doesn't happen again.
Fixes #242