celo-org/optics-monorepo

Move db logic from `optics-ethereum` to `optics-core`

Closed this issue · 7 comments

  • The chain-specific home currently keeps a DB and puts things into it.
  • instead, this should be handled 1 layer up in optics-core. Homes enum
  • chain should be unaware of the DB
pub struct CachingHome {
    db: HomeDb
    home: Homes
}

// replace `impl Home for Homes`
impl Home for CachingHome {
    // for each function, check the DB first (if appropriate)
    // cache afterwards (if appropriate)
    // basically replicate the existing functionality in `EthereumHome`
}

@prestwich So basically the goal here is that for every function that retrieves data (i.e. fetches from db), we want to implement that at the Homes/CachingHome level instead of at the chain-specific (e.g. EthereumHome) level? And while CachingHome contract call functions may match on different Homes variants (e.g. Ethereum, Mock, etc), the db fetching functions simply call out to the db directly without matching on any specific variants?

yes, exactly. The caching layer is not Ethereum-specific logic. It generalizes to all chains. So move it to the base instead of the chain

If we're going to refactor to bring db logic out of chain-specific home interfaces, would it make sense to just completely remove query capabilities from the concept of a Home/Homes and leave it all to HomeDB?

What I mean is that we just entirely separate command and query and that an agent accesses any home information directly from its HomeDB. The only reason we have query functions on the home contract interfaces is that we used to be able to call out directly to chain to retrieve info. Now that we have all home contract state is accessed locally through HomeDB I feel like queries should only go through HomeDB. Logically, it seems like functions like signed_update_by_new_root or leaf_by_tree_index don't need to go in two places (HomeDB and Home) and can just go in HomeDB.

CachingHome also makes sense but I get the feeling it's going to get logically complicated with HomeIndexer filling HomeDB and CachingHome being a mix of command and quering HomeDB. Idk maybe I'm just overthinking...

well we have to distinguish between state reads (not indexed locally) and events (indexed locally). But in general I like the direction you're going.

Because the current state isn't locally indexed, I think we want to keep a logical Home interface on top of the DB and chain connection

For the sake of time and getting a watcher up asap, I'm going to implement the Replica/ReplicaIndexer using the same pattern as the Home/HomeIndexer (db/indexing tied to specific chain) despite the fact that there is some technical debt here. Thinking we do this then get to this big conceptual refactor after we have a watcher stood up. Also, we'll have a better idea of what we needed after getting further on the watcher