Skip to content

Commit 3ca2d36

Browse files
feat: skip simulation when enforced simulations container is active (#8431)
## 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 - Extension PR: MetaMask/metamask-extension#41126 <!-- CURSOR_SUMMARY --> --- > [!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. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit b1b0199. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 90d8093 commit 3ca2d36

3 files changed

Lines changed: 47 additions & 3 deletions

File tree

packages/transaction-controller/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1111

1212
- Add `MantleLayer1GasFeeFlow` with `tokenRatio` conversion for accurate MNT-denominated gas estimates for Mantle and MantleSepolia ([#8386](https://github.com/MetaMask/core/pull/8386))
1313

14+
### Changed
15+
16+
- Skip simulation when transaction `containerTypes` includes `EnforcedSimulations` ([#8431](https://github.com/MetaMask/core/pull/8431))
17+
1418
## [64.2.0]
1519

1620
### Added

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2345,6 +2345,37 @@ describe('TransactionController', () => {
23452345
});
23462346
});
23472347

2348+
it('skips simulation when containerTypes includes EnforcedSimulations', async () => {
2349+
const { controller } = setupController();
2350+
2351+
const { transactionMeta } = await controller.addTransaction(
2352+
{
2353+
from: ACCOUNT_MOCK,
2354+
to: ACCOUNT_MOCK,
2355+
},
2356+
{
2357+
networkClientId: NETWORK_CLIENT_ID_MOCK,
2358+
},
2359+
);
2360+
2361+
await flushPromises();
2362+
2363+
expect(getBalanceChangesMock).toHaveBeenCalledTimes(1);
2364+
2365+
shouldResimulateMock.mockReturnValue({
2366+
blockTime: 123,
2367+
resimulate: true,
2368+
});
2369+
2370+
await controller.updateEditableParams(transactionMeta.id, {
2371+
containerTypes: [TransactionContainerType.EnforcedSimulations],
2372+
});
2373+
2374+
await flushPromises();
2375+
2376+
expect(getBalanceChangesMock).toHaveBeenCalledTimes(1);
2377+
});
2378+
23482379
describe('with beforeSign hook', () => {
23492380
it('calls beforeSign hook', async () => {
23502381
const beforeSignHook = jest.fn().mockResolvedValueOnce({});

packages/transaction-controller/src/TransactionController.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,6 @@ import type {
122122
TransactionBatchMeta,
123123
AfterSimulateHook,
124124
BeforeSignHook,
125-
TransactionContainerType,
126125
GetSimulationConfig,
127126
AddTransactionOptions,
128127
PublishHookResult,
@@ -131,6 +130,7 @@ import type {
131130
} from './types';
132131
import {
133132
GasFeeEstimateLevel,
133+
TransactionContainerType,
134134
TransactionEnvelopeType,
135135
TransactionType,
136136
TransactionStatus,
@@ -4241,6 +4241,15 @@ export class TransactionController extends BaseController<
42414241
return transactionMeta;
42424242
}
42434243

4244+
#isBalanceChangesSkipped(transactionMeta: TransactionMeta): boolean {
4245+
return (
4246+
this.#skipSimulationTransactionIds.has(transactionMeta.id) ||
4247+
transactionMeta.containerTypes?.includes(
4248+
TransactionContainerType.EnforcedSimulations,
4249+
) === true
4250+
);
4251+
}
4252+
42444253
async #updateSimulationData(
42454254
transactionMeta: TransactionMeta,
42464255
{
@@ -4272,7 +4281,7 @@ export class TransactionController extends BaseController<
42724281
let isGasFeeSponsored = false;
42734282

42744283
const isBalanceChangesSkipped =
4275-
this.#skipSimulationTransactionIds.has(transactionId);
4284+
this.#isBalanceChangesSkipped(transactionMeta);
42764285

42774286
if (this.#isSimulationEnabled() && !isBalanceChangesSkipped) {
42784287
const balanceChangesResult = await this.#trace(
@@ -4336,7 +4345,7 @@ export class TransactionController extends BaseController<
43364345
txMeta.isGasFeeSponsored = isGasFeeSponsored;
43374346
txMeta.gasUsed = gasUsed;
43384347

4339-
if (!isBalanceChangesSkipped) {
4348+
if (!this.#isBalanceChangesSkipped(txMeta)) {
43404349
txMeta.simulationData = simulationData;
43414350
}
43424351
},

0 commit comments

Comments
 (0)