Make NumberOrHex a common primitive. (#6321)

* Make NumberOrHex a common primitive.

* Update primitives/rpc/src/number.rs

Co-authored-by: Nikolay Volf <nikvolf@gmail.com>

Co-authored-by: Nikolay Volf <nikvolf@gmail.com>

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.

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),

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]
 	);
 }

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 should_serialize_deserialize_properly() {
+	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 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}"#);
 	}

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