Skip to content

Commit 70455d1

Browse files
pedronfigueiredoOGPoyrazvinistevam
authored
feat: Default addTransactionBatch to EIP7702 if supported, otherwise use sequential batch (#5853)
## Explanation We previously relied on the `useHook` property in the request. Instead, the deciding factor is only whether or not EIP 7702 is supported on the chain of the batch transaction. <!-- 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#4991 ## 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 - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] 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 --------- Co-authored-by: OGPoyraz <[email protected]> Co-authored-by: Vinicius Stevam <[email protected]>
1 parent b324d9b commit 70455d1

File tree

6 files changed

+101
-43
lines changed

6 files changed

+101
-43
lines changed

‎packages/transaction-controller/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1515

1616
- Include gas limit and gas fees in simulation requests ([#5754](https://github.com/MetaMask/core/pull/5754))
1717
- Add optional `fee` property to `GasFeeToken`.
18+
- Default addTransactionBatch to EIP7702 if supported, otherwise use sequential batch ([#5853](https://github.com/MetaMask/core/pull/5853))
1819

1920
## [57.0.0]
2021

‎packages/transaction-controller/src/hooks/ExtraTransactionsPublishHook.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ describe('ExtraTransactionsPublishHook', () => {
8888
params: BATCH_TRANSACTION_PARAMS_2_MOCK,
8989
},
9090
],
91-
useHook: true,
91+
disable7702: true,
92+
disableHook: false,
93+
disableSequential: true,
9294
});
9395
});
9496

‎packages/transaction-controller/src/hooks/ExtraTransactionsPublishHook.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,9 @@ export class ExtraTransactionsPublishHook {
107107
from,
108108
networkClientId,
109109
transactions,
110-
useHook: true,
110+
disable7702: true,
111+
disableHook: false,
112+
disableSequential: true,
111113
});
112114

113115
return resultPromise.promise;

‎packages/transaction-controller/src/types.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,9 +1609,21 @@ export type TransactionBatchRequest = {
16091609
/** Transactions to be submitted as part of the batch. */
16101610
transactions: TransactionBatchSingleRequest[];
16111611

1612+
/** Whether to disable batch transaction processing via an EIP-7702 upgraded account. */
1613+
disable7702?: boolean;
1614+
1615+
/** Whether to disable batch transaction via the `publishBatch` hook. */
1616+
disableHook?: boolean;
1617+
1618+
/** Whether to disable batch transaction via sequential transactions. */
1619+
disableSequential?: boolean;
1620+
16121621
/**
16131622
* Whether to use the publish batch hook to submit the batch.
16141623
* Defaults to false.
1624+
*
1625+
* @deprecated This is no longer used and will be removed in a future version.
1626+
* Use `disableHook`, `disable7702` and `disableSequential`.
16151627
*/
16161628
useHook?: boolean;
16171629

‎packages/transaction-controller/src/utils/batch.test.ts

Lines changed: 42 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,9 @@ describe('Batch Utils', () => {
265265
},
266266
},
267267
],
268+
disable7702: false,
269+
disableHook: false,
270+
disableSequential: false,
268271
},
269272
updateTransaction: updateTransactionMock,
270273
publishTransaction: publishTransactionMock,
@@ -274,7 +277,7 @@ describe('Batch Utils', () => {
274277
});
275278

