diff --git a/packages/bridge-status-controller/CHANGELOG.md b/packages/bridge-status-controller/CHANGELOG.md index 32c963a42e..b3da499071 100644 --- a/packages/bridge-status-controller/CHANGELOG.md +++ b/packages/bridge-status-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Pass `isInternal: true` to all internal `addTransaction` / `addTransactionBatch` calls to adopt the explicit `isInternal` flag introduced in `@metamask/transaction-controller` ([#8633](https://github.com/MetaMask/core/pull/8633)) - Bump `@metamask/profile-sync-controller` from `^28.0.2` to `^28.1.0` ([#8783](https://github.com/MetaMask/core/pull/8783)) - Bump `@metamask/transaction-controller` from `^65.3.0` to `^65.4.0` ([#8796](https://github.com/MetaMask/core/pull/8796)) - Bump `@metamask/gas-fee-controller` from `^26.2.1` to `^26.2.2` ([#8834](https://github.com/MetaMask/core/pull/8834)) diff --git a/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap b/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap index 8291c15791..06e65352d1 100644 --- a/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap +++ b/packages/bridge-status-controller/src/__snapshots__/bridge-status-controller.test.ts.snap @@ -686,6 +686,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should delay after submitti }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -734,6 +735,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should delay after submitti }, { "actionId": "1234567892.457", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -1009,6 +1011,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should delay after submitti }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -1057,6 +1060,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should delay after submitti }, { "actionId": "1234567892.457", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -1251,6 +1255,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should handle smart transac "from": "0xaccount1", "isGasFeeIncluded": false, "isGasFeeSponsored": false, + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -1633,6 +1638,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should not call handleMobil }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -1684,6 +1690,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should not call handleMobil }, { "actionId": "1234567892.457", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -1959,6 +1966,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should not call handleMobil }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -2010,6 +2018,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should not call handleMobil }, { "actionId": "1234567892.457", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -2285,6 +2294,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should reset USDT allowance }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -2336,6 +2346,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should reset USDT allowance }, { "actionId": "1234567892.457", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -2387,6 +2398,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should reset USDT allowance }, { "actionId": "1234567893.458", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -2637,6 +2649,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should successfully submit }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -2688,6 +2701,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should successfully submit }, { "actionId": "1234567892.457", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -2938,6 +2952,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should successfully submit }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -3040,6 +3055,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should throw an error if ap }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -3168,6 +3184,7 @@ exports[`BridgeStatusController submitTx: EVM bridge should throw an error if ap }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -3447,6 +3464,7 @@ exports[`BridgeStatusController submitTx: EVM bridge waits for approval tx confi }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": true, @@ -3498,6 +3516,7 @@ exports[`BridgeStatusController submitTx: EVM bridge waits for approval tx confi }, { "actionId": "1234567892.457", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": true, @@ -3644,6 +3663,7 @@ exports[`BridgeStatusController submitTx: EVM swap should gracefully handle isAt }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -3695,6 +3715,7 @@ exports[`BridgeStatusController submitTx: EVM swap should gracefully handle isAt }, { "actionId": "1234567892.457", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -3975,6 +3996,7 @@ exports[`BridgeStatusController submitTx: EVM swap should handle smart transacti "from": "0xaccount1", "isGasFeeIncluded": false, "isGasFeeSponsored": false, + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -4230,6 +4252,7 @@ exports[`BridgeStatusController submitTx: EVM swap should successfully submit an }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum-client-id", "origin": "metamask", "requireApproval": false, @@ -4281,6 +4304,7 @@ exports[`BridgeStatusController submitTx: EVM swap should successfully submit an }, { "actionId": "1234567892.457", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, @@ -4527,6 +4551,7 @@ exports[`BridgeStatusController submitTx: EVM swap should successfully submit an }, { "actionId": "1234567891.456", + "isInternal": true, "networkClientId": "arbitrum", "origin": "metamask", "requireApproval": false, diff --git a/packages/bridge-status-controller/src/utils/transaction.ts b/packages/bridge-status-controller/src/utils/transaction.ts index b4a4a39f1c..bd858cdbb7 100644 --- a/packages/bridge-status-controller/src/utils/transaction.ts +++ b/packages/bridge-status-controller/src/utils/transaction.ts @@ -239,6 +239,7 @@ export const addSyntheticTransaction = async ( origin: 'metamask', actionId: generateActionId(), isStateOnly: true, + isInternal: true, ...args[1], }, ); @@ -459,6 +460,7 @@ export const getAddTransactionBatchParams = async ({ networkClientId, requireApproval, origin: 'metamask', + isInternal: true, from: trade.from as Hex, transactions, }; @@ -663,6 +665,7 @@ export const submitEvmTransaction = async ({ requireApproval, type: transactionType, origin: 'metamask', + isInternal: true, }; // Exclude gasLimit from trade to avoid type issues (it can be null) const { gasLimit: tradeGasLimit, ...tradeWithoutGasLimit } = trade; diff --git a/packages/earn-controller/CHANGELOG.md b/packages/earn-controller/CHANGELOG.md index 0079a05a77..602358d97c 100644 --- a/packages/earn-controller/CHANGELOG.md +++ b/packages/earn-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Pass `isInternal: true` to `addTransactionFn` in `executeLendingDeposit`, `executeLendingWithdraw`, and `executeLendingTokenApprove` so lending transactions bypass dapp-origin restrictions ([#8633](https://github.com/MetaMask/core/pull/8633)) - Bump `@metamask/account-tree-controller` from `^7.3.0` to `^7.4.0` ([#8783](https://github.com/MetaMask/core/pull/8783)) - Bump `@metamask/transaction-controller` from `^65.3.0` to `^65.4.0` ([#8796](https://github.com/MetaMask/core/pull/8796)) diff --git a/packages/earn-controller/src/EarnController.test.ts b/packages/earn-controller/src/EarnController.test.ts index c0de9c04f9..7dbf3b1ca4 100644 --- a/packages/earn-controller/src/EarnController.test.ts +++ b/packages/earn-controller/src/EarnController.test.ts @@ -2151,6 +2151,7 @@ describe('EarnController', () => { }, { networkClientId: '1', + isInternal: true, }, ); }); @@ -2207,6 +2208,7 @@ describe('EarnController', () => { }, { networkClientId: '1', + isInternal: true, }, ); }); @@ -2378,6 +2380,7 @@ describe('EarnController', () => { }, { networkClientId: '1', + isInternal: true, }, ); }); @@ -2435,6 +2438,7 @@ describe('EarnController', () => { }, { networkClientId: '1', + isInternal: true, }, ); }); @@ -2572,6 +2576,7 @@ describe('EarnController', () => { }, { networkClientId: '1', + isInternal: true, }, ); }); @@ -2629,6 +2634,7 @@ describe('EarnController', () => { }, { networkClientId: '1', + isInternal: true, }, ); }); diff --git a/packages/earn-controller/src/EarnController.ts b/packages/earn-controller/src/EarnController.ts index dc33bdf343..8715795ba4 100644 --- a/packages/earn-controller/src/EarnController.ts +++ b/packages/earn-controller/src/EarnController.ts @@ -1062,6 +1062,7 @@ export class EarnController extends BaseController< { ...txOptions, networkClientId: selectedNetworkClientId, + isInternal: true, }, ); @@ -1139,6 +1140,7 @@ export class EarnController extends BaseController< { ...txOptions, networkClientId: selectedNetworkClientId, + isInternal: true, }, ); @@ -1216,6 +1218,7 @@ export class EarnController extends BaseController< { ...txOptions, networkClientId: selectedNetworkClientId, + isInternal: true, }, ); diff --git a/packages/perps-controller/CHANGELOG.md b/packages/perps-controller/CHANGELOG.md index 38b5a6b800..1a486f4b41 100644 --- a/packages/perps-controller/CHANGELOG.md +++ b/packages/perps-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Pass `isInternal: true` to all internal `addTransaction` calls to adopt the explicit `isInternal` flag introduced in `@metamask/transaction-controller` ([#8633](https://github.com/MetaMask/core/pull/8633)) + ## [6.1.0] ### Changed diff --git a/packages/perps-controller/src/PerpsController.ts b/packages/perps-controller/src/PerpsController.ts index 008abd0401..459f34d972 100644 --- a/packages/perps-controller/src/PerpsController.ts +++ b/packages/perps-controller/src/PerpsController.ts @@ -1365,7 +1365,7 @@ export class PerpsController extends BaseController< // eslint-disable-next-line @typescript-eslint/no-explicit-any txParams as any, // eslint-disable-next-line @typescript-eslint/no-explicit-any - options as any, + { ...(options as any), isInternal: true }, ); } diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 948db6e75c..ab90ed19f7 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING:** Replace implicit `origin === 'metamask'` trust checks with an explicit `isInternal` flag on `AddTransactionOptions`, `TransactionBatchRequest`, `TransactionMeta`, and `TransactionBatchMeta`; internal callers must now pass `isInternal: true` ([#8633](https://github.com/MetaMask/core/pull/8633)) - Bump `@metamask/core-backend` from `^6.2.2` to `^6.3.0` ([#8813](https://github.com/MetaMask/core/pull/8813)) - Bump `@metamask/gas-fee-controller` from `^26.2.1` to `^26.2.2` ([#8834](https://github.com/MetaMask/core/pull/8834)) diff --git a/packages/transaction-controller/src/TransactionController.test.ts b/packages/transaction-controller/src/TransactionController.test.ts index ea2b6da60d..1c53dc6808 100644 --- a/packages/transaction-controller/src/TransactionController.test.ts +++ b/packages/transaction-controller/src/TransactionController.test.ts @@ -1656,7 +1656,7 @@ describe('TransactionController', () => { await controller.addTransaction( { from: ACCOUNT_MOCK, to: ACCOUNT_MOCK, ...extraTxParamsToSet }, - { networkClientId: NETWORK_CLIENT_ID_MOCK }, + { isInternal: true, networkClientId: NETWORK_CLIENT_ID_MOCK }, ); expect(controller.state.transactions[0].txParams.type).toBe(type); @@ -3714,7 +3714,7 @@ describe('TransactionController', () => { ).rejects.toThrow('Batch ID already exists'); }); - it('does not throw if duplicate but internal origin', async () => { + it('does not throw if duplicate but isInternal', async () => { const { controller } = setupController({ options: { state: { @@ -3735,6 +3735,7 @@ describe('TransactionController', () => { await controller.addTransaction(txParams, { batchId: BATCH_ID_MOCK, + isInternal: true, networkClientId: NETWORK_CLIENT_ID_MOCK, }); }); diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 17c070630d..d75a550ae7 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -1204,6 +1204,7 @@ export class TransactionController extends BaseController< excludeNativeTokenForFee, isGasFeeIncluded, isGasFeeSponsored, + isInternal = false, isStateOnly, method, nestedTransactions, @@ -1240,6 +1241,7 @@ export class TransactionController extends BaseController< data: txParams.data, from: txParams.from, internalAccounts, + isInternal, origin, permittedAddresses, txParams, @@ -1268,7 +1270,7 @@ export class TransactionController extends BaseController< (tx) => tx.batchId?.toLowerCase() === batchId?.toLowerCase(), ); - if (isDuplicateBatchId && origin && origin !== ORIGIN_METAMASK) { + if (isDuplicateBatchId && !isInternal) { throw new JsonRpcError( ErrorCode.DuplicateBundleId, 'Batch ID already exists', @@ -1278,6 +1280,7 @@ export class TransactionController extends BaseController< const dappSuggestedGasFees = this.#generateDappSuggestedGasFees( txParams, origin, + isInternal, ); const transactionType = @@ -1313,6 +1316,7 @@ export class TransactionController extends BaseController< ? {} : { excludeNativeTokenForFee }), isFirstTimeInteraction: undefined, + isInternal, isStateOnly, nestedTransactions, networkClientId, @@ -3635,8 +3639,9 @@ export class TransactionController extends BaseController< #generateDappSuggestedGasFees( txParams: TransactionParams, origin?: string, + isInternal?: boolean, ): DappSuggestedGasFees | undefined { - if (!origin || origin === ORIGIN_METAMASK) { + if (isInternal || !origin) { return undefined; } diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index 6ec3b783f3..6694b67dba 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -325,6 +325,9 @@ export type TransactionMeta = { */ origin?: string; + /** Whether the transaction was added by trusted internal MetaMask code. */ + isInternal?: boolean; + /** * The original dapp proposed token approval amount before edit by user. */ @@ -616,6 +619,9 @@ export type TransactionBatchMeta = { */ origin?: string; + /** Whether the batch was added by trusted internal MetaMask code. */ + isInternal?: boolean; + /** * ID of the JSON-RPC request from DAPP. */ @@ -1874,6 +1880,9 @@ export type TransactionBatchRequest = { /** Origin of the request, such as a dApp hostname or `ORIGIN_METAMASK` if internal. */ origin?: string; + /** Whether the batch was added by trusted internal MetaMask code. */ + isInternal?: boolean; + /** Whether to overwrite existing EIP-7702 delegation with MetaMask contract. */ overwriteUpgrade?: boolean; @@ -2267,6 +2276,9 @@ export type AddTransactionOptions = { /** Origin of the transaction request, such as a dApp hostname. */ origin?: string; + /** Whether the transaction was added by trusted internal MetaMask code. */ + isInternal?: boolean; + /** Custom logic to publish the transaction. */ publishHook?: PublishHook; diff --git a/packages/transaction-controller/src/utils/batch.ts b/packages/transaction-controller/src/utils/batch.ts index 625fc6108c..0645744377 100644 --- a/packages/transaction-controller/src/utils/batch.ts +++ b/packages/transaction-controller/src/utils/batch.ts @@ -131,6 +131,7 @@ export async function addTransactionBatch( validateBatchRequest({ internalAccounts: getInternalAccounts(), + isInternal: transactionBatchRequest.isInternal, request: transactionBatchRequest, sizeLimit, }); @@ -296,6 +297,7 @@ async function addTransactionBatchWith7702( from, gasFeeToken, gasLimit7702, + isInternal, networkClientId, origin, overwriteUpgrade, @@ -438,6 +440,7 @@ async function addTransactionBatchWith7702( excludeNativeTokenForFee, isGasFeeIncluded, isGasFeeSponsored, + isInternal, nestedTransactions, networkClientId, origin, @@ -724,7 +727,7 @@ async function processTransactionWithHook( updateTransaction, } = request; - const { from, networkClientId, origin } = userRequest; + const { from, isInternal, networkClientId, origin } = userRequest; if (existingTransaction) { const { id, onPublish } = existingTransaction; @@ -797,6 +800,7 @@ async function processTransactionWithHook( assetsFiatValues, batchId, disableGasBuffer: true, + isInternal, networkClientId, origin, publishHook, @@ -941,6 +945,7 @@ async function prepareApprovalData({ const { from, + isInternal, origin, networkClientId, transactions: nestedTransactions, @@ -966,6 +971,7 @@ async function prepareApprovalData({ from, gas: gasLimit, id: batchId, + isInternal, networkClientId, origin, transactions: nestedTransactions, diff --git a/packages/transaction-controller/src/utils/gas-fees.test.ts b/packages/transaction-controller/src/utils/gas-fees.test.ts index 3a3912835c..b72854dc08 100644 --- a/packages/transaction-controller/src/utils/gas-fees.test.ts +++ b/packages/transaction-controller/src/utils/gas-fees.test.ts @@ -1,4 +1,3 @@ -import { ORIGIN_METAMASK } from '@metamask/controller-utils'; import type { NetworkClientId } from '@metamask/network-controller'; import type { TransactionControllerMessenger } from '../TransactionController'; @@ -506,9 +505,9 @@ describe('gas-fees', () => { ); }); - it('to custom if request gas price but no request maxFeePerGas or maxPriorityFeePerGas and origin is metamask', async () => { + it('to custom if request gas price but no request maxFeePerGas or maxPriorityFeePerGas and isInternal', async () => { updateGasFeeRequest.txMeta.txParams.gasPrice = GAS_HEX_MOCK; - updateGasFeeRequest.txMeta.origin = ORIGIN_METAMASK; + updateGasFeeRequest.txMeta.isInternal = true; await updateGasFees(updateGasFeeRequest); @@ -517,7 +516,7 @@ describe('gas-fees', () => { ); }); - it('to medium if request gas price but no request maxFeePerGas or maxPriorityFeePerGas and origin not metamask', async () => { + it('to dappSuggested if request gas price but no request maxFeePerGas or maxPriorityFeePerGas and not isInternal', async () => { updateGasFeeRequest.txMeta.txParams.gasPrice = GAS_HEX_MOCK; updateGasFeeRequest.txMeta.origin = ORIGIN_MOCK; @@ -538,8 +537,8 @@ describe('gas-fees', () => { ); }); - it('to medium if origin is metamask', async () => { - updateGasFeeRequest.txMeta.origin = ORIGIN_METAMASK; + it('to medium if isInternal', async () => { + updateGasFeeRequest.txMeta.isInternal = true; await updateGasFees(updateGasFeeRequest); @@ -548,7 +547,7 @@ describe('gas-fees', () => { ); }); - it('to dappSuggested if origin is not metamask', async () => { + it('to dappSuggested if not isInternal', async () => { updateGasFeeRequest.txMeta.origin = ORIGIN_MOCK; await updateGasFees(updateGasFeeRequest); diff --git a/packages/transaction-controller/src/utils/gas-fees.ts b/packages/transaction-controller/src/utils/gas-fees.ts index b6b9899ed6..3598fac809 100644 --- a/packages/transaction-controller/src/utils/gas-fees.ts +++ b/packages/transaction-controller/src/utils/gas-fees.ts @@ -1,8 +1,4 @@ -import { - ORIGIN_METAMASK, - gweiDecToWEIBN, - toHex, -} from '@metamask/controller-utils'; +import { gweiDecToWEIBN, toHex } from '@metamask/controller-utils'; import type { FetchGasFeeEstimateOptions, GasFeeState, @@ -291,7 +287,7 @@ function getUserFeeLevel(request: GetGasFeeRequest): UserFeeLevel | undefined { !initialParams.maxPriorityFeePerGas && initialParams.gasPrice ) { - return txMeta.origin === ORIGIN_METAMASK + return txMeta.isInternal ? UserFeeLevel.CUSTOM : UserFeeLevel.DAPP_SUGGESTED; } @@ -313,7 +309,7 @@ function getUserFeeLevel(request: GetGasFeeRequest): UserFeeLevel | undefined { return UserFeeLevel.MEDIUM; } - if (txMeta.origin === ORIGIN_METAMASK) { + if (txMeta.isInternal) { return UserFeeLevel.MEDIUM; } diff --git a/packages/transaction-controller/src/utils/validation.test.ts b/packages/transaction-controller/src/utils/validation.test.ts index c9a2f53d64..f9bc98e764 100644 --- a/packages/transaction-controller/src/utils/validation.test.ts +++ b/packages/transaction-controller/src/utils/validation.test.ts @@ -1,4 +1,3 @@ -import { ORIGIN_METAMASK } from '@metamask/controller-utils'; import { rpcErrors } from '@metamask/rpc-errors'; import type { Hex } from '@metamask/utils'; @@ -854,6 +853,25 @@ describe('validation', () => { }), ).toBeUndefined(); }); + + it('does not throw if isInternal is true regardless of origin', async () => { + expect( + await validateTransactionOrigin({ + data: DATA_MOCK, + from: FROM_MOCK, + internalAccounts: [TO_MOCK], + isInternal: true, + origin: ORIGIN_MOCK, + permittedAddresses: ['0x123'], + selectedAddress: '0x123', + txParams: { + authorizationList: [], + to: TO_MOCK, + type: TransactionEnvelopeType.setCode, + } as TransactionParams, + }), + ).toBeUndefined(); + }); }); describe('validateParamTo', () => { @@ -904,7 +922,7 @@ describe('validation', () => { ).not.toThrow(); }); - it('does not throw if no origin and any transaction target is internal account with data', () => { + it('throws if no origin and any transaction target is internal account with data', () => { expect(() => validateBatchRequest({ ...VALIDATE_BATCH_REQUEST_MOCK, @@ -914,18 +932,19 @@ describe('validation', () => { origin: undefined, }, }), - ).not.toThrow(); + ).toThrow( + rpcErrors.invalidParams( + 'External calls to internal accounts cannot include data', + ), + ); }); - it('does not throw if internal origin and any transaction target is internal account with data', () => { + it('does not throw if internal and any transaction target is internal account with data', () => { expect(() => validateBatchRequest({ ...VALIDATE_BATCH_REQUEST_MOCK, internalAccounts: ['0x123', TO_MOCK], - request: { - ...VALIDATE_BATCH_REQUEST_MOCK.request, - origin: ORIGIN_METAMASK, - }, + isInternal: true, }), ).not.toThrow(); }); @@ -939,14 +958,11 @@ describe('validation', () => { ).toThrow(rpcErrors.invalidParams('Batch size cannot exceed 1. got: 2')); }); - it('does not throw if transaction count is internal and greater than limit', () => { + it('does not throw if internal and transaction count is greater than limit', () => { expect(() => validateBatchRequest({ ...VALIDATE_BATCH_REQUEST_MOCK, - request: { - ...VALIDATE_BATCH_REQUEST_MOCK.request, - origin: ORIGIN_METAMASK, - }, + isInternal: true, sizeLimit: 1, }), ).not.toThrow(); diff --git a/packages/transaction-controller/src/utils/validation.ts b/packages/transaction-controller/src/utils/validation.ts index a523fb2e82..c3421bf202 100644 --- a/packages/transaction-controller/src/utils/validation.ts +++ b/packages/transaction-controller/src/utils/validation.ts @@ -1,5 +1,5 @@ import { Interface } from '@ethersproject/abi'; -import { ORIGIN_METAMASK, isValidHexAddress } from '@metamask/controller-utils'; +import { isValidHexAddress } from '@metamask/controller-utils'; import { abiERC20 } from '@metamask/metamask-eth-abis'; import { JsonRpcError, providerErrors, rpcErrors } from '@metamask/rpc-errors'; import type { Hex } from '@metamask/utils'; @@ -38,6 +38,7 @@ type GasFieldsToValidate = * @param options.data - The data included in the transaction. * @param options.from - The address from which the transaction is initiated. * @param options.internalAccounts - The internal accounts added to the wallet. + * @param options.isInternal - Whether the transaction was added by trusted internal MetaMask code. * @param options.origin - The origin or source of the transaction. * @param options.permittedAddresses - The permitted accounts for the given origin. * @param options.selectedAddress - The currently selected Ethereum address in the wallet. @@ -49,6 +50,7 @@ export async function validateTransactionOrigin({ data, from, internalAccounts, + isInternal, origin, permittedAddresses, txParams, @@ -57,14 +59,13 @@ export async function validateTransactionOrigin({ data?: string; from: string; internalAccounts?: string[]; + isInternal?: boolean; origin?: string; permittedAddresses?: string[]; selectedAddress?: string; txParams: TransactionParams; type?: TransactionType; }): Promise { - const isInternal = !origin || origin === ORIGIN_METAMASK; - if (isInternal) { return; } @@ -273,27 +274,30 @@ export function validateParamTo(to?: string): void { * * @param options - Options bag. * @param options.internalAccounts - The internal accounts added to the wallet. + * @param options.isInternal - Whether the batch was added by trusted internal MetaMask code. * @param options.request - The batch request object. * @param options.sizeLimit - The maximum number of calls allowed in a batch request. */ export function validateBatchRequest({ internalAccounts, + isInternal, request, sizeLimit, }: { internalAccounts: string[]; + isInternal?: boolean; request: TransactionBatchRequest; sizeLimit: number; }): void { - const { origin } = request; - const isExternal = origin && origin !== ORIGIN_METAMASK; + if (isInternal) { + return; + } const internalAccountsNormalized = internalAccounts.map((account) => account.toLowerCase(), ); if ( - isExternal && request.transactions.some((nestedTransaction) => { const normalizedCallTo = nestedTransaction.params.to?.toLowerCase() as string; @@ -313,7 +317,7 @@ export function validateBatchRequest({ ); } - if (isExternal && request.transactions.length > sizeLimit) { + if (request.transactions.length > sizeLimit) { throw new JsonRpcError( ErrorCode.BundleTooLarge, `Batch size cannot exceed ${sizeLimit}. got: ${request.transactions.length}`, diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 3a66a07a7c..4cb6b64a73 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Pass `isInternal: true` to all internal `addTransaction` calls to adopt the explicit `isInternal` flag introduced in `@metamask/transaction-controller` ([#8633](https://github.com/MetaMask/core/pull/8633)) - Bump `@metamask/gas-fee-controller` from `^26.2.1` to `^26.2.2` ([#8834](https://github.com/MetaMask/core/pull/8834)) ### Fixed diff --git a/packages/transaction-pay-controller/src/strategy/across/across-submit.ts b/packages/transaction-pay-controller/src/strategy/across/across-submit.ts index 3c52f2395d..087595167d 100644 --- a/packages/transaction-pay-controller/src/strategy/across/across-submit.ts +++ b/packages/transaction-pay-controller/src/strategy/across/across-submit.ts @@ -274,6 +274,7 @@ async function submitTransactions( gasFeeToken, networkClientId, origin: ORIGIN_METAMASK, + isInternal: true, requireApproval: false, type: transactions[0].type, }, @@ -294,6 +295,7 @@ async function submitTransactions( gasLimit7702, networkClientId, origin: ORIGIN_METAMASK, + isInternal: true, requireApproval: false, transactions: batchTransactions, }); diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-submit.test.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-submit.test.ts index 5278b7da4e..ae08364ae8 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-submit.test.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-submit.test.ts @@ -215,8 +215,10 @@ describe('Relay Submit Utils', () => { value: '0x4d2', }, { + gasFeeToken: undefined, networkClientId: NETWORK_CLIENT_ID_MOCK, origin: ORIGIN_METAMASK, + isInternal: true, requireApproval: false, type: TransactionType.relayDeposit, }, @@ -465,8 +467,11 @@ describe('Relay Submit Utils', () => { disableHook: false, disableSequential: false, from: FROM_MOCK, + gasFeeToken: undefined, + gasLimit7702: undefined, networkClientId: NETWORK_CLIENT_ID_MOCK, origin: ORIGIN_METAMASK, + isInternal: true, overwriteUpgrade: true, requireApproval: false, transactions: [ diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-submit.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-submit.ts index 372a016ab5..b6f0fc5431 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-submit.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-submit.ts @@ -654,6 +654,7 @@ async function submitViaTransactionController( gasFeeToken, networkClientId, origin: ORIGIN_METAMASK, + isInternal: true, requireApproval: false, type: getRelayDepositType(getEffectiveTransactionType(transaction)), }, @@ -695,6 +696,7 @@ async function submitViaTransactionController( gasLimit7702, networkClientId, origin: ORIGIN_METAMASK, + isInternal: true, overwriteUpgrade: true, requireApproval: false, transactions,