refactor(ethSignTypedData)!: Improve docs & types

BREAKING CHANGE: EthereumSignTypedData now requires passing Trezor T variables. Trezor Model 1 parameters are still optional, as blind signing requires using an extra dependency on an external library, and is less secure.
trezor · Jan 27, 2022 · c7e516b · c7e516b
1 parent be73334
commit c7e516b
Show file tree

Hide file tree

Showing 9 changed files with 166 additions and 59 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,7 +1,9 @@
 # 8.2.7 (not released)
 
 ### Changed
+
 - @trezor/blockchain-link 2.0.0 use workers as commonjs modules in nodejs and react-native env.
+- Ethereum: EthereumSignTypedData must always have at least Trezor T parameters.
 
 # 8.2.6
 

diff --git a/docs/methods/ethereumSignTypedData.md b/docs/methods/ethereumSignTypedData.md
@@ -19,52 +19,81 @@ TrezorConnect.ethereumSignTypedData(params).then(function(result) {
 ```
 
 > :warning: **Supported only by Trezor T with Firmware 2.4.3 or higher!** 
+> :warning: **Blind signing is supported only on Trezor Model 1 with Firmware 1.10.5 or higher!**
 
 ### Params
 
 [****Optional common params****](commonParams.md)
 
-###### [flowtype](../../src/js/types/networks/ethereum.js#L102-105)
+###### [flowtype](../../src/js/types/networks/ethereum.js#104-116)
 
 * `path` — *required* `string | Array<number>` minimum length is `3`. [read more](path.md)
 * `data` - *required* `Object` type of [`EthereumSignTypedDataMessage`](../../src/js/types/networks/ethereum.js#L90)`. A JSON Schema definition can be found in the [EIP-712 spec]([EIP-712](https://eips.ethereum.org/EIPS/eip-712)).
 * `metamask_v4_compat` - *required* `boolean` set to `true` for compatibility with [MetaMask's signTypedData_v4](https://docs.metamask.io/guide/signing-data.html#sign-typed-data-v4).
 
+#### Blind signing (optional addition for Trezor Model 1 compatibility)
+
+The Trezor Model 1 firmware currently does not support constructing EIP-712
+hashes.
+
+However, it supports signing pre-constructed hashes.
+
+EIP-712 hashes can be constructed with the plugin function at
+["trezor-connect/lib/plugins/ethereum/typedData.js"](../../src/js/plugins/ethereum/typedData.js)
+(included as a plugin due to a depedency on `@metamask/eth-sig-utils`).
+
+You may also wish to contruct your own hashes using a different library.
+
+###### [flowtype](../../src/js/types/networks/ethereum.js#L114-121)
+
+* `domain_separator_hash` - *required* `string` hex-encoded 32-byte hash of the EIP-712 domain.
+* `message_hash` - *required* `string` hex-encoded 32-byte hash of the EIP-712 message.
+
 ### Example
 
 ```javascript
+const eip712Data = {
+    types: {
+        EIP712Domain: [
+            {
+                name: 'name',
+                type: 'string',
+            },
+        ],
+        Message: [
+            {
+                name: "Best Wallet",
+                type: "string"
+            },
+            {
+                name: "Number",
+                type: "uint64"
+            }
+        ]
+    },
+    primaryType: 'Message',
+    domain: {
+        name: 'example.trezor.io',
+    },
+    message: {
+        "Best Wallet": "Trezor Model T",
+        // be careful with JavaScript numbers: MAX_SAFE_INTEGER is quite low
+        "Number": `${2n ** 55n}`,
+    },
+};
+
+// This functionality is separate from trezor-connect, as it requires @metamask/eth-sig-utils,
+// which is a large JavaScript dependency
+const transformTypedDataPlugin = require("trezor-connect/lib/plugins/ethereum/typedData.js");
+const {domain_separator_hash, message_hash} = transformTypedDataPlugin(eip712Data, true);
+
 TrezorConnect.ethereumSignTypedData({
     path: "m/44'/60'/0'",
-    data: {
-        types: {
-            EIP712Domain: [
-                {
-                    name: 'name',
-                    type: 'string',
-                },
-            ],
-            Message: [
-                {
-                    name: "Best Wallet",
-                    type: "string"
-                },
-                {
-                    name: "Number",
-                    type: "uint64"
-                }
-            ]
-        },
-        primaryType: 'Message',
-        domain: {
-            name: 'example.trezor.io',
-        },
-        message: {
-            "Best Wallet": "Trezor Model T",
-            // be careful with JavaScript numbers: MAX_SAFE_INTEGER is quite low
-            "Number": `${2n ** 55n}`,
-        },
-    },
+    data: eip712Data,
     metamask_v4_compat: true,
+    // These are optional, but required for Trezor Model 1 compatibility
+    domain_separator_hash,
+    message_hash,
 });
 ```
 

diff --git a/src/js/core/methods/EthereumSignTypedData.js b/src/js/core/methods/EthereumSignTypedData.js
@@ -9,15 +9,17 @@ import type { MessageResponse, EthereumTypedDataStructAck } from '../../types/tr
 import { ERRORS } from '../../constants';
 import type {
     EthereumSignTypedData as EthereumSignTypedDataParams,
-    EthereumSignTypedHash as EthereumSignTypedHashParams,
+    EthereumSignTypedHashOrData as EthereumSignTypedHashOrDataParams,
 } from '../../types/networks/ethereum';
 import { getFieldType, parseArrayType, encodeData } from './helpers/ethereumSignTypedData';
 
-type Params = {
-    address_n: number[],
-    ...$Exact<EthereumSignTypedDataParams>,
-    ...$Exact<EthereumSignTypedHashParams>,
-};
+type Params = $Diff<
+    {
+        address_n: number[],
+        ...$Exact<EthereumSignTypedDataParams> | $Exact<EthereumSignTypedHashOrDataParams>,
+    },
+    { path: any }, // removes the "path" variable from this.params
+>;
 
 export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignTypedData'> {
     params: Params;
@@ -30,10 +32,10 @@ export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignT
         // validate incoming parameters
         validateParams(payload, [
             { name: 'path', required: true },
-            { name: 'metamask_v4_compat', type: 'boolean' },
             // model T
-            { name: 'data', type: 'object' },
-            // model One
+            { name: 'metamask_v4_compat', type: 'boolean', required: true },
+            { name: 'data', type: 'object', required: true },
+            // model One (optional params)
             { name: 'domain_separator_hash', type: 'string' },
             { name: 'message_hash', type: 'string' },
         ]);
@@ -45,13 +47,18 @@ export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignT
         this.info = getNetworkLabel('Sign #NETWORK typed data', network);
 
         this.params = {
-            path,
             address_n: path,
             metamask_v4_compat: payload.metamask_v4_compat,
-            domain_separator_hash: payload.domain_separator_hash || '',
-            message_hash: payload.message_hash || '',
-            data: payload.data || undefined,
+            data: payload.data,
         };
+
+        if (payload.message_hash) {
+            this.params = {
+                ...this.params,
+                domain_separator_hash: payload.domain_separator_hash,
+                message_hash: payload.message_hash,
+            };
+        }
     }
 
     async run() {
@@ -64,7 +71,9 @@ export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignT
                 { name: 'message_hash', type: 'string', required: true },
             ]);
 
-            const { domain_separator_hash, message_hash } = this.params;
+            const { domain_separator_hash, message_hash } =
+                // $FlowIssue validateParams() confirms that these hashes exist
+                (this.params: EthereumSignTypedHashOrDataParams);
 
             // For Model 1 we use EthereumSignTypedHash
             const response = await cmd.typedCall(
@@ -84,9 +93,7 @@ export default class EthereumSignTypedData extends AbstractMethod<'ethereumSignT
             };
         }
 
-        validateParams(this.params, [{ name: 'data', type: 'object', required: true }]);
         const { data, metamask_v4_compat } = this.params;
-        // $FlowIssue
         const { types, primaryType, domain, message } = data;
 
         // For Model T we use EthereumSignTypedData

diff --git a/src/js/types/__tests__/ethereum.js b/src/js/types/__tests__/ethereum.js
@@ -242,7 +242,34 @@ export const signTypedData = async () => {
 
     await TrezorConnect.ethereumSignTypedData({
         path: 'm/44',
+        data: {
+            types: {
+                EIP712Domain: [],
+                EmptyMessage: [],
+            },
+            primaryType: 'EmptyMessage',
+            domain: {},
+            message: {},
+        },
         message_hash: '0x',
         domain_separator_hash: '0x',
+        metamask_v4_compat: true,
+    });
+
+    // $FlowExpectedError `message_hash` is given, but it's an invalid type.
+    await TrezorConnect.ethereumSignTypedData({
+        path: 'm/44',
+        data: {
+            types: {
+                EIP712Domain: [],
+                EmptyMessage: [],
+            },
+            primaryType: 'EmptyMessage',
+            domain: {},
+            message: {},
+        },
+        message_hash: 123456,
+        domain_separator_hash: '0x1234',
+        metamask_v4_compat: true,
     });
 };
diff --git a/src/js/types/api.js b/src/js/types/api.js
@@ -254,10 +254,8 @@ export type API = {
     ethereumGetPublicKey: Bundled<Ethereum.EthereumGetPublicKey, Bitcoin.HDNodeResponse>,
     ethereumSignTransaction: Method<Ethereum.EthereumSignTransaction, Ethereum.EthereumSignedTx>,
     ethereumSignMessage: Method<Ethereum.EthereumSignMessage, Protobuf.EthereumMessageSignature>,
-    ethereumSignTypedData: Mixed<
-        Ethereum.EthereumSignTypedData,
-        Ethereum.EthereumSignTypedHash,
-        Protobuf.EthereumTypedDataSignature,
+    ethereumSignTypedData: Method<
+        Ethereum.EthereumSignTypedData | Ethereum.EthereumSignTypedHashOrData,
         Protobuf.EthereumTypedDataSignature,
     >,
 

diff --git a/src/js/types/networks/ethereum.js b/src/js/types/networks/ethereum.js
@@ -101,22 +101,34 @@ type EthereumSignTypedDataMessage<T: EthereumSignTypedDataTypes> = {
     message: { [fieldName: string]: any },
 };
 
+/**
+ * Used for full EIP-712 signing
+ * (currently only supported on Trezor Model T)
+ */
 export type EthereumSignTypedData = {
     path: string | number[],
     metamask_v4_compat: boolean,
-    data: EthereumSignTypedDataMessage<any>,
-    domain_separator_hash?: typeof undefined,
-    message_hash?: typeof undefined,
+    data: EthereumSignTypedDataMessage<EthereumSignTypedDataTypes>,
 };
 
+/**
+ * Used for EIP-712 blind signing on Trezor Model 1 only
+ */
 export type EthereumSignTypedHash = {
     path: string | number[],
-    metamask_v4_compat?: typeof undefined,
-    data?: typeof undefined,
     domain_separator_hash: string,
     message_hash: string,
 };
 
+/**
+ * Used for full EIP-712 signing or blind signing.
+ * Supports both Trezor Model T and Trezor Model 1
+ */
+export type EthereumSignTypedHashOrData = {
+    ...$Exact<EthereumSignTypedData>,
+    ...$Exact<EthereumSignTypedHash>,
+};
+
 // verify message
 
 export type EthereumVerifyMessage = {

diff --git a/src/ts/types/__tests__/ethereum.ts b/src/ts/types/__tests__/ethereum.ts
@@ -235,7 +235,29 @@ export const signTypedData = async () => {
 
     await TrezorConnect.ethereumSignTypedData({
         path: 'm/44',
+        metamask_v4_compat: true,
+        data: {
+            types: { EIP712Domain: [] },
+            primaryType: 'EIP712Domain',
+            domain: {},
+            message: {},
+        },
         message_hash: '0x',
         domain_separator_hash: '0x',
     });
+
+    await TrezorConnect.ethereumSignTypedData({
+        path: 'm/44',
+        metamask_v4_compat: true,
+        data: {
+            types: { EIP712Domain: [] },
+            // @ts-expect-error: primaryType not in `types`
+            primaryType: 'UnknownType',
+            domain: {},
+            message: {},
+        },
+        // @ts-expect-error: incorrect type for message_hash
+        message_hash: 12345,
+        domain_separator_hash: '0x',
+    });
 };
diff --git a/src/ts/types/api.d.ts b/src/ts/types/api.d.ts
@@ -303,11 +303,17 @@ export namespace TrezorConnect {
     function ethereumSignMessage(
         params: P.CommonParams & Ethereum.EthereumSignMessage,
     ): P.Response<Protobuf.MessageSignature>;
+    /**
+     * @param params Passing:
+     * - {@link Ethereum.EthereumSignTypedData} is required for Trezor T
+     * - {@link Ethereum.EthereumSignTypedHash} is required for Trezor 1 compatability
+     */
     function ethereumSignTypedData<T extends Ethereum.EthereumSignTypedDataTypes>(
-        params: P.CommonParams & Ethereum.EthereumSignTypedData<T>,
-    ): P.Response<Protobuf.EthereumTypedDataSignature>;
-    function ethereumSignTypedData(
-        params: P.CommonParams & Ethereum.EthereumSignTypedHash,
+        params: P.CommonParams &
+            (
+                | Ethereum.EthereumSignTypedData<T>
+                | (Ethereum.EthereumSignTypedData<T> & Ethereum.EthereumSignTypedHash)
+            ),
     ): P.Response<Protobuf.EthereumTypedDataSignature>;
     function ethereumVerifyMessage(
         params: P.CommonParams & Ethereum.EthereumVerifyMessage,

diff --git a/src/ts/types/networks/ethereum.d.ts b/src/ts/types/networks/ethereum.d.ts
@@ -105,6 +105,10 @@ export interface EthereumSignTypedData<T extends EthereumSignTypedDataTypes> {
     metamask_v4_compat: boolean;
 }
 
+/**
+ * The Trezor Model 1 cannot currently calculate EIP-712 hashes by itself,
+ * so we have to precalculate them.
+ */
 export interface EthereumSignTypedHash {
     path: string | number[];
     domain_separator_hash: string;