Explicitly add the chain Id to the JSONRPCPROVIDER #62
Conversation
Without this ether js JsonRPC provider seems to call a detectNetwork Function every event loop which makes a call the node like {method: eth_ChainID, params: [], id: randomNumber()}
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/c-labs/use-contractkit/84w85TSp1Texb9LoQKmkzLCQiurB |
| const provider = kit.web3.currentProvider as unknown as ExternalProvider; | ||
| return useMemo(() => { | ||
| return new Web3Provider(provider); | ||
| return new Web3Provider(provider, {chainId: network.chainId, name: network.name}); |
There was a problem hiding this comment.
Looks good but like linter says you'll need to update the memo list too
There was a problem hiding this comment.
was wondering about your thoughts on moving the provider up into the App Context Provider. Or maybe its own ProviderProvider Context? so
There was a problem hiding this comment.
Your comment cuts out. What would be the benefit of the move?
There was a problem hiding this comment.
One shared instance rather than each component that uses this having its own
…snt need to ask the node for this already known info
| const nextKit = await getConnectedKit(); | ||
| const nextProvider = nextKit.web3 | ||
| .currentProvider as unknown as ExternalProvider; | ||
| return new Web3Provider(nextProvider).getSigner(nextKit.defaultAccount); | ||
| }, [kit.defaultAccount, getConnectedKit, signer]); | ||
| return new Web3Provider(nextProvider, { chainId, name }).getSigner( |
There was a problem hiding this comment.
@AlexBHarley would probably be the best person to ask. I haven't reviewed this part thoroughly but it's entirely possible it was an oversight. No disrespect to Alex who overall did a great job but some parts were written hastily I think!
There was a problem hiding this comment.
Looking at it a bit more, do you mean reuse the kit's provider? That's a Web3 provider, this creates an ethers.js provider
There was a problem hiding this comment.
new Web3Provider(nextProvider, { chainId, name }) looks to give us same kind of thing as useProvider no?
There was a problem hiding this comment.
I'm not quite understanding you I think. This is inside the useProvider hook.
There was a problem hiding this comment.
This PR is modifying 2 places. the useProvider and inside useGetConnectedSigner this line is inside the later
|
relates to #63 |
|
@aaronmgdr I've tested this solution in Celo Wallet and haven't found it reduced the eth_chainId calls. I think the recommended fix is to use the StaticJsonProvider: ethers-io/ethers.js#901 (comment) Though that may not work here before we need the provider to respond to network changes (like in Metamask) cc @dckesler |
Without this ether js JsonRPC provider seems to call a detectNetwork Function every event loop which makes a call the node like {method: eth_ChainID, params: [], id: randomNumber()}