diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index f8c38eebac4..53c78b3b8b8 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] +### Fixed + +- Harden gas fee token preflight by not treating pending gas estimates as zero-cost native gas, and by resetting `isExternalSign` when preflight validation fails ([#10071](https://github.com/MetaMask/core/pull/10071)) + ## [69.7.0] ### Added diff --git a/packages/transaction-controller/src/utils/balance.test.ts b/packages/transaction-controller/src/utils/balance.test.ts index a230667a78b..e3b145ccbbe 100644 --- a/packages/transaction-controller/src/utils/balance.test.ts +++ b/packages/transaction-controller/src/utils/balance.test.ts @@ -75,5 +75,40 @@ describe('Balance Utils', () => { expect(result).toBe(false); }); + + it('returns false if gas estimate is missing', async () => { + const result = await isNativeBalanceSufficientForGas( + { + ...TRANSACTION_META_MOCK, + txParams: { + ...TRANSACTION_META_MOCK.txParams, + gas: undefined, + }, + }, + MESSENGER_MOCK, + NETWORK_CLIENT_ID_MOCK, + ); + + expect(result).toBe(false); + expect(rpcRequestMock).not.toHaveBeenCalled(); + }); + + it('returns false if max fee per gas is missing', async () => { + const result = await isNativeBalanceSufficientForGas( + { + ...TRANSACTION_META_MOCK, + txParams: { + ...TRANSACTION_META_MOCK.txParams, + maxFeePerGas: undefined, + gasPrice: undefined, + }, + }, + MESSENGER_MOCK, + NETWORK_CLIENT_ID_MOCK, + ); + + expect(result).toBe(false); + expect(rpcRequestMock).not.toHaveBeenCalled(); + }); }); }); diff --git a/packages/transaction-controller/src/utils/balance.ts b/packages/transaction-controller/src/utils/balance.ts index ae1a8cd10a1..3bb28be7c59 100644 --- a/packages/transaction-controller/src/utils/balance.ts +++ b/packages/transaction-controller/src/utils/balance.ts @@ -51,16 +51,22 @@ export async function isNativeBalanceSufficientForGas( messenger: TransactionControllerMessenger, networkClientId: NetworkClientId, ): Promise { - const from = transaction.txParams.from as Hex; + const { + txParams: { from, gas, maxFeePerGas, gasPrice }, + } = transaction; + const maxFeePerGasValue = maxFeePerGas ?? gasPrice; - const gasCostRawValue = new BigNumber( - transaction.txParams.gas ?? '0x0', - ).multipliedBy( - transaction.txParams.maxFeePerGas ?? transaction.txParams.gasPrice ?? '0x0', - ); + // Gas estimates can still be in flight (e.g. skipInitialGasEstimate). Treating + // missing fee fields as zero makes every balance look sufficient and clears a + // selected gas fee token before publish. + if (!gas || !maxFeePerGasValue) { + return false; + } + + const gasCostRawValue = new BigNumber(gas).multipliedBy(maxFeePerGasValue); const { balanceRaw } = await getNativeBalance( - from, + from as Hex, messenger, networkClientId, ); diff --git a/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts b/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts index 6cad84fc4ec..9b769acd2da 100644 --- a/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts +++ b/packages/transaction-controller/src/utils/gas-fee-tokens.test.ts @@ -411,10 +411,20 @@ describe('Gas Fee Tokens Utils', () => { request.transaction.isGasFeeTokenIgnoredIfBalance = true; request.transaction.selectedGasFeeToken = TOKEN_ADDRESS_1_MOCK; request.transaction.gasFeeTokens = []; + request.transaction.isExternalSign = true; + + jest.mocked(request.fetchGasFeeTokens).mockResolvedValueOnce([]); await expect(checkGasFeeTokenBeforePublish(request)).rejects.toThrow( 'Gas fee token not found and insufficient native balance', ); + + jest + .mocked(request.updateTransaction) + .mock.calls[0][1](request.transaction); + + expect(request.transaction.isExternalSign).toBe(false); + expect(request.transaction.gasFeeTokens).toStrictEqual([]); }); it('updates gas fee tokens', async () => { diff --git a/packages/transaction-controller/src/utils/gas-fee-tokens.ts b/packages/transaction-controller/src/utils/gas-fee-tokens.ts index c51dcb37ac0..849cf607098 100644 --- a/packages/transaction-controller/src/utils/gas-fee-tokens.ts +++ b/packages/transaction-controller/src/utils/gas-fee-tokens.ts @@ -187,6 +187,20 @@ export async function checkGasFeeTokenBeforePublish({ isExternalSign: true, }); + const isSelectedGasFeeTokenAvailable = gasFeeTokens?.some( + (token) => + token.tokenAddress.toLowerCase() === selectedGasFeeToken.toLowerCase(), + ); + + if (!isSelectedGasFeeTokenAvailable) { + updateTransaction(transaction.id, (tx) => { + tx.gasFeeTokens = gasFeeTokens; + tx.isExternalSign = false; + }); + + throw new Error('Gas fee token not found and insufficient native balance'); + } + updateTransaction(transaction.id, (tx) => { tx.gasFeeTokens = gasFeeTokens; tx.isExternalSign = true; @@ -194,16 +208,6 @@ export async function checkGasFeeTokenBeforePublish({ }); log('Updated gas fee tokens before publish', gasFeeTokens); - - if ( - !gasFeeTokens?.some( - (token) => - token.tokenAddress.toLowerCase() === selectedGasFeeToken.toLowerCase(), - ) - ) { - throw new Error('Gas fee token not found and insufficient native balance'); - } - log('Publishing with selected gas fee token', { selectedGasFeeToken }); }