Support optional `networkClientId` in `usePPOM()` by jiexi · Pull Request #85 · MetaMask/ppom-validator

jiexi · 2023-11-08T18:54:02Z

onNetworkChange now updates state.chainStatus for networkState.providerConfig.chaindId and each network configuration in networkState.networkConfigurations
usePPOM() now accepts an optional networkClientId as the last param which will be used to override the default chainId and provider
PPOM Messenger now needs permission to call NetworkController:getNetworkClientById

socket-security · 2023-11-08T18:55:47Z

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

Packages	Version	New capabilities	Transitives	Size	Publisher
@metamask/controller-utils	5.0.2	None	`+39`	8.58 MB	metamaskbot

socket-security · 2023-11-08T18:55:50Z

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

Ignoring: @spruceid/siwe-parser@1.1.3, setimmediate@1.0.5, bs58@4.0.1, ripemd160@2.0.2, md5.js@1.3.5, rlp@2.2.7

Next steps

Take a deeper look at the dependency

Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev.

Remove the package

If you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency.

Mark a package as acceptable risk

To ignore an alert, reply with a comment starting with @SocketSecurity ignore followed by a space separated list of package-name@version specifiers. e.g. @SocketSecurity ignore foo@1.0.0 bar@* or ignore all packages with @SocketSecurity ignore-all

src/ppom-controller.ts

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

jpuri · 2023-11-09T15:23:06Z

src/ppom-controller.ts

+    );
    this.#deleteOldChainIds();
    this.#checkScheduleFileDownloadForAllChains();
  }


this.#updateChainStatus should be called for all chains recently seen, also I think should we called for chainId in usePPOM

What does "recently seen" mean here?

updateChainStatus in usePPOM added here 383b860

Also wondering if there is a callback in network controller now which can be used to add networks to chainStatus as soon as user connects to it.

Not from the network controller. Connecting to a network really just means that the network is permissioned for a certain dapp(s). Maybe we could get this from the PermissionsController?

Hey @jpuri I think we may need to talk through a better way to identify a chain status since as Jiexi said above, in the upcoming multichain API

Connecting to a network really just means that the network is permissioned for a certain dapp(s)

if chainStatus is used to determine whether an incoming request might come for that chain we can't rely on "connection" status since it is really just has a dapp been granted permission to interact with that chain. This is very different from how it works now where we just permission accounts and then the user can "connect" to whatever chain and the dapp automatically gets "connected" to that chain. Long run we need to be able to serve requests that come from any of the "permissioned" chains many of which may not actually be active. So we should think of some protocol for warming up a given chain status when a first RPC request comes in for that chain and pausing any updates when it has been inactive for a while.

In chainStatus we cache data for a chain so that for repeated requests we have it in cache, it is useful to add network to chainStatus as soon as user start interacting with it so that we have the data cache ready to be used when transaction is submitted.

Is there a way for us to identify when a user opens a DAPP connected to any chainId ?

I think we could do this. That would mean going from connections -> origin -> networkClientId -> chainId and passing that to PPOM.

Can we tackle this as part of a separate PR?

Can we tackle this as part of a separate PR?

➕

src/ppom-controller.ts

jpuri · 2023-11-09T15:27:02Z

src/ppom-controller.ts

+        const chainId = addHexPrefix(networkConfig.chainId);
+        this.#updateChainStatus(chainId);
+      },
+    );


We want to cache files only for recently visited networks (visited in past 1 week)

what exactly does "visited" mean here? sounds like you don't consider a network "visited" just because it shows up in the list of added networks. Does that mean "visited" is only true for the selected chain and any chains used inside usePPOM()?

lastVisited is the time network was last visited. We fetch files only for networks visited in last 1 week. If we fetch files for all possible networks it will be too many files.

If we can know when user connects to a new network we can add it to chainStatus it will be useful as it will also give chance to download network files before transaction comes.

…l/use-networkClientId-3

jiexi · 2023-11-13T21:36:11Z

@SocketSecurity ignore setimmediate@1.0.5
@SocketSecurity ignore bs58@4.0.1
@SocketSecurity ignore ripemd160@2.0.2
@SocketSecurity ignore md5.js@1.3.5

unmaintained

jiexi · 2023-11-13T21:40:05Z

@SocketSecurity ignore @spruceid/siwe-parser@1.1.3

The github repo itself has a README and has almost 1k stars

