Skip to content

feat: save custom gas settings#31649

Merged
pedronfigueiredo merged 11 commits into
mainfrom
pnf/save-custom-gas-settings
Jul 14, 2026
Merged

feat: save custom gas settings#31649
pedronfigueiredo merged 11 commits into
mainfrom
pnf/save-custom-gas-settings

Conversation

@pedronfigueiredo

@pedronfigueiredo pedronfigueiredo commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Persist custom gas fee settings from the advanced gas modals.
  • Read saved gas preferences when building fee options for send confirmations.
  • Wire mobile PreferencesController and TransactionController integration for saved gas fee preferences.

Related PRs

Testing

  • yarn jest app/components/Views/confirmations/components/modals/advanced-eip1559-modal/advanced-eip1559-modal.test.tsx app/components/Views/confirmations/components/modals/advanced-gas-price-modal/advanced-gas-price-modal.test.tsx app/components/Views/confirmations/hooks/gas/useGasFeeEstimateLevelOptions.test.ts app/components/Views/confirmations/hooks/gas/useGasPriceEstimateOption.test.ts app/components/Views/confirmations/hooks/gas/usePersistGasFeePreference.test.ts app/core/Engine/controllers/preferences-controller-init.test.ts app/core/Engine/controllers/transaction-controller/metrics_properties/gas.test.ts app/core/Engine/controllers/transaction-controller/transaction-controller-init.test.ts (8 suites, 78 tests).

Note

Medium Risk
Changes how default and custom gas fees are stored and applied for sends; incorrect lookup or validation could affect fee defaults, though saves are gated on complete custom values and covered by new tests.

Overview
Adds per-account, per-chain persistence for gas choices so confirmations can reuse the user’s last fee level or custom values.

On save, confirmation gas flows call usePersistGasFeePreference, which writes to PreferencesController.setAdvancedGasFee (advancedGasFee is persisted state). Advanced EIP-1559 and legacy gas price modals share useAdvancedGasFeeModal, which blocks save when custom fees are incomplete and persists mapped preferences (e.g. max base / priority fee). Preset estimate levels and gas-price estimate options persist the selected userFeeLevel as well.

TransactionController is upgraded to 69.0.0 and receives a getSavedGasFees hook that reads those preferences for the transaction’s from address and chain. Gas metrics now report dapp_proposed when UserFeeLevel.DAPP_SUGGESTED is selected.

Reviewed by Cursor Bugbot for commit 3041ea6. Bugbot is set up for automated code reviews on this repo. Configure here.

@pedronfigueiredo pedronfigueiredo requested review from a team as code owners June 12, 2026 16:33
@github-actions

Copy link
Copy Markdown
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@github-actions github-actions Bot added the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label Jun 12, 2026
@metamask-ci metamask-ci Bot added team-confirmations Push issues to confirmations team INVALID-PR-TEMPLATE PR's body doesn't match template labels Jun 12, 2026
@metamask-ci

metamask-ci Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

PR template — items to address before "Ready for review"

Warnings — informational, address before merging:

See docs/readme/ready-for-review.md for the full Definition of Ready for Review.

Comment thread app/components/Views/confirmations/hooks/send/useSendActions.ts Outdated
@github-actions github-actions Bot added the risk:medium AI analysis: medium risk label Jun 12, 2026
@pedronfigueiredo pedronfigueiredo force-pushed the pnf/save-custom-gas-settings branch 2 times, most recently from 54d7046 to 3cfc658 Compare June 12, 2026 17:03
@github-actions github-actions Bot added risk:high AI analysis: high risk and removed risk:medium AI analysis: medium risk labels Jun 12, 2026
@matthewwalsh0 matthewwalsh0 requested a review from dan437 June 13, 2026 11:00
dan437
dan437 previously approved these changes Jun 17, 2026
@dan437

dan437 commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Can you please add screenshots / video with it and resolve failing checks?

@pedronfigueiredo pedronfigueiredo force-pushed the pnf/save-custom-gas-settings branch from 3cfc658 to 8bde3a5 Compare July 3, 2026 10:50
@github-actions github-actions Bot added risk:medium AI analysis: medium risk and removed risk:high AI analysis: high risk labels Jul 3, 2026
Comment thread app/core/Engine/controllers/transaction-controller/transaction-controller-init.ts Outdated
@pedronfigueiredo pedronfigueiredo force-pushed the pnf/save-custom-gas-settings branch 2 times, most recently from 8c22fab to 2e7f7a6 Compare July 8, 2026 16:46
Comment thread app/core/Engine/controllers/transaction-controller/transaction-controller-init.ts Outdated
@pedronfigueiredo pedronfigueiredo force-pushed the pnf/save-custom-gas-settings branch from 2e7f7a6 to 43769f1 Compare July 8, 2026 17:06
@pedronfigueiredo

