feat: skip simulation when enforced simulations container is active#8431
Merged
matthewwalsh0 merged 5 commits intomainfrom Apr 14, 2026
Merged
feat: skip simulation when enforced simulations container is active#8431matthewwalsh0 merged 5 commits intomainfrom
matthewwalsh0 merged 5 commits intomainfrom
Conversation
6 tasks
54f1e0f to
013b194
Compare
68372e6 to
b2e2316
Compare
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 -->
vinistevam
reviewed
Apr 14, 2026
|
|
||
| ### Changed | ||
|
|
||
| - Skip simulation when transaction `containerTypes` includes `EnforcedSimulations` ([#8431](https://github.com/MetaMask/core/pull/8431)) |
Contributor
There was a problem hiding this comment.
Merge conflict?
Suggested change
| - Skip simulation when transaction `containerTypes` includes `EnforcedSimulations` ([#8431](https://github.com/MetaMask/core/pull/8431)) |
vinistevam
previously approved these changes
Apr 14, 2026
- 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
e180df1 to
665eb92
Compare
vinistevam
approved these changes
Apr 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
When a transaction has the
EnforcedSimulationscontainer 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
containerTypesincludesEnforcedSimulationscontainerTypesin the update callback to handle the race condition where simulation starts before the container type is setskipSimulationTransactionIdsin the callback instead of a stale closure, fixing an issue where toggling enforced simulations off would not restore balance changesReferences
Note
Medium Risk
Changes when/if
simulationDatais 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 avoidgetBalanceChangescalls) 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#isBalanceChangesSkippedhelper to avoid race conditions and prevent persistingsimulationDatawhen 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.