feat: add more bridge fields to txMeta for Bridge #4918

infiniteflower · 2024-11-12T20:45:07Z

Explanation

This PR adds a number of new fields to the TransactionMeta type to facilitate Bridge transactions.

We track overall Bridge status through a new BridgeStatusController in Extension. However, that assumes that transactions have a txHash on the source chain that can be used as a reference. For smart transactions (STX), a txHash is not returned immediately. Therefore, we need to attach some additional data to the txMeta so we can properly display information about the transaction even when there is no txHash.

Even if STX returns a txHash right away, it's gated behind a feature flag, which means it can be disabled at any time so we need to have a backup in place.

References

Related to MetaMask/metamask-extension#27740, MetaMask/metamask-extension#28460

Changelog

`@metamask/transaction-controller`

ADDED: bridgeSteps, destinationChainId to the TransactionMeta type
ADDED: In addTransaction(), update the txMeta object through the swaps.meta field when it is a Bridge tx, similar to what happens when it's a Swap tx
ADDED: TransactionControllerTransactionNewBridgeApprovalEvent, TransactionControllerTransactionNewBridgeEvent events

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've highlighted breaking changes using the "BREAKING" category above as appropriate
I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

## **Description** This PR implements the following: 1. Submit bridge transaction for normal transactions 3. Submit bridge transaction for native gas tokens that don't require approval 4. Submit bridge transaction for ERC20s that require approval Does not fully: 1. Submit bridge transaction for smart transactions - You can submit an STX, but the status screens don't make the most sense right now. - Improved STX support be handled by #28460 and MetaMask/core#4918 [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/27262?quickstart=1) ## **Related issues** - Targeting: #27522 ## **Manual testing steps** 1. Go to Bridge 2. Fill in source/dest token and amounts 3. Get a quote 4. Execute Bridge ## **Screenshots/Recordings**  ### **Before**  ### **After**  https://github.com/user-attachments/assets/b73f917d-e3e4-468b-b0fa-29f41f559488 ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

…ublish events

… tokens to read from later

matthewwalsh0 · 2024-11-25T16:35:49Z

packages/transaction-controller/src/TransactionController.ts

