Move DatabaseExtras back to trace (#10868)

* Move DatabaseExtras back to trace
Add a new BlockChainWithExtras newtype to ethcore
Impl DatabaseExtras for BlockChainWithExtras

* Avoid double Arcs
Impl From for BlockChainWithExtras for convenient instantiation
Change TraceDB::new to take a T: DatabaseExtras (instead of an Arc)

* Use local type for BlockNumber and reduce dependencies

* Update ethcore/src/client/client.rs

Co-Authored-By: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Update ethcore/src/client/client.rs

Co-Authored-By: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

* Revert "Update ethcore/src/client/client.rs"

This reverts commit cbf8a251d4acaff8b29e999aedcdaac473ddf300.

* Revert "Update ethcore/src/client/client.rs"

This reverts commit 2518873b3139e832fa57c11793624bf1bfe48d05.

* address grumbles

* Be explicit about using the BlockProvider provided block_hash()

Cargo.lock

@@ -4339,7 +4339,6 @@ dependencies = [
 name = "trace"
 version = "0.1.0"
 dependencies = [
- "common-types 0.1.0",
  "ethcore 1.12.0",
  "ethcore-blockchain 0.1.0",
  "ethcore-db 0.1.0",

ethcore/blockchain/src/database_extras.rs

@@ -1,53 +0,0 @@
-// Copyright 2015-2019 Parity Technologies (UK) Ltd.
-// This file is part of Parity Ethereum.
-
-// Parity Ethereum is free software: you can redistribute it and/or modify
-// it under the terms of the GNU General Public License as published by
-// the Free Software Foundation, either version 3 of the License, or
-// (at your option) any later version.
-
-// Parity Ethereum is distributed in the hope that it will be useful,
-// but WITHOUT ANY WARRANTY; without even the implied warranty of
-// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-// GNU General Public License for more details.
-
-// You should have received a copy of the GNU General Public License
-// along with Parity Ethereum.  If not, see <http://www.gnu.org/licenses/>.
-
-//! Provides a `DatabaseExtras` trait that defines an interface to query for block data not
-//! contained in a TraceDB.
-
-use common_types::BlockNumber;
-use ethereum_types::H256;
-use ethcore_db::keys::TransactionAddress;
-
-use crate::blockchain::{BlockProvider, BlockChain};
-
-/// `DatabaseExtras` provides an interface to query extra data which is not stored in TraceDB,
-/// but necessary to work correctly.
-pub trait DatabaseExtras {
-	/// Returns hash of given block number.
-	fn block_hash(&self, block_number: BlockNumber) -> Option<H256>;
-
-	/// Returns hash of transaction at given position.
-	fn transaction_hash(&self, block_number: BlockNumber, tx_position: usize) -> Option<H256>;
-}
-
-/// Bridge between TraceDB and Blockchain.
-impl DatabaseExtras for BlockChain {
-	fn block_hash(&self, block_number: BlockNumber) -> Option<H256> {
-		(self as &dyn BlockProvider).block_hash(block_number)
-	}
-
-	fn transaction_hash(&self, block_number: BlockNumber, tx_position: usize) -> Option<H256> {
-		(self as &dyn BlockProvider).block_hash(block_number)
-			.and_then(|block_hash| {
-				let tx_address = TransactionAddress {
-					block_hash,
-					index: tx_position
-				};
-				self.transaction(&tx_address)
-			})
-			.map(|tx| tx.hash())
-	}
-}

ethcore/blockchain/src/lib.rs

@@ -28,7 +28,6 @@ mod cache;
 mod config;
 mod import_route;
 mod update;
-mod database_extras;
 
 pub mod generator;
 
@@ -36,7 +35,6 @@ pub use crate::{
 	blockchain::{BlockProvider, BlockChain, BlockChainDB, BlockChainDBHandler},
 	cache::CacheSize,
 	config::Config,
-	database_extras::DatabaseExtras,
 	import_route::ImportRoute,
 	update::ExtrasInsert,
 };

ethcore/src/client/client.rs

@@ -684,7 +684,7 @@ impl Importer {
 			chain.insert_epoch_transition(&mut batch, header.number(), EpochTransition {
 				block_hash: header.hash(),
 				block_number: header.number(),
-				proof: proof,
+				proof,
 			});
 
 			// always write the batch directly since epoch transition proofs are

ethcore/trace/Cargo.toml

@@ -6,7 +6,6 @@ authors = ["Parity Technologies <admin@parity.io>"]
 edition = "2018"
 
 [dependencies]
-common-types = { path = "../types"}
 ethcore-blockchain = { path = "../blockchain" }
 ethcore-db = { path = "../db" }
 ethereum-types = "0.6"

ethcore/trace/src/db.rs

@@ -18,8 +18,7 @@
 use std::collections::HashMap;
 use std::sync::Arc;
 
-use common_types::BlockNumber;
-use ethcore_blockchain::{BlockChainDB, DatabaseExtras};
+use ethcore_blockchain::{BlockProvider, BlockChainDB, TransactionAddress};
 use ethcore_db::{
 	self as db,
 	cache_manager::CacheManager,
@@ -31,6 +30,7 @@ use parity_util_mem::MallocSizeOfExt;
 use parking_lot::RwLock;
 
 use crate::{
+	BlockNumber,
 	LocalizedTrace, Config, Filter, Database as TraceDatabase, ImportRequest,
 	flat::{FlatTrace, FlatBlockTraces, FlatTransactionTraces},
 };
@@ -57,6 +57,34 @@ impl Key<FlatBlockTraces> for H256 {
 	}
 }
 
+/// `DatabaseExtras` provides an interface to query extra data which is not stored in TraceDB,
+/// but necessary to work correctly.
+pub trait DatabaseExtras {
+	/// Returns hash of given block number.
+	fn block_hash(&self, block_number: BlockNumber) -> Option<H256>;
+
+	/// Returns hash of transaction at given position.
+	fn transaction_hash(&self, block_number: BlockNumber, tx_position: usize) -> Option<H256>;
+}
+
+impl<T: BlockProvider> DatabaseExtras for T {
+	fn block_hash(&self, block_number: BlockNumber) -> Option<H256> {
+		(&*self as &dyn BlockProvider).block_hash(block_number)
+	}
+
+	fn transaction_hash(&self, block_number: BlockNumber, tx_position: usize) -> Option<H256> {
+		self.block_hash(block_number)
+			.and_then(|block_hash| {
+				let tx_address = TransactionAddress {
+					block_hash,
+					index: tx_position
+				};
+				self.transaction(&tx_address)
+			})
+			.map(|tx| tx.hash())
+	}
+}
+
 /// Database to store transaction execution trace.
 ///
 /// Whenever a transaction is executed by EVM it's execution trace is stored
@@ -91,7 +119,7 @@ impl<T> TraceDB<T> where T: DatabaseExtras {
 			cache_manager: RwLock::new(CacheManager::new(config.pref_cache_size, config.max_cache_size, 10 * 1024)),
 			db,
 			enabled: config.enabled,
-			extras: extras,
+			extras,
 		}
 	}
 
@@ -175,8 +203,8 @@ impl<T> TraceDB<T> where T: DatabaseExtras {
 						trace_address: trace.trace_address.into_iter().collect(),
 						transaction_number: trace_tx_number,
 						transaction_hash: trace_tx_hash,
-						block_number: block_number,
-						block_hash: block_hash
+						block_number,
+						block_hash,
 					}),
 					false => None
 				}
@@ -254,8 +282,8 @@ impl<T> TraceDatabase for TraceDB<T> where T: DatabaseExtras {
 						trace_address: trace.trace_address.into_iter().collect(),
 						transaction_number: Some(tx_position),
 						transaction_hash: Some(tx_hash),
-						block_number: block_number,
-						block_hash: block_hash,
+						block_number,
+						block_hash,
 					}
 				})
 			)
@@ -278,8 +306,8 @@ impl<T> TraceDatabase for TraceDB<T> where T: DatabaseExtras {
 						trace_address: trace.trace_address.into_iter().collect(),
 						transaction_number: Some(tx_position),
 						transaction_hash: Some(tx_hash.clone()),
-						block_number: block_number,
-						block_hash: block_hash
+						block_number,
+						block_hash,
 					})
 					.collect()
 				})
@@ -308,8 +336,8 @@ impl<T> TraceDatabase for TraceDB<T> where T: DatabaseExtras {
 									trace_address: trace.trace_address.into_iter().collect(),
 									transaction_number: trace_tx_number,
 									transaction_hash: trace_tx_hash,
-									block_number: block_number,
-									block_hash: block_hash,
+									block_number,
+									block_hash,
 								})
 								.collect::<Vec<LocalizedTrace>>()
 						})
@@ -343,15 +371,14 @@ mod tests {
 		collections::HashMap,
 		sync::Arc,
 	};
-	use common_types::BlockNumber;
-	use ethcore_blockchain::DatabaseExtras;
+
 	use ethcore::test_helpers::new_db;
 	use ethereum_types::{H256, U256, Address};
 	use evm::CallType;
 	use kvdb::DBTransaction;
 
 	use crate::{
-		Config, TraceDB, Database as TraceDatabase, ImportRequest,
+		BlockNumber, Config, TraceDB, Database as TraceDatabase, ImportRequest, DatabaseExtras,
 		Filter, LocalizedTrace, AddressesFilter, TraceError,
 		trace::{Call, Action, Res},
 		flat::{FlatTrace, FlatBlockTraces, FlatTransactionTraces}
@@ -443,7 +470,7 @@ mod tests {
 				result: Res::FailedCall(TraceError::OutOfGas),
 			}])]),
 			block_hash: block_hash.clone(),
