From f02cc5ef9c2e927707da4119ea05c3aabcd50423 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Tue, 15 Sep 2026 14:47:30 +0100 Subject: [PATCH 1/2] fix(transaction-controller): use node gas estimation --- packages/transaction-controller/CHANGELOG.md | 1 + .../TransactionControllerIntegration.test.ts | 6 + .../src/utils/gas.test.ts | 118 ++++++++++++++---- .../transaction-controller/src/utils/gas.ts | 60 --------- .../tests/JsonRpcRequestMocks.ts | 6 +- 5 files changed, 101 insertions(+), 90 deletions(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 3349f05d2d0..337bf3a0243 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Use the selected network client's gas estimate for transactions without caller-provided gas instead of assigning a fixed 21,000 gas limit to plain transfers ([CONF-1995](https://consensyssoftware.atlassian.net/browse/CONF-1995)) - Bump `uuid` from `^8.3.2` to `^9.0.1` ([#10117](https://github.com/MetaMask/core/pull/10117)) - Bump `@metamask/core-backend` from `^10.0.0` to `^11.0.0` ([#10166](https://github.com/MetaMask/core/pull/10166), [#10242](https://github.com/MetaMask/core/pull/10242)) - Bump `@metamask/utils` from `^11.12.0` to `^12.0.0` ([#10192](https://github.com/MetaMask/core/pull/10192)) diff --git a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts index ab13c92de62..f7299bfa3a9 100644 --- a/packages/transaction-controller/src/TransactionControllerIntegration.test.ts +++ b/packages/transaction-controller/src/TransactionControllerIntegration.test.ts @@ -498,8 +498,10 @@ describe('TransactionController Integration', () => { ), mocks: [ buildEthBlockNumberRequestMock('0x1'), + buildEthGetBlockByNumberRequestMock('0x1'), buildEthGetCodeRequestMock(ACCOUNT_MOCK), buildEthGetCodeRequestMock(ACCOUNT_2_MOCK), + buildEthEstimateGasRequestMock(ACCOUNT_MOCK, ACCOUNT_2_MOCK), buildEthGasPriceRequestMock(), buildEthGasPriceRequestMock(), ], @@ -513,6 +515,9 @@ describe('TransactionController Integration', () => { { networkClientId: 'sepolia' }, ); expect(transactionController.state.transactions).toHaveLength(1); + expect(transactionController.state.transactions[0].txParams.gas).toBe( + '0x5208', + ); expect(transactionController.state.transactions[0].status).toBe( 'unapproved', ); @@ -1066,6 +1071,7 @@ describe('TransactionController Integration', () => { buildEthGetCodeRequestMock(ACCOUNT_2_MOCK), buildEthGetCodeRequestMock(ACCOUNT_3_MOCK), buildEthEstimateGasRequestMock(ACCOUNT_MOCK, ACCOUNT_2_MOCK), + buildEthEstimateGasRequestMock(ACCOUNT_MOCK, ACCOUNT_3_MOCK), buildEthGasPriceRequestMock(), buildEthGasPriceRequestMock(), buildEthGetTransactionCountRequestMock(ACCOUNT_MOCK), diff --git a/packages/transaction-controller/src/utils/gas.test.ts b/packages/transaction-controller/src/utils/gas.test.ts index 01e7d585a35..7384dcaa711 100644 --- a/packages/transaction-controller/src/utils/gas.test.ts +++ b/packages/transaction-controller/src/utils/gas.test.ts @@ -10,7 +10,7 @@ import type { import { simulateTransactions } from '../api/simulation-api.js'; import type { TransactionControllerMessenger } from '../TransactionController.js'; import { TransactionEnvelopeType } from '../types.js'; -import type { TransactionMeta } from '../types.js'; +import type { TransactionMeta, TransactionParams } from '../types.js'; import type { AuthorizationList, BatchTransactionParams, @@ -31,7 +31,6 @@ import { estimateGasBatch, getProvidedBatchGasLimits, updateGas, - FIXED_GAS, DEFAULT_GAS_MULTIPLIER, MAX_GAS_BLOCK_PERCENT, INTRINSIC_GAS, @@ -67,6 +66,8 @@ const GAS_2_MOCK = 12345; const SIMULATE_GAS_MOCK = 54321; const FROM_MOCK = '0xabc'; const TO_MOCK = '0xdef'; +const NEW_ACCOUNT_MOCK = '0x0000000000000000000000000000000000000001'; +const DELEGATED_ACCOUNT_MOCK = '0x0000000000000000000000000000000000000002'; const VALUE_MOCK = '0x1'; const VALUE_MOCK_2 = '0x2'; const DATA_MOCK = '0xabcdef'; @@ -461,45 +462,91 @@ describe('gas', () => { ); }); - describe('to fixed value', () => { - it('if not custom network and to parameter and no data and no code', async () => { - updateGasRequest.isCustomNetwork = false; - delete updateGasRequest.txMeta.txParams.data; + describe.each<[string, TransactionParams, Hex, Hex]>([ + [ + 'self-transfer', + { from: FROM_MOCK, to: FROM_MOCK, value: '0x0' }, + '0x5208', + '0x2ee0', + ], + [ + 'zero-value transfer to a distinct recipient', + { from: FROM_MOCK, to: TO_MOCK, value: '0x0' }, + '0x5208', + '0x3a98', + ], + [ + 'value transfer to an existing recipient', + { from: FROM_MOCK, to: TO_MOCK, value: VALUE_MOCK }, + '0x5208', + '0x5208', + ], + [ + 'value transfer to a new account', + { from: FROM_MOCK, to: NEW_ACCOUNT_MOCK, value: VALUE_MOCK }, + '0x5208', + '0xb3b0', + ], + [ + 'contract creation', + { from: FROM_MOCK, data: DATA_MOCK, value: '0x0' }, + '0xcf08', + '0x5dc0', + ], + [ + 'transfer to a delegated recipient', + { from: FROM_MOCK, to: DELEGATED_ACCOUNT_MOCK, value: '0x0' }, + '0x6590', + '0x6b6c', + ], + [ + 'transfer from a delegated sender', + { from: DELEGATED_ACCOUNT_MOCK, to: TO_MOCK, value: '0x0' }, + '0x6978', + '0x7334', + ], + ])('%s', (_description, txParams, legacyNodeGas, upgradedNodeGas) => { + it.each<[string, Hex]>([ + ['legacy node', legacyNodeGas], + ['upgraded node', upgradedNodeGas], + ])('uses the %s response', async (_node, estimatedGas) => { + updateGasRequest.txMeta.txParams = cloneDeep(txParams); + updateGasRequest.txMeta.disableGasBuffer = true; mockQuery({ - getCodeResponse: null, + getBlockByNumberResponse: { + gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK), + }, + estimateGasResponse: estimatedGas, }); await updateGas(updateGasRequest); - expect(updateGasRequest.txMeta.txParams.gas).toBe(FIXED_GAS); + expect(updateGasRequest.txMeta.txParams.gas).toBe(estimatedGas); + expect(updateGasRequest.txMeta.gasLimitNoBuffer).toBe(estimatedGas); expect(updateGasRequest.txMeta.originalGasEstimate).toBe( - updateGasRequest.txMeta.txParams.gas, + estimatedGas, ); - expectEstimateGasNotCalled(); - }); - - it('if not custom network and to parameter and no data and empty code', async () => { - updateGasRequest.isCustomNetwork = false; - delete updateGasRequest.txMeta.txParams.data; - - mockQuery({ - getCodeResponse: '0x', + expect(rpcRequestMock).toHaveBeenCalledWith({ + messenger: MESSENGER_MOCK, + networkClientId: NETWORK_CLIENT_ID_MOCK, + method: 'eth_estimateGas', + params: [ + expect.objectContaining({ + ...txParams, + value: txParams.value ?? '0x0', + }), + ], }); - - await updateGas(updateGasRequest); - - expect(updateGasRequest.txMeta.txParams.gas).toBe(FIXED_GAS); - expect(updateGasRequest.txMeta.originalGasEstimate).toBe( - updateGasRequest.txMeta.txParams.gas, + expect(rpcRequestMock).not.toHaveBeenCalledWith( + expect.objectContaining({ method: 'eth_getCode' }), ); - expectEstimateGasNotCalled(); }); }); }); describe('on estimate query error', () => { - it('sets gas to 35% of block gas limit', async () => { + it('sets gas to 35% of block gas limit for a plain transfer', async () => { const fallbackGas = Math.floor( BLOCK_GAS_LIMIT_MOCK * FALLBACK_MULTIPLIER_35_PERCENT, ); @@ -507,6 +554,7 @@ describe('gas', () => { getGasEstimateFallbackMock.mockReturnValue( GAS_ESTIMATE_FALLBACK_MULTIPLIER_MOCK, ); + delete updateGasRequest.txMeta.txParams.data; mockQuery({ getBlockByNumberResponse: { @@ -521,6 +569,9 @@ describe('gas', () => { expect(updateGasRequest.txMeta.originalGasEstimate).toBe( updateGasRequest.txMeta.txParams.gas, ); + expect(rpcRequestMock).toHaveBeenCalledWith( + expect.objectContaining({ method: 'eth_estimateGas' }), + ); }); it('sets simulationFails property', async () => { @@ -914,7 +965,16 @@ describe('gas', () => { }, ); - it('normalizes authorization list in estimate request', async () => { + it('forwards the complete transaction shape in estimate request', async () => { + const accessList = [ + { + address: '0x0000000000000000000000000000000000000001', + storageKeys: [ + '0x0000000000000000000000000000000000000000000000000000000000000002', + ], + }, + ]; + mockQuery({ getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, estimateGasResponse: toHex(GAS_MOCK), @@ -927,7 +987,9 @@ describe('gas', () => { messenger: MESSENGER_MOCK, txParams: { ...TRANSACTION_META_MOCK.txParams, + accessList, authorizationList: AUTHORIZATION_LIST_MOCK, + type: TransactionEnvelopeType.setCode, value: undefined, }, }); @@ -939,6 +1001,7 @@ describe('gas', () => { params: [ { ...TRANSACTION_META_MOCK.txParams, + accessList, authorizationList: [ { ...AUTHORIZATION_LIST_MOCK[0], @@ -949,6 +1012,7 @@ describe('gas', () => { yParity: '0x1', }, ], + type: TransactionEnvelopeType.setCode, value: '0x0', }, ], diff --git a/packages/transaction-controller/src/utils/gas.ts b/packages/transaction-controller/src/utils/gas.ts index 487c3092e14..d2ac7a68d44 100644 --- a/packages/transaction-controller/src/utils/gas.ts +++ b/packages/transaction-controller/src/utils/gas.ts @@ -57,7 +57,6 @@ export type EstimateGasBatchResult = { export const log = createModuleLogger(projectLogger, 'gas'); -export const FIXED_GAS = '0x5208'; export const DEFAULT_GAS_MULTIPLIER = 1.5; export const MAX_GAS_BLOCK_PERCENT = 90; export const INTRINSIC_GAS = 21000; @@ -516,11 +515,6 @@ async function getGas( return [txMeta.txParams.gas, undefined, txMeta.txParams.gas]; } - if (await requiresFixedGas(request)) { - log('Using fixed value', FIXED_GAS); - return [FIXED_GAS, undefined, FIXED_GAS]; - } - const { blockGasLimit, estimatedGas, @@ -569,60 +563,6 @@ async function getGas( return [bufferedGas, simulationFails, estimatedGas, gasRevert]; } -/** - * Determine if the gas for the provided request should be fixed. - * - * @param options - The options object. - * @param options.messenger - The messenger instance for communication. - * @param options.txMeta - The transaction meta object. - * @param options.isCustomNetwork - Whether the network is a custom network. - * @returns Whether the gas should be fixed. - */ -async function requiresFixedGas({ - messenger, - txMeta, - isCustomNetwork, -}: UpdateGasRequest): Promise { - const { - networkClientId, - txParams: { to, data, type }, - } = txMeta; - - if ( - isCustomNetwork || - !to || - data || - type === TransactionEnvelopeType.setCode - ) { - return false; - } - - const code = await getCode(messenger, networkClientId, to); - - return !code || code === '0x'; -} - -/** - * Get the contract code for the provided address. - * - * @param messenger - The messenger instance for communication. - * @param networkClientId - The network client ID. - * @param address - The address to get the code for. - * @returns The contract code. - */ -async function getCode( - messenger: TransactionControllerMessenger, - networkClientId: NetworkClientId, - address: string, -): Promise { - return (await rpcRequest({ - messenger, - networkClientId, - method: 'eth_getCode', - params: [address, 'latest'], - })) as string | undefined; -} - /** * Get the latest block from the network. * diff --git a/packages/transaction-controller/tests/JsonRpcRequestMocks.ts b/packages/transaction-controller/tests/JsonRpcRequestMocks.ts index 3d89349b061..66fa144d43a 100644 --- a/packages/transaction-controller/tests/JsonRpcRequestMocks.ts +++ b/packages/transaction-controller/tests/JsonRpcRequestMocks.ts @@ -45,7 +45,7 @@ export function buildEthBlockNumberRequestMock( /** * Builds mock eth_getCode request. - * Used by readAddressAsContract and requiresFixedGas. + * Used by readAddressAsContract. * * @param address - The hex address. * @param blockNumber - The hex block number. @@ -106,7 +106,7 @@ export function buildEthGetBlockByNumberRequestMock( export function buildEthEstimateGasRequestMock( from: Hex, to: Hex, - result: Hex = '0x1', + result: Hex = '0x5208', ): JsonRpcRequestMock { return { request: { @@ -116,7 +116,7 @@ export function buildEthEstimateGasRequestMock( from, to, value: '0x0', - gas: '0x0', + type: '0x2', }, ], }, From 4f787cc6f41a37df8c6db4b68846e0408f1eba4f Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Tue, 15 Sep 2026 14:48:43 +0100 Subject: [PATCH 2/2] chore(transaction-controller): link changelog to PR --- packages/transaction-controller/CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 337bf3a0243..92f54954633 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,11 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- Use the selected network client's gas estimate for transactions without caller-provided gas instead of assigning a fixed 21,000 gas limit to plain transfers ([#10245](https://github.com/MetaMask/core/pull/10245)) + ## [70.0.1] ### Changed -- Use the selected network client's gas estimate for transactions without caller-provided gas instead of assigning a fixed 21,000 gas limit to plain transfers ([CONF-1995](https://consensyssoftware.atlassian.net/browse/CONF-1995)) - Bump `uuid` from `^8.3.2` to `^9.0.1` ([#10117](https://github.com/MetaMask/core/pull/10117)) - Bump `@metamask/core-backend` from `^10.0.0` to `^11.0.0` ([#10166](https://github.com/MetaMask/core/pull/10166), [#10242](https://github.com/MetaMask/core/pull/10242)) - Bump `@metamask/utils` from `^11.12.0` to `^12.0.0` ([#10192](https://github.com/MetaMask/core/pull/10192))