fix: coinbase multichain not working #4068
Merged
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
🦋 Changeset detectedLatest commit: f0c1b62 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
12 Skipped Deployments
|
|
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
magiziz
commented
Mar 24, 2025
Comment on lines
+250
to
+254
| ChainController.state.chains.set('eip155', { | ||
| accountState: { | ||
| caipAddress: 'eip155:1' | ||
| } | ||
| } as unknown as ChainAdapter) |
There was a problem hiding this comment.
I noticed in most of our controllers tests we don't use vi.spyOn so i went with the current approach that we're doing which is setting state manually.
magiziz
commented
Mar 24, 2025
Comment on lines
+661
to
663
| if (caipAddress && adapter.connectionControllerClient?.disconnect) { | ||
| await adapter.connectionControllerClient.disconnect(ns) | ||
| } |
There was a problem hiding this comment.
We need to check if a user is connected before calling disconnect for every adapter
There was a problem hiding this comment.
shouldn't this be done at blueprint level then?
tomiir
approved these changes
Mar 24, 2025
enesozturk
reviewed
Mar 24, 2025
| getConnector(id: string, rdns?: string | null) { | ||
| return state.allConnectors.find(c => c.explorerId === id || c.info?.rdns === rdns) | ||
| const connectorsByNamespace = state.allConnectors.filter( |
There was a problem hiding this comment.
Suggested change
| const connectorsByNamespace = state.allConnectors.filter( | |
| const activeNamespaceConnectors = state.allConnectors.filter( |
maybe which indicates what does the filter do exactly - but not super imp
enesozturk
reviewed
Mar 24, 2025
| @@ -247,6 +247,11 @@ describe('ConnectionController', () => { | |||
| polkadot: 'polkadot-connector', | |||
| bip122: CommonConstantsUtil.CONNECTOR_ID.WALLET_CONNECT | |||
| } | |||
| ChainController.state.chains.set('eip155', { | |||
There was a problem hiding this comment.
Maybe use again ConstantsUtil.CHAIN.EVM as you did on ChainController.test.ts?
enesozturk
approved these changes
Mar 24, 2025
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.
Description
When using multichain to connect to Coinbase with Ethereum, there’s a bug where the active namespace isn’t properly checked which causes it to connect to the Coinbase Solana wallet instead.
Type of change
Associated Issues
For Linear issues: Closes APKT-2547
Checklist