feat: add ChainController by ccharly · Pull Request #4227 · MetaMask/core

ccharly · 2024-04-29T12:07:43Z

Explanation

This new controller will be responsible of exposing Snaps that implement the newly defined Chain API.

This controller also implements the chain API itself.

Reference: https://github.com/MetaMask/chain-api/

Changelog

`@metamask/chain-controller`

Added: New ChainController: exposes/interacts with chain API providers

Checklist

I've updated the test suite for new or updated code as appropriate
I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
I've highlighted breaking changes using the "BREAKING" category above as appropriate

socket-security · 2024-04-29T12:08:16Z

👍 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.

View full report↗︎

packages/chain-controller/src/ChainController.ts

…ontroller instance

…rClient

mcmire

Hello! I am not sure whether more work was planned for this PR, but I made some suggestions based on how we've written controllers and tests for controllers in other places. One question I have is whether ChainController needs to be a controller? I didn't notice any state being set, but perhaps that was just left out for now.

packages/chain-controller/LICENSE

packages/chain-controller/src/ChainController.test.ts

packages/chain-controller/src/ChainController.ts

packages/chain-controller/src/SnapChainProviderClient.test.ts

packages/chain-controller/src/SnapHandlerClient.ts

This matches better our coding style here: https://github.com/MetaMask/contributor-docs/blob/main/docs/unit-testing.md#-highlight-the-exercise-phase Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>

ccharly · 2024-05-14T15:00:53Z

@mcmire I did update the README, but I had to hack the script locally because it was removing the accounts-controller from the README because of the .slice(1) here.

I didn't want to fix this in this PR, cause I'm not sure it's still required or not.

ccharly · 2024-05-14T15:02:04Z

I've added the DO-NOT-MERGE label, cause I've added skipLibCheck=true for tsbuild temporarily, otherwise the CI complains (something the jsx-runtime, we faced that in another PR and that might be related to some Snaps dependencies, I'll wait for more info on this before removing it from the PR)

packages/chain-controller/src/ChainController.ts

packages/chain-controller/src/SnapHandlerClient.ts

packages/chain-controller/src/SnapHandlerClient.test.ts

packages/chain-controller/README.md

Co-authored-by: Gustavo Antunes <17601467+gantunesr@users.noreply.github.com>

…reverted later)

…with node v20

socket-security · 2024-05-15T15:23:57Z

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package	New capabilities	Transitives	Size	Publisher
npm/@metamask/chain-api@0.0.1	None	`+1`	229 kB	metamaskbot
npm/@metamask/chain-controller@0.0.0-use.local	None	`0`	0 B

View full report↗︎

ccharly · 2024-05-16T07:24:38Z

Removing the DO-NOT-MERGE label for now, to have a successful CI and allow preview builds

ccharly · 2024-05-16T07:28:53Z

@metamaskbot publish-preview

github-actions · 2024-05-16T07:32:27Z

Preview builds have been published. See these instructions for more information about preview builds.

Expand for full list of packages and versions.

{
  "@metamask-previews/accounts-controller": "14.0.0-preview-699063fc",
  "@metamask-previews/address-book-controller": "4.0.1-preview-699063fc",
  "@metamask-previews/announcement-controller": "6.1.0-preview-699063fc",
  "@metamask-previews/approval-controller": "6.0.2-preview-699063fc",
  "@metamask-previews/assets-controllers": "29.0.0-preview-699063fc",
  "@metamask-previews/base-controller": "5.0.2-preview-699063fc",
  "@metamask-previews/build-utils": "2.0.1-preview-699063fc",
  "@metamask-previews/chain-controller": "0.0.0-preview-699063fc",
  "@metamask-previews/composable-controller": "6.0.1-preview-699063fc",
  "@metamask-previews/controller-utils": "9.1.0-preview-699063fc",
  "@metamask-previews/ens-controller": "10.0.1-preview-699063fc",
  "@metamask-previews/eth-json-rpc-provider": "3.0.2-preview-699063fc",
  "@metamask-previews/gas-fee-controller": "15.1.2-preview-699063fc",
  "@metamask-previews/json-rpc-engine": "8.0.2-preview-699063fc",
  "@metamask-previews/json-rpc-middleware-stream": "7.0.1-preview-699063fc",
  "@metamask-previews/keyring-controller": "16.0.0-preview-699063fc",
  "@metamask-previews/logging-controller": "3.0.1-preview-699063fc",
  "@metamask-previews/message-manager": "8.0.2-preview-699063fc",
  "@metamask-previews/name-controller": "6.0.1-preview-699063fc",
  "@metamask-previews/network-controller": "18.1.0-preview-699063fc",
  "@metamask-previews/notification-controller": "5.0.1-preview-699063fc",
  "@metamask-previews/permission-controller": "9.0.2-preview-699063fc",
  "@metamask-previews/permission-log-controller": "2.0.1-preview-699063fc",
  "@metamask-previews/phishing-controller": "9.0.2-preview-699063fc",
  "@metamask-previews/polling-controller": "6.0.2-preview-699063fc",
  "@metamask-previews/preferences-controller": "11.0.0-preview-699063fc",
  "@metamask-previews/profile-sync-controller": "0.0.0-preview-699063fc",
  "@metamask-previews/queued-request-controller": "0.10.0-preview-699063fc",
  "@metamask-previews/rate-limit-controller": "5.0.1-preview-699063fc",
  "@metamask-previews/selected-network-controller": "13.0.0-preview-699063fc",
  "@metamask-previews/signature-controller": "16.0.0-preview-699063fc",
  "@metamask-previews/transaction-controller": "29.0.0-preview-699063fc",
  "@metamask-previews/user-operation-controller": "10.0.0-preview-699063fc"
}

