feat: calculate bridge quote metadata in @metamask/bridge-controller #5614
Conversation
This file contains 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
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Copilot reviewed 9 out of 12 changed files in this pull request and generated 2 comments.
Files not reviewed (3)
- packages/bridge-controller/package.json: Language not supported
- packages/bridge-controller/tsconfig.build.json: Language not supported
- packages/bridge-controller/tsconfig.json: Language not supported
There was a problem hiding this comment.
Copilot reviewed 9 out of 12 changed files in this pull request and generated 1 comment.
Files not reviewed (3)
- packages/bridge-controller/package.json: Language not supported
- packages/bridge-controller/tsconfig.build.json: Language not supported
- packages/bridge-controller/tsconfig.json: Language not supported
Comments suppressed due to low confidence (1)
packages/bridge-controller/src/bridge-controller.ts:254
- Merging multiple state objects via the spread operator can lead to key collisions; consider explicitly mapping or namespacing the keys from each controller to ensure state values are not inadvertently overwritten.
const exchangeRateSources = { ...this.messagingSystem.call('MultichainAssetsRatesController:getState'), ...this.messagingSystem.call('CurrencyRateController:getState'), ...this.messagingSystem.call('TokenRatesController:getState'), ...this.state };
ghgoodreau
previously approved these changes
Apr 9, 2025
amitabh94
reviewed
Apr 9, 2025
Co-authored-by: cryptodev-2s <[email protected]>
Co-authored-by: cryptodev-2s <[email protected]>
mcmire
reviewed
Apr 14, 2025
|
There appear to be some breaking changes in this PR. I noticed this item in the checklist was not checked:
Are there draft PRs that we've created on both extension and mobile, and if not, can we do that? That will help to ensure that we can upgrade to the new version quickly. |
|
@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. |
|
@mcmire here's the PR for upgrading the package in extension: MetaMask/metamask-extension#31752 the logic for consuming the new functionality I introduced here doesn't exist in mobile yet and they plan to implement it this week |
|
@micaelae Okay cool, thank you! |
mcmire
approved these changes
Apr 15, 2025
ejwessel
approved these changes
Apr 15, 2025
Merged
micaelae
added a commit
that referenced
this pull request
Apr 16, 2025
## Explanation <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> Releasing @metamask/bridge-controller 14.0.0 which includes cross-chain swaps quote metadata utilities added in #5614 ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Changelog <!-- THIS SECTION IS NO LONGER NEEDED. The process for updating changelogs has changed. Please consult the "Updating changelogs" section of the Contributing doc for more. --> ## Checklist [ X] 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 [X] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary [X] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
micaelae
added a commit
that referenced
this pull request
Apr 16, 2025
## Explanation Extension integration PR: MetaMask/metamask-extension#31907 This adds a `submitTx` method to the bridge-status-controller that submits a Solana tx to the Keyring/snap controller then starts polling for transaction status To use this in mobile/extension - replace the confirmation and status polling calls with `submitTx` - the new method needs to be bound - the controller init needs to be updated with snap permissions (see [extension example](https://github.com/MetaMask/metamask-extension/pull/31907/files#diff-6fbff2cfe97ac01b77296ef2122c7e0a5b3ff6a84b584b4d1a87482f35eea3d6)) - this change depends on type updates included in [bridge-controller](#5614) <!-- Thanks for your contribution! Take a moment to answer these questions so that reviewers have the information they need to properly understand your changes: * What is the current state of things and why does it need to change? * What is the solution your changes offer and how does it work? * Are there any changes whose purpose might not obvious to those unfamiliar with the domain? * If your primary goal was to update one package but you found you had to update another one along the way, why did you do so? * If you had to upgrade a dependency, why did you do so? --> ## References <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Changelog <!-- THIS SECTION IS NO LONGER NEEDED. The process for updating changelogs has changed. Please consult the "Updating changelogs" section of the Contributing doc for more. --> ## Checklist - [X] 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 - [X] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [X] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes --------- Co-authored-by: cryptodev-2s <[email protected]> Co-authored-by: infiniteflower <[email protected]>
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
Refer to extension integration PR for usage examples: MetaMask/metamask-extension#31752
Added
Changed
References
Checklist