EIP-1898: Allow default block parameter to be blockHash (#203)

* Allow default block parameter to be blockHash

Backport to 3.1 of https://github.com/openethereum/openethereum/pull/10932

Co-authored-by: Richard Patel <me@terorie.dev>

* Request canonical with BlockHash

Co-authored-by: Seun LanLege <seunlanlege@gmail.com>
Co-authored-by: Richard Patel <me@terorie.dev>

ethcore/src/client/client.rs

@@ -2069,6 +2069,10 @@ impl BlockChainClient for Client {
         self.config.spec_name.clone()
     }
 
+    fn is_canon(&self, hash: &H256) -> bool {
+        self.chain.read().is_canon(hash)
+    }
+
     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

@@ -847,6 +847,10 @@ impl BlockChainClient for TestBlockChainClient {
         unimplemented!();
     }
 
+    fn is_canon(&self, _hash: &H256) -> bool {
+        unimplemented!()
+    }
+
     fn block_number(&self, id: BlockId) -> Option<BlockNumber> {
         match id {
             BlockId::Number(number) => Some(number),

ethcore/src/client/traits.rs

@@ -271,6 +271,9 @@ pub trait BlockChainClient:
             .expect("code will return Some if given BlockId::Latest; qed")
     }
 
+    /// Returns true if the given block is known and in the canon chain.
+    fn is_canon(&self, hash: &H256) -> bool;
+
     /// Get address code hash at given block's state.
 
     /// Get value of the storage at given position at the given block's state.

rpc/src/v1/helpers/errors.rs

@@ -530,3 +530,13 @@ pub fn require_experimental(allow_experimental_rpcs: bool, eip: &str) -> Result<
 		})
     }
 }
+
+/// returns an error for when require_canonical was specified in RPC for EIP-1898
+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/impls/eth.rs

@@ -243,6 +243,7 @@ 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),
@@ -469,6 +470,7 @@ 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(),
@@ -539,6 +541,22 @@ where
         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.is_canon(&hash) {
+                return Err(errors::invalid_input());
+            }
+
+            return Ok(());
+        }
     };
 
     match client.block_status(id) {
@@ -688,6 +706,7 @@ where
 
         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,
@@ -893,6 +912,7 @@ where
         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)),
@@ -942,6 +962,10 @@ where
         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(),
@@ -1092,6 +1116,7 @@ where
             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,
@@ -1132,6 +1157,7 @@ where
             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

@@ -368,6 +368,7 @@ 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,
@@ -401,6 +402,7 @@ where
                     .ok_or_else(errors::unknown_block));
                 return Box::new(future::ok(receipts.into_iter().map(Into::into).collect()));
             }
+            BlockNumber::Hash { hash, .. } => BlockId::Hash(hash),
             BlockNumber::Num(num) => BlockId::Number(num),
             BlockNumber::Earliest => BlockId::Earliest,
             BlockNumber::Latest => BlockId::Latest,
@@ -434,6 +436,7 @@ 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

@@ -111,6 +111,7 @@ 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,
@@ -157,6 +158,7 @@ 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,
@@ -198,6 +200,7 @@ 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,
@@ -247,6 +250,7 @@ where
         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

@@ -15,8 +15,9 @@
 // along with OpenEthereum.  If not, see <http://www.gnu.org/licenses/>.
 
 use ethcore::client::BlockId;
+use hash::H256;
 use serde::{
-    de::{Error, Visitor},
+    de::{Error, MapAccess, Visitor},
     Deserialize, Deserializer, Serialize, Serializer,
 };
 use std::fmt;
@@ -24,6 +25,13 @@ use std::fmt;
 /// 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
@@ -65,6 +73,13 @@ impl Serialize for BlockNumber {
         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"),
@@ -85,6 +100,59 @@ impl<'a> Visitor<'a> for BlockNumberVisitor {
         )
     }
 
+    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,
@@ -113,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"),
     }
 }
@@ -126,26 +194,51 @@ mod tests {
     use super::*;
     use ethcore::client::BlockId;
     use serde_json;
+    use std::str::FromStr;
 
     #[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
+                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() {
+    fn should_not_deserialize() {
-        let s = r#""10""#;
+        let s = r#"[{}, "10"]"#;
-        assert!(serde_json::from_str::<BlockNumber>(s).is_err());
+        assert!(serde_json::from_str::<Vec<BlockNumber>>(s).is_err());
     }
 
     #[test]

rpc/src/v1/types/filter.rs

@@ -97,6 +97,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,

rpc/src/v1/types/trace_filter.rs

@@ -42,6 +42,7 @@ pub struct TraceFilter {
 impl Into<client::TraceFilter> for TraceFilter {
     fn into(self) -> client::TraceFilter {
         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 => BlockId::Latest,