Skip to content

Commit 083cd4f

Browse files
fix: disable automatic gas fee updates (MetaMask#24121)
## **Description** Disable automatic gas fee updates for source transactions generated by Perps and Predict deposits. ## **Changelog** CHANGELOG entry: null ## **Related issues** ## **Manual testing steps** ## **Screenshots/Recordings** ### **Before** ### **After** ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I've included tests if applicable - [x] I've documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I've applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Refines automatic gas fee update logic, disabling it for transactions with nested relayDeposit and MetaMask-origin token approvals, and updates tests accordingly. > > - **Transaction Controller**: > - **Automatic Gas Fee Updates**: > - Implement `isAutomaticGasFeeUpdateEnabled(transaction)` and wire into `TransactionController` options. > - Disable when transaction has nested `relayDeposit`. > - Disable for `tokenMethodApprove` when `origin === ORIGIN_METAMASK`. > - Maintain behavior: enabled for `REDESIGNED_TRANSACTION_TYPES`, disabled for non-redesigned types. > - Add `hasTransactionType` and `ORIGIN_METAMASK` usage. > - **Tests**: > - Expand `isAutomaticGasFeeUpdateEnabled` test coverage for redesigned/non-redesigned, nested `relayDeposit`, and MetaMask vs external origins. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit f664ff6. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent c72aba5 commit 083cd4f

2 files changed

Lines changed: 100 additions & 16 deletions

File tree

app/core/Engine/controllers/transaction-controller/transaction-controller-init.test.ts

Lines changed: 80 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
type PublishBatchHookTransaction,
1111
} from '@metamask/transaction-controller';
1212

13-
import { toHex } from '@metamask/controller-utils';
13+
import { ORIGIN_METAMASK, toHex } from '@metamask/controller-utils';
1414
import { MOCK_ANY_NAMESPACE, MockAnyNamespace } from '@metamask/messenger';
1515
import { Hex } from '@metamask/utils';
1616
import { selectSwapsChainFeatureFlags } from '../../../../reducers/swaps';
@@ -487,20 +487,87 @@ describe('Transaction Controller Init', () => {
487487
expect(updateTransactionsProp).toBe(true);
488488
});
489489

