Standardise and align db abstraction implementations

[damiannolan]

Description

The prover service ev-prover currently contains three storage abstractions over rocksdb embedded databases.
They are as follows:

• ProofStorage (storage_path: "~/.ev-prover/data/proofs.db"
• HyperlaneMessageStore (storage_path: "~/.ev-prover/data/messages.db"
• HyperlaneSnapshotStore (storage_path: "~/.ev-prover/data/snapshots.db"

Each implementation wraps an instance of rocksdb with an Arc<DB> smart pointer for shared access across async tasks.

At the moment there is some discrepancies between each implementation. We should aim to align the implementations such that they are consistent.

Before addressing this issue we should evaluate if having three distinct physical DBs is desirable (i.e. proofs.db, messages.db, snapshots.db). Or if these can be consolidated into one logical application specific DB. If one physical DB is preferred the 3 abstractions can still be kept within the application code and access to the DB object is shared via Arc<DB> so that logical separation is maintained.

Things to consider regarding using three independent DBs:

• 3× WALs, background threads, file descriptors, block caches, etc.
• More operational work (3 dirs to back up, monitor, migrate).
• Needs for different RocksDB options (cache size, compression, etc.).
• Very different access patterns / options
  • e.g. one write-heavy, one read-mostly, each tuned differently.

Furthermore, we should consider a wrapper object for Storage so that we do not need to pass three objects around. This can improve code readability and simplify various method arguments.
For example something very straight-forward such as:

pub struct AppStorage {
    proofs: ProofStorage,
    messages: HyperlaneMessageStore,
    snapshots: HyperlaneSnapshotStore,
}

The following issue should also be addressed:

• Remove the RwLock from the HyperlaneMessages db #274

[blasrodri]

Something else I think we should consider is the lack of atomicity on certain operations. This is an argument in favor for collapsing all three into one.

In this extract, failure on the last line could leave the different databases in an inconsistent state, perhaps?

We could leverage WriteBatchWithTransaction if all the operations were in the same DB.

# prover/programs/message.rs
        // Store the unfinalized snapshot
        snapshot.height = committed_height;
        let snapshot_index = self.snapshot_store.current_index()? + 1;
        self.snapshot_store.insert_snapshot(snapshot_index, snapshot)?;

        // Submit the proof to ZKISM
        info!("Submitting Hyperlane tree proof to ZKISM...");
        let response = ism_client.send_tx(message_proof_msg).await?;

        if !response.success {
            error!("Failed to submit Hyperlane tree proof to ZKISM: {:?}", response);
            return Err(anyhow::anyhow!("Failed to submit Hyperlane tree proof to ZKISM"));
        }

        info!("ZKISM was updated successfully");
        self.proof_store
            .store_membership_proof(committed_height, &message_proof.0, &message_proof.1)
            .await?;

        // TODO: check for unfinalized shapshots and retry
        // this is a necessary mainnet optimization
        self.snapshot_store.finalize_snapshot(trusted_snapshot_index)?;

[damiannolan]

Thank you for chiming in @blasrodri!

In this extract, failure on the last line could leave the different databases in an inconsistent state, perhaps?

I do agree, that there is definitely risks for inconsistent state. I haven't thoroughly walked through them mentally to identify if anything is critical right now.

Intuitively I think it would be better to consolidate each of the dbs into one physical db. Of course we can still have separate column families etc.. IMO it would lend itself to simpler recovery scenarios.