json: remove deprecated old style path+data request format (near#7601)

This has been deprecated at least since August 2020 (see near#3237), isn’t documented and it’s probably fair to say that anyone who cared migrated to JSON RPC requests. As a side effect, this allows us to get rid of from_base58 function.
mina86 · Sep 9, 2022 · be8a1ea · be8a1ea
1 parent 25eb491
commit be8a1ea
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 158 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,12 +23,15 @@
   `near_peer_message_sent_by_type_total` Prometheus metrics measuring
   size and number of messages sent to peers.
 * `near_peer_message_received_total` Prometheus metric is now deprecated.
-  Instead of it aggregate `near_peer_message_received_by_type_total` metric
-  instead.  For example, to get total rate of received messages use
+  Instead of it aggregate `near_peer_message_received_by_type_total` metric.
+  For example, to get total rate of received messages use
   `sum(rate(near_peer_message_received_by_type_total{...}[5m]))`.
 * Backtraces on panics are enabled by default, so you no longer need to set
   `RUST_BACKTRACE=1` environmental variable. To disable backtraces, set
   `RUST_BACKTRACE=0`.
+* A `[path, data]` JSON RPC query format has been removed.  It has been
+  deprecated for over two years and not documented anywhere.  Use proper
+  structured queries with `rquest_type` set instead.
 
 ## 1.28.0 [2022-07-27]
 

diff --git a/chain/jsonrpc/client/src/lib.rs b/chain/jsonrpc/client/src/lib.rs
@@ -202,16 +202,6 @@ jsonrpc_client!(pub struct JsonRpcClient {
 });
 
 impl JsonRpcClient {
-    /// This is a soft-deprecated method to do query RPC request with a path and data positional
-    /// parameters.
-    pub fn query_by_path(
-        &self,
-        path: String,
-        data: String,
-    ) -> RpcRequest<near_jsonrpc_primitives::types::query::RpcQueryResponse> {
-        call_method(&self.client, &self.server_addr, "query", [path, data])
-    }
-
     pub fn query(
         &self,
         request: near_jsonrpc_primitives::types::query::RpcQueryRequest,

diff --git a/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs b/chain/jsonrpc/jsonrpc-tests/tests/rpc_query.rs
@@ -127,29 +127,6 @@ fn test_chunk_invalid_shard_id() {
     });
 }
 
-/// Connect to json rpc and query account info with soft-deprecated query API.
-#[test]
-fn test_query_by_path_account() {
-    test_with_client!(test_utils::NodeType::NonValidator, client, async move {
-        let status = client.status().await.unwrap();
-        let block_hash = status.sync_info.latest_block_hash;
-        let query_response =
-            client.query_by_path("account/test".to_string(), "".to_string()).await.unwrap();
-        assert_eq!(query_response.block_height, 0);
-        assert_eq!(query_response.block_hash, block_hash);
-        let account_info = if let QueryResponseKind::ViewAccount(account) = query_response.kind {
-            account
-        } else {
-            panic!("queried account, but received something else: {:?}", query_response.kind);
-        };
-        assert_eq!(account_info.amount, 0);
-        assert_eq!(account_info.code_hash.as_ref(), &[0; 32]);
-        assert_eq!(account_info.locked, 0);
-        assert_eq!(account_info.storage_paid_at, 0);
-        assert_eq!(account_info.storage_usage, 0);
-    });
-}
-
 /// Connect to json rpc and query account info.
 #[test]
 fn test_query_account() {
@@ -196,25 +173,6 @@ fn test_query_account() {
     });
 }
 
-/// Connect to json rpc and query account info with soft-deprecated query API.
-#[test]
-fn test_query_by_path_access_keys() {
-    test_with_client!(test_utils::NodeType::NonValidator, client, async move {
-        let query_response =
-            client.query_by_path("access_key/test".to_string(), "".to_string()).await.unwrap();
-        assert_eq!(query_response.block_height, 0);
-        let access_keys = if let QueryResponseKind::AccessKeyList(access_keys) = query_response.kind
-        {
-            access_keys
-        } else {
-            panic!("queried access keys, but received something else: {:?}", query_response.kind);
-        };
-        assert_eq!(access_keys.keys.len(), 1);
-        assert_eq!(access_keys.keys[0].access_key, AccessKey::full_access().into());
-        assert_eq!(access_keys.keys[0].public_key, PublicKey::empty(KeyType::ED25519));
-    });
-}
-
 /// Connect to json rpc and query account info.
 #[test]
 fn test_query_access_keys() {
@@ -239,28 +197,6 @@ fn test_query_access_keys() {
     });
 }
 
