diff --git a/substrate/client/rpc-api/src/chain/mod.rs b/substrate/client/rpc-api/src/chain/mod.rs index a7b26f3024248343225c018181b746f86e28a4f1..753ac5617a29d70380e3502d95a2145af8a926a9 100644 --- a/substrate/client/rpc-api/src/chain/mod.rs +++ b/substrate/client/rpc-api/src/chain/mod.rs @@ -49,7 +49,7 @@ pub trait ChainApi<Number, Hash, Header, SignedBlock> { #[rpc(name = "chain_getBlockHash", alias("chain_getHead"))] fn block_hash( &self, - hash: Option<ListOrValue<NumberOrHex<Number>>>, + hash: Option<ListOrValue<NumberOrHex>>, ) -> Result<ListOrValue<Option<Hash>>>; /// Get hash of the last finalized block in the canon chain. diff --git a/substrate/client/rpc/src/chain/mod.rs b/substrate/client/rpc/src/chain/mod.rs index 7b13e7a6005ff9a42aeec1cac02f3f9791d73866..8b6bf19d235659b53d4da23551503d1e60f68467 100644 --- a/substrate/client/rpc/src/chain/mod.rs +++ b/substrate/client/rpc/src/chain/mod.rs @@ -75,17 +75,27 @@ trait ChainBackend<Client, Block: BlockT>: 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<NumberOrHex<NumberFor<Block>>>, - ) -> Result<Option<Block::Hash>> { - 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<NumberOrHex>) -> Result<Option<Block::Hash>> { + 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 = <NumberFor<Block>>::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<Block, Client> ChainApi<NumberFor<Block>, Block::Hash, Block::Header, Signe fn block_hash( &self, - number: Option<ListOrValue<NumberOrHex<NumberFor<Block>>>> + number: Option<ListOrValue<NumberOrHex>>, ) -> Result<ListOrValue<Option<Block::Hash>>> { match number { None => self.backend.block_hash(None).map(ListOrValue::Value), diff --git a/substrate/client/rpc/src/chain/tests.rs b/substrate/client/rpc/src/chain/tests.rs index 68d46135e36b12ae2a29ddd2d1f968767179f933..b36fc4eab1d865f5a3c97e2d615d88cf33955274 100644 --- a/substrate/client/rpc/src/chain/tests.rs +++ b/substrate/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/substrate/frame/contracts/rpc/src/lib.rs b/substrate/frame/contracts/rpc/src/lib.rs index 89f43f42c3ad1b1512f02312742d7f1644140dc0..18496c13af9fa345faadb7ff749c0ada7e06dbe3 100644 --- a/substrate/frame/contracts/rpc/src/lib.rs +++ b/substrate/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<AccountId, Balance> { origin: AccountId, dest: AccountId, value: Balance, - gas_limit: number::NumberOrHex<u64>, + 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<String, u128>; + 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/substrate/primitives/rpc/src/number.rs b/substrate/primitives/rpc/src/number.rs index 63aa643fb6fca6e323842afe94ade5d3cffc9881..3d7e74753526c9a756cdd1a9b9f4d9f0d78a1b6d 100644 --- a/substrate/primitives/rpc/src/number.rs +++ b/substrate/primitives/rpc/src/number.rs @@ -15,65 +15,79 @@ // 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 serde::{Serialize, Deserialize}; use std::{convert::TryFrom, fmt::Debug}; +use serde::{Serialize, Deserialize}; 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 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)] +/// 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<Number> { - /// 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<Number: TryFrom<u64> + From<u32> + Debug + PartialOrd> NumberOrHex<Number> { - /// Attempts to convert into concrete block number. - /// - /// Fails in case hex number is too big. - pub fn to_number(self) -> Result<Number, String> { - 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<u64> for NumberOrHex<u64> { +impl From<u64> for NumberOrHex { fn from(n: u64) -> Self { NumberOrHex::Number(n) } } -impl<Number> From<U256> for NumberOrHex<Number> { +impl From<U256> 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<NumberOrHex> for u32 { + type Error = TryFromIntError; + fn try_from(num_or_hex: NumberOrHex) -> Result<u32, TryFromIntError> { + 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<NumberOrHex> for u64 { + type Error = TryFromIntError; + fn try_from(num_or_hex: NumberOrHex) -> Result<u64, TryFromIntError> { + 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 +95,11 @@ mod tests { #[test] fn should_serialize_and_deserialize() { - assert_deser(r#""0x1234""#, NumberOrHex::<u128>::Hex(0x1234.into())); - assert_deser(r#""0x0""#, NumberOrHex::<u64>::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)); } }