MetaMask / eth-sig-util

A collection of functions for signing and verifying data with Ethereum keys.
ISC License
575 stars 227 forks source link

Remove external references from package meta data - codeload.github.com #297

Closed VamshikrishDanam closed 1 year ago

VamshikrishDanam commented 1 year ago

wagmi Version 0.11.5

Current Behavior Hey Team, we are trying to include wagmi package in our application. While installing this package in our network we are getting network timed out error as it is trying to access one of it's transient dependency from codeload.github.com which is restricted in our network.

Error: npm ERR! code ETIMEDOUT npm ERR! network request to https://codeload.github.com/ethereumjs/ethereumjs-abi/tar.gz/ee3994657fa7a427238e6ba92a84d0b529bbcde0 failed, reason: connect ETIMEDOUT 140.82.114.10:443 npm ERR! network This is a problem related to network connectivity.

Expected Behavior We want to install wagmi and enable wallet connect in our application.

As we analyzed, it is because of this following line: https://github.com/MetaMask/eth-sig-util/blob/1.x/package.json#L24 that codeload.github.com reference is hardcoded into the package meta data.

We believe, one possible solution for this issue is, if we can upgrade wagmi to have all external dependencies within the package instead of using external github references.

Steps To Reproduce If we try to install wagmi with common command, npm i wagmi within our ecosystem we are getting this error.

Error: npm ERR! code ETIMEDOUT npm ERR! network request to https://codeload.github.com/ethereumjs/ethereumjs-abi/tar.gz/ee3994657fa7a427238e6ba92a84d0b529bbcde0 failed, reason: connect ETIMEDOUT 140.82.114.10:443 npm ERR! network This is a problem related to network connectivity.

mcmire commented 1 year ago

Hi @VamshikrishDanam, it looks like the version of eth-sig-util you linked to is 1.x, which is very old. In the latest version (5.x at this time of writing) the problem you mention is fixed. I'm not quite sure how wagmi works, but is it possible to use the latest version of eth-sig-util?

VamshikrishDanam commented 1 year ago

Thank you, @Elliot Winkler. Actually, we haven't included this package in our repo. We are trying to install wagmi and internally it is using eth-sig-util.

