Skip to content

Normalizes fourByte data in method selector #6102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Normalizes fourByte data in method selector #6102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
09d555c
fix: normalize fourByte data
vinistevam Jul 10, 2025
93c156e
Merge branch 'main' into fix/normalize-data-four-byte
vinistevam Jul 10, 2025
d3c93a5
add changelog
vinistevam Jul 10, 2025
7705978
fix lint
vinistevam Jul 10, 2025
3be020e
Merge branch 'main' into fix/normalize-data-four-byte
vinistevam Jul 16, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/transaction-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
142 changes: 50 additions & 92 deletions packages/transaction-controller/src/utils/transaction-type.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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({
Expand All @@ -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,
Expand All @@ -63,7 +72,7 @@ describe('determineTransactionType', () => {
...txParams,
},

new MockEthQuery(new FakeProvider()),
createMockEthQuery('0xab'),
);

expect(resultWithEmptyValue2).toMatchObject({
Expand All @@ -77,7 +86,7 @@ describe('determineTransactionType', () => {
...txParams,
},

new MockEthQuery(new FakeProvider()),
createMockEthQuery('0xab'),
);
expect(resultWithValue).toMatchObject({
type: TransactionType.contractInteraction,
Expand All @@ -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,
Expand All @@ -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({
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion packages/transaction-controller/src/utils/transaction-type.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading