Allow default block parameter to be blockHash (#10932)

* allow default block parameter to be blockHash

* requireCanonical

* block check takes precedence over canon check

Author: seunlanlege

ethcore/src/client/client.rs

@@ -1793,6 +1793,10 @@ impl BlockChainClient for Client {
 		self.config.spec_name.clone()
 	}
 
+	fn chain(&self) -> Arc<BlockProvider> {
+		self.chain.read().clone()
+	}
+
 	fn set_spec_name(&self, new_spec_name: String) -> Result<(), ()> {
 		trace!(target: "mode", "Client::set_spec_name({:?})", new_spec_name);
 		if !self.enabled.load(AtomicOrdering::Relaxed) {

ethcore/src/client/test_client.rs

@@ -20,6 +20,7 @@ use std::str::FromStr;
 use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering as AtomicOrder};
 use std::sync::Arc;
 use std::collections::{HashMap, BTreeMap};
+use blockchain::BlockProvider;
 use std::mem;
 
 use blockchain::{TreeRoute, BlockReceipts};
@@ -703,6 +704,10 @@ impl BlockChainClient for TestBlockChainClient {
 		}
 	}
 
+	fn chain(&self) -> Arc<BlockProvider> {
+		unimplemented!()
+	}
+
 	fn list_accounts(&self, _id: BlockId, _after: Option<&Address>, _count: u64) -> Option<Vec<Address>> {
 		None
 	}

ethcore/src/client/traits.rs

@@ -17,7 +17,7 @@
 use std::collections::BTreeMap;
 use std::sync::Arc;
 
-use blockchain::{BlockReceipts, TreeRoute};
+use blockchain::{BlockReceipts, TreeRoute, BlockProvider};
 use bytes::Bytes;
 use call_contract::{CallContract, RegistryInfo};
 use ethcore_miner::pool::VerifiedTransaction;
@@ -238,6 +238,8 @@ pub trait BlockChainClient : Sync + Send + AccountData + BlockChain + CallContra
 			.expect("code will return Some if given BlockId::Latest; qed")
 	}
 
+	fn chain(&self) -> Arc<BlockProvider>;
+
 	/// Get block queue information.
 	fn queue_info(&self) -> BlockQueueInfo;
 

rpc/src/v1/helpers/errors.rs

@@ -609,3 +609,13 @@ pub fn require_experimental(allow_experimental_rpcs: bool, eip: &str) -> Result<
 		})
 	}
 }
+
+/// returns an error for when require_canonical was specified and
+pub fn invalid_input() -> Error {
+	Error {
+		// UNSUPPORTED_REQUEST shares the same error code for EIP-1898
+		code: ErrorCode::ServerError(codes::UNSUPPORTED_REQUEST),
+		message: "Invalid input".into(),
+		data: None
+	}
+}

rpc/src/v1/helpers/light_fetch.rs