490-
it('determines if automatic gas fee update is enabled based on transaction type', () => {
491-
const option = testConstructorOption('isAutomaticGasFeeUpdateEnabled');
492-
const isEnabledFn = option as ({ type }: { type: string }) => boolean;
490+
describe('isAutomaticGasFeeUpdateEnabled', () => {
491+
it('returns true for redesigned transaction types', () => {
492+
const option = testConstructorOption('isAutomaticGasFeeUpdateEnabled');
493+
const isEnabledFn = option as ({
494+
type,
495+
}: {
496+
type: string;
497+
origin?: string;
498+
}) => boolean;
499+
500+
expect(isEnabledFn({ type: TransactionType.stakingDeposit })).toBe(true);
501+
expect(isEnabledFn({ type: TransactionType.stakingUnstake })).toBe(true);
502+
expect(isEnabledFn({ type: TransactionType.stakingClaim })).toBe(true);
503+
expect(isEnabledFn({ type: TransactionType.contractInteraction })).toBe(
504+
true,
505+
);
506+
});
493507

494-
// Redesigned transaction types
495-
expect(isEnabledFn({ type: TransactionType.stakingDeposit })).toBe(true);
496-
expect(isEnabledFn({ type: TransactionType.stakingUnstake })).toBe(true);
497-
expect(isEnabledFn({ type: TransactionType.stakingClaim })).toBe(true);
498-
expect(isEnabledFn({ type: TransactionType.contractInteraction })).toBe(
499-
true,
500-
);
508+
it('returns false for non-redesigned transaction types', () => {
509+
const option = testConstructorOption('isAutomaticGasFeeUpdateEnabled');
510+
const isEnabledFn = option as ({
511+
type,
512+
}: {
513+
type: string;
514+
origin?: string;
515+
}) => boolean;
516+
517+
expect(isEnabledFn({ type: TransactionType.bridge })).toBe(false);
518+
});
519+
520+
it('returns false for transaction with nested relayDeposit type', () => {
521+
const option = testConstructorOption('isAutomaticGasFeeUpdateEnabled');
522+
const isEnabledFn = option as (transaction: {
523+
type: string;
524+
origin?: string;
525+
nestedTransactions?: { type: string }[];
526+
}) => boolean;
527+
528+
const result = isEnabledFn({
529+
type: TransactionType.contractInteraction,
530+
nestedTransactions: [{ type: TransactionType.relayDeposit }],
531+
});
532+
533+
expect(result).toBe(false);
534+
});
535+
536+
it('returns false for tokenMethodApprove with ORIGIN_METAMASK', () => {
537+
const option = testConstructorOption('isAutomaticGasFeeUpdateEnabled');
538+
const isEnabledFn = option as ({
539+
type,
540+
origin,
541+
}: {
542+
type: string;
543+
origin?: string;
544+
}) => boolean;
545+
546+
const result = isEnabledFn({
547+
type: TransactionType.tokenMethodApprove,
548+
origin: ORIGIN_METAMASK,
549+
});
550+
551+
expect(result).toBe(false);
552+
});
553+
554+
it('returns true for tokenMethodApprove with non-MetaMask origin', () => {
555+
const option = testConstructorOption('isAutomaticGasFeeUpdateEnabled');
556+
const isEnabledFn = option as ({
557+
type,
558+
origin,
559+
}: {
560+
type: string;
561+
origin?: string;
562+
}) => boolean;
563+
564+
const result = isEnabledFn({
565+
type: TransactionType.tokenMethodApprove,
566+
origin: 'https://external-dapp.com',
567+
});
501568

502-
// Non-redesigned transaction types
503-
expect(isEnabledFn({ type: TransactionType.bridge })).toBe(false);
569+
expect(result).toBe(true);
570+
});
504571
});
505572

506573
it('gets network state from network controller on option getNetworkState', () => {

app/core/Engine/controllers/transaction-controller/transaction-controller-init.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ import { trace } from '../../../../util/trace';
5050
import { Delegation7702PublishHook } from '../../../../util/transactions/hooks/delegation-7702-publish';
5151
import { isSendBundleSupported } from '../../../../util/transactions/sentinel-api';
5252
import { NetworkClientId } from '@metamask/network-controller';
53-
import { toHex } from '@metamask/controller-utils';
53+
import { ORIGIN_METAMASK, toHex } from '@metamask/controller-utils';
54+
import { hasTransactionType } from '../../../../components/Views/confirmations/utils/transaction';
5455

5556
export const TransactionControllerInit: ControllerInitFunction<
5657
TransactionController,
@@ -78,8 +79,7 @@ export const TransactionControllerInit: ControllerInitFunction<
7879
try {
7980
const transactionController: TransactionController =
8081
new TransactionController({
81-
isAutomaticGasFeeUpdateEnabled: ({ type }) =>
82-
REDESIGNED_TRANSACTION_TYPES.includes(type as TransactionType),
82+
isAutomaticGasFeeUpdateEnabled,
8383
disableHistory: true,
8484
disableSendFlowHistory: true,
8585
disableSwaps: true,
@@ -359,6 +359,23 @@ function beforeSign(
359359
return predictController.beforeSign(hookRequest);
360360
}
361361

362+
function isAutomaticGasFeeUpdateEnabled(transaction: TransactionMeta) {
363+
if (hasTransactionType(transaction, [TransactionType.relayDeposit])) {
364+
return false;
365+
}
366+
367+
if (
368+
transaction.origin === ORIGIN_METAMASK &&
369+
transaction.type === TransactionType.tokenMethodApprove
370+
) {
371+
return false;
372+
}
373+
374+
return REDESIGNED_TRANSACTION_TYPES.includes(
375+
transaction.type as TransactionType,
376+
);
377+
}
378+
362379
function addTransactionControllerListeners(
363380
transactionEventHandlerRequest: TransactionEventHandlerRequest,
364381
) {

0 commit comments

Comments
 (0)