fix: preserve transaction type of batched transactions #6056
+149
−5
Conversation
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
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
micaelae
commented
Jul 1, 2025
bfullam
previously approved these changes
Jul 1, 2025
4 tasks
|
@metamaskbot publish-preview |
|
Preview builds have been published. See these instructions for more information about preview builds. Expand for full list of packages and versions. |
matthewwalsh0
requested changes
Jul 2, 2025
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
vinistevam
approved these changes
Jul 11, 2025
matthewwalsh0
requested changes
Jul 14, 2025
| @@ -1722,6 +1722,9 @@ export type PublishBatchHookTransaction = { | |||
|
|
|||
| /** Signed transaction data to publish. */ | |||
| signedTx: Hex; | |||
|
|
|||
| /** Type of the nested transaction. */ | |||
| type?: TransactionType; | |||
There was a problem hiding this comment.
Why do we need the type here?
The intent of providing the id is so the hook can require any specific transaction metadata it needs via the transaction controller state and that reference.
| expect(publishBatchHook.mock.calls[0][0].transactions[0].type).toBe( | ||
| TransactionType.swap, | ||
| ); | ||
| expect(publishBatchHook.mock.calls[0][0].transactions[1].type).toBe( |
There was a problem hiding this comment.
Rather than asserting the hook is called with the type, ideally the hook wouldn't know, and instead we could verify that addTransaction is called multiple times with the correct types?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
When a batch of transactions with specific TransactionType values is submitted (example: bridgeApproval, swap, bridge), the txs get published with generic types like
contractInteractionandapprove, and the requested types are not preservedThe
typevalues are needed in the swaps and bridge experiences in order to track and display accurate transaction statuses so they need to be propagatedReferences
Changelog
Checklist