Skip to content

Commit 85d1522

Browse files
authored
fix: Remove async operation from updateTransactionGasFees (#5539)
## 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? --> This PR aims to fix an issue where `updateTransactionGasFees` updater rejects updating draft state with following error: `TypeError: Cannot perform 'get' on a proxy that has been revoked` The fix is to remove that async operation because it's unnecessary to check it again since it's been check while `addTransaction` ## 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 <!-- If you're making any consumer-facing changes, list those changes here as if you were updating a changelog, using the template below as a guide. (CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or FIXED. For security-related issues, follow the Security Advisory process.) Please take care to name the exact pieces of the API you've added or changed (e.g. types, interfaces, functions, or methods). If there are any breaking changes, make sure to offer a solution for consumers to follow once they upgrade to the changes. Finally, if you're only making changes to development scripts or tests, you may replace the template below with "None". --> ### `@metamask/transaction-controller` - **FIXED**: Fix type error when `enableTxParamsGasFeeUpdates` is set to `true` ## 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 highlighted breaking changes using the "BREAKING" category above as appropriate - [X] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
1 parent bfb6a00 commit 85d1522

File tree

5 files changed

+117
-75
lines changed

5 files changed

+117
-75
lines changed

packages/transaction-controller/src/TransactionController.test.ts

Lines changed: 49 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ import {
7373
GasFeeEstimateType,
7474
SimulationErrorCode,
7575
SimulationTokenStandard,
76+
TransactionEnvelopeType,
7677
TransactionStatus,
7778
TransactionType,
7879
WalletDevice,
@@ -1486,6 +1487,41 @@ describe('TransactionController', () => {
14861487
);
14871488
});
14881489

1490+
it.each([
1491+
[TransactionEnvelopeType.legacy],
1492+
[
1493+
TransactionEnvelopeType.feeMarket,
1494+
{
1495+
maxFeePerGas: '0x1',
1496+
maxPriorityFeePerGas: '0x1',
1497+
},
1498+
{
1499+
getCurrentNetworkEIP1559Compatibility: async () => true,
1500+
getCurrentAccountEIP1559Compatibility: async () => true,
1501+
},
1502+
],
1503+
[TransactionEnvelopeType.accessList, { accessList: [] }],
1504+
[TransactionEnvelopeType.setCode, { authorizationList: [] }],
1505+
])(
1506+
'sets txParams.type to %s if not defined in given txParams',
1507+
async (
1508+
type: TransactionEnvelopeType,
1509+
extraTxParamsToSet: Partial<TransactionParams> = {},
1510+
options: Partial<
1511+
ConstructorParameters<typeof TransactionController>[0]
1512+
> = {},
1513+
) => {
1514+
const { controller } = setupController({ options });
1515+
1516+
await controller.addTransaction(
1517+
{ from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, ...extraTxParamsToSet },
1518+
{ networkClientId: NETWORK_CLIENT_ID_MOCK },
1519+
);
1520+
1521+
expect(controller.state.transactions[0].txParams.type).toBe(type);
1522+
},
1523+
);
1524+
14891525
it('does not check account address relationship if a transaction with the same from, to, and chainId exists', async () => {
14901526
const { controller } = setupController({
14911527
options: {
@@ -1677,21 +1713,25 @@ describe('TransactionController', () => {
16771713
).toBeUndefined();
16781714
});
16791715

1680-
it.each<[keyof DappSuggestedGasFees]>([
1681-
['gasPrice'],
1682-
['maxFeePerGas'],
1683-
['maxPriorityFeePerGas'],
1684-
['gas'],
1716+
it.each<[keyof DappSuggestedGasFees, TransactionEnvelopeType]>([
1717+
['gasPrice', TransactionEnvelopeType.legacy],
1718+
['maxFeePerGas', TransactionEnvelopeType.feeMarket],
1719+
['maxPriorityFeePerGas', TransactionEnvelopeType.feeMarket],
1720+
['gas', TransactionEnvelopeType.feeMarket],
16851721
])(
16861722
'if %s is defined',
1687-
async (gasPropName: keyof DappSuggestedGasFees) => {
1723+
async (
1724+
gasPropName: keyof DappSuggestedGasFees,
1725+
type: TransactionEnvelopeType,
1726+
) => {
16881727
const { controller } = setupController();
16891728
const mockDappOrigin = 'MockDappOrigin';
16901729
const mockGasValue = '0x1';
16911730
await controller.addTransaction(
16921731
{
16931732
from: ACCOUNT_MOCK,
16941733
to: ACCOUNT_MOCK,
1734+
type,
16951735
[gasPropName]: mockGasValue,
16961736
},
16971737
{
@@ -2739,6 +2779,7 @@ describe('TransactionController', () => {
27392779
from: ACCOUNT_MOCK,
27402780
nonce: '0xc',
27412781
to: ACCOUNT_MOCK,
2782+
type: TransactionEnvelopeType.legacy,
27422783
value: '0x0',
27432784
},
27442785
},
@@ -3343,6 +3384,7 @@ describe('TransactionController', () => {
33433384
gasPrice: '0x105',
33443385
nonce: '0xc',
33453386
to: ACCOUNT_MOCK,
3387+
type: TransactionEnvelopeType.legacy,
33463388
value: '0x0',
33473389
},
33483390
},
@@ -3802,6 +3844,7 @@ describe('TransactionController', () => {
38023844
gasPrice: '0x105',
38033845
nonce: '0xc',
38043846
to: ACCOUNT_MOCK,
3847+
type: TransactionEnvelopeType.legacy,
38053848
value: '0x0',
38063849
},
38073850
},

packages/transaction-controller/src/TransactionController.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ import {
146146
validateIfTransactionUnapproved,
147147
normalizeTxError,
148148
normalizeGasFeeValues,
149+
setEnvelopeType,
149150
} from './utils/utils';
150151
import {
151152
validateParamTo,
@@ -1157,6 +1158,11 @@ export class TransactionController extends BaseController<
11571158

11581159
validateTxParams(txParams, isEIP1559Compatible);
11591160

1161+
if (!txParams.type) {
1162+
// Determine transaction type based on transaction parameters and network compatibility
1163+
setEnvelopeType(txParams, isEIP1559Compatible);
1164+
}
1165+
11601166
const isDuplicateBatchId =
11611167
batchId?.length &&
11621168
this.state.transactions.some(
@@ -4018,12 +4024,10 @@ export class TransactionController extends BaseController<
40184024
this.#updateTransactionInternal(
40194025
{ transactionId, skipHistory: true },
40204026
(txMeta) => {
4021-
// eslint-disable-next-line @typescript-eslint/no-floating-promises
40224027
updateTransactionGasFees({
40234028
txMeta,
40244029
gasFeeEstimates,
40254030
gasFeeEstimatesLoaded,
4026-
getEIP1559Compatibility: this.getEIP1559Compatibility.bind(this),
40274031
isTxParamsGasFeeUpdatesEnabled: this.isTxParamsGasFeeUpdatesEnabled,
40284032
layer1GasFee,
40294033
});

0 commit comments

Comments
 (0)