feat(keyring-eth-ledger-bridge)!: enable ledger clear signing

packages/keyring-eth-ledger-bridge/jest.config.js

@@ -23,10 +23,10 @@ module.exports = merge(baseConfig, {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 90.14,
-      functions: 95.95,
-      lines: 94.77,
-      statements: 94.83,
+      branches: 90.9,
+      functions: 96,
+      lines: 95.03,
+      statements: 95.09,
     },
   },
 });

packages/keyring-eth-ledger-bridge/package.json

@@ -61,6 +61,7 @@
     "@ledgerhq/hw-transport": "^6.31.3",
     "@ledgerhq/types-cryptoassets": "^7.15.1",
     "@ledgerhq/types-devices": "^6.25.3",
+    "@ledgerhq/types-live": "^6.52.0",
     "@metamask/auto-changelog": "^3.4.4",
     "@metamask/utils": "^9.3.0",
     "@ts-bridge/cli": "^0.6.0",

packages/keyring-eth-ledger-bridge/src/ledger-bridge.ts

@@ -1,5 +1,6 @@
 import type LedgerHwAppEth from '@ledgerhq/hw-app-eth';
 import type Transport from '@ledgerhq/hw-transport';
+import { EIP712Message } from '@ledgerhq/types-live';
 
 export type GetPublicKeyParams = { hdPath: string };
 export type GetPublicKeyResponse = Awaited<
@@ -18,8 +19,7 @@ export type LedgerSignMessageResponse = Awaited<
 
 export type LedgerSignTypedDataParams = {
   hdPath: string;
-  domainSeparatorHex: string;
-  hashStructMessageHex: string;
+  message: EIP712Message;
 };
 export type LedgerSignTypedDataResponse = Awaited<
   ReturnType<LedgerHwAppEth['signEIP712HashedMessage']>

packages/keyring-eth-ledger-bridge/src/ledger-iframe-bridge.test.ts

@@ -177,6 +177,29 @@ describe('LedgerIframeBridge', function () {
       // eslint-disable-next-line @typescript-eslint/unbound-method
       expect(bridge.iframe?.contentWindow?.postMessage).toHaveBeenCalled();
     });
