From 09d555c0d68275b9030b3772072ab6291b64f26d Mon Sep 17 00:00:00 2001 From: Vinicius Stevam Date: Thu, 10 Jul 2025 15:01:04 +0100 Subject: [PATCH 1/3] fix: normalize fourByte data --- .../src/utils/transaction-type.test.ts | 141 +++++++----------- .../src/utils/transaction-type.ts | 2 +- 2 files changed, 51 insertions(+), 92 deletions(-) diff --git a/packages/transaction-controller/src/utils/transaction-type.test.ts b/packages/transaction-controller/src/utils/transaction-type.test.ts index bfb1f6319b8..df9c8de746b 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,13 +107,6 @@ 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( { @@ -119,7 +114,7 @@ describe('determineTransactionType', () => { data: '0xabd', from: FROM_MOCK, }, - new MockEthQuery(new FakeProvider()), + createMockEthQuery(`${DELEGATION_PREFIX}1234567890abcdef`), ); expect(result).toMatchObject({ @@ -129,19 +124,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 +152,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 +167,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 +181,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 +195,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 +209,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 +223,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, From d3c93a59ac89d99256bcc7542aa86e7cc2fd0b9f Mon Sep 17 00:00:00 2001 From: Vinicius Stevam Date: Thu, 10 Jul 2025 16:40:09 +0100 Subject: [PATCH 2/3] add changelog --- packages/transaction-controller/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) 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 From 77059784af1019ae85925c8e1aefe87a6b8d0cde Mon Sep 17 00:00:00 2001 From: Vinicius Stevam Date: Thu, 10 Jul 2025 17:10:42 +0100 Subject: [PATCH 3/3] fix lint --- .../transaction-controller/src/utils/transaction-type.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/transaction-controller/src/utils/transaction-type.test.ts b/packages/transaction-controller/src/utils/transaction-type.test.ts index df9c8de746b..7b3a4dd2190 100644 --- a/packages/transaction-controller/src/utils/transaction-type.test.ts +++ b/packages/transaction-controller/src/utils/transaction-type.test.ts @@ -107,7 +107,6 @@ describe('determineTransactionType', () => { }); it('does not identify contract codes with DELEGATION_PREFIX as contract addresses', async () => { - const result = await determineTransactionType( { to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9',