jiexi · 2023-11-13T21:42:07Z

@SocketSecurity ignore rlp@2.2.7

ralxz has contributed to numerous ethereumjs projects already

MajorLift · 2023-11-16T00:08:48Z

I added some any fixes here: #89.
Most importantly, it applies the Provider type from @metamask/network-controller.

src/ppom-controller.ts

…l/use-networkClientId-3

adonesky1 · 2023-11-28T21:00:13Z

package.json

+    "@metamask/base-controller": "^3.2.1",
+    "@metamask/controller-utils": "^5.0.0",
+    "@metamask/network-controller": "^15.2.0",


[nit] Not strictly necessary but all of these have newer versions available now.

mcmire · 2023-11-29T19:04:14Z

Hi, just checking in — what's the status of this PR? Is this waiting on further review?

mcmire · 2024-01-18T18:52:32Z

@jpuri @jiexi Do we know if this PR is necessary anymore? I notice that #130 got merged recently, so I don't know if that was intended to replace this, or if this PR addresses something else.

jiexi added 4 commits November 7, 2023 14:36

WIP

ae0e2a1

Add new networkConfigs to state

c3d4cfa

Fix spec for onNetwork changes

18aa10a

fix specs

eecf3de

jiexi requested a review from a team as a code owner November 8, 2023 18:54

MajorLift reviewed Nov 8, 2023

View reviewed changes

src/ppom-controller.ts Outdated Show resolved Hide resolved

MajorLift reviewed Nov 8, 2023

View reviewed changes

src/ppom-controller.ts Outdated Show resolved Hide resolved

jiexi and others added 2 commits November 8, 2023 15:59

Update src/ppom-controller.ts

73b64f6

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

Update src/ppom-controller.ts

5a01342

Co-authored-by: Jongsun Suh <jongsun.suh@icloud.com>

jpuri reviewed Nov 9, 2023

View reviewed changes

src/ppom-controller.ts Show resolved Hide resolved

jpuri reviewed Nov 9, 2023

View reviewed changes

jiexi added 4 commits November 9, 2023 14:22

add updateChainStatus to usePPOM

383b860

Merge remote-tracking branch 'origin/jl/use-networkClientId-3' into j…

d49cd02

…l/use-networkClientId-3

do not updateChainStatus for all network configs in state

80fc4e3

get new files if some chains are missing

fc62bb9

Merge branch 'main' into jl/use-networkClientId-3

dc2b7f4

MajorLift force-pushed the jl/use-networkClientId-3 branch from fceeda5 to dc2b7f4 Compare November 15, 2023 23:41

MajorLift mentioned this pull request Nov 16, 2023

Typing fixes for provider, ppom and other any usage #89

Merged

adonesky1 reviewed Nov 17, 2023

View reviewed changes

src/ppom-controller.ts Show resolved Hide resolved

adonesky1 reviewed Nov 17, 2023

View reviewed changes

src/ppom-controller.ts Outdated Show resolved Hide resolved

adonesky1 reviewed Nov 17, 2023

View reviewed changes

src/ppom-controller.ts Show resolved Hide resolved

adonesky1 reviewed Nov 17, 2023

View reviewed changes

src/ppom-controller.ts Outdated Show resolved Hide resolved

update onNetworkChange desc

d984abb

jiexi added 4 commits November 17, 2023 13:23

Merge remote-tracking branch 'origin/jl/use-networkClientId-3' into j…

33e8930

…l/use-networkClientId-3

update isDataRequiredForAllChain name

95ce0db

updateChainStatus after checking network support

d842016

Merge branch 'main' into jl/use-networkClientId-3

188dc22

adonesky1 reviewed Nov 28, 2023

View reviewed changes

jiexi closed this Mar 27, 2024

Uh oh!

Conversation

jiexi commented Nov 8, 2023

Uh oh!

socket-security bot commented Nov 8, 2023

Uh oh!

socket-security bot commented Nov 8, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Next steps

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adonesky1 Nov 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiexi commented Nov 13, 2023

Uh oh!

jiexi commented Nov 13, 2023

Uh oh!

jiexi commented Nov 13, 2023

Uh oh!

MajorLift commented Nov 16, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mcmire commented Nov 29, 2023

Uh oh!

mcmire commented Jan 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

socket-security bot commented Nov 8, 2023 •

edited

Loading

adonesky1 Nov 28, 2023 •

edited

Loading