+
+    it('throws an error when unknown error occur', async function () {
+      const errorMessage = 'Unknown error occurred';
+
+      stubKeyringIFramePostMessage(bridge, (message) => {
+        expect(message).toStrictEqual({
+          action: IFrameMessageAction.LedgerMakeApp,
+          messageId: 1,
+          target: LEDGER_IFRAME_ID,
+        });
+
+        bridge.messageCallbacks[message.messageId]?.({
+          action: IFrameMessageAction.LedgerMakeApp,
+          messageId: 1,
+          success: false,
+        });
+      });
+
+      await expect(bridge.attemptMakeApp()).rejects.toThrow(errorMessage);
+
+      // eslint-disable-next-line @typescript-eslint/unbound-method
+      expect(bridge.iframe?.contentWindow?.postMessage).toHaveBeenCalled();
+    });
   });
 
   describe('updateTransportMethod', function () {
@@ -435,17 +458,28 @@ describe('LedgerIframeBridge', function () {
   });
 
   describe('deviceSignTypedData', function () {
+    const params = {
+      hdPath: "m/44'/60'/0'/0",
+      message: {
+        domain: {
+          chainId: 1,
+          verifyingContract: '0xdsf',
+        },
+        primaryType: 'string',
+        types: {
+          EIP712Domain: [],
+          string: [],
+        },
+        message: { test: 'test' },
+      },
+    };
+
     it('sends and processes a successful ledger-sign-typed-data message', async function () {
       const payload = {
         v: 0,
         r: '',
         s: '',
       };
-      const params = {
-        hdPath: "m/44'/60'/0'/0",
-        domainSeparatorHex: '',
-        hashStructMessageHex: '',
-      };
 
       stubKeyringIFramePostMessage(bridge, (message) => {
         expect(message).toStrictEqual({
@@ -473,11 +507,6 @@ describe('LedgerIframeBridge', function () {
 
     it('throws an error when a ledger-sign-typed-data message is not successful', async function () {
       const errorMessage = 'Ledger Error';
-      const params = {
-        hdPath: "m/44'/60'/0'/0",
-        domainSeparatorHex: '',
-        hashStructMessageHex: '',
-      };
 
       stubKeyringIFramePostMessage(bridge, (message) => {
         expect(message).toStrictEqual({

packages/keyring-eth-ledger-bridge/src/ledger-keyring.test.ts

@@ -928,6 +928,7 @@ describe('LedgerKeyring', function () {
           name: 'Ether Mail',
           verifyingContract: '0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC',
           version: '1',
+          salt: new TextEncoder().encode('hello'),
         },
         message: {
           contents: 'Hello, Bob!',
@@ -1000,6 +1001,33 @@ describe('LedgerKeyring', function () {
         );
       });
 
+      it('resolves properly when message domain salt is undefined', async function () {
+        const fixtureDataWithoutSalt = {
+          ...fixtureData,
+          domain: {
+            ...fixtureData.domain,
+            salt: undefined,
+          },
+        };
+
+        jest
+          .spyOn(keyring.bridge, 'deviceSignTypedData')
+          .mockImplementation(async () => ({
+            v: 27,
+            r: '72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b9',
+            s: '46759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e32',
+          }));
+
+        const result = await keyring.signTypedData(
+          fakeAccounts[15],
+          fixtureDataWithoutSalt,
+          options,
+        );
+        expect(result).toBe(
+          '0x72d4e38a0e582e09a620fd38e236fe687a1ec782206b56d576f579c026a7e5b946759735981cd0c3efb02d36df28bb2feedfec3d90e408efc93f45b894946e321b',
+        );
+      });
+
       it('throws error when address does not match', async function () {
         jest
           .spyOn(keyring.bridge, 'deviceSignTypedData')

packages/keyring-eth-ledger-bridge/src/ledger-keyring.ts

@@ -494,18 +494,6 @@ export class LedgerKeyring extends EventEmitter {
 
     const { domain, types, primaryType, message } =
       TypedDataUtils.sanitizeData(data);
-    const domainSeparatorHex = TypedDataUtils.hashStruct(
-      'EIP712Domain',
-      domain,
-      types,
-      SignTypedDataVersion.V4,
-    ).toString('hex');
-    const hashStructMessageHex = TypedDataUtils.hashStruct(
-      primaryType.toString(),
-      message,
-      types,
-      SignTypedDataVersion.V4,
-    ).toString('hex');
 
     const hdPath = await this.unlockAccountByAddress(withAccount);
 
@@ -517,8 +505,18 @@ export class LedgerKeyring extends EventEmitter {
     try {
       payload = await this.bridge.deviceSignTypedData({
         hdPath,
-        domainSeparatorHex,
-        hashStructMessageHex,
+        message: {
+          domain: {
+            name: domain.name,
+            chainId: domain.chainId,
+            version: domain.version,
+            verifyingContract: domain.verifyingContract,
+            salt: this.#convertSaltIfAny(domain.salt),
+          },
+          types,
+          primaryType: primaryType.toString(),
+          message,
+        },
       });
     } catch (error) {
       throw error instanceof Error
@@ -560,6 +558,17 @@ export class LedgerKeyring extends EventEmitter {
     this.hdk = new HDKey();
   }
 
+  #convertSaltIfAny(salt: ArrayBuffer | undefined): string | undefined {
+    if (!salt) {
+      return undefined;
+    }
+
+    // We convert this to a plain string to avoid encoding issue on the
+    // mobile side (to avoid using `TextDecoder`).
+    const saltBytes = new Uint8Array(salt);
+    return String.fromCharCode(...saltBytes);
+  }
+
   /* PRIVATE METHODS */
   async #getPage(increment: number): Promise<AccountPage> {
     this.page += increment;

packages/keyring-eth-ledger-bridge/src/ledger-mobile-bridge.test.ts

@@ -1,5 +1,5 @@
-import ledgerService from '@ledgerhq/hw-app-eth/lib/services/ledger';
 import Transport from '@ledgerhq/hw-transport';
+import { EIP712Message } from '@ledgerhq/types-live';
 
 import { MetaMaskLedgerHwAppEth } from './ledger-hw-app';
 import { LedgerMobileBridge } from './ledger-mobile-bridge';
@@ -19,8 +19,8 @@ describe('LedgerMobileBridge', function () {
   let transportMiddlewareGetEthAppSpy: jest.SpyInstance;
 
   const mockEthApp = {
-    signEIP712HashedMessage: jest.fn(),
-    signTransaction: jest.fn(),
+    signEIP712Message: jest.fn(),
+    clearSignTransaction: jest.fn(),
     getAddress: jest.fn(),
     signPersonalMessage: jest.fn(),
     openEthApp: jest.fn(),
@@ -155,50 +155,62 @@ describe('LedgerMobileBridge', function () {
       const hdPath = "m/44'/60'/0'/0/0";
       const tx =
         'f86d8202b38477359400825208944592d8f8d7b001e72cb26a73e4fa1806a51ac79d880de0b6b3a7640000802ba0699ff162205967ccbabae13e07cdd4284258d46ec1051a70a51be51ec2bc69f3a04e6944d508244ea54a62ebf9a72683eeadacb73ad7c373ee542f1998147b220e';
-      const resolution = await ledgerService.resolveTransaction(tx, {}, {});
       await bridge.deviceSignTransaction({
         hdPath,
         tx,
       });
       expect(transportMiddlewareGetEthAppSpy).toHaveBeenCalledTimes(1);
-      expect(mockEthApp.signTransaction).toHaveBeenCalledTimes(1);
-      expect(mockEthApp.signTransaction).toHaveBeenCalledWith(
-        hdPath,
-        tx,
-        resolution,
-      );
+      expect(mockEthApp.clearSignTransaction).toHaveBeenCalledTimes(1);
+      expect(mockEthApp.clearSignTransaction).toHaveBeenCalledWith(hdPath, tx, {
+        externalPlugins: true,
+        erc20: true,
+        nft: true,
+      });
     });
 
-    it('throws an error when tx format is not correct', async function () {
+    it('returns undefined when tx format is not correct', async function () {
       const hdPath = "m/44'/60'/0'/0/0";
       const tx = '';
-      await expect(
-        bridge.deviceSignTransaction({
+      expect(
+        await bridge.deviceSignTransaction({
           hdPath,
           tx,
         }),
-      ).rejects.toThrow(Error);
-      expect(transportMiddlewareGetEthAppSpy).toHaveBeenCalledTimes(0);
-      expect(mockEthApp.signTransaction).toHaveBeenCalledTimes(0);
+      ).toBeUndefined();
+      expect(transportMiddlewareGetEthAppSpy).toHaveBeenCalledTimes(1);
+      expect(mockEthApp.clearSignTransaction).toHaveBeenCalledTimes(1);
+      expect(mockEthApp.clearSignTransaction).toHaveBeenCalledWith(hdPath, tx, {
+        externalPlugins: true,
+        erc20: true,
+        nft: true,
+      });
     });
   });
 
   describe('deviceSignTypedData', function () {
     it('sends and processes a successful ledger-sign-typed-data message', async function () {
       const hdPath = "m/44'/60'/0'/0/0";
-      const domainSeparatorHex = 'domainSeparatorHex';
-      const hashStructMessageHex = 'hashStructMessageHex';
+      const message: EIP712Message = {
+        domain: {
+          chainId: 1,
+          verifyingContract: '0xdsf',
+        },
+        primaryType: 'string',
+        types: {
+          EIP712Domain: [],
+          string: [],
+        },
+        message: { test: 'test' },
+      };
       await bridge.deviceSignTypedData({
         hdPath,
-        domainSeparatorHex,
-        hashStructMessageHex,
+        message,
       });
       expect(transportMiddlewareGetEthAppSpy).toHaveBeenCalledTimes(1);
-      expect(mockEthApp.signEIP712HashedMessage).toHaveBeenCalledTimes(1);
-      expect(mockEthApp.signEIP712HashedMessage).toHaveBeenCalledWith(
+      expect(mockEthApp.signEIP712Message).toHaveBeenCalledTimes(1);
+      expect(mockEthApp.signEIP712Message).toHaveBeenCalledWith(
         hdPath,
-        domainSeparatorHex,
-        hashStructMessageHex,
+        message,
       );
     });
   });

packages/keyring-eth-ledger-bridge/src/ledger-mobile-bridge.ts

@@ -1,4 +1,3 @@
-import ledgerService from '@ledgerhq/hw-app-eth/lib/services/ledger';
 import type Transport from '@ledgerhq/hw-transport';
 
 import {
@@ -90,20 +89,14 @@ export class LedgerMobileBridge implements MobileBridge {
    *
    * @param params - An object contains hdPath, domainSeparatorHex and hashStructMessageHex.
    * @param params.hdPath - The BIP 32 path of the account.
-   * @param params.domainSeparatorHex - The domain separator.
-   * @param params.hashStructMessageHex - The hashed struct message.
+   * @param params.message - The EIP712 message to sign.
    * @returns Retrieve v, r, s from the signed message.
    */
   async deviceSignTypedData({
     hdPath,
-    domainSeparatorHex,
-    hashStructMessageHex,
+    message,
   }: LedgerSignTypedDataParams): Promise<LedgerSignTypedDataResponse> {
-    return this.#getEthApp().signEIP712HashedMessage(
-      hdPath,
-      domainSeparatorHex,
-      hashStructMessageHex,
-    );
+    return this.#getEthApp().signEIP712Message(hdPath, message);
   }
 
   /**
@@ -119,8 +112,11 @@ export class LedgerMobileBridge implements MobileBridge {
     tx,
     hdPath,
   }: LedgerSignTransactionParams): Promise<LedgerSignTransactionResponse> {
-    const resolution = await ledgerService.resolveTransaction(tx, {}, {});
-    return this.#getEthApp().signTransaction(hdPath, tx, resolution);
+    return this.#getEthApp().clearSignTransaction(hdPath, tx, {
+      externalPlugins: true,
+      erc20: true,
+      nft: true,
+    });
   }
 
   /**

yarn.lock

@@ -1900,6 +1900,7 @@ __metadata:
     "@ledgerhq/hw-transport": "npm:^6.31.3"
     "@ledgerhq/types-cryptoassets": "npm:^7.15.1"
     "@ledgerhq/types-devices": "npm:^6.25.3"
+    "@ledgerhq/types-live": "npm:^6.52.0"
     "@metamask/auto-changelog": "npm:^3.4.4"
     "@metamask/eth-sig-util": "npm:^8.0.0"
     "@metamask/utils": "npm:^9.3.0"