fixed genesis serialization, enabled RPC parameters validation for EX…

…PERIMENTAL_genesis_records, added a test for invalid parameters to EXPERIMENTAL_genesis_records

chain/jsonrpc/Cargo.toml

@@ -17,8 +17,9 @@ chrono = { version = "0.4.4", features = ["serde"] }
 lazy_static = "1.4"
 log = "0.4"
 prometheus = "^0.7"
-serde_json = { version = "1.0", features = ["arbitrary_precision"] }
 serde = { version = "1.0", features = ["derive"] }
+serde_json = "1.0"
+validator = "0.10"
 uuid = { version = "~0.8", features = ["v4"] }
 borsh = "0.2.10"
 

chain/jsonrpc/client/Cargo.toml

@@ -7,9 +7,8 @@ edition = "2018"
 [dependencies]
 actix-web = "2.0.0"
 futures = "0.3"
-serde_derive = "1.0"
+serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
-serde = "1.0"
 uuid = { version = "~0.8", features = ["v4"] }
 
 near-chain-configs = { path = "../../../core/chain-configs" }

chain/jsonrpc/client/src/message.rs

@@ -10,13 +10,11 @@
 //! The main entrypoint here is the [Message](enum.Message.html). The others are just building
 //! blocks and you should generally work with `Message` instead.
 
-#![allow(unused)]
-
 use std::fmt::{Formatter, Result as FmtResult};
 
-use serde::de::{Deserialize, Deserializer, Error, Unexpected, Visitor};
-use serde::ser::{Serialize, SerializeStruct, Serializer};
-use serde_derive::{Deserialize, Serialize};
+use serde::de::{Deserializer, Error, Unexpected, Visitor};
+use serde::ser::{SerializeStruct, Serializer};
+use serde::{Deserialize, Serialize};
 use serde_json::{to_value, Result as JsonResult, Value};
 use uuid::Uuid;
 
@@ -92,8 +90,17 @@ impl RpcError {
         RpcError { code, message, data }
     }
     /// Create an Invalid Param error.