└─┬ @amfi/connect-wallet@1.1.21 └─┬ @walletconnect/web3-provider@1.7.1 └─┬ web3-provider-engine@16.0.1 └─┬ eth-sig-util@1.4.2 └── ethereumjs-abi@0.6.8 (git+ssh://git@github.com/ethereumjs/ethereumjs-abi.git#ee3994657fa7a427238e6ba92a84d0b529bbcde0)

I have raised the issue with wagmi as well. May be I should ask them to use latest version of eth-sig-util? Culd you please confirm if the latest version, 5.0.2 has the fix?

mcmire commented 1 year ago

Culd you please confirm if the latest version, 5.0.2 has the fix?

Yes, and in fact 5.0.2 doesn't rely on ethereumjs-abi anymore: https://github.com/MetaMask/eth-sig-util/blob/v5.0.2/package.json#L41

Can you run yarn why eth-sig-util and post the output?

VamshikrishDanam commented 1 year ago

We are not able to install wagmi because of this dependency. When I run this command locally, I get this.

yarn why eth-sig-util yarn why v1.22.19 [1/4] Why do we have the module "eth-sig-util"...? [2/4] Initialising dependency graph... [3/4] Finding dependency... error We couldn't find a match! Done in 0.03s.

mcmire commented 1 year ago

Ah, I misunderstood how you were installing wagmi.

I managed to track this down. Here's the dependency graph that's relevant to your problem:

wagmi@0.11.5
└ @wagmi/core@0.9.5
  └ @wagmi/connectors@0.2.5
    └ @coinbase/wallet-sdk@3.6.3
      └ eth-json-rpc-filters@4.2.2
        └ eth-json-rpc-middleware@6.0.0
          └ eth-sig-util@1.4.2

So, the issue here is with @coinbase/wallet-sdk, not with wagmi. If @coinbase/wallet-sdk were using eth-json-rpc-filters 5.1.0, then the dependency graph would look like:

wagmi@0.11.5
└ @wagmi/core@0.9.5
  └ @wagmi/connectors@0.2.5
    └ @coinbase/wallet-sdk@3.6.3
      └ eth-json-rpc-filters@5.1.0
        └ eth-json-rpc-middleware@9.0.1
          └ @metamask/eth-sig-util@5.0.2

and therefore you wouldn't get this issue.

It may very well be that Coinbase is not aware of these new versions. I'll see if I can dedicate some time to submitting a pull request to fix this soon.

In the meantime, you should be able to fix this by adding a resolutions field to your package.json which forces NPM to use eth-json-rpc-filters 5.1.0.

VamshikrishDanam commented 1 year ago

Thank you, @mcmire. After adding eth-json-rpc-filters 5.1.0, I am getting new 404 error. Any clue?

npm ERR! code E404

npm ERR! 404 npm ERR! 404 '@walletconnect/window-metadata is not in this registry. npm ERR! 404 npm ERR! 404 Note that you can also install from a npm ERR! 404 tarball, folder, http url, or git url.

mcmire commented 1 year ago

@VamshikrishDanam Hmm, great question. I'm not sure how that could happen. However, I gave you the wrong instructions, so that could be it. resolutions is a Yarn thing — I think overrides might work better in your case.

By the way, I looked into this some more and it looks like Coinbase did update the Wallet SDK to use a newer version of eth-json-rpc-filters: https://github.com/coinbase/coinbase-wallet-sdk/pull/816. This isn't quite available in a stable release yet, however — you can see here that it's marked under the beta tag — but I think you could conceivably cheat by adding "@coinbase/wallet-sdk": "3.6.4" to the list of overrides.

VamshikrishDanam commented 1 year ago

@mcmire No problem, I figured it out for npm as overrides, https://stackoverflow.com/questions/52416312/npm-equivalent-of-yarn-resolutions.

Yeah, I saw this and was not sure when they will publish. I didn't realize it was in beta, added in overrides and it works now, Thank you! I am able to install wagmi!!

mcmire commented 1 year ago

@VamshikrishDanam Awesome, glad you got it working! I'm going to close this as it seems the problem is solved, but let me know if you run into any more issues.

VamshikrishDanam commented 1 year ago

Thank you, @mcmire!

VamshikrishDanam commented 1 year ago

Hey @mcmire I am getting this same error when trying to install walletconnect. Could you please help me identify which dependency is causing it?

npm ERR! code ETIMEDOUT npm ERR! syscall connect npm ERR! errno ETIMEDOUT npm ERR! network request to https://codeload.github.com/ethereumjs/ethereumjs-abi/tar.gz/ee3994657fa7a427238e6ba92a84d0b529bbcde0 failed, reason: connect ETIMEDOUT 140.82.113.9:443 npm ERR! network This is a problem related to network connectivity. npm ERR! network In most cases you are behind a proxy or have bad network settings. npm ERR! network npm ERR! network If you are behind a proxy, please make sure that the npm ERR! network 'proxy' config is set properly. See: 'npm help config

mcmire commented 1 year ago

Hi @VamshikrishDanam, I can tell you how to do it this time!

The first thing you can try is to look up the dependency graph for the package you're trying to install. There are a couple of sites I know of to do this: https://npm.anvaka.com/ and https://npmgraph.js.org/. In both cases, type in the name of the package and it'll crawl dependencies of the package, dependencies of the dependencies, etc. In this case the second site gives us a clue. Once you load the graph you can press Ctrl-F / Cmd-F and search for ethereumjs-abi to find it. Once you do that you will be able to trace the connections between packages. Based on this graph, it looks like this is how walletconnect links to ethereumjs-abi:

walletconnect@1.7.8
└ @walletconnect/web3-provider@1.8.0
  └ web3-provider-engine@16.0.1
    └ eth-sig-util@1.4.2
      └ git+https://github.com/ethereumjs/ethereumjs-abi.git

That said, there is some context here that you wouldn't see just from viewing the graph. In essence, you're trying to install WalletConnect v1. Unfortunately, this relies on an old package of ours, web3-provider-engine. We have deprecated this package, but in addition, WalletConnect has deprecated v1 itself and are really pushing people to use v2.

I'd like to point you in the right direction here, but it depends on which platform you're using. Are you trying to build a dapp on desktop or on mobile?

VamshikrishDanam commented 1 year ago

Thank you, @mcmire, for the breakdown, now I know how to check for dependency tree with npm! We are building a dapp compatible to both mobile and desktop but our majority of users will be using mobile.

I have integrated AuthClient and generateNonce from "@WalletProvider/auth-client". Now I am trying to migrate from AuthClient to web3Wallet as mentioned here. And they recommend to fetch Core and Web3Wallet which are coming from walletconnect v1. Probably I need to connect with walletconnect team and understand how to fetch these from v2.

mcmire commented 1 year ago

@VamshikrishDanam Gotcha, makes sense! It looks like the official docs for v2 also recommend using @walletconnect/core and @walletconnect/web3wallet. And it looks like judging by the dependency graphs that neither of these packages import web3-provider-engine. So you should be good if you go this route, but of course let me know if you run into any more issues.