Miner tweaks (#1797)

* Mining fixes.

- Use queue to determine whether we're mining
- Kick stale hash rates

Fixes #1794
Fixes #1641

* Fix tests.

* Address grumbles.

db/src/database.rs

@@ -25,12 +25,6 @@ use std::mem;
 use ipc::binary::BinaryConvertError;
 use std::collections::{VecDeque, HashMap, BTreeMap};
 
-impl From<String> for Error {
-	fn from(s: String) -> Error {
-		Error::RocksDb(s)
-	}
-}
-
 enum WriteCacheEntry {
 	Remove,
 	Write(Vec<u8>),

db/src/traits.rs

@@ -41,6 +41,12 @@ pub struct KeyValue {
 	UncommitedTransactions,
 }
 
+impl From<String> for Error {
+	fn from(s: String) -> Error {
+		Error::RocksDb(s)
+	}
+}
+
 /// Database configuration
 #[derive(Binary)]
 pub struct DatabaseConfig {

ethcore/src/client/client.rs

@@ -178,15 +178,15 @@ impl Client {
 		db_config.compaction = config.db_compaction.compaction_profile();
 		db_config.wal = config.db_wal;
 
-		let db = Arc::new(Database::open(&db_config, &path.to_str().unwrap()).expect("Error opening database"));
+		let db = Arc::new(try!(Database::open(&db_config, &path.to_str().unwrap()).map_err(ClientError::Database)));
 		let chain = Arc::new(BlockChain::new(config.blockchain, &gb, db.clone()));
 		let tracedb = Arc::new(try!(TraceDB::new(config.tracing, db.clone(), chain.clone())));
 
 		let mut state_db = journaldb::new(db.clone(), config.pruning, DB_COL_STATE);
 		if state_db.is_empty() && spec.ensure_db_good(state_db.as_hashdb_mut()) {
 			let batch = DBTransaction::new(&db);
-			state_db.commit(&batch, 0, &spec.genesis_header().hash(), None).expect("Error commiting genesis state to state DB");
+			try!(state_db.commit(&batch, 0, &spec.genesis_header().hash(), None));
-			db.write(batch).expect("Error writing genesis state to state DB");
+			try!(db.write(batch).map_err(ClientError::Database));
 		}
 
 		if !chain.block_header(&chain.best_block_hash()).map_or(true, |h| state_db.contains(h.state_root())) {

ethcore/src/client/error.rs

@@ -1,4 +1,5 @@
 use trace::Error as TraceError;
+use util::UtilError;
 use std::fmt::{Display, Formatter, Error as FmtError};
 
 /// Client configuration errors.
@@ -6,6 +7,10 @@ use std::fmt::{Display, Formatter, Error as FmtError};
 pub enum Error {
 	/// TraceDB configuration error.
 	Trace(TraceError),
+	/// Database error
+	Database(String),
+	/// Util error
+	Util(UtilError),
 }
 
 impl From<TraceError> for Error {
@@ -14,10 +19,18 @@ impl From<TraceError> for Error {
 	}
 }
 
+impl From<UtilError> for Error {
+	fn from(err: UtilError) -> Self {
+		Error::Util(err)
+	}
+}
+
 impl Display for Error {
 	fn fmt(&self, f: &mut Formatter) -> Result<(), FmtError> {
 		match *self {
-			Error::Trace(ref err) => write!(f, "{}", err)
+			Error::Trace(ref err) => write!(f, "{}", err),
+			Error::Util(ref err) => write!(f, "{}", err),
+			Error::Database(ref s) => write!(f, "Database error: {}", s),
 		}
 	}
 }

ethcore/src/miner/external.rs

@@ -16,7 +16,8 @@
 
 use std::collections::HashMap;
 use std::sync::Arc;
-use util::{RwLock, U256, H256};
+use std::time::{Instant, Duration};
+use util::{Mutex, U256, H256};
 
 /// External miner interface.
 pub trait ExternalMinerService: Send + Sync {
@@ -25,50 +26,50 @@ pub trait ExternalMinerService: Send + Sync {
 
 	/// Total hashrate.
 	fn hashrate(&self) -> U256;
-
-	/// Returns true if external miner is mining.
-	fn is_mining(&self) -> bool;
 }
 
 /// External Miner.
 pub struct ExternalMiner {
-	hashrates: Arc<RwLock<HashMap<H256, U256>>>,
+	hashrates: Arc<Mutex<HashMap<H256, (Instant, U256)>>>,
 }
 
 impl Default for ExternalMiner {
 	fn default() -> Self {
 		ExternalMiner {
-			hashrates: Arc::new(RwLock::new(HashMap::new())),
+			hashrates: Arc::new(Mutex::new(HashMap::new())),
 		}
 	}
 }
 
 impl ExternalMiner {
 	/// Creates new external miner with prefilled hashrates.
-	pub fn new(hashrates: Arc<RwLock<HashMap<H256, U256>>>) -> Self {
+	pub fn new(hashrates: Arc<Mutex<HashMap<H256, (Instant, U256)>>>) -> Self {
 		ExternalMiner {
-			hashrates: hashrates
+			hashrates: hashrates,
 		}
 	}
 }
 
+const ENTRY_TIMEOUT: u64 = 2;
+
 impl ExternalMinerService for ExternalMiner {
 	fn submit_hashrate(&self, hashrate: U256, id: H256) {
-		self.hashrates.write().insert(id, hashrate);
+		self.hashrates.lock().insert(id, (Instant::now() + Duration::from_secs(ENTRY_TIMEOUT), hashrate));
 	}
 
 	fn hashrate(&self) -> U256 {
-		self.hashrates.read().iter().fold(U256::from(0), |sum, (_, v)| sum + *v)
+		let mut hashrates = self.hashrates.lock();
-	}
+		let h = hashrates.drain().filter(|&(_, (t, _))| t > Instant::now()).collect();
-
+		*hashrates = h;
-	fn is_mining(&self) -> bool {
+		hashrates.iter().fold(U256::from(0), |sum, (_, &(_, v))| sum + v)
-		!self.hashrates.read().is_empty()
 	}
 }
 
 #[cfg(test)]
 mod tests {
 	use super::*;
+	use std::thread::sleep;
+	use std::time::Duration;
 	use util::{H256, U256};
 
 	fn miner() -> ExternalMiner {
@@ -76,16 +77,18 @@ mod tests {
 	}
 
 	#[test]
-	fn should_return_that_is_mining_if_there_is_at_least_one_entry() {
+	fn it_should_forget_old_hashrates() {
 		// given
 		let m = miner();
-		assert_eq!(m.is_mining(), false);
+		assert_eq!(m.hashrate(), U256::from(0));
+		m.submit_hashrate(U256::from(10), H256::from(1));
+		assert_eq!(m.hashrate(), U256::from(10));
 
 		// when
-		m.submit_hashrate(U256::from(10), H256::from(1));
+		sleep(Duration::from_secs(3));
 
 		// then
-		assert_eq!(m.is_mining(), true);
+		assert_eq!(m.hashrate(), U256::from(0));
 	}
 
 	#[test]

ethcore/src/miner/miner.rs

@@ -780,6 +780,10 @@ impl MinerService for Miner {
 		}
 	}
 
+	fn is_sealing(&self) -> bool {
+		self.sealing_work.lock().queue.is_in_use()
+	}
+
 	fn map_sealing_work<F, T>(&self, chain: &MiningBlockChainClient, f: F) -> Option<T> where F: FnOnce(&ClosedBlock) -> T {
 		trace!(target: "miner", "map_sealing_work: entering");
 		self.enable_and_prepare_sealing(chain);

ethcore/src/miner/mod.rs

@@ -150,6 +150,9 @@ pub trait MinerService : Send + Sync {
 	/// Returns highest transaction nonce for given address.
 	fn last_nonce(&self, address: &Address) -> Option<U256>;
 
+	/// Is it currently sealing?
+	fn is_sealing(&self) -> bool;
+
 	/// Suggested gas price.
 	fn sensible_gas_price(&self) -> U256 { 20000000000u64.into() }
 

rpc/src/v1/impls/eth.rs

@@ -316,7 +316,7 @@ impl<C, S: ?Sized, M, EM> Eth for EthClient<C, S, M, EM> where
 	fn is_mining(&self, params: Params) -> Result<Value, Error> {
 		try!(self.active());
 		match params {
-			Params::None => to_value(&self.external_miner.is_mining()),
+			Params::None => to_value(&(take_weak!(self.miner).is_sealing())),
 			_ => Err(Error::invalid_params())
 		}
 	}

rpc/src/v1/tests/helpers/miner_service.rs

@@ -205,6 +205,10 @@ impl MinerService for TestMinerService {
 		self.last_nonces.read().get(address).cloned()
 	}
 
+	fn is_sealing(&self) -> bool {
+		false
+	}
+
 	/// Submit `seal` as a valid solution for the header of `pow_hash`.
 	/// Will check the seal, but not actually insert the block into the chain.
 	fn submit_seal(&self, _chain: &MiningBlockChainClient, _pow_hash: H256, _seal: Vec<Bytes>) -> Result<(), Error> {

rpc/src/v1/tests/mocked/eth.rs

@@ -17,10 +17,11 @@
 use std::str::FromStr;
 use std::collections::HashMap;
 use std::sync::Arc;
+use std::time::{Instant, Duration};
 use jsonrpc_core::IoHandler;
 use util::hash::{Address, H256, FixedHash};
 use util::numbers::{Uint, U256};
-use util::RwLock;
+use util::Mutex;
 use ethcore::account_provider::AccountProvider;
 use ethcore::client::{TestBlockChainClient, EachBlockWith, Executed, TransactionID};
 use ethcore::log_entry::{LocalizedLogEntry, LogEntry};
@@ -57,7 +58,7 @@ struct EthTester {
 	pub sync: Arc<TestSyncProvider>,
 	pub accounts_provider: Arc<AccountProvider>,
 	pub miner: Arc<TestMinerService>,
-	hashrates: Arc<RwLock<HashMap<H256, U256>>>,
+	hashrates: Arc<Mutex<HashMap<H256, (Instant, U256)>>>,
 	pub io: IoHandler,
 }
 
@@ -67,7 +68,7 @@ impl Default for EthTester {
 		let sync = sync_provider();
 		let ap = accounts_provider();
 		let miner = miner_service();
-		let hashrates = Arc::new(RwLock::new(HashMap::new()));
+		let hashrates = Arc::new(Mutex::new(HashMap::new()));
 		let external_miner = Arc::new(ExternalMiner::new(hashrates.clone()));
 		let eth = EthClient::new(&client, &sync, &ap, &miner, &external_miner, true).to_delegate();
 		let sign = EthSigningUnsafeClient::new(&client, &ap, &miner).to_delegate();
@@ -133,9 +134,9 @@ fn rpc_eth_syncing() {
 #[test]
 fn rpc_eth_hashrate() {
 	let tester = EthTester::default();
-	tester.hashrates.write().insert(H256::from(0), U256::from(0xfffa));
+	tester.hashrates.lock().insert(H256::from(0), (Instant::now() + Duration::from_secs(2), U256::from(0xfffa)));
-	tester.hashrates.write().insert(H256::from(0), U256::from(0xfffb));
+	tester.hashrates.lock().insert(H256::from(0), (Instant::now() + Duration::from_secs(2), U256::from(0xfffb)));
-	tester.hashrates.write().insert(H256::from(1), U256::from(0x1));
+	tester.hashrates.lock().insert(H256::from(1), (Instant::now() + Duration::from_secs(2), U256::from(0x1)));
 
 	let request = r#"{"jsonrpc": "2.0", "method": "eth_hashrate", "params": [], "id": 1}"#;
 	let response = r#"{"jsonrpc":"2.0","result":"0xfffc","id":1}"#;
@@ -158,8 +159,8 @@ fn rpc_eth_submit_hashrate() {
 	let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#;
 
 	assert_eq!(tester.io.handle_request(request), Some(response.to_owned()));
-	assert_eq!(tester.hashrates.read().get(&H256::from("0x59daa26581d0acd1fce254fb7e85952f4c09d0915afd33d3886cd914bc7d283c")).cloned(),
+	assert_eq!(tester.hashrates.lock().get(&H256::from("0x59daa26581d0acd1fce254fb7e85952f4c09d0915afd33d3886cd914bc7d283c")).cloned().unwrap().1,
-		Some(U256::from(0x500_000)));
+		U256::from(0x500_000));
 }
 
 #[test]
@@ -210,16 +211,11 @@ fn rpc_eth_author() {
 #[test]
 fn rpc_eth_mining() {
 	let tester = EthTester::default();
+	tester.miner.set_author(Address::from_str("d46e8dd67c5d32be8058bb8eb970870f07244567").unwrap());
 
 	let request = r#"{"jsonrpc": "2.0", "method": "eth_mining", "params": [], "id": 1}"#;
 	let response = r#"{"jsonrpc":"2.0","result":false,"id":1}"#;
 	assert_eq!(tester.io.handle_request(request), Some(response.to_owned()));
-
-	tester.hashrates.write().insert(H256::from(1), U256::from(0x1));
-
-	let request = r#"{"jsonrpc": "2.0", "method": "eth_mining", "params": [], "id": 1}"#;
-	let response = r#"{"jsonrpc":"2.0","result":true,"id":1}"#;
-	assert_eq!(tester.io.handle_request(request), Some(response.to_owned()));
 }
 
 #[test]

util/using_queue/src/lib.rs

@@ -71,6 +71,9 @@ impl<T> UsingQueue<T> where T: Clone {
 		self.pending = Some(b);
 	}
 
+	/// Is there anything in the queue currently?
+	pub fn is_in_use(&self) -> bool { self.in_use.len() > 0 }
+
 	/// Clears everything; the queue is entirely reset.
 	pub fn reset(&mut self) {
 		self.pending = None;