mcmire · 2024-05-17T16:13:29Z

@ccharly Thank you for the changes! I agree it looks a lot better. Nothing jumps out at me, so feel free to get approval from your team and continue on with this PR :)

gantunesr

LGTM. Just one minor comment

packages/chain-controller/src/SnapChainProviderClient.test.ts

ccharly added 8 commits May 2, 2024 19:10

feat(chain-controller): add new package chain-controller

58f799f

feat(chain-controller): add boilerplate for ChainController

415b9c2

chore(chain-controller): add deps (use local chain-api for now)

280de01

feat(chain-controller): add SnapControllerClient

1c9c1ee

feat(chain-controller): add SnapChainProviderClient

602658a

feat(chain-controller): add getBalances support

28ed771

chore(chain-controller): use @metamask/chain-api@^0.0.01

e134e41

chore(chain-controller): align snaps-* dependencies

395a3c3

ccharly force-pushed the feature/chain-controller branch from 8fd62cf to 395a3c3 Compare May 2, 2024 17:16

ccharly added 2 commits May 2, 2024 19:19

chore: lint

0328a35

chore(chain-controller): default CHANGELOG.md

6165cd2

ccharly force-pushed the feature/chain-controller branch from ea51726 to 6165cd2 Compare May 2, 2024 20:14

ccharly added 3 commits May 2, 2024 22:42

fix(chain-controller): fix tests

404495b

test: add test for getBalancesFromAccount

57229cc

chore: re-generate yarn.lock (based from main)

2448d24

ccharly marked this pull request as ready for review May 2, 2024 21:06

ccharly requested a review from a team May 2, 2024 21:06

test: wording

ae58123

montelaidev reviewed May 3, 2024

View reviewed changes

packages/chain-controller/src/ChainController.ts Outdated Show resolved Hide resolved

packages/chain-controller/src/ChainController.ts Show resolved Hide resolved

ccharly and others added 6 commits May 6, 2024 14:31

refactor(chain-controller): remove dummy field in controller's state

80002cc

refactor(chain-controller): use messenger instead of relying on SnapC…

0e333ed

…ontroller instance

refactor(chain-controller): rename SnapControllerClient -> SnapHandle…

6b92491

…rClient

chore: lint

9b91909

docs: adapt docs with Snap request handler

9dcbdbf

Merge branch 'main' into feature/chain-controller

4ce91aa

mcmire reviewed May 6, 2024

View reviewed changes

ccharly and others added 3 commits May 7, 2024 12:48

test(chain-controller): better test code structure

12eaa49

This matches better our coding style here: https://github.com/MetaMask/contributor-docs/blob/main/docs/unit-testing.md#-highlight-the-exercise-phase Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>

chore(chain-controller): update copyright

865d60f

refactor(chain-controller): remove unused type assertion

b805e50

ccharly added the DO-NOT-MERGE label May 14, 2024

gantunesr reviewed May 14, 2024

View reviewed changes

ccharly and others added 6 commits May 15, 2024 09:56

test(chain-controller): better test case phrasing

cd6b4a1

Co-authored-by: Gustavo Antunes <17601467+gantunesr@users.noreply.github.com>

refactor(chain-controller): remove use of SnapHandlerClient.withSnapId

969ae38

docs(chain-controller): add brief description + diagrams in README

a0cb74f

chore(assets-controller/tsbuild): use skipLibCheck=true (needs to be …

cc9fe31

…reverted later)

chore: remove use of skipLibCheck for {assets,chain}-controller

90cd72b

build: use resolutions for snaps-{sdk,utils} to prevent failed build …

d9fe89c

…with node v20

ccharly added 2 commits May 16, 2024 09:14

Merge branch 'main' into feature/chain-controller

0480100

chore: re-generate yarn.lock (based from main)

699063f

ccharly removed the DO-NOT-MERGE label May 16, 2024

ccharly added 2 commits May 17, 2024 14:14

Merge branch 'main' into feature/chain-controller

4323e27

chore: remove use of resolutions for snaps-{sdk,utils}

5d5d9f4

ccharly force-pushed the feature/chain-controller branch from 2958fdb to 0b1caeb Compare May 17, 2024 13:03

chore(chain-controller): bump dependencies

61ff417

ccharly force-pushed the feature/chain-controller branch from 0b1caeb to 61ff417 Compare May 17, 2024 13:14

Merge branch 'main' into feature/chain-controller

abe1266

gantunesr previously approved these changes May 17, 2024

View reviewed changes

packages/chain-controller/src/SnapChainProviderClient.test.ts Outdated Show resolved Hide resolved

refactor(chain-controller): options0 -> request

69fae5b

ccharly dismissed gantunesr’s stale review via 69fae5b May 17, 2024 16:41

gantunesr approved these changes May 17, 2024

View reviewed changes

ccharly merged commit 2752a88 into main May 20, 2024

ccharly deleted the feature/chain-controller branch May 20, 2024 09:16

Uh oh!

Conversation

ccharly commented Apr 29, 2024

Explanation

Changelog

@metamask/chain-controller

Checklist

Uh oh!

socket-security bot commented Apr 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mcmire left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ccharly commented May 14, 2024

Uh oh!

ccharly commented May 14, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

socket-security bot commented May 15, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ccharly commented May 16, 2024

Uh oh!

ccharly commented May 16, 2024

Uh oh!

github-actions bot commented May 16, 2024

Uh oh!

mcmire commented May 17, 2024

Uh oh!

gantunesr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

`@metamask/chain-controller`

socket-security bot commented Apr 29, 2024 •

edited

Loading

socket-security bot commented May 15, 2024 •

edited

Loading