@@ -261,6 +261,7 @@ where
 		//       (they don't have state) we can safely fallback to `Latest`.
 		let id = match num.unwrap_or_default() {
 			BlockNumber::Num(n) => BlockId::Number(n),
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 			BlockNumber::Earliest => BlockId::Earliest,
 			BlockNumber::Latest => BlockId::Latest,
 			BlockNumber::Pending => {

rpc/src/v1/impls/eth.rs

@@ -243,6 +243,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T> EthClient<C, SN, S, M, EM> where
 
 			BlockNumberOrId::Number(num) => {
 				let id = match num {
+					BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 					BlockNumber::Latest => BlockId::Latest,
 					BlockNumber::Earliest => BlockId::Earliest,
 					BlockNumber::Num(n) => BlockId::Number(n),
@@ -439,10 +440,10 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T> EthClient<C, SN, S, M, EM> where
 	/// if no state found for the best pending block.
 	fn get_state(&self, number: BlockNumber) -> StateOrBlock {
 		match number {
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash).into(),
 			BlockNumber::Num(num) => BlockId::Number(num).into(),
 			BlockNumber::Earliest => BlockId::Earliest.into(),
 			BlockNumber::Latest => BlockId::Latest.into(),
-
 			BlockNumber::Pending => {
 				let info = self.client.chain_info();
 
@@ -500,10 +501,22 @@ fn check_known<C>(client: &C, number: BlockNumber) -> Result<()> where C: BlockC
 
 	let id = match number {
 		BlockNumber::Pending => return Ok(()),
-
 		BlockNumber::Num(n) => BlockId::Number(n),
 		BlockNumber::Latest => BlockId::Latest,
 		BlockNumber::Earliest => BlockId::Earliest,
+		BlockNumber::Hash { hash, require_canonical } => {
+			// block check takes precedence over canon check.
+			match client.block_status(BlockId::Hash(hash.clone())) {
+				BlockStatus::InChain => {},
+				_ => return Err(errors::unknown_block()),
+			};
+
+			if require_canonical && !client.chain().is_canon(&hash) {
+				return Err(errors::invalid_input())
+			}
+
+			return Ok(())
+		}
 	};
 
 	match client.block_status(id) {
@@ -615,6 +628,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 
 		let num = num.unwrap_or_default();
 		let id = match num {
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 			BlockNumber::Num(n) => BlockId::Number(n),
 			BlockNumber::Earliest => BlockId::Earliest,
 			BlockNumber::Latest => BlockId::Latest,
@@ -789,6 +803,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 
 	fn transaction_by_block_number_and_index(&self, num: BlockNumber, index: Index) -> BoxFuture<Option<Transaction>> {
 		let block_id = match num {
+			BlockNumber::Hash { hash, .. } => PendingOrBlock::Block(BlockId::Hash(hash)),
 			BlockNumber::Latest => PendingOrBlock::Block(BlockId::Latest),
 			BlockNumber::Earliest => PendingOrBlock::Block(BlockId::Earliest),
 			BlockNumber::Num(num) => PendingOrBlock::Block(BlockId::Number(num)),
@@ -825,6 +840,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 
 	fn uncle_by_block_number_and_index(&self, num: BlockNumber, index: Index) -> BoxFuture<Option<RichBlock>> {
 		let id = match num {
+			BlockNumber::Hash { hash, .. } => PendingUncleId { id: PendingOrBlock::Block(BlockId::Hash(hash)), position: index.value() },
 			BlockNumber::Latest => PendingUncleId { id: PendingOrBlock::Block(BlockId::Latest), position: index.value() },
 			BlockNumber::Earliest => PendingUncleId { id: PendingOrBlock::Block(BlockId::Earliest), position: index.value() },
 			BlockNumber::Num(num) => PendingUncleId { id: PendingOrBlock::Block(BlockId::Number(num)), position: index.value() },
@@ -941,12 +957,14 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 		let signed = try_bf!(fake_sign::sign_call(request));
 
 		let num = num.unwrap_or_default();
+		try_bf!(check_known(&*self.client, num.clone()));
 
 		let (mut state, header) =
 			if num == BlockNumber::Pending {
 				self.pending_state_and_header_with_fallback()
 			} else {
 				let id = match num {
+					BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 					BlockNumber::Num(num) => BlockId::Number(num),
 					BlockNumber::Earliest => BlockId::Earliest,
 					BlockNumber::Latest => BlockId::Latest,
@@ -985,6 +1003,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
 			self.pending_state_and_header_with_fallback()
 		} else {
 			let id = match num {
+				BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 				BlockNumber::Num(num) => BlockId::Number(num),
 				BlockNumber::Earliest => BlockId::Earliest,
 				BlockNumber::Latest => BlockId::Latest,

rpc/src/v1/impls/parity.rs

@@ -350,6 +350,7 @@ impl<C, M, U, S> Parity for ParityClient<C, M, U> where
 			(header.encoded(), None)
 		} else {
 			let id = match number {
+				BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 				BlockNumber::Num(num) => BlockId::Number(num),
 				BlockNumber::Earliest => BlockId::Earliest,
 				BlockNumber::Latest => BlockId::Latest,
@@ -381,6 +382,7 @@ impl<C, M, U, S> Parity for ParityClient<C, M, U> where
 					.collect()
 				))
 			},
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 			BlockNumber::Num(num) => BlockId::Number(num),
 			BlockNumber::Earliest => BlockId::Earliest,
 			BlockNumber::Latest => BlockId::Latest,
@@ -412,6 +414,7 @@ impl<C, M, U, S> Parity for ParityClient<C, M, U> where
 			(state, header)
 		} else {
 			let id = match num {
+				BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 				BlockNumber::Num(num) => BlockId::Number(num),
 				BlockNumber::Earliest => BlockId::Earliest,
 				BlockNumber::Latest => BlockId::Latest,

rpc/src/v1/impls/traces.rs

@@ -95,6 +95,7 @@ impl<C, S> Traces for TracesClient<C> where
 		let signed = fake_sign::sign_call(request)?;
 
 		let id = match block {
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 			BlockNumber::Num(num) => BlockId::Number(num),
 			BlockNumber::Earliest => BlockId::Earliest,
 			BlockNumber::Latest => BlockId::Latest,
@@ -122,6 +123,7 @@ impl<C, S> Traces for TracesClient<C> where
 			.collect::<Result<Vec<_>>>()?;
 
 		let id = match block {
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 			BlockNumber::Num(num) => BlockId::Number(num),
 			BlockNumber::Earliest => BlockId::Earliest,
 			BlockNumber::Latest => BlockId::Latest,
@@ -144,6 +146,7 @@ impl<C, S> Traces for TracesClient<C> where
 		let signed = SignedTransaction::new(tx).map_err(errors::transaction)?;
 
 		let id = match block {
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 			BlockNumber::Num(num) => BlockId::Number(num),
 			BlockNumber::Earliest => BlockId::Earliest,
 			BlockNumber::Latest => BlockId::Latest,
@@ -167,6 +170,7 @@ impl<C, S> Traces for TracesClient<C> where
 
 	fn replay_block_transactions(&self, block_number: BlockNumber, flags: TraceOptions) -> Result<Vec<TraceResultsWithTransactionHash>> {
 		let id = match block_number {
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 			BlockNumber::Num(num) => BlockId::Number(num),
 			BlockNumber::Earliest => BlockId::Earliest,
 			BlockNumber::Latest => BlockId::Latest,

rpc/src/v1/types/block_number.rs

@@ -16,12 +16,20 @@
 
 use std::fmt;
 use serde::{Deserialize, Deserializer, Serialize, Serializer};
-use serde::de::{Error, Visitor};
+use serde::de::{Error, Visitor, MapAccess};
 use ethcore::client::BlockId;
+use ethereum_types::H256;
 
 /// Represents rpc api block number param.
 #[derive(Debug, PartialEq, Clone, Hash, Eq)]
 pub enum BlockNumber {
+	/// Hash
+	Hash {
+		/// block hash
+		hash: H256,
+		/// only return blocks part of the canon chain
+		require_canonical: bool,
+	},
 	/// Number
 	Num(u64),
 	/// Latest block
@@ -68,6 +76,7 @@ impl LightBlockNumber for BlockNumber {
 		// Since light clients don't produce pending blocks
 		// (they don't have state) we can safely fallback to `Latest`.
 		match self {
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 			BlockNumber::Num(n) => BlockId::Number(n),
 			BlockNumber::Earliest => BlockId::Earliest,
 			BlockNumber::Latest => BlockId::Latest,
@@ -82,6 +91,9 @@ impl LightBlockNumber for BlockNumber {
 impl Serialize for BlockNumber {
 	fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> where S: Serializer {
 		match *self {
+			BlockNumber::Hash{ hash, require_canonical } => serializer.serialize_str(
+				&format!("{{ 'hash': '{}', 'requireCanonical': '{}'  }}", hash, require_canonical)
+			),
 			BlockNumber::Num(ref x) => serializer.serialize_str(&format!("0x{:x}", x)),
 			BlockNumber::Latest => serializer.serialize_str("latest"),
 			BlockNumber::Earliest => serializer.serialize_str("earliest"),
@@ -99,6 +111,54 @@ impl<'a> Visitor<'a> for BlockNumberVisitor {
 		write!(formatter, "a block number or 'latest', 'earliest' or 'pending'")
 	}
 
+	fn visit_map<V>(self, mut visitor: V) -> Result<Self::Value, V::Error> where V: MapAccess<'a> {
+		let (mut require_canonical, mut block_number, mut block_hash) = (false, None::<u64>, None::<H256>);
+
+		loop {
+			let key_str: Option<String> = visitor.next_key()?;
+
+			match key_str {
+				Some(key) => match key.as_str() {
+					"blockNumber" => {
+						let value: String = visitor.next_value()?;
+						if value.starts_with("0x") {
+							let number = u64::from_str_radix(&value[2..], 16).map_err(|e| {
+								Error::custom(format!("Invalid block number: {}", e))
+							})?;
+
+							block_number = Some(number);
+							break;
+						} else {
+							return Err(Error::custom("Invalid block number: missing 0x prefix".to_string()))
+						}
+					}
+					"blockHash" => {
+						block_hash = Some(visitor.next_value()?);
+					}
+					"requireCanonical" => {
+						require_canonical = visitor.next_value()?;
+					}
+					key => {
+						return Err(Error::custom(format!("Unknown key: {}", key)))
+					}
+				}
+				None => {
+					break
+				}
+			};
+		}
+
+		if let Some(number) = block_number {
+			return Ok(BlockNumber::Num(number))
+		}
+
+		if let Some(hash) = block_hash {
+			return Ok(BlockNumber::Hash { hash, require_canonical })
+		}
+
+		return Err(Error::custom("Invalid input"))
+	}
+
 	fn visit_str<E>(self, value: &str) -> Result<Self::Value, E> where E: Error {
 		match value {
 			"latest" => Ok(BlockNumber::Latest),
@@ -107,7 +167,9 @@ impl<'a> Visitor<'a> for BlockNumberVisitor {
 			_ if value.starts_with("0x") => u64::from_str_radix(&value[2..], 16).map(BlockNumber::Num).map_err(|e| {
 				Error::custom(format!("Invalid block number: {}", e))
 			}),
-			_ => Err(Error::custom("Invalid block number: missing 0x prefix".to_string())),
+			_ => {
+				Err(Error::custom("Invalid block number: missing 0x prefix".to_string()))
+			},
 		}
 	}
 
@@ -119,10 +181,10 @@ impl<'a> Visitor<'a> for BlockNumberVisitor {
 /// Converts `BlockNumber` to `BlockId`, panics on `BlockNumber::Pending`
 pub fn block_number_to_id(number: BlockNumber) -> BlockId {
 	match number {
+		BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 		BlockNumber::Num(num) => BlockId::Number(num),
 		BlockNumber::Earliest => BlockId::Earliest,
 		BlockNumber::Latest => BlockId::Latest,
-
 		BlockNumber::Pending => panic!("`BlockNumber::Pending` should be handled manually")
 	}
 }
@@ -131,19 +193,40 @@ pub fn block_number_to_id(number: BlockNumber) -> BlockId {
 mod tests {
 	use ethcore::client::BlockId;
 	use super::*;
+	use std::str::FromStr;
 	use serde_json;
 
 	#[test]
 	fn block_number_deserialization() {
-		let s = r#"["0xa", "latest", "earliest", "pending"]"#;
+		let s = r#"[
+			"0xa",
+			"latest",
+			"earliest",
+			"pending",
+			{"blockNumber": "0xa"},
+			{"blockHash": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347"},
+			{"blockHash": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347", "requireCanonical": true}
+		]"#;
 		let deserialized: Vec<BlockNumber> = serde_json::from_str(s).unwrap();
-		assert_eq!(deserialized, vec![BlockNumber::Num(10), BlockNumber::Latest, BlockNumber::Earliest, BlockNumber::Pending])
+
+		assert_eq!(
+			deserialized,
+			vec![
+				BlockNumber::Num(10),
+				BlockNumber::Latest,
+				BlockNumber::Earliest,
+				BlockNumber::Pending,
+				BlockNumber::Num(10),
+				BlockNumber::Hash { hash: H256::from_str("1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347").unwrap(), require_canonical: false },
+				BlockNumber::Hash { hash: H256::from_str("1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347").unwrap(), require_canonical: true }
+			]
+		)
 	}
 
 	#[test]
-	fn should_not_deserialize_decimal() {
-		let s = r#""10""#;
-		assert!(serde_json::from_str::<BlockNumber>(s).is_err());
+	fn should_not_deserialize() {
+		let s = r#"[{}, "10"]"#;
+		assert!(serde_json::from_str::<Vec<BlockNumber>>(s).is_err());
 	}
 
 	#[test]

rpc/src/v1/types/filter.rs

@@ -82,6 +82,7 @@ impl Filter {
 		}
 
 		let num_to_id = |num| match num {
+			BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
 			BlockNumber::Num(n) => BlockId::Number(n),
 			BlockNumber::Earliest => BlockId::Earliest,
 			BlockNumber::Latest | BlockNumber::Pending => BlockId::Latest,