+/**
+ * Represents the `TransactionController:transactionNewBridgeApproval` event.
+ */
+export type TransactionControllerTransactionNewBridgeApprovalEvent = {


In an ideal world, the controller wouldn't be coupled to specific transaction types in the events since we already have the unapprovedTransactionAdded.

Could we instead just subscribe to that in the client and check the type property?

Yes we can, I was just referencing the similar Swap types here initially.

packages/transaction-controller/src/TransactionController.ts

matthewwalsh0 · 2024-11-25T16:40:49Z

packages/transaction-controller/src/types.ts

@@ -1354,3 +1364,37 @@ export type SubmitHistoryEntry = {
 export type InternalAccount = ReturnType<
  AccountsController['getSelectedAccount']
 >;
+enum BridgeActionTypes {


Minor, line break.

matthewwalsh0 · 2024-11-25T16:41:11Z

packages/transaction-controller/src/types.ts

+  protocol: BridgeProtocol;
+};
+
+export type BridgeAsset = {


JSDoc for types and all properties?

matthewwalsh0 · 2024-11-25T16:42:09Z

packages/transaction-controller/src/types.ts

+  /**
+   *
+   */
+  bridgeSteps?: BridgeStep[];


To clarify, do we definitely need to store this here? We don't have a bridge specific controller that will persist this state also and can link to a transaction ID?

infiniteflower · 2024-11-27T16:15:06Z

@matthewwalsh0 Thank you for the insightful comments. I've decided to close this PR in favor of #4988 which only adds destintationChainId.

Figured out a way to get around the STX issues in the client that does not require all these extraneous fields being added to the TransactionController as you noted. Managed to link the transaction to the Bridge history data through the txMeta.id.

## Explanation  This PR adds a new field to the `TransactionMeta` type to facilitate Bridge transactions. The primary purpose is to ensure that `destinationTokenAddress` refers to the correct token on the correct chain. ## References  A slimmed down version of #4918 Related to MetaMask/metamask-extension#28460 ## Changelog  ### `@metamask/transaction-controller` - **ADDED**: `destinationChainId` to the `TransactionMeta` type ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've highlighted breaking changes using the "BREAKING" category above as appropriate - [ ] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

## **Description**  This PR improves support for Smart Transactions (STX). It will not show the STX status page if the tx is a Bridge tx. Instead users will be immediately sent to the Activity list. If the user opens the Bridge tx detaills, they should be able to see the proper info. The main change is that in `BridgeStatusController` the `txHistory` is now keyed by `txMeta.id` rather than `txMeta.hash`. This is because for an STX we might not have the hash immediately, but we still want to look things up. We also add a way to call `addTransaction` from the UI side. Previously we used `addTransactionAndWaitForPublish`. However for STX, this will cause the app to wait until a `txHash` is available before returning a `txMeta`, which means we can't even use the `txMeta.id` to key a `BridgeHistoryItem`. If we use `addTransaction` this will allow us to receive a `txMeta` object back immediately and use `txMeta.id`. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28460?quickstart=1) ## **Related issues** Depends on MetaMask/core#4918 ## **Manual testing steps** 1. Add `BRIDGE_USE_DEV_APIS=1` to `.metamaskrc` 1. Go to Settings > Advanced, make sure Smart Transactions is on 2. Switch network to Ethereum 3. Go to Bridge 4. Selection source network, source token, dest network, dest token 5. Request a quote 6. Execute a bridge 7. Go to Activity 8. Click on the Bridge tx ## **Screenshots/Recordings**  ### **Before**  ### **After**  https://github.com/user-attachments/assets/364b93ec-3b54-4ce6-acd5-5001236c128c ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

infiniteflower changed the title ~~feat: add sourceChainId and destinatonChainId to txMeta~~ feat: add more bridge fields to txMeta Nov 14, 2024

This was referenced Nov 14, 2024

feat: cross chain swaps - tx submit MetaMask/metamask-extension#27262

Merged

feat: cross chain swap STX MetaMask/metamask-extension#28460

Merged

infiniteflower changed the title ~~feat: add more bridge fields to txMeta~~ feat: add more bridge fields to txMeta for Bridge Nov 14, 2024

infiniteflower mentioned this pull request Nov 21, 2024

feat: cross chain swaps - tx status MetaMask/metamask-extension#27740

Closed

7 tasks

infiniteflower force-pushed the feat/tx-controller-bridge-source-dest-chain-ids branch from 4adbdd1 to b81c944 Compare November 22, 2024 21:15

infiniteflower added 11 commits November 25, 2024 11:13

feat: add sourceChainId and destinatonChainId to txMeta

0e62557

chore: improve comments

b5e3828

chore: update bridge type transactions with the new txMeta data and p…

845bee0

…ublish events

chore: update gas fees request for bridge type txs

d6d4ebd

chore: add bridge steps to txMeta

cd92b59

chore: update txMeta obj if bridge tx with proper fields for src/dest…

34a2425

… tokens to read from later

fix: broken tests

2dc6c51

fix: broken tests

dcfef8e

chore: add tests for updateBridgeTypesTransaction

df0821e

fix: broken tests

dea5e3e

chore: remove only test statement

bd9efa2

infiniteflower force-pushed the feat/tx-controller-bridge-source-dest-chain-ids branch from 5079a84 to bd9efa2 Compare November 25, 2024 16:14

infiniteflower marked this pull request as ready for review November 25, 2024 16:20

infiniteflower requested a review from a team as a code owner November 25, 2024 16:20

matthewwalsh0 requested changes Nov 25, 2024

View reviewed changes

infiniteflower mentioned this pull request Nov 27, 2024

feat: add destinationChainId to support cross chain txs #4988

Merged

4 tasks

infiniteflower closed this Nov 27, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add more bridge fields to txMeta for Bridge #4918

feat: add more bridge fields to txMeta for Bridge #4918

infiniteflower commented Nov 12, 2024 •

edited

Loading

matthewwalsh0 Nov 25, 2024

infiniteflower Nov 25, 2024 •

edited

Loading

matthewwalsh0 Nov 25, 2024

matthewwalsh0 Nov 25, 2024

matthewwalsh0 Nov 25, 2024

infiniteflower commented Nov 27, 2024 •

edited

Loading

feat: add more bridge fields to txMeta for Bridge #4918

feat: add more bridge fields to txMeta for Bridge #4918

Conversation

infiniteflower commented Nov 12, 2024 • edited Loading

Explanation

References

Changelog

@metamask/transaction-controller

Checklist

matthewwalsh0 Nov 25, 2024

Choose a reason for hiding this comment

infiniteflower Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

matthewwalsh0 Nov 25, 2024

Choose a reason for hiding this comment

matthewwalsh0 Nov 25, 2024

Choose a reason for hiding this comment

matthewwalsh0 Nov 25, 2024

Choose a reason for hiding this comment

infiniteflower commented Nov 27, 2024 • edited Loading

infiniteflower commented Nov 12, 2024 •

edited

Loading

`@metamask/transaction-controller`

infiniteflower Nov 25, 2024 •

edited

Loading

infiniteflower commented Nov 27, 2024 •

edited

Loading