Skip to content

Conversation

@adonesky1
Copy link
Contributor

@adonesky1 adonesky1 commented Mar 12, 2025

Description

Adds support for metamask_accountChanged notification via wallet_notify (CAIP-319) notification to enable our wallet-standard implementation to forward account changed events via the wallet standard change event

This is not a pattern we want to continue since the Multichain API should not require account changing. This is a one off bespoke implementation to support a Solana wallet-standard interface.

Open in GitHub Codespaces

E2E testing for this feature should be handled in the Wallet Standard powered test dapp one we have it cc @EdouardBougon @baptiste-marchand

Demo video

Screen.Recording.2025-03-25.at.1.38.28.PM.mov

Related issues

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/4193

Manual testing steps

  1. Build in flask
  2. Generate 2 Solana Accounts
  3. Go to any site
  4. Open the dev console and run the following script:
const extensionPort = chrome.runtime.connect(<your extension id>)

extensionPort.onMessage.addListener((msg) => {
  console.log(msg.data)
})

extensionPort.postMessage({
  type: "caip-x",
  data: {
    jsonrpc: "2.0",
    method: "wallet_createSession",
    params: {
      requiredScopes: {},
      optionalScopes: {
        "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp": {
          methods: ["getAccount"],
          notifications: [],
          accounts: ['solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:<SOLANA_ACCOUNT_ADDRESS_1>',
          'solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp:<SOLANA_ACCOUNT_ADDRESS_2>']
        }
      },
  sessionProperties: {'solana_accountChanged_notifications': true},
};,
  }
})

  1. Switch between the two Solana accounts in the account selector in the wallet UI
  2. You should see console logs in the dev console:
{
    "method": "wallet_notify",
    "params": {
        "scope": "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp",
        "notification": {
            "method": "solana_accountChanged",
            "params": "<YOUR_SOLANA_WALLET_ADDRESS>"
        }
    }
}

Screenshots/Recordings

Before

After

Pre-merge author checklist

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.

@github-actions
Copy link
Contributor

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.

@metamaskbot metamaskbot added the team-wallet-api-platform-deprecated DEPRECATED: please use "team-wallet-integrations" instead label Mar 12, 2025
@adonesky1 adonesky1 force-pushed the ad/solana-accountChanged-sip26 branch from 7ec7603 to a3ed385 Compare March 12, 2025 22:22
@adonesky1 adonesky1 changed the title start enabling solana accounts changed events solana accounts changed event Mar 17, 2025
@adonesky1 adonesky1 changed the title solana accounts changed event feat: add Solana accountsChanged event support Mar 17, 2025
@adonesky1 adonesky1 force-pushed the ad/solana-accountChanged-sip26 branch 3 times, most recently from 48cbf3a to f246120 Compare March 17, 2025 21:56
@adonesky1 adonesky1 force-pushed the ad/solana-accountChanged-sip26 branch from f286ac9 to a92a66e Compare March 18, 2025 20:27
@metamaskbot
Copy link
Collaborator

❌ Multichain API Spec Test Failed. View the report here.

@metamaskbot
Copy link
Collaborator

❌ Multichain API Spec Test Failed. View the report here.

@adonesky1 adonesky1 force-pushed the ad/solana-accountChanged-sip26 branch from 444a28b to 86f636f Compare March 19, 2025 17:34
@metamaskbot
Copy link
Collaborator

❌ Multichain API Spec Test Failed. View the report here.

@metamaskbot
Copy link
Collaborator

Builds ready [27cc99c]
Page Load Metrics (3345 ± 1591 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint8561042424881902913
domContentLoaded153113950293630211451
load160014832334533141591
domInteractive17251696230
backgroundConnect381662410482231
firstReactRender18275798139
getState14739174219105
initialActions01000
loadScripts114211501227626001248
setupStore970411017483
uiStartup175320801509550312416

@metamaskbot
Copy link
Collaborator

❌ Multichain API Spec Test Failed. View the report here.

@metamaskbot
Copy link
Collaborator

Builds ready [10b079d]
Page Load Metrics (6128 ± 3591 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint171827723587774393572
domContentLoaded159927580542172123463
load173027985612874783591
domInteractive262215248507243
backgroundConnect1032419670680327
firstReactRender282371095929
getState401199282328157
initialActions01000
loadScripts119827114450867303232
setupStore1052812314469
uiStartup210028811865774543579

@metamaskbot
Copy link
Collaborator

Builds ready [10b9cc5]
Page Load Metrics (4146 ± 1668 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint187416727370132131543
domContentLoaded174313333309225551227
load192118030414634741668
domInteractive283939910450
backgroundConnect2173374911715343
firstReactRender592451344321
getState822001442423203
initialActions01000
loadScripts12981055822412077997
setupStore3546318312761
uiStartup318722467857852972544

@@ -0,0 +1,11 @@
export enum KnownSessionProperties {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to export this from the @metamask/chain-agnostic-permission package, can do as a fast follow: MetaMask/core#5522

@adonesky1 adonesky1 marked this pull request as ready for review March 21, 2025 19:34
@adonesky1 adonesky1 requested review from a team as code owners March 21, 2025 19:34
@metamaskbot
Copy link
Collaborator

Builds ready [6317531]
Page Load Metrics (3735 ± 2074 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint169420198360243362082
domContentLoaded158620063339343352082
load174320189373543192074
domInteractive251300135272131
backgroundConnect7881032420197
firstReactRender23113652613
getState2639615710048
initialActions01000
loadScripts111819596276642572044
setupStore9221847134
uiStartup193220423506642342033


if (
parsedSolanaAddresses.includes(account.address) &&
originsWithSolanaAccountChangedNotifications[origin]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor performance optimization if we move this check to the top of this loop

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 we need the parsedSolanaAddresses to be parsed first though?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if originsWithSolanaAccountChangedNotifications[origin] is false, then we don't need to do parsedSolanaAddresses and its includes checks in the first place

if (!hasProperty(caveat.value, 'sessionProperties')) {
caveat.value.sessionProperties = {};
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay if we add a break in here? or use find instead of the for loop? definitely a nit

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done here: 2ea1fcd & here 4450c7e

@jiexi
Copy link
Contributor

jiexi commented Mar 25, 2025

tested manually. looks great

@adonesky1 adonesky1 merged commit d0e141c into sip-26 Mar 25, 2025
71 checks passed
@adonesky1 adonesky1 deleted the ad/solana-accountChanged-sip26 branch March 25, 2025 18:49
@github-actions github-actions bot locked and limited conversation to collaborators Mar 25, 2025
@metamaskbot
Copy link
Collaborator

Builds ready [bbcfb23]
Page Load Metrics (3297 ± 852 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint19701038730261777853
domContentLoaded15841029626151812870
load20701038332971775852
domInteractive2646311710550
backgroundConnect871106595344165
firstReactRender182491165326
getState35791306241116
initialActions01000
loadScripts1146989819771835881
setupStore1837716411957
uiStartup290914101615130821480

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

team-wallet-api-platform-deprecated DEPRECATED: please use "team-wallet-integrations" instead

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants