-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: add Solana Wallet Standard to in-app browser #15707
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR integrates Solana Wallet Standard support into the in-app browser by extending the inpage bridge, background engine, and UI layers to handle non-EVM (CAIP-based) networks and accounts.
- Adds a new Solana Wallet Standard injector and dependencies to the inpage bridge
- Extends engine and selectors to include non-EVM network configurations and permissions
- Updates UI components (
BrowserTab
andAccountConnect
) and tests to display CAIP account addresses and handle EIP-1193 request metadata
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
scripts/inpage-bridge/src/solanaWalletStandard.js | New injector for Solana Wallet Standard in the inpage bridge |
scripts/inpage-bridge/src/index.js | Calls the new Solana injector when provider shouldInject |
package.json | Adds @metamask/multichain-api-client and Solana standard deps |
scripts/inpage-bridge/src/provider.js | Minor whitespace removal |
e2e/specs/multichain/permissions/chains/permission-system-initial-connection.spec.js | Adds a test step for “Select All Networks” |
app/selectors/networkController.ts | Extends selector to merge non-EVM network configurations |
app/core/mocks/MockedEngine.ts | Mocks multichain network controller state |
app/core/Permissions/specifications.js | Updates caveat type signatures for non-EVM functions |
app/core/Permissions/index.ts | Hooks into MultichainNetworkController state for permissions |
app/core/Engine/types.ts | Registers new MultichainRouterActions |
app/core/Engine/controllers/multichain-router/constants.ts | Defines router event constants |
app/core/Engine/Engine.ts | Configures and instantiates MultichainRouter |
app/core/BackgroundBridge/BackgroundBridge.js & .test.js | Passes isEip1193Request metadata and fixes messenger binding |
app/components/Views/BrowserTab/BrowserTab.tsx | Renders CAIP account addresses instead of EVM hex addresses |
app/components/Views/AccountConnect/AccountConnect.types.ts | Adds optional isEip1193Request metadata to route params |
app/components/Views/AccountConnect/AccountConnect.tsx & .test.tsx | Implements EIP-1193 fallback and memoized network avatars |
Comments suppressed due to low confidence (4)
scripts/inpage-bridge/src/solanaWalletStandard.js:14
- Solana injector file mixes CommonJS
require
with ES moduleexport default
. Consider using a consistent module system (either convert toimport
/export
or usemodule.exports
) to avoid bundler/runtime issues.
export default injectSolanaWalletStandard;
e2e/specs/multichain/permissions/chains/permission-system-initial-connection.spec.js:63
- The test calls
tapSelectAllNetworksButton
but there’s no import or definition for this helper inConnectedAccountsModal
. Ensure the method exists and is implemented, or update the test to match available UI actions.
await ConnectedAccountsModal.tapSelectAllNetworksButton();
app/components/Views/BrowserTab/BrowserTab.tsx:1407
- The
connectedAccounts
prop now only receives non-EVM CAIP addresses, dropping EVM hex accounts. Consider merging bothpermittedEvmAccountsList
andpermittedCaipAccountAddressesList
so users still see EVM accounts in the browser UI.
connectedAccounts={permittedCaipAccountAddressesList}
app/core/Permissions/specifications.js:55
- [nitpick] The return type for
isNonEvmScopeSupported
was widened toJson | unknown
, reducing type safety. If possible, replaceunknown
with a more specific boolean or typed union to improve maintainability.
* isNonEvmScopeSupported: (scope: import('@metamask/chain-agnostic-permission').InternalScopeString) => import('@metamask/utils').Json | unknown
4220058
to
d8dc3e2
Compare
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Caution Review the following alerts detected in dependencies. According to your organization's Security Policy, you must resolve all "Block" alerts before proceeding. Learn more about Socket for GitHub.
|
EDIT: nvm these issues have to do with Solana devnet not being available in Mobile |
Code LGTM, but don't have time to pull down and test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Tested and works great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code makes sense, and tested locally with some dapps, nothing out of the ordinary spotted. LGTM!
|
|
Description
This PR adds Solana Wallet Standard to the in-app browser.
Related issues
Extension PR: MetaMask/metamask-extension#31705
Manual testing steps
a.
yarn setup:flask && yarn start:ios:flask
b.
yarn watch
, then pressi
and open MetaMask FlaskScreenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist