Skip to content

feat: Solana accountChanged event #15561

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

Open
wants to merge 67 commits into
base: main
Choose a base branch
from

Conversation

jiexi
Copy link
Contributor

@jiexi jiexi commented May 22, 2025

Description

  • Adds solana accountChange support
  • Fixes issue with selecting non-evm account from dapp account switcher

Related issues

Fixes:

Manual testing steps

  1. yarn setup
  2. yarn start:ios:flask
  3. Launch flask build
  4. Make two solana accounts
  5. Go to any webpage with in app browser
  6. Enter the block of code below in the Safari dev console for the webpage (Open safari on your computer, Click Develop, Select the metamask app browser tab running in the simulator. You have to enable safari dev mode first)
  7. Permit 2 solana accounts and 1 eth account (the eth account is needed so that you can get back into modifying the permissions since there is a bug with the selectors right now)
  8. Play around
window.addEventListener('message', (event) => {
  const {
    target,
    data
  } = event.data;
  if (
    target !== "metamask-inpage" ||
    data?.name !== 'metamask-multichain-provider'
  ) {
    return;
  }
  console.log(data.data)
})

const caipPostMessage = (data) => {
  window.postMessage({
    target: 'metamask-contentscript',
    data: {
      name: 'metamask-multichain-provider',
      data
    }
  }, location.origin)
}

caipPostMessage({
    jsonrpc: "2.0",
    method: "wallet_createSession",
    params: {
      optionalScopes: {
        "solana:5eykt4UsFv8P8NJdTREpY1vzqKqZKvdp": {
          methods: [],
          notifications: [],
          accounts: [],
        },
      },
      sessionProperties: {
        solana_accountChanged_notifications: true,
      },
    },
  })

Screenshots/Recordings

Before

After

Screen.Recording.2025-05-22.at.2.54.56.PM.mov

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.

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.

@jiexi jiexi changed the title WIP: Jl/sip 26 multichain router solana account changed feat: Solana accountChanged event May 28, 2025
@jiexi jiexi added the skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. label May 28, 2025
@jiexi
Copy link
Contributor Author

jiexi commented May 28, 2025

skipping sonar-cloud because coverage artificially lower due to non-functional stylistic changes

@jiexi jiexi added Run Smoke E2E Requires smoke E2E testing and removed Run Smoke E2E Requires smoke E2E testing labels May 28, 2025
Copy link
Contributor

github-actions bot commented May 28, 2025

https://bitrise.io/ Bitrise

❌❌❌ pr_smoke_e2e_pipeline failed on Bitrise! ❌❌❌

Commit hash: 861510c
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/39edb141-6a4b-456f-ab3f-30bbf62103f4

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Tip

  • Check the documentation if you have any doubts on how to understand the failure on bitrise

@adonesky1
Copy link
Contributor

still seeing all the accounts selected in the dapp account selector view:

Screen.Recording.2025-05-29.at.9.54.37.AM.mov

@adonesky1
Copy link
Contributor

Another bug I'm seeing:

Screen.Recording.2025-05-29.at.9.58.59.AM.mov

@jiexi jiexi added Run Smoke E2E Requires smoke E2E testing and removed Run Smoke E2E Requires smoke E2E testing labels May 29, 2025
Copy link
Contributor

github-actions bot commented May 29, 2025

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: 75ac09a
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/210cd1db-86ca-429e-8461-4a9aa80a26e1

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
77.2% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@jiexi jiexi added Run Smoke E2E Requires smoke E2E testing and removed Run Smoke E2E Requires smoke E2E testing labels May 29, 2025
SolScope.Mainnet,
)?.address;
} catch {
// noop
Copy link
Member

Choose a reason for hiding this comment

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

Should we place some logs here to help debugging if it comes to it ?

Copy link
Member

@ffmcgee725 ffmcgee725 left a comment

Choose a reason for hiding this comment

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

left 1 non blocking comment, lgtm 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Run Smoke E2E Requires smoke E2E testing skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. team-wallet-api-platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants