diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 72850236af..0499f75553 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -7654,6 +7654,7 @@ dependencies = [ "parking_lot 0.10.2", "sp-block-builder", "sp-consensus", + "sp-database", "sp-runtime", "sp-state-machine", ] diff --git a/substrate/client/api/src/leaves.rs b/substrate/client/api/src/leaves.rs index 25f9f3d29b..d10fa7ac0e 100644 --- a/substrate/client/api/src/leaves.rs +++ b/substrate/client/api/src/leaves.rs @@ -314,7 +314,7 @@ mod tests { let mut tx = Transaction::new(); set.prepare_transaction(&mut tx, 0, PREFIX); - db.commit(tx); + db.commit(tx).unwrap(); let set2 = LeafSet::read_from_db(&*db, 0, PREFIX).unwrap(); assert_eq!(set, set2); @@ -348,12 +348,12 @@ mod tests { let mut tx = Transaction::new(); set.prepare_transaction(&mut tx, 0, PREFIX); - db.commit(tx); + db.commit(tx).unwrap(); let _ = set.finalize_height(11); let mut tx = Transaction::new(); set.prepare_transaction(&mut tx, 0, PREFIX); - db.commit(tx); + db.commit(tx).unwrap(); assert!(set.contains(11, 11_1)); assert!(set.contains(11, 11_2)); diff --git a/substrate/client/db/src/cache/mod.rs b/substrate/client/db/src/cache/mod.rs index 2b7cd2e620..5501f0f186 100644 --- a/substrate/client/db/src/cache/mod.rs +++ b/substrate/client/db/src/cache/mod.rs @@ -342,8 +342,9 @@ impl BlockchainCache for DbCacheSync { EntryType::Genesis, )?; let tx_ops = tx.into_ops(); - db.commit(dbtx); + db.commit(dbtx)?; cache.commit(tx_ops)?; + Ok(()) } diff --git a/substrate/client/db/src/changes_tries_storage.rs b/substrate/client/db/src/changes_tries_storage.rs index 958e6e39f4..a2299a8233 100644 --- a/substrate/client/db/src/changes_tries_storage.rs +++ b/substrate/client/db/src/changes_tries_storage.rs @@ -719,7 +719,7 @@ mod tests { None, None, ).unwrap(); - backend.storage.db.commit(tx); + backend.storage.db.commit(tx).unwrap(); backend.changes_tries_storage.post_commit(Some(cache_ops)); }; diff --git a/substrate/client/db/src/children.rs b/substrate/client/db/src/children.rs index 3916321f17..bfba797cd4 100644 --- a/substrate/client/db/src/children.rs +++ b/substrate/client/db/src/children.rs @@ -99,7 +99,7 @@ mod tests { children2.push(1_6); write_children(&mut tx, 0, PREFIX, 1_2, children2); - db.commit(tx.clone()); + db.commit(tx.clone()).unwrap(); let r1: Vec = read_children(&*db, 0, PREFIX, 1_1).expect("(1) Getting r1 failed"); let r2: Vec = read_children(&*db, 0, PREFIX, 1_2).expect("(1) Getting r2 failed"); @@ -108,7 +108,7 @@ mod tests { assert_eq!(r2, vec![1_4, 1_6]); remove_children(&mut tx, 0, PREFIX, 1_2); - db.commit(tx); + db.commit(tx).unwrap(); let r1: Vec = read_children(&*db, 0, PREFIX, 1_1).expect("(2) Getting r1 failed"); let r2: Vec = read_children(&*db, 0, PREFIX, 1_2).expect("(2) Getting r2 failed"); diff --git a/substrate/client/db/src/lib.rs b/substrate/client/db/src/lib.rs index b4f4892a04..7cfde1e1d9 100644 --- a/substrate/client/db/src/lib.rs +++ b/substrate/client/db/src/lib.rs @@ -1243,7 +1243,7 @@ impl Backend { None }; - self.storage.db.commit(transaction); + self.storage.db.commit(transaction)?; if let Some(( number, @@ -1356,7 +1356,7 @@ impl sc_client_api::backend::AuxStore for Backend where Block: Blo for k in delete { transaction.remove(columns::AUX, k); } - self.storage.db.commit(transaction); + self.storage.db.commit(transaction)?; Ok(()) } @@ -1438,7 +1438,7 @@ impl sc_client_api::backend::Backend for Backend { &mut changes_trie_cache_ops, &mut displaced, )?; - self.storage.db.commit(transaction); + self.storage.db.commit(transaction)?; self.blockchain.update_meta(hash, number, is_best, is_finalized); self.changes_tries_storage.post_commit(changes_trie_cache_ops); Ok(()) @@ -1536,7 +1536,7 @@ impl sc_client_api::backend::Backend for Backend { transaction.set_from_vec(columns::META, meta_keys::BEST_BLOCK, key); transaction.remove(columns::KEY_LOOKUP, removed.hash().as_ref()); children::remove_children(&mut transaction, columns::META, meta_keys::CHILDREN_PREFIX, best_hash); - self.storage.db.commit(transaction); + self.storage.db.commit(transaction)?; self.changes_tries_storage.post_commit(Some(changes_trie_cache_ops)); self.blockchain.update_meta(best_hash, best_number, true, update_finalized); } @@ -1555,7 +1555,7 @@ impl sc_client_api::backend::Backend for Backend { leaves.revert(best_hash, best_number); leaves.prepare_transaction(&mut transaction, columns::META, meta_keys::LEAF_PREFIX); - self.storage.db.commit(transaction); + self.storage.db.commit(transaction)?; Ok(()) }; diff --git a/substrate/client/db/src/light.rs b/substrate/client/db/src/light.rs index f115ac9599..3dc6453cd9 100644 --- a/substrate/client/db/src/light.rs +++ b/substrate/client/db/src/light.rs @@ -402,7 +402,8 @@ impl AuxStore for LightStorage for k in delete { transaction.remove(columns::AUX, k); } - self.db.commit(transaction); + self.db.commit(transaction)?; + Ok(()) } @@ -495,7 +496,7 @@ impl Storage for LightStorage debug!("Light DB Commit {:?} ({})", hash, number); - self.db.commit(transaction); + self.db.commit(transaction)?; cache.commit(cache_ops) .expect("only fails if cache with given name isn't loaded yet;\ cache is already loaded because there are cache_ops; qed"); @@ -513,8 +514,9 @@ impl Storage for LightStorage let mut transaction = Transaction::new(); self.set_head_with_transaction(&mut transaction, hash.clone(), (number.clone(), hash.clone()))?; - self.db.commit(transaction); + self.db.commit(transaction)?; self.update_meta(hash, header.number().clone(), true, false); + Ok(()) } else { Err(ClientError::UnknownBlock(format!("Cannot set head {:?}", id))) @@ -552,7 +554,7 @@ impl Storage for LightStorage )? .into_ops(); - self.db.commit(transaction); + self.db.commit(transaction)?; cache.commit(cache_ops) .expect("only fails if cache with given name isn't loaded yet;\ cache is already loaded because there are cache_ops; qed"); diff --git a/substrate/client/db/src/offchain.rs b/substrate/client/db/src/offchain.rs index f6a0925a08..c4f0ce115c 100644 --- a/substrate/client/db/src/offchain.rs +++ b/substrate/client/db/src/offchain.rs @@ -25,6 +25,7 @@ use std::{ use crate::{columns, Database, DbHash, Transaction}; use parking_lot::Mutex; +use log::error; /// Offchain local storage #[derive(Clone)] @@ -64,7 +65,9 @@ impl sp_core::offchain::OffchainStorage for LocalStorage { let mut tx = Transaction::new(); tx.set(columns::OFFCHAIN, &key, value); - self.db.commit(tx); + if let Err(err) = self.db.commit(tx) { + error!("Error setting on local storage: {}", err) + } } fn remove(&mut self, prefix: &[u8], key: &[u8]) { @@ -72,7 +75,9 @@ impl sp_core::offchain::OffchainStorage for LocalStorage { let mut tx = Transaction::new(); tx.remove(columns::OFFCHAIN, &key); - self.db.commit(tx); + if let Err(err) = self.db.commit(tx) { + error!("Error removing on local storage: {}", err) + } } fn get(&self, prefix: &[u8], key: &[u8]) -> Option> { diff --git a/substrate/client/db/src/parity_db.rs b/substrate/client/db/src/parity_db.rs index ad1c6c7656..7085aa3bf8 100644 --- a/substrate/client/db/src/parity_db.rs +++ b/substrate/client/db/src/parity_db.rs @@ -17,7 +17,7 @@ // along with this program. If not, see . /// A `Database` adapter for parity-db. -use sp_database::{Database, Change, Transaction, ColumnId}; +use sp_database::{Database, Change, ColumnId, Transaction, error::DatabaseError}; use crate::utils::NUM_COLUMNS; use crate::columns; @@ -44,7 +44,7 @@ pub fn open(path: &std::path::Path) -> parity_db::Result Database for DbAdapter { - fn commit(&self, transaction: Transaction) { + fn commit(&self, transaction: Transaction) -> Result<(), DatabaseError> { handle_err(self.0.commit(transaction.0.into_iter().map(|change| match change { Change::Set(col, key, value) => (col as u8, key, Some(value)), @@ -52,6 +52,8 @@ impl Database for DbAdapter { _ => unimplemented!(), })) ); + + Ok(()) } fn get(&self, col: ColumnId, key: &[u8]) -> Option> { diff --git a/substrate/client/db/src/utils.rs b/substrate/client/db/src/utils.rs index b531001cf9..c25b978be0 100644 --- a/substrate/client/db/src/utils.rs +++ b/substrate/client/db/src/utils.rs @@ -297,7 +297,7 @@ pub fn check_database_type(db: &dyn Database, db_type: DatabaseType) -> None => { let mut transaction = Transaction::new(); transaction.set(COLUMN_META, meta_keys::TYPE, db_type.as_str().as_bytes()); - db.commit(transaction) + db.commit(transaction)?; }, } diff --git a/substrate/primitives/blockchain/Cargo.toml b/substrate/primitives/blockchain/Cargo.toml index 956ae1a8fc..0ce19cba33 100644 --- a/substrate/primitives/blockchain/Cargo.toml +++ b/substrate/primitives/blockchain/Cargo.toml @@ -22,3 +22,4 @@ sp-consensus = { version = "0.8.0-rc4", path = "../consensus/common" } sp-runtime = { version = "2.0.0-rc4", path = "../runtime" } sp-block-builder = { version = "2.0.0-rc4", path = "../block-builder" } sp-state-machine = { version = "0.8.0-rc4", path = "../state-machine" } +sp-database = { version = "2.0.0-rc4", path = "../database" } diff --git a/substrate/primitives/blockchain/src/error.rs b/substrate/primitives/blockchain/src/error.rs index 17c276d870..bc412e8358 100644 --- a/substrate/primitives/blockchain/src/error.rs +++ b/substrate/primitives/blockchain/src/error.rs @@ -130,6 +130,8 @@ pub enum Error { IncompletePipeline, #[display(fmt = "Transaction pool not ready for block production.")] TransactionPoolNotReady, + #[display(fmt = "Database: {}", _0)] + DatabaseError(sp_database::error::DatabaseError), /// A convenience variant for String #[display(fmt = "{}", _0)] Msg(String), diff --git a/substrate/primitives/database/src/error.rs b/substrate/primitives/database/src/error.rs new file mode 100644 index 0000000000..2e5d4557a9 --- /dev/null +++ b/substrate/primitives/database/src/error.rs @@ -0,0 +1,35 @@ +// This file is part of Substrate. + +// Copyright (C) 2020 Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +/// The error type for database operations. +#[derive(Debug)] +pub struct DatabaseError(pub Box); + +impl std::fmt::Display for DatabaseError { + fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl std::error::Error for DatabaseError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + None + } +} + +/// A specialized `Result` type for database operations. +pub type Result = std::result::Result; diff --git a/substrate/primitives/database/src/kvdb.rs b/substrate/primitives/database/src/kvdb.rs index e05320deed..f436979aaf 100644 --- a/substrate/primitives/database/src/kvdb.rs +++ b/substrate/primitives/database/src/kvdb.rs @@ -19,7 +19,7 @@ use ::kvdb::{DBTransaction, KeyValueDB}; -use crate::{Database, Change, Transaction, ColumnId}; +use crate::{Database, Change, ColumnId, Transaction, error}; struct DbAdapter(D); @@ -38,7 +38,7 @@ pub fn as_database(db: D) -> std::sync::Arc Database for DbAdapter { - fn commit(&self, transaction: Transaction) { + fn commit(&self, transaction: Transaction) -> error::Result<()> { let mut tx = DBTransaction::new(); for change in transaction.0.into_iter() { match change { @@ -47,7 +47,7 @@ impl Database for DbAdapter { _ => unimplemented!(), } } - handle_err(self.0.write(tx)); + self.0.write(tx).map_err(|e| error::DatabaseError(Box::new(e))) } fn get(&self, col: ColumnId, key: &[u8]) -> Option> { diff --git a/substrate/primitives/database/src/lib.rs b/substrate/primitives/database/src/lib.rs index 1fb7b15666..1908eb49bb 100644 --- a/substrate/primitives/database/src/lib.rs +++ b/substrate/primitives/database/src/lib.rs @@ -17,6 +17,7 @@ //! The main database trait, allowing Substrate to store data persistently. +pub mod error; mod mem; mod kvdb; @@ -82,20 +83,22 @@ impl Transaction { pub trait Database: Send + Sync { /// Commit the `transaction` to the database atomically. Any further calls to `get` or `lookup` /// will reflect the new state. - fn commit(&self, transaction: Transaction) { + fn commit(&self, transaction: Transaction) -> error::Result<()> { for change in transaction.0.into_iter() { match change { Change::Set(col, key, value) => self.set(col, &key, &value), Change::Remove(col, key) => self.remove(col, &key), Change::Store(hash, preimage) => self.store(&hash, &preimage), Change::Release(hash) => self.release(&hash), - } + }?; } + + Ok(()) } /// Commit the `transaction` to the database atomically. Any further calls to `get` or `lookup` /// will reflect the new state. - fn commit_ref<'a>(&self, transaction: &mut dyn Iterator>) { + fn commit_ref<'a>(&self, transaction: &mut dyn Iterator>) -> error::Result<()> { let mut tx = Transaction::new(); for change in transaction { match change { @@ -105,13 +108,13 @@ pub trait Database: Send + Sync { ChangeRef::Release(hash) => tx.release(hash), } } - self.commit(tx); + self.commit(tx) } /// Retrieve the value previously stored against `key` or `None` if /// `key` is not currently in the database. fn get(&self, col: ColumnId, key: &[u8]) -> Option>; - + /// Call `f` with the value previously stored against `key`. /// /// This may be faster than `get` since it doesn't allocate. @@ -119,24 +122,24 @@ pub trait Database: Send + Sync { fn with_get(&self, col: ColumnId, key: &[u8], f: &mut dyn FnMut(&[u8])) { self.get(col, key).map(|v| f(&v)); } - + /// Set the value of `key` in `col` to `value`, replacing anything that is there currently. - fn set(&self, col: ColumnId, key: &[u8], value: &[u8]) { + fn set(&self, col: ColumnId, key: &[u8], value: &[u8]) -> error::Result<()> { let mut t = Transaction::new(); t.set(col, key, value); - self.commit(t); + self.commit(t) } /// Remove the value of `key` in `col`. - fn remove(&self, col: ColumnId, key: &[u8]) { + fn remove(&self, col: ColumnId, key: &[u8]) -> error::Result<()> { let mut t = Transaction::new(); t.remove(col, key); - self.commit(t); + self.commit(t) } /// Retrieve the first preimage previously `store`d for `hash` or `None` if no preimage is /// currently stored. fn lookup(&self, hash: &H) -> Option>; - + /// Call `f` with the preimage stored for `hash` and return the result, or `None` if no preimage /// is currently stored. /// @@ -145,23 +148,23 @@ pub trait Database: Send + Sync { fn with_lookup(&self, hash: &H, f: &mut dyn FnMut(&[u8])) { self.lookup(hash).map(|v| f(&v)); } - + /// Store the `preimage` of `hash` into the database, so that it may be looked up later with /// `Database::lookup`. This may be called multiple times, but `Database::lookup` but subsequent /// calls will ignore `preimage` and simply increase the number of references on `hash`. - fn store(&self, hash: &H, preimage: &[u8]) { + fn store(&self, hash: &H, preimage: &[u8]) -> error::Result<()> { let mut t = Transaction::new(); t.store(hash.clone(), preimage); - self.commit(t); + self.commit(t) } - + /// Release the preimage of `hash` from the database. An equal number of these to the number of /// corresponding `store`s must have been given before it is legal for `Database::lookup` to /// be unable to provide the preimage. - fn release(&self, hash: &H) { + fn release(&self, hash: &H) -> error::Result<()> { let mut t = Transaction::new(); t.release(hash.clone()); - self.commit(t); + self.commit(t) } } diff --git a/substrate/primitives/database/src/mem.rs b/substrate/primitives/database/src/mem.rs index cbfc4f31d9..51cb854334 100644 --- a/substrate/primitives/database/src/mem.rs +++ b/substrate/primitives/database/src/mem.rs @@ -18,7 +18,7 @@ //! In-memory implementation of `Database` use std::collections::HashMap; -use crate::{Database, Transaction, ColumnId, Change}; +use crate::{Database, Change, ColumnId, Transaction, error}; use parking_lot::RwLock; #[derive(Default)] @@ -29,7 +29,7 @@ pub struct MemDb Database for MemDb where H: Clone + Send + Sync + Eq + PartialEq + Default + std::hash::Hash { - fn commit(&self, transaction: Transaction) { + fn commit(&self, transaction: Transaction) -> error::Result<()> { let mut s = self.0.write(); for change in transaction.0.into_iter() { match change { @@ -39,6 +39,8 @@ impl Database for MemDb Change::Release(hash) => { s.1.remove(&hash); }, } } + + Ok(()) } fn get(&self, col: ColumnId, key: &[u8]) -> Option> {