Skip to content

Commit b3cf3c0

Browse files
authored
Normalizes fourByte data in method selector (#6102)
## Explanation This PR updates the `getMethodName` utility to normalise the transaction data to lowercase before comparing it against known ABI method selectors. <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> Fixes MetaMask/MetaMask-planning#5338 ## Changelog <!-- THIS SECTION IS NO LONGER NEEDED. The process for updating changelogs has changed. Please consult the "Updating changelogs" section of the Contributing doc for more. --> ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent 6d049eb commit b3cf3c0

File tree

3 files changed

+55
-93
lines changed

3 files changed

+55
-93
lines changed

packages/transaction-controller/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Add fallback to the sequential hook when `publishBatchHook` returns empty ([#6063](https://github.com/MetaMask/core/pull/6063))
1313

14+
### Fixed
15+
16+
- Normalize transaction `data` to ensure case-insensitive detection ([#6102](https://github.com/MetaMask/core/pull/6102))
17+
1418
## [58.1.1]
1519

1620
### Changed

packages/transaction-controller/src/utils/transaction-type.test.ts

Lines changed: 50 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,29 @@ import { determineTransactionType } from './transaction-type';
55
import { FakeProvider } from '../../../../tests/fake-provider';
66
import { TransactionType } from '../types';
77

8+
type GetCodeCallback = (err: Error | null, result?: string) => void;
9+
10+
/**
11+
* Creates a mock EthQuery instance for testing.
12+
*
13+
* @param getCodeResponse The response string to return from getCode, or undefined/null.
14+
* @param shouldThrow Whether getCode should throw an error instead of returning a response.
15+
* @returns An EthQuery instance with a mocked getCode method.
16+
*/
17+
function createMockEthQuery(
18+
getCodeResponse: string | undefined | null,
19+
shouldThrow = false,
20+
): EthQuery {
21+
return new (class extends EthQuery {
22+
getCode(_to: string, cb: GetCodeCallback): void {
23+
if (shouldThrow) {
24+
return cb(new Error('Some error'));
25+
}
26+
return cb(null, getCodeResponse ?? undefined);
27+
}
28+
})(new FakeProvider());
29+
}
30+
831
describe('determineTransactionType', () => {
932
const FROM_MOCK = '0x9e';
1033
const txParams = {
@@ -14,20 +37,13 @@ describe('determineTransactionType', () => {
1437
};
1538

1639
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 () => {
17-
class MockEthQuery extends EthQuery {
18-
// TODO: Replace `any` with type
19-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
20-
getCode(_to: any, cb: any) {
21-
cb(null, '0xab');
22-
}
23-
}
2440
const result = await determineTransactionType(
2541
{
2642
to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9',
2743
data: '0xa9059cbb0000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C970000000000000000000000000000000000000000000000000000000000000000a',
2844
from: FROM_MOCK,
2945
},
30-
new MockEthQuery(new FakeProvider()),
46+
createMockEthQuery('0xab'),
3147
);
3248

3349
expect(result).toMatchObject({
@@ -40,17 +56,10 @@ describe('determineTransactionType', () => {
4056
'does NOT return a token transfer type and instead returns contract interaction' +
4157
' when the recipient is a contract, the data matches the respective method call, but there is a value passed',
4258
async () => {
43-
class MockEthQuery extends EthQuery {
44-
// TODO: Replace `any` with type
45-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
46-
getCode(_to: any, cb: any) {
47-
cb(null, '0xab');
48-
}
49-
}
5059
const resultWithEmptyValue = await determineTransactionType(
5160
txParams,
5261

53-
new MockEthQuery(new FakeProvider()),
62+
createMockEthQuery('0xab'),
5463
);
5564
expect(resultWithEmptyValue).toMatchObject({
5665
type: TransactionType.tokenMethodTransfer,
@@ -63,7 +72,7 @@ describe('determineTransactionType', () => {
6372
...txParams,
6473
},
6574

66-
new MockEthQuery(new FakeProvider()),
75+
createMockEthQuery('0xab'),
6776
);
6877

6978
expect(resultWithEmptyValue2).toMatchObject({
@@ -77,7 +86,7 @@ describe('determineTransactionType', () => {
7786
...txParams,
7887
},
7988

80-
new MockEthQuery(new FakeProvider()),
89+
createMockEthQuery('0xab'),
8190
);
8291
expect(resultWithValue).toMatchObject({
8392
type: TransactionType.contractInteraction,
@@ -87,16 +96,9 @@ describe('determineTransactionType', () => {
8796
);
8897

8998
it('does NOT return a token transfer type when the recipient is not a contract but the data matches the respective method call', async () => {
90-
class MockEthQuery extends EthQuery {
91-
// TODO: Replace `any` with type
92-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
93-
getCode(_to: any, cb: any) {
94-
cb(null, '0x');
95-
}
96-
}
9799
const result = await determineTransactionType(
98100
txParams,
99-
new MockEthQuery(new FakeProvider()),
101+
createMockEthQuery('0x'),
100102
);
101103
expect(result).toMatchObject({
102104
type: TransactionType.simpleSend,
@@ -105,21 +107,13 @@ describe('determineTransactionType', () => {
105107
});
106108

107109
it('does not identify contract codes with DELEGATION_PREFIX as contract addresses', async () => {
108-
class MockEthQuery extends EthQuery {
109-
// TODO: Replace `any` with type
110-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
111-
getCode(_to: any, cb: any) {
112-
cb(null, `${DELEGATION_PREFIX}1234567890abcdef`);
113-
}
114-
}
115-
116110
const result = await determineTransactionType(
117111
{
118112
to: '0x9e673399f795D01116e9A8B2dD2F156705131ee9',
119113
data: '0xabd',
120114
from: FROM_MOCK,
121115
},
122-
new MockEthQuery(new FakeProvider()),
116+
createMockEthQuery(`${DELEGATION_PREFIX}1234567890abcdef`),
123117
);
124118

125119
expect(result).toMatchObject({
@@ -129,19 +123,26 @@ describe('determineTransactionType', () => {
129123
});
130124

131125
it('returns a token approve type when the recipient is a contract and data is for the respective method call', async () => {
132-
class MockEthQuery extends EthQuery {
133-
// TODO: Replace `any` with type
134-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
135-
getCode(_to: any, cb: any) {
136-
cb(null, '0xab');
137-
}
138-
}
139126
const result = await determineTransactionType(
140127
{
141128
...txParams,
142129
data: '0x095ea7b30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005',
143130
},
144-
new MockEthQuery(new FakeProvider()),
131+
createMockEthQuery('0xab'),
132+
);
133+
expect(result).toMatchObject({
134+
type: TransactionType.tokenMethodApprove,
135+
getCodeResponse: '0xab',
136+
});
137+
});
138+
139+
it('returns a token approve type when data is uppercase', async () => {
140+
const result = await determineTransactionType(
141+
{
142+
...txParams,
143+
data: '0x095EA7B30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005',
144+
},
145+
createMockEthQuery('0xab'),
145146
);
146147
expect(result).toMatchObject({
147148
type: TransactionType.tokenMethodApprove,
@@ -150,20 +151,13 @@ describe('determineTransactionType', () => {
150151
});
151152

152153
it('returns a contract deployment type when "to" is falsy and there is data', async () => {
153-
class MockEthQuery extends EthQuery {
154-
// TODO: Replace `any` with type
155-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
156-
getCode(_to: any, cb: any) {
157-
cb(null, '');
158-
}
159-
}
160154
const result = await determineTransactionType(
161155
{
162156
...txParams,
163157
to: '',
164158
data: '0xabd',
165159
},
166-
new MockEthQuery(new FakeProvider()),
160+
createMockEthQuery(''),
167161
);
168162
expect(result).toMatchObject({
169163
type: TransactionType.deployContract,
@@ -172,19 +166,12 @@ describe('determineTransactionType', () => {
172166
});
173167

174168
it('returns a simple send type with a 0x getCodeResponse when there is data, but the "to" address is not a contract address', async () => {
175-
class MockEthQuery extends EthQuery {
176-
// TODO: Replace `any` with type
177-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
178-
getCode(_to: any, cb: any) {
179-
cb(null, '0x');
180-
}
181-
}
182169
const result = await determineTransactionType(
183170
{
184171
...txParams,
185172
data: '0xabd',
186173
},
187-
new MockEthQuery(new FakeProvider()),
174+
createMockEthQuery('0x'),
188175
);
189176
expect(result).toMatchObject({
190177
type: TransactionType.simpleSend,
@@ -193,19 +180,12 @@ describe('determineTransactionType', () => {
193180
});
194181

195182
it('returns a simple send type with a null getCodeResponse when "to" is truthy and there is data, but getCode returns an error', async () => {
196-
class MockEthQuery extends EthQuery {
197-
// TODO: Replace `any` with type
198-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
199-
getCode(_to: any, cb: any) {
200-
cb(new Error('Some error'));
201-
}
202-
}
203183
const result = await determineTransactionType(
204184
{
205185
...txParams,
206186
data: '0xabd',
207187
},
208-
new MockEthQuery(new FakeProvider()),
188+
createMockEthQuery(null, true),
209189
);
210190
expect(result).toMatchObject({
211191
type: TransactionType.simpleSend,
@@ -214,19 +194,12 @@ describe('determineTransactionType', () => {
214194
});
215195

216196
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 () => {
217-
class MockEthQuery extends EthQuery {
218-
// TODO: Replace `any` with type
219-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
220-
getCode(_to: any, cb: any) {
221-
cb(null, '0xa');
222-
}
223-
}
224197
const result = await determineTransactionType(
225198
{
226199
...txParams,
227200
data: 'abd',
228201
},
229-
new MockEthQuery(new FakeProvider()),
202+
createMockEthQuery('0xa'),
230203
);
231204
expect(result).toMatchObject({
232205
type: TransactionType.contractInteraction,
@@ -235,19 +208,12 @@ describe('determineTransactionType', () => {
235208
});
236209

237210
it('returns a contract interaction type with the correct getCodeResponse when "to" is a contract address and data is falsy', async () => {
238-
class MockEthQuery extends EthQuery {
239-
// TODO: Replace `any` with type
240-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
241-
getCode(_to: any, cb: any) {
242-
cb(null, '0xa');
243-
}
244-
}
245211
const result = await determineTransactionType(
246212
{
247213
...txParams,
248214
data: '',
249215
},
250-
new MockEthQuery(new FakeProvider()),
216+
createMockEthQuery('0xa'),
251217
);
252218
expect(result).toMatchObject({
253219
type: TransactionType.contractInteraction,
@@ -256,21 +222,13 @@ describe('determineTransactionType', () => {
256222
});
257223

258224
it('returns contractInteraction for send with approve', async () => {
259-
class MockEthQuery extends EthQuery {
260-
// TODO: Replace `any` with type
261-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
262-
getCode(_to: any, cb: any) {
263-
cb(null, '0xa');
264-
}
265-
}
266-
267225
const result = await determineTransactionType(
268226
{
269227
...txParams,
270228
value: '0x5af3107a4000',
271229
data: '0x095ea7b30000000000000000000000002f318C334780961FB129D2a6c30D0763d9a5C9700000000000000000000000000000000000000000000000000000000000000005',
272230
},
273-
new MockEthQuery(new FakeProvider()),
231+
createMockEthQuery('0xa'),
274232
);
275233
expect(result).toMatchObject({
276234
type: TransactionType.contractInteraction,

packages/transaction-controller/src/utils/transaction-type.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ function getMethodName(data?: string): string | undefined {
100100
return undefined;
101101
}
102102

103-
const fourByte = data.substring(0, 10);
103+
const fourByte = data.substring(0, 10).toLowerCase();
104104

105105
for (const interfaceInstance of [
106106
ERC20Interface,

0 commit comments

Comments
 (0)