bluealloy / revm

Ethereum Virtual Machine written in rust that is fast and simple to use
https://bluealloy.github.io/revm/
MIT License
1.5k stars 482 forks source link

Why not having an async Database trait? #1575

Open wtdcode opened 3 weeks ago

wtdcode commented 3 weeks ago

Reading through https://github.com/bluealloy/revm/issues/554 and https://github.com/bluealloy/revm/issues/1534 , I notice that the root cause is that current revm doesn't have async traits.

Given the fact that we already split the original single Database trait into both Database and DatabaseRef traits, can we have another trait like DatabaseAsync (and maybe DatabaseRefAsync) by using https://github.com/dtolnay/async-trait ? In this case:

  1. All databases implemented Database immediately get DatabaseAsync implemented by simply forwarding function calls. However, implementing DatabaseAsync doesn't imply Database trait.
  2. Current ethersdb/alloydb can have a more clean and reasonable implementation because the functions like basic is async now. And they will no longer implement Database.
  3. This avoids confusion like: Does this Database implementation play with async world?

Generally, the modification is a bit huge because we need to add new async API since transact() and many places to support the async trait. Thus, I'm opening the issue for discussion before starting to work on it.

wtdcode commented 3 weeks ago

The new API style may be similar to reqwest, btw.

lostRating commented 1 week ago

Appreciate if async db trait feature can be done. I used to fork and modify it to async for v3.5.0. And it's more difficult to do the same thing for the latest version.

mattsse commented 1 week ago

fyi https://github.com/foundry-rs/foundry-fork-db

wtdcode commented 1 week ago

fyi https://github.com/foundry-rs/foundry-fork-db

tokio::task::block_in_place is not the elegant way. Underhood, it coerces current thread to a blocking thread while spawning a new worker thread. In other words, the overhead (in worst cases) can be as much as spawning a current_thread runtime.

mattsse commented 1 week ago

this can be optimized by adding another variant that does not use block in place for every call.

but it's unreasonable to make entire evm execution async hence there's no async variant

wtdcode commented 1 week ago

this can be optimized by adding another variant that does not use block in place for every call.

I understand your solution because I once also tried to dedicate futures to standalone runtimes, which can also be achieved by my previous PR. It works and generally should be the intended solution as suggested by tokio documents.

but it's unreasonable to make entire evm execution async hence there's no async variant

I agree that evm exution is sync by nature but the db access is not, no?

This also reminds me of another solution: how about other async executors instead of tokio? Since we only need to spin and block in the Database trait, a minimal executor can be just endlessly calling poll until the requests are done. But I doubt alloy-transport compatibility here, especially ws backends.