diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 24443eb4cb..edaa1cc775 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add optional `maxGasLimit` to the `gasEstimateFallback` feature flag config, clamping the fixed or percentage-derived gas estimate fallback to a chain's per-transaction gas cap so it can never exceed the limit the RPC will accept ([#9191](https://github.com/MetaMask/core/pull/9191)) + ## [68.0.1] ### Fixed diff --git a/packages/transaction-controller/src/utils/feature-flags.test.ts b/packages/transaction-controller/src/utils/feature-flags.test.ts index 624d73ff18..405008c10b 100644 --- a/packages/transaction-controller/src/utils/feature-flags.test.ts +++ b/packages/transaction-controller/src/utils/feature-flags.test.ts @@ -36,6 +36,7 @@ const SIGNATURE_MOCK = '0xcba' as Hex; const DEFAULT_GAS_ESTIMATE_FALLBACK_MOCK = 35; const GAS_ESTIMATE_FALLBACK_MOCK = 50; const FIXED_GAS_MOCK = 100000; +const MAX_GAS_LIMIT_MOCK = 33554432; const GAS_BUFFER_MOCK = 1.1; const GAS_BUFFER_2_MOCK = 1.2; const GAS_BUFFER_3_MOCK = 1.3; @@ -611,6 +612,7 @@ describe('Feature Flags Utils', () => { [CHAIN_ID_MOCK]: { fixed: FIXED_GAS_MOCK, percentage: GAS_ESTIMATE_FALLBACK_MOCK, + maxGasLimit: MAX_GAS_LIMIT_MOCK, }, }, }, @@ -622,6 +624,7 @@ describe('Feature Flags Utils', () => { ).toStrictEqual({ fixed: FIXED_GAS_MOCK, percentage: GAS_ESTIMATE_FALLBACK_MOCK, + maxGasLimit: MAX_GAS_LIMIT_MOCK, }); }); @@ -642,6 +645,55 @@ describe('Feature Flags Utils', () => { ).toStrictEqual({ fixed: undefined, percentage: DEFAULT_GAS_ESTIMATE_FALLBACK_MOCK, + maxGasLimit: undefined, + }); + }); + + it('returns maxGasLimit from the default config when no chain-specific value is set', () => { + mockFeatureFlags({ + [FeatureFlag.Transactions]: { + gasEstimateFallback: { + default: { + percentage: DEFAULT_GAS_ESTIMATE_FALLBACK_MOCK, + maxGasLimit: MAX_GAS_LIMIT_MOCK, + }, + }, + }, + }); + + expect( + getGasEstimateFallback(CHAIN_ID_MOCK, controllerMessenger), + ).toStrictEqual({ + fixed: undefined, + percentage: DEFAULT_GAS_ESTIMATE_FALLBACK_MOCK, + maxGasLimit: MAX_GAS_LIMIT_MOCK, + }); + }); + + it('prefers the chain-specific maxGasLimit over the default', () => { + mockFeatureFlags({ + [FeatureFlag.Transactions]: { + gasEstimateFallback: { + default: { + percentage: DEFAULT_GAS_ESTIMATE_FALLBACK_MOCK, + maxGasLimit: MAX_GAS_LIMIT_MOCK * 2, + }, + perChainConfig: { + [CHAIN_ID_MOCK]: { + percentage: GAS_ESTIMATE_FALLBACK_MOCK, + maxGasLimit: MAX_GAS_LIMIT_MOCK, + }, + }, + }, + }, + }); + + expect( + getGasEstimateFallback(CHAIN_ID_MOCK, controllerMessenger), + ).toStrictEqual({ + fixed: undefined, + percentage: GAS_ESTIMATE_FALLBACK_MOCK, + maxGasLimit: MAX_GAS_LIMIT_MOCK, }); }); }); diff --git a/packages/transaction-controller/src/utils/feature-flags.ts b/packages/transaction-controller/src/utils/feature-flags.ts index ee65bc0967..128e5844cf 100644 --- a/packages/transaction-controller/src/utils/feature-flags.ts +++ b/packages/transaction-controller/src/utils/feature-flags.ts @@ -34,6 +34,14 @@ type GasEstimateFallback = { * The percentage multiplier gas estimate fallback for a transaction. */ percentage?: number; + + /** + * The maximum gas limit the fallback can resolve to, representing the chain's + * per-transaction gas cap. Clamps the `fixed` or `percentage`-derived fallback + * so it can never exceed the gas limit the RPC will accept (e.g. ~33.5M on + * Polygon, whereas 35% of its ~140M block gas limit would otherwise be ~49M). + */ + maxGasLimit?: number; }; export type TransactionControllerFeatureFlags = { @@ -369,6 +377,7 @@ export function getGasEstimateFallback( ): { fixed?: number; percentage: number; + maxGasLimit?: number; } { const featureFlags = getFeatureFlags(messenger); @@ -384,7 +393,10 @@ export function getGasEstimateFallback( const fixed = chainFlags?.fixed ?? gasEstimateFallbackFlags?.default?.fixed; - return { fixed, percentage }; + const maxGasLimit = + chainFlags?.maxGasLimit ?? gasEstimateFallbackFlags?.default?.maxGasLimit; + + return { fixed, percentage, maxGasLimit }; } /** diff --git a/packages/transaction-controller/src/utils/gas.test.ts b/packages/transaction-controller/src/utils/gas.test.ts index b17f6e07df..a54716235e 100644 --- a/packages/transaction-controller/src/utils/gas.test.ts +++ b/packages/transaction-controller/src/utils/gas.test.ts @@ -724,6 +724,84 @@ describe('gas', () => { }); }); + it('clamps the percentage-derived fallback to maxGasLimit when it exceeds the chain per-tx cap on error', async () => { + const maxGasLimit = + Math.floor(BLOCK_GAS_LIMIT_MOCK * FALLBACK_MULTIPLIER_35_PERCENT) - 1; + + getGasEstimateFallbackMock.mockReturnValue({ + percentage: DEFAULT_GAS_ESTIMATE_FALLBACK_MOCK, + fixed: undefined, + maxGasLimit, + }); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: { message: 'TestError', errorKey: 'TestKey' }, + }); + + const result = await estimateGas({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + isSimulationEnabled: false, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + messenger: MESSENGER_MOCK, + txParams: TRANSACTION_META_MOCK.txParams, + }); + + expect(result.estimatedGas).toBe(toHex(maxGasLimit)); + }); + + it('does not clamp the fallback when it is below maxGasLimit on error', async () => { + const fallbackGas = Math.floor( + BLOCK_GAS_LIMIT_MOCK * FALLBACK_MULTIPLIER_35_PERCENT, + ); + + getGasEstimateFallbackMock.mockReturnValue({ + percentage: DEFAULT_GAS_ESTIMATE_FALLBACK_MOCK, + fixed: undefined, + maxGasLimit: BLOCK_GAS_LIMIT_MOCK, + }); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: { message: 'TestError', errorKey: 'TestKey' }, + }); + + const result = await estimateGas({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + isSimulationEnabled: false, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + messenger: MESSENGER_MOCK, + txParams: TRANSACTION_META_MOCK.txParams, + }); + + expect(result.estimatedGas).toBe(toHex(fallbackGas)); + }); + + it('clamps the fixed fallback to maxGasLimit when it exceeds the chain per-tx cap on error', async () => { + const maxGasLimit = FIXED_ESTIMATE_GAS_MOCK - 1; + + getGasEstimateFallbackMock.mockReturnValue({ + percentage: DEFAULT_GAS_ESTIMATE_FALLBACK_MOCK, + fixed: FIXED_ESTIMATE_GAS_MOCK, + maxGasLimit, + }); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: { message: 'TestError', errorKey: 'TestKey' }, + }); + + const result = await estimateGas({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + isSimulationEnabled: false, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + messenger: MESSENGER_MOCK, + txParams: TRANSACTION_META_MOCK.txParams, + }); + + expect(result.estimatedGas).toBe(toHex(maxGasLimit)); + }); + it('removes gas fee properties from estimate request', async () => { mockQuery({ getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, diff --git a/packages/transaction-controller/src/utils/gas.ts b/packages/transaction-controller/src/utils/gas.ts index 9cf9e7bbf0..f04568a6a9 100644 --- a/packages/transaction-controller/src/utils/gas.ts +++ b/packages/transaction-controller/src/utils/gas.ts @@ -135,12 +135,26 @@ export async function estimateGas({ ); const blockGasLimitBN = hexToBN(blockGasLimit); - const { percentage, fixed } = getGasEstimateFallback(chainId, messenger); + const { percentage, fixed, maxGasLimit } = getGasEstimateFallback( + chainId, + messenger, + ); - const fallback = fixed + const uncappedFallback = fixed ? toHex(fixed) : BNToHex(fractionBN(blockGasLimitBN, percentage, 100)); + // Clamp the fallback to the chain's per-transaction gas cap so it can never + // exceed the gas limit the RPC will accept. Without this, a percentage of a + // high block gas limit can overshoot the cap (e.g. 35% of Polygon's ~140M + // block limit is ~49M, but the node caps a tx at ~33.5M and rejects it with + // "transaction gas limit too high"). + const fallback = + maxGasLimit !== undefined && + hexToBN(uncappedFallback).gt(hexToBN(toHex(maxGasLimit))) + ? toHex(maxGasLimit) + : uncappedFallback; + log('Estimation fallback values', fallback); request.data = data ? add0x(data) : data;