-			block_number: block_number,
+			block_number,
 			enacted: vec![block_hash],
 			retracted: 0,
 		}
@@ -465,7 +492,7 @@ mod tests {
 				result: Res::FailedCall(TraceError::OutOfGas),
 			}])]),
 			block_hash: block_hash.clone(),
-			block_number: block_number,
+			block_number,
 			enacted: vec![],
 			retracted: 0,
 		}
@@ -486,8 +513,8 @@ mod tests {
 			subtraces: 0,
 			transaction_number: Some(0),
 			transaction_hash: Some(tx_hash),
-			block_number: block_number,
-			block_hash: block_hash,
+			block_number,
+			block_hash,
 		}
 	}
 

ethcore/trace/src/import.rs

@@ -16,9 +16,8 @@
 
 //! Traces import request.
 use ethereum_types::H256;
-use common_types::BlockNumber;
 
-use crate::FlatBlockTraces;
+use crate::{FlatBlockTraces, BlockNumber};
 
 /// Traces import request.
 pub struct ImportRequest {

ethcore/trace/src/lib.rs

@@ -16,7 +16,6 @@
 
 //! Tracing
 
-use common_types::BlockNumber;
 use ethereum_types::{U256, Address};
 use kvdb::DBTransaction;
 use vm::{Error as VmError, ActionParams};
@@ -32,7 +31,7 @@ mod types;
 
 pub use crate::{
 	config::Config,
-	db::TraceDB,
+	db::{TraceDB, DatabaseExtras},
 	localized::LocalizedTrace,
 	executive_tracer::{ExecutiveTracer, ExecutiveVMTracer},
 	import::ImportRequest,
@@ -47,6 +46,9 @@ pub use crate::{
 	}
 };
 
+/// Type for block number.
+pub(crate) type BlockNumber = u64;
+
 /// This trait is used by executive to build traces.
 pub trait Tracer: Send {
 	/// Data returned when draining the Tracer.

ethcore/trace/src/types/localized.rs

@@ -17,8 +17,10 @@
 //! Localized traces type definitions
 
 use ethereum_types::H256;
-use super::trace::{Action, Res};
-use common_types::BlockNumber;
+use crate::{
+	BlockNumber,
+	trace::{Action, Res}
+};
 
 /// Localized trace.
 #[derive(Debug, PartialEq, Clone)]