Skip to content

feat: skip simulation when enforced simulations container is active#8431

Merged
matthewwalsh0 merged 5 commits intomainfrom
feat/enforced-simulations-skip-simulation
Apr 14, 2026
Merged

feat: skip simulation when enforced simulations container is active#8431
matthewwalsh0 merged 5 commits intomainfrom
feat/enforced-simulations-skip-simulation

Conversation

@matthewwalsh0
Copy link
Copy Markdown
Member

@matthewwalsh0 matthewwalsh0 commented Apr 13, 2026

Explanation

When a transaction has the EnforcedSimulations container type, the simulation is redundant since the enforced simulation flow handles balance change protection separately. This PR skips the standard simulation in that case to avoid unnecessary network calls and potential conflicts.

Changes

  • Skip balance change simulation when containerTypes includes EnforcedSimulations
  • Re-check containerTypes in the update callback to handle the race condition where simulation starts before the container type is set
  • Use fresh state from skipSimulationTransactionIds in the callback instead of a stale closure, fixing an issue where toggling enforced simulations off would not restore balance changes

References


Note

Medium Risk
Changes when/if simulationData is produced and persisted during transaction updates, which could affect balance-change protection behavior and downstream consumers that expect simulation results. Scope is contained and covered by a new unit test, but it touches core transaction lifecycle logic.

Overview
Transactions marked with containerTypes: [EnforcedSimulations] now skip balance-change simulation (and avoid getBalanceChanges calls) when editable params change and would otherwise trigger re-simulation.

The simulation update path now re-checks the latest containerTypes/skip state inside the state update callback via a new #isBalanceChangesSkipped helper to avoid race conditions and prevent persisting simulationData when simulation is meant to be bypassed. Changelog updated and a regression test added to assert simulation is not re-run in this mode.

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

@matthewwalsh0 matthewwalsh0 force-pushed the feat/enforced-simulations-skip-simulation branch from 54f1e0f to 013b194 Compare April 13, 2026 01:04
@matthewwalsh0 matthewwalsh0 force-pushed the feat/enforced-simulations-skip-simulation branch from 68372e6 to b2e2316 Compare April 14, 2026 10:11
github-merge-queue bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Apr 14, 2026
…41126)

## **Description**

Adds trust signal awareness to enforced simulations. The enforced
simulations row only appears when at least one transaction recipient is
**not trusted** by the security alerts API. When shown, it defaults ON
but the user can opt out.

**Key changes:**

- Consolidated eligibility logic in a single shared function checking
env, origin, delegation, balance changes, and trust signals
- Supports nested transactions — scans all `to` addresses including
those from `wallet_sendCalls` batches
- Uses `txParamsOriginal.to` to check the real recipient, not the
delegation manager address after container wrapping
- Unsupported chains skip the trust signal check (remain eligible since
trust can't be verified)
- Background hook simplified to `beforeSign` only — simulation skip
moved to core via `containerTypes`
- Fixed toggle stability: row persists during re-simulation, auto-enable
fires only once, loading spinner shown during initialization
- Fixed container unwrapping not clearing `data` field when reverting to
original params

**Core dependency:** MetaMask/core#8431
**Related:** #41679
(wallet_sendCalls trust signal middleware support)

## **Changelog**

CHANGELOG entry: null

## **Related issues**

Fixes:

## **Manual testing steps**

1. Enable `ENABLE_ENFORCED_SIMULATIONS` env flag and build
2. Initiate a transaction with a delegated account to a non-trusted
address
3. Verify the enforced simulations row appears and defaults ON
4. Verify it does not appear for trusted addresses or while trust
signals load
5. Toggle off — verify it stays off and balance changes remain visible

<!--
## **Screenshots/Recordings**
### **Before**
### **After**
-->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/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-extension/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).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Updates transaction pre-sign hooking and eligibility logic for
enforced simulations, affecting when simulations are enforced during
signing and when the opt-out UI appears. Moderate risk due to changes in
transaction-flow gating and reliance on trust-signal cache/state.
> 
> **Overview**
> **Enforced simulations eligibility is now trust-signal aware.**
`isEnforcedSimulationsEligible` accepts optional trust-signal state and
only returns eligible (on supported chains) when at least one recipient
(`txParamsOriginal.to`, `txParams.to`, or `nestedTransactions[].to`) is
loaded and *not* `Trusted`; unsupported chains bypass the trust check.
> 
> **Transaction flow enforcement is simplified to `beforeSign`.** The
`EnforceSimulationHook` drops the `afterSimulate` path, takes an
injected `isEligible` predicate, and only applies containers when
`containerTypes` explicitly include
`TransactionContainerType.EnforcedSimulations`.
> 
> **Confirmation UI now uses a dedicated hook and auto-enables once.**
Adds `useIsEnforcedSimulationsEligible` (wired to
`metamask.addressSecurityAlertResponses`) and updates
`EnforcedSimulationsRow` to (a) render based on eligibility, (b) default
ON by writing `containerTypes` once when unset, and (c) show a loading
spinner during initialization/toggles.
> 
> **Fixes a container unwrap edge case.**
`applyTransactionContainersExisting` now defaults `txParams.data` to
`0x` when unwrapping leaves it undefined, with added unit coverage;
tests and console baselines are updated accordingly.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
70f3d05. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
github-merge-queue bot pushed a commit to MetaMask/metamask-extension that referenced this pull request Apr 14, 2026
…41126)