276279
it('returns generated batch ID', async () => {
277-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
280+
doesChainSupportEIP7702Mock.mockReturnValue(true);
278281

279282
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
280283
delegationAddress: undefined,
@@ -298,7 +301,7 @@ describe('Batch Utils', () => {
298301
});
299302

300303
it('returns provided batch ID', async () => {
301-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
304+
doesChainSupportEIP7702Mock.mockReturnValue(true);
302305

303306
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
304307
delegationAddress: undefined,
@@ -324,7 +327,7 @@ describe('Batch Utils', () => {
324327
});
325328

326329
it('adds generated EIP-7702 transaction', async () => {
327-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
330+
doesChainSupportEIP7702Mock.mockReturnValue(true);
328331

329332
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
330333
delegationAddress: undefined,
@@ -361,7 +364,7 @@ describe('Batch Utils', () => {
361364
});
362365

363366
it('uses type 4 transaction if not upgraded', async () => {
364-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
367+
doesChainSupportEIP7702Mock.mockReturnValue(true);
365368

366369
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
367370
delegationAddress: undefined,
@@ -403,7 +406,7 @@ describe('Batch Utils', () => {
403406
});
404407

405408
it('passes nested transactions to add transaction', async () => {
406-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
409+
doesChainSupportEIP7702Mock.mockReturnValue(true);
407410

408411
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
409412
delegationAddress: undefined,
@@ -444,7 +447,7 @@ describe('Batch Utils', () => {
444447
});
445448

446449
it('determines transaction type for nested transactions', async () => {
447-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
450+
doesChainSupportEIP7702Mock.mockReturnValue(true);
448451

449452
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
450453
delegationAddress: undefined,
@@ -489,23 +492,23 @@ describe('Batch Utils', () => {
489492
});
490493

491494
it('throws if chain not supported', async () => {
492-
doesChainSupportEIP7702Mock.mockReturnValueOnce(false);
495+
doesChainSupportEIP7702Mock.mockReturnValue(false);
493496

494497
await expect(addTransactionBatch(request)).rejects.toThrow(
495-
rpcErrors.internal('Chain does not support EIP-7702'),
498+
rpcErrors.internal("Can't process batch"),
496499
);
497500
});
498501

499502
it('throws if no public key', async () => {
500-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
503+
doesChainSupportEIP7702Mock.mockReturnValue(true);
501504

502505
await expect(
503506
addTransactionBatch({ ...request, publicKeyEIP7702: undefined }),
504507
).rejects.toThrow(rpcErrors.internal(ERROR_MESSGE_PUBLIC_KEY));
505508
});
506509

507510
it('throws if account upgraded to unsupported contract', async () => {
508-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
511+
doesChainSupportEIP7702Mock.mockReturnValue(true);
509512
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
510513
delegationAddress: CONTRACT_ADDRESS_MOCK,
511514
isSupported: false,
@@ -517,7 +520,7 @@ describe('Batch Utils', () => {
517520
});
518521

519522
it('throws if account not upgraded and no upgrade address', async () => {
520-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
523+
doesChainSupportEIP7702Mock.mockReturnValue(true);
521524

522525
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
523526
delegationAddress: undefined,
@@ -542,7 +545,7 @@ describe('Batch Utils', () => {
542545
});
543546

544547
it('adds security alert ID to transaction', async () => {
545-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
548+
doesChainSupportEIP7702Mock.mockReturnValue(true);
546549

547550
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
548551
delegationAddress: undefined,
@@ -577,7 +580,7 @@ describe('Batch Utils', () => {
577580

578581
describe('validates security', () => {
579582
it('using transaction params', async () => {
580-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
583+
doesChainSupportEIP7702Mock.mockReturnValue(true);
581584

582585
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
583586
delegationAddress: undefined,
@@ -624,7 +627,7 @@ describe('Batch Utils', () => {
624627
});
625628

626629
it('using delegation mock if not upgraded', async () => {
627-
doesChainSupportEIP7702Mock.mockReturnValueOnce(true);
630+
doesChainSupportEIP7702Mock.mockReturnValue(true);
628631

629632
isAccountUpgradedToEIP7702Mock.mockResolvedValueOnce({
630633
delegationAddress: undefined,
@@ -680,6 +683,7 @@ describe('Batch Utils', () => {
680683
mockRequestApproval(MESSENGER_MOCK, {
681684
state: 'approved',
682685
});
686+
doesChainSupportEIP7702Mock.mockReturnValueOnce(false);
683687
});
684688

685689
it('adds each nested transaction', async () => {
@@ -693,7 +697,7 @@ describe('Batch Utils', () => {
693697
addTransactionBatch({
694698
...request,
695699
publishBatchHook,
696-
request: { ...request.request, useHook: true },
700+
request: { ...request.request, disable7702: true },
697701
}).catch(() => {
698702
// Intentionally empty
699703
});
@@ -745,7 +749,7 @@ describe('Batch Utils', () => {
745749
addTransactionBatch({
746750
...request,
747751
publishBatchHook,
748-
request: { ...request.request, useHook: true, origin },
752+
request: { ...request.request, origin, disable7702: true },
749753
}).catch(() => {
750754
// Intentionally empty
751755
});
@@ -799,7 +803,7 @@ describe('Batch Utils', () => {
799803
addTransactionBatch({
800804
...request,
801805
publishBatchHook,
802-
request: { ...request.request, useHook: true },
806+
request: { ...request.request, disable7702: true },
803807
}).catch(() => {
804808
// Intentionally empty
805809
});
@@ -878,7 +882,7 @@ describe('Batch Utils', () => {
878882
addTransactionBatch({
879883
...request,
880884
publishBatchHook,
881-
request: { ...request.request, useHook: true },
885+
request: { ...request.request, disable7702: true },
882886
}).catch(() => {
883887
// Intentionally empty
884888
});
@@ -960,14 +964,14 @@ describe('Batch Utils', () => {
960964
publishBatchHook,
961965
request: {
962966
...request.request,
967+
disable7702: true,
963968
transactions: [
964969
{
965970
...request.request.transactions[0],
966971
existingTransaction: EXISTING_TRANSACTION_MOCK,
967972
},
968973
request.request.transactions[1],
969974
],
970-
useHook: true,
971975
},
972976
}).catch(() => {
973977
// Intentionally empty
@@ -1063,6 +1067,7 @@ describe('Batch Utils', () => {
10631067
publishBatchHook,
10641068
request: {
10651069
...request.request,
1070+
disable7702: true,
10661071
transactions: [
10671072
{
10681073
...request.request.transactions[0],
@@ -1074,7 +1079,6 @@ describe('Batch Utils', () => {
10741079
},
10751080
request.request.transactions[1],
10761081
],
1077-
useHook: true,
10781082
},
10791083
}).catch(() => {
10801084
// Intentionally empty
@@ -1125,7 +1129,7 @@ describe('Batch Utils', () => {
11251129
const resultPromise = addTransactionBatch({
11261130
...request,
11271131
publishBatchHook,
1128-
request: { ...request.request, useHook: true },
1132+
request: { ...request.request, disable7702: true },
11291133
});
11301134

11311135
resultPromise.catch(() => {
@@ -1187,8 +1191,8 @@ describe('Batch Utils', () => {
11871191
publishBatchHook,
11881192
request: {
11891193
...request.request,
1190-
useHook: true,
11911194
requireApproval: false,
1195+
disable7702: true,
11921196
},
11931197
}).catch(() => {
11941198
// Intentionally empty
@@ -1244,8 +1248,8 @@ describe('Batch Utils', () => {
12441248
publishBatchHook,
12451249
request: {
12461250
...request.request,
1247-
useHook: true,
12481251
requireApproval: false,
1252+
disable7702: true,
12491253
},
12501254
}).catch(() => {
12511255
// Intentionally empty
@@ -1274,6 +1278,8 @@ describe('Batch Utils', () => {
12741278
let sequentialPublishBatchHook: jest.MockedFn<PublishBatchHook>;
12751279

12761280
beforeEach(() => {
1281+
doesChainSupportEIP7702Mock.mockReturnValue(false);
1282+
12771283
sequentialPublishBatchHook = jest.fn();
12781284

12791285
addTransactionMock
@@ -1360,9 +1366,7 @@ describe('Batch Utils', () => {
13601366
isSimulationEnabled: () => isSimulationSupportedMock(),
13611367
request: { ...request.request, useHook: true },
13621368
}),
1363-
).rejects.toThrow(
1364-
'Cannot create transaction batch as simulation not supported',
1365-
);
1369+
).rejects.toThrow(`Can't process batch`);
13661370
});
13671371

13681372
it('invokes sequentialPublishBatchHook when publishBatchHook is undefined', async () => {
@@ -1374,8 +1378,8 @@ describe('Batch Utils', () => {
13741378
publishBatchHook: undefined,
13751379
request: {
13761380
...request.request,
1377-
useHook: true,
13781381
requireApproval: false,
1382+
disable7702: true,
13791383
},
13801384
}).catch(() => {
13811385
// Intentionally empty
@@ -1399,7 +1403,7 @@ describe('Batch Utils', () => {
13991403
addTransactionBatch({
14001404
...request,
14011405
publishBatchHook: undefined,
1402-
request: { ...request.request, useHook: true },
1406+
request: { ...request.request, disable7702: true },
14031407
}),
14041408
).rejects.toThrow('Test error');
14051409

@@ -1417,7 +1421,11 @@ describe('Batch Utils', () => {
14171421
...request,
14181422
publishBatchHook: undefined,
14191423
messenger: MESSENGER_MOCK,
1420-
request: { ...request.request, useHook: true, origin: ORIGIN_MOCK },
1424+
request: {
1425+
...request.request,
1426+
origin: ORIGIN_MOCK,
1427+
disable7702: true,
1428+
},
14211429
}).catch(() => {
14221430
// Intentionally empty
14231431
});
@@ -1457,8 +1465,8 @@ describe('Batch Utils', () => {
14571465
messenger: MESSENGER_MOCK,
14581466
request: {
14591467
...request.request,
1460-
useHook: true,
14611468
origin: ORIGIN_MOCK,
1469+
disable7702: true,
14621470
},
14631471
}).catch(() => {
14641472
// Intentionally empty
@@ -1526,6 +1534,10 @@ describe('Batch Utils', () => {
15261534
});
15271535

15281536
describe('isAtomicBatchSupported', () => {
1537+
beforeEach(() => {
1538+
jest.resetAllMocks();
1539+
});
1540+
15291541
it('includes all feature flag chains if chain IDs not specified', async () => {
15301542
getEIP7702SupportedChainsMock.mockReturnValueOnce([
15311543
CHAIN_ID_MOCK,

0 commit comments

Comments
 (0)