feat: Support Ethereum EIP-712 Sign Typed Data for Trezor T

[aloisklink]

Adds support for EIP-712 Typed Signed Data #825.

This is mostly adapted from the Python implementation in python/src/trezorlib/ethereum.py in trezor/trezor-firmware#1835

One potential issue is TextEncoder, which is what I'm using to convert a UTF-8 string to bytes. It's supported in all the browsers Trezor Connect targets, except IE, and I'm unsure if Babel/Webpack is polyfilling it automatically. I've only run karma tests on Chrome. Replaced with Buffer.from('utf-8').toString('hex') in 0103d17

I also found quite a few Flow issues that I'm pretty sure would be fixed by updating Flow, (e.g. await Promise not always resolving the promise type) but unfortunately our .flowconfig has deprecated options that aren't supported in the latest versions, so it's not trivial to update.

[mroz22]

Hello, thanks for contribution! We are looking into it now.

[aloisklink]

Ah, I'm sorry, I forgot to run an yarn run lint on my final test changes! I've added a fixup! commit that fixes those style issues.

[aloisklink]

Hi @matejcik, sorry, I didn't see your PR #982 that does the same thing.

Feel free to close this if you'd prefer to go with your implementation. I'm happy for you to copy-and-paste any docs/type entries from this PR to your PR if you decide to merge your one instead.

[matejcik]

i'm OK with using this code, you seem to be generally better at javascript than I am :) so this PR is the more complete one of the two

[mroz22]

It looks very well overall. Only if you could please:

• add some very basic unit tests for helpers/ethereumTypeDataConversions.js. Not really needed to get 100% coverage. Basic tests for exported methods will be enough.
• Use Buffer instead of TextEncoder to keep it consistent with the rest of the codebase. Is that possible for you?

[aloisklink]

Description of commits I made, since they're pretty much all just called fixup! ...

• 0103d17 I replaced TextEncoder with Buffer.from(x, "utf-8).toString("hex") for Internet Explorer support.
• bcfab25 Reverts 34d9392 which added support for TextEncoder in jest tests. We'll manually delete this commit and the original before merging, since I don't think you can use fixup! to delete a commit.
• c2e9887 Fix misspelling in twosComplement()
• be4e076 Unit tests added! It was a good idea to add them too! I wrote a -1 instead of a +1 when trying to calculate when a two's compliment conversion should overflow.
  • I also renamed helpers/ethereumTypedDataConversions.js just to helpers/ethereumTypedData.js.
  • I changed all throw new Error(... with throw ERRORS.TypedError('Method_InvalidParameter', ...
• 1c1fea2 I removed bigint and string number integration tests, since they're now covered by unit tests.
• a7d3656 Test and handle all integer overflow scenarios.
• 36d1201 Remove an error message for nested arrays, as Trezor Firmware may add support for it down the line (discussion)

Just missed one more:

• af80c5c Refactor some BigNumber.js code. Replaced a .plus(x.negated()) with .minus(x) and removed the redundant arg in .isNegative().

[mroz22]

Hello @aloisklink
thanks for the latest changes you have made. They look good.

Now I believe this is the last batch of changes requests. As you can see, these are mostly nitpicks but we would like to have them resolved to keep codebase consistent.

[mroz22]

Great, from my perspective this is ready to go. I would still like to wait for @matejcik to resolve his conversation and my question on whether we need to do the client side checksum (I believe we don't).

[mroz22]

Ok, so please now squash your commits. I think this can all fit into 1 commit? Do you think it makes sense? Or is it better to keep some of the commits separated?

[aloisklink]

Hi @mroz22, I merged everything into one commit 30d17ab, except the change in .eslintrc that allows variables starting with _ to be unused, which I left in 82f58e3.

(I use a bunch of [ _, group1, group2 ] = regexMatch; for doing regex matches).

Let me know if you want me to merge them into one commit too.

[aloisklink]

Is it worth me rebasing on the develop branch due to #988?

There might be some style errors/merge conflicts in typescript files otherwise.

[mroz22]

Yes, please also rebase and fix potential conflicts and I believe we are done after that.

[aloisklink]

Rebased onto f0009cf (aka the current develop HEAD).

No merge conflicts, but there were a couple of TypeScript style issues that went away after a yarn run lint:fix.
(I've already squashed them into 7075456)