-/// Connect to json rpc and query account info with soft-deprecated query API.
-#[test]
-fn test_query_by_path_access_key() {
-    test_with_client!(test_utils::NodeType::NonValidator, client, async move {
-        let query_response = client
-            .query_by_path(
-                "access_key/test/ed25519:23vYngy8iL7q94jby3gszBnZ9JptpMf5Hgf7KVVa2yQ2".to_string(),
-                "".to_string(),
-            )
-            .await
-            .unwrap();
-        assert_eq!(query_response.block_height, 0);
-        let access_key = if let QueryResponseKind::AccessKey(access_keys) = query_response.kind {
-            access_keys
-        } else {
-            panic!("queried access keys, but received something else: {:?}", query_response.kind);
-        };
-        assert_eq!(access_key.nonce, 0);
-        assert_eq!(access_key.permission, AccessKeyPermission::FullAccess.into());
-    });
-}
-
 /// Connect to json rpc and query account info.
 #[test]
 fn test_query_access_key() {

diff --git a/chain/jsonrpc/src/api/query.rs b/chain/jsonrpc/src/api/query.rs
@@ -3,72 +3,13 @@ use serde_json::Value;
 use near_client_primitives::types::QueryError;
 use near_jsonrpc_primitives::errors::RpcParseError;
 use near_jsonrpc_primitives::types::query::{RpcQueryError, RpcQueryRequest, RpcQueryResponse};
-use near_primitives::serialize;
-use near_primitives::types::BlockReference;
-use near_primitives::views::{QueryRequest, QueryResponse};
+use near_primitives::views::QueryResponse;
 
 use super::{parse_params, RpcFrom, RpcRequest};
 
-/// Max size of the query path (soft-deprecated)
-const QUERY_DATA_MAX_SIZE: usize = 10 * 1024;
-
 impl RpcRequest for RpcQueryRequest {
     fn parse(value: Option<Value>) -> Result<Self, RpcParseError> {
-        let params = parse_params::<(String, String)>(value.clone());
-        let query_request = if let Ok((path, data)) = params {
-            // Handle a soft-deprecated version of the query API, which is based on
-            // positional arguments with a "path"-style first argument.
-            //
-            // This whole block can be removed one day, when the new API is 100% adopted.
-            let data =
-                serialize::from_base58(&data).map_err(|err| RpcParseError(err.to_string()))?;
-            let query_data_size = path.len() + data.len();
-            if query_data_size > QUERY_DATA_MAX_SIZE {
-                return Err(RpcParseError(format!(
-                    "Query data size {} is too large",
-                    query_data_size
-                )));
-            }
-
-            let mut path_parts = path.splitn(3, '/');
-            let make_err = || RpcParseError("Not enough query parameters provided".to_string());
-            let query_command = path_parts.next().ok_or_else(make_err)?;
-            let account_id = path_parts
-                .next()
-                .ok_or_else(make_err)?
-                .parse()
-                .map_err(|err| RpcParseError(format!("{}", err)))?;
-            let maybe_extra_arg = path_parts.next();
-
-            let request = match query_command {
-                "account" => QueryRequest::ViewAccount { account_id },
-                "access_key" => match maybe_extra_arg {
-                    None => QueryRequest::ViewAccessKeyList { account_id },
-                    Some(pk) => QueryRequest::ViewAccessKey {
-                        account_id,
-                        public_key: pk
-                            .parse()
-                            .map_err(|_| RpcParseError("Invalid public key".to_string()))?,
-                    },
-                },
-                "code" => QueryRequest::ViewCode { account_id },
-                "contract" => QueryRequest::ViewState { account_id, prefix: data.into() },
-                "call" => match maybe_extra_arg {
-                    Some(method_name) => QueryRequest::CallFunction {
-                        account_id,
-                        method_name: method_name.to_string(),
-                        args: data.into(),
-                    },
-                    None => return Err(RpcParseError("Method name is missing".to_string())),
-                },
-                _ => return Err(RpcParseError(format!("Unknown path {}", query_command))),
-            };
-            // Use Finality::None here to make backward compatibility tests work
-            Self { request, block_reference: BlockReference::latest() }
-        } else {
-            parse_params::<Self>(value)?
-        };
-        Ok(query_request)
+        Ok(parse_params::<Self>(value)?)
     }
 }
 

diff --git a/core/primitives-core/src/serialize.rs b/core/primitives-core/src/serialize.rs
@@ -2,10 +2,6 @@ pub fn to_base58<T: AsRef<[u8]>>(input: T) -> String {
     bs58::encode(input).into_string()
 }
 
-pub fn from_base58(s: &str) -> Result<Vec<u8>, Box<dyn std::error::Error + Send + Sync>> {
-    bs58::decode(s).into_vec().map_err(|err| err.into())
-}
-
 pub fn to_base64<T: AsRef<[u8]>>(input: T) -> String {
     base64::encode(&input)
 }

diff --git a/integration-tests/src/user/rpc_user.rs b/integration-tests/src/user/rpc_user.rs
@@ -10,17 +10,18 @@ use near_crypto::{PublicKey, Signer};
 use near_jsonrpc::client::{new_client, JsonRpcClient};
 use near_jsonrpc_client::ChunkId;
 use near_jsonrpc_primitives::errors::ServerError;
-use near_jsonrpc_primitives::types::query::RpcQueryResponse;
+use near_jsonrpc_primitives::types::query::{RpcQueryRequest, RpcQueryResponse};
 use near_primitives::hash::CryptoHash;
 use near_primitives::receipt::Receipt;
-use near_primitives::serialize::{to_base58, to_base64};
+use near_primitives::serialize::to_base64;
 use near_primitives::transaction::SignedTransaction;
 use near_primitives::types::{
     AccountId, BlockHeight, BlockId, BlockReference, MaybeBlockId, ShardId,
 };
 use near_primitives::views::{
     AccessKeyView, AccountView, BlockView, CallResult, ChunkView, ContractCodeView,
-    EpochValidatorInfo, ExecutionOutcomeView, FinalExecutionOutcomeView, ViewStateResult,
+    EpochValidatorInfo, ExecutionOutcomeView, FinalExecutionOutcomeView, QueryRequest,
+    ViewStateResult,
 };
 
 use crate::user::User;
@@ -50,9 +51,9 @@ impl RpcUser {
         self.actix(|client| client.status()).ok()
     }
 
-    pub fn query(&self, path: String, data: &[u8]) -> Result<RpcQueryResponse, String> {
-        let data = to_base58(data);
-        self.actix(move |client| client.query_by_path(path, data).map_err(|err| err.to_string()))
+    pub fn query(&self, request: QueryRequest) -> Result<RpcQueryResponse, String> {
+        let request = RpcQueryRequest { request, block_reference: BlockReference::latest() };
+        self.actix(move |client| client.query(request).map_err(|err| err.to_string()))
     }
 
     pub fn validators(&self, block_id: MaybeBlockId) -> Result<EpochValidatorInfo, String> {
@@ -62,8 +63,8 @@ impl RpcUser {
 
 impl User for RpcUser {
     fn view_account(&self, account_id: &AccountId) -> Result<AccountView, String> {
-        let query_response = self.query(format!("account/{}", account_id), &[])?;
-        match query_response.kind {
+        let query = QueryRequest::ViewAccount { account_id: account_id.clone() };
+        match self.query(query)?.kind {
             near_jsonrpc_primitives::types::query::QueryResponseKind::ViewAccount(account_view) => {
                 Ok(account_view)
             }
@@ -72,8 +73,11 @@ impl User for RpcUser {
     }
 
     fn view_state(&self, account_id: &AccountId, prefix: &[u8]) -> Result<ViewStateResult, String> {
-        let query_response = self.query(format!("contract/{}", account_id), prefix)?;
-        match query_response.kind {
+        let query = QueryRequest::ViewState {
+            account_id: account_id.clone(),
+            prefix: prefix.to_vec().into(),
+        };
+        match self.query(query)?.kind {
             near_jsonrpc_primitives::types::query::QueryResponseKind::ViewState(
                 view_state_result,
             ) => Ok(view_state_result),
@@ -82,8 +86,8 @@ impl User for RpcUser {
     }
 
     fn view_contract_code(&self, account_id: &AccountId) -> Result<ContractCodeView, String> {
-        let query_response = self.query(format!("code/{}", account_id), &[])?;
-        match query_response.kind {
+        let query = QueryRequest::ViewCode { account_id: account_id.clone() };
+        match self.query(query)?.kind {
             near_jsonrpc_primitives::types::query::QueryResponseKind::ViewCode(
                 contract_code_view,
             ) => Ok(contract_code_view),
@@ -97,8 +101,12 @@ impl User for RpcUser {
         method_name: &str,
         args: &[u8],
     ) -> Result<CallResult, String> {
-        let query_response = self.query(format!("call/{}/{}", account_id, method_name), args)?;
-        match query_response.kind {
+        let query = QueryRequest::CallFunction {
+            account_id: account_id.clone(),
+            method_name: method_name.to_string(),
+            args: args.to_vec().into(),
+        };
+        match self.query(query)?.kind {
             near_jsonrpc_primitives::types::query::QueryResponseKind::CallResult(call_result) => {
                 Ok(call_result)
             }
@@ -185,9 +193,11 @@ impl User for RpcUser {
         account_id: &AccountId,
         public_key: &PublicKey,
     ) -> Result<AccessKeyView, String> {
-        let query_response =
-            self.query(format!("access_key/{}/{}", account_id, public_key), &[])?;
-        match query_response.kind {
+        let query = QueryRequest::ViewAccessKey {
+            account_id: account_id.clone(),
+            public_key: public_key.clone(),
+        };
+        match self.query(query)?.kind {
             near_jsonrpc_primitives::types::query::QueryResponseKind::AccessKey(access_key) => {
                 Ok(access_key)
             }