Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eth_chainId is called every time the provider is used #901

Closed
zemse opened this issue Jun 22, 2020 · 12 comments
Closed

eth_chainId is called every time the provider is used #901

zemse opened this issue Jun 22, 2020 · 12 comments
Labels
bug Verified to be an issue. enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@zemse
Copy link
Collaborator

zemse commented Jun 22, 2020

Hi, I am seeing a lot of unexpected eth_chainId calls:

Screenshot 2020-06-23 00 48 09

My application is a node server calling provider.getBlockNumber() and contract.constantMethod() repeatedly. Based on the response it decides to build a transaction by calling provider.send('eth_getBlockByNumber') for a range of blocks (since I need transaction and receipt roots of these blocks hence justifies 5725 calls) send a transaction (justifies 46 calls to eth_gasPrice). The application has to perform this routine 24x7.

If number of eth_blockNumber requests and eth_call requests is added, it approximately gives number of requests to eth_chainId. Looks like every eth_blockNumber and eth_call requests eth_chainId before the actual call while custom methods using .send don't do that.

At first, it comes to my mind that this can be some sort of safety check to prevent errors due to changing backend. Most of the developers might not be concerned about these calls with a new dapp, but if users increase then it would double the costs, this would then be an issue for them. Can something be done to reduce requests?

@ricmoo
Copy link
Member

ricmoo commented Jun 23, 2020

You are correct, this is a safety feature for v5. In v4, if a backend changed, results were simply undefined (which could result in hanging the UI, returning wrong or inconsistent data), so v5 added the ability (to match EIP-1193) to support a changing backend which requires verifying each call that the backend hasn't changed. An upcoming multi-call provider will mitigate much of this, since the multi call contract will execute these operations in a single call.

For most uses it is not a huge overhead, since the default provider uses sources that intrinsically know their chain ID (and hence do not make external calls) and the next largest group of users are MetaMask (or similar clients), for which the chainId is free and does not hit the network.

But you are absolutely correct that there are cases that you would like to discard safety (since you know the network won't change) and this just adds overhead. The InfuraProvider and ilk do not need to do this (since the network is intrinsic and dictates the URL to hit), so I will probably add a provider similar to:

class StaticJsonRpcProvider extends JsonRpcProvider {
    async getNetwork(): Promise<Network> {
        if (this._network) { return Promise.resolve(this._network); }
        return super.getNetwork();
    }
}

The getNetwork() method is where the network is verified, so overriding this to skip verification is what you need. All other calls call this to do verification. I'll think another day about this and if nothing better comes to mind add this.

However, for your case, I think there is a better solution:

provider.on("block", (blockNumber) => {
    contract.constantMethod()
    // ...
});

Since the "block" event uses the internal blockNumber and fast blockNumber, it will likely make far fewer calls to the chain ID.

Also, if you are using INFURA, Make sure you are using the ethers.providers.InfuraProvider and not the ethers.providers.JsonRpcProvider, as it has these sorts of optimizations built-in.

Make sense? Other suggestions?

@ricmoo ricmoo added the enhancement New feature or improvement. label Jun 23, 2020
@zemse
Copy link
Collaborator Author

zemse commented Jun 23, 2020

I am temporarily using Infura for development. The application expects a RPC url of an ETH node from the user, so user should be able to run their own node or choose Infura, QuickNode or any provider.

Also if the application is shut for few hours and then restarted, it would start from the current block, but the logic needs it to start from where it left off previously. That's why I cannot use the block event in this situation.

I think StaticJsonRpcProvider is what I want! Is it possible that it can be available in next release? Else I can add the code as you have given until that.

@EvilJordan
Copy link

Commenting to watch this thread. I also enabled the getDefaultProvider() today, while passing along my Infura API key, and... hooboy!
Screen Shot 2020-06-28 at 10 43 08 PM

@ricmoo ricmoo added the on-deck This Enhancement or Bug is currently being worked on. label Jun 29, 2020
@ricmoo
Copy link
Member

ricmoo commented Jun 29, 2020

There is now a StaticJsonRpcProvider in 5.0.3. Try it out and let me know if you have any issues.

@ricmoo
Copy link
Member

ricmoo commented Jun 29, 2020

@EvilJordan There isn't really a good way around getBlockNumber. The default poll interval is 4s, but you can make it longer to reduce the number of calls.

I'm contemplating using a more friendly round-robin method in the default provider, since we only need an advisory hint at new blocks, so we could relax the quorum for it.

This shouldn't have changed from v4 though, which did the same thing. :)

@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. labels Jun 29, 2020
@zemse
Copy link
Collaborator Author

zemse commented Jun 29, 2020

Thanks for adding in to the library. I changed JsonRpcProvider to StaticJsonRpcProvider and saw some weird test results.

Can you try to reproduce this?

import { ethers } from 'ethers';

const provider = new ethers.providers.StaticJsonRpcProvider(
  'https://mainnet.infura.io/v3/84842078b09946638c03157f83405213'
);

(async () => {
  console.log('hey');
  const result = await provider.getBlockNumber();
  console.log('yeh');
  console.log(result);
})();

The process exits immediately after hey, it doesn't wait for promise to resolve. yeh doesn't get printed to the console. But if you try the same code with JsonRpcProvider, it works as expected.

However when I used the code you gave instead of importing, it works as expected. Looks like some issue with the import?

import { ethers } from 'ethers';

class StaticJsonRpcProvider extends ethers.providers.JsonRpcProvider {
  async getNetwork(): Promise<ethers.providers.Network> {
    if (this._network) {
      return Promise.resolve(this._network);
    }
    return super.getNetwork();
  }
}

const provider = new StaticJsonRpcProvider(
  'https://mainnet.infura.io/v3/84842078b09946638c03157f83405213'
);

(async () => {
  console.log('hey');
  const result = await provider.getBlockNumber();
  console.log('yeh');
  console.log(result);
})();

@ricmoo
Copy link
Member

ricmoo commented Jun 30, 2020

You're right. I don't know what I broke, but something. I'll look into this immediately.

@ricmoo ricmoo added bug Verified to be an issue. and removed fixed/complete This Bug is fixed or Enhancement is complete and published. labels Jun 30, 2020
@ricmoo ricmoo added the fixed/complete This Bug is fixed or Enhancement is complete and published. label Jul 5, 2020
@ricmoo
Copy link
Member

ricmoo commented Jul 5, 2020

This should be fixed in 5.0.4.

Try it out and let me know. :)

@zemse
Copy link
Collaborator Author

zemse commented Jul 5, 2020

Works super!!

@zemse zemse closed this as completed Jul 5, 2020
zemse added a commit to KMPARDS/kami that referenced this issue Jul 5, 2020
This change was due to eth_chainId checks in JsonRpcProvider in every
call. Discussion thread: ethers-io/ethers.js#901
michaeltout pushed a commit to michaeltout/ethers.js that referenced this issue Aug 23, 2020
michaeltout pushed a commit to michaeltout/ethers.js that referenced this issue Aug 23, 2020
@cruzdanilo
Copy link

Screen Shot 2020-09-18 at 23 49 51
i'm seeing this behavior when using InfuraProvider.getWebSocketProvider. any suggestion?

@ricmoo
Copy link
Member

ricmoo commented Sep 18, 2020

Oh. That’s odd. Maybe I didn’t add the needed logic to the WebSocketProvider to short-circuit that check.

I’ll look into that next week because that is definitely unnecessary. I think detectNetwork can actually cache this indefinitely since the network cannot change without a disconnect.

@ricmoo
Copy link
Member

ricmoo commented Sep 18, 2020

Opened #1054 to track caching chainId.

pxrl added a commit to across-protocol/relayer that referenced this issue Sep 21, 2022
Since ethers v5, all RPC requests have been preceed by an eth_chainId
lookup. This is described by ethers as a safety feature to mitigate
being "RPC rugged" - i.e. where a wallet silently changes RPC without
notifying the provider.

Inspecting the Across logs, eth_chainId is the second-most popular RPC
call, after only eth_getLogs. Over the past 30 days, Infura usage has
been:

  eth_getLogs                264M
  eth_chainId                 30M
  eth_call                    24M
  eth_getTransactionReceipt   11M
  eth_blockNumber              3M

  Total                      336M

Given that the Across bots maintain a 1:1 relationship between provider
instance and back-end RPC provider, there's no obvious way for the
chainId to ever change. The StaticJsonRpcProvider is therefore provided
by ethers for this scenario.

It should be noted that the 30 day figures might be lower than normal
due to downtime for both Arbitrum Nitro and the merge. In any case,
migrating to StaticJsonRpcProvider would reduce total requests by about
9% based on these figures, and would predominantly help reduce latency
in the bots.

See also: ethers-io/ethers.js#901

Ref: ACX-67
pxrl added a commit to across-protocol/relayer that referenced this issue Sep 21, 2022
Since ethers v5, all RPC requests have been preceed by an eth_chainId
lookup. This is described by ethers as a safety feature to mitigate
being "RPC rugged" - i.e. where a wallet silently changes RPC without
notifying the provider.

Inspecting the Across logs, eth_chainId is the second-most popular RPC
call, after only eth_getLogs. Over the past 30 days, Infura usage has
been:

  eth_getLogs                264M
  eth_chainId                 30M
  eth_call                    24M
  eth_getTransactionReceipt   11M
  eth_blockNumber              3M

  Total                      336M

Given that the Across bots maintain a 1:1 relationship between provider
instance and back-end RPC provider, there's no obvious way for the
chainId to ever change. The StaticJsonRpcProvider is therefore provided
by ethers for this scenario.

It should be noted that the 30 day figures might be lower than normal
due to downtime for both Arbitrum Nitro and the merge. In any case,
migrating to StaticJsonRpcProvider would reduce total requests by about
9% based on these figures, and would predominantly help reduce latency
in the bots.

See also: ethers-io/ethers.js#901
MaximusHaximus added a commit to LIT-Protocol/js-sdk that referenced this issue Jul 16, 2024
…ests

Reference issue - ethers-io/ethers.js#901
Cuts our RPC request cost approx in half by eliminating a sequential call to get `chainId` from every(!) contract call we make
MaximusHaximus added a commit to LIT-Protocol/js-sdk that referenced this issue Jul 16, 2024
…ests

Reference issue - ethers-io/ethers.js#901
Cuts our RPC request cost approx in half by eliminating a sequential call to get `chainId` from every(!) contract call we make
MaximusHaximus added a commit to LIT-Protocol/js-sdk that referenced this issue Jul 16, 2024
Reference issue - ethers-io/ethers.js#901
Cuts our RPC request cost approx in half by eliminating a sequential call to get `chainId` from every(!) contract call we make
MaximusHaximus added a commit to LIT-Protocol/js-sdk that referenced this issue Jul 16, 2024
…requests

Reference issue - ethers-io/ethers.js#901
Cuts our RPC request cost approx in half by eliminating a sequential call to get `chainId` from every(!) contract call we make
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified to be an issue. enhancement New feature or improvement. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

4 participants