Skip to content

Commit 277165e

Browse files
fix: Avoid coercing publishBatchHook to a boolean (#5934)
## Explanation <!-- 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? --> With the previous syntax, publishBatchHook would be coerced into a boolean, which would halt execution further down the function when it is awaited. This PR fixes normal function execution by avoiding coercion. ## 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 --> ## 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
1 parent 140693b commit 277165e

File tree

3 files changed

+29
-5
lines changed

3 files changed

+29
-5
lines changed

packages/transaction-controller/CHANGELOG.md

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

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Avoid coercing `publishBatchHook` to a boolean ([#5934](https://github.com/MetaMask/core/pull/5934))
13+
1014
## [57.1.0]
1115

1216
### Added

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1364,7 +1364,12 @@ describe('Batch Utils', () => {
13641364
...request,
13651365
publishBatchHook: undefined,
13661366
isSimulationEnabled: () => isSimulationSupportedMock(),
1367-
request: { ...request.request, useHook: true },
1367+
request: {
1368+
...request.request,
1369+
disable7702: true,
1370+
disableHook: true,
1371+
disableSequential: false,
1372+
},
13681373
}),
13691374
).rejects.toThrow(`Can't process batch`);
13701375
});
@@ -1380,6 +1385,8 @@ describe('Batch Utils', () => {
13801385
...request.request,
13811386
requireApproval: false,
13821387
disable7702: true,
1388+
disableHook: true,
1389+
disableSequential: false,
13831390
},
13841391
}).catch(() => {
13851392
// Intentionally empty
@@ -1403,7 +1410,12 @@ describe('Batch Utils', () => {
14031410
addTransactionBatch({
14041411
...request,
14051412
publishBatchHook: undefined,
1406-
request: { ...request.request, disable7702: true },
1413+
request: {
1414+
...request.request,
1415+
disable7702: true,
1416+
disableHook: true,
1417+
disableSequential: false,
1418+
},
14071419
}),
14081420
).rejects.toThrow('Test error');
14091421

@@ -1425,6 +1437,8 @@ describe('Batch Utils', () => {
14251437
...request.request,
14261438
origin: ORIGIN_MOCK,
14271439
disable7702: true,
1440+
disableHook: true,
1441+
disableSequential: false,
14281442
},
14291443
}).catch(() => {
14301444
// Intentionally empty
@@ -1467,6 +1481,8 @@ describe('Batch Utils', () => {
14671481
...request.request,
14681482
origin: ORIGIN_MOCK,
14691483
disable7702: true,
1484+
disableHook: true,
1485+
disableSequential: false,
14701486
},
14711487
}).catch(() => {
14721488
// Intentionally empty

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -425,9 +425,13 @@ async function addTransactionBatchWithHook(
425425
disableSequential = true;
426426
}
427427

428-
const publishBatchHook =
429-
(!disableHook && requestPublishBatchHook) ??
430-
(!disableSequential && sequentialPublishBatchHook.getHook());
428+
let publishBatchHook = null;
429+
if (!disableHook) {
430+
publishBatchHook = requestPublishBatchHook;
431+
} else if (!disableSequential) {
432+
publishBatchHook = sequentialPublishBatchHook.getHook();
433+
}
434+
431435
if (!publishBatchHook) {
432436
log(`No supported batch methods found`, {
433437
disable7702,

0 commit comments

Comments
 (0)