Make it possible to put in metamask provider

[mathijs81]

Thanks for this awesome simplification of working with uniswap!

I'm working on a dapp and running into issues of a mismatch between metamask and the calls this library does. E.g. when I switch metamask to localhost or back to mainnet or a testnet, it would be nice if this library just switches transparently. I've tried everything I could to get an 'underlying' RPC URL from metamask, but this seems to be not possible, and passing a providerUrl is the only API this library currently supports, so this would mean that I need to make a manual mapping somewhere to map current metamask settings to ChainId + URL combinations.

When I look at the code of the library, it in the end also just uses an ethers provider, so would it be possible to pass in the existing provider I got from metamask? Then everything would work without extra code needed.

[joshstevens19]

Hey yes this is exactly what I want to extend it so you can pass in a ethers or web3 instance which hooks onto network calls changes etc!

It’s in the backlog but because you have raised a ticket il have a look at it this week for you. No promises on a release though! 😅👍

[joshstevens19]

The only issue I see is your contract address pairs won’t be the same on another network well it may be the same but in a lot of examples it will not so changing that context instance without you creating a new instance probably doesn’t make sense. But I think passing in a ethereum provider makes sense just don’t think the flow of it changing network automatically would make sense, I think on those cases you have to create a new uniswap pair instance again. So would still need some custom code for you handling network change.

[mathijs81]

Dynamic switching of data because you change metamask while running the dApp is not really a usecase that's very important. It's more that if you load the dApp while metamask is talking to localhost:8545 (that I'm running with a mainnet fork) that everything works then.

Sorry for tagging this onto this issue as well, but I was playing around with TokenFactoryPublic::balanceOf and I think that's not making use of the fast multicall batching right? Is this for a specific reason?

[joshstevens19]

Yeah makes sense. Il look into allowing you to pass in an ethereum provider, been meaning to just not had time.

In terms of TokenFactoryPublic > balanceOf that’s a singular call aka 1 contract lookup so it doesn’t need to use multicall for that logic. If you look at anything which does > 1 call like TokensFactory it uses the multicall logic!

[joshstevens19]

im looking at this today! @mathijs81

[joshstevens19]

Hey @mathijs81 2.4.0 is now deployed with this logic in:

2.4.0 release:

  - You can now pass in an ethereum provider to the UniswapPair creation
  - breaking change on the public factories as you now have to pass it ChainIdAndProvider | EthereumProvider

doc can be found here https://github.com/uniswap-integration/simple-uniswap-sdk#ethereum-provider

All the public factories now take an object in the parameters so will be a breaking change!

Let me know all is well! i tested it myself using metamask and worked fine! closing.

Thanks again for raising this feature request!

[mathijs81]

Thanks for launching this.

It's not 100% working for me yet though, but I have trouble figuring out what's exactly going wrong. First, I was passing a 'full' ethers provider around in my app (i.e my own instance of new ethers.providers.Web3Provider(window.ethereum)) and that doesn't work to pass as ethereumProvider for me (getting very cryptic "Error: 'args.method' must be a non-empty string." error somewhere, I think caused by uniswap-sdk trying to wrap it another time in a Web3Provider). Passing window.ethereum goes better, but then I get "UniswapError: invalid from or to contract tokens". If I just swap out ethereumProvider: window.ethereum for chainId+providerUrl:localhost:8545 (what metamask is also pointing to) then everything works again, so I know the code is working. Do you have any idea where to look further? The console doesn't give me more information unfortunately.

[joshstevens19]

so error number one ethers.providers.Web3Provider(window.ethereum) kinda makes sense we wrap anything passed in here within a Web3Provider which seems to work for JsonRpcProvider and StaticProvider but I guess not for Web3Provider. Error number 2 is a bit odd I have tested it on mainnet and rinkeby both seem to work by just passed window.ethereum in as the ethereum provider.

Just so I know there this line in the readme:

You must supply the ethereum address and the wallet be approved to use for the dApp and unlocked before passing it in. The uniswap sdk makes those assumptions without them it will not work as MetaMask is not allowed access to your dApp. Any change of network or ethereum address change you will need to handle in your dApp and regenerate the uniswap pair context. Most the time the contract addresses for your tokens are different anyway.

so are you sure the ethereum provider on window.ethereum has been approved for your dApp access for your wallet aka ethereum.request({ method: 'eth_requestAccounts' }) has been called and resolved? without that it won't work. If that's the case I'm going to assume here my docs didn't make sense.

Il reopen as I know number 1 is probably a issue I can recreate but number 2 i can not (I tested on mainnet and rinkeby)

[mathijs81]

Yes, access is all correct.

I got the stacktrace that gets caught and replaced by "invalid from or to contract tokens" and the orignal error is

Network - 31337 is not got a contract defined it only supports mainnet, kovan, rinkeby, bsc and ropsten
  getContractBasedOnNetwork multicall.js:459

So the thing is that I'm running a hardhat fork of mainnet but this still has this special own chainId. I can force hardhat to just also have chainId 1 I guess. It would maybe be nicer if the library does a check for the chainId earlier and fail with a more descriptive error message than just getting "invalid from or to contract tokens".

[joshstevens19]

So this error is actually to do with multicall lib as that only will do calls on networks it knows about. If you can force hardhat config and that works for you then no worries.. I will improve on this error message so it throws if your trying to use a chainId which the lib does not support.

[joshstevens19]

Once you changed to network 1 does it work using window.ethereum @mathijs81

[mathijs81]

Yes, can confirm it's working as expected now.

[joshstevens19]

cool i have added that check so people can be informed better if their are using a chainId which is not supported

https://github.com/uniswap-integration/simple-uniswap-sdk/commit/1b65d649c536cf532b01465fba8ce6378526a6a2

Will look into if passing in a web3 provider it not working now.

[joshstevens19]

ok this should now work with the way you did it before or window.ethereum + adding better error messages so if it happens in your situation then people know what they need to do. This is deployed with 2.4.1