Add verifyContract address normalization (#309)

* Add verifyContract address normalization

* Fix suggestions

* Fix usage of normalizeTypedMessage

* Relocate type assertion

* Add extra unit test for parser method

* Add isStrictHexString to narrow down type check and remove cast

* Remove int conversion

* Fix lint

* Remove error

* Add non parsable unit test case

* Add basic signTypedDataV3 test

package.json

@@ -34,6 +34,8 @@
     "@metamask/json-rpc-engine": "^8.0.2",
     "@metamask/rpc-errors": "^6.0.0",
     "@metamask/utils": "^8.1.0",
+    "@types/bn.js": "^5.1.5",
+    "bn.js": "^5.2.1",
     "klona": "^2.0.6",
     "pify": "^5.0.0",
     "safe-stable-stringify": "^2.4.3"

src/utils/normalize.test.ts

@@ -0,0 +1,154 @@
+import { normalizeTypedMessage } from './normalize';
+
+const MESSAGE_DATA_MOCK = {
+  types: {
+    Permit: [
+      {
+        name: 'owner',
+        type: 'address',
+      },
+      {
+        name: 'spender',
+        type: 'address',
+      },
+      {
+        name: 'value',
+        type: 'uint256',
+      },
+      {
+        name: 'nonce',
+        type: 'uint256',
+      },
+      {
+        name: 'deadline',
+        type: 'uint256',
+      },
+    ],
+    EIP712Domain: [
+      {
+        name: 'name',
+        type: 'string',
+      },
+      {
+        name: 'version',
+        type: 'string',
+      },
+      {
+        name: 'chainId',
+        type: 'uint256',
+      },
+      {
+        name: 'verifyingContract',
+        type: 'address',
+      },
+    ],
+  },
+  domain: {
+    name: 'Liquid staked Ether 2.0',
+    version: '2',
+    chainId: '0x1',
+    verifyingContract: '996101235222674412020337938588541139382869425796',
+  },
+  primaryType: 'Permit',
+  message: {
+    owner: '0x6d404afe1a6a07aa3cbcbf9fd027671df628ebfc',
+    spender: '0x63605E53D422C4F1ac0e01390AC59aAf84C44A51',
+    value:
+      '115792089237316195423570985008687907853269984665640564039457584007913129639935',
+    nonce: '0',
+    deadline: '4482689033',
+  },
+};
+
+describe('normalizeTypedMessage', () => {
+  function parseNormalizerResult(data: Record<string, unknown>) {
+    return JSON.parse(normalizeTypedMessage(JSON.stringify(data)));
+  }
+
+  it('should normalize verifyingContract address in domain', () => {
+    const normalizedData = parseNormalizerResult(MESSAGE_DATA_MOCK);
+    expect(normalizedData.domain.verifyingContract).toBe(
+      '0xae7ab96520de3a18e5e111b5eaab095312d7fe84',
+    );
+  });
+
+  it('should normalize verifyingContract address in domain when provided data is an object', () => {
+    const NON_STRINGIFIED_MESSAGE_DATA_MOCK = MESSAGE_DATA_MOCK;
+    const normalizedData = JSON.parse(
+      normalizeTypedMessage(
+        NON_STRINGIFIED_MESSAGE_DATA_MOCK as unknown as string,
+      ),
+    );
+    expect(normalizedData.domain.verifyingContract).toBe(
+      '0xae7ab96520de3a18e5e111b5eaab095312d7fe84',
+    );
+  });
+
+  it('should handle octal verifyingContract address by normalizing it', () => {
+    const expectedNormalizedOctalAddress = '0x53';
+    const messageDataWithOctalAddress = {
+      ...MESSAGE_DATA_MOCK,
+      domain: {
+        ...MESSAGE_DATA_MOCK.domain,
+        verifyingContract: '0o123',
+      },
+    };
+
+    const normalizedData = parseNormalizerResult(messageDataWithOctalAddress);
+
+    expect(normalizedData.domain.verifyingContract).toBe(
+      expectedNormalizedOctalAddress,
+    );
+  });
+
+  it('should not modify if verifyingContract is already hexadecimal', () => {
+    const expectedVerifyingContract =
+      '0xae7ab96520de3a18e5e111b5eaab095312d7fe84';
+    const messageDataWithHexAddress = {
+      ...MESSAGE_DATA_MOCK,
+      domain: {
+        ...MESSAGE_DATA_MOCK.domain,
+        verifyingContract: expectedVerifyingContract,
+      },
+    };
+
+    const normalizedData = parseNormalizerResult(messageDataWithHexAddress);
+
+    expect(normalizedData.domain.verifyingContract).toBe(
+      expectedVerifyingContract,
+    );
+  });
+
+  it('should not modify if verifyingContract is not parsable', () => {
+    const expectedVerifyingContract =
+      'Notparsableaddress1234567890123456789012345678901234567890';
+    const messageDataWithHexAddress = {
+      ...MESSAGE_DATA_MOCK,
+      domain: {
+        ...MESSAGE_DATA_MOCK.domain,
+        verifyingContract: expectedVerifyingContract,
+      },
+    };
+
+    const normalizedData = parseNormalizerResult(messageDataWithHexAddress);
+
+    expect(normalizedData.domain.verifyingContract).toBe(
+      expectedVerifyingContract,
+    );
+  });
+
+  it('should not modify other parts of the message data', () => {
+    const normalizedData = parseNormalizerResult(MESSAGE_DATA_MOCK);
+    expect(normalizedData.message).toStrictEqual(MESSAGE_DATA_MOCK.message);
+    expect(normalizedData.types).toStrictEqual(MESSAGE_DATA_MOCK.types);
+    expect(normalizedData.primaryType).toStrictEqual(
+      MESSAGE_DATA_MOCK.primaryType,
+    );
+  });
+
+  it('should return data as is if not parsable', () => {
+    expect(normalizeTypedMessage('Not parsable data')).toBe(
+      'Not parsable data',
+    );
+  });
+});

src/utils/normalize.ts

@@ -0,0 +1,89 @@
+import { add0x, isValidHexAddress, isStrictHexString } from '@metamask/utils';
+import type { Hex } from '@metamask/utils';
+import BN from 'bn.js';
+
+type EIP712Domain = {
+  verifyingContract: string;
+};
+
+type SignTypedMessageDataV3V4 = {
+  types: Record<string, unknown>;
+  domain: EIP712Domain;
+  primaryType: string;
+  message: unknown;
+};
+
+/**
+ * Normalizes the messageData for the eth_signTypedData
+ *
+ * @param messageData - The messageData to normalize.
+ * @returns The normalized messageData.
+ */
+export function normalizeTypedMessage(messageData: string) {
+  let data;
+  try {
+    data = parseTypedMessage(messageData);
+  } catch (e) {
+    // Ignore normalization errors and pass the message as is
+    return messageData;
+  }
+
+  const { verifyingContract } = data.domain ?? {};
+
+  if (!verifyingContract) {
+    return messageData;
+  }
+
+  data.domain.verifyingContract = normalizeContractAddress(verifyingContract);
+
+  return JSON.stringify(data);
+}
+
+/**
+ * Parses the messageData to obtain the data object for EIP712 normalization
+ *
+ * @param data - The messageData to parse.
+ * @returns The data object for EIP712 normalization.
+ */
+function parseTypedMessage(data: string) {
+  if (typeof data !== 'string') {
+    return data;
+  }
+
+  return JSON.parse(data) as unknown as SignTypedMessageDataV3V4;
+}
+
+/**
+ * Normalizes the address to a hexadecimal format
+ *
+ * @param address - The address to normalize.
+ * @returns The normalized address.
+ */
+function normalizeContractAddress(address: string): Hex | string {
+  if (isStrictHexString(address) && isValidHexAddress(address)) {
+    return address;
+  }
+
+  // Check if the address is in octal format, convert to hexadecimal
+  if (address.startsWith('0o')) {
+    // If octal, convert to hexadecimal
+    return octalToHex(address);
+  }
+
+  // Check if the address is in decimal format, convert to hexadecimal
+  try {
+    const decimalBN = new BN(address, 10);
+    const hexString = decimalBN.toString(16);
+    return add0x(hexString);
+  } catch (e) {
+    // Ignore errors and return the original address
+  }
+
+  // Returning the original address without normalization
+  return address;
+}
+
+function octalToHex(octalAddress: string): Hex {
+  const decimalAddress = parseInt(octalAddress.slice(2), 8).toString(16);
+  return add0x(decimalAddress);
+}

src/wallet.test.ts

@@ -2,7 +2,12 @@ import { providerFromEngine } from '@metamask/eth-json-rpc-provider';
 import { JsonRpcEngine } from '@metamask/json-rpc-engine';
 import pify from 'pify';
 
-import type { TransactionParams, MessageParams, TypedMessageV1Params } from '.';
+import type {
+  MessageParams,
+  TransactionParams,
+  TypedMessageParams,
+  TypedMessageV1Params,
+} from '.';
 import { createWalletMiddleware } from '.';
 
 const testAddresses = [
@@ -326,6 +331,68 @@ describe('wallet', () => {
     });
   });
 
+  describe('signTypedDataV3', () => {
+    it('should sign data and normalizes verifyingContract', async () => {
+      const { engine } = createTestSetup();
+      const getAccounts = async () => testAddresses.slice();
+      const witnessedMsgParams: TypedMessageParams[] = [];
+      const processTypedMessageV3 = async (msgParams: TypedMessageParams) => {
+        witnessedMsgParams.push(msgParams);
+        // Assume testMsgSig is the expected signature result
+        return testMsgSig;
+      };
+
+      engine.push(
+        createWalletMiddleware({ getAccounts, processTypedMessageV3 }),
+      );
+
+      const message = {
+        types: {
+          EIP712Domain: [
+            { name: 'name', type: 'string' },
+            { name: 'version', type: 'string' },
+            { name: 'chainId', type: 'uint256' },
+            { name: 'verifyingContract', type: 'address' },
+          ],
+        },
+        primaryType: 'EIP712Domain',
+        domain: {
+          verifyingContract: '996101235222674412020337938588541139382869425796',
+        },
+        message: {},
+      };
+
+      const stringifiedMessage = JSON.stringify(message);
+      const expectedStringifiedMessage = JSON.stringify({
+        ...message,
+        domain: {
+          verifyingContract: '0xae7ab96520de3a18e5e111b5eaab095312d7fe84',
+        },
+      });
+
+      const payload = {
+        method: 'eth_signTypedData_v3',
+        params: [testAddresses[0], stringifiedMessage], // Assuming testAddresses[0] is a valid address from your setup
+      };
+
+      const signTypedDataV3Response = await pify(engine.handle).call(
+        engine,
+        payload,
+      );
+      const signTypedDataV3Result = signTypedDataV3Response.result;
+
+      expect(signTypedDataV3Result).toBeDefined();
+      expect(signTypedDataV3Result).toStrictEqual(testMsgSig);
+      expect(witnessedMsgParams).toHaveLength(1);
+      expect(witnessedMsgParams[0]).toMatchObject({
+        from: testAddresses[0],
+        data: expectedStringifiedMessage,
+        version: 'V3',
+        signatureMethod: 'eth_signTypedData_v3',
+      });
+    });
+  });
+
   describe('sign', () => {
     it('should sign with a valid address', async () => {
       const { engine } = createTestSetup();

src/wallet.ts

@@ -12,6 +12,7 @@ import type {
 } from '@metamask/utils';
 
 import type { Block } from './types';
+import { normalizeTypedMessage } from './utils/normalize';
 
 /*
 export type TransactionParams = {
@@ -276,7 +277,7 @@ WalletMiddlewareOptions): JsonRpcMiddleware<any, Block> {
     const params = req.params as [string, string];
 
     const address = await validateAndNormalizeKeyholder(params[0], req);
-    const message = params[1];
+    const message = normalizeTypedMessage(params[1]);
     const version = 'V3';
     const msgParams: TypedMessageParams = {
       data: message,
@@ -306,7 +307,7 @@ WalletMiddlewareOptions): JsonRpcMiddleware<any, Block> {
     const params = req.params as [string, string];
 
     const address = await validateAndNormalizeKeyholder(params[0], req);
-    const message = params[1];
+    const message = normalizeTypedMessage(params[1]);
     const version = 'V4';
     const msgParams: TypedMessageParams = {
       data: message,