Copy link
Copy Markdown
Contributor Author
mobile-save-gas-settings.mov

pull Bot pushed a commit to Reality2byte/core that referenced this pull request Jul 10, 2026
…8993)

## Explanation

This expands transaction-controller saved gas fee handling so clients
can resolve saved preferences with full transaction metadata instead of
only a chain ID. That lets consumers key saved gas settings by both
chain and account while keeping the controller API responsible for
applying saved fees.

The saved gas payload now supports gas fee estimate levels (`low`,
`medium`, `high`) and legacy custom `gasPrice` values in addition to
EIP-1559 custom fee values. Saved estimate levels reuse the current gas
fee flow estimate for the saved level; custom values override only the
specific saved fields.

Transactions that already include gas fee params continue to use those
initial params, so dapp-proposed fees keep precedence over saved
preferences. Swap transaction types continue to skip saved gas fees.

## References

Related PRs:
- MetaMask/metamask-extension#43317
- MetaMask/metamask-mobile#31649

## 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
- [x] I’ve introduced breaking changes in this PR and have prepared
client adoption work to resolve them

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Breaking callback signature and changed gas-fee precedence logic can
misprice dApp txs or break extension/mobile until consumers update;
changes are confined to gas fee application paths with substantial test
coverage.
> 
> **Overview**
> **Breaking change:** `getSavedGasFees` now receives full
`TransactionMeta` instead of chain ID so clients can key saved
preferences by chain and account. The `SavedGasFees` shape adds optional
estimate `level`, legacy `gasPrice`, and makes EIP-1559 fields optional.
> 
> Gas fee application in `updateGasFees` now skips saved lookup when the
tx already has initial gas params (dapp fees win), still skips internal
txs, and can pick **low/medium/high** from the current gas fee flow when
only a level is saved. Custom `maxBaseFee` / `priorityFee` / `gasPrice`
override specific fields; `userFeeLevel` stays on the saved level only
when pricing is truly level-derived (otherwise `custom`).
> 
> Deprecated **incoming transaction** constructor options and restored
**no-op** `startIncomingTransactionPolling` /
`stopIncomingTransactionPolling` for backward compatibility, with tests.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
a173b00. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
@socket-security

socket-security Bot commented Jul 13, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Updatednpm/​@​metamask/​transaction-controller@​68.4.0 ⏵ 69.0.098 +110081 +1100 +1100

View full report

@socket-security

socket-security Bot commented Jul 13, 2026

Copy link
Copy Markdown

All alerts resolved. Learn more about Socket for GitHub.

This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored.

Ignoring alerts on:

  • npm/@metamask/transaction-controller@69.0.0

View full report

@github-actions github-actions Bot added risk:high AI analysis: high risk and removed risk:medium AI analysis: medium risk labels Jul 13, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0799ea2. Configure here.

@pedronfigueiredo

Copy link
Copy Markdown
Contributor Author

@SocketSecurity ignore npm/@metamask/transaction-controller@69.0.0

TransactionController's getSavedGasFees hook only ever supported
(chainId) => {maxBaseFee, priorityFee}, with no account context and no
level/gasPrice fields, even at the version pinned before this branch's
rebase onto main. Resolve the account via AccountsController's selected
account instead of the (unavailable) transaction account, and only
auto-restore complete custom EIP-1559 overrides, since that's what the
real API can express.
useSendActions.ts and send.ts had leftover diffs from an earlier draft
that are unrelated to the save-custom-gas-settings feature:
- Re-added an 'await' before submitEvmTransaction that was deliberately
  removed in #30088 because it blocks navigation until addTransaction
  resolves, delaying the confirmation skeleton loader mount.
- Made submitEvmTransaction return an unused transactionMeta.

Neither is used by the gas-fee persistence feature (usePersistGasFeePreference
is only wired into the advanced gas modals / estimate option hooks), so
restore both files to match main.
…init

SonarCloud flagged new_coverage at 78.7% (below the 80% gate), all
attributable to preferences-controller-init.ts, where setAdvancedGasFee's
update callback (clear preference, merge preference, account
normalization) was never invoked by any test - only its existence was
asserted indirectly via controller construction.

Add direct tests that call setAdvancedGasFee and execute the captured
update callback against a fake state, covering:
- persisting a normalized (lowercased) account key
- merging with existing preferences for other accounts on the chain
- clearing a preference when gasFeePreferences is undefined

100% line/branch coverage locally for this file.
The new setAdvancedGasFee tests failed yarn lint:tsc:
- accessed controller.update directly, which is protected on
  BaseController; cast through MutablePreferencesControllerWithSavedGasFees
  (same pattern used in preferences-controller-init.ts itself) instead.