-    pub fn invalid_params(msg: Option<String>) -> Self {
-        RpcError::new(-32_602, "Invalid params".to_owned(), msg.map(Value::String))
+    pub fn invalid_params(data: impl Serialize) -> Self {
+        let value = match to_value(data) {
+            Ok(value) => value,
+            Err(err) => {
+                return Self::server_error(Some(format!(
+                    "Failed to serialize invalid parameters error: {:?}",
+                    err.to_string()
+                )))
+            }
+        };
+        RpcError::new(-32_602, "Invalid params".to_owned(), Some(value))
     }
     /// Create a server error.
     pub fn server_error<E: Serialize>(e: Option<E>) -> Self {
@@ -172,7 +179,6 @@ struct WireResponse {
 // structure that directly corresponds to whatever is on the wire and then convert it to our more
 // convenient representation.
 impl<'de> Deserialize<'de> for Response {
-    #[allow(unreachable_code)] // For that unreachable below
     fn deserialize<D: Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
         let wr: WireResponse = Deserialize::deserialize(deserializer)?;
         let result = match (wr.result, wr.error) {

chain/jsonrpc/src/lib.rs

@@ -15,6 +15,7 @@ use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
 use serde_json::{json, Value};
 use tokio::time::{delay_for, timeout};
+use validator::Validate;
 
 use near_chain_configs::GenesisConfig;
 use near_client::{
@@ -99,9 +100,9 @@ fn from_base64_or_parse_err(encoded: String) -> Result<Vec<u8>, RpcError> {
 fn parse_params<T: DeserializeOwned>(value: Option<Value>) -> Result<T, RpcError> {
     if let Some(value) = value {
         serde_json::from_value(value)
-            .map_err(|err| RpcError::invalid_params(Some(format!("Failed parsing args: {}", err))))
+            .map_err(|err| RpcError::invalid_params(format!("Failed parsing args: {}", err)))
     } else {
-        Err(RpcError::invalid_params(Some("Require at least one parameter".to_owned())))
+        Err(RpcError::invalid_params("Require at least one parameter".to_owned()))
     }
 }
 
@@ -120,7 +121,7 @@ fn parse_tx(params: Option<Value>) -> Result<SignedTransaction, RpcError> {
     let (encoded,) = parse_params::<(String,)>(params)?;
     let bytes = from_base64_or_parse_err(encoded)?;
     SignedTransaction::try_from_slice(&bytes)
-        .map_err(|e| RpcError::invalid_params(Some(format!("Failed to decode transaction: {}", e))))
+        .map_err(|e| RpcError::invalid_params(format!("Failed to decode transaction: {}", e)))
 }
 
 /// A general Server Error
@@ -332,15 +333,15 @@ impl JsonRpcHandler {
         match self.client_addr.send(Status { is_health_check: true }).await {
             Ok(Ok(_)) => Ok(Value::Null),
             Ok(Err(err)) => Err(RpcError::new(-32_001, err, None)),
-            Err(_) => Err(RpcError::server_error::<String>(None)),
+            Err(_) => Err(RpcError::server_error::<()>(None)),
         }
     }
 
     pub async fn status(&self) -> Result<Value, RpcError> {
         match self.client_addr.send(Status { is_health_check: false }).await {
             Ok(Ok(result)) => jsonify(Ok(Ok(result))),
             Ok(Err(err)) => Err(RpcError::new(-32_001, err, None)),
-            Err(_) => Err(RpcError::server_error::<String>(None)),
+            Err(_) => Err(RpcError::server_error::<()>(None)),
         }
     }
 
@@ -363,7 +364,9 @@ impl JsonRpcHandler {
     }
 
     async fn genesis_records(&self, params: Option<Value>) -> Result<Value, RpcError> {
-        let RpcGenesisRecordsRequest { pagination } = parse_params(params)?;
+        let params: RpcGenesisRecordsRequest = parse_params(params)?;
+        params.validate().map_err(RpcError::invalid_params)?;
+        let RpcGenesisRecordsRequest { pagination } = params;
         let mut records = &self.genesis_config.records[..];
         if records.len() < pagination.offset {
             records = &[];
@@ -461,10 +464,7 @@ impl JsonRpcHandler {
     async fn tx_status(&self, params: Option<Value>) -> Result<Value, RpcError> {
         let (hash, account_id) = parse_params::<(String, String)>(params)?;
         if !is_valid_account_id(&account_id) {
-            return Err(RpcError::invalid_params(Some(format!(
-                "Invalid account id: {}",
-                account_id
-            ))));
+            return Err(RpcError::invalid_params(format!("Invalid account id: {}", account_id)));
         }
         let tx_hash = from_base_or_parse_err(hash).and_then(|bytes| {
             CryptoHash::try_from(bytes).map_err(|err| RpcError::parse_error(err.to_string()))

chain/jsonrpc/tests/rpc_query.rs

@@ -431,7 +431,7 @@ fn test_genesis_config() {
                 "dynamic_resharding": false,
                 "epoch_length": 600,
                 "gas_limit": 1000000000000000u64,
-                "min_gas_price": 5000,
+                "min_gas_price": "5000",
                 "block_producer_kickout_threshold": 80,
                 "chunk_producer_kickout_threshold": 60,
                 "gas_price_adjustment_rate": 1,
@@ -656,19 +656,38 @@ fn test_genesis_records() {
     });
 }
 
-/* TODO
 /// Check invalid arguments to genesis records via JSON RPC.
 #[test]
 fn test_invalid_genesis_records_arguments() {
     test_with_client!(client, async move {
-        assert!(client
+        let genesis_records_response = client
             .EXPERIMENTAL_genesis_records(RpcGenesisRecordsRequest {
                 pagination: RpcPagination { offset: 1, limit: 101 },
             })
-            .await
-            .is_err());
+            .await;
+        let validation_error = genesis_records_response.err().unwrap();
+        assert_eq!(validation_error.code, -32_602);
+        assert_eq!(validation_error.message, "Invalid params");
+        assert_eq!(
+            validation_error.data.unwrap(),
+            serde_json::json!({
+                "pagination": {
+                    "limit": [
+                        {
+                            "code": "range",
+                            "message": null,
+                            "params": {
+                                "max": 100.0,
+                                "value": 101,
+                                "min": 1.0,
+                            }
+                        }
+                    ]
+                }
+            })
+        );
     });
-}*/
+}
 
 /// Retrieve gas price
 #[test]

core/chain-configs/src/genesis_config.rs

@@ -11,7 +11,7 @@ use chrono::{DateTime, Utc};
 use serde::{Deserialize, Serialize};
 use smart_default::SmartDefault;
 
-use near_primitives::serialize::u128_dec_format;
+use near_primitives::serialize::{u128_dec_format, u128_dec_format_compatible};
 use near_primitives::state_record::StateRecord;
 use near_primitives::types::{
     AccountId, AccountInfo, Balance, BlockHeightDelta, Gas, NumBlocks, NumSeats,
@@ -49,6 +49,7 @@ pub struct GenesisConfig {
     /// Initial gas limit.
     pub gas_limit: Gas,
     /// Minimum gas price. It is also the initial gas price.
+    #[serde(with = "u128_dec_format_compatible")]
     pub min_gas_price: Balance,
     /// Criterion for kicking out block producers (this is a number between 0 and 100)
     pub block_producer_kickout_threshold: u8,

core/primitives/src/rpc.rs

@@ -17,6 +17,7 @@ pub struct RpcPagination {
 
 #[derive(Serialize, Deserialize, Validate)]
 pub struct RpcGenesisRecordsRequest {
+    #[validate]
     pub pagination: RpcPagination,
 }
 

core/primitives/src/serialize.rs

@@ -290,6 +290,34 @@ pub mod u128_dec_format {
     }
 }
 
+pub mod u128_dec_format_compatible {
+    //! This in an extension to `u128_dec_format` that serves a compatibility layer role to
+    //! deserialize u128 from a "small" JSON number (u64).
+    //!
+    //! It is unfortunate that we cannot enable "arbitrary_precision" feature in serde_json due to
+    //! a bug: https://github.com/serde-rs/json/issues/505
+    use serde::{de, Deserialize, Deserializer};
+
+    pub use super::u128_dec_format::serialize;
+
+    #[derive(Deserialize)]
+    #[serde(untagged)]
+    enum U128 {
+        Number(u64),
+        String(String),
+    }
+
+    pub fn deserialize<'de, D>(deserializer: D) -> Result<u128, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        match U128::deserialize(deserializer)? {
+            U128::Number(value) => Ok(u128::from(value)),
+            U128::String(value) => u128::from_str_radix(&value, 10).map_err(de::Error::custom),
+        }
+    }
+}
+
 pub mod option_u128_dec_format {
     use serde::de;
     use serde::{Deserialize, Deserializer, Serializer};