diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 8ee9adaeba4..45c46753354 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add fallback to the sequential hook when `publishBatchHook` returns empty ([#6063](https://github.com/MetaMask/core/pull/6063)) +### Fixed + +- Normalize transaction `data` to ensure case-insensitive detection ([#6102](https://github.com/MetaMask/core/pull/6102)) + ## [58.1.1] ### Changed diff --git a/packages/transaction-controller/src/utils/transaction-type.test.ts b/packages/transaction-controller/src/utils/transaction-type.test.ts index bfb1f6319b8..7b3a4dd2190 100644 --- a/packages/transaction-controller/src/utils/transaction-type.test.ts +++ b/packages/transaction-controller/src/utils/transaction-type.test.ts @@ -5,6 +5,29 @@ import { determineTransactionType } from './transaction-type'; import { FakeProvider } from '../../../../tests/fake-provider'; import { TransactionType } from '../types'; +type GetCodeCallback = (err: Error | null, result?: string) => void; + +/** + * Creates a mock EthQuery instance for testing. + * + * @param getCodeResponse The response string to return from getCode, or undefined/null. + * @param shouldThrow Whether getCode should throw an error instead of returning a response. + * @returns An EthQuery instance with a mocked getCode method. + */ +function createMockEthQuery( + getCodeResponse: string | undefined | null, + shouldThrow = false, +): EthQuery { + return new (class extends EthQuery { + getCode(_to: string, cb: GetCodeCallback): void { + if (shouldThrow) { + return cb(new Error('Some error')); + } + return cb(null, getCodeResponse ?? undefined); + } + })(new FakeProvider()); +} + describe('determineTransactionType', () => { const FROM_MOCK = '0x9e'; const txParams = { @@ -14,20 +37,13 @@ describe('determineTransactionType', () => { }; it('returns a token transfer type when the recipient is a contract, there is no value passed, and data is for the respective method call', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, '0xab'); - } - } const result = await determineTransactionType( { to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9', data: '0xa9059cbb0000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C970000000000000000000000000000000000000000000000000000000000000000a', from: FROM_MOCK, }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0xab'), ); expect(result).toMatchObject({ @@ -40,17 +56,10 @@ describe('determineTransactionType', () => { 'does NOT return a token transfer type and instead returns contract interaction' + ' when the recipient is a contract, the data matches the respective method call, but there is a value passed', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, '0xab'); - } - } const resultWithEmptyValue = await determineTransactionType( txParams, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0xab'), ); expect(resultWithEmptyValue).toMatchObject({ type: TransactionType.tokenMethodTransfer, @@ -63,7 +72,7 @@ describe('determineTransactionType', () => { ...txParams, }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0xab'), ); expect(resultWithEmptyValue2).toMatchObject({ @@ -77,7 +86,7 @@ describe('determineTransactionType', () => { ...txParams, }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0xab'), ); expect(resultWithValue).toMatchObject({ type: TransactionType.contractInteraction, @@ -87,16 +96,9 @@ describe('determineTransactionType', () => { ); it('does NOT return a token transfer type when the recipient is not a contract but the data matches the respective method call', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, '0x'); - } - } const result = await determineTransactionType( txParams, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0x'), ); expect(result).toMatchObject({ type: TransactionType.simpleSend, @@ -105,21 +107,13 @@ describe('determineTransactionType', () => { }); it('does not identify contract codes with DELEGATION_PREFIX as contract addresses', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, `${DELEGATION_PREFIX}1234567890abcdef`); - } - } - const result = await determineTransactionType( { to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9', data: '0xabd', from: FROM_MOCK, }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery(`${DELEGATION_PREFIX}1234567890abcdef`), ); expect(result).toMatchObject({ @@ -129,19 +123,26 @@ describe('determineTransactionType', () => { }); it('returns a token approve type when the recipient is a contract and data is for the respective method call', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, '0xab'); - } - } const result = await determineTransactionType( { ...txParams, data: '0x095ea7b30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005', }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0xab'), + ); + expect(result).toMatchObject({ + type: TransactionType.tokenMethodApprove, + getCodeResponse: '0xab', + }); + }); + + it('returns a token approve type when data is uppercase', async () => { + const result = await determineTransactionType( + { + ...txParams, + data: '0x095EA7B30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005', + }, + createMockEthQuery('0xab'), ); expect(result).toMatchObject({ type: TransactionType.tokenMethodApprove, @@ -150,20 +151,13 @@ describe('determineTransactionType', () => { }); it('returns a contract deployment type when "to" is falsy and there is data', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, ''); - } - } const result = await determineTransactionType( { ...txParams, to: '', data: '0xabd', }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery(''), ); expect(result).toMatchObject({ type: TransactionType.deployContract, @@ -172,19 +166,12 @@ describe('determineTransactionType', () => { }); it('returns a simple send type with a 0x getCodeResponse when there is data, but the "to" address is not a contract address', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, '0x'); - } - } const result = await determineTransactionType( { ...txParams, data: '0xabd', }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0x'), ); expect(result).toMatchObject({ type: TransactionType.simpleSend, @@ -193,19 +180,12 @@ describe('determineTransactionType', () => { }); it('returns a simple send type with a null getCodeResponse when "to" is truthy and there is data, but getCode returns an error', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(new Error('Some error')); - } - } const result = await determineTransactionType( { ...txParams, data: '0xabd', }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery(null, true), ); expect(result).toMatchObject({ type: TransactionType.simpleSend, @@ -214,19 +194,12 @@ describe('determineTransactionType', () => { }); it('returns a contract interaction type with the correct getCodeResponse when "to" is truthy and there is data, and it is not a token transaction', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, '0xa'); - } - } const result = await determineTransactionType( { ...txParams, data: 'abd', }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0xa'), ); expect(result).toMatchObject({ type: TransactionType.contractInteraction, @@ -235,19 +208,12 @@ describe('determineTransactionType', () => { }); it('returns a contract interaction type with the correct getCodeResponse when "to" is a contract address and data is falsy', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, '0xa'); - } - } const result = await determineTransactionType( { ...txParams, data: '', }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0xa'), ); expect(result).toMatchObject({ type: TransactionType.contractInteraction, @@ -256,21 +222,13 @@ describe('determineTransactionType', () => { }); it('returns contractInteraction for send with approve', async () => { - class MockEthQuery extends EthQuery { - // TODO: Replace `any` with type - // eslint-disable-next-line @typescript-eslint/no-explicit-any - getCode(_to: any, cb: any) { - cb(null, '0xa'); - } - } - const result = await determineTransactionType( { ...txParams, value: '0x5af3107a4000', data: '0x095ea7b30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005', }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery('0xa'), ); expect(result).toMatchObject({ type: TransactionType.contractInteraction, diff --git a/packages/transaction-controller/src/utils/transaction-type.ts b/packages/transaction-controller/src/utils/transaction-type.ts index 8f28b9bf127..7c913d8830d 100644 --- a/packages/transaction-controller/src/utils/transaction-type.ts +++ b/packages/transaction-controller/src/utils/transaction-type.ts @@ -100,7 +100,7 @@ function getMethodName(data?: string): string | undefined { return undefined; } - const fourByte = data.substring(0, 10); + const fourByte = data.substring(0, 10).toLowerCase(); for (const interfaceInstance of [ ERC20Interface,