From 0706e5468dbd65babd05a6ebd5d40b0251bfb6f9 Mon Sep 17 00:00:00 2001 From: rakita Date: Mon, 11 Jan 2021 11:00:27 +0100 Subject: [PATCH] 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 * Request canonical with BlockHash Co-authored-by: Seun LanLege Co-authored-by: Richard Patel --- ethcore/src/client/client.rs | 4 ++ ethcore/src/client/test_client.rs | 4 ++ ethcore/src/client/traits.rs | 3 + rpc/src/v1/helpers/errors.rs | 10 +++ rpc/src/v1/impls/eth.rs | 26 ++++++++ rpc/src/v1/impls/parity.rs | 3 + rpc/src/v1/impls/traces.rs | 4 ++ rpc/src/v1/types/block_number.rs | 107 ++++++++++++++++++++++++++++-- rpc/src/v1/types/filter.rs | 1 + rpc/src/v1/types/trace_filter.rs | 1 + 10 files changed, 156 insertions(+), 7 deletions(-) diff --git a/ethcore/src/client/client.rs b/ethcore/src/client/client.rs index e7ca0fd6f..c5e8dd7a8 100644 --- a/ethcore/src/client/client.rs +++ b/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) { diff --git a/ethcore/src/client/test_client.rs b/ethcore/src/client/test_client.rs index 2de1b6292..6e284e6c2 100644 --- a/ethcore/src/client/test_client.rs +++ b/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 { match id { BlockId::Number(number) => Some(number), diff --git a/ethcore/src/client/traits.rs b/ethcore/src/client/traits.rs index 69eca8c77..62ce8ef38 100644 --- a/ethcore/src/client/traits.rs +++ b/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. diff --git a/rpc/src/v1/helpers/errors.rs b/rpc/src/v1/helpers/errors.rs index c9d04995f..9d841bd6e 100644 --- a/rpc/src/v1/helpers/errors.rs +++ b/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, + } +} diff --git a/rpc/src/v1/impls/eth.rs b/rpc/src/v1/impls/eth.rs index 2896edf8d..928990d4e 100644 --- a/rpc/src/v1/impls/eth.rs +++ b/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> { 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> { 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, diff --git a/rpc/src/v1/impls/parity.rs b/rpc/src/v1/impls/parity.rs index 893e9a492..3c1c9c762 100644 --- a/rpc/src/v1/impls/parity.rs +++ b/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, diff --git a/rpc/src/v1/impls/traces.rs b/rpc/src/v1/impls/traces.rs index 329e0547e..bf528e252 100644 --- a/rpc/src/v1/impls/traces.rs +++ b/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::>>()?; 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> { 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, diff --git a/rpc/src/v1/types/block_number.rs b/rpc/src/v1/types/block_number.rs index 8e88f49bf..6ac3afb57 100644 --- a/rpc/src/v1/types/block_number.rs +++ b/rpc/src/v1/types/block_number.rs @@ -15,8 +15,9 @@ // along with OpenEthereum. If not, see . 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(self, mut visitor: V) -> Result + where + V: MapAccess<'a>, + { + let (mut require_canonical, mut block_number, mut block_hash) = + (false, None::, None::); + + loop { + let key_str: Option = 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(self, value: &str) -> Result 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 = 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() { - let s = r#""10""#; - assert!(serde_json::from_str::(s).is_err()); + fn should_not_deserialize() { + let s = r#"[{}, "10"]"#; + assert!(serde_json::from_str::>(s).is_err()); } #[test] diff --git a/rpc/src/v1/types/filter.rs b/rpc/src/v1/types/filter.rs index 87c08e3c3..07c841c32 100644 --- a/rpc/src/v1/types/filter.rs +++ b/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, diff --git a/rpc/src/v1/types/trace_filter.rs b/rpc/src/v1/types/trace_filter.rs index 693b4b1bc..50b3ea47f 100644 --- a/rpc/src/v1/types/trace_filter.rs +++ b/rpc/src/v1/types/trace_filter.rs @@ -42,6 +42,7 @@ pub struct TraceFilter { impl Into 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,