diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index c61ad8d1ae..0b7d798d00 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- `estimateGasBatch` now falls back to the sum of per-tx `gas` values in the EIP-7702 path when node simulation fails, instead of returning the block-gas-limit fallback ([#8735](https://github.com/MetaMask/core/pull/8735)) + - Real 7702 simulation is still preferred when it succeeds (the bundled call has no per-tx intrinsic gas cost so the estimate is typically tighter than summing per-tx limits). + - Required for callers that submit batches whose individual sub-calls cannot be simulated standalone, for example predict-withdraw, where the batch's first sub-call (`Safe.execTransaction`) provides source-token balance to subsequent sub-calls (approve + swap), and simulating the relay-only sub-calls in isolation reverts because the EOA has no balance until the Safe sub-call has run. + ## [65.3.0] ### Changed diff --git a/packages/transaction-controller/src/utils/gas.test.ts b/packages/transaction-controller/src/utils/gas.test.ts index 89af467dfd..b17f6e07df 100644 --- a/packages/transaction-controller/src/utils/gas.test.ts +++ b/packages/transaction-controller/src/utils/gas.test.ts @@ -23,6 +23,7 @@ import { addGasBuffer, estimateGas, estimateGasBatch, + getProvidedBatchGasLimits, updateGas, FIXED_GAS, DEFAULT_GAS_MULTIPLIER, @@ -1325,6 +1326,116 @@ describe('gas', () => { }); }); + it('prefers 7702 simulated gas over provided gas when simulation succeeds', async () => { + // The bundled 7702 call has no per-tx intrinsic gas cost so the + // simulated estimate is typically lower than the sum of provided per-tx + // limits — prefer it when available. + const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ + { + chainId: CHAIN_ID_MOCK, + isSupported: true, + upgradeContractAddress: UPGRADE_CONTRACT_ADDRESS_MOCK, + }, + ]); + + generateEIP7702BatchTransactionMock.mockReturnValue({ + to: TO_MOCK, + data: DATA_MOCK, + } as BatchTransactionParams); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasResponse: toHex(GAS_MOCK), + }); + + const result = await estimateGasBatch({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + from: FROM_MOCK, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + isAtomicBatchSupported: isAtomicBatchSupportedMock, + messenger: MESSENGER_MOCK, + transactions: BATCH_TX_PARAMS_WITH_GAS_MOCK, + }); + + expect(result).toStrictEqual({ + totalGasLimit: GAS_MOCK, + gasLimits: [GAS_MOCK], + }); + }); + + it('falls back to provided gas in 7702 path when simulation fails', async () => { + // Callers that submit batches whose individual sub-calls cannot be + // simulated standalone (e.g. predict-withdraw, where the batch's first + // sub-call provides token balance to the rest) rely on this fallback — + // otherwise simulation reverts and falls back to the block gas limit. + const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ + { + chainId: CHAIN_ID_MOCK, + isSupported: true, + upgradeContractAddress: UPGRADE_CONTRACT_ADDRESS_MOCK, + }, + ]); + + generateEIP7702BatchTransactionMock.mockReturnValue({ + to: TO_MOCK, + data: DATA_MOCK, + } as BatchTransactionParams); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: new Error('execution reverted'), + }); + + const result = await estimateGasBatch({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + from: FROM_MOCK, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + isAtomicBatchSupported: isAtomicBatchSupportedMock, + messenger: MESSENGER_MOCK, + transactions: BATCH_TX_PARAMS_WITH_GAS_MOCK, + }); + + expect(result).toStrictEqual({ + totalGasLimit: 521000, + gasLimits: [521000], + }); + }); + + it('preserves requiresAuthorizationList when 7702 fallback fires for upgrade-required account', async () => { + const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ + { + chainId: CHAIN_ID_MOCK, + isSupported: false, + upgradeContractAddress: UPGRADE_CONTRACT_ADDRESS_MOCK, + }, + ]); + + generateEIP7702BatchTransactionMock.mockReturnValue({ + to: TO_MOCK, + data: DATA_MOCK, + } as BatchTransactionParams); + + mockQuery({ + getBlockByNumberResponse: { gasLimit: toHex(BLOCK_GAS_LIMIT_MOCK) }, + estimateGasError: new Error('execution reverted'), + }); + + const result = await estimateGasBatch({ + networkClientId: NETWORK_CLIENT_ID_MOCK, + from: FROM_MOCK, + getSimulationConfig: GET_SIMULATION_CONFIG_MOCK, + isAtomicBatchSupported: isAtomicBatchSupportedMock, + messenger: MESSENGER_MOCK, + transactions: BATCH_TX_PARAMS_WITH_GAS_MOCK, + }); + + expect(result).toStrictEqual({ + totalGasLimit: 521000, + gasLimits: [521000], + requiresAuthorizationList: true, + }); + }); + it('throws error when upgrade contract address not found', async () => { const isAtomicBatchSupportedMock = jest.fn().mockResolvedValue([ { @@ -1490,6 +1601,48 @@ describe('gas', () => { }); }); + describe('getProvidedBatchGasLimits', () => { + it('returns parsed limits + sum when every transaction has a gas value', () => { + expect( + getProvidedBatchGasLimits(BATCH_TX_PARAMS_WITH_GAS_MOCK), + ).toStrictEqual({ + gasLimits: [21000, 500000], + totalGasLimit: 521000, + }); + }); + + it('returns undefined when none of the transactions have gas', () => { + expect(getProvidedBatchGasLimits(BATCH_TX_PARAMS_MOCK)).toBeUndefined(); + }); + + it('returns undefined when only some transactions have gas', () => { + const mixed = [BATCH_TX_PARAMS_WITH_GAS_MOCK[0], BATCH_TX_PARAMS_MOCK[0]]; + expect(getProvidedBatchGasLimits(mixed)).toBeUndefined(); + }); + + it('parses hex gas values correctly', () => { + const txWithHexGas = [ + { ...BATCH_TX_PARAMS_MOCK[0], gas: '0x5208' as Hex }, + { ...BATCH_TX_PARAMS_MOCK[1], gas: '0x7a120' as Hex }, + ]; + expect(getProvidedBatchGasLimits(txWithHexGas)).toStrictEqual({ + gasLimits: [21000, 500000], + totalGasLimit: 521000, + }); + }); + + it('returns zero-length result for an empty batch', () => { + // `every` on empty array returns true, so the function returns a valid + // (but empty) result rather than `undefined`. Callers of `estimateGasBatch` + // always pass at least one transaction, so this is documenting current + // behaviour rather than a guarantee. + expect(getProvidedBatchGasLimits([])).toStrictEqual({ + gasLimits: [], + totalGasLimit: 0, + }); + }); + }); + describe('simulateGasBatch', () => { it('returns the total gas limit as a hex string', async () => { simulateTransactionsMock.mockResolvedValueOnce( diff --git a/packages/transaction-controller/src/utils/gas.ts b/packages/transaction-controller/src/utils/gas.ts index 4bf791ccc0..9cf9e7bbf0 100644 --- a/packages/transaction-controller/src/utils/gas.ts +++ b/packages/transaction-controller/src/utils/gas.ts @@ -208,6 +208,39 @@ export async function estimateGas({ }; } +/** + * Sum caller-provided gas limits across a batch. + * + * If every transaction in the batch already has a `gas` value, returns the + * parsed per-tx limits and their sum. Otherwise returns `undefined`. + * + * Used by `estimateGasBatch`: + * - non-7702 path: short-circuits simulation entirely when present. + * - EIP-7702 path: used as a fallback when simulation fails — required for + * callers that submit batches whose individual sub-calls cannot be simulated + * standalone (e.g. predict-withdraw, where the batch's first sub-call + * provides source-token balance to subsequent sub-calls). When 7702 + * simulation succeeds it is preferred since the bundled call has no per-tx + * intrinsic gas cost and produces a tighter estimate. + * + * @param transactions - Batch transactions to inspect. + * @returns Parsed gas limits and total when every transaction has gas; otherwise `undefined`. + */ +export function getProvidedBatchGasLimits( + transactions: BatchTransactionParams[], +): { gasLimits: number[]; totalGasLimit: number } | undefined { + if (!transactions.every((transaction) => transaction.gas !== undefined)) { + return undefined; + } + + const gasLimits = transactions.map((transaction) => + new BigNumber(transaction.gas as Hex).toNumber(), + ); + const totalGasLimit = gasLimits.reduce((acc, gasLimit) => acc + gasLimit, 0); + + return { gasLimits, totalGasLimit }; +} + export async function estimateGasBatch({ from, getSimulationConfig, @@ -245,6 +278,8 @@ export async function estimateGasBatch({ } if (chainResult) { + const providedBatchGasLimits = getProvidedBatchGasLimits(transactions); + const authorizationList = isUpgradeRequired ? [{ address: chainResult.upgradeContractAddress as Hex }] : undefined; @@ -260,7 +295,12 @@ export async function estimateGasBatch({ type, }; - const { estimatedGas: gasLimitHex } = await estimateGas({ + // Prefer real EIP-7702 simulation when it succeeds — the bundled call has + // no per-tx intrinsic gas cost so the estimate is typically lower than + // summing per-tx provided limits. Fall back to the provided sum when the + // node-level simulation fails (e.g. predict-withdraw, where the batch's + // first sub-call provides source-token balance to subsequent sub-calls). + const { estimatedGas: gasLimitHex, simulationFails } = await estimateGas({ isSimulationEnabled: true, getSimulationConfig, messenger, @@ -268,6 +308,19 @@ export async function estimateGasBatch({ txParams: params, }); + if (simulationFails && providedBatchGasLimits) { + log( + 'EIP-7702 estimation failed, using batch parameter gas limits', + providedBatchGasLimits, + simulationFails, + ); + return { + gasLimits: [providedBatchGasLimits.totalGasLimit], + ...(isUpgradeRequired ? { requiresAuthorizationList: true } : {}), + totalGasLimit: providedBatchGasLimits.totalGasLimit, + }; + } + const totalGasLimit = new BigNumber(gasLimitHex).toNumber(); log('Estimated EIP-7702 gas limit', totalGasLimit); @@ -279,20 +332,10 @@ export async function estimateGasBatch({ }; } - const allTransactionsHaveGas = transactions.every( - (transaction) => transaction.gas !== undefined, - ); - - if (allTransactionsHaveGas) { - const gasLimits = transactions.map((transaction) => - new BigNumber(transaction.gas as Hex).toNumber(), - ); - - const total = gasLimits.reduce((acc, gasLimit) => acc + gasLimit, 0); - - log('Using batch parameter gas limits', { gasLimits, total }); - - return { totalGasLimit: total, gasLimits }; + const providedBatchGasLimits = getProvidedBatchGasLimits(transactions); + if (providedBatchGasLimits) { + log('Using batch parameter gas limits', providedBatchGasLimits); + return providedBatchGasLimits; } const { gasLimits: gasLimitsHex } = await simulateGasBatch({ diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index db1b2799c5..e3c7eb4efd 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -7,6 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `POLYGON_PUSD_ADDRESS` constant and treat Polymarket pUSD as a Polygon stablecoin in display/fiat-rate logic ([#8735](https://github.com/MetaMask/core/pull/8735)) + +### Fixed + +- Predict same-chain withdraw quote no longer falls back to block-gas-limit (~30M+) on swap-only Relay routes ([#8735](https://github.com/MetaMask/core/pull/8735)) + - `fromOverride = Safe proxy` is now gated on the route having a `deposit` step. Same-chain destinations route through DEX swap aggregators that reject contract callers (anti-MEV `msg.sender == tx.origin` checks etc.) — for those, the relay params' EOA `from` is used so simulation succeeds. + - Gas-fee-token lookup still uses the Safe proxy for ALL Predict withdraws (gated on `isPredictWithdraw && refundTo`), preserving the gasless flow for users who hold pUSD in the Safe but no native POL on the EOA. + ## [22.2.0] ### Changed diff --git a/packages/transaction-pay-controller/src/constants.test.ts b/packages/transaction-pay-controller/src/constants.test.ts new file mode 100644 index 0000000000..5bb589f50c --- /dev/null +++ b/packages/transaction-pay-controller/src/constants.test.ts @@ -0,0 +1,27 @@ +import { + CHAIN_ID_POLYGON, + POLYGON_PUSD_ADDRESS, + POLYGON_USDCE_ADDRESS, + STABLECOINS, +} from './constants'; + +describe('STABLECOINS', () => { + it('includes both Polygon USDC.e and Polymarket pUSD as Polygon stablecoins', () => { + // pUSD is treated as a USD-pegged stablecoin so post-quote display logic + // uses currencyOut.amountFormatted (1:1 USD) instead of going through + // the USD-rate API. Without pUSD in this list, predict-withdraw quote + // displays would round-trip through fiat conversion needlessly. + const polygonStablecoins = STABLECOINS[CHAIN_ID_POLYGON]; + + expect(polygonStablecoins).toContain(POLYGON_USDCE_ADDRESS.toLowerCase()); + expect(polygonStablecoins).toContain(POLYGON_PUSD_ADDRESS.toLowerCase()); + }); + + it('lower-cases all stablecoin entries for case-insensitive lookup', () => { + for (const [, addresses] of Object.entries(STABLECOINS)) { + for (const address of addresses) { + expect(address).toBe(address.toLowerCase()); + } + } + }); +}); diff --git a/packages/transaction-pay-controller/src/constants.ts b/packages/transaction-pay-controller/src/constants.ts index c740bdbd6d..52e39b0efb 100644 --- a/packages/transaction-pay-controller/src/constants.ts +++ b/packages/transaction-pay-controller/src/constants.ts @@ -15,6 +15,9 @@ export const ARBITRUM_USDC_ADDRESS = export const POLYGON_USDCE_ADDRESS = '0x2791Bca1f2de4661ED88A30C99A7a9449Aa84174' as Hex; +export const POLYGON_PUSD_ADDRESS = + '0xC011a7E12a19f7B1f670d46F03B03f3342E82DFB' as Hex; + export const HYPERCORE_USDC_ADDRESS = '0x00000000000000000000000000000000'; export const HYPERCORE_USDC_DECIMALS = 8; @@ -38,7 +41,10 @@ export const STABLECOINS: Record = { '0x176211869ca2b568f2a7d4ee941e073a821ee1ff', // USDC '0xa219439258ca9da29e9cc4ce5596924745e12b93', // USDT ], - [CHAIN_ID_POLYGON]: [POLYGON_USDCE_ADDRESS.toLowerCase() as Hex], + [CHAIN_ID_POLYGON]: [ + POLYGON_USDCE_ADDRESS.toLowerCase() as Hex, + POLYGON_PUSD_ADDRESS.toLowerCase() as Hex, + ], [CHAIN_ID_HYPERCORE]: [HYPERCORE_USDC_ADDRESS], // USDC }; diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts index f5d7c72a2f..337b829e1d 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.test.ts @@ -83,7 +83,7 @@ const QUOTE_REQUEST_MOCK: QuoteRequest = { }; const STEP_MOCK: RelayTransactionStep = { - id: 'swap', + id: 'deposit', requestId: '0x1', kind: 'transaction', items: [ @@ -1538,6 +1538,93 @@ describe('Relay Quotes Utils', () => { ); }); + it('uses original (EOA) from for predictWithdraw post-quote when route has no deposit step', async () => { + // Same-chain swap routes (e.g. Polygon pUSD -> Polygon USDC) only emit + // `approve` + `swap` steps. Simulating from the Safe proxy reverts in + // the swap step because DEX aggregators reject contract callers, so the + // override must be skipped and the relay params' EOA `from` used instead. + const quoteMock = cloneDeep(QUOTE_MOCK); + quoteMock.steps = [ + { ...STEP_MOCK, id: 'approve' }, + { ...STEP_MOCK, id: 'swap' }, + ]; + + successfulFetchMock.mockResolvedValue({ + ok: true, + json: async () => quoteMock, + } as never); + + estimateGasBatchMock.mockResolvedValue({ + totalGasLimit: 100000, + gasLimits: [50000, 50000], + }); + + const proxyAddress = '0xproxyAddress1234567890123456789012345678' as Hex; + + await getRelayQuotes({ + messenger, + requests: [ + { + ...QUOTE_REQUEST_MOCK, + targetAmountMinimum: '0', + isPostQuote: true, + refundTo: proxyAddress, + }, + ], + transaction: PREDICT_WITHDRAW_TRANSACTION_MOCK, + }); + + expect(estimateGasBatchMock).toHaveBeenCalledWith( + expect.objectContaining({ + from: FROM_MOCK, + }), + ); + }); + + it('still uses Safe proxy for gas-fee-token lookup on swap-only predictWithdraw routes', async () => { + // The gas-fee-token lookup must always use the Safe proxy for Predict + // withdraws (because the source token lives in the Safe, not the EOA), + // even when the gas-estimation path falls back to the EOA `from`. + const quoteMock = cloneDeep(QUOTE_MOCK); + quoteMock.steps = [ + { ...STEP_MOCK, id: 'approve' }, + { ...STEP_MOCK, id: 'swap' }, + ]; + + successfulFetchMock.mockResolvedValue({ + ok: true, + json: async () => quoteMock, + } as never); + + estimateGasBatchMock.mockResolvedValue({ + totalGasLimit: 100000, + gasLimits: [50000, 50000], + }); + + getTokenBalanceMock.mockReturnValue('0'); + getGasFeeTokensMock.mockResolvedValue([GAS_FEE_TOKEN_MOCK]); + + const proxyAddress = '0xproxyAddress1234567890123456789012345678' as Hex; + + const result = await getRelayQuotes({ + messenger, + requests: [ + { + ...QUOTE_REQUEST_MOCK, + targetAmountMinimum: '0', + isPostQuote: true, + refundTo: proxyAddress, + }, + ], + transaction: PREDICT_WITHDRAW_TRANSACTION_MOCK, + }); + + expect(getGasFeeTokensMock).toHaveBeenCalledWith( + expect.objectContaining({ from: proxyAddress }), + ); + expect(result[0].fees.isSourceGasFeeToken).toBe(true); + }); + it('sets isSourceGasFeeToken for predictWithdraw post-quote when insufficient native balance', async () => { successfulFetchMock.mockResolvedValue({ ok: true, diff --git a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts index 14b95c089d..7d639eea2d 100644 --- a/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts +++ b/packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts @@ -669,7 +669,17 @@ async function calculateSourceNetworkCost( const isPredictWithdraw = request.isPostQuote && isPredictWithdrawTransaction(transaction); - const fromOverride = isPredictWithdraw ? request.refundTo : undefined; + // `fromOverride = Safe proxy` is only valid for deposit-style Relay routes + // where the deposit contract reads the user's source-token balance directly. + // Same-chain destinations route through DEX swap aggregators that frequently + // reject contract callers (anti-MEV `msg.sender == tx.origin` checks, + // ERC777-style callback interfaces, native wrap/unwrap requiring caller + // native balance). Simulating those from the Safe proxy reverts and breaks + // gas estimation. For swap-only routes, fall back to the relay params' + // EOA `from` so simulation succeeds. + const hasDepositStep = quote.steps.some((step) => step.id === 'deposit'); + const useFromOverride = isPredictWithdraw && hasDepositStep; + const fromOverride = useFromOverride ? request.refundTo : undefined; const relayOnlyGas = await calculateSourceNetworkGasLimit( relayParams, @@ -745,6 +755,13 @@ async function calculateSourceNetworkCost( max: max.raw, }); + // Gas-fee-token lookup must use the Safe proxy for ALL Predict withdraws, + // not only deposit-style routes. The user's source token (pUSD) lives in + // the Safe; the EOA is empty until the Safe.execTransaction sub-call runs + // mid-batch. Querying the EOA for gas-fee-token availability would always + // return nothing and force users to hold POL. + // (`useFromOverride` only governs the gas-estimation `from` address, where + // swap-style routes need EOA because DEX routers reject contract callers.) if (isPredictWithdraw && request.refundTo) { log('Using proxy address for predict withdraw gas station simulation', { proxyAddress: request.refundTo,