Merge pull request #402 from ethcore/jsonrpc2

api changes

ethcore/src/blockchain.rs

@@ -23,24 +23,6 @@ use extras::*;
 use transaction::*;
 use views::*;
 
-/// Uniquely identifies block.
-pub enum BlockId {
-	/// Block's sha3.
-	/// Querying by hash is always faster.
-	Hash(H256),
-	/// Block number within canon blockchain.
-	Number(BlockNumber)
-}
-
-/// Uniquely identifies transaction.
-pub enum TransactionId {
-	/// Transaction's sha3.
-	Hash(H256),
-	/// Block id and transaction index within this block.
-	/// Querying by block position is always faster.
-	Location(BlockId, usize)
-}
-
 /// Represents a tree route between `from` block and `to` block:
 pub struct TreeRoute {
 	/// A vector of hashes of all blocks, ordered from `from` to `to`.
@@ -129,18 +111,8 @@ pub trait BlockProvider {
 	}
 
 	/// Get transaction with given transaction hash.
-	fn transaction(&self, id: TransactionId) -> Option<LocalizedTransaction> {
+	fn transaction(&self, address: &TransactionAddress) -> Option<LocalizedTransaction> {
-		match id {
+		self.block(&address.block_hash).and_then(|bytes| BlockView::new(&bytes).localized_transaction_at(address.index))
-			TransactionId::Hash(ref hash) => self.transaction_address(hash),
-			TransactionId::Location(BlockId::Hash(hash), index) => Some(TransactionAddress {
-				block_hash: hash,
-				index: index
-			}),
-			TransactionId::Location(BlockId::Number(number), index) => self.block_hash(number).map(|hash| TransactionAddress {
-				block_hash: hash,
-				index: index
-			})
-		}.and_then(|address| self.block(&address.block_hash).and_then(|bytes| BlockView::new(&bytes).localized_transaction_at(address.index)))
 	}
 
 	/// Get a list of transactions for a given block.
@@ -887,7 +859,7 @@ mod tests {
 		let transactions = bc.transactions(&b1_hash).unwrap();
 		assert_eq!(transactions.len(), 7);
 		for t in transactions {
-			assert_eq!(bc.transaction(TransactionId::Hash(t.hash())).unwrap(), t);
+			assert_eq!(bc.transaction(&bc.transaction_address(&t.hash()).unwrap()).unwrap(), t);
 		}
 	}
 }

ethcore/src/client.rs

@@ -19,7 +19,7 @@
 use util::*;
 use util::panics::*;
 use rocksdb::{Options, DB, DBCompactionStyle};
-use blockchain::{BlockChain, BlockProvider, CacheSize, TransactionId};
+use blockchain::{BlockChain, BlockProvider, CacheSize};
 use views::BlockView;
 use error::*;
 use header::BlockNumber;
@@ -33,8 +33,33 @@ use env_info::LastHashes;
 use verification::*;
 use block::*;
 use transaction::LocalizedTransaction;
+use extras::TransactionAddress;
 pub use blockchain::TreeRoute;
 
+/// Uniquely identifies block.
+#[derive(Debug, PartialEq, Clone)]
+pub enum BlockId {
+	/// Block's sha3.
+	/// Querying by hash is always faster.
+	Hash(H256),
+	/// Block number within canon blockchain.
+	Number(BlockNumber),
+	/// Earliest block (genesis).
+	Earliest,
+	/// Latest mined block.
+	Latest
+}
+
+/// Uniquely identifies transaction.
+#[derive(Debug, PartialEq, Clone)]
+pub enum TransactionId {
+	/// Transaction's sha3.
+	Hash(H256),
+	/// Block id and transaction index within this block.
+	/// Querying by block position is always faster.
+	Location(BlockId, usize)
+}
+
 /// General block status
 #[derive(Debug, Eq, PartialEq)]
 pub enum BlockStatus {
@@ -71,41 +96,25 @@ impl fmt::Display for BlockChainInfo {
 
 /// Blockchain database client. Owns and manages a blockchain and a block queue.
 pub trait BlockChainClient : Sync + Send {
-	/// Get raw block header data by block header hash.
+	/// Get raw block header data by block id.
-	fn block_header(&self, hash: &H256) -> Option<Bytes>;
+	fn block_header(&self, id: BlockId) -> Option<Bytes>;
 
-	/// Get raw block body data by block header hash.
+	/// Get raw block body data by block id.
 	/// Block body is an RLP list of two items: uncles and transactions.
-	fn block_body(&self, hash: &H256) -> Option<Bytes>;
+	fn block_body(&self, id: BlockId) -> Option<Bytes>;
 
 	/// Get raw block data by block header hash.
-	fn block(&self, hash: &H256) -> Option<Bytes>;
+	fn block(&self, id: BlockId) -> Option<Bytes>;
 
 	/// Get block status by block header hash.
-	fn block_status(&self, hash: &H256) -> BlockStatus;
+	fn block_status(&self, id: BlockId) -> BlockStatus;
 
 	/// Get block total difficulty.
-	fn block_total_difficulty(&self, hash: &H256) -> Option<U256>;
+	fn block_total_difficulty(&self, id: BlockId) -> Option<U256>;
 
 	/// Get address code.
 	fn code(&self, address: &Address) -> Option<Bytes>;
 
-	/// Get raw block header data by block number.
-	fn block_header_at(&self, n: BlockNumber) -> Option<Bytes>;
-
-	/// Get raw block body data by block number.
-	/// Block body is an RLP list of two items: uncles and transactions.
-	fn block_body_at(&self, n: BlockNumber) -> Option<Bytes>;
-
-	/// Get raw block data by block number.
-	fn block_at(&self, n: BlockNumber) -> Option<Bytes>;
-
-	/// Get block status by block number.
-	fn block_status_at(&self, n: BlockNumber) -> BlockStatus;
-
-	/// Get block total difficulty.
-	fn block_total_difficulty_at(&self, n: BlockNumber) -> Option<U256>;
-
 	/// Get transaction with given hash.
 	fn transaction(&self, id: TransactionId) -> Option<LocalizedTransaction>;
 
@@ -133,7 +142,7 @@ pub trait BlockChainClient : Sync + Send {
 
 	/// Get the best block header.
 	fn best_block_header(&self) -> Bytes {
-		self.block_header(&self.chain_info().best_block_hash).unwrap()
+		self.block_header(BlockId::Hash(self.chain_info().best_block_hash)).unwrap()
 	}
 }
 
@@ -340,68 +349,64 @@ impl Client {
 	pub fn configure_cache(&self, pref_cache_size: usize, max_cache_size: usize) {
 		self.chain.write().unwrap().configure_cache(pref_cache_size, max_cache_size);
 	}
+
+	fn block_hash(&self, id: BlockId) -> Option<H256> {
+		match id {
+			BlockId::Hash(hash) => Some(hash),
+			BlockId::Number(number) => self.chain.read().unwrap().block_hash(number),
+			BlockId::Earliest => self.chain.read().unwrap().block_hash(0),
+			BlockId::Latest => Some(self.chain.read().unwrap().best_block_hash())
+		}
+	}
 }
 
 impl BlockChainClient for Client {
-	fn block_header(&self, hash: &H256) -> Option<Bytes> {
+	fn block_header(&self, id: BlockId) -> Option<Bytes> {
-		self.chain.read().unwrap().block(hash).map(|bytes| BlockView::new(&bytes).rlp().at(0).as_raw().to_vec())
+		self.block_hash(id).and_then(|hash| self.chain.read().unwrap().block(&hash).map(|bytes| BlockView::new(&bytes).rlp().at(0).as_raw().to_vec()))
 	}
 
-	fn block_body(&self, hash: &H256) -> Option<Bytes> {
+	fn block_body(&self, id: BlockId) -> Option<Bytes> {
-		self.chain.read().unwrap().block(hash).map(|bytes| {
+		self.block_hash(id).and_then(|hash| {
-			let rlp = Rlp::new(&bytes);
+			self.chain.read().unwrap().block(&hash).map(|bytes| {
-			let mut body = RlpStream::new_list(2);
+				let rlp = Rlp::new(&bytes);
-			body.append_raw(rlp.at(1).as_raw(), 1);
+				let mut body = RlpStream::new_list(2);
-			body.append_raw(rlp.at(2).as_raw(), 1);
+				body.append_raw(rlp.at(1).as_raw(), 1);
-			body.out()
+				body.append_raw(rlp.at(2).as_raw(), 1);
+				body.out()
+			})
 		})
 	}
 
-	fn block(&self, hash: &H256) -> Option<Bytes> {
+	fn block(&self, id: BlockId) -> Option<Bytes> {
-		self.chain.read().unwrap().block(hash)
+		self.block_hash(id).and_then(|hash| {
+			self.chain.read().unwrap().block(&hash)
+		})
 	}
 
-	fn block_status(&self, hash: &H256) -> BlockStatus {
+	fn block_status(&self, id: BlockId) -> BlockStatus {
-		if self.chain.read().unwrap().is_known(&hash) {
+		match self.block_hash(id) {
-			BlockStatus::InChain
+			Some(ref hash) if self.chain.read().unwrap().is_known(hash) => BlockStatus::InChain,
-		} else {
+			Some(hash) => self.block_queue.read().unwrap().block_status(&hash),
-			self.block_queue.read().unwrap().block_status(hash)
+			None => BlockStatus::Unknown
 		}
 	}
 	
-	fn block_total_difficulty(&self, hash: &H256) -> Option<U256> {
+	fn block_total_difficulty(&self, id: BlockId) -> Option<U256> {
-		self.chain.read().unwrap().block_details(hash).map(|d| d.total_difficulty)
+		self.block_hash(id).and_then(|hash| self.chain.read().unwrap().block_details(&hash)).map(|d| d.total_difficulty)
 	}
 
 	fn code(&self, address: &Address) -> Option<Bytes> {
 		self.state().code(address)
 	}
 
-	fn block_header_at(&self, n: BlockNumber) -> Option<Bytes> {
-		self.chain.read().unwrap().block_hash(n).and_then(|h| self.block_header(&h))
-	}
-
-	fn block_body_at(&self, n: BlockNumber) -> Option<Bytes> {
-		self.chain.read().unwrap().block_hash(n).and_then(|h| self.block_body(&h))
-	}
-
-	fn block_at(&self, n: BlockNumber) -> Option<Bytes> {
-		self.chain.read().unwrap().block_hash(n).and_then(|h| self.block(&h))
-	}
-
-	fn block_status_at(&self, n: BlockNumber) -> BlockStatus {
-		match self.chain.read().unwrap().block_hash(n) {
-			Some(h) => self.block_status(&h),
-			None => BlockStatus::Unknown
-		}
-	}
-
-	fn block_total_difficulty_at(&self, n: BlockNumber) -> Option<U256> {
-		self.chain.read().unwrap().block_hash(n).and_then(|h| self.block_total_difficulty(&h))
-	}
-
 	fn transaction(&self, id: TransactionId) -> Option<LocalizedTransaction> {
-		self.chain.read().unwrap().transaction(id)
+		match id {
+			TransactionId::Hash(ref hash) => self.chain.read().unwrap().transaction_address(hash),
+			TransactionId::Location(id, index) => self.block_hash(id).map(|hash| TransactionAddress {
+				block_hash: hash,
+				index: index
+			})
+		}.and_then(|address| self.chain.read().unwrap().transaction(&address))
 	}
 
 	fn tree_route(&self, from: &H256, to: &H256) -> Option<TreeRoute> {
@@ -421,7 +426,7 @@ impl BlockChainClient for Client {
 		if self.chain.read().unwrap().is_known(&header.hash()) {
 			return Err(ImportError::AlreadyInChain);
 		}
-		if self.block_status(&header.parent_hash) == BlockStatus::Unknown {
+		if self.block_status(BlockId::Hash(header.parent_hash)) == BlockStatus::Unknown {
 			return Err(ImportError::UnknownParent);
 		}
 		self.block_queue.write().unwrap().import_block(bytes)

ethcore/src/tests/client.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
-use client::{BlockChainClient,Client};
+use client::{BlockChainClient, Client, BlockId};
 use tests::helpers::*;
 use common::*;
 
@@ -44,7 +44,7 @@ fn imports_good_block() {
 	client.flush_queue();
 	client.import_verified_blocks(&IoChannel::disconnected());
 
-	let block = client.block_header_at(1).unwrap();
+	let block = client.block_header(BlockId::Number(1)).unwrap();
 	assert!(!block.is_empty());
 }
 
@@ -53,7 +53,7 @@ fn query_none_block() {
 	let dir = RandomTempPath::new();
 	let client = Client::new(get_test_spec(), dir.as_path(), IoChannel::disconnected()).unwrap();
 
-    let non_existant = client.block_header_at(188);
+    let non_existant = client.block_header(BlockId::Number(188));
 	assert!(non_existant.is_none());
 }
 
@@ -61,7 +61,7 @@ fn query_none_block() {
 fn query_bad_block() {
 	let client_result = get_test_client_with_blocks(vec![get_bad_state_dummy_block()]);
 	let client = client_result.reference();
-	let bad_block:Option<Bytes> = client.block_header_at(1);
+	let bad_block:Option<Bytes> = client.block_header(BlockId::Number(1));
 
 	assert!(bad_block.is_none());
 }
@@ -82,7 +82,7 @@ fn returns_block_body() {
 	let client_result = get_test_client_with_blocks(vec![dummy_block.clone()]);
 	let client = client_result.reference();
 	let block = BlockView::new(&dummy_block);
-	let body = client.block_body(&block.header().hash()).unwrap();
+	let body = client.block_body(BlockId::Hash(block.header().hash())).unwrap();
 	let body = Rlp::new(&body);
 	assert_eq!(body.item_count(), 2);
 	assert_eq!(body.at(0).as_raw()[..], block.rlp().at(1).as_raw()[..]);
@@ -93,7 +93,7 @@ fn returns_block_body() {
 fn imports_block_sequence() {
 	let client_result = generate_dummy_client(6);
 	let client = client_result.reference();
-	let block = client.block_header_at(5).unwrap();
+	let block = client.block_header(BlockId::Number(5)).unwrap();
 
 	assert!(!block.is_empty());
 }

rpc/src/v1/impls/eth.rs

@@ -23,7 +23,6 @@ use util::uint::*;
 use util::sha3::*;
 use ethcore::client::*;
 use ethcore::views::*;
-use ethcore::blockchain::{BlockId, TransactionId};
 use ethcore::ethereum::denominations::shannon;
 use v1::traits::{Eth, EthFilter};
 use v1::types::{Block, BlockTransactions, BlockNumber, Bytes, SyncStatus, SyncInfo, Transaction, OptionalValue, Index};
@@ -42,6 +41,50 @@ impl EthClient {
 			sync: sync
 		}
 	}
+
+	fn block(&self, id: BlockId, include_txs: bool) -> Result<Value, Error> {
+		match (self.client.block(id.clone()), self.client.block_total_difficulty(id)) {
+			(Some(bytes), Some(total_difficulty)) => {
+				let block_view = BlockView::new(&bytes);
+				let view = block_view.header_view();
+				let block = Block {
+					hash: OptionalValue::Value(view.sha3()),
+					parent_hash: view.parent_hash(),
+					uncles_hash: view.uncles_hash(),
+					author: view.author(),
+					miner: view.author(),
+					state_root: view.state_root(),
+					transactions_root: view.transactions_root(),
+					receipts_root: view.receipts_root(),
+					number: OptionalValue::Value(U256::from(view.number())),
+					gas_used: view.gas_used(),
+					gas_limit: view.gas_limit(),
+					logs_bloom: view.log_bloom(),
+					timestamp: U256::from(view.timestamp()),
+					difficulty: view.difficulty(),
+					total_difficulty: total_difficulty,
+					uncles: vec![],
+					transactions: {
+						if include_txs {
+							BlockTransactions::Full(block_view.localized_transactions().into_iter().map(From::from).collect())
+						} else {
+							BlockTransactions::Hashes(block_view.transaction_hashes())
+						}
+					},
+					extra_data: Bytes::default()
+				};
+				to_value(&block)
+			},
+			_ => Ok(Value::Null)
+		}
+	}
+	
+	fn transaction(&self, id: TransactionId) -> Result<Value, Error> {
+		match self.client.transaction(id) {
+			Some(t) => to_value(&Transaction::from(t)),
+			None => Ok(Value::Null)
+		}
+	}
 }
 
 impl Eth for EthClient {
@@ -110,7 +153,7 @@ impl Eth for EthClient {
 
 	fn block_transaction_count(&self, params: Params) -> Result<Value, Error> {
 		from_params::<(H256,)>(params)
-			.and_then(|(hash,)| match self.client.block(&hash) {
+			.and_then(|(hash,)| match self.client.block(BlockId::Hash(hash)) {
 				Some(bytes) => to_value(&BlockView::new(&bytes).transactions_count()),
 				None => Ok(Value::Null)
 			})
@@ -118,7 +161,7 @@ impl Eth for EthClient {
 
 	fn block_uncles_count(&self, params: Params) -> Result<Value, Error> {
 		from_params::<(H256,)>(params)
-			.and_then(|(hash,)| match self.client.block(&hash) {
+			.and_then(|(hash,)| match self.client.block(BlockId::Hash(hash)) {
 				Some(bytes) => to_value(&BlockView::new(&bytes).uncles_count()),
 				None => Ok(Value::Null)
 			})
@@ -130,62 +173,29 @@ impl Eth for EthClient {
 			.and_then(|(address, _block_number)| to_value(&self.client.code(&address).map_or_else(Bytes::default, Bytes::new)))
 	}
 
-	fn block(&self, params: Params) -> Result<Value, Error> {
+	fn block_by_hash(&self, params: Params) -> Result<Value, Error> {
 		from_params::<(H256, bool)>(params)
-			.and_then(|(hash, include_txs)| match (self.client.block(&hash), self.client.block_total_difficulty(&hash)) {
+			.and_then(|(hash, include_txs)| self.block(BlockId::Hash(hash), include_txs))
-				(Some(bytes), Some(total_difficulty)) => {
+	}
-					let block_view = BlockView::new(&bytes);
+
-					let view = block_view.header_view();
+	fn block_by_number(&self, params: Params) -> Result<Value, Error> {
-					let block = Block {
+		from_params::<(BlockNumber, bool)>(params)
-						hash: OptionalValue::Value(view.sha3()),
+			.and_then(|(number, include_txs)| self.block(number.into(), include_txs))
-						parent_hash: view.parent_hash(),
-						uncles_hash: view.uncles_hash(),
-						author: view.author(),
-						miner: view.author(),
-						state_root: view.state_root(),
-						transactions_root: view.transactions_root(),
-						receipts_root: view.receipts_root(),
-						number: OptionalValue::Value(U256::from(view.number())),
-						gas_used: view.gas_used(),
-						gas_limit: view.gas_limit(),
-						logs_bloom: view.log_bloom(),
-						timestamp: U256::from(view.timestamp()),
-						difficulty: view.difficulty(),
-						total_difficulty: total_difficulty,
-						uncles: vec![],
-						transactions: {
-							if include_txs {
-								BlockTransactions::Full(block_view.localized_transactions().into_iter().map(From::from).collect())
-							} else {
-								BlockTransactions::Hashes(block_view.transaction_hashes())
-							}
-						},
-						extra_data: Bytes::default()
-					};
-					to_value(&block)
-				},
-				_ => Ok(Value::Null)
-			})
 	}
 
 	fn transaction_by_hash(&self, params: Params) -> Result<Value, Error> {
 		from_params::<(H256,)>(params)
-			.and_then(|(hash,)| match self.client.transaction(TransactionId::Hash(hash)) {
+			.and_then(|(hash,)| self.transaction(TransactionId::Hash(hash)))
-				Some(t) => to_value(&Transaction::from(t)),
-				None => Ok(Value::Null)
-			})
 	}
 
 	fn transaction_by_block_hash_and_index(&self, params: Params) -> Result<Value, Error> {
 		from_params::<(H256, Index)>(params)
-			.and_then(|(hash, index)| match self.client.transaction(TransactionId::Location(BlockId::Hash(hash), index.value())) {
+			.and_then(|(hash, index)| self.transaction(TransactionId::Location(BlockId::Hash(hash), index.value())))
-				Some(t) => to_value(&Transaction::from(t)),
-				None => Ok(Value::Null)
-			})
 	}
 
-	fn transaction_by_block_number_and_index(&self, _params: Params) -> Result<Value, Error> {
+	fn transaction_by_block_number_and_index(&self, params: Params) -> Result<Value, Error> {
-		unimplemented!()
+		from_params::<(BlockNumber, Index)>(params)
+			.and_then(|(number, index)| self.transaction(TransactionId::Location(number.into(), index.value())))
 	}
 }
 

rpc/src/v1/traits/eth.rs

@@ -50,8 +50,11 @@ pub trait Eth: Sized + Send + Sync + 'static {
 	/// Returns content of the storage at given address.
 	fn storage_at(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
 
-	/// Returns block with given index / hash.
+	/// Returns block with given hash.
-	fn block(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
+	fn block_by_hash(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
+
+	/// Returns block with given number.
+	fn block_by_number(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
 	
 	/// Returns the number of transactions sent from given address at given time (block number).
 	fn transaction_count(&self, _: Params) -> Result<Value, Error> { rpc_unimplemented!() }
@@ -135,8 +138,8 @@ pub trait Eth: Sized + Send + Sync + 'static {
 		delegate.add_method("eth_sendTransaction", Eth::send_transaction);
 		delegate.add_method("eth_call", Eth::call);
 		delegate.add_method("eth_estimateGas", Eth::estimate_gas);
-		delegate.add_method("eth_getBlockByHash", Eth::block);
+		delegate.add_method("eth_getBlockByHash", Eth::block_by_hash);
-		delegate.add_method("eth_getBlockByNumber", Eth::block);
+		delegate.add_method("eth_getBlockByNumber", Eth::block_by_number);
 		delegate.add_method("eth_getTransactionByHash", Eth::transaction_by_hash);
 		delegate.add_method("eth_getTransactionByBlockHashAndIndex", Eth::transaction_by_block_hash_and_index);
 		delegate.add_method("eth_getTransactionByBlockNumberAndIndex", Eth::transaction_by_block_number_and_index);

rpc/src/v1/types/block_number.rs

@@ -16,6 +16,7 @@
 
 use serde::{Deserialize, Deserializer, Error};
 use serde::de::Visitor;
+use ethcore::client::BlockId;
 
 /// Represents rpc api block number param.
 #[derive(Debug, PartialEq)]
@@ -53,8 +54,20 @@ impl Visitor for BlockNumberVisitor {
 	}
 }
 
+impl Into<BlockId> for BlockNumber {
+	fn into(self) -> BlockId {
+		match self {
+			BlockNumber::Num(n) => BlockId::Number(n),
+			BlockNumber::Earliest => BlockId::Earliest,
+			BlockNumber::Latest => BlockId::Latest,
+			BlockNumber::Pending => BlockId::Latest // TODO: change this once blockid support pending
+		}
+	}
+}
+
 #[cfg(test)]
 mod tests {
+	use ethcore::client::BlockId;
 	use super::*;
 	use serde_json;
 
@@ -64,5 +77,13 @@ mod tests {
 		let deserialized: Vec<BlockNumber> = serde_json::from_str(s).unwrap();
 		assert_eq!(deserialized, vec![BlockNumber::Num(10), BlockNumber::Num(10), BlockNumber::Latest, BlockNumber::Earliest, BlockNumber::Pending])
 	}
+
+	#[test]
+	fn block_number_into() {
+		assert_eq!(BlockId::Number(100), BlockNumber::Num(100).into());
+		assert_eq!(BlockId::Earliest, BlockNumber::Earliest.into());
+		assert_eq!(BlockId::Latest, BlockNumber::Latest.into());
+		assert_eq!(BlockId::Latest, BlockNumber::Pending.into());
+	}
 }
 

sync/src/chain.rs

@@ -33,7 +33,7 @@ use util::*;
 use std::mem::{replace};
 use ethcore::views::{HeaderView};
 use ethcore::header::{BlockNumber, Header as BlockHeader};
-use ethcore::client::{BlockChainClient, BlockStatus};
+use ethcore::client::{BlockChainClient, BlockStatus, BlockId};
 use range_collection::{RangeCollection, ToUsize, FromUsize};
 use ethcore::error::*;
 use ethcore::block::Block;
@@ -331,7 +331,7 @@ impl ChainSync {
 				self.highest_block = Some(number);
 			}
 			let hash = info.hash();
-			match io.chain().block_status(&hash) {
+			match io.chain().block_status(BlockId::Hash(hash.clone())) {
 				BlockStatus::InChain => {
 					self.have_common_block = true;
 					self.last_imported_block = Some(number);
@@ -491,7 +491,7 @@ impl ChainSync {
 		for (rh, rd) in hashes {
 			let h = try!(rh);
 			let d = try!(rd);
-			match io.chain().block_status(&h) {
+			match io.chain().block_status(BlockId::Hash(h.clone())) {
 				BlockStatus::InChain  => {
 					trace!(target: "sync", "New block hash already in chain {:?}", h);
 				},
@@ -877,7 +877,7 @@ impl ChainSync {
 			// id is a hash
 			let hash: H256 = try!(r.val_at(0));
 			trace!(target: "sync", "-> GetBlockHeaders (hash: {}, max: {}, skip: {}, reverse:{})", hash, max_headers, skip, reverse);
-			match io.chain().block_header(&hash) {
+			match io.chain().block_header(BlockId::Hash(hash)) {
 				Some(hdr) => From::from(HeaderView::new(&hdr).number()),
 				None => last
 			}
@@ -897,7 +897,7 @@ impl ChainSync {
 		let mut data = Bytes::new();
 		let inc = (skip + 1) as BlockNumber;
 		while number <= last && number > 0 && count < max_count {
-			if let Some(mut hdr) = io.chain().block_header_at(number) {
+			if let Some(mut hdr) = io.chain().block_header(BlockId::Number(number)) {
 				data.append(&mut hdr);
 				count += 1;
 			}
@@ -929,7 +929,7 @@ impl ChainSync {
 		let mut added = 0usize;
 		let mut data = Bytes::new();
 		for i in 0..count {
-			if let Some(mut hdr) = io.chain().block_body(&try!(r.val_at::<H256>(i))) {
+			if let Some(mut hdr) = io.chain().block_body(BlockId::Hash(try!(r.val_at::<H256>(i)))) {
 				data.append(&mut hdr);
 				added += 1;
 			}
@@ -1060,7 +1060,8 @@ impl ChainSync {
 						let mut rlp_stream = RlpStream::new_list(route.blocks.len());
 						for block_hash in route.blocks {
 							let mut hash_rlp = RlpStream::new_list(2);
-							let difficulty = chain.block_total_difficulty(&block_hash).expect("Mallformed block without a difficulty on the chain!");
+							let difficulty = chain.block_total_difficulty(BlockId::Hash(block_hash.clone())).expect("Mallformed block without a difficulty on the chain!");
+		
 							hash_rlp.append(&block_hash);
 							hash_rlp.append(&difficulty);
 							rlp_stream.append_raw(&hash_rlp.out(), 1);
@@ -1076,7 +1077,7 @@ impl ChainSync {
 	/// creates latest block rlp for the given client
 	fn create_latest_block_rlp(chain: &BlockChainClient) -> Bytes {
 		let mut rlp_stream = RlpStream::new_list(2);
-		rlp_stream.append_raw(&chain.block(&chain.chain_info().best_block_hash).expect("Creating latest block when there is none"), 1);
+		rlp_stream.append_raw(&chain.block(BlockId::Hash(chain.chain_info().best_block_hash)).expect("Creating latest block when there is none"), 1);
 		rlp_stream.append(&chain.chain_info().total_difficulty);
 		rlp_stream.out()
 	}
@@ -1088,10 +1089,10 @@ impl ChainSync {
 		let latest_hash = chain_info.best_block_hash;
 		let latest_number = chain_info.best_block_number;
 		self.peers.iter().filter(|&(_, peer_info)|
-			match io.chain().block_status(&peer_info.latest)
+			match io.chain().block_status(BlockId::Hash(peer_info.latest.clone()))
 			{
 				BlockStatus::InChain => {
-					let peer_number = HeaderView::new(&io.chain().block_header(&peer_info.latest).unwrap()).number();
+					let peer_number = HeaderView::new(&io.chain().block_header(BlockId::Hash(peer_info.latest.clone())).unwrap()).number();
 					peer_info.latest != latest_hash && latest_number > peer_number && latest_number - peer_number < MAX_PEER_LAG_PROPAGATION
 				},
 				_ => false

sync/src/tests/chain.rs

@@ -15,7 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 use util::*;
-use ethcore::client::{BlockChainClient};
+use ethcore::client::{BlockChainClient, BlockId};
 use io::SyncIo;
 use chain::{SyncState};
 use super::helpers::*;
@@ -27,7 +27,7 @@ fn two_peers() {
 	net.peer_mut(1).chain.add_blocks(1000, false);
 	net.peer_mut(2).chain.add_blocks(1000, false);
 	net.sync();
-	assert!(net.peer(0).chain.block_at(1000).is_some());
+	assert!(net.peer(0).chain.block(BlockId::Number(1000)).is_some());
 	assert_eq!(net.peer(0).chain.blocks.read().unwrap().deref(), net.peer(1).chain.blocks.read().unwrap().deref());
 }
 
@@ -60,7 +60,7 @@ fn empty_blocks() {
 		net.peer_mut(2).chain.add_blocks(5, n % 2 == 0);
 	}
 	net.sync();
-	assert!(net.peer(0).chain.block_at(1000).is_some());
+	assert!(net.peer(0).chain.block(BlockId::Number(1000)).is_some());
 	assert_eq!(net.peer(0).chain.blocks.read().unwrap().deref(), net.peer(1).chain.blocks.read().unwrap().deref());
 }
 

sync/src/tests/helpers.rs

@@ -15,7 +15,7 @@
 // along with Parity.  If not, see <http://www.gnu.org/licenses/>.
 
 use util::*;
-use ethcore::client::{BlockChainClient, BlockStatus, TreeRoute, BlockChainInfo};
+use ethcore::client::{BlockChainClient, BlockStatus, TreeRoute, BlockChainInfo, TransactionId, BlockId};
 use ethcore::block_queue::BlockQueueInfo;
 use ethcore::header::{Header as BlockHeader, BlockNumber};
 use ethcore::error::*;
@@ -23,7 +23,6 @@ use io::SyncIo;
 use chain::{ChainSync};
 use ethcore::receipt::Receipt;
 use ethcore::transaction::LocalizedTransaction;
-use ethcore::blockchain::TransactionId;
 
 pub struct TestBlockChainClient {
 	pub blocks: RwLock<HashMap<H256, Bytes>>,
@@ -77,10 +76,19 @@ impl TestBlockChainClient {
 		let index = blocks_read.len() - delta;
 		blocks_read[&index].clone()
 	}
+
+	fn block_hash(&self, id: BlockId) -> Option<H256> {
+		match id {
+			BlockId::Hash(hash) => Some(hash),
+			BlockId::Number(n) => self.numbers.read().unwrap().get(&(n as usize)).cloned(),
+			BlockId::Earliest => self.numbers.read().unwrap().get(&0).cloned(),
+			BlockId::Latest => self.numbers.read().unwrap().get(&(self.numbers.read().unwrap().len() - 1)).cloned()
+		}
+	}
 }
 
 impl BlockChainClient for TestBlockChainClient {
-	fn block_total_difficulty(&self, _h: &H256) -> Option<U256> {
+	fn block_total_difficulty(&self, _id: BlockId) -> Option<U256> {
 		Some(U256::zero())
 	}
 
@@ -92,51 +100,28 @@ impl BlockChainClient for TestBlockChainClient {
 		unimplemented!();
 	}
 
-	fn block_header(&self, h: &H256) -> Option<Bytes> {
+	fn block_header(&self, id: BlockId) -> Option<Bytes> {
-		self.blocks.read().unwrap().get(h).map(|r| Rlp::new(r).at(0).as_raw().to_vec())
+		self.block_hash(id).and_then(|hash| self.blocks.read().unwrap().get(&hash).map(|r| Rlp::new(r).at(0).as_raw().to_vec()))
 	}
 
-	fn block_body(&self, h: &H256) -> Option<Bytes> {
+	fn block_body(&self, id: BlockId) -> Option<Bytes> {
-		self.blocks.read().unwrap().get(h).map(|r| {
+		self.block_hash(id).and_then(|hash| self.blocks.read().unwrap().get(&hash).map(|r| {
 			let mut stream = RlpStream::new_list(2);
 			stream.append_raw(Rlp::new(&r).at(1).as_raw(), 1);
 			stream.append_raw(Rlp::new(&r).at(2).as_raw(), 1);
 			stream.out()
-		})
+		}))
 	}
 
-	fn block(&self, h: &H256) -> Option<Bytes> {
+	fn block(&self, id: BlockId) -> Option<Bytes> {
-		self.blocks.read().unwrap().get(h).cloned()
+		self.block_hash(id).and_then(|hash| self.blocks.read().unwrap().get(&hash).cloned())
 	}
 
-	fn block_status(&self, h: &H256) -> BlockStatus {
+	fn block_status(&self, id: BlockId) -> BlockStatus {
-		match self.blocks.read().unwrap().get(h) {
+		match id {
-			Some(_) => BlockStatus::InChain,
+			BlockId::Number(number) if (number as usize) < self.blocks.read().unwrap().len() => BlockStatus::InChain,
-			None => BlockStatus::Unknown
+			BlockId::Hash(ref hash) if self.blocks.read().unwrap().get(hash).is_some() => BlockStatus::InChain,
-		}
+			_ => BlockStatus::Unknown
-	}
-
-	fn block_total_difficulty_at(&self, _number: BlockNumber) -> Option<U256> {
-		unimplemented!();
-	}
-
-	fn block_header_at(&self, n: BlockNumber) -> Option<Bytes> {
-		self.numbers.read().unwrap().get(&(n as usize)).and_then(|h| self.block_header(h))
-	}
-
-	fn block_body_at(&self, n: BlockNumber) -> Option<Bytes> {
-		self.numbers.read().unwrap().get(&(n as usize)).and_then(|h| self.block_body(h))
-	}
-
-	fn block_at(&self, n: BlockNumber) -> Option<Bytes> {
-		self.numbers.read().unwrap().get(&(n as usize)).map(|h| self.blocks.read().unwrap().get(h).unwrap().clone())
-	}
-
-	fn block_status_at(&self, n: BlockNumber) -> BlockStatus {
-		if (n as usize) < self.blocks.read().unwrap().len() {
-			BlockStatus::InChain
-		} else {
-			BlockStatus::Unknown
 		}
 	}