From 336378b7340d7d956361eb8512268cca3b4ab1ed Mon Sep 17 00:00:00 2001 From: Michael Sproul Date: Wed, 30 Sep 2020 10:01:51 +1000 Subject: [PATCH] Clean up and clarify single-column DB keys --- beacon_node/beacon_chain/src/beacon_chain.rs | 22 +++++++++----------- beacon_node/beacon_chain/src/builder.rs | 10 ++++----- beacon_node/beacon_chain/tests/tests.rs | 3 +-- beacon_node/network/src/persisted_dht.rs | 18 +++++++--------- beacon_node/store/src/lib.rs | 2 +- 5 files changed, 24 insertions(+), 31 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 3bf5ae282d4..d189b01e2de 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -66,10 +66,11 @@ pub const ATTESTATION_CACHE_LOCK_TIMEOUT: Duration = Duration::from_secs(1); /// validator pubkey cache. pub const VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT: Duration = Duration::from_secs(1); -pub const BEACON_CHAIN_DB_KEY: [u8; 32] = [0; 32]; -pub const OP_POOL_DB_KEY: [u8; 32] = [0; 32]; -pub const ETH1_CACHE_DB_KEY: [u8; 32] = [0; 32]; -pub const FORK_CHOICE_DB_KEY: [u8; 32] = [0; 32]; +// These keys are all zero because they get stored in different columns, see `DBColumn` type. +pub const BEACON_CHAIN_DB_KEY: Hash256 = Hash256::zero(); +pub const OP_POOL_DB_KEY: Hash256 = Hash256::zero(); +pub const ETH1_CACHE_DB_KEY: Hash256 = Hash256::zero(); +pub const FORK_CHOICE_DB_KEY: Hash256 = Hash256::zero(); /// The result of a chain segment processing. pub enum ChainSegmentResult { @@ -260,7 +261,7 @@ impl BeaconChain { let fork_choice = self.fork_choice.read(); self.store.put_item( - &Hash256::from_slice(&FORK_CHOICE_DB_KEY), + &FORK_CHOICE_DB_KEY, &PersistedForkChoice { fork_choice: fork_choice.to_persisted(), fork_choice_store: fork_choice.fc_store().to_persisted(), @@ -272,8 +273,7 @@ impl BeaconChain { metrics::stop_timer(fork_choice_timer); let head_timer = metrics::start_timer(&metrics::PERSIST_HEAD); - self.store - .put_item(&Hash256::from_slice(&BEACON_CHAIN_DB_KEY), &persisted_head)?; + self.store.put_item(&BEACON_CHAIN_DB_KEY, &persisted_head)?; metrics::stop_timer(head_timer); @@ -290,7 +290,7 @@ impl BeaconChain { let _timer = metrics::start_timer(&metrics::PERSIST_OP_POOL); self.store.put_item( - &Hash256::from_slice(&OP_POOL_DB_KEY), + &OP_POOL_DB_KEY, &PersistedOperationPool::from_operation_pool(&self.op_pool), )?; @@ -302,10 +302,8 @@ impl BeaconChain { let _timer = metrics::start_timer(&metrics::PERSIST_OP_POOL); if let Some(eth1_chain) = self.eth1_chain.as_ref() { - self.store.put_item( - &Hash256::from_slice(Ð1_CACHE_DB_KEY), - ð1_chain.as_ssz_container(), - )?; + self.store + .put_item(Ð1_CACHE_DB_KEY, ð1_chain.as_ssz_container())?; } Ok(()) diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index ff47c7a2b81..5dbabcdd862 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -229,7 +229,7 @@ where .ok_or_else(|| "get_persisted_eth1_backend requires a store.".to_string())?; store - .get_item::(&Hash256::from_slice(Ð1_CACHE_DB_KEY)) + .get_item::(Ð1_CACHE_DB_KEY) .map_err(|e| format!("DB error whilst reading eth1 cache: {:?}", e)) } @@ -241,7 +241,7 @@ where .ok_or_else(|| "store_contains_beacon_chain requires a store.".to_string())?; Ok(store - .get_item::(&Hash256::from_slice(&BEACON_CHAIN_DB_KEY)) + .get_item::(&BEACON_CHAIN_DB_KEY) .map_err(|e| format!("DB error when reading persisted beacon chain: {:?}", e))? .is_some()) } @@ -272,7 +272,7 @@ where .ok_or_else(|| "resume_from_db requires a store.".to_string())?; let chain = store - .get_item::(&Hash256::from_slice(&BEACON_CHAIN_DB_KEY)) + .get_item::(&BEACON_CHAIN_DB_KEY) .map_err(|e| format!("DB error when reading persisted beacon chain: {:?}", e))? .ok_or_else(|| { "No persisted beacon chain found in store. Try purging the beacon chain database." @@ -280,7 +280,7 @@ where })?; let persisted_fork_choice = store - .get_item::(&Hash256::from_slice(&FORK_CHOICE_DB_KEY)) + .get_item::(&FORK_CHOICE_DB_KEY) .map_err(|e| format!("DB error when reading persisted fork choice: {:?}", e))? .ok_or_else(|| "No persisted fork choice present in database.".to_string())?; @@ -307,7 +307,7 @@ where self.op_pool = Some( store - .get_item::>(&Hash256::from_slice(&OP_POOL_DB_KEY)) + .get_item::>(&OP_POOL_DB_KEY) .map_err(|e| format!("DB error whilst reading persisted op pool: {:?}", e))? .map(PersistedOperationPool::into_operation_pool) .unwrap_or_else(OperationPool::new), diff --git a/beacon_node/beacon_chain/tests/tests.rs b/beacon_node/beacon_chain/tests/tests.rs index 721eb409167..cd8b564787c 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -357,11 +357,10 @@ fn roundtrip_operation_pool() { .persist_op_pool() .expect("should persist op pool"); - let key = Hash256::from_slice(&OP_POOL_DB_KEY); let restored_op_pool = harness .chain .store - .get_item::>(&key) + .get_item::>(&OP_POOL_DB_KEY) .expect("should read db") .expect("should find op pool") .into_operation_pool(); diff --git a/beacon_node/network/src/persisted_dht.rs b/beacon_node/network/src/persisted_dht.rs index 2149324422b..c11fcd44852 100644 --- a/beacon_node/network/src/persisted_dht.rs +++ b/beacon_node/network/src/persisted_dht.rs @@ -3,15 +3,14 @@ use std::sync::Arc; use store::{DBColumn, Error as StoreError, HotColdDB, ItemStore, StoreItem}; use types::{EthSpec, Hash256}; -/// 32-byte key for accessing the `DhtEnrs`. -pub const DHT_DB_KEY: &str = "PERSISTEDDHTPERSISTEDDHTPERSISTE"; +/// 32-byte key for accessing the `DhtEnrs`. All zero because `DhtEnrs` has its own column. +pub const DHT_DB_KEY: Hash256 = Hash256::zero(); pub fn load_dht, Cold: ItemStore>( store: Arc>, ) -> Vec { // Load DHT from store - let key = Hash256::from_slice(&DHT_DB_KEY.as_bytes()); - match store.get_item(&key) { + match store.get_item(&DHT_DB_KEY) { Ok(Some(p)) => { let p: PersistedDht = p; p.enrs @@ -25,9 +24,7 @@ pub fn persist_dht, Cold: ItemStore>( store: Arc>, enrs: Vec, ) -> Result<(), store::Error> { - let key = Hash256::from_slice(&DHT_DB_KEY.as_bytes()); - store.put_item(&key, &PersistedDht { enrs })?; - Ok(()) + store.put_item(&DHT_DB_KEY, &PersistedDht { enrs }) } /// Wrapper around DHT for persistence to disk. @@ -61,7 +58,7 @@ mod tests { use std::str::FromStr; use store::config::StoreConfig; use store::{HotColdDB, MemoryStore}; - use types::{ChainSpec, Hash256, MinimalEthSpec}; + use types::{ChainSpec, MinimalEthSpec}; #[test] fn test_persisted_dht() { let log = NullLoggerBuilder.build().unwrap(); @@ -71,11 +68,10 @@ mod tests { MemoryStore, > = HotColdDB::open_ephemeral(StoreConfig::default(), ChainSpec::minimal(), log).unwrap(); let enrs = vec![Enr::from_str("enr:-IS4QHCYrYZbAKWCBRlAy5zzaDZXJBGkcnh4MHcBFZntXNFrdvJjX04jRzjzCBOonrkTfj499SZuOh8R33Ls8RRcy5wBgmlkgnY0gmlwhH8AAAGJc2VjcDI1NmsxoQPKY0yuDUmstAHYpMa2_oxVtw0RW_QAdpzBQA8yWM0xOIN1ZHCCdl8").unwrap()]; - let key = Hash256::from_slice(&DHT_DB_KEY.as_bytes()); store - .put_item(&key, &PersistedDht { enrs: enrs.clone() }) + .put_item(&DHT_DB_KEY, &PersistedDht { enrs: enrs.clone() }) .unwrap(); - let dht: PersistedDht = store.get_item(&key).unwrap().unwrap(); + let dht: PersistedDht = store.get_item(&DHT_DB_KEY).unwrap().unwrap(); assert_eq!(dht.enrs, enrs); } } diff --git a/beacon_node/store/src/lib.rs b/beacon_node/store/src/lib.rs index b872d637d3b..f249be1f897 100644 --- a/beacon_node/store/src/lib.rs +++ b/beacon_node/store/src/lib.rs @@ -154,7 +154,7 @@ pub enum DBColumn { } impl Into<&'static str> for DBColumn { - /// Returns a `&str` that can be used for keying a key-value data base. + /// Returns a `&str` prefix to be added to keys before they hit the key-value database. fn into(self) -> &'static str { match self { DBColumn::BeaconMeta => "bma",