From 3b0ce354c1f6d4e0100d48f2dc67329f4d557e19 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Wed, 10 Jun 2020 15:37:51 +0200 Subject: [PATCH 1/2] Make NumberOrHex a common primitive. --- client/rpc-api/src/chain/mod.rs | 2 +- client/rpc/src/chain/mod.rs | 34 ++++++++---- client/rpc/src/chain/tests.rs | 2 +- frame/contracts/rpc/src/lib.rs | 28 ++++++++-- primitives/rpc/src/number.rs | 98 +++++++++++++++++++-------------- 5 files changed, 104 insertions(+), 60 deletions(-) diff --git a/client/rpc-api/src/chain/mod.rs b/client/rpc-api/src/chain/mod.rs index a7b26f3024248..753ac5617a29d 100644 --- a/client/rpc-api/src/chain/mod.rs +++ b/client/rpc-api/src/chain/mod.rs @@ -49,7 +49,7 @@ pub trait ChainApi { #[rpc(name = "chain_getBlockHash", alias("chain_getHead"))] fn block_hash( &self, - hash: Option>>, + hash: Option>, ) -> Result>>; /// Get hash of the last finalized block in the canon chain. diff --git a/client/rpc/src/chain/mod.rs b/client/rpc/src/chain/mod.rs index 7b13e7a6005ff..8b6bf19d23565 100644 --- a/client/rpc/src/chain/mod.rs +++ b/client/rpc/src/chain/mod.rs @@ -75,17 +75,27 @@ trait ChainBackend: Send + Sync + 'static /// Get hash of the n-th block in the canon chain. /// /// By default returns latest block hash. - fn block_hash( - &self, - number: Option>>, - ) -> Result> { - Ok(match number { - None => Some(self.client().info().best_hash), - Some(num_or_hex) => self.client() - .header(BlockId::number(num_or_hex.to_number()?)) - .map_err(client_err)? - .map(|h| h.hash()), - }) + fn block_hash(&self, number: Option) -> Result> { + match number { + None => Ok(Some(self.client().info().best_hash)), + Some(num_or_hex) => { + use std::convert::TryInto; + + // FIXME <2329>: Database seems to limit the block number to u32 for no reason + let block_num: u32 = num_or_hex.try_into().map_err(|_| { + Error::from(format!( + "`{:?}` > u32::max_value(), the max block number is u32.", + num_or_hex + )) + })?; + let block_num = >::from(block_num); + Ok(self + .client() + .header(BlockId::number(block_num)) + .map_err(client_err)? + .map(|h| h.hash())) + } + } } /// Get hash of the last finalized block in the canon chain. @@ -233,7 +243,7 @@ impl ChainApi, Block::Hash, Block::Header, Signe fn block_hash( &self, - number: Option>>> + number: Option>, ) -> Result>> { match number { None => self.backend.block_hash(None).map(ListOrValue::Value), diff --git a/client/rpc/src/chain/tests.rs b/client/rpc/src/chain/tests.rs index 68d46135e36b1..b36fc4eab1d86 100644 --- a/client/rpc/src/chain/tests.rs +++ b/client/rpc/src/chain/tests.rs @@ -149,7 +149,7 @@ fn should_return_block_hash() { ); assert_matches!( - api.block_hash(Some(vec![0u64.into(), 1.into(), 2.into()].into())), + api.block_hash(Some(vec![0u64.into(), 1u64.into(), 2u64.into()].into())), Ok(ListOrValue::List(list)) if list == &[client.genesis_hash().into(), block.hash().into(), None] ); } diff --git a/frame/contracts/rpc/src/lib.rs b/frame/contracts/rpc/src/lib.rs index 89f43f42c3ad1..18496c13af9fa 100644 --- a/frame/contracts/rpc/src/lib.rs +++ b/frame/contracts/rpc/src/lib.rs @@ -32,6 +32,7 @@ use sp_runtime::{ generic::BlockId, traits::{Block as BlockT, Header as HeaderT}, }; +use std::convert::TryInto; pub use self::gen_client::Client as ContractsClient; pub use pallet_contracts_rpc_runtime_api::{ @@ -80,7 +81,7 @@ pub struct CallRequest { origin: AccountId, dest: AccountId, value: Balance, - gas_limit: number::NumberOrHex, + gas_limit: number::NumberOrHex, input_data: Bytes, } @@ -203,9 +204,11 @@ where gas_limit, input_data, } = call_request; - let gas_limit = gas_limit.to_number().map_err(|e| Error { + + // Make sure that gas_limit fits into 64 bits. + let gas_limit: u64 = gas_limit.try_into().map_err(|_| Error { code: ErrorCode::InvalidParams, - message: e, + message: format!("{:?} doesn't fit in 64 bit unsigned value", gas_limit), data: None, })?; @@ -282,15 +285,30 @@ fn runtime_error_into_rpc_err(err: impl std::fmt::Debug) -> Error { #[cfg(test)] mod tests { use super::*; + use sp_core::U256; + + #[test] + fn call_request_should_serialize_deserialize_properly() { + type Req = CallRequest; + let req: Req = serde_json::from_str(r#" + { + "origin": "5CiPPseXPECbkjWCa6MnjNokrgYjMqmKndv2rSnekmSK2DjL", + "dest": "5DRakbLVnjVrW6niwLfHGW24EeCEvDAFGEXrtaYS5M4ynoom", + "value": 0, + "gasLimit": 1000000000000, + "inputData": "0x8c97db39" + } + "#).unwrap(); + assert_eq!(req.gas_limit.into_u256(), U256::from(0xe8d4a51000u64)); + } #[test] - fn should_serialize_deserialize_properly() { + fn result_should_serialize_deserialize_properly() { fn test(expected: &str) { let res: RpcContractExecResult = serde_json::from_str(expected).unwrap(); let actual = serde_json::to_string(&res).unwrap(); assert_eq!(actual, expected); } - test(r#"{"success":{"status":5,"data":"0x1234"}}"#); test(r#"{"error":null}"#); } diff --git a/primitives/rpc/src/number.rs b/primitives/rpc/src/number.rs index 63aa643fb6fca..1dd1a92054aa9 100644 --- a/primitives/rpc/src/number.rs +++ b/primitives/rpc/src/number.rs @@ -15,65 +15,80 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Chain RPC Block number type. +//! A number type that can be serialized both as a number or a string that encodes a number in a +//! string. +use std::fmt::Debug; +use std::convert::TryFrom; use serde::{Serialize, Deserialize}; -use std::{convert::TryFrom, fmt::Debug}; use sp_core::U256; -/// RPC Block number type +/// A number type that can be serialized both as a number or a string that encodes a number in a +/// string. /// -/// We allow two representations of the block number as input. -/// Either we deserialize to the type that is specified in the block type -/// or we attempt to parse given hex value. -/// We do that for consistency with the returned type, default generic header -/// serializes block number as hex to avoid overflows in JavaScript. -#[derive(Serialize, Deserialize, Debug, PartialEq)] +/// We allow two representations of the block number as input. Either we deserialize to the type +/// that is specified in the block type or we attempt to parse given hex value. +/// +/// The primary motivation for having this type is to avoid overflows when using big integers in +/// JavaScript (which we consider as an important RPC API consumer). +#[derive(Copy, Clone, Serialize, Deserialize, Debug, PartialEq)] #[serde(untagged)] -pub enum NumberOrHex { - /// The original header number type of block. - Number(Number), - /// Hex representation of the block number. +pub enum NumberOrHex { + /// The number represented directly. + Number(u64), + /// Hex representation of the number. Hex(U256), } -impl + From + Debug + PartialOrd> NumberOrHex { - /// Attempts to convert into concrete block number. - /// - /// Fails in case hex number is too big. - pub fn to_number(self) -> Result { - let num = match self { - NumberOrHex::Number(n) => n, - NumberOrHex::Hex(h) => { - let l = h.low_u64(); - if U256::from(l) != h { - return Err(format!("`{}` does not fit into u64 type; unsupported for now.", h)) - } else { - Number::try_from(l) - .map_err(|_| format!("`{}` does not fit into block number type.", h))? - } - }, - }; - // FIXME <2329>: Database seems to limit the block number to u32 for no reason - if num > Number::from(u32::max_value()) { - return Err(format!("`{:?}` > u32::max_value(), the max block number is u32.", num)) +impl NumberOrHex { + /// Converts this number into an U256. + pub fn into_u256(self) -> U256 { + match self { + NumberOrHex::Number(n) => n.into(), + NumberOrHex::Hex(h) => h, } - Ok(num) } } -impl From for NumberOrHex { +impl From for NumberOrHex { fn from(n: u64) -> Self { NumberOrHex::Number(n) } } -impl From for NumberOrHex { +impl From for NumberOrHex { fn from(n: U256) -> Self { NumberOrHex::Hex(n) } } +/// An error type that signals an out-of-range conversion attempt. +pub struct TryFromIntError(pub(crate) ()); + +impl TryFrom for u32 { + type Error = TryFromIntError; + fn try_from(num_or_hex: NumberOrHex) -> Result { + let num_or_hex = num_or_hex.into_u256(); + if num_or_hex > U256::from(u32::max_value()) { + return Err(TryFromIntError(())); + } else { + Ok(num_or_hex.as_u32()) + } + } +} + +impl TryFrom for u64 { + type Error = TryFromIntError; + fn try_from(num_or_hex: NumberOrHex) -> Result { + let num_or_hex = num_or_hex.into_u256(); + if num_or_hex > U256::from(u64::max_value()) { + return Err(TryFromIntError(())); + } else { + Ok(num_or_hex.as_u64()) + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -81,10 +96,11 @@ mod tests { #[test] fn should_serialize_and_deserialize() { - assert_deser(r#""0x1234""#, NumberOrHex::::Hex(0x1234.into())); - assert_deser(r#""0x0""#, NumberOrHex::::Hex(0.into())); - assert_deser(r#"5"#, NumberOrHex::Number(5_u64)); - assert_deser(r#"10000"#, NumberOrHex::Number(10000_u32)); - assert_deser(r#"0"#, NumberOrHex::Number(0_u16)); + assert_deser(r#""0x1234""#, NumberOrHex::Hex(0x1234.into())); + assert_deser(r#""0x0""#, NumberOrHex::Hex(0.into())); + assert_deser(r#"5"#, NumberOrHex::Number(5)); + assert_deser(r#"10000"#, NumberOrHex::Number(10000)); + assert_deser(r#"0"#, NumberOrHex::Number(0)); + assert_deser(r#"1000000000000"#, NumberOrHex::Number(1000000000000)); } } From 2d4ff110b3a5daf140c6e0e67caa18782b060c95 Mon Sep 17 00:00:00 2001 From: Sergei Shulepov Date: Wed, 10 Jun 2020 16:04:59 +0200 Subject: [PATCH 2/2] Update primitives/rpc/src/number.rs Co-authored-by: Nikolay Volf --- primitives/rpc/src/number.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/primitives/rpc/src/number.rs b/primitives/rpc/src/number.rs index 1dd1a92054aa9..3d7e74753526c 100644 --- a/primitives/rpc/src/number.rs +++ b/primitives/rpc/src/number.rs @@ -18,8 +18,7 @@ //! A number type that can be serialized both as a number or a string that encodes a number in a //! string. -use std::fmt::Debug; -use std::convert::TryFrom; +use std::{convert::TryFrom, fmt::Debug}; use serde::{Serialize, Deserialize}; use sp_core::U256;