From 919c81fe7dc58a6058d793006647c573d3e04952 Mon Sep 17 00:00:00 2001 From: Adam Szkoda Date: Mon, 25 May 2020 02:26:54 +0200 Subject: [PATCH] Ditch StoreItem trait (#1185) --- beacon_node/store/src/hot_cold_store.rs | 29 ++++---- beacon_node/store/src/lib.rs | 91 +++++++------------------ 2 files changed, 42 insertions(+), 78 deletions(-) diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index fdbad31d6..08c586af1 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -7,8 +7,7 @@ use crate::impls::beacon_state::store_full_state; use crate::iter::{ParentRootBlockIterator, StateRootsIterator}; use crate::metrics; use crate::{ - leveldb_store::LevelDB, DBColumn, Error, PartialBeaconState, SimpleStoreItem, Store, StoreItem, - StoreOp, + leveldb_store::LevelDB, DBColumn, Error, PartialBeaconState, SimpleStoreItem, Store, StoreOp, }; use lru::LruCache; use parking_lot::{Mutex, RwLock}; @@ -460,7 +459,7 @@ impl HotColdDB { // 1. Convert to PartialBeaconState and store that in the DB. let partial_state = PartialBeaconState::from_state_forgetful(state); - partial_state.db_put(&self.cold_db, state_root)?; + self.cold_db.put(state_root, &partial_state)?; // 2. Store updated vector entries. let db = &self.cold_db; @@ -500,7 +499,9 @@ impl HotColdDB { /// Load a restore point state by its `state_root`. fn load_restore_point(&self, state_root: &Hash256) -> Result, Error> { - let mut partial_state = PartialBeaconState::db_get(&self.cold_db, state_root)? + let mut partial_state: PartialBeaconState = self + .cold_db + .get(state_root)? .ok_or_else(|| HotColdDBError::MissingRestorePoint(*state_root))?; // Fill in the fields of the partial state. @@ -680,8 +681,9 @@ impl HotColdDB { /// Load the state root of a restore point. fn load_restore_point_hash(&self, restore_point_index: u64) -> Result { let key = Self::restore_point_key(restore_point_index); - RestorePointHash::db_get(&self.cold_db, &key)? - .map(|r| r.state_root) + self.cold_db + .get(&key)? + .map(|r: RestorePointHash| r.state_root) .ok_or_else(|| HotColdDBError::MissingRestorePointHash(restore_point_index).into()) } @@ -692,8 +694,8 @@ impl HotColdDB { state_root: Hash256, ) -> Result<(), Error> { let key = Self::restore_point_key(restore_point_index); - RestorePointHash { state_root } - .db_put(&self.cold_db, &key) + self.cold_db + .put(&key, &RestorePointHash { state_root }) .map_err(Into::into) } @@ -704,13 +706,16 @@ impl HotColdDB { /// Load a frozen state's slot, given its root. fn load_cold_state_slot(&self, state_root: &Hash256) -> Result, Error> { - Ok(ColdStateSummary::db_get(&self.cold_db, state_root)?.map(|s| s.slot)) + Ok(self + .cold_db + .get(state_root)? + .map(|s: ColdStateSummary| s.slot)) } /// Store the slot of a frozen state. fn store_cold_state_slot(&self, state_root: &Hash256, slot: Slot) -> Result<(), Error> { - ColdStateSummary { slot } - .db_put(&self.cold_db, state_root) + self.cold_db + .put(state_root, &ColdStateSummary { slot }) .map_err(Into::into) } @@ -719,7 +724,7 @@ impl HotColdDB { &self, state_root: &Hash256, ) -> Result, Error> { - HotStateSummary::db_get(&self.hot_db, state_root) + self.hot_db.get(state_root) } /// Check that the restore point frequency is valid. diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index b1d622292..558342609 100644 --- a/beacon_node/store/src/lib.rs +++ b/beacon_node/store/src/lib.rs @@ -59,23 +59,39 @@ pub trait Store: Sync + Send + Sized + 'static { fn key_delete(&self, column: &str, key: &[u8]) -> Result<(), Error>; /// Store an item in `Self`. - fn put(&self, key: &Hash256, item: &I) -> Result<(), Error> { - item.db_put(self, key) + fn put(&self, key: &Hash256, item: &I) -> Result<(), Error> { + let column = I::db_column().into(); + let key = key.as_bytes(); + + self.put_bytes(column, key, &item.as_store_bytes()) + .map_err(Into::into) } /// Retrieve an item from `Self`. - fn get(&self, key: &Hash256) -> Result, Error> { - I::db_get(self, key) + fn get(&self, key: &Hash256) -> Result, Error> { + let column = I::db_column().into(); + let key = key.as_bytes(); + + match self.get_bytes(column, key)? { + Some(bytes) => Ok(Some(I::from_store_bytes(&bytes[..])?)), + None => Ok(None), + } } /// Returns `true` if the given key represents an item in `Self`. - fn exists(&self, key: &Hash256) -> Result { - I::db_exists(self, key) + fn exists(&self, key: &Hash256) -> Result { + let column = I::db_column().into(); + let key = key.as_bytes(); + + self.key_exists(column, key) } /// Remove an item from `Self`. - fn delete(&self, key: &Hash256) -> Result<(), Error> { - I::db_delete(self, key) + fn delete(&self, key: &Hash256) -> Result<(), Error> { + let column = I::db_column().into(); + let key = key.as_bytes(); + + self.key_delete(column, key) } /// Store a block in the store. @@ -107,7 +123,7 @@ pub trait Store: Sync + Send + Sized + 'static { state_root: &Hash256, summary: HotStateSummary, ) -> Result<(), Error> { - summary.db_put(self, state_root).map_err(Into::into) + self.put(state_root, &summary).map_err(Into::into) } /// Fetch a state from the store. @@ -249,63 +265,6 @@ pub trait SimpleStoreItem: Sized { fn from_store_bytes(bytes: &[u8]) -> Result; } -/// An item that may be stored in a `Store`. -pub trait StoreItem: Sized { - /// Store `self`. - fn db_put, E: EthSpec>(&self, store: &S, key: &Hash256) -> Result<(), Error>; - - /// Retrieve an instance of `Self` from `store`. - fn db_get, E: EthSpec>(store: &S, key: &Hash256) -> Result, Error>; - - /// Return `true` if an instance of `Self` exists in `store`. - fn db_exists, E: EthSpec>(store: &S, key: &Hash256) -> Result; - - /// Delete an instance of `Self` from `store`. - fn db_delete, E: EthSpec>(store: &S, key: &Hash256) -> Result<(), Error>; -} - -impl StoreItem for T -where - T: SimpleStoreItem, -{ - /// Store `self`. - fn db_put, E: EthSpec>(&self, store: &S, key: &Hash256) -> Result<(), Error> { - let column = Self::db_column().into(); - let key = key.as_bytes(); - - store - .put_bytes(column, key, &self.as_store_bytes()) - .map_err(Into::into) - } - - /// Retrieve an instance of `Self`. - fn db_get, E: EthSpec>(store: &S, key: &Hash256) -> Result, Error> { - let column = Self::db_column().into(); - let key = key.as_bytes(); - - match store.get_bytes(column, key)? { - Some(bytes) => Ok(Some(Self::from_store_bytes(&bytes[..])?)), - None => Ok(None), - } - } - - /// Return `true` if an instance of `Self` exists in `Store`. - fn db_exists, E: EthSpec>(store: &S, key: &Hash256) -> Result { - let column = Self::db_column().into(); - let key = key.as_bytes(); - - store.key_exists(column, key) - } - - /// Delete `self` from the `Store`. - fn db_delete, E: EthSpec>(store: &S, key: &Hash256) -> Result<(), Error> { - let column = Self::db_column().into(); - let key = key.as_bytes(); - - store.key_delete(column, key) - } -} - #[cfg(test)] mod tests { use super::*;