- cast partial fake state objects directly to PreferencesStateWithSavedGasFees,
  which doesn't sufficiently overlap (missing most PreferencesState
  properties); route through 'as unknown as' since only advancedGasFee
  is exercised by the code under test.
@pedronfigueiredo pedronfigueiredo force-pushed the pnf/save-custom-gas-settings branch from 76cf2ee to a9bcd3d Compare July 13, 2026 16:17
@github-actions

Copy link
Copy Markdown
Contributor

🧪 Flaky unit test detection

Run history flaky detection

View recent run history

Historical failure rate is a hint, not proof — review each suggestion in context. See the flaky-test-detection skill for the full pattern reference and manual audit workflow.

Failures / runs sampled per window:

File 7d 15d 30d
app/core/Engine/controllers/preferences-controller-init.test.ts 0/54 0/182 0/374

AI-detected flaky patterns

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

  • J3 — Missing jest.clearAllMocks() / jest.resetAllMocks() (high)
    • The entire describe('PreferencesControllerInit') block has no beforeEach that calls jest.clearAllMocks() or jest.resetAllMocks(). The PreferencesController constructor mock and the controller.update mock accumulate call history across all tests.

This is especially risky in the newly added setAdvancedGasFee describe block: applyUpdate reads updateMock.mock.calls[updateMock.mock.calls.length - 1][0] to retrieve the last update callback. If tests run in a different order (e.g. with --randomize) or if a prior test's update call is not cleared, mock.calls.length - 1 may point to a stale callback from a previous test, causing the wrong state mutation to be applied and the assertion to fail intermittently.

Adding a top-level beforeEach(() => { jest.resetAllMocks(); }) ensures each test starts with a clean mock slate.

  • Suggested fix in app/core/Engine/controllers/preferences-controller-init.test.ts:37:
    -describe('PreferencesControllerInit', () => {
    -  it('initializes the controller', () => {
    -    const { controller } = preferencesControllerInit(getInitRequestMock());
    -    expect(controller).toBeInstanceOf(PreferencesController);
    -  });
    -
    -  it('passes the proper arguments to the controller', () => {
    -    preferencesControllerInit(getInitRequestMock());
    -
    -    const controllerMock = jest.mocked(PreferencesController);
    -    expect(controllerMock).toHaveBeenCalledWith({
    +describe('PreferencesControllerInit', () => {
    +  beforeEach(() => {
    +    jest.resetAllMocks();
    +  });
    +
    +  it('initializes the controller', () => {
    +    const { controller } = preferencesControllerInit(getInitRequestMock());
    +    expect(controller).toBeInstanceOf(PreferencesController);
    +  });
    +
    +  it('passes the proper arguments to the controller', () => {
    +    preferencesControllerInit(getInitRequestMock());
    +
    +    const controllerMock = jest.mocked(PreferencesController);
    +    expect(controllerMock).toHaveBeenCalledWith({

This check is informational only and does not block merging.

@pedronfigueiredo pedronfigueiredo removed the pr-not-ready-for-e2e Skip E2E and block merging. Remove this label once the PR is ready to run the E2E tests. label Jul 13, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokeAccounts, SmokeConfirmations, SmokeNetworkAbstractions, SmokeNetworkExpansion, SmokeSwap, SmokeStake, SmokeWalletPlatform, SmokeMoney, SmokePerps, SmokeMultiChainAPI, SmokePredictions, SmokeSeedlessOnboarding, SmokeBrowser, SmokeSnaps
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: high
  • AI Confidence: 100%
click to see 🤖 AI reasoning details

E2E Test Selection:
Hard rule (controller-version-update): @MetaMask controller package version updated in package.json: @metamask/transaction-controller. Running all tests.

Performance Test Selection:
The changes are focused on gas fee preference persistence logic and a TransactionController major version upgrade. While these affect transaction flows, they don't directly impact app launch time, login performance, asset loading, account list rendering, or other performance-measured scenarios. The gas fee changes are user-interaction-driven (modal saves) rather than affecting rendering performance or load times. No performance spec files were changed.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

@pedronfigueiredo pedronfigueiredo added this pull request to the merge queue Jul 14, 2026
Merged via the queue into main with commit 2684ed1 Jul 14, 2026
276 of 278 checks passed
@pedronfigueiredo pedronfigueiredo deleted the pnf/save-custom-gas-settings branch July 14, 2026 12:58
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 14, 2026
@metamask-ci metamask-ci Bot added the release-8.4.0 Issue or pull request that will be included in release 8.4.0 label Jul 14, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

INVALID-PR-TEMPLATE PR's body doesn't match template release-8.4.0 Issue or pull request that will be included in release 8.4.0 risk:high AI analysis: high risk size-L team-confirmations Push issues to confirmations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants