feat: Add Multichain API to Flask #27782
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. |
|
@metamaskbot update-policies |
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
|
this PR needs the patches from here: https://github.com/MetaMask/metamask-extension/pull/27847/files#r1801195961 |
jiexi
changed the base branch from
caip-multichain
to
caip25-permission-migration
|
|
app/scripts/lib/rpc-method-middleware/handlers/wallet-createSession/handler.js
Outdated
Show resolved
Hide resolved
app/scripts/lib/rpc-method-middleware/handlers/wallet-createSession/handler.js
Outdated
Show resolved
Hide resolved
app/scripts/lib/rpc-method-middleware/handlers/wallet-createSession/handler.js
Outdated
Show resolved
Hide resolved
app/scripts/lib/rpc-method-middleware/handlers/wallet-createSession/handler.js
Outdated
Show resolved
Hide resolved
|
Done here #29003 |
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** <!-- Write a short description of the changes included in this pull request, also include relevant motivation and context. Have in mind the following questions: 1. What is the reason for the change? 2. What is the improvement/solution? --> [](https://codespaces.new/MetaMask/metamask-extension/pull/27940?quickstart=1) ## **Related issues** Fixes: #27782 (comment) ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [ ] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md). - [ ] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [ ] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **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.
|
@metamaskbot update-policies |
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff |
|
❌ Multichain API Spec Test Failed. View the report here. |
|
New, updated, and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected] |
|
❌ Multichain API Spec Test Failed. View the report here. |
|
❌ Multichain API Spec Test Failed. View the report here. |
|
❌ API Spec Test Failed. View the report here. |
|
❌ Multichain API Spec Test Failed. View the report here. |
|
@metamaskbot update-policies |
|
Policy update failed. You can review the logs or retry the policy update here |
… into jl/caip-multichain-migrate-core
|
@metamaskbot update-policies |
|
No policy changes |
|
❌ API Spec Test Failed. View the report here. |
|
❌ Multichain API Spec Test Failed. View the report here. |
|
Note my changes that add and use the |
|
❌ API Spec Test Failed. View the report here. |
|
❌ Multichain API Spec Test Failed. View the report here. |
|
❌ API Spec Test Failed. View the report here. |
|
❌ Multichain API Spec Test Failed. View the report here. |
|
looks like CI is passing now minus api-specs which will pass once this PR gets merged into main |
…tichain-migrate-core
|
@metamaskbot update-policies |
|
❌ Multichain API Spec Test Failed. View the report here. |
|
No policy changes |
](https://codespaces.new/MetaMask/metamask-extension/pull/27940?quickstart=1){kind=link}
| normalizedOptionalScopes, | ||
| ); | ||
|
|
||
| const existsNetworkClientForChainId = (chainId: Hex) => { |
There was a problem hiding this comment.
Nit:
| const existsNetworkClientForChainId = (chainId: Hex) => { | |
| const networkClientExistsForChainId = (chainId: Hex) => { |
| engine.push((req, res, _next, end) => { | ||
| const { provider } = this.networkController.getNetworkClientById( | ||
| req.networkClientId, | ||
| ); | ||
|
|
||
| // send request to provider | ||
| provider.sendAsync(req, (err, providerRes) => { | ||
| // forward any error | ||
| if (err) { | ||
| return end(err); | ||
| } | ||
| // copy provider response onto original response | ||
| Object.assign(res, providerRes); | ||
| return end(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Nit: If this is the right provider, we should be able to use request now instead of sendAsync:
| engine.push((req, res, _next, end) => { | |
| const { provider } = this.networkController.getNetworkClientById( | |
| req.networkClientId, | |
| ); | |
| // send request to provider | |
| provider.sendAsync(req, (err, providerRes) => { | |
| // forward any error | |
| if (err) { | |
| return end(err); | |
| } | |
| // copy provider response onto original response | |
| Object.assign(res, providerRes); | |
| return end(); | |
| }); | |
| }); | |
| engine.push(async (req, res) => { | |
| const { provider } = this.networkController.getNetworkClientById( | |
| req.networkClientId, | |
| ); | |
| res.result = await provider.request(req); | |
| }); |
| // will hang in selenium since it can only do one thing at a time. | ||
| // the workaround is to put the response on window.asyncResult and poll for it. | ||
| driver.executeScript( | ||
| ([m, p, g, s, e]: [ |
There was a problem hiding this comment.
Nit: Are there longer names we can give to these variables?
| export const createDriverTransport = (driver: Driver) => { | ||
| return async ( | ||
| _: string, | ||
| __: string, |
There was a problem hiding this comment.
Nit: Why is one underscore not enough? 🤔
|
|
||
| const existsNetworkClientForChainId = (chainId: Hex) => { | ||
| try { | ||
| hooks.findNetworkClientIdByChainId(chainId); |
There was a problem hiding this comment.
Not worth changing now, but instead of trying to find a network client ID we could just try to find the presence of the chain ID in the list of network configurations. i.e., NetworkController.getNetworkConfigurationByChainId might be a better candidate to use here (although it doesn't throw, unlike findNetworkClientIdByChainId).
Description
This branch adds support for the Multichain API to the Flask build of the Extension.
The existing API (via injected provider) should be completely unchanged.
(Very Briefly) What is the MetaMask Multichain API
externally_connectable. Not accessible via an injected global likewindow.ethereumKey Documents/Standards
mip = MetaMask Improvement Proposal
Manual testing steps
Then
(RECOMMENDED) Use the Multichain Test Dapp
OR
Form requests manually
Pre-merge author checklist
Pre-merge reviewer checklist