diff --git a/packages/internal/toolkit/package.json b/packages/internal/toolkit/package.json index 0ba440999a..45b292ce7a 100644 --- a/packages/internal/toolkit/package.json +++ b/packages/internal/toolkit/package.json @@ -6,6 +6,7 @@ "bugs": "https://github.com/immutable/ts-immutable-sdk/issues", "dependencies": { "@imtbl/x-client": "workspace:*", + "@imtbl/metrics": "workspace:*", "@magic-ext/oidc": "12.0.2", "@metamask/detect-provider": "^2.0.0", "axios": "^1.6.5", diff --git a/packages/internal/toolkit/src/crypto.ts b/packages/internal/toolkit/src/crypto.ts index 67e2815e2c..bacbd1e795 100644 --- a/packages/internal/toolkit/src/crypto.ts +++ b/packages/internal/toolkit/src/crypto.ts @@ -1,6 +1,7 @@ import BN from 'bn.js'; import * as encUtils from 'enc-utils'; -import { Signer } from 'ethers'; +import { Signer, toUtf8Bytes } from 'ethers'; +import { track } from '@imtbl/metrics'; type SignatureOptions = { r: BN; @@ -42,7 +43,70 @@ export async function signRaw( payload: string, signer: Signer, ): Promise { - const signature = deserializeSignature(await signer.signMessage(payload)); + const address = await signer.getAddress(); + track('xProvider', 'log', { + address, + param: 'signRaw.payload', + val: payload, + }); + track('xProvider', 'log', { + address, + param: 'signRaw.toUtf8Bytes', + val: toUtf8Bytes(payload).toString(), + }); + track('xProvider', 'log', { + address, + param: 'signRaw.payload.normalize() === payload', + val: payload === payload.normalize(), + }); + + // prevent utf-8 encoding issues + const encoder = new TextEncoder(); + const buffer = encoder.encode(payload); + // use this message to sign + const message = new TextDecoder('utf-8').decode(buffer); + + const buffer2 = Buffer.from(payload, 'utf8'); + const message2 = new TextDecoder('utf-8').decode(buffer2); + + // compare message utf8 bytes with payload.normalize() + track('xProvider', 'log', { + address, + param: 'signRaw.message === payload.normalize()', + val: message === payload.normalize(), + }); + track('xProvider', 'log', { + address, + param: 'signRaw.message2 === payload.normalize()', + val: message2 === payload.normalize(), + }); + + // output utf8 bytes + track('xProvider', 'log', { + address, + param: 'signRaw.message', + val: message, + bytes: toUtf8Bytes(message).toString(), + }); + track('xProvider', 'log', { + address, + param: 'signRaw.message2', + val: message2, + bytes: toUtf8Bytes(message2).toString(), + }); + // compare utf8 bytes output + track('xProvider', 'log', { + address, + param: 'signRaw.toUtf8Bytes === toUtf8Bytes(message)', + val: toUtf8Bytes(payload).toString() === toUtf8Bytes(message).toString(), + }); + track('xProvider', 'log', { + address, + param: 'signRaw.toUtf8Bytes === toUtf8Bytes(message2)', + val: toUtf8Bytes(payload).toString() === toUtf8Bytes(message2).toString(), + }); + + const signature = deserializeSignature(await signer.signMessage(toUtf8Bytes(message))); return serializeEthSignature(signature); } diff --git a/packages/passport/sdk/jest.setup.js b/packages/passport/sdk/jest.setup.js index 2e10cd9913..19bb4a9b80 100644 --- a/packages/passport/sdk/jest.setup.js +++ b/packages/passport/sdk/jest.setup.js @@ -1,7 +1,7 @@ -import { TextEncoder } from 'util'; +import { TextEncoder, TextDecoder } from 'util'; global.TextEncoder = TextEncoder; - +global.TextDecoder = TextDecoder; /** * Required for ethers v6 * @see https://github.com/ethers-io/ethers.js/issues/4365 diff --git a/packages/x-client/src/utils/crypto/crypto.ts b/packages/x-client/src/utils/crypto/crypto.ts index a13f0029eb..2732cfb84d 100644 --- a/packages/x-client/src/utils/crypto/crypto.ts +++ b/packages/x-client/src/utils/crypto/crypto.ts @@ -48,7 +48,18 @@ export async function signRaw( payload: string, signer: Signer, ): Promise { - const signature = deserializeSignature(await signer.signMessage(payload)); + // prevent utf-8 encoding issues + const encoder = new TextEncoder(); + const buffer = encoder.encode(payload); + const message = new TextDecoder('utf-8').decode(buffer); + + const buffer2 = Buffer.from(payload, 'utf8'); + const message2 = new TextDecoder('utf-8').decode(buffer2); + + console.log('signRaw.message', { message }); + console.log('signRaw.message2', { message2 }); + + const signature = deserializeSignature(await signer.signMessage(message)); return serializeEthSignature(signature); } diff --git a/packages/x-client/src/utils/stark/legacy/crypto/constants.ts b/packages/x-client/src/utils/stark/legacy/crypto/constants.ts index 73fc31b236..2e49de617c 100644 --- a/packages/x-client/src/utils/stark/legacy/crypto/constants.ts +++ b/packages/x-client/src/utils/stark/legacy/crypto/constants.ts @@ -4,11 +4,21 @@ import BN from 'bn.js'; import elliptic from 'elliptic'; import hashJS from 'hash.js'; +import { toUtf8String } from 'ethers'; import { constantPointsHex } from './points'; import { Instruction, InstructionWithFee } from './types'; const DEFAULT_ACCOUNT_MAPPING_KEY = 'STARKWARE_ACCOUNT_MAPPING'; -const DEFAULT_SIGNATURE_MESSAGE = 'Only sign this request if you’ve initiated an action with Immutable X.'; +// const DEFAULT_SIGNATURE_MESSAGE = 'Only sign this request if you’ve initiated an action with Immutable X.'; +// non-english systems may not encode this correctly +const DEFAULT_SIGNATURE_BYTES = new Uint8Array([ + 79, 110, 108, 121, 32, 115, 105, 103, 110, 32, 116, 104, 105, 115, 32, 114, + 101, 113, 117, 101, 115, 116, 32, 105, 102, 32, 121, 111, 117, 226, 128, 153, + 118, 101, 32, 105, 110, 105, 116, 105, 97, 116, 101, 100, 32, 97, 110, 32, + 97, 99, 116, 105, 111, 110, 32, 119, 105, 116, 104, 32, 73, 109, 109, 117, + 116, 97, 98, 108, 101, 32, 88, 46, +]); +const DEFAULT_SIGNATURE_MESSAGE = toUtf8String(DEFAULT_SIGNATURE_BYTES); const DEFAULT_ACCOUNT_LAYER = 'starkex'; const DEFAULT_ACCOUNT_APPLICATION = 'immutablex'; @@ -95,6 +105,7 @@ export { DEFAULT_ACCOUNT_INDEX, DEFAULT_ACCOUNT_LAYER, DEFAULT_ACCOUNT_MAPPING_KEY, + DEFAULT_SIGNATURE_BYTES, DEFAULT_SIGNATURE_MESSAGE, instructionEncodingMap, MAX_ECDSA_BN, diff --git a/packages/x-client/src/utils/stark/starkCurve.ts b/packages/x-client/src/utils/stark/starkCurve.ts index b79dd5db72..9b080075c1 100644 --- a/packages/x-client/src/utils/stark/starkCurve.ts +++ b/packages/x-client/src/utils/stark/starkCurve.ts @@ -6,7 +6,7 @@ import * as encUtils from 'enc-utils'; // eslint-disable-next-line @typescript-eslint/naming-convention import BN from 'bn.js'; import { hdkey } from '@ethereumjs/wallet'; -import { Signature, Signer, toUtf8Bytes } from 'ethers'; +import { Signature, Signer } from 'ethers'; import { createStarkSigner } from './starkSigner'; import * as legacy from './legacy/crypto'; import { getStarkPublicKeyFromImx } from './getStarkPublicKeyFromImx'; @@ -286,7 +286,7 @@ export async function generateLegacyStarkPrivateKey( signer: Signer, ): Promise { const address = (await signer.getAddress()).toLowerCase(); - const signature = await signer.signMessage(toUtf8Bytes(legacy.DEFAULT_SIGNATURE_MESSAGE)); + const signature = await signer.signMessage(legacy.DEFAULT_SIGNATURE_BYTES); const seed = Signature.from(signature).s; const path = legacy.getAccountPath( legacy.DEFAULT_ACCOUNT_LAYER, diff --git a/packages/x-provider/package.json b/packages/x-provider/package.json index 13d7ab37de..b06eae0e03 100644 --- a/packages/x-provider/package.json +++ b/packages/x-provider/package.json @@ -9,6 +9,7 @@ "@imtbl/generated-clients": "workspace:*", "@imtbl/toolkit": "workspace:*", "@imtbl/x-client": "workspace:*", + "@imtbl/metrics": "workspace:*", "@magic-ext/oidc": "12.0.2", "@metamask/detect-provider": "^2.0.0", "axios": "^1.6.5", diff --git a/packages/x-provider/src/imx-wallet/imxWallet.ts b/packages/x-provider/src/imx-wallet/imxWallet.ts index 46e1c94787..e2308fbbe0 100644 --- a/packages/x-provider/src/imx-wallet/imxWallet.ts +++ b/packages/x-provider/src/imx-wallet/imxWallet.ts @@ -1,5 +1,10 @@ import { Environment } from '@imtbl/config'; -import { BrowserProvider, toUtf8Bytes } from 'ethers'; +import { + BrowserProvider, + toUtf8Bytes, + toUtf8String, +} from 'ethers'; +import { track } from '@imtbl/metrics'; import { ConnectRequest, ConnectResponse, @@ -16,16 +21,92 @@ import { messageResponseListener } from './messageResponseListener'; import { ImxSigner } from './ImxSigner'; import { getOrSetupIFrame } from './imxWalletIFrame'; -const DEFAULT_CONNECTION_MESSAGE = 'Only sign this request if you’ve initiated an action with Immutable X.'; +// "Only sign this request if you've initiated an action with Immutable X." +const DEFAULT_CONNECTION_BYTES = new Uint8Array([ + 79, 110, 108, 121, 32, 115, 105, 103, 110, 32, 116, 104, 105, 115, 32, 114, + 101, 113, 117, 101, 115, 116, 32, 105, 102, 32, 121, 111, 117, 226, 128, 153, + 118, 101, 32, 105, 110, 105, 116, 105, 97, 116, 101, 100, 32, 97, 110, 32, + 97, 99, 116, 105, 111, 110, 32, 119, 105, 116, 104, 32, 73, 109, 109, 117, + 116, 97, 98, 108, 101, 32, 88, 46, +]); +const DEFAULT_CONNECTION_STRING_1 = 'Only sign this request if you’ve initiated an action with Immutable X.'; +const DEFAULT_CONNECTION_STRING_2 = Buffer.from(DEFAULT_CONNECTION_STRING_1, 'utf8').toString('utf8'); const CONNECTION_FAILED_ERROR = 'The L2 IMX Wallet connection has failed'; +function trackConnectionDetails(address: string) { + // track language and charset + track('xProvider', 'log', { address, param: 'navigator.language', val: navigator?.language }); + track('xProvider', 'log', { address, param: 'navigator.languages', val: navigator?.languages?.join(',') }); + track('xProvider', 'log', { address, param: 'document.characterSet', val: document?.characterSet }); + + // track connection encoding details + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_STRING_2', + val: DEFAULT_CONNECTION_STRING_2, + }); + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_BYTES', + val: DEFAULT_CONNECTION_BYTES.toString(), + }); + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_STRING_1', + val: toUtf8Bytes(DEFAULT_CONNECTION_STRING_1).toString(), + }); + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_STRING_2', + val: toUtf8Bytes(DEFAULT_CONNECTION_STRING_2).toString(), + }); + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_STRING_1.normalize()', + val: toUtf8Bytes(DEFAULT_CONNECTION_STRING_1.normalize()).toString(), + }); + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_STRING_2.normalize()', + val: toUtf8Bytes(DEFAULT_CONNECTION_STRING_2.normalize()).toString(), + }); + track('xProvider', 'log', { + address, + param: 'Buffer.from(DEFAULT_CONNECTION_STRING_1, utf8).toString()', + val: Buffer.from(DEFAULT_CONNECTION_STRING_1, 'utf8').toString(), + }); + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_BYTES === toUtf8Bytes(DEFAULT_CONNECTION_STRING_1)', + val: DEFAULT_CONNECTION_BYTES.toString() === toUtf8Bytes(DEFAULT_CONNECTION_STRING_1).toString(), + }); + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_BYTES === toUtf8Bytes(DEFAULT_CONNECTION_STRING_2)', + val: DEFAULT_CONNECTION_BYTES.toString() === toUtf8Bytes(DEFAULT_CONNECTION_STRING_2).toString(), + }); + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_BYTES', + val: DEFAULT_CONNECTION_BYTES.toString(), + }); + track('xProvider', 'log', { + address, + param: 'DEFAULT_CONNECTION_BYTES.toUtf8String()', + val: toUtf8String(DEFAULT_CONNECTION_BYTES), + }); +} + export async function connect( l1Provider: BrowserProvider, env: Environment, ): Promise { const l1Signer = await l1Provider.getSigner(); const address = await l1Signer.getAddress(); - const signature = await l1Signer.signMessage(toUtf8Bytes(DEFAULT_CONNECTION_MESSAGE)); + + trackConnectionDetails(address); + + const signature = await l1Signer.signMessage(DEFAULT_CONNECTION_BYTES); const iframe = await getOrSetupIFrame(env); return new Promise((resolve, reject) => { diff --git a/packages/x-provider/src/l1-providers/metaMaskWrapper.test.ts b/packages/x-provider/src/l1-providers/metaMaskWrapper.test.ts index a694e92194..a72ec330e2 100644 --- a/packages/x-provider/src/l1-providers/metaMaskWrapper.test.ts +++ b/packages/x-provider/src/l1-providers/metaMaskWrapper.test.ts @@ -42,8 +42,13 @@ describe('metaMetaWrapper', () => { describe('connect', () => { it('should create a metamask imx provider with a eth signer and imx signer when calling connect', async () => { - const ethSigner = {}; - const imxSigner = {}; + const ethSigner = { + getAddress: jest.fn().mockResolvedValue('0x123'), + }; + const imxSigner = { + getAddress: jest.fn().mockResolvedValue('0x456'), + signMessage: jest.fn().mockResolvedValue('signed-message'), + }; const getSignerMock = jest.fn().mockReturnValue(ethSigner); (connect as jest.Mock).mockResolvedValue({ @@ -95,40 +100,51 @@ describe('metaMetaWrapper', () => { describe('signMessage', () => { it('should call sign message on imx signer and return a string', async () => { - const getSignerMock = jest.fn().mockReturnValue({}); + const ethSigner = { + getAddress: jest.fn().mockResolvedValue('0x123'), + }; + const imxSigner = { + getAddress: jest.fn().mockResolvedValue('0x456'), + signMessage: jest.fn().mockResolvedValue('signed-message'), + }; + + const getSignerMock = jest.fn().mockReturnValue(ethSigner); (connect as jest.Mock).mockResolvedValue({ getSigner: getSignerMock, }); - const signMessageMock = jest.fn().mockReturnValue('Signed message'); - (buildImxSigner as jest.Mock).mockResolvedValue({ - signMessage: signMessageMock, - }); + (buildImxSigner as jest.Mock).mockResolvedValue(imxSigner); await MetaMaskIMXProvider.connect(config); const signedMessage = await MetaMaskIMXProvider.signMessage( 'Message to sign', ); - expect(signMessageMock).toBeCalledTimes(1); - expect(signMessageMock).toBeCalledWith('Message to sign'); - expect(signedMessage).toEqual('Signed message'); + expect(imxSigner.signMessage).toBeCalledTimes(1); + expect(imxSigner.signMessage).toBeCalledWith('Message to sign'); + expect(signedMessage).toEqual('signed-message'); }); it('should throw provider error when error calling sign message', async () => { + const ethSigner = { + getAddress: jest.fn().mockResolvedValue('0x123'), + }; + const imxSigner = { + getAddress: jest.fn().mockResolvedValue('0x456'), + signMessage: jest.fn().mockRejectedValue(new Error('Sign message failed')), + }; + + const getSignerMock = jest.fn().mockReturnValue(ethSigner); (connect as jest.Mock).mockResolvedValue({ - getSigner: jest.fn().mockReturnValue({}), - }); - (buildImxSigner as jest.Mock).mockResolvedValue({ - signMessage: jest - .fn() - .mockRejectedValue(new Error('Error signing the message')), + getSigner: getSignerMock, }); + (buildImxSigner as jest.Mock).mockResolvedValue(imxSigner); + await MetaMaskIMXProvider.connect(config); await expect( MetaMaskIMXProvider.signMessage('Message to sign'), ).rejects.toThrow( new ProviderError( - 'Error signing the message', + 'Sign message failed', ProviderErrorType.PROVIDER_CONNECTION_ERROR, ), ); @@ -137,10 +153,19 @@ describe('metaMetaWrapper', () => { describe('disconnect', () => { it('should call disconnect with the imx signer', async () => { + const ethSigner = { + getAddress: jest.fn().mockResolvedValue('0x123'), + }; + const imxSigner = { + getAddress: jest.fn().mockResolvedValue('0x456'), + signMessage: jest.fn().mockResolvedValue('signed-message'), + }; + + const getSignerMock = jest.fn().mockReturnValue(ethSigner); (connect as jest.Mock).mockResolvedValue({ - getSigner: jest.fn(), + getSigner: getSignerMock, }); - (buildImxSigner as jest.Mock).mockResolvedValue({}); + (buildImxSigner as jest.Mock).mockResolvedValue(imxSigner); (disconnectImxSigner as jest.Mock).mockResolvedValue({}); await MetaMaskIMXProvider.connect(config); await MetaMaskIMXProvider.disconnect(); @@ -148,10 +173,19 @@ describe('metaMetaWrapper', () => { }); it('should throw provider error when error calling disconnect', async () => { + const ethSigner = { + getAddress: jest.fn().mockResolvedValue('0x123'), + }; + const imxSigner = { + getAddress: jest.fn().mockResolvedValue('0x456'), + signMessage: jest.fn().mockResolvedValue('signed-message'), + }; + + const getSignerMock = jest.fn().mockReturnValue(ethSigner); (connect as jest.Mock).mockResolvedValue({ - getSigner: jest.fn().mockReturnValue({}), + getSigner: getSignerMock, }); - (buildImxSigner as jest.Mock).mockResolvedValue({}); + (buildImxSigner as jest.Mock).mockResolvedValue(imxSigner); (disconnectImxSigner as jest.Mock).mockRejectedValue( new Error('Error disconnecting'), ); diff --git a/packages/x-provider/src/l1-providers/metaMaskWrapper.ts b/packages/x-provider/src/l1-providers/metaMaskWrapper.ts index 4707897d81..9072055b55 100644 --- a/packages/x-provider/src/l1-providers/metaMaskWrapper.ts +++ b/packages/x-provider/src/l1-providers/metaMaskWrapper.ts @@ -1,3 +1,4 @@ +import { track } from '@imtbl/metrics'; import { ProviderConfiguration } from '../config'; import { connect } from './metaMask'; import { @@ -27,9 +28,24 @@ export class MetaMaskIMXProvider extends GenericIMXProvider { metaMaskProvider, config.baseConfig.environment, ); + + const signer = await metaMaskProvider.getSigner(); + const address = await signer.getAddress(); + + track('xProvider', 'log', { + address, + param: 'metaMaskProvider.getSigner().getAddress()', + val: address, + }); + track('xProvider', 'log', { + address, + param: 'imxSigner.getAddress()', + val: this.imxSigner.getAddress(), + }); + return new MetaMaskIMXProvider( config, - await metaMaskProvider.getSigner(), + signer, this.imxSigner, ); }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 88d5e26a7c..6a7a8a0bf9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1858,6 +1858,9 @@ importers: packages/internal/toolkit: dependencies: + '@imtbl/metrics': + specifier: workspace:* + version: link:../metrics '@imtbl/x-client': specifier: workspace:* version: link:../../x-client @@ -2391,6 +2394,9 @@ importers: '@imtbl/generated-clients': specifier: workspace:* version: link:../internal/generated-clients + '@imtbl/metrics': + specifier: workspace:* + version: link:../internal/metrics '@imtbl/toolkit': specifier: workspace:* version: link:../internal/toolkit