Add EIP-712 signTypedData_v4 support (#117)

* Add EIP-712 signTypedData_v4 support for Model T

Required updating trezor-connect to v8.2.5-extended

* Call this.getModel() after this.unlock()

There is potentially an issue the this.getModel() branch,
see #117 (comment)

I think this might be because the model information
has not yet been loaded, so I've moved it to after the unlock,
and added the current model in the Error message for debugging.

* Fix failing test after EIP-712 model check moved

* Load model if this.getModel() returns undefined

Apparently this.getModel() sometimes returns `undefined`.

If this happens, since we're in an `async` function,
we can load the model using the official Trezor API.

Co-authored-by: alisinabh <[email protected]>

* Cleanup keyring while running tests

Prevents NodeJS from printing a memory leak warning.

* Adapt signTypedData to support Trezor one

* Remove primaryType: EIP712Domain tests

There is currently a mismatch between Metamask and Trezor connect
in this case, so future versions of Trezor connect will most
likely change this behaviour.

See trezor/trezor-firmware#2036

* Fix linting issues in @alisinabh commit

Fixes lint issues in f368848

* Update trezor-connect to 8.2.6-extended

This should add EIP-712 support for Model 1 too.

* Fix incorrect EIP-712 hash passing for Model 1

The domain and message hash do not go into the data object,
see #117 (comment)

Co-authored-by: Brandon Noad <[email protected]>

Co-authored-by: alisinabh <[email protected]>
Co-authored-by: Brandon Noad <[email protected]>

Author: 3 people

CHANGELOG.md

@@ -5,6 +5,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
+### Added
+- Support for EIP-721 signTypedData_v4 ([#117](https://github.com/MetaMask/eth-trezor-keyring/pull/117))
 
 ## [0.9.1]
 ### Changed

index.js

@@ -3,6 +3,7 @@ const ethUtil = require('ethereumjs-util');
 const HDKey = require('hdkey');
 const TrezorConnect = require('trezor-connect').default;
 const { TransactionFactory } = require('@ethereumjs/tx');
+const transformTypedData = require('trezor-connect/lib/plugins/ethereum/typedData');
 
 const hdPathString = `m/44'/60'/0'/0`;
 const SLIP0044TestnetPath = `m/44'/1'/0'/0`;
@@ -67,6 +68,12 @@ class TrezorKeyring extends EventEmitter {
     TrezorConnect.init({ manifest: TREZOR_CONNECT_MANIFEST });
   }
 
+  /**
+   * Gets the model, if known.
+   * This may be `undefined` if the model hasn't been loaded yet.
+   *
+   * @returns {"T" | "1" | undefined}
+   */
   getModel() {
     return this.model;
   }
@@ -384,9 +391,53 @@ class TrezorKeyring extends EventEmitter {
     });
   }
 
-  signTypedData() {
-    // Waiting on trezor to enable this
-    return Promise.reject(new Error('Not supported on this device'));
+  /**
+   * EIP-712 Sign Typed Data
+   */
+  async signTypedData(address, data, { version }) {
+    const dataWithHashes = transformTypedData(data, version === 'V4');
+
+    // set default values for signTypedData
+    // Trezor is stricter than @metamask/eth-sig-util in what it accepts
+    const {
+      types: { EIP712Domain = [], ...otherTypes } = {},
+      message = {},
+      domain = {},
+      primaryType,
+      // snake_case since Trezor uses Protobuf naming conventions here
+      domain_separator_hash, // eslint-disable-line camelcase
+      message_hash, // eslint-disable-line camelcase
+    } = dataWithHashes;
+
+    // This is necessary to avoid popup collision
+    // between the unlock & sign trezor popups
+    const status = await this.unlock();
+    await wait(status === 'just unlocked' ? DELAY_BETWEEN_POPUPS : 0);
+
+    const response = await TrezorConnect.ethereumSignTypedData({
+      path: this._pathFromAddress(address),
+      data: {
+        types: { EIP712Domain, ...otherTypes },
+        message,
+        domain,
+        primaryType,
+      },
+      metamask_v4_compat: true,
+      // Trezor 1 only supports blindly signing hashes
+      domain_separator_hash,
+      message_hash,
+    });
+
+    if (response.success) {
+      if (ethUtil.toChecksumAddress(address) !== response.payload.address) {
+        throw new Error('signature doesnt match the right address');
+      }
+      return response.payload.signature;
+    }
+
+    throw new Error(
+      (response.payload && response.payload.error) || 'Unknown error',
+    );
   }
 
   exportAccount() {

package.json

@@ -35,9 +35,10 @@
   },
   "dependencies": {
     "@ethereumjs/tx": "^3.2.1",
+    "@metamask/eth-sig-util": "^4.0.0",
     "ethereumjs-util": "^7.0.9",
     "hdkey": "0.8.0",
-    "trezor-connect": "8.2.3-extended"
+    "trezor-connect": "8.2.6-extended"
   },
   "devDependencies": {
     "@ethereumjs/common": "^2.4.0",

test/test-eth-trezor-keyring.js

@@ -108,6 +108,9 @@ describe('TrezorKeyring', function () {
   });
 
   afterEach(function () {
+    if (keyring) {
+      keyring.dispose();
+    }
     sinon.restore();
   });
 
@@ -556,16 +559,52 @@ describe('TrezorKeyring', function () {
   });
 
   describe('signTypedData', function () {
-    it('should throw an error because it is not supported', async function () {
+    it('should throw an error on signTypedData_v3 because it is not supported', async function () {
       let error = null;
       try {
-        await keyring.signTypedData();
+        await keyring.signTypedData(null, null, { version: 'V3' });
       } catch (e) {
         error = e;
       }
 
-      expect(error instanceof Error, true);
-      expect(error.toString(), 'Not supported on this device');
+      expect(error).to.be.an.instanceof(Error);
+      expect(error.toString()).to.contain(
+        'Only version 4 of typed data signing is supported',
+      );
+    });
+
+    it('should call TrezorConnect.ethereumSignTypedData', async function () {
+      sinon
+        .stub(TrezorConnect, 'ethereumSignTypedData')
+        .callsFake(async () => ({
+          success: true,
+          payload: { signature: '0x00', address: fakeAccounts[0] },
+        }));
+
+      this.timeout = 60000;
+      await keyring.signTypedData(
+        fakeAccounts[0],
+        // Message with missing data that @metamask/eth-sig-util accepts
+        { types: { EmptyMessage: [] }, primaryType: 'EmptyMessage' },
+        { version: 'V4' },
+      );
+
+      assert(TrezorConnect.ethereumSignTypedData.calledOnce);
+      sinon.assert.calledWithExactly(TrezorConnect.ethereumSignTypedData, {
+        path: "m/44'/60'/0'/0/0",
+        data: {
+          // Empty message that trezor-connect/EIP-712 spec accepts
+          types: { EIP712Domain: [], EmptyMessage: [] },
+          primaryType: 'EmptyMessage',
+          domain: {},
+          message: {},
+        },
+        metamask_v4_compat: true,
+        domain_separator_hash:
+          '6192106f129ce05c9075d319c1fa6ea9b3ae37cbd0c1ef92e2be7137bb07baa1',
+        message_hash:
+          'c9e71eb57cf9fa86ec670283b58cb15326bb6933c8d8e2ecb2c0849021b3ef42',
+      });
     });
   });