Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

Allow default block parameter to be blockHash #10932

Merged
merged 3 commits into from
Aug 6, 2019
Merged

Allow default block parameter to be blockHash #10932

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8a4ced5
allow default block parameter to be blockHash
seunlanlege Aug 1, 2019
ef2367f
requireCanonical
seunlanlege Aug 5, 2019
c2d29c2
block check takes precedence over canon check
seunlanlege Aug 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rpc/src/v1/helpers/light_fetch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,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 => {
Expand Down
10 changes: 8 additions & 2 deletions rpc/src/v1/impls/eth.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> EthClient<C, SN, S

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),
Expand Down Expand Up @@ -433,10 +434,10 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> EthClient<C, SN, S

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();

Expand Down Expand Up @@ -472,7 +473,7 @@ fn check_known<C>(client: &C, number: BlockNumber) -> Result<()> where C: BlockC

let id = match number {
BlockNumber::Pending => return Ok(()),

BlockNumber::Hash(hash) => BlockId::Hash(hash),
BlockNumber::Num(n) => BlockId::Number(n),
BlockNumber::Latest => BlockId::Latest,
BlockNumber::Earliest => BlockId::Earliest,
Expand Down Expand Up @@ -589,6 +590,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,
Expand Down Expand Up @@ -762,6 +764,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)),
Expand Down Expand Up @@ -798,6 +801,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() },
Expand Down Expand Up @@ -923,6 +927,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
(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,
Expand Down Expand Up @@ -964,6 +969,7 @@ impl<C, SN: ?Sized, S: ?Sized, M, EM, T: StateInfo + 'static> Eth for EthClient<
(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,
Expand Down
3 changes: 3 additions & 0 deletions rpc/src/v1/impls/parity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions rpc/src/v1/impls/traces.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
71 changes: 63 additions & 8 deletions rpc/src/v1/types/block_number.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,15 @@

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(H256),
/// Number
Num(u64),
/// Latest block
Expand Down Expand Up @@ -68,6 +71,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,
Expand All @@ -82,6 +86,7 @@ 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) => serializer.serialize_str(&format!("{{ 'hash': '{}' }}", hash)),
BlockNumber::Num(ref x) => serializer.serialize_str(&format!("0x{:x}", x)),
BlockNumber::Latest => serializer.serialize_str("latest"),
BlockNumber::Earliest => serializer.serialize_str("earliest"),
Expand All @@ -99,6 +104,35 @@ 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 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(BlockNumber::Num).map_err(|e| {
Error::custom(format!("Invalid block number: {}", e))
})?;

Ok(number)
} else {
Err(Error::custom("Invalid block number: missing 0x prefix".to_string()))
}
}
"blockHash" => Ok(BlockNumber::Hash(visitor.next_value()?)),
key => {
Err(Error::custom(format!("Unknown key: {}", key)))
}
}
_ => {
// user has sent an empty map
Err(Error::custom(format!("Invalid input, empty object!")))
}
}
}

fn visit_str<E>(self, value: &str) -> Result<Self::Value, E> where E: Error {
match value {
"latest" => Ok(BlockNumber::Latest),
Expand All @@ -107,7 +141,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()))
},
}
}

Expand All @@ -119,10 +155,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")
}
}
Expand All @@ -131,19 +167,38 @@ 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"}
]"#;
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(H256::from_str("1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347").unwrap())
]
)
}

#[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"]"#;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we reject requireCanonical with BlockNumber or any other invalid combinations?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the code does not reject if used with blockNumber instead it is ignored. Do you think we should reject?

Copy link
Member

@ordian ordian Aug 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we derive Serialize/Deserialize (seems possible with serde attributes) instead of manually implementing it, it would be automatically supported. This can be done in a separate PR though.

assert!(serde_json::from_str::<Vec<BlockNumber>>(s).is_err());
}

#[test]
Expand Down
1 change: 1 addition & 0 deletions rpc/src/v1/types/filter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions rpc/src/v1/types/trace_filter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,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,
Expand Down