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

improve(ProviderUtils): Avoid redundant eth_chainId rpc calls #308

Merged
merged 1 commit into from
Sep 21, 2022

Conversation

pxrl
Copy link
Contributor

@pxrl pxrl commented 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. It's therefore worth considering using StaticJsonRpcProvider instead, since it's intended for exactly 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

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
Copy link
Contributor Author

pxrl commented Sep 21, 2022

Note I half suspect that the impact of this might be a little bit muted, since this is likely to be an extremely fast request for the RPC endpoint to service. Still, it is redundant and it accounts for a significant portion of our request load, so I think the arguments for using the StaticJsonRpcProvider are pretty favourable.

Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes a lot of sense, StaticJsonRpcProvider is awesome! How do you plan to track network usage post merging this PR to determine its efficacy?

@pxrl
Copy link
Contributor Author

pxrl commented Sep 21, 2022

This change makes a lot of sense, StaticJsonRpcProvider is awesome! How do you plan to track network usage post merging this PR to determine its efficacy?

We have reasonably good reporting via Infura so I was planning to keep an eye on the usage dashboard. Alchemy's reporting seems even better and more fine-grained, but we're using them less.

If you have any other suggestions for where else to look then I'm happy to follow up on that too!

@nicholaspai
Copy link
Member

This change makes a lot of sense, StaticJsonRpcProvider is awesome! How do you plan to track network usage post merging this PR to determine its efficacy?

We have reasonably good reporting via Infura so I was planning to keep an eye on the usage dashboard. Alchemy's reporting seems even better and more fine-grained, but we're using them less.

If you have any other suggestions for where else to look then I'm happy to follow up on that too!

Ah yes good point on watching Infura usage, LGTM!

@pxrl pxrl merged commit 42eca55 into master Sep 21, 2022
@pxrl pxrl deleted the paul/rpcProvider branch September 21, 2022 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants