feat: inject new parameters in addTransaction for gas fee tokens (Tempo)#8052
Open
feat: inject new parameters in addTransaction for gas fee tokens (Tempo)#8052
Conversation
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
from
1764dfd to
1b21bb7
Compare
7 tasks
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
from
1b21bb7 to
c64b46d
Compare
maxime-oe
changed the title
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
3 times, most recently
from
692c786 to
15dd7d6
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
7 tasks
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
from
15dd7d6 to
87d1261
Compare
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
2 times, most recently
from
a4a16c8 to
222195b
Compare
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
from
222195b to
3047a7f
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
from
3047a7f to
3ce0062
Compare
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
from
0807fc6 to
4f74a56
Compare
Contributor
Author
|
@metamaskbot publish-preview |
maxime-oe
changed the title
maxime-oe
changed the title
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
from
4f74a56 to
3df0deb
Compare
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
from
3df0deb to
624f1df
Compare
matthewwalsh0
requested changes
Mar 30, 2026
| batchId?: Hex; | ||
|
|
||
| /** Added for tx to appear in Activity tab */ | ||
| actionId?: string; |
Member
There was a problem hiding this comment.
Can you elaborate? Your transactions should be visible regardless of actionId?
Contributor
Author
There was a problem hiding this comment.
In this specific case and based on my understanding, only passing the actionId to this function from the client led to the transaction history state not to contain actionId, causing it to never appear. Adding it as explicit argument here made it appear in the state, which was enough for having it appear in the tx history. Since it's been a month, I can try locally to remove it and send you what the state looks like without this change, because maybe not having an actionId was a expected behavior ?
Member
There was a problem hiding this comment.
Please do, this was originally intended for avoiding duplication but is now deprecated so shouldn't be needed at all, otherwise there is a bug.
Contributor
Author
There was a problem hiding this comment.
Ok it is a bug then, because I just had pushed another commit where I "force-injected" the actionId somewhere else because otherwise I would definitely not show up in the Activity tab.
For reference the part in the code that filters the tx if there is no actionId (only filters if not actionId AND origin is not MetaMask):
Member
There was a problem hiding this comment.
That is a bug, they're using actionId to identify dApp requests when we should just be checking that origin is not empty and not ORIGIN_METAMASK.
Contributor
Author
There was a problem hiding this comment.
I see. Then this return should probably have the origin clause negated.
Such as
return isPending || !isLocalOrigin
Contributor
Author
There was a problem hiding this comment.
Actually !isLocalOrigin doesn't really make sense there because if it is a Send transaction there is no reason to display it.
Only alternative is to return true there regardless of actionId and batchId, since those conditions might have been there for historical reasons and may not have any reason to be there today.
I don't think the solution would impact this PR anymore, though (only MetaMask/metamask-extension#40449)
maxime-oe
commented
Mar 30, 2026
| /** Function to validate if auxiliary funds capability is supported. */ | ||
| isAuxiliaryFundsSupported: (chainId: Hex) => boolean; | ||
| /** Created for Tempo for ability to signal preferred fee token and absence of native */ | ||
| getChainSpecificExtraParams: (chainId: Hex) => { |
Member
There was a problem hiding this comment.
Minor, could we define this return value as a type, and make it more generic so the client can inject more in future?
Maybe just an optional getOverrideParams and pass it a ParamsOverrideRequest that returns a ParamsOverrideResponse etc?
Member
There was a problem hiding this comment.
Or actually, are we overcomplicating, could we just inject in the hooks themselves inside MetaMaskController and check the networkClientId and then mutate the call?
Contributor
Author
There was a problem hiding this comment.
I am not sure to fully get your second comment.
Right now in Extension for example we set the hooks to add Tempo parameters here.
Since this area was a "function binding" area and not a "business logic" area I wanted to follow how things were done - specifically isAuxiliaryFundsSupported appeared similar in intent.
If we would to directly add the arguments there, wouldn't we have to add gasFeeToken and excludeNativeTokenForFee in even more places ? (top level of the middleware)
Contributor
Author
There was a problem hiding this comment.
Is your suggestion that instead of passing getChainSpecificExtraParams we directly pass gasFeeToken and excludeNativeTokenForFee ? I am open to it. It just would make it a bit less generic in my opinion.
Member
There was a problem hiding this comment.
Since it's a client decision, I'm saying do we need to add another hook, or can we just update the existing addTransactionBatch and addTransaction hooks to do more than just bind?
Contributor
Author
There was a problem hiding this comment.
I suppose we could, indeed.
- Pros: No need to modify
eip-5792-middlewareat all. - Cons: Tiny performance overhead - to get the
chainIdfromnetworkClientIdfrom one extra place.
Just to confirm that this is what you are thinking about:
addTransaction: (txParams, txOptions) => {
// ... obtain tempo options here if any ...
return this.txController.addTransaction(txParams, {
...txOptions,
...tempoOptions, // assuming get got them above
});
},
Contributor
Author
There was a problem hiding this comment.
Made the change in Extension in accordance to what you said: https://github.com/MetaMask/metamask-extension/pull/40449/changes#diff-6fbff2cfe97ac01b77296ef2122c7e0a5b3ff6a84b584b4d1a87482f35eea3d6R1019-R1038
This removes the need for the eip-5792-middleware upgrade in Extension.
I will do the same thing for Mobile to check before I permanently remove it from this PR.
Contributor
Author
There was a problem hiding this comment.
@matthewwalsh0 This is what it will look like on Mobile: https://github.com/MetaMask/metamask-mobile/pull/27142/changes#diff-e39cb6f200b31535fa6c6cebdc19d7721a389d27c1749ce705d527c84117349cR877-R901
(although I haven't managed to actually test it on Mobile yet, maybe my device is too slow)
maxime-oe
force-pushed
the
feat/poc-tempo-transactions-7702
branch
from
624f1df to
27fa5d8
Compare
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
matthewwalsh0
requested changes
Mar 31, 2026
| * Unless true, gasFeeToken is only taken as a suggestion and native balance will be used in batch 7702 transactions | ||
| * This was first implemented for Tempo, since Tempo doesn't have the notion of native token at all | ||
| */ | ||
| excludeNativeTokenForFee?: boolean; |
Member
There was a problem hiding this comment.
Minor, do we need to persist this? Could it just be a add transaction option?
Contributor
Author
There was a problem hiding this comment.
Right now the Extension and Mobile implementations rely on this excludeNativeTokenForFee to indicate that there is no native token selectable when picking the gas token.
This allowed it to remain generic instead of having "if Tempo, then hide native" for the gas confirmation modal.
Contributor
Author
There was a problem hiding this comment.
Without this, USD would appear as selectable token.
Alternative to this approach is to "pull" Tempo config in those UI components, but to me since we already rely on transactionMeta in those, I preferred this current approach.
| batchId?: Hex; | ||
|
|
||
| /** Added for tx to appear in Activity tab */ | ||
| actionId?: string; |
Member
There was a problem hiding this comment.
Please do, this was originally intended for avoiding duplication but is now deprecated so shouldn't be needed at all, otherwise there is a bug.
Contributor
Author
|
@metamaskbot publish-preview |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Contributor
Author
|
@matthewwalsh0 testing with preview build, it looks like things work as expected for Extension. Is this PR good to go ? |
Contributor
Author
|
Setting to "merge when ready" in case it gets approved and conflicts arise so I don't have to ask re-approve |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
(Context and Approach Motivation)
Context:
With upcoming phase 1 for Tempo support, the initial approach will leverage EIP-7702 mechanics and UX.
One difference vs. other chains is that Tempo doesn't have a concept of native token.
--> This means that some current logic when we "fall back" to the native token for gas fee when user has enough native balance won't work.
excludeNativeTokenForFeeis a new parameter for signaling that native token should not be an option when set totrueand whengasFeeTokenis provided.Since the client dApps will trigger a call
addTransactionBatchfor some Tempo transactions - was only called internally until now - we add theactionIdso such transactions appear on the Activity tab (transaction history). Otherwise such transactions still made it to the state but those withoutactionIddon't appear in the activity tab.Approach Motivation
TransactionController - why a new
excludeNativeTokenForFeeargument:This controller already supports a
gasFeeTokenargument that does almost exactly what we want (gasless flow behavior). However there was still behavior where:gasFeeTokenin the TransactionController -->excludeNativeTokenForFeeallows to say "we do NOT want this behavior".excludeNativeTokenForFeeis also included intransactionMeta, this allows UI to understand that the native token shouldn't be displayed on selected when this argument is present.Overall, this argument is:
eip-5792-middleware - why a new
getChainSpecificExtraParamshook ?The above outlines the intent of having a client-controlled approach. However when using some dApps with Tempo some are calling
wallet_sendCallsdirectly to send batch transactions (those are regular EVM batch transactions, not Tempo Transactions), triggering theeip-5792-middlewareflow, which is not aware ofgasFeeTokennorexcludeNativeTokenForFeeparameters. Instead of adding Tempo-related confirm in thiseip-5792-middleware, the approach here is to have a genericgetChainSpecificExtraParamsthat will be called back by the middleware so the client can control what is then passed to the Transactions controller. In this specific case, when it is a Tempo chain, the client will inject bothgasFeeTokenandexcludeNativeTokenForFeeas it does when calling the Transactions Controller directly. This approach is purely to respect the "client is in control" philosophy.References
Checklist
Note
Medium Risk
Changes transaction metadata/options and alters gas-fee-token selection behavior in
addTransaction/batch 7702 flows, which can affect fee payment logic across supported networks. Risk is mitigated by being opt-in via a new optional flag and covered by added unit tests.Overview
Adds optional
excludeNativeTokenForFeeplumbing acrossaddTransaction/batch transaction request types and persists it onTransactionMetawhen explicitly provided.Updates
TransactionController.addTransactionsoisGasFeeTokenIgnoredIfBalanceis no longer automaticallytruewhengasFeeTokenis set; it becomesfalsewhenexcludeNativeTokenForFeeis passed, ensuring the gas fee token is used regardless of native balance.Extends the EIP-7702 batch path (
utils/batch.ts) to forwardexcludeNativeTokenForFeeintoaddTransaction, with new tests covering default omission and explicit inclusion, plus controller tests validating the new gas-fee-token behavior.Written by Cursor Bugbot for commit 53b6320. This will update automatically on new commits. Configure here.