fix: replace underpriced dapp-suggested and saved gas fees with suggested estimates - #9704
fix: replace underpriced dapp-suggested and saved gas fees with suggested estimates#9704cloudonshore wants to merge 9 commits into
Conversation
Known edge case: dapp max-spend calculationsIf a dapp computes a maximum spendable amount from its own fee suggestion (e.g. Worth noting: for the gate to fire, the base fee has already risen past the dapp's data — so the dapp's amount is stale too, not just its fee. There is no fee assignment that both fits the balance and can currently be included: the affordable (dapp) fee can't mine, and any mineable fee breaks the max-spend arithmetic. The replacement converts a silent, un-actionable failure (stuck pending, nonce occupied) into an explicit one, but it can still contradict the dapp's arithmetic in a user-visible way. Proposed refinement (follow-up): clamp the replacement to what the balance can bear, since the actual charge ( and only proceed if the result is still ≥ the low estimate. If it is, the transaction fits the balance and can mine — the max-spend flow works unchanged. If it isn't, the amount is genuinely unaffordable at any viable fee, and the replacement is skipped (fail open, current behavior). Cost is one balance read, only when the gate fires. The clamp should also be skipped for sponsored-gas / gas-fee-token transactions, where Suggested alongside: break down the eventual metrics by outcome ( |
| }, | ||
| // TODO: Replace `any` with type | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| } as any as GasFeeFlowResponse); |
There was a problem hiding this comment.
can we use unknown here or try to define this?
There was a problem hiding this comment.
Done in 7f37616 — replaced with as unknown as GasFeeFlowResponse and dropped the eslint suppression.
| const hasCompleteDappFees = | ||
| eip1559 && | ||
| Boolean(txMeta.txParams.maxFeePerGas) && | ||
| Boolean(txMeta.txParams.maxPriorityFeePerGas); | ||
|
|
||
| // Estimates are still required for transactions with complete dapp-suggested | ||
| // fees if they may be replaced due to being underpriced. | ||
| const isEstimateRequiredForDappFees = | ||
| hasCompleteDappFees && | ||
| !txMeta.isInternal && | ||
| getReplaceUnderpricedDappGasFeesEnabled(txMeta.chainId, messenger); | ||
|
|
||
| if ( | ||
| (!eip1559 && txMeta.txParams.gasPrice) || | ||
| (eip1559 && | ||
| txMeta.txParams.maxFeePerGas && | ||
| txMeta.txParams.maxPriorityFeePerGas) | ||
| (hasCompleteDappFees && !isEstimateRequiredForDappFees) |
There was a problem hiding this comment.
It would be good to preserve complete dapp-supplied fees when fee estimation and the fallback RPC both fail.
Enabling replaceUnderpricedDappGasFees makes transactions with complete fees enter the estimation path at the bottom of the file:
const gasPriceHex = (await rpcRequest({
messenger,
networkClientId,
method: 'eth_gasPrice',
})) as Hex | undefined;An estimation failure falls through to the unguarded eth_gasPrice request; if that request rejects, transaction creation now fails instead of retaining the original complete fees. Catch the fallback failure or return empty suggestions on this comparison-only path so the feature continues to fail open.
In summary:
- Flag disabled: no regression; the old early-return behavior remains.
- Flag enabled, estimate succeeds: intended new behavior.
- Flag enabled, estimate fails but eth_gasPrice succeeds: likely still fails open and retains the dapp fees.
- Flag enabled, both estimate and eth_gasPrice fail: new failure mode; transaction creation rejects where it previously retained the complete dapp fees.
There was a problem hiding this comment.
Good catch, fixed in 7f37616. The comparison-only path now returns empty suggestions after a flow failure instead of falling through to the unguarded eth_gasPrice request, so both-failing can no longer reject transaction creation and the dapp fees are retained.
pedronfigueiredo
left a comment
There was a problem hiding this comment.
looking good just two small nits
Explanation
Underpriced gas fees — a
maxFeePerGasbelow (or barely above) the current base fee — produce transactions that are very unlikely to be included before fee conditions change further. They typically end up stuck as pending until eventually marked as failed (e.g.Transaction not found on network after timeout), with no actionable feedback to the user. Telemetry shows this is a meaningful contributor to dropped/timed-out transactions.Two fee sources can produce such values, and nothing in the client validates either against current network conditions. This PR adds a feature-flag-gated staleness gate for both, at transaction creation time — the last point where fees can still be changed before signing.
1. Dapp-suggested fees (
replaceUnderpricedDappGasFeesflag)Currently:
updateGasFeesusesinitialParams.maxFeePerGasas-is.GasFeePolleronly refreshes fees for preset levels, sodappSuggestedfees stay frozen while the confirmation is open.A dapp that computes fees from stale data can therefore produce a signed transaction that cannot mine. With the flag enabled:
shouldIgnoreDappGasFeescheck fires when the dapp-suggestedmaxFeePerGas(or a legacygasPriceused asmaxFeePerGason EIP-1559 networks) is below the current low estimate. The low estimate is the floor rather than the raw base fee, since fees between the base fee and the low estimate have near-zero effective priority fee and no headroom for base fee movement, so they share the same failure mode.maxFeePerGasandmaxPriorityFeePerGas.userFeeLevelis set tomediuminstead ofdappSuggested, which also opts the transaction intoGasFeePollerrefresh while unapproved.dappSuggestedGasFeeson the transaction metadata, so UIs can still display the site suggestion and users can revert via the edit modal.Note: for transactions with complete dapp-suggested fees,
getSuggestedGasFeespreviously skipped fetching estimates entirely. When the flag is enabled, estimates are now fetched for these transactions so the comparison can run — one additional gas fee estimate call per dapp transaction on enabled chains.2. Saved (advanced) gas fee preferences (
replaceUnderpricedSavedGasFeesflag)A saved custom preference is a static
maxBaseFeecaptured at some point in the past; once the base fee rises past it, every transaction priced from it strands. Nothing else guards this source: the dapp gate deliberately excludessavedGasFees, andhasInitialGasFeeParamsonly protects transactions carrying explicit fee params — saved fees apply precisely when they don't. This is increasingly relevant as clients expand saved-preference usage (per-account custom gas settings persisted from the confirmation flow, and #9682 extending saved fees to wallet-initiated transfers).With the flag enabled:
maxBaseFeebelow the current low estimate, the saved preference is ignored for this transaction and the suggested (medium) values are used instead.low/medium/high) are never touched — they track current estimates by construction.userFeeLevelbecomesmediuminstead ofcustom, with the sameGasFeePollerrefresh benefit.Guards / fail-open behavior (both gates)
Fees are kept unchanged when: the respective feature flag is not enabled for the chain (both default to disabled; per-chain rollout via
perChainConfigwithdefaultfallback, mirroringtimeoutAttempts); the transaction is internal (dapp gate; swaps/bridges are priced by the aggregator/relay); the fee is at or above the low estimate; or estimates are unavailable / not fee-market type.The product decision (saved-fees gate)
Overriding a value the user saved is a stronger intervention than overriding a dapp's computed value. Three defensible behaviors when the saved-fees gate fires: (1) silently use suggested fees (this PR's behavior), (2) use suggested fees + surface an alert ("your saved gas setting is below current network fees"), (3) keep the saved value + warn only. The flag ships disabled, so this can be settled before enablement.
An argument for intervening (1 or 2) rather than deferring entirely to the saved value: saved preferences already do not grant full autonomy. The controller already ignores them for swaps and bridges (
SAVED_GAS_FEES_IGNORED_TRANSACTION_TYPES, #9401/#9682) — explicitly because a saved fee could underprice aggregator-priced transactions — and already yields them to explicit fee params (hasInitialGasFeeParams, #8993). The established principle is that saved preferences apply where they can work and yield where they would predictably break the transaction. A custommaxBaseFeebelow the current low estimate is the clearest instance of the latter — this extends existing precedent rather than setting new one.References
Checklist
Note
Medium Risk
Changes pre-sign gas selection for dapp and saved custom fees when flags are enabled; incorrect thresholds or rollout could surprise users or sites, though behavior is gated and fail-open when estimates are missing.
Overview
Adds feature-flag-gated gas fee validation at transaction creation so fees below the current low estimate are swapped for wallet medium suggestions instead of leaving txs stuck pending.
When
replaceUnderpricedDappGasFeesis on, external dapp EIP-1559 fees (or legacygasPriceused as max fee) below the low tier are ignored;userFeeLevelbecomesmediumsoGasFeePollercan refresh while unapproved. Original values stay ondappSuggestedGasFees.When
replaceUnderpricedSavedGasFeesis on, saved custommaxBaseFeepreferences below low are dropped (level-based saved fees are unchanged), with the same medium fallback anduserFeeLevelbehavior.getSuggestedGasFeesnow fetches estimates for complete dapp-suggested fee txs when the dapp flag is enabled, exposes the low tier on fee-market responses, and skips RPC fallback on that path so estimate failures do not block creation. Both flags default off with per-chainperChainConfig.Reviewed by Cursor Bugbot for commit 7f37616. Bugbot is set up for automated code reviews on this repo. Configure here.