## **Description**

Adds trust signal awareness to enforced simulations. The enforced
simulations row only appears when at least one transaction recipient is
**not trusted** by the security alerts API. When shown, it defaults ON
but the user can opt out.

**Key changes:**

- Consolidated eligibility logic in a single shared function checking
env, origin, delegation, balance changes, and trust signals
- Supports nested transactions — scans all `to` addresses including
those from `wallet_sendCalls` batches
- Uses `txParamsOriginal.to` to check the real recipient, not the
delegation manager address after container wrapping
- Unsupported chains skip the trust signal check (remain eligible since
trust can't be verified)
- Background hook simplified to `beforeSign` only — simulation skip
moved to core via `containerTypes`
- Fixed toggle stability: row persists during re-simulation, auto-enable
fires only once, loading spinner shown during initialization
- Fixed container unwrapping not clearing `data` field when reverting to
original params

**Core dependency:** MetaMask/core#8431
**Related:** #41679
(wallet_sendCalls trust signal middleware support)

## **Changelog**

CHANGELOG entry: null

## **Related issues**

Fixes:

## **Manual testing steps**

1. Enable `ENABLE_ENFORCED_SIMULATIONS` env flag and build
2. Initiate a transaction with a delegated account to a non-trusted
address
3. Verify the enforced simulations row appears and defaults ON
4. Verify it does not appear for trusted addresses or while trust
signals load
5. Toggle off — verify it stays off and balance changes remain visible

<!--
## **Screenshots/Recordings**
### **Before**
### **After**
-->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/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-extension/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).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Updates transaction pre-sign hooking and eligibility logic for
enforced simulations, affecting when simulations are enforced during
signing and when the opt-out UI appears. Moderate risk due to changes in
transaction-flow gating and reliance on trust-signal cache/state.
> 
> **Overview**
> **Enforced simulations eligibility is now trust-signal aware.**
`isEnforcedSimulationsEligible` accepts optional trust-signal state and
only returns eligible (on supported chains) when at least one recipient
(`txParamsOriginal.to`, `txParams.to`, or `nestedTransactions[].to`) is
loaded and *not* `Trusted`; unsupported chains bypass the trust check.
> 
> **Transaction flow enforcement is simplified to `beforeSign`.** The
`EnforceSimulationHook` drops the `afterSimulate` path, takes an
injected `isEligible` predicate, and only applies containers when
`containerTypes` explicitly include
`TransactionContainerType.EnforcedSimulations`.
> 
> **Confirmation UI now uses a dedicated hook and auto-enables once.**
Adds `useIsEnforcedSimulationsEligible` (wired to
`metamask.addressSecurityAlertResponses`) and updates
`EnforcedSimulationsRow` to (a) render based on eligibility, (b) default
ON by writing `containerTypes` once when unset, and (c) show a loading
spinner during initialization/toggles.
> 
> **Fixes a container unwrap edge case.**
`applyTransactionContainersExisting` now defaults `txParams.data` to
`0x` when unwrapping leaves it undefined, with added unit coverage;
tests and console baselines are updated accordingly.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
70f3d05. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@matthewwalsh0 matthewwalsh0 marked this pull request as ready for review April 14, 2026 12:42
@matthewwalsh0 matthewwalsh0 requested review from a team as code owners April 14, 2026 12:42

### Changed

- Skip simulation when transaction `containerTypes` includes `EnforcedSimulations` ([#8431](https://github.com/MetaMask/core/pull/8431))
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.

Merge conflict?

Suggested change
- Skip simulation when transaction `containerTypes` includes `EnforcedSimulations` ([#8431](https://github.com/MetaMask/core/pull/8431))

vinistevam
vinistevam previously approved these changes Apr 14, 2026
@matthewwalsh0 matthewwalsh0 enabled auto-merge April 14, 2026 13:01
- Check containerTypes for EnforcedSimulations to skip balance change simulation
- Re-check containerTypes in update callback to handle race conditions
- Use fresh state in callback to avoid stale closure when toggling off
@matthewwalsh0 matthewwalsh0 force-pushed the feat/enforced-simulations-skip-simulation branch from e180df1 to 665eb92 Compare April 14, 2026 13:11
@matthewwalsh0 matthewwalsh0 added this pull request to the merge queue Apr 14, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 14, 2026
@matthewwalsh0 matthewwalsh0 added this pull request to the merge queue Apr 14, 2026
Merged via the queue into main with commit 3ca2d36 Apr 14, 2026
341 checks passed
@matthewwalsh0 matthewwalsh0 deleted the feat/enforced-simulations-skip-simulation branch April 14, 2026 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants