Skip to content

feat(perf): batch eth_call requests in tests/scripts #5363

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

Open
mds1 opened this issue Jul 12, 2023 · 20 comments
Open

feat(perf): batch eth_call requests in tests/scripts #5363

mds1 opened this issue Jul 12, 2023 · 20 comments
Assignees
Labels
C-forge Command: forge Cmd-forge-script Command: forge script Cmd-forge-test Command: forge test T-feature Type: feature T-perf Type: performance
Milestone

Comments

@mds1
Copy link
Collaborator

mds1 commented Jul 12, 2023

Component

Forge

Describe the feature you would like

See gakonst/ethers-rs#2508 for details/rationale on the request here

forge batching is not necessarily dependent on that ethers-rs feature, as even without it forge could presumably convert batches calls into multicalls. This could result in significant performance improvements for RPC-heavy scripts and fork tests

Currently instead of eth_call we simulate the call and use eth_getStorageAt as needed. This is suboptimal than just using eth_call directly because:

  • Alchemy prices eth_getStorageAt at 17 CUPS but eth_call is 26, so any tx reading 2+ slots is currently paying more (and running slower due to multiple requests) than necessary, especially when you consider that we can batch eth_calls but can't batch eth_getStorageAt
  • Simulating might not give the right result for chains where some opcodes behave differently than on mainnet (e.g. NUMBER returns L2 block number on optimism but L1 block number on arbitrum)

So I think the best path forward is:

  1. Replace the current behavior to use eth_call instead
  2. Then, batch eth_calls. The approach here would be:
  • Collect all consecutive staticcalls, stop collecting when there's a state changing operation
  • If Multicall3 is available on the chain, batch calls with it. If Multicall3 is not available on the chain, either use eth_call state overrides to place it there as part of the call, or just send normal requests without Multicall3

Additional context

No response

@mds1 mds1 added the T-feature Type: feature label Jul 12, 2023
@gakonst gakonst added this to Foundry Jul 12, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Jul 12, 2023
@mds1
Copy link
Collaborator Author

mds1 commented Aug 31, 2023

Another approach to this could be to use the standard JSON RPC batching—not all providers support this, and it also doesn’t result in reduced RPC usage, but it can still be useful for users using their own node or a provider that supports it. This would probably have to be opt-in as a result, whereas the approach described above can be abstracted from the user as the default

@mds1 mds1 added the T-perf Type: performance label Sep 20, 2023
@JONEmoad
Copy link

Perhaps this is a suggestion that can be implemented

https://github.com/foundry-rs/foundry/blob/529559c01fabad0e6316d605fd2c4326b8ad6567/crates/evm/core/src/fork/backend.rs#L326C61-L326C61

impl<M> Future for BackendHandler<M>
where
    M: Middleware + Clone + Unpin + 'static,
{
    type Output = ();

    fn poll(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {
        let pin = self.get_mut();


        let futures = pin.pending_requests.iter_mut().map(|request| {
            match request {
                ProviderRequest::Storage(fut) => fut.as_mut(),
            }
        }).collect::<Vec<_>>();

        let all_futures = futures::future::join_all(futures);

        match all_futures.poll_unpin(cx) {
            Poll::Ready(results) => {
                // ...

                if pin.handlers.is_empty() && pin.incoming.is_done() {
                    Poll::Ready(())
                } else {
                    Poll::Pending
                }
            },
            Poll::Pending => Poll::Pending,
        }
    }
}


@mds1
Copy link
Collaborator Author

mds1 commented Feb 7, 2024

Another approach: assuming the RPC URL supports state overrides (like geth does), use Dedaub's storage extractor code to batch eth_getStorageAt: https://github.com/Dedaub/storage-extractor

@BABA3344

This comment was marked as abuse.

@zerosnacks zerosnacks added Cmd-forge-test Command: forge test C-forge Command: forge Cmd-forge-script Command: forge script labels Jul 2, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@zerosnacks
Copy link
Member

This will be trivial to add after Alloy multicall support has been added: alloy-rs/alloy#328

@grandizzy grandizzy removed this from the v1.0.0 milestone Nov 12, 2024
@sakulstra
Copy link
Contributor

Just stumbled over this when looking for why some of our scripts are so slow.
Seems like this might be the problem as when checking with RUST_LOG=DEBUG the id counter goes well above ~2000 requests in some cases.

Looking forward to an improvement on that front. Happy to test once there is a pr.

@mds1
Copy link
Collaborator Author

mds1 commented Feb 7, 2025

@zerosnacks @grandizzy any chance we can get this in for v1? A lot of folks have scripts or fork tests that fetch a lot of data and this would significantly improve performance

@zerosnacks
Copy link
Member

Related upstream PR in Alloy: alloy-rs/alloy#2010

The 1.0 pre-release has been cut but our release cycle should be short enough to get this in soon

cc @yash-atreya

@mds1
Copy link
Collaborator Author

mds1 commented Feb 7, 2025

that would be amazing, thank you!

@zerosnacks
Copy link
Member

zerosnacks commented Feb 10, 2025

On a further note the upstream PR (alloy-rs/alloy#2010) I linked is, while tangentially related, not directly related and would not automatically resolve this issue.

From conversation / discussion it appears my note here #5363 (comment) that it would be trivial to add is incorrect and it was based on some incorrect assumptions I had at the time about the proposed implementation

@mds1
Copy link
Collaborator Author

mds1 commented Feb 11, 2025

Got it, so what would be your estimate for when this can be implemented?

@blmalone
Copy link

@grandizzy @mattsse This would really help speed up our CI pipelines and improve productivity by a lot. Can we get this prioritized?

@jenpaff jenpaff assigned grandizzy and unassigned grandizzy Apr 15, 2025
@blmalone
Copy link

@grandizzy, following up with details on our use case:

In the superchain-ops repo at OP Labs, which handles upgrades across the Superchain, we perform a network-heavy operation per chain. You can see the relevant code here: https://github.com/ethereum-optimism/superchain-ops/blob/main/src/improvements/SuperchainAddressRegistry.sol#L214-L257

We’d like to parallelize these network calls to improve performance (it's pretty slow right now). They’re all read-only operations, no writes involved.

@blmalone
Copy link

blmalone commented May 2, 2025

@grandizzy any updates on this issue?

@grandizzy
Copy link
Collaborator

@mattsse and I discussed about this and something we're looking for improving such is adding new eth_getAccountInfo API in reth, see paradigmxyz/reth#16027 Mind that the calls are already parallelized, but the fact that we make 3 calls for each account is the bottleneck. The new API will allow single request to fetch all we need, though this will be only available in reth. Looking for other suggestions.

@mds1
Copy link
Collaborator Author

mds1 commented May 2, 2025

@grandizzy What do you think about this idea from above? This is similar to how viem works by default when making calls. Note this is a separate type of RPC call from the kind where we need to collect all account info before simulating state changing operations, and is specifically a performance increase when users have multiple consecutive view calls, but it's nice because it doesn't rely on reth

So I think the best path forward is:

  1. Replace the current behavior to use eth_call instead
  2. Then, batch eth_calls. The approach here would be:
  • Collect all consecutive staticcalls, stop collecting when there's a state changing operation
  • If Multicall3 is available on the chain, batch calls with it. If Multicall3 is not available on the chain, either use eth_call state overrides to place it there as part of the call, or just send normal requests without Multicall3

@grandizzy
Copy link
Collaborator

@mds1 Ah, I see, so you mean if we need to fetch 2 accounts, instead doing 6 calls (3 for each account to get their balance, nonce, code) we could batch 2 eth_calls and use multicall3 (if available on chain) https://github.com/mds1/multicall3?tab=readme-ov-file#batch-contract-reads to read these? I think that could work, would those eth_call be able to retrieve data that we need for account? (balance, nonce, code)

@mds1
Copy link
Collaborator Author

mds1 commented May 2, 2025

Yep that's exactly correct! Multicall3 has a method to get balance, but not code or nonce. Do you always need to fetch code and nonce even if the user is just making an eth_call?

@DaniPopes
Copy link
Member

DaniPopes commented May 2, 2025

Yep that's exactly correct! Multicall3 has a method to get balance, but not code or nonce. Do you always need to fetch code and nonce even if the user is just making an eth_call?

They're the fields of an Ethereum account (AccountInfo), which we can't lazy-load individually since the entire data structure has to be populated on first access.

Here's where the requests happen: https://github.com/foundry-rs/foundry-fork-db/blob/5b03d6e9320d502daa3a6687298e59a51967de3e/src/backend.rs#L281

What do you think about this idea from above? This is similar to how viem works by default when making calls. Note this is a separate type of RPC call from the kind where we need to collect all account info before simulating state changing operations, and is specifically a performance increase when users have multiple consecutive view calls, but it's nice because it doesn't rely on reth

This is not as straight forward as you think. Requests are issued inside/by the EVM, which is sequential. This essentially means that every single statement in your Solidity code must execute fully before the next one can even begin to be evaluated; there is no "async" in the EVM. Even if we had the perfect viem-like multicall backend, every CALL still has to have the result value of the fork eth_call in memory before the next opcode, let alone the next CALL can execute.

The only actionable improvement I can see here is the aforementioned eth_getAccountInfo. You could also try using IMulticall3 directly yourselves in the script, since that would be a single eth_call, and it's what a potential vm.multicall would be doing anyway (type-unsafely).

@jenpaff jenpaff added this to the v1.3.0 milestone May 6, 2025
@yash-atreya
Copy link
Member

@mattsse and I discussed about this and something we're looking for improving such is adding new eth_getAccountInfo API in reth, see paradigmxyz/reth#16027 Mind that the calls are already parallelized, but the fact that we make 3 calls for each account is the bottleneck. The new API will allow single request to fetch all we need, though this will be only available in reth. Looking for other suggestions.

Relevant fork-db PR foundry-rs/foundry-fork-db#48 that add supports for eth_getAccountInfo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-script Command: forge script Cmd-forge-test Command: forge test T-feature Type: feature T-perf Type: performance
Projects
Status: Todo
Development

No branches or pull requests

10 participants