Conversation
packages/transaction-pay-controller/src/strategy/relay/relay-max-gas-station.ts
Show resolved
Hide resolved
packages/transaction-pay-controller/src/strategy/relay/max-amount-with-gas-station-fallback.ts
Outdated
Show resolved
Hide resolved
packages/transaction-pay-controller/src/strategy/relay/max-amount-with-gas-station-fallback.ts
Outdated
Show resolved
Hide resolved
packages/transaction-pay-controller/src/strategy/relay/relay-max-gas-station.ts
Show resolved
Hide resolved
packages/transaction-pay-controller/src/strategy/relay/max-amount-with-gas-station-fallback.ts
Outdated
Show resolved
Hide resolved
packages/transaction-pay-controller/src/strategy/relay/max-amount-with-gas-station-fallback.ts
Outdated
Show resolved
Hide resolved
| if ( | ||
| !isAdjustedAmountAffordable( | ||
| adjustedSourceAmount, | ||
| validationGasEstimate.amount, |
There was a problem hiding this comment.
We don't care about the new amount do we, as we calculated the old one to be exactly what is remaining of the balance, so do we want to use that to avoid dust?
There was a problem hiding this comment.
I think we shouldn't blindly trust the new amount, but we also avoid forcing the original estimate when quote internals shift slightly. This keeps max safety and minimizes dust risk without introducing a new submit API in this PR.
There was a problem hiding this comment.
Makes sense as MVP I think, but avoiding dust wherever possible is a good goal, we can circle back later.
packages/transaction-pay-controller/src/strategy/relay/types.ts
Outdated
Show resolved
Hide resolved
packages/transaction-pay-controller/src/strategy/relay/relay-max-gas-station.ts
Show resolved
Hide resolved
|
|
||
| markQuoteAsTwoPhaseForMaxAmount(phase2Quote); | ||
|
|
||
| return phase2Quote; |
There was a problem hiding this comment.
Potentially a separate PR, but are the gas fee tokens consistent in your testing?
Or should we also update relay-submit to specify a new gasFeeTokenAmount property to the TransactionController to avoid any dust?
There was a problem hiding this comment.
Discussed in other comments, no new submit API for now for simplicity, at the cost of minor dust 👍
packages/transaction-pay-controller/src/strategy/relay/relay-quotes.ts
Outdated
Show resolved
Hide resolved
|
|
||
| function parseGasValue(value: string): BigNumber { | ||
| if (value.toLowerCase().startsWith('0x')) { | ||
| return new BigNumber(value.slice(2), 16); |
There was a problem hiding this comment.
Minor, BigNumber should handle 0x automatically, even without the base argument.
| if ( | ||
| !isAdjustedAmountAffordable( | ||
| adjustedSourceAmount, | ||
| validationGasEstimate.amount, |
There was a problem hiding this comment.
Makes sense as MVP I think, but avoiding dust wherever possible is a good goal, we can circle back later.
|
|
||
| markQuoteAsTwoPhaseForMaxAmount(phase2Quote); | ||
|
|
||
| return phase2Quote; |
There was a problem hiding this comment.
Discussed in other comments, no new submit API for now for simplicity, at the cost of minor dust 👍
| request, | ||
| }; | ||
|
|
||
| const { relayDisabledGasStationChains } = getFeatureFlags(messenger); |
There was a problem hiding this comment.
Minor, could we encapsulate this within getGasStationEligibility since we pass the messenger also?
There was a problem hiding this comment.
Good call, relocated in getGasStationEligibility
| request: QuoteRequest; | ||
| }; | ||
|
|
||
| export async function getRelayMaxGasStationQuote( |
There was a problem hiding this comment.
Minor, could we get a high level summary of the algorithm here in JSDoc?
| ); | ||
|
|
||
| if ( | ||
| !isAdjustedAmountAffordable( |
There was a problem hiding this comment.
Is this another benefit of the future add transaction API changes, since we can guarantee we can still afford the original value since we subtracted it from our balance?
But this new one may have gone up.
There was a problem hiding this comment.
Yes, that’s right, subtraction used an earlier estimate, and the phase2 gas cost can come back higher.
A future submit API change (explicit gasFeeTokenAmount) would help execution time determinism/dust, but it does not fully replace this quote-time affordability guard. We still need to validate that adjusted + latestGasCost <= originalBalance before accepting phase2.
| return primaryEstimate; | ||
| } | ||
|
|
||
| const probeCost = await getProbeGasCostInSourceTokenRaw(context); |
There was a problem hiding this comment.
How can the above fail?
If no gas fee tokens are returned from the original quote, then should we not just fallback at that point?
Why would it help to try get another quote with a smaller amount?
There was a problem hiding this comment.
From testing, at full max, fee-token estimation/simulation can return no usable gas-fee-token result (or no match) because the transaction is too tight at the balance boundary.
With a smaller probe amount, the same route often becomes estimable and returns either:
isSourceGasFeeToken directly, or a usable getGasFeeTokens result we can normalize.
So this is a recovery path, not a bypass. It lets us rescue cases that would otherwise unnecessarily fall back.
| return undefined; | ||
| } | ||
|
|
||
| if (probeQuote.fees.isSourceGasFeeToken) { |
There was a problem hiding this comment.
Is everything below a duplication of getGasCostFromQuoteOrGasStation?
There was a problem hiding this comment.
Good call, removed below by re-using getGasCostFromQuoteOrGasStation
| sourceTokenAddress, | ||
| }, | ||
| totalGasEstimate, | ||
| totalItemCount: Math.max(relayParams.length, gasLimits.length), |
There was a problem hiding this comment.
I can see this was part of the old code, but I can't see how we could have multiple gas limits with a single relay params.
There was a problem hiding this comment.
Isn't that comes from post-quote handling? If it's a post-quote with an original transaction, in non-7702 path prepends original transaction gas (gasLimits = [originalTxGas, ...gasLimits])
| }; | ||
| metamask: { | ||
| gasLimits: number[]; | ||
| isMaxGasStation?: boolean; |
There was a problem hiding this comment.
Is this purely for future use if we change the submission flow to use the exact gas fee token amount?
Should we omit now if not used?
There was a problem hiding this comment.
This is purely for metric purposes, so we can track if 2 phase quoting successfully initiated and quote replaced then we will flag metrics it on client.
Explanation
This PR introduces a dedicated max-amount gas-station fallback flow for Relay, extracted into a focused helper and wired into quote orchestration. The goal was fixing max-amount mUSD conversion failures caused by gas-fee-token estimation dead-ends but in fact it fixes for any kind of intent.
The helper (
getMaxAmountQuoteWithGasStationFallback) now implements a clear two-phase flow with explicit fallback points:maxGaslessEnabledisfalse.PROBE_AMOUNT_PERCENTAGE = 0.25) to discover gas fee token + amount.TransactionController:getGasFeeTokensandcalculateGasFeeTokenCostto normalize source-token gas fee amount.twoPhaseQuoteForMaxAmount = trueand return phase-2.References
gasless.max.mov
Checklist
Note
Medium Risk
Changes core Relay quote selection for
isMaxAmountrequests and introduces new gas-cost estimation/validation paths; while heavily tested, errors could affect max-amount pricing or fallback behavior in production.Overview
Adds a dedicated two-phase max-amount quoting path for the Relay strategy that can fall back to gas-station-derived gas costs (and optionally a smaller “probe” quote) to produce an adjusted max quote when the initial max quote can’t be priced safely in source-token gas units.
Refactors Relay quote gas-fee-token handling by extracting
getGasStationEligibility/getGasStationCostInSourceTokenRaw(including multi-item gas normalization and support for decimal simulation fields), wires the new max-amount flow intogetRelayQuotes, and tags successful phase-2 quotes viaoriginal.metamask.isMaxGasStation(type extended accordingly). Test coverage is expanded significantly, including post-quote normalization and ensuring max-gas-station submissions keep the existinggasFeeToken-only behavior.Written by Cursor Bugbot for commit 7a5d8d4. This will update automatically on new commits. Configure here.