From e680c13a1f050c185e7de483822f3367af1ad43a Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Wed, 10 Feb 2021 18:47:11 +0200 Subject: [PATCH 01/22] Introduce some error struct into runtime/runtime/state_viewer. Refactoring query method to add structurred errors there WIP --- Cargo.lock | 3 + chain/chain-primitives/Cargo.toml | 1 + chain/chain-primitives/src/error.rs | 18 ++ chain/chain/src/lib.rs | 1 + chain/chain/src/test_utils.rs | 2 +- chain/chain/src/types.rs | 2 +- chain/client-primitives/src/types.rs | 37 +++- chain/client/Cargo.toml | 1 + chain/client/src/view_client.rs | 174 ++++++------------ chain/jsonrpc-primitives/src/types/mod.rs | 1 + chain/jsonrpc-primitives/src/types/query.rs | 11 ++ chain/network/src/peer.rs | 6 - chain/network/src/types.rs | 4 - neard/Cargo.toml | 1 + neard/src/lib.rs | 1 + neard/src/runtime/errors.rs | 72 ++++++++ neard/src/{runtime.rs => runtime/mod.rs} | 67 +++---- runtime/runtime/src/state_viewer/errors.rs | 73 ++++++++ .../{state_viewer.rs => state_viewer/mod.rs} | 68 +++++-- 19 files changed, 363 insertions(+), 180 deletions(-) create mode 100644 chain/jsonrpc-primitives/src/types/query.rs create mode 100644 neard/src/runtime/errors.rs rename neard/src/{runtime.rs => runtime/mod.rs} (98%) create mode 100644 runtime/runtime/src/state_viewer/errors.rs rename runtime/runtime/src/{state_viewer.rs => state_viewer/mod.rs} (83%) diff --git a/Cargo.lock b/Cargo.lock index f901c20e140..6c2f92ccfe2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3051,6 +3051,7 @@ dependencies = [ "failure_derive", "log", "near-primitives", + "thiserror", ] [[package]] @@ -3115,6 +3116,7 @@ dependencies = [ "strum", "sysinfo", "testlib", + "thiserror", ] [[package]] @@ -3699,6 +3701,7 @@ dependencies = [ "serde_json", "tempfile", "testlib", + "thiserror", "tracing", "tracing-subscriber", ] diff --git a/chain/chain-primitives/Cargo.toml b/chain/chain-primitives/Cargo.toml index d35b373bd90..ab4fac84053 100644 --- a/chain/chain-primitives/Cargo.toml +++ b/chain/chain-primitives/Cargo.toml @@ -11,5 +11,6 @@ chrono = { version = "0.4.4", features = ["serde"] } failure = "0.1" failure_derive = "0.1" log = "0.4" +thiserror = "1.0" near-primitives = { path = "../../core/primitives" } diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index 5f608454878..dc89eaf0269 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -12,6 +12,24 @@ use near_primitives::serialize::to_base; use near_primitives::sharding::{ChunkHash, ShardChunkHeader}; use near_primitives::types::{BlockHeight, EpochId, ShardId}; +#[derive(thiserror::Error, Debug)] +pub enum QueryError { + #[error("Invalid account ID {0}")] + InvalidAccount(near_primitives::types::AccountId), + #[error("Account ID {0} does not exist while viewing")] + AccountDoesNotExist(near_primitives::types::AccountId), + #[error("Contract ID {0} code does not exist while viewing")] + ContractCodeDoesNotExist(near_primitives::types::AccountId), + #[error("Access key for public key {0} does not exist while viewing")] + AccessKeyDoesNotExist(String), + #[error("Storage error occurred: {0:?}")] + StorageError(#[from] near_primitives::errors::StorageError), + #[error("Internal error occurred: {0}")] + InternalError(String), + #[error("VM error occurred: {0}")] + VMError(String), +} + #[derive(Debug)] pub struct Error { inner: Context, diff --git a/chain/chain/src/lib.rs b/chain/chain/src/lib.rs index e03c1a51918..11a4b2b18e6 100644 --- a/chain/chain/src/lib.rs +++ b/chain/chain/src/lib.rs @@ -4,6 +4,7 @@ extern crate lazy_static; pub use chain::{collect_receipts, Chain, MAX_ORPHAN_SIZE}; pub use doomslug::{Doomslug, DoomslugBlockProductionReadiness, DoomslugThresholdMode}; pub use lightclient::{create_light_client_block_view, get_epoch_block_producers_view}; +pub use near_chain_primitives; pub use near_chain_primitives::{Error, ErrorKind}; pub use store::{ChainStore, ChainStoreAccess, ChainStoreUpdate}; pub use store_validator::{ErrorMessage, StoreValidator}; diff --git a/chain/chain/src/test_utils.rs b/chain/chain/src/test_utils.rs index 51916d5dd19..fc9575fb0ad 100644 --- a/chain/chain/src/test_utils.rs +++ b/chain/chain/src/test_utils.rs @@ -787,7 +787,7 @@ impl RuntimeAdapter for KeyValueRuntime { block_hash: &CryptoHash, _epoch_id: &EpochId, request: &QueryRequest, - ) -> Result> { + ) -> Result { match request { QueryRequest::ViewAccount { account_id, .. } => Ok(QueryResponse { kind: QueryResponseKind::ViewAccount( diff --git a/chain/chain/src/types.rs b/chain/chain/src/types.rs index 2d3d9ccd1ed..e8315084bb1 100644 --- a/chain/chain/src/types.rs +++ b/chain/chain/src/types.rs @@ -575,7 +575,7 @@ pub trait RuntimeAdapter: Send + Sync { block_hash: &CryptoHash, epoch_id: &EpochId, request: &QueryRequest, - ) -> Result>; + ) -> Result; fn get_validator_info( &self, diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs index b837a1c9831..d1e01b25507 100644 --- a/chain/client-primitives/src/types.rs +++ b/chain/client-primitives/src/types.rs @@ -267,7 +267,42 @@ impl Query { } impl Message for Query { - type Result = Result, String>; + type Result = Result; +} + +#[derive(thiserror::Error, Debug)] +pub enum QueryError { + #[error("IO Error: {0}")] + IOError(String), + #[error("There are no fully synchronized blocks yet")] + NotSyncedYet, + #[error("The node does not track the shard")] + DoesNotTrackShard, + #[error("Invalid account ID {0}")] + InvalidAccount(near_primitives::types::AccountId), + #[error("Account ID {0} has never been observed on the node")] + AccountDoesNotExist(near_primitives::types::AccountId), + #[error("Contract code for contract ID {0} has never been observed on the node")] + ContractCodeDoesNotExist(near_primitives::types::AccountId), + #[error("Access key for public key {0} has never been observed on the node")] + AccessKeyDoesNotExist(String), + #[error("VM error occurred: {0}")] + VMError(String), + // NOTE: Currently, the underlying errors are too broad, and while we tried to handle + // expected cases, we cannot statically guarantee that no other errors will be returned + // in the future. + // TODO #3851: Remove this variant once we can exhaustively match all the underlying errors + #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: {0}")] + Unreachable(String), +} + +impl From for QueryError { + fn from(error: near_chain_primitives::Error) -> Self { + match error.kind() { + near_chain_primitives::ErrorKind::IOErr(s) => Self::IOError(s), + _ => Self::Unreachable(error.to_string()), + } + } } pub struct Status { diff --git a/chain/client/Cargo.toml b/chain/client/Cargo.toml index 19c3238299e..e3b7b8c7cc1 100644 --- a/chain/client/Cargo.toml +++ b/chain/client/Cargo.toml @@ -23,6 +23,7 @@ borsh = "0.8.1" reed-solomon-erasure = "4" num-rational = "0.3" linked-hash-map = "0.5.3" +thiserror = "1.0" near-crypto = { path = "../../core/crypto" } near-primitives = { path = "../../core/primitives" } diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index 0ddd83f9783..0ce2b62bc8b 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -193,15 +193,7 @@ impl ViewClientActor { } } - fn handle_query(&mut self, msg: Query) -> Result, String> { - { - let mut request_manager = self.request_manager.write().expect(POISONED_LOCK_ERR); - if let Some(response) = request_manager.query_responses.cache_remove(&msg.query_id) { - request_manager.query_requests.cache_remove(&msg.query_id); - return response.map(Some); - } - } - + fn handle_query(&mut self, msg: Query) -> Result { let header = match msg.block_reference { BlockReference::BlockId(BlockId::Height(block_height)) => { self.chain.get_header_by_height(block_height) @@ -210,22 +202,20 @@ impl ViewClientActor { self.chain.get_block_header(&block_hash) } BlockReference::Finality(ref finality) => { - let block_hash = - self.get_block_hash_by_finality(&finality).map_err(|e| e.to_string())?; + let block_hash = self.get_block_hash_by_finality(&finality)?; self.chain.get_block_header(&block_hash) } BlockReference::SyncCheckpoint(ref synchronization_checkpoint) => { - if let Some(block_hash) = self - .get_block_hash_by_sync_checkpoint(&synchronization_checkpoint) - .map_err(|e| e.to_string())? + if let Some(block_hash) = + self.get_block_hash_by_sync_checkpoint(&synchronization_checkpoint)? { self.chain.get_block_header(&block_hash) } else { - return Err("There are no fully synchronized blocks yet".into()); + return Err(QueryError::NotSyncedYet); } } }; - let header = header.map_err(|e| e.to_string())?.clone(); + let header = header?.clone(); let account_id = match &msg.request { QueryRequest::ViewAccount { account_id, .. } => account_id, @@ -236,50 +226,31 @@ impl ViewClientActor { QueryRequest::ViewCode { account_id, .. } => account_id, }; let shard_id = self.runtime_adapter.account_id_to_shard_id(account_id); + let head = self.chain.head()?; - // If we have state for the shard that we query return query result directly. - // Otherwise route query to peers. - match self.chain.get_chunk_extra(header.hash(), shard_id) { - Ok(chunk_extra) => { - let state_root = chunk_extra.state_root; - self.runtime_adapter - .query( - shard_id, - &state_root, - header.height(), - header.raw_timestamp(), - header.prev_hash(), - header.hash(), - header.epoch_id(), - &msg.request, - ) - .map(Some) - .map_err(|e| e.to_string()) - } - Err(e) => { - match e.kind() { - ErrorKind::DBNotFoundErr(_) => {} - _ => { - warn!(target: "client", "Getting chunk extra failed: {}", e.to_string()); - } - } - // route request - let mut request_manager = self.request_manager.write().expect(POISONED_LOCK_ERR); - if Self::need_request(msg.query_id.clone(), &mut request_manager.query_requests) { - let validator = self - .chain - .find_validator_for_forwarding(shard_id) - .map_err(|e| e.to_string())?; - self.network_adapter.do_send(NetworkRequests::Query { - query_id: msg.query_id, - account_id: validator, - block_reference: msg.block_reference, - request: msg.request, - }); - } - - Ok(None) - } + if self.runtime_adapter.cares_about_shard( + self.validator_account_id.as_ref(), + &head.last_block_hash, + shard_id, + true, + ) { + let chunk_extra = self.chain.get_chunk_extra(header.hash(), shard_id)?; + let state_root = chunk_extra.state_root; + Ok(self + .runtime_adapter + .query( + shard_id, + &state_root, + header.height(), + header.raw_timestamp(), + header.prev_hash(), + header.hash(), + header.epoch_id(), + &msg.request, + ) + .map_err(RuntimeQueryError::from)?) + } else { + Err(QueryError::DoesNotTrackShard) } } @@ -469,7 +440,7 @@ impl Actor for ViewClientActor { } impl Handler for ViewClientActor { - type Result = Result, String>; + type Result = Result; #[perf] fn handle(&mut self, msg: Query, _: &mut Self::Context) -> Self::Result { @@ -477,6 +448,36 @@ impl Handler for ViewClientActor { } } +#[derive(thiserror::Error, Debug)] +#[error(transparent)] +struct RuntimeQueryError(#[from] pub near_chain::near_chain_primitives::error::QueryError); + +impl From for near_client_primitives::types::QueryError { + fn from(error: RuntimeQueryError) -> Self { + match error.0 { + near_chain::near_chain_primitives::error::QueryError::InternalError(s) => { + Self::Unreachable(s) + } + near_chain::near_chain_primitives::error::QueryError::InvalidAccount(account_id) => { + Self::InvalidAccount(account_id) + } + near_chain::near_chain_primitives::error::QueryError::AccountDoesNotExist( + account_id, + ) => Self::AccountDoesNotExist(account_id), + near_chain::near_chain_primitives::error::QueryError::ContractCodeDoesNotExist( + account_id, + ) => Self::ContractCodeDoesNotExist(account_id), + near_chain::near_chain_primitives::error::QueryError::AccessKeyDoesNotExist( + public_key, + ) => Self::AccessKeyDoesNotExist(public_key), + near_chain::near_chain_primitives::error::QueryError::StorageError(storage_error) => { + Self::IOError(storage_error.to_string()) + } + near_chain::near_chain_primitives::error::QueryError::VMError(s) => Self::VMError(s), + } + } +} + /// Handles retrieving block from the chain. impl Handler for ViewClientActor { type Result = Result; @@ -851,40 +852,6 @@ impl Handler for ViewClientActor { } } -impl Handler for ViewClientActor { - type Result = Result; - - #[perf] - fn handle(&mut self, msg: GetProtocolConfig, _: &mut Self::Context) -> Self::Result { - let block_header = match msg.0 { - BlockReference::Finality(finality) => { - let block_hash = self.get_block_hash_by_finality(&finality)?; - self.chain.get_block_header(&block_hash).map(Clone::clone) - } - BlockReference::BlockId(BlockId::Height(height)) => { - self.chain.get_header_by_height(height).map(Clone::clone) - } - BlockReference::BlockId(BlockId::Hash(hash)) => { - self.chain.get_block_header(&hash).map(Clone::clone) - } - BlockReference::SyncCheckpoint(sync_checkpoint) => { - if let Some(block_hash) = - self.get_block_hash_by_sync_checkpoint(&sync_checkpoint)? - { - self.chain.get_block_header(&block_hash).map(Clone::clone) - } else { - return Err(GetProtocolConfigError::UnknownBlock(format!( - "{:?}", - sync_checkpoint - ))); - } - } - }?; - let config = self.runtime_adapter.get_protocol_config(block_header.epoch_id())?; - Ok(config.into()) - } -} - impl Handler for ViewClientActor { type Result = NetworkViewClientResponses; @@ -945,25 +912,6 @@ impl Handler for ViewClientActor { } NetworkViewClientResponses::NoResponse } - NetworkViewClientMessages::Query { query_id, block_reference, request } => { - let query = Query { query_id: query_id.clone(), block_reference, request }; - match self.handle_query(query) { - Ok(Some(r)) => { - NetworkViewClientResponses::QueryResponse { query_id, response: Ok(r) } - } - Ok(None) => NetworkViewClientResponses::NoResponse, - Err(e) => { - NetworkViewClientResponses::QueryResponse { query_id, response: Err(e) } - } - } - } - NetworkViewClientMessages::QueryResponse { query_id, response } => { - let mut request_manager = self.request_manager.write().expect(POISONED_LOCK_ERR); - if request_manager.query_requests.cache_get(&query_id).is_some() { - request_manager.query_responses.cache_set(query_id, response); - } - NetworkViewClientResponses::NoResponse - } NetworkViewClientMessages::ReceiptOutcomeRequest(receipt_id) => { if let Ok(outcome_with_proof) = self.chain.get_execution_outcome(&receipt_id) { NetworkViewClientResponses::ReceiptOutcomeResponse(Box::new( diff --git a/chain/jsonrpc-primitives/src/types/mod.rs b/chain/jsonrpc-primitives/src/types/mod.rs index 1cbb0868831..53c87d7347b 100644 --- a/chain/jsonrpc-primitives/src/types/mod.rs +++ b/chain/jsonrpc-primitives/src/types/mod.rs @@ -1,5 +1,6 @@ pub mod blocks; pub mod chunks; pub mod config; +pub mod query; pub mod receipts; pub mod validator; diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs new file mode 100644 index 00000000000..86bd2b82110 --- /dev/null +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -0,0 +1,11 @@ +use serde::{Deserialize, Serialize}; +use serde_json::Value; + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct QueryReference {} + +#[derive(Serialize, Deserialize)] +pub struct RpcQueryRequest { + #[serde(flatten)] + pub query_reference: QueryReference, +} diff --git a/chain/network/src/peer.rs b/chain/network/src/peer.rs index 5b76b2ff844..1bf5ec6b456 100644 --- a/chain/network/src/peer.rs +++ b/chain/network/src/peer.rs @@ -382,12 +382,6 @@ impl Peer { PeerMessage::Routed(message) => { msg_hash = Some(message.hash()); match message.body { - RoutedMessageBody::QueryRequest { query_id, block_reference, request } => { - NetworkViewClientMessages::Query { query_id, block_reference, request } - } - RoutedMessageBody::QueryResponse { query_id, response } => { - NetworkViewClientMessages::QueryResponse { query_id, response } - } RoutedMessageBody::TxStatusRequest(account_id, tx_hash) => { NetworkViewClientMessages::TxStatus { tx_hash, diff --git a/chain/network/src/types.rs b/chain/network/src/types.rs index 878f0f73b98..7e7f57a4301 100644 --- a/chain/network/src/types.rs +++ b/chain/network/src/types.rs @@ -1503,10 +1503,6 @@ pub enum NetworkViewClientMessages { TxStatus { tx_hash: CryptoHash, signer_account_id: AccountId }, /// Transaction status response TxStatusResponse(Box), - /// General query - Query { query_id: String, block_reference: BlockReference, request: QueryRequest }, - /// Query response - QueryResponse { query_id: String, response: Result }, /// Request for receipt outcome ReceiptOutcomeRequest(CryptoHash), /// Receipt outcome response diff --git a/neard/Cargo.toml b/neard/Cargo.toml index a7030a929a0..69e394664fa 100644 --- a/neard/Cargo.toml +++ b/neard/Cargo.toml @@ -23,6 +23,7 @@ serde_json = "1" lazy_static = "1.4" dirs = "3" borsh = "0.8.1" +thiserror = "1.0" tracing = "0.1.13" tracing-subscriber = "0.2.4" num-rational = { version = "0.3", features = ["serde"] } diff --git a/neard/src/lib.rs b/neard/src/lib.rs index e524d2e80cc..bb8d7833960 100644 --- a/neard/src/lib.rs +++ b/neard/src/lib.rs @@ -29,6 +29,7 @@ use near_store::migrations::{ #[cfg(feature = "protocol_feature_rectify_inflation")] use near_store::migrations::migrate_18_to_rectify_inflation; +pub use runtime::errors as runtime_errors; pub mod config; pub mod genesis_validate; diff --git a/neard/src/runtime/errors.rs b/neard/src/runtime/errors.rs new file mode 100644 index 00000000000..db8fa49e5a8 --- /dev/null +++ b/neard/src/runtime/errors.rs @@ -0,0 +1,72 @@ +use near_chain::near_chain_primitives::error::QueryError; + +#[derive(thiserror::Error, Debug)] +#[error(transparent)] +pub struct RuntimeQueryError(#[from] pub QueryError); + +impl From for RuntimeQueryError { + fn from(error: node_runtime::state_viewer::errors::ViewAccountError) -> Self { + match error { + node_runtime::state_viewer::errors::ViewAccountError::InvalidAccountId(account_id) => { + Self(QueryError::InvalidAccount(account_id)) + } + node_runtime::state_viewer::errors::ViewAccountError::AccountDoesNotExist( + account_id, + ) => Self(QueryError::AccountDoesNotExist(account_id)), + node_runtime::state_viewer::errors::ViewAccountError::StorageError(storage_error) => { + Self(QueryError::StorageError(storage_error)) + } + } + } +} + +impl From for QueryError { + fn from(error: node_runtime::state_viewer::errors::ViewContractCodeError) -> Self { + match error { + node_runtime::state_viewer::errors::ViewContractCodeError::InvalidAccountId( + account_id, + ) => Self(QueryError::InvalidAccount(account_id)), + node_runtime::state_viewer::errors::ViewContractCodeError::AccountDoesNotExist( + account_id, + ) => Self(QueryError::AccountDoesNotExist(account_id)), + node_runtime::state_viewer::errors::ViewContractCodeError::StorageError( + storage_error, + ) => Self(QueryError::StorageError(storage_error)), + node_runtime::state_viewer::errors::ViewContractCodeError::ContractCodeDoesNotExist( + contract_id, + ) => Self(QueryError::ContractCodeDoesNotExist(contract_id)), + } + } +} + +impl From for QueryError { + fn from(error: node_runtime::state_viewer::errors::CallFunctionError) -> Self { + match error { + node_runtime::state_viewer::errors::CallFunctionError::InvalidAccountId(account_id) => { + Self(QueryError::InvalidAccount(account_id)) + } + node_runtime::state_viewer::errors::CallFunctionError::AccountDoesNotExist( + account_id, + ) => Self(QueryError::AccountDoesNotExist(account_id)), + node_runtime::state_viewer::errors::CallFunctionError::StorageError(storage_error) => { + Self(QueryError::StorageError(storage_error)) + } + node_runtime::state_viewer::errors::CallFunctionError::VMError(s) => { + Self(QueryError::VMError(s)) + } + } + } +} + +impl From for QueryError { + fn from(error: node_runtime::state_viewer::errors::ViewStateError) -> Self { + match error { + node_runtime::state_viewer::errors::ViewStateError::InvalidAccountId(account_id) => { + Self(QueryError::InvalidAccount(account_id)) + } + node_runtime::state_viewer::errors::ViewStateError::StorageError(storage_error) => { + Self(QueryError::StorageError(storage_error)) + } + } + } +} diff --git a/neard/src/runtime.rs b/neard/src/runtime/mod.rs similarity index 98% rename from neard/src/runtime.rs rename to neard/src/runtime/mod.rs index 5d621af33c9..a790cce54c3 100644 --- a/neard/src/runtime.rs +++ b/neard/src/runtime/mod.rs @@ -61,6 +61,8 @@ use near_primitives::runtime::config::RuntimeConfig; #[cfg(feature = "protocol_feature_rectify_inflation")] use near_epoch_manager::NUM_SECONDS_IN_A_YEAR; +pub mod errors; + const POISONED_LOCK_ERR: &str = "The lock was poisoned."; const STATE_DUMP_FILE: &str = "state_dump"; const GENESIS_ROOTS_FILE: &str = "genesis_roots"; @@ -1246,27 +1248,27 @@ impl RuntimeAdapter for NightshadeRuntime { block_hash: &CryptoHash, epoch_id: &EpochId, request: &QueryRequest, - ) -> Result> { + ) -> Result { match request { QueryRequest::ViewAccount { account_id } => { - match self.view_account(shard_id, *state_root, account_id) { - Ok(r) => Ok(QueryResponse { - kind: QueryResponseKind::ViewAccount(r.into()), - block_height, - block_hash: *block_hash, - }), - Err(e) => Err(e), - } + let account = self + .view_account(shard_id, *state_root, account_id) + .map_err(errors::RuntimeQueryError::from)?; + Ok(QueryResponse { + kind: QueryResponseKind::ViewAccount(account.into()), + block_height, + block_hash: *block_hash, + }) } QueryRequest::ViewCode { account_id } => { - match self.view_contract_code(shard_id, *state_root, account_id) { - Ok(r) => Ok(QueryResponse { - kind: QueryResponseKind::ViewCode(r.into()), - block_height, - block_hash: *block_hash, - }), - Err(e) => Err(e), - } + let contract_code = self + .view_contract_code(shard_id, *state_root, account_id) + .map_err(errors::RuntimeQueryError::from)?; + Ok(QueryResponse { + kind: QueryResponseKind::ViewCode(contract_code.into()), + block_height, + block_hash: *block_hash, + }) } QueryRequest::CallFunction { account_id, method_name, args } => { let mut logs = vec![]; @@ -1538,7 +1540,7 @@ impl node_runtime::adapter::ViewRuntimeAdapter for NightshadeRuntime { shard_id: ShardId, state_root: MerkleHash, account_id: &AccountId, - ) -> Result> { + ) -> Result { let state_update = self.get_tries().new_trie_update_view(shard_id, state_root); self.trie_viewer.view_account(&state_update, account_id) } @@ -1548,7 +1550,7 @@ impl node_runtime::adapter::ViewRuntimeAdapter for NightshadeRuntime { shard_id: ShardId, state_root: MerkleHash, account_id: &AccountId, - ) -> Result> { + ) -> Result { let state_update = self.get_tries().new_trie_update_view(shard_id, state_root); self.trie_viewer.view_contract_code(&state_update, account_id) } @@ -1570,7 +1572,7 @@ impl node_runtime::adapter::ViewRuntimeAdapter for NightshadeRuntime { epoch_info_provider: &dyn EpochInfoProvider, current_protocol_version: ProtocolVersion, #[cfg(feature = "protocol_feature_evm")] evm_chain_id: u64, - ) -> Result, Box> { + ) -> Result, node_runtime::state_viewer::errors::CallFunctionError> { let state_update = self.get_tries().new_trie_update_view(shard_id, state_root); let view_state = ViewApplyState { block_height: height, @@ -1601,7 +1603,7 @@ impl node_runtime::adapter::ViewRuntimeAdapter for NightshadeRuntime { state_root: MerkleHash, account_id: &AccountId, public_key: &PublicKey, - ) -> Result> { + ) -> Result { let state_update = self.get_tries().new_trie_update_view(shard_id, state_root); self.trie_viewer.view_access_key(&state_update, account_id, public_key) } @@ -1611,25 +1613,10 @@ impl node_runtime::adapter::ViewRuntimeAdapter for NightshadeRuntime { shard_id: ShardId, state_root: MerkleHash, account_id: &AccountId, - ) -> Result, Box> { + ) -> Result, node_runtime::state_viewer::errors::ViewAccessKeyError> + { let state_update = self.get_tries().new_trie_update_view(shard_id, state_root); - let prefix = trie_key_parsers::get_raw_prefix_for_access_keys(account_id); - let raw_prefix: &[u8] = prefix.as_ref(); - let access_keys = match state_update.iter(&prefix) { - Ok(iter) => iter - .map(|key| { - let key = key?; - let public_key = &key[raw_prefix.len()..]; - let access_key = get_access_key_raw(&state_update, &key)? - .ok_or("Missing key from iterator")?; - PublicKey::try_from_slice(public_key) - .map_err(|err| format!("{}", err).into()) - .map(|key| (key, access_key)) - }) - .collect::, Box>>(), - Err(e) => Err(e.into()), - }; - access_keys + self.trie_viewer.view_access_keys(&state_update, account_id) } fn view_state( @@ -1638,7 +1625,7 @@ impl node_runtime::adapter::ViewRuntimeAdapter for NightshadeRuntime { state_root: MerkleHash, account_id: &AccountId, prefix: &[u8], - ) -> Result> { + ) -> Result { let state_update = self.get_tries().new_trie_update_view(shard_id, state_root); self.trie_viewer.view_state(&state_update, account_id, prefix) } diff --git a/runtime/runtime/src/state_viewer/errors.rs b/runtime/runtime/src/state_viewer/errors.rs new file mode 100644 index 00000000000..402f095d652 --- /dev/null +++ b/runtime/runtime/src/state_viewer/errors.rs @@ -0,0 +1,73 @@ +pub enum ViewAccountError { + InvalidAccountId(near_primitives::types::AccountId), + AccountDoesNotExist(near_primitives::types::AccountId), + StorageError(near_primitives::errors::StorageError), +} + +pub enum ViewContractCodeError { + InvalidAccountId(near_primitives::types::AccountId), + AccountDoesNotExist(near_primitives::types::AccountId), + StorageError(near_primitives::errors::StorageError), + ContractCodeDoesNotExist(near_primitives::types::AccountId), +} + +pub enum ViewAccessKeyError { + InvalidAccountId(near_primitives::types::AccountId), + StorageError(near_primitives::errors::StorageError), + AccessKeyDoesNotExist(near_crypto::PublicKey), + InternalError(String), +} + +pub enum ViewStateError { + InvalidAccountId(near_primitives::types::AccountId), + StorageError(near_primitives::errors::StorageError), +} + +pub enum CallFunctionError { + InvalidAccountId(near_primitives::types::AccountId), + AccountDoesNotExist(near_primitives::types::AccountId), + StorageError(near_primitives::errors::StorageError), + VMError(String), +} + +impl From for ViewAccountError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::StorageError(storage_error) + } +} + +impl From for ViewContractCodeError { + fn from(view_account_error: ViewAccountError) -> Self { + match view_account_error { + ViewAccountError::InvalidAccountId(account_id) => Self::AccountDoesNotExist(account_id), + ViewAccountError::AccountDoesNotExist(account_id) => { + Self::AccountDoesNotExist(account_id) + } + ViewAccountError::StorageError(storage_error) => Self::StorageError(storage_error), + } + } +} + +impl From for ViewContractCodeError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::StorageError(storage_error) + } +} + +impl From for ViewAccessKeyError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::StorageError(storage_error) + } +} + +impl From for ViewStateError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::StorageError(storage_error) + } +} + +impl From for CallFunctionError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::StorageError(storage_error) + } +} diff --git a/runtime/runtime/src/state_viewer.rs b/runtime/runtime/src/state_viewer/mod.rs similarity index 83% rename from runtime/runtime/src/state_viewer.rs rename to runtime/runtime/src/state_viewer/mod.rs index 5b21f62d7f7..4296c3a6d4d 100644 --- a/runtime/runtime/src/state_viewer.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -2,6 +2,7 @@ use log::debug; use near_crypto::{KeyType, PublicKey}; use near_primitives::{ account::{AccessKey, Account}, + borsh::BorshDeserialize, contract::ContractCode, hash::CryptoHash, receipt::ActionReceipt, @@ -19,6 +20,8 @@ use std::{str, sync::Arc, time::Instant}; use crate::{actions::execute_function_call, ext::RuntimeExt}; +pub mod errors; + pub struct TrieViewer {} impl TrieViewer { @@ -30,23 +33,23 @@ impl TrieViewer { &self, state_update: &TrieUpdate, account_id: &AccountId, - ) -> Result> { + ) -> Result { if !is_valid_account_id(account_id) { - return Err(format!("Account ID '{}' is not valid", account_id).into()); + return Err(errors::ViewAccountError::InvalidAccountId(account_id.clone())); } get_account(state_update, &account_id)? - .ok_or_else(|| format!("account {} does not exist while viewing", account_id).into()) + .ok_or_else(|| errors::ViewAccountError::AccountDoesNotExist(account_id.clone())) } pub fn view_contract_code( &self, state_update: &TrieUpdate, account_id: &AccountId, - ) -> Result> { + ) -> Result { let account = self.view_account(state_update, account_id)?; get_code(state_update, account_id, Some(account.code_hash))?.ok_or_else(|| { - format!("contract code of account {} does not exist while viewing", account_id).into() + errors::ViewContractCodeError::ContractCodeDoesNotExist(account_id.clone()) }) } @@ -55,13 +58,50 @@ impl TrieViewer { state_update: &TrieUpdate, account_id: &AccountId, public_key: &PublicKey, - ) -> Result> { + ) -> Result { if !is_valid_account_id(account_id) { - return Err(format!("Account ID '{}' is not valid", account_id).into()); + return Err(errors::ViewAccessKeyError::InvalidAccountId(account_id.clone())); } get_access_key(state_update, account_id, public_key)? - .ok_or_else(|| format!("access key {} does not exist while viewing", public_key).into()) + .ok_or_else(|| errors::ViewAccessKeyError::AccessKeyDoesNotExist(public_key.clone())) + } + + pub fn view_access_keys( + &self, + state_update: &TrieUpdate, + account_id: &AccountId, + ) -> Result, errors::ViewAccessKeyError> { + if !is_valid_account_id(account_id) { + return Err(errors::ViewAccessKeyError::InvalidAccountId(account_id.clone())); + } + + let prefix = trie_key_parsers::get_raw_prefix_for_access_keys(account_id); + let raw_prefix: &[u8] = prefix.as_ref(); + let access_keys = match state_update.iter(&prefix) { + Ok(iter) => iter + .map(|key| { + let key = key?; + let public_key = &key[raw_prefix.len()..]; + let access_key = near_store::get_access_key_raw(&state_update, &key)? + .ok_or_else(|| { + errors::ViewAccessKeyError::InternalError( + "Unexpected missing key from iterator".to_string(), + ) + })?; + PublicKey::try_from_slice(public_key) + .map_err(|_| { + errors::ViewAccessKeyError::InternalError(format!( + "Unexpected invalid public key {:?} received from store", + public_key + )) + }) + .map(|key| (key, access_key)) + }) + .collect::, errors::ViewAccessKeyError>>(), + Err(e) => Err(e.into()), + }; + access_keys } pub fn view_state( @@ -69,9 +109,9 @@ impl TrieViewer { state_update: &TrieUpdate, account_id: &AccountId, prefix: &[u8], - ) -> Result> { + ) -> Result { if !is_valid_account_id(account_id) { - return Err(format!("Account ID '{}' is not valid", account_id).into()); + return Err(errors::ViewStateError::InvalidAccountId(account_id.clone())); } let mut values = vec![]; let query = trie_key_parsers::get_raw_prefix_for_contract_data(account_id, prefix); @@ -102,14 +142,14 @@ impl TrieViewer { args: &[u8], logs: &mut Vec, epoch_info_provider: &dyn EpochInfoProvider, - ) -> Result, Box> { + ) -> Result, errors::CallFunctionError> { let now = Instant::now(); if !is_valid_account_id(contract_id) { - return Err(format!("Contract ID {:?} is not valid", contract_id).into()); + return Err(errors::CallFunctionError::InvalidAccountId(contract_id.clone())); } let root = state_update.get_root(); let mut account = get_account(&state_update, contract_id)? - .ok_or_else(|| format!("Account {:?} doesn't exist", contract_id))?; + .ok_or_else(|| errors::CallFunctionError::AccountDoesNotExist(contract_id.clone()))?; // TODO(#1015): Add ability to pass public key and originator_id let originator_id = contract_id; let public_key = PublicKey::empty(KeyType::ED25519); @@ -184,7 +224,7 @@ impl TrieViewer { } let message = format!("wasm execution failed with error: {:?}", err); debug!(target: "runtime", "(exec time {}) {}", time_str, message); - Err(message.into()) + Err(errors::CallFunctionError::VMError(message)) } else { let outcome = outcome.unwrap(); debug!(target: "runtime", "(exec time {}) result of execution: {:#?}", time_str, outcome); From 27cc88d29570d8163577e61af8923b75daf2ae5b Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Thu, 11 Feb 2021 17:34:43 +0200 Subject: [PATCH 02/22] Fix rosetta-rpc, update runtime's adapter trait --- Cargo.lock | 3 + chain/jsonrpc-primitives/Cargo.toml | 1 + chain/jsonrpc-primitives/src/types/query.rs | 148 +++++++++++++++++++- chain/jsonrpc/src/lib.rs | 122 ++++------------ chain/rosetta-rpc/Cargo.toml | 1 + chain/rosetta-rpc/src/utils.rs | 52 +++---- neard/src/runtime/errors.rs | 18 ++- neard/src/runtime/mod.rs | 4 +- runtime/runtime/Cargo.toml | 1 + runtime/runtime/src/adapter.rs | 12 +- runtime/runtime/src/state_viewer/errors.rs | 22 +++ 11 files changed, 238 insertions(+), 146 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c2f92ccfe2..d638ca79617 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3292,6 +3292,7 @@ dependencies = [ "near-client-primitives", "near-metrics", "near-primitives", + "near-primitives-core", "serde", "serde_json", "thiserror", @@ -3465,6 +3466,7 @@ dependencies = [ "lazy_static", "near-chain-configs", "near-client", + "near-client-primitives", "near-crypto", "near-network", "near-primitives", @@ -3752,6 +3754,7 @@ dependencies = [ "serde_json", "tempfile", "testlib", + "thiserror", ] [[package]] diff --git a/chain/jsonrpc-primitives/Cargo.toml b/chain/jsonrpc-primitives/Cargo.toml index 6e1dc3f9a04..5eff37ae8a6 100644 --- a/chain/jsonrpc-primitives/Cargo.toml +++ b/chain/jsonrpc-primitives/Cargo.toml @@ -16,5 +16,6 @@ uuid = { version = "~0.8", features = ["v4"] } near-client-primitives = { path = "../client-primitives" } near-primitives = { path = "../../core/primitives" } +near-primitives-core = { path = "../../core/primitives-core" } near-metrics = { path = "../../core/metrics" } near-chain-configs = { path = "../../core/chain-configs" } diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index 86bd2b82110..cf73d511afd 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -1,11 +1,151 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct QueryReference {} - #[derive(Serialize, Deserialize)] pub struct RpcQueryRequest { #[serde(flatten)] - pub query_reference: QueryReference, + pub block_reference: near_primitives::types::BlockReference, + #[serde(flatten)] + pub request: near_primitives::views::QueryRequest, +} + +#[derive(thiserror::Error, Debug)] +pub enum RpcQueryError { + #[error("IO Error: {0}")] + IOError(String), + #[error("There are no fully synchronized blocks yet")] + NotSyncedYet, + #[error("The node does not track the shard")] + DoesNotTrackShard, + #[error("Invalid account ID {0}")] + InvalidAccount(near_primitives::types::AccountId), + #[error("Account ID {0} has never been observed on the node")] + AccountDoesNotExist(near_primitives::types::AccountId), + #[error("Contract code for contract ID {0} has never been observed on the node")] + ContractCodeDoesNotExist(near_primitives::types::AccountId), + #[error("Access key for public key {0} has never been observed on the node")] + AccessKeyDoesNotExist(String), + #[error("VM error occurred: {0}")] + VMError(String), + #[error("The node reached its limits. Try again later. More details: {0}")] + InternalError(String), + // NOTE: Currently, the underlying errors are too broad, and while we tried to handle + // expected cases, we cannot statically guarantee that no other errors will be returned + // in the future. + // TODO #3851: Remove this variant once we can exhaustively match all the underlying errors + #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: {0}")] + Unreachable(String), +} + +#[derive(Serialize, Deserialize)] +pub struct RpcQueryResponse { + #[serde(flatten)] + pub query_response: near_primitives::views::QueryResponse, +} + +impl RpcQueryRequest { + pub fn parse(value: Option) -> Result { + let query_request = if let Ok((path, data)) = + crate::utils::parse_params::<(String, String)>(value.clone()) + { + let data = near_primitives_core::serialize::from_base(&data) + .map_err(|err| crate::errors::RpcParseError(err.to_string()))?; + let query_data_size = path.len() + data.len(); + + let mut path_parts = path.splitn(3, '/'); + let make_err = + || crate::errors::RpcParseError("Not enough query parameters provided".to_string()); + let query_command = path_parts.next().ok_or_else(make_err)?; + let account_id = + near_primitives::types::AccountId::from(path_parts.next().ok_or_else(make_err)?); + let maybe_extra_arg = path_parts.next(); + + let request = match query_command { + "account" => near_primitives::views::QueryRequest::ViewAccount { account_id }, + "access_key" => match maybe_extra_arg { + None => near_primitives::views::QueryRequest::ViewAccessKeyList { account_id }, + Some(pk) => near_primitives::views::QueryRequest::ViewAccessKey { + account_id, + public_key: pk.parse().map_err(|_| { + crate::errors::RpcParseError("Invalid public key".to_string()) + })?, + }, + }, + "code" => near_primitives::views::QueryRequest::ViewCode { account_id }, + "contract" => near_primitives::views::QueryRequest::ViewState { + account_id, + prefix: data.into(), + }, + "call" => match maybe_extra_arg { + Some(method_name) => near_primitives::views::QueryRequest::CallFunction { + account_id, + method_name: method_name.to_string(), + args: data.into(), + }, + None => { + return Err(crate::errors::RpcParseError( + "Method name is missing".to_string(), + )) + } + }, + _ => { + return Err(crate::errors::RpcParseError(format!( + "Unknown path {}", + query_command + ))) + } + }; + RpcQueryRequest { + request, + block_reference: near_primitives::types::BlockReference::latest(), + } + } else { + crate::utils::parse_params::(value)? + }; + Ok(query_request) + } +} + +impl From for RpcQueryError { + fn from(error: near_client_primitives::types::QueryError) -> Self { + match error { + near_client_primitives::types::QueryError::IOError(s) => Self::InternalError(s), + near_client_primitives::types::QueryError::NotSyncedYet => Self::NotSyncedYet, + near_client_primitives::types::QueryError::DoesNotTrackShard => Self::DoesNotTrackShard, + near_client_primitives::types::QueryError::InvalidAccount(account_id) => { + Self::InvalidAccount(account_id) + } + near_client_primitives::types::QueryError::AccountDoesNotExist(account_id) => { + Self::AccountDoesNotExist(account_id) + } + near_client_primitives::types::QueryError::ContractCodeDoesNotExist(account_id) => { + Self::ContractCodeDoesNotExist(account_id) + } + near_client_primitives::types::QueryError::AccessKeyDoesNotExist(public_key) => { + Self::AccessKeyDoesNotExist(public_key) + } + near_client_primitives::types::QueryError::VMError(s) => Self::VMError(s), + near_client_primitives::types::QueryError::Unreachable(s) => { + near_metrics::inc_counter_vec( + &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, + &["RpcQueryError", &s], + ); + Self::Unreachable(s) + } + } + } +} + +impl From for crate::errors::RpcError { + fn from(error: RpcQueryError) -> Self { + let error_data = Some(Value::String(error.to_string())); + + Self::new(-32_000, "Server error".to_string(), error_data) + } +} + +impl From for RpcQueryError { + fn from(error: actix::MailboxError) -> Self { + Self::InternalError(error.to_string()) + } } diff --git a/chain/jsonrpc/src/lib.rs b/chain/jsonrpc/src/lib.rs index 35a22dd50f0..324b388eaba 100644 --- a/chain/jsonrpc/src/lib.rs +++ b/chain/jsonrpc/src/lib.rs @@ -27,7 +27,7 @@ use near_jsonrpc_primitives::errors::RpcError; use near_jsonrpc_primitives::message::{Message, Request}; use near_jsonrpc_primitives::rpc::{ RpcBroadcastTxSyncResponse, RpcLightClientExecutionProofRequest, - RpcLightClientExecutionProofResponse, RpcQueryRequest, RpcStateChangesInBlockRequest, + RpcLightClientExecutionProofResponse, RpcStateChangesInBlockRequest, RpcStateChangesInBlockResponse, RpcStateChangesRequest, RpcStateChangesResponse, RpcValidatorsOrderedRequest, TransactionInfo, }; @@ -38,12 +38,10 @@ use near_network::types::{NetworkAdversarialMessage, NetworkViewClientMessages}; use near_network::{NetworkClientMessages, NetworkClientResponses}; use near_primitives::errors::{InvalidTxError, TxExecutionError}; use near_primitives::hash::CryptoHash; -use near_primitives::serialize::{from_base, from_base64, BaseEncode}; +use near_primitives::serialize::{from_base64, BaseEncode}; use near_primitives::transaction::SignedTransaction; -use near_primitives::types::{AccountId, BlockReference, MaybeBlockId}; -use near_primitives::views::{ - FinalExecutionOutcomeView, FinalExecutionOutcomeViewEnum, QueryRequest, -}; +use near_primitives::types::{AccountId, MaybeBlockId}; +use near_primitives::views::{FinalExecutionOutcomeView, FinalExecutionOutcomeViewEnum}; use near_runtime_utils::is_valid_account_id; mod metrics; @@ -104,10 +102,6 @@ impl RpcConfig { } } -fn from_base_or_parse_err(encoded: String) -> Result, RpcError> { - from_base(&encoded).map_err(|err| RpcError::parse_error(err.to_string())) -} - fn from_base64_or_parse_err(encoded: String) -> Result, RpcError> { from_base64(&encoded).map_err(|err| RpcError::parse_error(err.to_string())) } @@ -251,6 +245,9 @@ impl JsonRpcHandler { "EXPERIMENTAL_changes_in_block" => self.changes_in_block(request.params).await, "EXPERIMENTAL_check_tx" => self.check_tx(request.params).await, "EXPERIMENTAL_genesis_config" => self.genesis_config().await, + "EXPERIMENTAL_light_client_proof" => { + self.light_client_execution_outcome_proof(request.params).await + } "EXPERIMENTAL_protocol_config" => { let rpc_protocol_config_request = near_jsonrpc_primitives::types::config::RpcProtocolConfigRequest::parse( @@ -259,9 +256,6 @@ impl JsonRpcHandler { let config = self.protocol_config(rpc_protocol_config_request).await?; serde_json::to_value(config).map_err(|err| RpcError::parse_error(err.to_string())) } - "EXPERIMENTAL_light_client_proof" => { - self.light_client_execution_outcome_proof(request.params).await - } "EXPERIMENTAL_receipt" => { let rpc_receipt_request = near_jsonrpc_primitives::types::receipts::RpcReceiptRequest::parse( @@ -277,7 +271,13 @@ impl JsonRpcHandler { "light_client_proof" => self.light_client_execution_outcome_proof(request.params).await, "next_light_client_block" => self.next_light_client_block(request.params).await, "network_info" => self.network_info().await, - "query" => self.query(request.params).await, + "query" => { + let rpc_query_request = + near_jsonrpc_primitives::types::query::RpcQueryRequest::parse(request.params)?; + let query_response = self.query(rpc_query_request).await?; + serde_json::to_value(query_response) + .map_err(|err| RpcError::parse_error(err.to_string())) + } "status" => self.status().await, "tx" => self.tx_status_common(request.params, false).await, "validators" => { @@ -580,90 +580,16 @@ impl JsonRpcHandler { Ok(RpcProtocolConfigResponse { config_view }) } - async fn query(&self, params: Option) -> Result { - let query_request = if let Ok((path, data)) = - parse_params::<(String, String)>(params.clone()) - { - // 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 = from_base_or_parse_err(data)?; - let query_data_size = path.len() + data.len(); - if query_data_size > QUERY_DATA_MAX_SIZE { - return Err(RpcError::server_error(Some(format!( - "Query data size {} is too large", - query_data_size - )))); - } - let mut path_parts = path.splitn(3, '/'); - let make_err = - || RpcError::server_error(Some("Not enough query parameters provided".to_string())); - let query_command = path_parts.next().ok_or_else(make_err)?; - let account_id = AccountId::from(path_parts.next().ok_or_else(make_err)?); - let maybe_extra_arg = path_parts.next(); - - let request = match query_command { - "account" => QueryRequest::ViewAccount { account_id }, - "code" => QueryRequest::ViewCode { 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(|_| RpcError::server_error(Some("Invalid public key")))?, - }, - }, - "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(RpcError::server_error(Some( - "Method name is missing".to_string(), - ))) - } - }, - _ => { - return Err(RpcError::server_error(Some(format!( - "Unknown path {}", - query_command - )))) - } - }; - // Use Finality::None here to make backward compatibility tests work - RpcQueryRequest { request, block_reference: BlockReference::latest() } - } else { - parse_params::(params.clone())? - }; - let query = Query::new(query_request.block_reference, query_request.request); - timeout(self.polling_config.polling_timeout, async { - loop { - let result = self.view_client_addr.send(query.clone()).await; - match result { - Ok(ref r) => match r { - Ok(Some(_)) => break jsonify(result), - Ok(None) => {} - Err(e) => break Err(RpcError::server_error(Some(e))), - }, - Err(e) => break Err(RpcError::server_error(Some(e.to_string()))), - } - sleep(self.polling_config.polling_interval).await; - } - }) - .await - .map_err(|_| { - near_metrics::inc_counter(&metrics::RPC_TIMEOUT_TOTAL); - tracing::warn!( - target: "jsonrpc", "Timeout: query method. params {:?}", - params, - ); - RpcError::server_error(Some("query has timed out".to_string())) - })? + async fn query( + &self, + request_data: near_jsonrpc_primitives::types::query::RpcQueryRequest, + ) -> Result< + near_jsonrpc_primitives::types::query::RpcQueryResponse, + near_jsonrpc_primitives::types::query::RpcQueryError, + > { + let query = Query::new(request_data.block_reference, request_data.request); + let query_response = self.view_client_addr.send(query.clone()).await??; + Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { query_response }) } async fn tx_status_common( diff --git a/chain/rosetta-rpc/Cargo.toml b/chain/rosetta-rpc/Cargo.toml index 969f9cc92ad..9cdc35694f1 100644 --- a/chain/rosetta-rpc/Cargo.toml +++ b/chain/rosetta-rpc/Cargo.toml @@ -28,6 +28,7 @@ near-primitives = { path = "../../core/primitives" } near-crypto = { path = "../../core/crypto" } near-chain-configs = { path = "../../core/chain-configs" } near-client = { path = "../client" } +near-client-primitives = { path = "../client-primitives" } near-network = { path = "../network" } [dev-dependencies] diff --git a/chain/rosetta-rpc/src/utils.rs b/chain/rosetta-rpc/src/utils.rs index c88c8ff7b4c..828d5003121 100644 --- a/chain/rosetta-rpc/src/utils.rs +++ b/chain/rosetta-rpc/src/utils.rs @@ -313,23 +313,15 @@ pub(crate) async fn query_account( block_id, near_primitives::views::QueryRequest::ViewAccount { account_id }, ); - let account_info_response = tokio::time::timeout(std::time::Duration::from_secs(10), async { - loop { - match view_client_addr.send(query.clone()).await? { - Ok(Some(query_response)) => return Ok(query_response), - Ok(None) => {} - // TODO: update this once we return structured errors from the view_client handlers - Err(err) => { - if err.contains("does not exist") { - return Err(crate::errors::ErrorKind::NotFound(err)); - } - return Err(crate::errors::ErrorKind::InternalError(err)); - } + let account_info_response = match view_client_addr.send(query.clone()).await? { + Ok(query_response) => query_response, + Err(err) => match err { + near_client_primitives::types::QueryError::AccountDoesNotExist(_) => { + return Err(crate::errors::ErrorKind::NotFound(err.to_string())) } - tokio::time::sleep(std::time::Duration::from_millis(100)).await; - } - }) - .await??; + _ => return Err(crate::errors::ErrorKind::InternalError(err.to_string())), + }, + }; match account_info_response.kind { near_primitives::views::QueryResponseKind::ViewAccount(account_info) => { @@ -399,26 +391,18 @@ pub(crate) async fn query_access_key( block_id, near_primitives::views::QueryRequest::ViewAccessKey { account_id, public_key }, ); - - let access_key_query_response = - tokio::time::timeout(std::time::Duration::from_secs(10), async { - loop { - match view_client_addr.send(access_key_query.clone()).await? { - Ok(Some(query_response)) => return Ok(query_response), - Ok(None) => {} - // TODO: update this once we return structured errors in the - // view_client handlers - Err(err) => { - if err.contains("does not exist") { - return Err(crate::errors::ErrorKind::NotFound(err)); - } - return Err(crate::errors::ErrorKind::InternalError(err)); - } + let access_key_query_response = match view_client_addr.send(access_key_query.clone()).await? { + Ok(query_response) => query_response, + Err(err) => { + return match err { + near_client_primitives::types::QueryError::AccountDoesNotExist(_) + | near_client_primitives::types::QueryError::AccessKeyDoesNotExist(_) => { + Err(crate::errors::ErrorKind::NotFound(err.to_string())) } - tokio::time::sleep(std::time::Duration::from_millis(100)).await; + _ => Err(crate::errors::ErrorKind::InternalError(err.to_string())), } - }) - .await??; + } + }; match access_key_query_response.kind { near_primitives::views::QueryResponseKind::AccessKey(access_key) => Ok(( diff --git a/neard/src/runtime/errors.rs b/neard/src/runtime/errors.rs index db8fa49e5a8..47d2d9a4ba4 100644 --- a/neard/src/runtime/errors.rs +++ b/neard/src/runtime/errors.rs @@ -4,6 +4,12 @@ use near_chain::near_chain_primitives::error::QueryError; #[error(transparent)] pub struct RuntimeQueryError(#[from] pub QueryError); +impl From for QueryError { + fn from(error: RuntimeQueryError) -> Self { + error.0 + } +} + impl From for RuntimeQueryError { fn from(error: node_runtime::state_viewer::errors::ViewAccountError) -> Self { match error { @@ -20,7 +26,7 @@ impl From for RuntimeQuery } } -impl From for QueryError { +impl From for RuntimeQueryError { fn from(error: node_runtime::state_viewer::errors::ViewContractCodeError) -> Self { match error { node_runtime::state_viewer::errors::ViewContractCodeError::InvalidAccountId( @@ -39,7 +45,7 @@ impl From for QueryEr } } -impl From for QueryError { +impl From for RuntimeQueryError { fn from(error: node_runtime::state_viewer::errors::CallFunctionError) -> Self { match error { node_runtime::state_viewer::errors::CallFunctionError::InvalidAccountId(account_id) => { @@ -58,7 +64,7 @@ impl From for QueryError } } -impl From for QueryError { +impl From for RuntimeQueryError { fn from(error: node_runtime::state_viewer::errors::ViewStateError) -> Self { match error { node_runtime::state_viewer::errors::ViewStateError::InvalidAccountId(account_id) => { @@ -70,3 +76,9 @@ impl From for QueryError { } } } + +impl From for RuntimeQueryError { + fn from(error: near_primitives::errors::EpochError) -> Self { + Self(QueryError::InternalError(error.to_string())) + } +} diff --git a/neard/src/runtime/mod.rs b/neard/src/runtime/mod.rs index a790cce54c3..e4219757985 100644 --- a/neard/src/runtime/mod.rs +++ b/neard/src/runtime/mod.rs @@ -1275,7 +1275,9 @@ impl RuntimeAdapter for NightshadeRuntime { let (epoch_height, current_protocol_version) = { let mut epoch_manager = self.epoch_manager.as_ref().write().expect(POISONED_LOCK_ERR); - let epoch_info = epoch_manager.get_epoch_info(&epoch_id)?; + let epoch_info = epoch_manager + .get_epoch_info(&epoch_id) + .map_err(errors::RuntimeQueryError::from)?; (epoch_info.epoch_height, epoch_info.protocol_version) }; diff --git a/runtime/runtime/Cargo.toml b/runtime/runtime/Cargo.toml index 880bc90ea4f..8b9bc4a54bf 100644 --- a/runtime/runtime/Cargo.toml +++ b/runtime/runtime/Cargo.toml @@ -16,6 +16,7 @@ num-bigint = "0.3" num-traits = "0.2.11" hex = "0.4.2" ethereum-types = "0.11.0" +thiserror = "1.0" borsh = "0.8.1" diff --git a/runtime/runtime/src/adapter.rs b/runtime/runtime/src/adapter.rs index b1f79d908e2..8b907d60be0 100644 --- a/runtime/runtime/src/adapter.rs +++ b/runtime/runtime/src/adapter.rs @@ -15,14 +15,14 @@ pub trait ViewRuntimeAdapter { shard_id: ShardId, state_root: MerkleHash, account_id: &AccountId, - ) -> Result>; + ) -> Result; fn view_contract_code( &self, shard_id: ShardId, state_root: MerkleHash, account_id: &AccountId, - ) -> Result>; + ) -> Result; fn call_function( &self, @@ -41,7 +41,7 @@ pub trait ViewRuntimeAdapter { epoch_info_provider: &dyn EpochInfoProvider, current_protocol_version: ProtocolVersion, #[cfg(feature = "protocol_feature_evm")] evm_chain_id: u64, - ) -> Result, Box>; + ) -> Result, crate::state_viewer::errors::CallFunctionError>; fn view_access_key( &self, @@ -49,14 +49,14 @@ pub trait ViewRuntimeAdapter { state_root: MerkleHash, account_id: &AccountId, public_key: &PublicKey, - ) -> Result>; + ) -> Result; fn view_access_keys( &self, shard_id: ShardId, state_root: MerkleHash, account_id: &AccountId, - ) -> Result, Box>; + ) -> Result, crate::state_viewer::errors::ViewAccessKeyError>; fn view_state( &self, @@ -64,5 +64,5 @@ pub trait ViewRuntimeAdapter { state_root: MerkleHash, account_id: &AccountId, prefix: &[u8], - ) -> Result>; + ) -> Result; } diff --git a/runtime/runtime/src/state_viewer/errors.rs b/runtime/runtime/src/state_viewer/errors.rs index 402f095d652..7dbb7a665f8 100644 --- a/runtime/runtime/src/state_viewer/errors.rs +++ b/runtime/runtime/src/state_viewer/errors.rs @@ -1,32 +1,54 @@ +#[derive(thiserror::Error, Debug)] pub enum ViewAccountError { + #[error("Account ID {0} is invalid")] InvalidAccountId(near_primitives::types::AccountId), + #[error("Account ID {0} does not exist")] AccountDoesNotExist(near_primitives::types::AccountId), + #[error("Storage error: {0}")] StorageError(near_primitives::errors::StorageError), } +#[derive(thiserror::Error, Debug)] pub enum ViewContractCodeError { + #[error("Account ID {0} is invalid")] InvalidAccountId(near_primitives::types::AccountId), + #[error("Account ID {0} does not exist")] AccountDoesNotExist(near_primitives::types::AccountId), + #[error("Storage error: {0}")] StorageError(near_primitives::errors::StorageError), + #[error("Contract code for contract ID {0} does not exist")] ContractCodeDoesNotExist(near_primitives::types::AccountId), } +#[derive(thiserror::Error, Debug)] pub enum ViewAccessKeyError { + #[error("Account ID {0} is invalid")] InvalidAccountId(near_primitives::types::AccountId), + #[error("Storage error: {0}")] StorageError(near_primitives::errors::StorageError), + #[error("Access key for public key {0} does not exist")] AccessKeyDoesNotExist(near_crypto::PublicKey), + #[error("Internal error occurred: {0}")] InternalError(String), } +#[derive(thiserror::Error, Debug)] pub enum ViewStateError { + #[error("Account ID {0} is invalid")] InvalidAccountId(near_primitives::types::AccountId), + #[error("Storage error: {0}")] StorageError(near_primitives::errors::StorageError), } +#[derive(thiserror::Error, Debug)] pub enum CallFunctionError { + #[error("Account ID {0} is invalid")] InvalidAccountId(near_primitives::types::AccountId), + #[error("Account ID {0} does not exist")] AccountDoesNotExist(near_primitives::types::AccountId), + #[error("Storage error: {0}")] StorageError(near_primitives::errors::StorageError), + #[error("VM error occurred: {0}")] VMError(String), } From 8347eb712262947e3a0c413406b8e93a159e11b4 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Thu, 11 Feb 2021 18:18:26 +0200 Subject: [PATCH 03/22] Add missed handler from rebasing with master --- chain/client/src/view_client.rs | 34 +++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index 0ce2b62bc8b..c88a6889649 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -852,6 +852,40 @@ impl Handler for ViewClientActor { } } +impl Handler for ViewClientActor { + type Result = Result; + + #[perf] + fn handle(&mut self, msg: GetProtocolConfig, _: &mut Self::Context) -> Self::Result { + let block_header = match msg.0 { + BlockReference::Finality(finality) => { + let block_hash = self.get_block_hash_by_finality(&finality)?; + self.chain.get_block_header(&block_hash).map(Clone::clone) + } + BlockReference::BlockId(BlockId::Height(height)) => { + self.chain.get_header_by_height(height).map(Clone::clone) + } + BlockReference::BlockId(BlockId::Hash(hash)) => { + self.chain.get_block_header(&hash).map(Clone::clone) + } + BlockReference::SyncCheckpoint(sync_checkpoint) => { + if let Some(block_hash) = + self.get_block_hash_by_sync_checkpoint(&sync_checkpoint)? + { + self.chain.get_block_header(&block_hash).map(Clone::clone) + } else { + return Err(GetProtocolConfigError::UnknownBlock(format!( + "{:?}", + sync_checkpoint + ))); + } + } + }?; + let config = self.runtime_adapter.get_protocol_config(block_header.epoch_id())?; + Ok(config.into()) + } +} + impl Handler for ViewClientActor { type Result = NetworkViewClientResponses; From cb778dece32a00ed4e1394cf195a1d978dbf7f80 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Tue, 16 Feb 2021 18:35:08 +0200 Subject: [PATCH 04/22] Refactor new error structs to be more explicit. Runtime call function, view state and access key methods return typical errors instead of QueryError --- chain/chain-primitives/src/error.rs | 20 +-- chain/chain/src/lib.rs | 3 +- chain/client-primitives/src/types.rs | 40 +++--- chain/client/src/view_client.rs | 36 ++--- chain/jsonrpc-primitives/src/types/query.rs | 92 +++++++----- chain/jsonrpc/src/lib.rs | 5 +- chain/rosetta-rpc/src/utils.rs | 6 +- neard/src/runtime/errors.rs | 87 ++++++----- neard/src/runtime/mod.rs | 152 +++++++++----------- runtime/runtime/src/state_viewer/errors.rs | 92 +++++------- runtime/runtime/src/state_viewer/mod.rs | 64 +++++---- 11 files changed, 303 insertions(+), 294 deletions(-) diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index dc89eaf0269..8dec2d609da 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -14,18 +14,18 @@ use near_primitives::types::{BlockHeight, EpochId, ShardId}; #[derive(thiserror::Error, Debug)] pub enum QueryError { - #[error("Invalid account ID {0}")] - InvalidAccount(near_primitives::types::AccountId), - #[error("Account ID {0} does not exist while viewing")] - AccountDoesNotExist(near_primitives::types::AccountId), - #[error("Contract ID {0} code does not exist while viewing")] - ContractCodeDoesNotExist(near_primitives::types::AccountId), - #[error("Access key for public key {0} does not exist while viewing")] - AccessKeyDoesNotExist(String), + #[error("Invalid account ID #{requested_account_id}")] + InvalidAccount { requested_account_id: near_primitives::types::AccountId }, + #[error("Account ID #{requested_account_id} does not exist while viewing")] + AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, + #[error("Contract ID #{contract_account_id} code does not exist while viewing")] + ContractCodeDoesNotExist { contract_account_id: near_primitives::types::AccountId }, + #[error("Access key for public key #{public_key} does not exist while viewing")] + AccessKeyDoesNotExist { public_key: String }, #[error("Storage error occurred: {0:?}")] StorageError(#[from] near_primitives::errors::StorageError), - #[error("Internal error occurred: {0}")] - InternalError(String), + #[error("Internal error occurred: #{error_message}")] + InternalError { error_message: String }, #[error("VM error occurred: {0}")] VMError(String), } diff --git a/chain/chain/src/lib.rs b/chain/chain/src/lib.rs index 11a4b2b18e6..96e8d57a3e1 100644 --- a/chain/chain/src/lib.rs +++ b/chain/chain/src/lib.rs @@ -4,8 +4,7 @@ extern crate lazy_static; pub use chain::{collect_receipts, Chain, MAX_ORPHAN_SIZE}; pub use doomslug::{Doomslug, DoomslugBlockProductionReadiness, DoomslugThresholdMode}; pub use lightclient::{create_light_client_block_view, get_epoch_block_producers_view}; -pub use near_chain_primitives; -pub use near_chain_primitives::{Error, ErrorKind}; +pub use near_chain_primitives::{self, Error, ErrorKind}; pub use store::{ChainStore, ChainStoreAccess, ChainStoreUpdate}; pub use store_validator::{ErrorMessage, StoreValidator}; pub use types::{ diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs index d1e01b25507..55836fcb8e5 100644 --- a/chain/client-primitives/src/types.rs +++ b/chain/client-primitives/src/types.rs @@ -272,35 +272,39 @@ impl Message for Query { #[derive(thiserror::Error, Debug)] pub enum QueryError { - #[error("IO Error: {0}")] - IOError(String), + #[error("IO Error: #{error_message}")] + IOError { error_message: String }, #[error("There are no fully synchronized blocks yet")] NotSyncedYet, - #[error("The node does not track the shard")] - DoesNotTrackShard, - #[error("Invalid account ID {0}")] - InvalidAccount(near_primitives::types::AccountId), - #[error("Account ID {0} has never been observed on the node")] - AccountDoesNotExist(near_primitives::types::AccountId), - #[error("Contract code for contract ID {0} has never been observed on the node")] - ContractCodeDoesNotExist(near_primitives::types::AccountId), - #[error("Access key for public key {0} has never been observed on the node")] - AccessKeyDoesNotExist(String), - #[error("VM error occurred: {0}")] - VMError(String), + #[error("The node does not track the shard #{requested_shard_id} where the requested account resides")] + UnavailableShard { requested_shard_id: near_primitives::types::ShardId }, + #[error("Invalid account ID #{requested_account_id}")] + InvalidAccount { requested_account_id: near_primitives::types::AccountId }, + #[error("Account ID #{requested_account_id} has never been observed on the node")] + AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, + #[error( + "Contract code for contract ID #{contract_account_id} has never been observed on the node" + )] + ContractCodeDoesNotExist { contract_account_id: near_primitives::types::AccountId }, + #[error("Access key for public key #{public_key} has never been observed on the node")] + AccessKeyDoesNotExist { public_key: String }, + #[error("VM error occurred: #{error_message}")] + VMError { error_message: String }, // NOTE: Currently, the underlying errors are too broad, and while we tried to handle // expected cases, we cannot statically guarantee that no other errors will be returned // in the future. // TODO #3851: Remove this variant once we can exhaustively match all the underlying errors - #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: {0}")] - Unreachable(String), + #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: #{error_message}")] + Unreachable { error_message: String }, } impl From for QueryError { fn from(error: near_chain_primitives::Error) -> Self { match error.kind() { - near_chain_primitives::ErrorKind::IOErr(s) => Self::IOError(s), - _ => Self::Unreachable(error.to_string()), + near_chain_primitives::ErrorKind::IOErr(error_message) => { + Self::IOError { error_message } + } + _ => Self::Unreachable { error_message: error.to_string() }, } } } diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index c88a6889649..dbb0cc8e035 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -250,7 +250,7 @@ impl ViewClientActor { ) .map_err(RuntimeQueryError::from)?) } else { - Err(QueryError::DoesNotTrackShard) + Err(QueryError::UnavailableShard { requested_shard_id: shard_id }) } } @@ -455,25 +455,27 @@ struct RuntimeQueryError(#[from] pub near_chain::near_chain_primitives::error::Q impl From for near_client_primitives::types::QueryError { fn from(error: RuntimeQueryError) -> Self { match error.0 { - near_chain::near_chain_primitives::error::QueryError::InternalError(s) => { - Self::Unreachable(s) - } - near_chain::near_chain_primitives::error::QueryError::InvalidAccount(account_id) => { - Self::InvalidAccount(account_id) - } - near_chain::near_chain_primitives::error::QueryError::AccountDoesNotExist( - account_id, - ) => Self::AccountDoesNotExist(account_id), - near_chain::near_chain_primitives::error::QueryError::ContractCodeDoesNotExist( - account_id, - ) => Self::ContractCodeDoesNotExist(account_id), - near_chain::near_chain_primitives::error::QueryError::AccessKeyDoesNotExist( + near_chain::near_chain_primitives::error::QueryError::InternalError { + error_message, + } => Self::Unreachable { error_message }, + near_chain::near_chain_primitives::error::QueryError::InvalidAccount { + requested_account_id, + } => Self::InvalidAccount { requested_account_id }, + near_chain::near_chain_primitives::error::QueryError::AccountDoesNotExist { + requested_account_id, + } => Self::AccountDoesNotExist { requested_account_id }, + near_chain::near_chain_primitives::error::QueryError::ContractCodeDoesNotExist { + contract_account_id, + } => Self::ContractCodeDoesNotExist { contract_account_id }, + near_chain::near_chain_primitives::error::QueryError::AccessKeyDoesNotExist { public_key, - ) => Self::AccessKeyDoesNotExist(public_key), + } => Self::AccessKeyDoesNotExist { public_key }, near_chain::near_chain_primitives::error::QueryError::StorageError(storage_error) => { - Self::IOError(storage_error.to_string()) + Self::IOError { error_message: storage_error.to_string() } + } + near_chain::near_chain_primitives::error::QueryError::VMError(error_message) => { + Self::VMError { error_message } } - near_chain::near_chain_primitives::error::QueryError::VMError(s) => Self::VMError(s), } } } diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index cf73d511afd..0f202c74a65 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -1,6 +1,9 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; +/// Max size of the query path (soft-deprecated) +const QUERY_DATA_MAX_SIZE: usize = 10 * 1024; + #[derive(Serialize, Deserialize)] pub struct RpcQueryRequest { #[serde(flatten)] @@ -11,30 +14,32 @@ pub struct RpcQueryRequest { #[derive(thiserror::Error, Debug)] pub enum RpcQueryError { - #[error("IO Error: {0}")] - IOError(String), - #[error("There are no fully synchronized blocks yet")] - NotSyncedYet, + #[error("IO Error: #{error_message}")] + IOError { error_message: String }, + #[error("There are no fully synchronized blocks on the node yet")] + NoSyncedBlocks, #[error("The node does not track the shard")] - DoesNotTrackShard, - #[error("Invalid account ID {0}")] - InvalidAccount(near_primitives::types::AccountId), - #[error("Account ID {0} has never been observed on the node")] - AccountDoesNotExist(near_primitives::types::AccountId), - #[error("Contract code for contract ID {0} has never been observed on the node")] - ContractCodeDoesNotExist(near_primitives::types::AccountId), - #[error("Access key for public key {0} has never been observed on the node")] - AccessKeyDoesNotExist(String), - #[error("VM error occurred: {0}")] - VMError(String), - #[error("The node reached its limits. Try again later. More details: {0}")] - InternalError(String), + DoesNotTrackShard { requested_shard_id: near_primitives::types::ShardId }, + #[error("Account ID #{requested_account_id} is invalid")] + InvalidAccount { requested_account_id: near_primitives::types::AccountId }, + #[error("account #{requested_account_id} does not exist while viewing")] + UnknownAccount { requested_account_id: near_primitives::types::AccountId }, + #[error( + "Contract code for contract ID #{contract_account_id} has never been observed on the node" + )] + NoContractCode { contract_account_id: near_primitives::types::AccountId }, + #[error("Access key for public key #{public_key} has never been observed on the node")] + UnknownAccessKey { public_key: String }, + #[error("Function call returned an error: #{vm_error}")] + FunctionCall { vm_error: String }, + #[error("The node reached its limits. Try again later. More details: #{error_message}")] + InternalError { error_message: String }, // NOTE: Currently, the underlying errors are too broad, and while we tried to handle // expected cases, we cannot statically guarantee that no other errors will be returned // in the future. // TODO #3851: Remove this variant once we can exhaustively match all the underlying errors - #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: {0}")] - Unreachable(String), + #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: #{error_message}")] + Unreachable { error_message: String }, } #[derive(Serialize, Deserialize)] @@ -48,9 +53,19 @@ impl RpcQueryRequest { let query_request = if let Ok((path, data)) = crate::utils::parse_params::<(String, String)>(value.clone()) { + // 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 = near_primitives_core::serialize::from_base(&data) .map_err(|err| crate::errors::RpcParseError(err.to_string()))?; let query_data_size = path.len() + data.len(); + if query_data_size > QUERY_DATA_MAX_SIZE { + return Err(crate::errors::RpcParseError(format!( + "Query data size {} is too large", + query_data_size + ))); + } let mut path_parts = path.splitn(3, '/'); let make_err = @@ -95,6 +110,7 @@ impl RpcQueryRequest { ))) } }; + // Use Finality::None here to make backward compatibility tests work RpcQueryRequest { request, block_reference: near_primitives::types::BlockReference::latest(), @@ -109,28 +125,34 @@ impl RpcQueryRequest { impl From for RpcQueryError { fn from(error: near_client_primitives::types::QueryError) -> Self { match error { - near_client_primitives::types::QueryError::IOError(s) => Self::InternalError(s), - near_client_primitives::types::QueryError::NotSyncedYet => Self::NotSyncedYet, - near_client_primitives::types::QueryError::DoesNotTrackShard => Self::DoesNotTrackShard, - near_client_primitives::types::QueryError::InvalidAccount(account_id) => { - Self::InvalidAccount(account_id) + near_client_primitives::types::QueryError::IOError { error_message } => { + Self::InternalError { error_message } + } + near_client_primitives::types::QueryError::NotSyncedYet => Self::NoSyncedBlocks, + near_client_primitives::types::QueryError::UnavailableShard { requested_shard_id } => { + Self::DoesNotTrackShard { requested_shard_id } } - near_client_primitives::types::QueryError::AccountDoesNotExist(account_id) => { - Self::AccountDoesNotExist(account_id) + near_client_primitives::types::QueryError::InvalidAccount { requested_account_id } => { + Self::InvalidAccount { requested_account_id } } - near_client_primitives::types::QueryError::ContractCodeDoesNotExist(account_id) => { - Self::ContractCodeDoesNotExist(account_id) + near_client_primitives::types::QueryError::AccountDoesNotExist { + requested_account_id, + } => Self::UnknownAccount { requested_account_id }, + near_client_primitives::types::QueryError::ContractCodeDoesNotExist { + contract_account_id, + } => Self::NoContractCode { contract_account_id }, + near_client_primitives::types::QueryError::AccessKeyDoesNotExist { public_key } => { + Self::UnknownAccessKey { public_key } } - near_client_primitives::types::QueryError::AccessKeyDoesNotExist(public_key) => { - Self::AccessKeyDoesNotExist(public_key) + near_client_primitives::types::QueryError::VMError { error_message } => { + Self::FunctionCall { vm_error: error_message } } - near_client_primitives::types::QueryError::VMError(s) => Self::VMError(s), - near_client_primitives::types::QueryError::Unreachable(s) => { + near_client_primitives::types::QueryError::Unreachable { error_message } => { near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, - &["RpcQueryError", &s], + &["RpcQueryError", &error_message], ); - Self::Unreachable(s) + Self::Unreachable { error_message } } } } @@ -146,6 +168,6 @@ impl From for crate::errors::RpcError { impl From for RpcQueryError { fn from(error: actix::MailboxError) -> Self { - Self::InternalError(error.to_string()) + Self::InternalError { error_message: error.to_string() } } } diff --git a/chain/jsonrpc/src/lib.rs b/chain/jsonrpc/src/lib.rs index 324b388eaba..e50cf927c8e 100644 --- a/chain/jsonrpc/src/lib.rs +++ b/chain/jsonrpc/src/lib.rs @@ -46,9 +46,6 @@ use near_runtime_utils::is_valid_account_id; mod metrics; -/// Max size of the query path (soft-deprecated) -const QUERY_DATA_MAX_SIZE: usize = 10 * 1024; - #[derive(Serialize, Deserialize, Clone, Copy, Debug)] pub struct RpcPollingConfig { pub polling_interval: Duration, @@ -588,7 +585,7 @@ impl JsonRpcHandler { near_jsonrpc_primitives::types::query::RpcQueryError, > { let query = Query::new(request_data.block_reference, request_data.request); - let query_response = self.view_client_addr.send(query.clone()).await??; + let query_response = self.view_client_addr.send(query).await??; Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { query_response }) } diff --git a/chain/rosetta-rpc/src/utils.rs b/chain/rosetta-rpc/src/utils.rs index 828d5003121..db4e6e7d0dc 100644 --- a/chain/rosetta-rpc/src/utils.rs +++ b/chain/rosetta-rpc/src/utils.rs @@ -316,7 +316,7 @@ pub(crate) async fn query_account( let account_info_response = match view_client_addr.send(query.clone()).await? { Ok(query_response) => query_response, Err(err) => match err { - near_client_primitives::types::QueryError::AccountDoesNotExist(_) => { + near_client_primitives::types::QueryError::AccountDoesNotExist { .. } => { return Err(crate::errors::ErrorKind::NotFound(err.to_string())) } _ => return Err(crate::errors::ErrorKind::InternalError(err.to_string())), @@ -395,8 +395,8 @@ pub(crate) async fn query_access_key( Ok(query_response) => query_response, Err(err) => { return match err { - near_client_primitives::types::QueryError::AccountDoesNotExist(_) - | near_client_primitives::types::QueryError::AccessKeyDoesNotExist(_) => { + near_client_primitives::types::QueryError::AccountDoesNotExist { .. } + | near_client_primitives::types::QueryError::AccessKeyDoesNotExist { .. } => { Err(crate::errors::ErrorKind::NotFound(err.to_string())) } _ => Err(crate::errors::ErrorKind::InternalError(err.to_string())), diff --git a/neard/src/runtime/errors.rs b/neard/src/runtime/errors.rs index 47d2d9a4ba4..ba4ce1df983 100644 --- a/neard/src/runtime/errors.rs +++ b/neard/src/runtime/errors.rs @@ -1,24 +1,26 @@ use near_chain::near_chain_primitives::error::QueryError; +// This wrapper struct is necessary because we cannot implement +// From<> trait for the original QueryError struct since it is a foreign type #[derive(thiserror::Error, Debug)] #[error(transparent)] -pub struct RuntimeQueryError(#[from] pub QueryError); +pub struct WrappedQueryError(pub QueryError); -impl From for QueryError { - fn from(error: RuntimeQueryError) -> Self { +impl From for QueryError { + fn from(error: WrappedQueryError) -> Self { error.0 } } -impl From for RuntimeQueryError { +impl From for WrappedQueryError { fn from(error: node_runtime::state_viewer::errors::ViewAccountError) -> Self { match error { - node_runtime::state_viewer::errors::ViewAccountError::InvalidAccountId(account_id) => { - Self(QueryError::InvalidAccount(account_id)) - } - node_runtime::state_viewer::errors::ViewAccountError::AccountDoesNotExist( - account_id, - ) => Self(QueryError::AccountDoesNotExist(account_id)), + node_runtime::state_viewer::errors::ViewAccountError::InvalidAccountId { + requested_account_id, + } => Self(QueryError::InvalidAccount { requested_account_id }), + node_runtime::state_viewer::errors::ViewAccountError::AccountDoesNotExist { + requested_account_id, + } => Self(QueryError::AccountDoesNotExist { requested_account_id }), node_runtime::state_viewer::errors::ViewAccountError::StorageError(storage_error) => { Self(QueryError::StorageError(storage_error)) } @@ -26,34 +28,34 @@ impl From for RuntimeQuery } } -impl From for RuntimeQueryError { +impl From for WrappedQueryError { fn from(error: node_runtime::state_viewer::errors::ViewContractCodeError) -> Self { match error { - node_runtime::state_viewer::errors::ViewContractCodeError::InvalidAccountId( - account_id, - ) => Self(QueryError::InvalidAccount(account_id)), - node_runtime::state_viewer::errors::ViewContractCodeError::AccountDoesNotExist( - account_id, - ) => Self(QueryError::AccountDoesNotExist(account_id)), + node_runtime::state_viewer::errors::ViewContractCodeError::InvalidAccountId { + requested_account_id, + } => Self(QueryError::InvalidAccount { requested_account_id }), + node_runtime::state_viewer::errors::ViewContractCodeError::AccountDoesNotExist { + requested_account_id, + } => Self(QueryError::AccountDoesNotExist { requested_account_id }), node_runtime::state_viewer::errors::ViewContractCodeError::StorageError( storage_error, ) => Self(QueryError::StorageError(storage_error)), - node_runtime::state_viewer::errors::ViewContractCodeError::ContractCodeDoesNotExist( - contract_id, - ) => Self(QueryError::ContractCodeDoesNotExist(contract_id)), + node_runtime::state_viewer::errors::ViewContractCodeError::NoContractCode { + contract_account_id, + } => Self(QueryError::ContractCodeDoesNotExist { contract_account_id }), } } } -impl From for RuntimeQueryError { +impl From for WrappedQueryError { fn from(error: node_runtime::state_viewer::errors::CallFunctionError) -> Self { match error { - node_runtime::state_viewer::errors::CallFunctionError::InvalidAccountId(account_id) => { - Self(QueryError::InvalidAccount(account_id)) - } - node_runtime::state_viewer::errors::CallFunctionError::AccountDoesNotExist( - account_id, - ) => Self(QueryError::AccountDoesNotExist(account_id)), + node_runtime::state_viewer::errors::CallFunctionError::InvalidAccountId { + requested_account_id, + } => Self(QueryError::InvalidAccount { requested_account_id }), + node_runtime::state_viewer::errors::CallFunctionError::AccountDoesNotExist { + requested_account_id, + } => Self(QueryError::AccountDoesNotExist { requested_account_id }), node_runtime::state_viewer::errors::CallFunctionError::StorageError(storage_error) => { Self(QueryError::StorageError(storage_error)) } @@ -64,12 +66,12 @@ impl From for RuntimeQuer } } -impl From for RuntimeQueryError { +impl From for WrappedQueryError { fn from(error: node_runtime::state_viewer::errors::ViewStateError) -> Self { match error { - node_runtime::state_viewer::errors::ViewStateError::InvalidAccountId(account_id) => { - Self(QueryError::InvalidAccount(account_id)) - } + node_runtime::state_viewer::errors::ViewStateError::InvalidAccountId { + requested_account_id, + } => Self(QueryError::InvalidAccount { requested_account_id }), node_runtime::state_viewer::errors::ViewStateError::StorageError(storage_error) => { Self(QueryError::StorageError(storage_error)) } @@ -77,8 +79,27 @@ impl From for RuntimeQueryEr } } -impl From for RuntimeQueryError { +impl From for WrappedQueryError { + fn from(error: node_runtime::state_viewer::errors::ViewAccessKeyError) -> Self { + match error { + node_runtime::state_viewer::errors::ViewAccessKeyError::InvalidAccountId { + requested_account_id, + } => Self(QueryError::InvalidAccount { requested_account_id }), + node_runtime::state_viewer::errors::ViewAccessKeyError::StorageError(storage_error) => { + Self(QueryError::StorageError(storage_error)) + } + node_runtime::state_viewer::errors::ViewAccessKeyError::AccessKeyDoesNotExist { + public_key, + } => Self(QueryError::AccessKeyDoesNotExist { public_key: public_key.to_string() }), + node_runtime::state_viewer::errors::ViewAccessKeyError::InternalError { + error_message, + } => Self(QueryError::InternalError { error_message }), + } + } +} + +impl From for WrappedQueryError { fn from(error: near_primitives::errors::EpochError) -> Self { - Self(QueryError::InternalError(error.to_string())) + Self(QueryError::InternalError { error_message: error.to_string() }) } } diff --git a/neard/src/runtime/mod.rs b/neard/src/runtime/mod.rs index e4219757985..be68c90b26f 100644 --- a/neard/src/runtime/mod.rs +++ b/neard/src/runtime/mod.rs @@ -33,20 +33,18 @@ use near_primitives::receipt::Receipt; use near_primitives::sharding::ChunkHash; use near_primitives::state_record::StateRecord; use near_primitives::transaction::SignedTransaction; -use near_primitives::trie_key::trie_key_parsers; use near_primitives::types::{ AccountId, ApprovalStake, Balance, BlockHeight, EpochHeight, EpochId, EpochInfoProvider, Gas, MerkleHash, NumShards, ShardId, StateChangeCause, StateRoot, StateRootNode, ValidatorStake, }; use near_primitives::version::ProtocolVersion; use near_primitives::views::{ - AccessKeyInfoView, CallResult, EpochValidatorInfo, QueryError, QueryRequest, QueryResponse, + AccessKeyInfoView, CallResult, EpochValidatorInfo, QueryRequest, QueryResponse, QueryResponseKind, ViewApplyState, ViewStateResult, }; use near_store::{ - get_access_key_raw, get_genesis_hash, get_genesis_state_roots, set_genesis_hash, - set_genesis_state_roots, ColState, PartialStorage, ShardTries, Store, - StoreCompiledContractCache, Trie, WrappedTrieChanges, + get_genesis_hash, get_genesis_state_roots, set_genesis_hash, set_genesis_state_roots, ColState, + PartialStorage, ShardTries, Store, StoreCompiledContractCache, Trie, WrappedTrieChanges, }; use node_runtime::adapter::ViewRuntimeAdapter; use node_runtime::state_viewer::TrieViewer; @@ -1253,7 +1251,7 @@ impl RuntimeAdapter for NightshadeRuntime { QueryRequest::ViewAccount { account_id } => { let account = self .view_account(shard_id, *state_root, account_id) - .map_err(errors::RuntimeQueryError::from)?; + .map_err(errors::WrappedQueryError::from)?; Ok(QueryResponse { kind: QueryResponseKind::ViewAccount(account.into()), block_height, @@ -1263,7 +1261,7 @@ impl RuntimeAdapter for NightshadeRuntime { QueryRequest::ViewCode { account_id } => { let contract_code = self .view_contract_code(shard_id, *state_root, account_id) - .map_err(errors::RuntimeQueryError::from)?; + .map_err(errors::WrappedQueryError::from)?; Ok(QueryResponse { kind: QueryResponseKind::ViewCode(contract_code.into()), block_height, @@ -1277,98 +1275,76 @@ impl RuntimeAdapter for NightshadeRuntime { self.epoch_manager.as_ref().write().expect(POISONED_LOCK_ERR); let epoch_info = epoch_manager .get_epoch_info(&epoch_id) - .map_err(errors::RuntimeQueryError::from)?; + .map_err(errors::WrappedQueryError::from)?; (epoch_info.epoch_height, epoch_info.protocol_version) }; - match self.call_function( - shard_id, - *state_root, - block_height, - block_timestamp, - prev_block_hash, - block_hash, - epoch_height, - epoch_id, - account_id, - method_name, - args.as_ref(), - &mut logs, - &self.epoch_manager, - current_protocol_version, - #[cfg(feature = "protocol_feature_evm")] - self.evm_chain_id(), - ) { - Ok(result) => Ok(QueryResponse { - kind: QueryResponseKind::CallResult(CallResult { result, logs }), + let call_function_result = self + .call_function( + shard_id, + *state_root, block_height, - block_hash: *block_hash, - }), - Err(err) => Ok(QueryResponse { - kind: QueryResponseKind::Error(QueryError { error: err.to_string(), logs }), - block_height, - block_hash: *block_hash, + block_timestamp, + prev_block_hash, + block_hash, + epoch_height, + epoch_id, + account_id, + method_name, + args.as_ref(), + &mut logs, + &self.epoch_manager, + current_protocol_version, + #[cfg(feature = "protocol_feature_evm")] + self.evm_chain_id(), + ) + .map_err(errors::WrappedQueryError::from)?; + Ok(QueryResponse { + kind: QueryResponseKind::CallResult(CallResult { + result: call_function_result, + logs, }), - } + block_height, + block_hash: *block_hash, + }) } QueryRequest::ViewState { account_id, prefix } => { - match self.view_state(shard_id, *state_root, account_id, prefix.as_ref()) { - Ok(result) => Ok(QueryResponse { - kind: QueryResponseKind::ViewState(result), - block_height, - block_hash: *block_hash, - }), - Err(err) => Ok(QueryResponse { - kind: QueryResponseKind::Error(QueryError { - error: err.to_string(), - logs: vec![], - }), - block_height, - block_hash: *block_hash, - }), - } + let view_state_result = self + .view_state(shard_id, *state_root, account_id, prefix.as_ref()) + .map_err(errors::WrappedQueryError::from)?; + Ok(QueryResponse { + kind: QueryResponseKind::ViewState(view_state_result), + block_height, + block_hash: *block_hash, + }) } QueryRequest::ViewAccessKeyList { account_id } => { - match self.view_access_keys(shard_id, *state_root, account_id) { - Ok(result) => Ok(QueryResponse { - kind: QueryResponseKind::AccessKeyList( - result - .into_iter() - .map(|(public_key, access_key)| AccessKeyInfoView { - public_key, - access_key: access_key.into(), - }) - .collect(), - ), - block_height, - block_hash: *block_hash, - }), - Err(err) => Ok(QueryResponse { - kind: QueryResponseKind::Error(QueryError { - error: err.to_string(), - logs: vec![], - }), - block_height, - block_hash: *block_hash, - }), - } + let access_key_list = self + .view_access_keys(shard_id, *state_root, account_id) + .map_err(errors::WrappedQueryError::from)?; + Ok(QueryResponse { + kind: QueryResponseKind::AccessKeyList( + access_key_list + .into_iter() + .map(|(public_key, access_key)| AccessKeyInfoView { + public_key, + access_key: access_key.into(), + }) + .collect(), + ), + block_height, + block_hash: *block_hash, + }) } QueryRequest::ViewAccessKey { account_id, public_key } => { - match self.view_access_key(shard_id, *state_root, account_id, public_key) { - Ok(access_key) => Ok(QueryResponse { - kind: QueryResponseKind::AccessKey(access_key.into()), - block_height, - block_hash: *block_hash, - }), - Err(err) => Ok(QueryResponse { - kind: QueryResponseKind::Error(QueryError { - error: err.to_string(), - logs: vec![], - }), - block_height, - block_hash: *block_hash, - }), - } + let access_key = self + .view_access_key(shard_id, *state_root, account_id, public_key) + .map_err(errors::WrappedQueryError::from)?; + Ok(QueryResponse { + kind: QueryResponseKind::AccessKey(access_key.into()), + block_height, + block_hash: *block_hash, + }) } } } diff --git a/runtime/runtime/src/state_viewer/errors.rs b/runtime/runtime/src/state_viewer/errors.rs index 7dbb7a665f8..453a1dea446 100644 --- a/runtime/runtime/src/state_viewer/errors.rs +++ b/runtime/runtime/src/state_viewer/errors.rs @@ -1,95 +1,67 @@ #[derive(thiserror::Error, Debug)] pub enum ViewAccountError { - #[error("Account ID {0} is invalid")] - InvalidAccountId(near_primitives::types::AccountId), - #[error("Account ID {0} does not exist")] - AccountDoesNotExist(near_primitives::types::AccountId), + #[error("Account ID #{requested_account_id} is invalid")] + InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, + #[error("Account ID #{requested_account_id} does not exist")] + AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, #[error("Storage error: {0}")] - StorageError(near_primitives::errors::StorageError), + StorageError(#[from] near_primitives::errors::StorageError), } #[derive(thiserror::Error, Debug)] pub enum ViewContractCodeError { - #[error("Account ID {0} is invalid")] - InvalidAccountId(near_primitives::types::AccountId), - #[error("Account ID {0} does not exist")] - AccountDoesNotExist(near_primitives::types::AccountId), + #[error("Account ID #{requested_account_id} is invalid")] + InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, + #[error("Account ID #{requested_account_id} does not exist")] + AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, #[error("Storage error: {0}")] - StorageError(near_primitives::errors::StorageError), - #[error("Contract code for contract ID {0} does not exist")] - ContractCodeDoesNotExist(near_primitives::types::AccountId), + StorageError(#[from] near_primitives::errors::StorageError), + #[error("Contract code for contract ID #{contract_account_id} does not exist")] + NoContractCode { contract_account_id: near_primitives::types::AccountId }, } #[derive(thiserror::Error, Debug)] pub enum ViewAccessKeyError { - #[error("Account ID {0} is invalid")] - InvalidAccountId(near_primitives::types::AccountId), + #[error("Account ID #{requested_account_id} is invalid")] + InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Storage error: {0}")] - StorageError(near_primitives::errors::StorageError), - #[error("Access key for public key {0} does not exist")] - AccessKeyDoesNotExist(near_crypto::PublicKey), - #[error("Internal error occurred: {0}")] - InternalError(String), + StorageError(#[from] near_primitives::errors::StorageError), + #[error("Access key for public key #{public_key} does not exist")] + AccessKeyDoesNotExist { public_key: near_crypto::PublicKey }, + #[error("Internal error occurred: #{error_message}")] + InternalError { error_message: String }, } #[derive(thiserror::Error, Debug)] pub enum ViewStateError { - #[error("Account ID {0} is invalid")] - InvalidAccountId(near_primitives::types::AccountId), + #[error("Account ID #{requested_account_id} is invalid")] + InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Storage error: {0}")] - StorageError(near_primitives::errors::StorageError), + StorageError(#[from] near_primitives::errors::StorageError), } #[derive(thiserror::Error, Debug)] pub enum CallFunctionError { - #[error("Account ID {0} is invalid")] - InvalidAccountId(near_primitives::types::AccountId), - #[error("Account ID {0} does not exist")] - AccountDoesNotExist(near_primitives::types::AccountId), + #[error("Account ID #{requested_account_id} is invalid")] + InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, + #[error("Account ID #{requested_account_id} does not exist")] + AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, #[error("Storage error: {0}")] - StorageError(near_primitives::errors::StorageError), + StorageError(#[from] near_primitives::errors::StorageError), #[error("VM error occurred: {0}")] VMError(String), } -impl From for ViewAccountError { - fn from(storage_error: near_primitives::errors::StorageError) -> Self { - Self::StorageError(storage_error) - } -} - impl From for ViewContractCodeError { fn from(view_account_error: ViewAccountError) -> Self { match view_account_error { - ViewAccountError::InvalidAccountId(account_id) => Self::AccountDoesNotExist(account_id), - ViewAccountError::AccountDoesNotExist(account_id) => { - Self::AccountDoesNotExist(account_id) + ViewAccountError::InvalidAccountId { requested_account_id } => { + Self::AccountDoesNotExist { requested_account_id } + } + ViewAccountError::AccountDoesNotExist { requested_account_id } => { + Self::AccountDoesNotExist { requested_account_id } } ViewAccountError::StorageError(storage_error) => Self::StorageError(storage_error), } } } - -impl From for ViewContractCodeError { - fn from(storage_error: near_primitives::errors::StorageError) -> Self { - Self::StorageError(storage_error) - } -} - -impl From for ViewAccessKeyError { - fn from(storage_error: near_primitives::errors::StorageError) -> Self { - Self::StorageError(storage_error) - } -} - -impl From for ViewStateError { - fn from(storage_error: near_primitives::errors::StorageError) -> Self { - Self::StorageError(storage_error) - } -} - -impl From for CallFunctionError { - fn from(storage_error: near_primitives::errors::StorageError) -> Self { - Self::StorageError(storage_error) - } -} diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index 4296c3a6d4d..afcab3fe245 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -35,11 +35,16 @@ impl TrieViewer { account_id: &AccountId, ) -> Result { if !is_valid_account_id(account_id) { - return Err(errors::ViewAccountError::InvalidAccountId(account_id.clone())); + return Err(errors::ViewAccountError::InvalidAccountId { + requested_account_id: account_id.clone(), + }); } - get_account(state_update, &account_id)? - .ok_or_else(|| errors::ViewAccountError::AccountDoesNotExist(account_id.clone())) + get_account(state_update, &account_id)?.ok_or_else(|| { + errors::ViewAccountError::AccountDoesNotExist { + requested_account_id: account_id.clone(), + } + }) } pub fn view_contract_code( @@ -49,7 +54,9 @@ impl TrieViewer { ) -> Result { let account = self.view_account(state_update, account_id)?; get_code(state_update, account_id, Some(account.code_hash))?.ok_or_else(|| { - errors::ViewContractCodeError::ContractCodeDoesNotExist(account_id.clone()) + errors::ViewContractCodeError::NoContractCode { + contract_account_id: account_id.clone(), + } }) } @@ -60,11 +67,14 @@ impl TrieViewer { public_key: &PublicKey, ) -> Result { if !is_valid_account_id(account_id) { - return Err(errors::ViewAccessKeyError::InvalidAccountId(account_id.clone())); + return Err(errors::ViewAccessKeyError::InvalidAccountId { + requested_account_id: account_id.clone(), + }); } - get_access_key(state_update, account_id, public_key)? - .ok_or_else(|| errors::ViewAccessKeyError::AccessKeyDoesNotExist(public_key.clone())) + get_access_key(state_update, account_id, public_key)?.ok_or_else(|| { + errors::ViewAccessKeyError::AccessKeyDoesNotExist { public_key: public_key.clone() } + }) } pub fn view_access_keys( @@ -73,34 +83,33 @@ impl TrieViewer { account_id: &AccountId, ) -> Result, errors::ViewAccessKeyError> { if !is_valid_account_id(account_id) { - return Err(errors::ViewAccessKeyError::InvalidAccountId(account_id.clone())); + return Err(errors::ViewAccessKeyError::InvalidAccountId { + requested_account_id: account_id.clone(), + }); } let prefix = trie_key_parsers::get_raw_prefix_for_access_keys(account_id); let raw_prefix: &[u8] = prefix.as_ref(); - let access_keys = match state_update.iter(&prefix) { - Ok(iter) => iter + let access_keys = + state_update + .iter(&prefix)? .map(|key| { let key = key?; let public_key = &key[raw_prefix.len()..]; let access_key = near_store::get_access_key_raw(&state_update, &key)? - .ok_or_else(|| { - errors::ViewAccessKeyError::InternalError( - "Unexpected missing key from iterator".to_string(), - ) + .ok_or_else(|| errors::ViewAccessKeyError::InternalError { + error_message: "Unexpected missing key from iterator".to_string(), })?; PublicKey::try_from_slice(public_key) - .map_err(|_| { - errors::ViewAccessKeyError::InternalError(format!( + .map_err(|_| errors::ViewAccessKeyError::InternalError { + error_message: format!( "Unexpected invalid public key {:?} received from store", public_key - )) + ), }) .map(|key| (key, access_key)) }) - .collect::, errors::ViewAccessKeyError>>(), - Err(e) => Err(e.into()), - }; + .collect::, errors::ViewAccessKeyError>>(); access_keys } @@ -111,7 +120,9 @@ impl TrieViewer { prefix: &[u8], ) -> Result { if !is_valid_account_id(account_id) { - return Err(errors::ViewStateError::InvalidAccountId(account_id.clone())); + return Err(errors::ViewStateError::InvalidAccountId { + requested_account_id: account_id.clone(), + }); } let mut values = vec![]; let query = trie_key_parsers::get_raw_prefix_for_contract_data(account_id, prefix); @@ -145,11 +156,16 @@ impl TrieViewer { ) -> Result, errors::CallFunctionError> { let now = Instant::now(); if !is_valid_account_id(contract_id) { - return Err(errors::CallFunctionError::InvalidAccountId(contract_id.clone())); + return Err(errors::CallFunctionError::InvalidAccountId { + requested_account_id: contract_id.clone(), + }); } let root = state_update.get_root(); - let mut account = get_account(&state_update, contract_id)? - .ok_or_else(|| errors::CallFunctionError::AccountDoesNotExist(contract_id.clone()))?; + let mut account = get_account(&state_update, contract_id)?.ok_or_else(|| { + errors::CallFunctionError::AccountDoesNotExist { + requested_account_id: contract_id.clone(), + } + })?; // TODO(#1015): Add ability to pass public key and originator_id let originator_id = contract_id; let public_key = PublicKey::empty(KeyType::ED25519); From a4617981ce02406dded3ab5bfa87bd41bedc3553 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Wed, 17 Feb 2021 16:58:07 +0200 Subject: [PATCH 05/22] Extend missed error variants to be explicit --- chain/chain-primitives/src/error.rs | 11 ++++--- chain/client/src/view_client.rs | 8 ++--- chain/rosetta-rpc/src/utils.rs | 4 +-- neard/src/runtime/errors.rs | 26 ++++++++-------- runtime/runtime/src/state_viewer/errors.rs | 36 +++++++++++++++------- runtime/runtime/src/state_viewer/mod.rs | 2 +- 6 files changed, 52 insertions(+), 35 deletions(-) diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index 8dec2d609da..aecfe39ee77 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -22,12 +22,15 @@ pub enum QueryError { ContractCodeDoesNotExist { contract_account_id: near_primitives::types::AccountId }, #[error("Access key for public key #{public_key} does not exist while viewing")] AccessKeyDoesNotExist { public_key: String }, - #[error("Storage error occurred: {0:?}")] - StorageError(#[from] near_primitives::errors::StorageError), + #[error("Storage error occurred: #{storage_error:?}")] + StorageError { + #[from] + storage_error: near_primitives::errors::StorageError, + }, #[error("Internal error occurred: #{error_message}")] InternalError { error_message: String }, - #[error("VM error occurred: {0}")] - VMError(String), + #[error("VM error occurred: #{error_message}")] + VMError { error_message: String }, } #[derive(Debug)] diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index dbb0cc8e035..0256b50a5f8 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -470,10 +470,10 @@ impl From for near_client_primitives::types::QueryError { near_chain::near_chain_primitives::error::QueryError::AccessKeyDoesNotExist { public_key, } => Self::AccessKeyDoesNotExist { public_key }, - near_chain::near_chain_primitives::error::QueryError::StorageError(storage_error) => { - Self::IOError { error_message: storage_error.to_string() } - } - near_chain::near_chain_primitives::error::QueryError::VMError(error_message) => { + near_chain::near_chain_primitives::error::QueryError::StorageError { + storage_error, + } => Self::IOError { error_message: storage_error.to_string() }, + near_chain::near_chain_primitives::error::QueryError::VMError { error_message } => { Self::VMError { error_message } } } diff --git a/chain/rosetta-rpc/src/utils.rs b/chain/rosetta-rpc/src/utils.rs index db4e6e7d0dc..b4fde4ba20c 100644 --- a/chain/rosetta-rpc/src/utils.rs +++ b/chain/rosetta-rpc/src/utils.rs @@ -313,7 +313,7 @@ pub(crate) async fn query_account( block_id, near_primitives::views::QueryRequest::ViewAccount { account_id }, ); - let account_info_response = match view_client_addr.send(query.clone()).await? { + let account_info_response = match view_client_addr.send(query).await? { Ok(query_response) => query_response, Err(err) => match err { near_client_primitives::types::QueryError::AccountDoesNotExist { .. } => { @@ -391,7 +391,7 @@ pub(crate) async fn query_access_key( block_id, near_primitives::views::QueryRequest::ViewAccessKey { account_id, public_key }, ); - let access_key_query_response = match view_client_addr.send(access_key_query.clone()).await? { + let access_key_query_response = match view_client_addr.send(access_key_query).await? { Ok(query_response) => query_response, Err(err) => { return match err { diff --git a/neard/src/runtime/errors.rs b/neard/src/runtime/errors.rs index ba4ce1df983..4100bb35144 100644 --- a/neard/src/runtime/errors.rs +++ b/neard/src/runtime/errors.rs @@ -21,9 +21,9 @@ impl From for WrappedQuery node_runtime::state_viewer::errors::ViewAccountError::AccountDoesNotExist { requested_account_id, } => Self(QueryError::AccountDoesNotExist { requested_account_id }), - node_runtime::state_viewer::errors::ViewAccountError::StorageError(storage_error) => { - Self(QueryError::StorageError(storage_error)) - } + node_runtime::state_viewer::errors::ViewAccountError::StorageError { + storage_error, + } => Self(QueryError::StorageError { storage_error }), } } } @@ -37,9 +37,9 @@ impl From for Wrapped node_runtime::state_viewer::errors::ViewContractCodeError::AccountDoesNotExist { requested_account_id, } => Self(QueryError::AccountDoesNotExist { requested_account_id }), - node_runtime::state_viewer::errors::ViewContractCodeError::StorageError( + node_runtime::state_viewer::errors::ViewContractCodeError::StorageError { storage_error, - ) => Self(QueryError::StorageError(storage_error)), + } => Self(QueryError::StorageError { storage_error }), node_runtime::state_viewer::errors::ViewContractCodeError::NoContractCode { contract_account_id, } => Self(QueryError::ContractCodeDoesNotExist { contract_account_id }), @@ -57,10 +57,10 @@ impl From for WrappedQuer requested_account_id, } => Self(QueryError::AccountDoesNotExist { requested_account_id }), node_runtime::state_viewer::errors::CallFunctionError::StorageError(storage_error) => { - Self(QueryError::StorageError(storage_error)) + Self(QueryError::StorageError { storage_error }) } - node_runtime::state_viewer::errors::CallFunctionError::VMError(s) => { - Self(QueryError::VMError(s)) + node_runtime::state_viewer::errors::CallFunctionError::VMError { error_message } => { + Self(QueryError::VMError { error_message }) } } } @@ -72,8 +72,8 @@ impl From for WrappedQueryEr node_runtime::state_viewer::errors::ViewStateError::InvalidAccountId { requested_account_id, } => Self(QueryError::InvalidAccount { requested_account_id }), - node_runtime::state_viewer::errors::ViewStateError::StorageError(storage_error) => { - Self(QueryError::StorageError(storage_error)) + node_runtime::state_viewer::errors::ViewStateError::StorageError { storage_error } => { + Self(QueryError::StorageError { storage_error }) } } } @@ -85,9 +85,9 @@ impl From for WrappedQue node_runtime::state_viewer::errors::ViewAccessKeyError::InvalidAccountId { requested_account_id, } => Self(QueryError::InvalidAccount { requested_account_id }), - node_runtime::state_viewer::errors::ViewAccessKeyError::StorageError(storage_error) => { - Self(QueryError::StorageError(storage_error)) - } + node_runtime::state_viewer::errors::ViewAccessKeyError::StorageError { + storage_error, + } => Self(QueryError::StorageError { storage_error }), node_runtime::state_viewer::errors::ViewAccessKeyError::AccessKeyDoesNotExist { public_key, } => Self(QueryError::AccessKeyDoesNotExist { public_key: public_key.to_string() }), diff --git a/runtime/runtime/src/state_viewer/errors.rs b/runtime/runtime/src/state_viewer/errors.rs index 453a1dea446..cc61a325df6 100644 --- a/runtime/runtime/src/state_viewer/errors.rs +++ b/runtime/runtime/src/state_viewer/errors.rs @@ -4,8 +4,11 @@ pub enum ViewAccountError { InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Account ID #{requested_account_id} does not exist")] AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, - #[error("Storage error: {0}")] - StorageError(#[from] near_primitives::errors::StorageError), + #[error("Storage error: #{storage_error}")] + StorageError { + #[from] + storage_error: near_primitives::errors::StorageError, + }, } #[derive(thiserror::Error, Debug)] @@ -14,8 +17,11 @@ pub enum ViewContractCodeError { InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Account ID #{requested_account_id} does not exist")] AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, - #[error("Storage error: {0}")] - StorageError(#[from] near_primitives::errors::StorageError), + #[error("Storage error: #{storage_error}")] + StorageError { + #[from] + storage_error: near_primitives::errors::StorageError, + }, #[error("Contract code for contract ID #{contract_account_id} does not exist")] NoContractCode { contract_account_id: near_primitives::types::AccountId }, } @@ -24,8 +30,11 @@ pub enum ViewContractCodeError { pub enum ViewAccessKeyError { #[error("Account ID #{requested_account_id} is invalid")] InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, - #[error("Storage error: {0}")] - StorageError(#[from] near_primitives::errors::StorageError), + #[error("Storage error: #{storage_error:?}")] + StorageError { + #[from] + storage_error: near_primitives::errors::StorageError, + }, #[error("Access key for public key #{public_key} does not exist")] AccessKeyDoesNotExist { public_key: near_crypto::PublicKey }, #[error("Internal error occurred: #{error_message}")] @@ -36,8 +45,11 @@ pub enum ViewAccessKeyError { pub enum ViewStateError { #[error("Account ID #{requested_account_id} is invalid")] InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, - #[error("Storage error: {0}")] - StorageError(#[from] near_primitives::errors::StorageError), + #[error("Storage error: #{storage_error}")] + StorageError { + #[from] + storage_error: near_primitives::errors::StorageError, + }, } #[derive(thiserror::Error, Debug)] @@ -48,8 +60,8 @@ pub enum CallFunctionError { AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, #[error("Storage error: {0}")] StorageError(#[from] near_primitives::errors::StorageError), - #[error("VM error occurred: {0}")] - VMError(String), + #[error("VM error occurred: #{error_message}")] + VMError { error_message: String }, } impl From for ViewContractCodeError { @@ -61,7 +73,9 @@ impl From for ViewContractCodeError { ViewAccountError::AccountDoesNotExist { requested_account_id } => { Self::AccountDoesNotExist { requested_account_id } } - ViewAccountError::StorageError(storage_error) => Self::StorageError(storage_error), + ViewAccountError::StorageError { storage_error } => { + Self::StorageError { storage_error } + } } } } diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index afcab3fe245..1e4e9b2f11e 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -240,7 +240,7 @@ impl TrieViewer { } let message = format!("wasm execution failed with error: {:?}", err); debug!(target: "runtime", "(exec time {}) {}", time_str, message); - Err(errors::CallFunctionError::VMError(message)) + Err(errors::CallFunctionError::VMError { error_message: message }) } else { let outcome = outcome.unwrap(); debug!(target: "runtime", "(exec time {}) result of execution: {:#?}", time_str, outcome); From 4d54c91bb919ed50f5b42e0aa7d0db1d0368071e Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Wed, 17 Feb 2021 18:44:36 +0200 Subject: [PATCH 06/22] Address review suggestion, create dummy for testing rpc query method --- Cargo.lock | 4 ++ chain/chain-primitives/Cargo.toml | 1 + chain/chain-primitives/src/error.rs | 4 +- chain/client-primitives/Cargo.toml | 1 + chain/client-primitives/src/types.rs | 2 +- chain/jsonrpc-primitives/Cargo.toml | 6 ++- chain/jsonrpc-primitives/src/types/blocks.rs | 8 ++-- chain/jsonrpc-primitives/src/types/chunks.rs | 8 ++-- chain/jsonrpc-primitives/src/types/config.rs | 8 ++-- chain/jsonrpc-primitives/src/types/query.rs | 4 +- .../jsonrpc-primitives/src/types/receipts.rs | 8 ++-- chain/jsonrpc/client/src/lib.rs | 8 +++- neard/src/runtime/errors.rs | 2 +- neard/tests/rpc_nodes.rs | 39 +++++++++++++++++++ 14 files changed, 82 insertions(+), 21 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d638ca79617..ebae686afcc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3050,6 +3050,7 @@ dependencies = [ "failure", "failure_derive", "log", + "near-crypto", "near-primitives", "thiserror", ] @@ -3128,6 +3129,7 @@ dependencies = [ "near-chain-configs", "near-chain-primitives", "near-chunks", + "near-crypto", "near-network", "near-primitives", "serde", @@ -3290,12 +3292,14 @@ dependencies = [ "lazy_static", "near-chain-configs", "near-client-primitives", + "near-crypto", "near-metrics", "near-primitives", "near-primitives-core", "serde", "serde_json", "thiserror", + "tracing", "uuid", ] diff --git a/chain/chain-primitives/Cargo.toml b/chain/chain-primitives/Cargo.toml index ab4fac84053..0b1c0d4fb77 100644 --- a/chain/chain-primitives/Cargo.toml +++ b/chain/chain-primitives/Cargo.toml @@ -14,3 +14,4 @@ log = "0.4" thiserror = "1.0" near-primitives = { path = "../../core/primitives" } +near-crypto = { path = "../../core/crypto" } diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index aecfe39ee77..cebe56dc590 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -14,14 +14,14 @@ use near_primitives::types::{BlockHeight, EpochId, ShardId}; #[derive(thiserror::Error, Debug)] pub enum QueryError { - #[error("Invalid account ID #{requested_account_id}")] + #[error("Account ID #{requested_account_id} is invalid")] InvalidAccount { requested_account_id: near_primitives::types::AccountId }, #[error("Account ID #{requested_account_id} does not exist while viewing")] AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, #[error("Contract ID #{contract_account_id} code does not exist while viewing")] ContractCodeDoesNotExist { contract_account_id: near_primitives::types::AccountId }, #[error("Access key for public key #{public_key} does not exist while viewing")] - AccessKeyDoesNotExist { public_key: String }, + AccessKeyDoesNotExist { public_key: near_crypto::PublicKey }, #[error("Storage error occurred: #{storage_error:?}")] StorageError { #[from] diff --git a/chain/client-primitives/Cargo.toml b/chain/client-primitives/Cargo.toml index 0940a7138e1..ac30ebde123 100644 --- a/chain/client-primitives/Cargo.toml +++ b/chain/client-primitives/Cargo.toml @@ -17,6 +17,7 @@ near-chain-primitives = { path = "../chain-primitives" } near-chain-configs = { path = "../../core/chain-configs" } near-chunks = { path = "../chunks" } +near-crypto = { path = "../../core/crypto" } near-network = { path = "../network" } near-primitives = { path = "../../core/primitives" } diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs index 55836fcb8e5..20407edddd7 100644 --- a/chain/client-primitives/src/types.rs +++ b/chain/client-primitives/src/types.rs @@ -287,7 +287,7 @@ pub enum QueryError { )] ContractCodeDoesNotExist { contract_account_id: near_primitives::types::AccountId }, #[error("Access key for public key #{public_key} has never been observed on the node")] - AccessKeyDoesNotExist { public_key: String }, + AccessKeyDoesNotExist { public_key: near_crypto::PublicKey }, #[error("VM error occurred: #{error_message}")] VMError { error_message: String }, // NOTE: Currently, the underlying errors are too broad, and while we tried to handle diff --git a/chain/jsonrpc-primitives/Cargo.toml b/chain/jsonrpc-primitives/Cargo.toml index 5eff37ae8a6..4608efdd4db 100644 --- a/chain/jsonrpc-primitives/Cargo.toml +++ b/chain/jsonrpc-primitives/Cargo.toml @@ -12,10 +12,12 @@ lazy_static = "1.4" serde = { version = "1", features = ["derive"] } serde_json = "1" thiserror = "1.0" +tracing = "0.1.13" uuid = { version = "~0.8", features = ["v4"] } +near-chain-configs = { path = "../../core/chain-configs" } near-client-primitives = { path = "../client-primitives" } +near-crypto = { path = "../../core/crypto" } +near-metrics = { path = "../../core/metrics" } near-primitives = { path = "../../core/primitives" } near-primitives-core = { path = "../../core/primitives-core" } -near-metrics = { path = "../../core/metrics" } -near-chain-configs = { path = "../../core/chain-configs" } diff --git a/chain/jsonrpc-primitives/src/types/blocks.rs b/chain/jsonrpc-primitives/src/types/blocks.rs index adecc05f7d8..d929127a352 100644 --- a/chain/jsonrpc-primitives/src/types/blocks.rs +++ b/chain/jsonrpc-primitives/src/types/blocks.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; +use tracing::error; #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -64,12 +65,13 @@ impl From for RpcBlockError { } near_client_primitives::types::GetBlockError::NotSyncedYet => Self::NotSyncedYet, near_client_primitives::types::GetBlockError::IOError(s) => Self::InternalError(s), - near_client_primitives::types::GetBlockError::Unreachable(s) => { + near_client_primitives::types::GetBlockError::Unreachable(error_message) => { + error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, - &["RpcBlockError", &s], + &["RpcBlockError", &error_message], ); - Self::Unreachable(s) + Self::Unreachable(error_message) } } } diff --git a/chain/jsonrpc-primitives/src/types/chunks.rs b/chain/jsonrpc-primitives/src/types/chunks.rs index 83f7df24504..88fb54bd35c 100644 --- a/chain/jsonrpc-primitives/src/types/chunks.rs +++ b/chain/jsonrpc-primitives/src/types/chunks.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; +use tracing::error; #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(untagged)] @@ -93,12 +94,13 @@ impl From for RpcChunkError { near_client_primitives::types::GetChunkError::UnknownChunk(hash) => { Self::UnknownChunk(hash) } - near_client_primitives::types::GetChunkError::Unreachable(s) => { + near_client_primitives::types::GetChunkError::Unreachable(error_message) => { + error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, - &["RpcChunkError", &s], + &["RpcChunkError", &error_message], ); - Self::Unreachable(s) + Self::Unreachable(error_message) } } } diff --git a/chain/jsonrpc-primitives/src/types/config.rs b/chain/jsonrpc-primitives/src/types/config.rs index bcd4236c33a..076f9ac1a44 100644 --- a/chain/jsonrpc-primitives/src/types/config.rs +++ b/chain/jsonrpc-primitives/src/types/config.rs @@ -1,6 +1,7 @@ use crate::types::blocks::BlockReference; use serde::{Deserialize, Serialize}; use serde_json::Value; +use tracing::error; #[derive(Serialize, Deserialize)] pub struct RpcProtocolConfigRequest { @@ -46,12 +47,13 @@ impl From for RpcProtocol near_client_primitives::types::GetProtocolConfigError::IOError(s) => { Self::InternalError(s) } - near_client_primitives::types::GetProtocolConfigError::Unreachable(s) => { + near_client_primitives::types::GetProtocolConfigError::Unreachable(error_message) => { + error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, - &["RpcProtocolConfigError", &s], + &["RpcProtocolConfigError", &error_message], ); - Self::Unreachable(s) + Self::Unreachable(error_message) } } } diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index 0f202c74a65..f1f67f38047 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; +use tracing::error; /// Max size of the query path (soft-deprecated) const QUERY_DATA_MAX_SIZE: usize = 10 * 1024; @@ -29,7 +30,7 @@ pub enum RpcQueryError { )] NoContractCode { contract_account_id: near_primitives::types::AccountId }, #[error("Access key for public key #{public_key} has never been observed on the node")] - UnknownAccessKey { public_key: String }, + UnknownAccessKey { public_key: near_crypto::PublicKey }, #[error("Function call returned an error: #{vm_error}")] FunctionCall { vm_error: String }, #[error("The node reached its limits. Try again later. More details: #{error_message}")] @@ -148,6 +149,7 @@ impl From for RpcQueryError { Self::FunctionCall { vm_error: error_message } } near_client_primitives::types::QueryError::Unreachable { error_message } => { + error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, &["RpcQueryError", &error_message], diff --git a/chain/jsonrpc-primitives/src/types/receipts.rs b/chain/jsonrpc-primitives/src/types/receipts.rs index a17c16f87d7..b9f560effb1 100644 --- a/chain/jsonrpc-primitives/src/types/receipts.rs +++ b/chain/jsonrpc-primitives/src/types/receipts.rs @@ -1,5 +1,6 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; +use tracing::error; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ReceiptReference { @@ -52,12 +53,13 @@ impl From for RpcReceiptError { near_client_primitives::types::GetReceiptError::UnknownReceipt(hash) => { Self::UnknownReceipt(hash) } - near_client_primitives::types::GetReceiptError::Unreachable(s) => { + near_client_primitives::types::GetReceiptError::Unreachable(error_message) => { + error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, - &["RpcReceiptError", &s], + &["RpcReceiptError", &error_message], ); - Self::Unreachable(s) + Self::Unreachable(error_message) } } } diff --git a/chain/jsonrpc/client/src/lib.rs b/chain/jsonrpc/client/src/lib.rs index daa7caba775..4b8475e9849 100644 --- a/chain/jsonrpc/client/src/lib.rs +++ b/chain/jsonrpc/client/src/lib.rs @@ -8,8 +8,9 @@ use serde::Serialize; use near_jsonrpc_primitives::errors::RpcError; use near_jsonrpc_primitives::message::{from_slice, Message}; use near_jsonrpc_primitives::rpc::{ - RpcQueryRequest, RpcStateChangesRequest, RpcStateChangesResponse, RpcValidatorsOrderedRequest, + RpcStateChangesRequest, RpcStateChangesResponse, RpcValidatorsOrderedRequest, }; +use near_jsonrpc_primitives::types::query::RpcQueryRequest; use near_primitives::hash::CryptoHash; use near_primitives::types::{BlockId, BlockReference, MaybeBlockId, ShardId}; use near_primitives::views::{ @@ -198,7 +199,10 @@ impl JsonRpcClient { call_method(&self.client, &self.server_addr, "query", [path, data]) } - pub fn query(&self, request: RpcQueryRequest) -> RpcRequest { + pub fn query( + &self, + request: near_jsonrpc_primitives::types::query::RpcQueryRequest, + ) -> RpcRequest { call_method(&self.client, &self.server_addr, "query", request) } diff --git a/neard/src/runtime/errors.rs b/neard/src/runtime/errors.rs index 4100bb35144..2e4b8d1c9ec 100644 --- a/neard/src/runtime/errors.rs +++ b/neard/src/runtime/errors.rs @@ -90,7 +90,7 @@ impl From for WrappedQue } => Self(QueryError::StorageError { storage_error }), node_runtime::state_viewer::errors::ViewAccessKeyError::AccessKeyDoesNotExist { public_key, - } => Self(QueryError::AccessKeyDoesNotExist { public_key: public_key.to_string() }), + } => Self(QueryError::AccessKeyDoesNotExist { public_key }), node_runtime::state_viewer::errors::ViewAccessKeyError::InternalError { error_message, } => Self(QueryError::InternalError { error_message }), diff --git a/neard/tests/rpc_nodes.rs b/neard/tests/rpc_nodes.rs index eded9308a87..ce1e258d931 100644 --- a/neard/tests/rpc_nodes.rs +++ b/neard/tests/rpc_nodes.rs @@ -708,3 +708,42 @@ fn test_protocol_config_rpc() { }); }); } + +#[test] +fn test_query_rpc() { + init_integration_logger(); + heavy_test(|| { + System::builder() + .stop_on_panic(true) + .run(move || { + let num_nodes = 1; + let dirs = (0..num_nodes) + .map(|i| { + tempfile::Builder::new() + .prefix(&format!("protocol_config{}", i)) + .tempdir() + .unwrap() + }) + .collect::>(); + let (_genesis, rpc_addrs, _) = start_nodes(1, &dirs, 1, 0, 10, 0); + + actix::spawn(async move { + let client = new_client(&format!("http://{}", rpc_addrs[0])); + let query_response = client + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { + block_reference: near_primitives::types::BlockReference::Finality( + Finality::Final, + ), + request: near_primitives::views::QueryRequest::ViewAccount { + account_id: "test.near".to_string(), + }, + }) + .await + .unwrap(); + assert_eq!(false, true); + System::current().stop(); + }); + }) + .unwrap(); + }); +} From bcf4a04ebf8707d5f5a103bd6fe5051d2be59c00 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Thu, 18 Feb 2021 10:38:51 +0200 Subject: [PATCH 07/22] Fix some naming issues --- chain/client-primitives/src/types.rs | 20 ++++++++-------- chain/client/src/view_client.rs | 10 ++++---- chain/jsonrpc-primitives/src/types/query.rs | 26 ++++++++++----------- chain/jsonrpc/client/src/lib.rs | 1 - chain/rosetta-rpc/src/utils.rs | 6 ++--- 5 files changed, 31 insertions(+), 32 deletions(-) diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs index 20407edddd7..73bfb798a94 100644 --- a/chain/client-primitives/src/types.rs +++ b/chain/client-primitives/src/types.rs @@ -274,22 +274,22 @@ impl Message for Query { pub enum QueryError { #[error("IO Error: #{error_message}")] IOError { error_message: String }, - #[error("There are no fully synchronized blocks yet")] - NotSyncedYet, - #[error("The node does not track the shard #{requested_shard_id} where the requested account resides")] + #[error("There are no fully synchronized blocks on the node yet")] + NoSyncedBlocks, + #[error("The node does not track the shard")] UnavailableShard { requested_shard_id: near_primitives::types::ShardId }, - #[error("Invalid account ID #{requested_account_id}")] + #[error("Account ID #{requested_account_id} is invalid")] InvalidAccount { requested_account_id: near_primitives::types::AccountId }, - #[error("Account ID #{requested_account_id} has never been observed on the node")] - AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, + #[error("account #{requested_account_id} does not exist while viewing")] + UnknownAccount { requested_account_id: near_primitives::types::AccountId }, #[error( "Contract code for contract ID #{contract_account_id} has never been observed on the node" )] - ContractCodeDoesNotExist { contract_account_id: near_primitives::types::AccountId }, + NoContractCode { contract_account_id: near_primitives::types::AccountId }, #[error("Access key for public key #{public_key} has never been observed on the node")] - AccessKeyDoesNotExist { public_key: near_crypto::PublicKey }, - #[error("VM error occurred: #{error_message}")] - VMError { error_message: String }, + UnknownAccessKey { public_key: near_crypto::PublicKey }, + #[error("Function call returned an error: #{vm_error}")] + ContractExecutionError { vm_error: String }, // NOTE: Currently, the underlying errors are too broad, and while we tried to handle // expected cases, we cannot statically guarantee that no other errors will be returned // in the future. diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index 0256b50a5f8..5d2633fd2c4 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -211,7 +211,7 @@ impl ViewClientActor { { self.chain.get_block_header(&block_hash) } else { - return Err(QueryError::NotSyncedYet); + return Err(QueryError::NoSyncedBlocks); } } }; @@ -463,18 +463,18 @@ impl From for near_client_primitives::types::QueryError { } => Self::InvalidAccount { requested_account_id }, near_chain::near_chain_primitives::error::QueryError::AccountDoesNotExist { requested_account_id, - } => Self::AccountDoesNotExist { requested_account_id }, + } => Self::UnknownAccount { requested_account_id }, near_chain::near_chain_primitives::error::QueryError::ContractCodeDoesNotExist { contract_account_id, - } => Self::ContractCodeDoesNotExist { contract_account_id }, + } => Self::NoContractCode { contract_account_id }, near_chain::near_chain_primitives::error::QueryError::AccessKeyDoesNotExist { public_key, - } => Self::AccessKeyDoesNotExist { public_key }, + } => Self::UnknownAccessKey { public_key }, near_chain::near_chain_primitives::error::QueryError::StorageError { storage_error, } => Self::IOError { error_message: storage_error.to_string() }, near_chain::near_chain_primitives::error::QueryError::VMError { error_message } => { - Self::VMError { error_message } + Self::ContractExecutionError { vm_error: error_message } } } } diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index f1f67f38047..8ccf5a24c64 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -20,7 +20,7 @@ pub enum RpcQueryError { #[error("There are no fully synchronized blocks on the node yet")] NoSyncedBlocks, #[error("The node does not track the shard")] - DoesNotTrackShard { requested_shard_id: near_primitives::types::ShardId }, + UnavailableShard { requested_shard_id: near_primitives::types::ShardId }, #[error("Account ID #{requested_account_id} is invalid")] InvalidAccount { requested_account_id: near_primitives::types::AccountId }, #[error("account #{requested_account_id} does not exist while viewing")] @@ -32,7 +32,7 @@ pub enum RpcQueryError { #[error("Access key for public key #{public_key} has never been observed on the node")] UnknownAccessKey { public_key: near_crypto::PublicKey }, #[error("Function call returned an error: #{vm_error}")] - FunctionCall { vm_error: String }, + ContractExecutionError { vm_error: String }, #[error("The node reached its limits. Try again later. More details: #{error_message}")] InternalError { error_message: String }, // NOTE: Currently, the underlying errors are too broad, and while we tried to handle @@ -129,24 +129,24 @@ impl From for RpcQueryError { near_client_primitives::types::QueryError::IOError { error_message } => { Self::InternalError { error_message } } - near_client_primitives::types::QueryError::NotSyncedYet => Self::NoSyncedBlocks, + near_client_primitives::types::QueryError::NoSyncedBlocks => Self::NoSyncedBlocks, near_client_primitives::types::QueryError::UnavailableShard { requested_shard_id } => { - Self::DoesNotTrackShard { requested_shard_id } + Self::UnavailableShard { requested_shard_id } } near_client_primitives::types::QueryError::InvalidAccount { requested_account_id } => { Self::InvalidAccount { requested_account_id } } - near_client_primitives::types::QueryError::AccountDoesNotExist { - requested_account_id, - } => Self::UnknownAccount { requested_account_id }, - near_client_primitives::types::QueryError::ContractCodeDoesNotExist { - contract_account_id, - } => Self::NoContractCode { contract_account_id }, - near_client_primitives::types::QueryError::AccessKeyDoesNotExist { public_key } => { + near_client_primitives::types::QueryError::UnknownAccount { requested_account_id } => { + Self::UnknownAccount { requested_account_id } + } + near_client_primitives::types::QueryError::NoContractCode { contract_account_id } => { + Self::NoContractCode { contract_account_id } + } + near_client_primitives::types::QueryError::UnknownAccessKey { public_key } => { Self::UnknownAccessKey { public_key } } - near_client_primitives::types::QueryError::VMError { error_message } => { - Self::FunctionCall { vm_error: error_message } + near_client_primitives::types::QueryError::ContractExecutionError { vm_error } => { + Self::ContractExecutionError { vm_error } } near_client_primitives::types::QueryError::Unreachable { error_message } => { error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); diff --git a/chain/jsonrpc/client/src/lib.rs b/chain/jsonrpc/client/src/lib.rs index 4b8475e9849..5603444ce8d 100644 --- a/chain/jsonrpc/client/src/lib.rs +++ b/chain/jsonrpc/client/src/lib.rs @@ -10,7 +10,6 @@ use near_jsonrpc_primitives::message::{from_slice, Message}; use near_jsonrpc_primitives::rpc::{ RpcStateChangesRequest, RpcStateChangesResponse, RpcValidatorsOrderedRequest, }; -use near_jsonrpc_primitives::types::query::RpcQueryRequest; use near_primitives::hash::CryptoHash; use near_primitives::types::{BlockId, BlockReference, MaybeBlockId, ShardId}; use near_primitives::views::{ diff --git a/chain/rosetta-rpc/src/utils.rs b/chain/rosetta-rpc/src/utils.rs index b4fde4ba20c..ddc831e876d 100644 --- a/chain/rosetta-rpc/src/utils.rs +++ b/chain/rosetta-rpc/src/utils.rs @@ -316,7 +316,7 @@ pub(crate) async fn query_account( let account_info_response = match view_client_addr.send(query).await? { Ok(query_response) => query_response, Err(err) => match err { - near_client_primitives::types::QueryError::AccountDoesNotExist { .. } => { + near_client_primitives::types::QueryError::UnknownAccount { .. } => { return Err(crate::errors::ErrorKind::NotFound(err.to_string())) } _ => return Err(crate::errors::ErrorKind::InternalError(err.to_string())), @@ -395,8 +395,8 @@ pub(crate) async fn query_access_key( Ok(query_response) => query_response, Err(err) => { return match err { - near_client_primitives::types::QueryError::AccountDoesNotExist { .. } - | near_client_primitives::types::QueryError::AccessKeyDoesNotExist { .. } => { + near_client_primitives::types::QueryError::UnknownAccount { .. } + | near_client_primitives::types::QueryError::UnknownAccessKey { .. } => { Err(crate::errors::ErrorKind::NotFound(err.to_string())) } _ => Err(crate::errors::ErrorKind::InternalError(err.to_string())), From 09be16f7f764ef47aa8fd65c9ea071719df55411 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Fri, 19 Feb 2021 19:22:39 +0200 Subject: [PATCH 08/22] Adjust error structures --- chain/chain-primitives/src/error.rs | 21 +++--- chain/client-primitives/src/types.rs | 2 +- chain/client/src/view_client.rs | 59 +++++++---------- chain/jsonrpc-primitives/src/types/blocks.rs | 3 +- chain/jsonrpc-primitives/src/types/chunks.rs | 3 +- chain/jsonrpc-primitives/src/types/config.rs | 3 +- chain/jsonrpc-primitives/src/types/query.rs | 3 +- .../jsonrpc-primitives/src/types/receipts.rs | 3 +- neard/src/runtime/errors.rs | 37 +++++------ runtime/runtime/src/state_viewer/errors.rs | 66 ++++++++++++------- 10 files changed, 97 insertions(+), 103 deletions(-) diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index cebe56dc590..ee937940d8d 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -16,21 +16,18 @@ use near_primitives::types::{BlockHeight, EpochId, ShardId}; pub enum QueryError { #[error("Account ID #{requested_account_id} is invalid")] InvalidAccount { requested_account_id: near_primitives::types::AccountId }, - #[error("Account ID #{requested_account_id} does not exist while viewing")] - AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, - #[error("Contract ID #{contract_account_id} code does not exist while viewing")] - ContractCodeDoesNotExist { contract_account_id: near_primitives::types::AccountId }, + #[error("Account #{requested_account_id} does not exist while viewing")] + UnknownAccount { requested_account_id: near_primitives::types::AccountId }, + #[error( + "Contract code for contract ID #{contract_account_id} has never been observed on the node" + )] + NoContractCode { contract_account_id: near_primitives::types::AccountId }, #[error("Access key for public key #{public_key} does not exist while viewing")] - AccessKeyDoesNotExist { public_key: near_crypto::PublicKey }, - #[error("Storage error occurred: #{storage_error:?}")] - StorageError { - #[from] - storage_error: near_primitives::errors::StorageError, - }, + UnknownAccessKey { public_key: near_crypto::PublicKey }, #[error("Internal error occurred: #{error_message}")] InternalError { error_message: String }, - #[error("VM error occurred: #{error_message}")] - VMError { error_message: String }, + #[error("Function call returned an error: #{error_message}")] + ContractExecutionError { error_message: String }, } #[derive(Debug)] diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs index 73bfb798a94..f6783ea3192 100644 --- a/chain/client-primitives/src/types.rs +++ b/chain/client-primitives/src/types.rs @@ -280,7 +280,7 @@ pub enum QueryError { UnavailableShard { requested_shard_id: near_primitives::types::ShardId }, #[error("Account ID #{requested_account_id} is invalid")] InvalidAccount { requested_account_id: near_primitives::types::AccountId }, - #[error("account #{requested_account_id} does not exist while viewing")] + #[error("Account #{requested_account_id} does not exist while viewing")] UnknownAccount { requested_account_id: near_primitives::types::AccountId }, #[error( "Contract code for contract ID #{contract_account_id} has never been observed on the node" diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index 5d2633fd2c4..cae7176fad7 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -236,7 +236,7 @@ impl ViewClientActor { ) { let chunk_extra = self.chain.get_chunk_extra(header.hash(), shard_id)?; let state_root = chunk_extra.state_root; - Ok(self + match self .runtime_adapter .query( shard_id, @@ -247,8 +247,29 @@ impl ViewClientActor { header.hash(), header.epoch_id(), &msg.request, - ) - .map_err(RuntimeQueryError::from)?) + ) { + Ok(query_response) => Ok(query_response), + Err(query_error) => Err(match query_error { + near_chain::near_chain_primitives::error::QueryError::InternalError { + error_message, + } => QueryError::Unreachable { error_message }, + near_chain::near_chain_primitives::error::QueryError::InvalidAccount { + requested_account_id, + } => QueryError::InvalidAccount { requested_account_id }, + near_chain::near_chain_primitives::error::QueryError::UnknownAccount { + requested_account_id, + } => QueryError::UnknownAccount { requested_account_id }, + near_chain::near_chain_primitives::error::QueryError::NoContractCode { + contract_account_id, + } => QueryError::NoContractCode { contract_account_id }, + near_chain::near_chain_primitives::error::QueryError::UnknownAccessKey { + public_key, + } => QueryError::UnknownAccessKey { public_key }, + near_chain::near_chain_primitives::error::QueryError::ContractExecutionError { + error_message, + } => QueryError::ContractExecutionError { vm_error: error_message }, + }) + } } else { Err(QueryError::UnavailableShard { requested_shard_id: shard_id }) } @@ -448,38 +469,6 @@ impl Handler for ViewClientActor { } } -#[derive(thiserror::Error, Debug)] -#[error(transparent)] -struct RuntimeQueryError(#[from] pub near_chain::near_chain_primitives::error::QueryError); - -impl From for near_client_primitives::types::QueryError { - fn from(error: RuntimeQueryError) -> Self { - match error.0 { - near_chain::near_chain_primitives::error::QueryError::InternalError { - error_message, - } => Self::Unreachable { error_message }, - near_chain::near_chain_primitives::error::QueryError::InvalidAccount { - requested_account_id, - } => Self::InvalidAccount { requested_account_id }, - near_chain::near_chain_primitives::error::QueryError::AccountDoesNotExist { - requested_account_id, - } => Self::UnknownAccount { requested_account_id }, - near_chain::near_chain_primitives::error::QueryError::ContractCodeDoesNotExist { - contract_account_id, - } => Self::NoContractCode { contract_account_id }, - near_chain::near_chain_primitives::error::QueryError::AccessKeyDoesNotExist { - public_key, - } => Self::UnknownAccessKey { public_key }, - near_chain::near_chain_primitives::error::QueryError::StorageError { - storage_error, - } => Self::IOError { error_message: storage_error.to_string() }, - near_chain::near_chain_primitives::error::QueryError::VMError { error_message } => { - Self::ContractExecutionError { vm_error: error_message } - } - } - } -} - /// Handles retrieving block from the chain. impl Handler for ViewClientActor { type Result = Result; diff --git a/chain/jsonrpc-primitives/src/types/blocks.rs b/chain/jsonrpc-primitives/src/types/blocks.rs index d929127a352..37d53e7b057 100644 --- a/chain/jsonrpc-primitives/src/types/blocks.rs +++ b/chain/jsonrpc-primitives/src/types/blocks.rs @@ -1,6 +1,5 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; -use tracing::error; #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] @@ -66,7 +65,7 @@ impl From for RpcBlockError { near_client_primitives::types::GetBlockError::NotSyncedYet => Self::NotSyncedYet, near_client_primitives::types::GetBlockError::IOError(s) => Self::InternalError(s), near_client_primitives::types::GetBlockError::Unreachable(error_message) => { - error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); + tracing::warn!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, &["RpcBlockError", &error_message], diff --git a/chain/jsonrpc-primitives/src/types/chunks.rs b/chain/jsonrpc-primitives/src/types/chunks.rs index 88fb54bd35c..a4aa314544b 100644 --- a/chain/jsonrpc-primitives/src/types/chunks.rs +++ b/chain/jsonrpc-primitives/src/types/chunks.rs @@ -1,6 +1,5 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; -use tracing::error; #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(untagged)] @@ -95,7 +94,7 @@ impl From for RpcChunkError { Self::UnknownChunk(hash) } near_client_primitives::types::GetChunkError::Unreachable(error_message) => { - error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); + tracing::warn!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, &["RpcChunkError", &error_message], diff --git a/chain/jsonrpc-primitives/src/types/config.rs b/chain/jsonrpc-primitives/src/types/config.rs index 076f9ac1a44..82362bf5426 100644 --- a/chain/jsonrpc-primitives/src/types/config.rs +++ b/chain/jsonrpc-primitives/src/types/config.rs @@ -1,7 +1,6 @@ use crate::types::blocks::BlockReference; use serde::{Deserialize, Serialize}; use serde_json::Value; -use tracing::error; #[derive(Serialize, Deserialize)] pub struct RpcProtocolConfigRequest { @@ -48,7 +47,7 @@ impl From for RpcProtocol Self::InternalError(s) } near_client_primitives::types::GetProtocolConfigError::Unreachable(error_message) => { - error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); + tracing::warn!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, &["RpcProtocolConfigError", &error_message], diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index 8ccf5a24c64..c7300049ed7 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -1,6 +1,5 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; -use tracing::error; /// Max size of the query path (soft-deprecated) const QUERY_DATA_MAX_SIZE: usize = 10 * 1024; @@ -149,7 +148,7 @@ impl From for RpcQueryError { Self::ContractExecutionError { vm_error } } near_client_primitives::types::QueryError::Unreachable { error_message } => { - error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); + tracing::warn!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, &["RpcQueryError", &error_message], diff --git a/chain/jsonrpc-primitives/src/types/receipts.rs b/chain/jsonrpc-primitives/src/types/receipts.rs index b9f560effb1..0962c7e1a26 100644 --- a/chain/jsonrpc-primitives/src/types/receipts.rs +++ b/chain/jsonrpc-primitives/src/types/receipts.rs @@ -1,6 +1,5 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; -use tracing::error; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ReceiptReference { @@ -54,7 +53,7 @@ impl From for RpcReceiptError { Self::UnknownReceipt(hash) } near_client_primitives::types::GetReceiptError::Unreachable(error_message) => { - error!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); + tracing::warn!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( &crate::metrics::RPC_UNREACHABLE_ERROR_COUNT, &["RpcReceiptError", &error_message], diff --git a/neard/src/runtime/errors.rs b/neard/src/runtime/errors.rs index 2e4b8d1c9ec..0adb5280191 100644 --- a/neard/src/runtime/errors.rs +++ b/neard/src/runtime/errors.rs @@ -20,10 +20,10 @@ impl From for WrappedQuery } => Self(QueryError::InvalidAccount { requested_account_id }), node_runtime::state_viewer::errors::ViewAccountError::AccountDoesNotExist { requested_account_id, - } => Self(QueryError::AccountDoesNotExist { requested_account_id }), - node_runtime::state_viewer::errors::ViewAccountError::StorageError { - storage_error, - } => Self(QueryError::StorageError { storage_error }), + } => Self(QueryError::UnknownAccount { requested_account_id }), + node_runtime::state_viewer::errors::ViewAccountError::InternalError { + error_message, + } => Self(QueryError::InternalError { error_message }), } } } @@ -36,13 +36,13 @@ impl From for Wrapped } => Self(QueryError::InvalidAccount { requested_account_id }), node_runtime::state_viewer::errors::ViewContractCodeError::AccountDoesNotExist { requested_account_id, - } => Self(QueryError::AccountDoesNotExist { requested_account_id }), - node_runtime::state_viewer::errors::ViewContractCodeError::StorageError { - storage_error, - } => Self(QueryError::StorageError { storage_error }), + } => Self(QueryError::UnknownAccount { requested_account_id }), + node_runtime::state_viewer::errors::ViewContractCodeError::InternalError { + error_message, + } => Self(QueryError::InternalError { error_message }), node_runtime::state_viewer::errors::ViewContractCodeError::NoContractCode { contract_account_id, - } => Self(QueryError::ContractCodeDoesNotExist { contract_account_id }), + } => Self(QueryError::NoContractCode { contract_account_id }), } } } @@ -55,12 +55,12 @@ impl From for WrappedQuer } => Self(QueryError::InvalidAccount { requested_account_id }), node_runtime::state_viewer::errors::CallFunctionError::AccountDoesNotExist { requested_account_id, - } => Self(QueryError::AccountDoesNotExist { requested_account_id }), - node_runtime::state_viewer::errors::CallFunctionError::StorageError(storage_error) => { - Self(QueryError::StorageError { storage_error }) - } + } => Self(QueryError::UnknownAccount { requested_account_id }), + node_runtime::state_viewer::errors::CallFunctionError::InternalError { + error_message, + } => Self(QueryError::InternalError { error_message }), node_runtime::state_viewer::errors::CallFunctionError::VMError { error_message } => { - Self(QueryError::VMError { error_message }) + Self(QueryError::ContractExecutionError { error_message }) } } } @@ -72,8 +72,8 @@ impl From for WrappedQueryEr node_runtime::state_viewer::errors::ViewStateError::InvalidAccountId { requested_account_id, } => Self(QueryError::InvalidAccount { requested_account_id }), - node_runtime::state_viewer::errors::ViewStateError::StorageError { storage_error } => { - Self(QueryError::StorageError { storage_error }) + node_runtime::state_viewer::errors::ViewStateError::InternalError { error_message } => { + Self(QueryError::InternalError { error_message }) } } } @@ -85,12 +85,9 @@ impl From for WrappedQue node_runtime::state_viewer::errors::ViewAccessKeyError::InvalidAccountId { requested_account_id, } => Self(QueryError::InvalidAccount { requested_account_id }), - node_runtime::state_viewer::errors::ViewAccessKeyError::StorageError { - storage_error, - } => Self(QueryError::StorageError { storage_error }), node_runtime::state_viewer::errors::ViewAccessKeyError::AccessKeyDoesNotExist { public_key, - } => Self(QueryError::AccessKeyDoesNotExist { public_key }), + } => Self(QueryError::UnknownAccessKey { public_key }), node_runtime::state_viewer::errors::ViewAccessKeyError::InternalError { error_message, } => Self(QueryError::InternalError { error_message }), diff --git a/runtime/runtime/src/state_viewer/errors.rs b/runtime/runtime/src/state_viewer/errors.rs index cc61a325df6..acd0e0f3e94 100644 --- a/runtime/runtime/src/state_viewer/errors.rs +++ b/runtime/runtime/src/state_viewer/errors.rs @@ -4,11 +4,8 @@ pub enum ViewAccountError { InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Account ID #{requested_account_id} does not exist")] AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, - #[error("Storage error: #{storage_error}")] - StorageError { - #[from] - storage_error: near_primitives::errors::StorageError, - }, + #[error("Internal error: #{error_message}")] + InternalError { error_message: String }, } #[derive(thiserror::Error, Debug)] @@ -17,27 +14,19 @@ pub enum ViewContractCodeError { InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Account ID #{requested_account_id} does not exist")] AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, - #[error("Storage error: #{storage_error}")] - StorageError { - #[from] - storage_error: near_primitives::errors::StorageError, - }, #[error("Contract code for contract ID #{contract_account_id} does not exist")] NoContractCode { contract_account_id: near_primitives::types::AccountId }, + #[error("Internal error: #{error_message}")] + InternalError { error_message: String }, } #[derive(thiserror::Error, Debug)] pub enum ViewAccessKeyError { #[error("Account ID #{requested_account_id} is invalid")] InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, - #[error("Storage error: #{storage_error:?}")] - StorageError { - #[from] - storage_error: near_primitives::errors::StorageError, - }, #[error("Access key for public key #{public_key} does not exist")] AccessKeyDoesNotExist { public_key: near_crypto::PublicKey }, - #[error("Internal error occurred: #{error_message}")] + #[error("Internal error: #{error_message}")] InternalError { error_message: String }, } @@ -45,11 +34,8 @@ pub enum ViewAccessKeyError { pub enum ViewStateError { #[error("Account ID #{requested_account_id} is invalid")] InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, - #[error("Storage error: #{storage_error}")] - StorageError { - #[from] - storage_error: near_primitives::errors::StorageError, - }, + #[error("Internal error: #{error_message}")] + InternalError { error_message: String }, } #[derive(thiserror::Error, Debug)] @@ -58,8 +44,8 @@ pub enum CallFunctionError { InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Account ID #{requested_account_id} does not exist")] AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, - #[error("Storage error: {0}")] - StorageError(#[from] near_primitives::errors::StorageError), + #[error("Internal error: #{error_message}")] + InternalError { error_message: String }, #[error("VM error occurred: #{error_message}")] VMError { error_message: String }, } @@ -73,9 +59,39 @@ impl From for ViewContractCodeError { ViewAccountError::AccountDoesNotExist { requested_account_id } => { Self::AccountDoesNotExist { requested_account_id } } - ViewAccountError::StorageError { storage_error } => { - Self::StorageError { storage_error } + ViewAccountError::InternalError { error_message } => { + Self::InternalError { error_message } } } } } + +impl From for ViewAccountError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::InternalError { error_message: storage_error.to_string() } + } +} + +impl From for ViewContractCodeError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::InternalError { error_message: storage_error.to_string() } + } +} + +impl From for ViewAccessKeyError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::InternalError { error_message: storage_error.to_string() } + } +} + +impl From for ViewStateError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::InternalError { error_message: storage_error.to_string() } + } +} + +impl From for CallFunctionError { + fn from(storage_error: near_primitives::errors::StorageError) -> Self { + Self::InternalError { error_message: storage_error.to_string() } + } +} From 8827cf2a790db5ffeeb599f3057be5e584d23458 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Fri, 19 Feb 2021 19:52:06 +0200 Subject: [PATCH 09/22] Update rpc_query tests --- chain/jsonrpc/tests/rpc_query.rs | 75 ++++++++++++++++---------------- 1 file changed, 37 insertions(+), 38 deletions(-) diff --git a/chain/jsonrpc/tests/rpc_query.rs b/chain/jsonrpc/tests/rpc_query.rs index 02090de87e4..d9acc9a9b26 100644 --- a/chain/jsonrpc/tests/rpc_query.rs +++ b/chain/jsonrpc/tests/rpc_query.rs @@ -8,7 +8,6 @@ use near_actix_test_utils::run_actix_until_stop; use near_crypto::{KeyType, PublicKey, Signature}; use near_jsonrpc::client::new_client; use near_jsonrpc_client::ChunkId; -use near_jsonrpc_primitives::rpc::RpcQueryRequest; use near_jsonrpc_primitives::rpc::RpcValidatorsOrderedRequest; use near_logger_utils::init_test_logger; use near_network::test_utils::WaitOrTimeout; @@ -161,35 +160,35 @@ fn test_query_account() { let status = client.status().await.unwrap(); let block_hash = status.sync_info.latest_block_hash; let query_response_1 = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::ViewAccount { account_id: "test".to_string() }, }) .await .unwrap(); let query_response_2 = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::BlockId(BlockId::Height(0)), request: QueryRequest::ViewAccount { account_id: "test".to_string() }, }) .await .unwrap(); let query_response_3 = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::BlockId(BlockId::Hash(block_hash)), request: QueryRequest::ViewAccount { account_id: "test".to_string() }, }) .await .unwrap(); for query_response in [query_response_1, query_response_2, query_response_3].iter() { - assert_eq!(query_response.block_height, 0); - assert_eq!(query_response.block_hash, block_hash); + assert_eq!(query_response.query_response.block_height, 0); + assert_eq!(query_response.query_response.block_hash, block_hash); let account_info = if let QueryResponseKind::ViewAccount(ref account) = - query_response.kind + query_response.query_response.kind { account } else { - panic!("queried account, but received something else: {:?}", query_response.kind); + panic!("queried account, but received something else: {:?}", query_response.query_response.kind); }; assert_eq!(account_info.amount, 0); assert_eq!(account_info.code_hash.as_ref(), &[0; 32]); @@ -224,18 +223,18 @@ fn test_query_by_path_access_keys() { fn test_query_access_keys() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::ViewAccessKeyList { account_id: "test".to_string() }, }) .await .unwrap(); - assert_eq!(query_response.block_height, 0); - let access_keys = if let QueryResponseKind::AccessKeyList(access_keys) = query_response.kind + assert_eq!(query_response.query_response.block_height, 0); + let access_keys = if let QueryResponseKind::AccessKeyList(access_keys) = query_response.query_response.kind { access_keys } else { - panic!("queried access keys, but received something else: {:?}", query_response.kind); + panic!("queried access keys, but received something else: {:?}", query_response.query_response.kind); }; assert_eq!(access_keys.keys.len(), 1); assert_eq!(access_keys.keys[0].access_key, AccessKey::full_access().into()); @@ -270,7 +269,7 @@ fn test_query_by_path_access_key() { fn test_query_access_key() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::ViewAccessKey { account_id: "test".to_string(), @@ -281,11 +280,11 @@ fn test_query_access_key() { }) .await .unwrap(); - assert_eq!(query_response.block_height, 0); - let access_key = if let QueryResponseKind::AccessKey(access_keys) = query_response.kind { + assert_eq!(query_response.query_response.block_height, 0); + let access_key = if let QueryResponseKind::AccessKey(access_keys) = query_response.query_response.kind { access_keys } else { - panic!("queried access keys, but received something else: {:?}", query_response.kind); + panic!("queried access keys, but received something else: {:?}", query_response.query_response.kind); }; assert_eq!(access_key.nonce, 0); assert_eq!(access_key.permission, AccessKeyPermission::FullAccess.into()); @@ -297,7 +296,7 @@ fn test_query_access_key() { fn test_query_state() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::ViewState { account_id: "test".to_string(), @@ -306,11 +305,11 @@ fn test_query_state() { }) .await .unwrap(); - assert_eq!(query_response.block_height, 0); - let state = if let QueryResponseKind::ViewState(state) = query_response.kind { + assert_eq!(query_response.query_response.block_height, 0); + let state = if let QueryResponseKind::ViewState(state) = query_response.query_response.kind { state } else { - panic!("queried state, but received something else: {:?}", query_response.kind); + panic!("queried state, but received something else: {:?}", query_response.query_response.kind); }; assert_eq!(state.values.len(), 0); }); @@ -321,7 +320,7 @@ fn test_query_state() { fn test_query_call_function() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::CallFunction { account_id: "test".to_string(), @@ -331,13 +330,13 @@ fn test_query_call_function() { }) .await .unwrap(); - assert_eq!(query_response.block_height, 0); - let call_result = if let QueryResponseKind::CallResult(call_result) = query_response.kind { + assert_eq!(query_response.query_response.block_height, 0); + let call_result = if let QueryResponseKind::CallResult(call_result) = query_response.query_response.kind { call_result } else { panic!( "expected a call function result, but received something else: {:?}", - query_response.kind + query_response.query_response.kind ); }; assert_eq!(call_result.result.len(), 0); @@ -350,17 +349,17 @@ fn test_query_call_function() { fn test_query_contract_code() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::ViewCode { account_id: "test".to_string() }, }) .await .unwrap(); - assert_eq!(query_response.block_height, 0); - let code = if let QueryResponseKind::ViewCode(code) = query_response.kind { + assert_eq!(query_response.query_response.block_height, 0); + let code = if let QueryResponseKind::ViewCode(code) = query_response.query_response.kind { code } else { - panic!("queried code, but received something else: {:?}", query_response.kind); + panic!("queried code, but received something else: {:?}", query_response.query_response.kind); }; assert_eq!(code.code, Vec::::new()); assert_eq!(code.hash.to_string(), "11111111111111111111111111111111"); @@ -559,7 +558,7 @@ fn test_invalid_methods() { fn test_query_view_account_non_existing_account_must_return_error() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::ViewAccount { account_id: "invalidaccount".to_string() }, }) @@ -567,7 +566,7 @@ fn test_query_view_account_non_existing_account_must_return_error() { .unwrap(); assert!( - !matches!(query_response.kind, QueryResponseKind::ViewAccount(_)), + !matches!(query_response.query_response.kind, QueryResponseKind::ViewAccount(_)), "queried view account for not exsiting account, but received success instead of error" ); }); @@ -578,7 +577,7 @@ fn test_query_view_account_non_existing_account_must_return_error() { fn test_view_access_key_non_existing_account_id_and_public_key_must_return_error() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::ViewAccessKey { account_id: "\u{0}\u{0}\u{0}\u{0}\u{0}9".to_string(), @@ -589,7 +588,7 @@ fn test_view_access_key_non_existing_account_id_and_public_key_must_return_error .unwrap(); assert!( - !matches!(query_response.kind, QueryResponseKind::AccessKey(_)), + !matches!(query_response.query_response.kind, QueryResponseKind::AccessKey(_)), "queried access key with not existing account and public key, received success instead of error" ); }); @@ -600,7 +599,7 @@ fn test_view_access_key_non_existing_account_id_and_public_key_must_return_error fn test_call_function_non_existing_account_method_name() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::CallFunction { method_name: @@ -614,7 +613,7 @@ fn test_call_function_non_existing_account_method_name() { .unwrap(); assert!( - !matches!(query_response.kind, QueryResponseKind::CallResult(_)), + !matches!(query_response.query_response.kind, QueryResponseKind::CallResult(_)), "queried call function with not existing account and method name, received success instead of error" ); }); @@ -625,7 +624,7 @@ fn test_call_function_non_existing_account_method_name() { fn test_view_access_key_list_non_existing_account() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::ViewAccessKeyList { account_id: "\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{c}\u{0}\u{0}\u{0}\u{0}\u{0}\u{0},".to_string(), @@ -635,7 +634,7 @@ fn test_view_access_key_list_non_existing_account() { .unwrap(); assert!( - !matches!(query_response.kind, QueryResponseKind::AccessKeyList(_)), + !matches!(query_response.query_response.kind, QueryResponseKind::AccessKeyList(_)), "queried access key list with not existing account, received success instead of error" ); }); @@ -646,7 +645,7 @@ fn test_view_access_key_list_non_existing_account() { fn test_view_state_non_existing_account_invalid_prefix() { test_with_client!(test_utils::NodeType::NonValidator, client, async move { let query_response = client - .query(RpcQueryRequest { + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { block_reference: BlockReference::latest(), request: QueryRequest::ViewState { account_id: "\u{0}\u{0}\u{0}\u{0}\u{0}\u{4}\u{0}\u{0}\u{0}\u{8}\u{0}\u{0}\u{0}\u{0}\u{0}eeeeeeeeeeeeeeeeeeeeeeeeeeeee".to_string(), @@ -657,7 +656,7 @@ fn test_view_state_non_existing_account_invalid_prefix() { .unwrap(); assert!( - !matches!(query_response.kind, QueryResponseKind::ViewState(_)), + !matches!(query_response.query_response.kind, QueryResponseKind::ViewState(_)), "queried view account for not exsiting account, but received success instead of error" ); }); From bbb6dcc6cfca15996b992abe0c941d2cbf7f7457 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Mon, 22 Feb 2021 12:24:37 +0200 Subject: [PATCH 10/22] Add block_height and block_hash to error variants where it is possible --- chain/chain-primitives/src/error.rs | 36 +++++- chain/client-primitives/src/types.rs | 30 ++++- chain/client/src/view_client.rs | 22 ++-- chain/jsonrpc-primitives/src/types/query.rs | 84 +++++++++----- chain/jsonrpc/src/lib.rs | 2 + neard/src/runtime/errors.rs | 117 ++++++++++---------- neard/src/runtime/mod.rs | 45 ++++++-- 7 files changed, 220 insertions(+), 116 deletions(-) diff --git a/chain/chain-primitives/src/error.rs b/chain/chain-primitives/src/error.rs index ee937940d8d..98766580675 100644 --- a/chain/chain-primitives/src/error.rs +++ b/chain/chain-primitives/src/error.rs @@ -15,19 +15,43 @@ use near_primitives::types::{BlockHeight, EpochId, ShardId}; #[derive(thiserror::Error, Debug)] pub enum QueryError { #[error("Account ID #{requested_account_id} is invalid")] - InvalidAccount { requested_account_id: near_primitives::types::AccountId }, + InvalidAccount { + requested_account_id: near_primitives::types::AccountId, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error("Account #{requested_account_id} does not exist while viewing")] - UnknownAccount { requested_account_id: near_primitives::types::AccountId }, + UnknownAccount { + requested_account_id: near_primitives::types::AccountId, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error( "Contract code for contract ID #{contract_account_id} has never been observed on the node" )] - NoContractCode { contract_account_id: near_primitives::types::AccountId }, + NoContractCode { + contract_account_id: near_primitives::types::AccountId, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error("Access key for public key #{public_key} does not exist while viewing")] - UnknownAccessKey { public_key: near_crypto::PublicKey }, + UnknownAccessKey { + public_key: near_crypto::PublicKey, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error("Internal error occurred: #{error_message}")] - InternalError { error_message: String }, + InternalError { + error_message: String, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error("Function call returned an error: #{error_message}")] - ContractExecutionError { error_message: String }, + ContractExecutionError { + error_message: String, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, } #[derive(Debug)] diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs index f6783ea3192..58cce322fbe 100644 --- a/chain/client-primitives/src/types.rs +++ b/chain/client-primitives/src/types.rs @@ -279,17 +279,37 @@ pub enum QueryError { #[error("The node does not track the shard")] UnavailableShard { requested_shard_id: near_primitives::types::ShardId }, #[error("Account ID #{requested_account_id} is invalid")] - InvalidAccount { requested_account_id: near_primitives::types::AccountId }, + InvalidAccount { + requested_account_id: near_primitives::types::AccountId, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error("Account #{requested_account_id} does not exist while viewing")] - UnknownAccount { requested_account_id: near_primitives::types::AccountId }, + UnknownAccount { + requested_account_id: near_primitives::types::AccountId, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error( "Contract code for contract ID #{contract_account_id} has never been observed on the node" )] - NoContractCode { contract_account_id: near_primitives::types::AccountId }, + NoContractCode { + contract_account_id: near_primitives::types::AccountId, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error("Access key for public key #{public_key} has never been observed on the node")] - UnknownAccessKey { public_key: near_crypto::PublicKey }, + UnknownAccessKey { + public_key: near_crypto::PublicKey, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error("Function call returned an error: #{vm_error}")] - ContractExecutionError { vm_error: String }, + ContractExecutionError { + vm_error: String, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, // NOTE: Currently, the underlying errors are too broad, and while we tried to handle // expected cases, we cannot statically guarantee that no other errors will be returned // in the future. diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index cae7176fad7..d974809d427 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -251,23 +251,23 @@ impl ViewClientActor { Ok(query_response) => Ok(query_response), Err(query_error) => Err(match query_error { near_chain::near_chain_primitives::error::QueryError::InternalError { - error_message, + error_message, .. } => QueryError::Unreachable { error_message }, near_chain::near_chain_primitives::error::QueryError::InvalidAccount { - requested_account_id, - } => QueryError::InvalidAccount { requested_account_id }, + requested_account_id, block_height, block_hash + } => QueryError::InvalidAccount { requested_account_id, block_height, block_hash }, near_chain::near_chain_primitives::error::QueryError::UnknownAccount { - requested_account_id, - } => QueryError::UnknownAccount { requested_account_id }, + requested_account_id, block_height, block_hash + } => QueryError::UnknownAccount { requested_account_id, block_height, block_hash }, near_chain::near_chain_primitives::error::QueryError::NoContractCode { - contract_account_id, - } => QueryError::NoContractCode { contract_account_id }, + contract_account_id, block_height, block_hash + } => QueryError::NoContractCode { contract_account_id, block_height, block_hash }, near_chain::near_chain_primitives::error::QueryError::UnknownAccessKey { - public_key, - } => QueryError::UnknownAccessKey { public_key }, + public_key, block_height, block_hash + } => QueryError::UnknownAccessKey { public_key, block_height, block_hash }, near_chain::near_chain_primitives::error::QueryError::ContractExecutionError { - error_message, - } => QueryError::ContractExecutionError { vm_error: error_message }, + error_message, block_hash, block_height + } => QueryError::ContractExecutionError { vm_error: error_message, block_height, block_hash }, }) } } else { diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index c7300049ed7..6318eb3aaf7 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -14,31 +14,51 @@ pub struct RpcQueryRequest { #[derive(thiserror::Error, Debug)] pub enum RpcQueryError { - #[error("IO Error: #{error_message}")] + #[error("IO Error: {error_message}")] IOError { error_message: String }, #[error("There are no fully synchronized blocks on the node yet")] NoSyncedBlocks, #[error("The node does not track the shard")] UnavailableShard { requested_shard_id: near_primitives::types::ShardId }, - #[error("Account ID #{requested_account_id} is invalid")] - InvalidAccount { requested_account_id: near_primitives::types::AccountId }, - #[error("account #{requested_account_id} does not exist while viewing")] - UnknownAccount { requested_account_id: near_primitives::types::AccountId }, + #[error("Account ID {requested_account_id} is invalid")] + InvalidAccount { + requested_account_id: near_primitives::types::AccountId, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, + #[error("account {requested_account_id} does not exist while viewing")] + UnknownAccount { + requested_account_id: near_primitives::types::AccountId, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, #[error( "Contract code for contract ID #{contract_account_id} has never been observed on the node" )] - NoContractCode { contract_account_id: near_primitives::types::AccountId }, - #[error("Access key for public key #{public_key} has never been observed on the node")] - UnknownAccessKey { public_key: near_crypto::PublicKey }, - #[error("Function call returned an error: #{vm_error}")] - ContractExecutionError { vm_error: String }, - #[error("The node reached its limits. Try again later. More details: #{error_message}")] + NoContractCode { + contract_account_id: near_primitives::types::AccountId, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, + #[error("Access key for public key {public_key} has never been observed on the node")] + UnknownAccessKey { + public_key: near_crypto::PublicKey, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, + #[error("Function call returned an error: {vm_error}")] + ContractExecutionError { + vm_error: String, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + }, + #[error("The node reached its limits. Try again later. More details: {error_message}")] InternalError { error_message: String }, // NOTE: Currently, the underlying errors are too broad, and while we tried to handle // expected cases, we cannot statically guarantee that no other errors will be returned // in the future. // TODO #3851: Remove this variant once we can exhaustively match all the underlying errors - #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: #{error_message}")] + #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: {error_message}")] Unreachable { error_message: String }, } @@ -132,21 +152,31 @@ impl From for RpcQueryError { near_client_primitives::types::QueryError::UnavailableShard { requested_shard_id } => { Self::UnavailableShard { requested_shard_id } } - near_client_primitives::types::QueryError::InvalidAccount { requested_account_id } => { - Self::InvalidAccount { requested_account_id } - } - near_client_primitives::types::QueryError::UnknownAccount { requested_account_id } => { - Self::UnknownAccount { requested_account_id } - } - near_client_primitives::types::QueryError::NoContractCode { contract_account_id } => { - Self::NoContractCode { contract_account_id } - } - near_client_primitives::types::QueryError::UnknownAccessKey { public_key } => { - Self::UnknownAccessKey { public_key } - } - near_client_primitives::types::QueryError::ContractExecutionError { vm_error } => { - Self::ContractExecutionError { vm_error } - } + near_client_primitives::types::QueryError::InvalidAccount { + requested_account_id, + block_height, + block_hash, + } => Self::InvalidAccount { requested_account_id, block_height, block_hash }, + near_client_primitives::types::QueryError::UnknownAccount { + requested_account_id, + block_height, + block_hash, + } => Self::UnknownAccount { requested_account_id, block_height, block_hash }, + near_client_primitives::types::QueryError::NoContractCode { + contract_account_id, + block_height, + block_hash, + } => Self::NoContractCode { contract_account_id, block_height, block_hash }, + near_client_primitives::types::QueryError::UnknownAccessKey { + public_key, + block_height, + block_hash, + } => Self::UnknownAccessKey { public_key, block_height, block_hash }, + near_client_primitives::types::QueryError::ContractExecutionError { + vm_error, + block_height, + block_hash, + } => Self::ContractExecutionError { vm_error, block_height, block_hash }, near_client_primitives::types::QueryError::Unreachable { error_message } => { tracing::warn!(target: "jsonrpc", "Unreachable error occurred: {}", &error_message); near_metrics::inc_counter_vec( diff --git a/chain/jsonrpc/src/lib.rs b/chain/jsonrpc/src/lib.rs index e50cf927c8e..ba165b5fa74 100644 --- a/chain/jsonrpc/src/lib.rs +++ b/chain/jsonrpc/src/lib.rs @@ -586,6 +586,8 @@ impl JsonRpcHandler { > { let query = Query::new(request_data.block_reference, request_data.request); let query_response = self.view_client_addr.send(query).await??; + // view_access_key + // call_function Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { query_response }) } diff --git a/neard/src/runtime/errors.rs b/neard/src/runtime/errors.rs index 0adb5280191..0d9caab9956 100644 --- a/neard/src/runtime/errors.rs +++ b/neard/src/runtime/errors.rs @@ -1,102 +1,107 @@ -use near_chain::near_chain_primitives::error::QueryError; +use easy_ext::ext; -// This wrapper struct is necessary because we cannot implement -// From<> trait for the original QueryError struct since it is a foreign type -#[derive(thiserror::Error, Debug)] -#[error(transparent)] -pub struct WrappedQueryError(pub QueryError); +use near_chain::near_chain_primitives::error::QueryError; -impl From for QueryError { - fn from(error: WrappedQueryError) -> Self { - error.0 +#[ext(FromStateViewerErrors)] +impl QueryError { + pub fn from_call_function_error( + error: node_runtime::state_viewer::errors::CallFunctionError, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + ) -> Self { + match error { + node_runtime::state_viewer::errors::CallFunctionError::InvalidAccountId { + requested_account_id, + } => Self::InvalidAccount { requested_account_id, block_height, block_hash }, + node_runtime::state_viewer::errors::CallFunctionError::AccountDoesNotExist { + requested_account_id, + } => Self::UnknownAccount { requested_account_id, block_height, block_hash }, + node_runtime::state_viewer::errors::CallFunctionError::InternalError { + error_message, + } => Self::InternalError { error_message, block_height, block_hash }, + node_runtime::state_viewer::errors::CallFunctionError::VMError { error_message } => { + Self::ContractExecutionError { error_message, block_height, block_hash } + } + } } -} -impl From for WrappedQueryError { - fn from(error: node_runtime::state_viewer::errors::ViewAccountError) -> Self { + pub fn from_view_account_error( + error: node_runtime::state_viewer::errors::ViewAccountError, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + ) -> Self { match error { node_runtime::state_viewer::errors::ViewAccountError::InvalidAccountId { requested_account_id, - } => Self(QueryError::InvalidAccount { requested_account_id }), + } => Self::InvalidAccount { requested_account_id, block_height, block_hash }, node_runtime::state_viewer::errors::ViewAccountError::AccountDoesNotExist { requested_account_id, - } => Self(QueryError::UnknownAccount { requested_account_id }), + } => Self::UnknownAccount { requested_account_id, block_height, block_hash }, node_runtime::state_viewer::errors::ViewAccountError::InternalError { error_message, - } => Self(QueryError::InternalError { error_message }), + } => Self::InternalError { error_message, block_height, block_hash }, } } -} -impl From for WrappedQueryError { - fn from(error: node_runtime::state_viewer::errors::ViewContractCodeError) -> Self { + pub fn from_view_contract_code_error( + error: node_runtime::state_viewer::errors::ViewContractCodeError, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + ) -> Self { match error { node_runtime::state_viewer::errors::ViewContractCodeError::InvalidAccountId { requested_account_id, - } => Self(QueryError::InvalidAccount { requested_account_id }), + } => Self::InvalidAccount { requested_account_id, block_height, block_hash }, node_runtime::state_viewer::errors::ViewContractCodeError::AccountDoesNotExist { requested_account_id, - } => Self(QueryError::UnknownAccount { requested_account_id }), + } => Self::UnknownAccount { requested_account_id, block_height, block_hash }, node_runtime::state_viewer::errors::ViewContractCodeError::InternalError { error_message, - } => Self(QueryError::InternalError { error_message }), + } => Self::InternalError { error_message, block_height, block_hash }, node_runtime::state_viewer::errors::ViewContractCodeError::NoContractCode { contract_account_id, - } => Self(QueryError::NoContractCode { contract_account_id }), - } - } -} - -impl From for WrappedQueryError { - fn from(error: node_runtime::state_viewer::errors::CallFunctionError) -> Self { - match error { - node_runtime::state_viewer::errors::CallFunctionError::InvalidAccountId { - requested_account_id, - } => Self(QueryError::InvalidAccount { requested_account_id }), - node_runtime::state_viewer::errors::CallFunctionError::AccountDoesNotExist { - requested_account_id, - } => Self(QueryError::UnknownAccount { requested_account_id }), - node_runtime::state_viewer::errors::CallFunctionError::InternalError { - error_message, - } => Self(QueryError::InternalError { error_message }), - node_runtime::state_viewer::errors::CallFunctionError::VMError { error_message } => { - Self(QueryError::ContractExecutionError { error_message }) - } + } => Self::NoContractCode { contract_account_id, block_height, block_hash }, } } -} -impl From for WrappedQueryError { - fn from(error: node_runtime::state_viewer::errors::ViewStateError) -> Self { + pub fn from_view_state_error( + error: node_runtime::state_viewer::errors::ViewStateError, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + ) -> Self { match error { node_runtime::state_viewer::errors::ViewStateError::InvalidAccountId { requested_account_id, - } => Self(QueryError::InvalidAccount { requested_account_id }), + } => Self::InvalidAccount { requested_account_id, block_height, block_hash }, node_runtime::state_viewer::errors::ViewStateError::InternalError { error_message } => { - Self(QueryError::InternalError { error_message }) + Self::InternalError { error_message, block_height, block_hash } } } } -} -impl From for WrappedQueryError { - fn from(error: node_runtime::state_viewer::errors::ViewAccessKeyError) -> Self { + pub fn from_view_access_key_error( + error: node_runtime::state_viewer::errors::ViewAccessKeyError, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + ) -> Self { match error { node_runtime::state_viewer::errors::ViewAccessKeyError::InvalidAccountId { requested_account_id, - } => Self(QueryError::InvalidAccount { requested_account_id }), + } => Self::InvalidAccount { requested_account_id, block_height, block_hash }, node_runtime::state_viewer::errors::ViewAccessKeyError::AccessKeyDoesNotExist { public_key, - } => Self(QueryError::UnknownAccessKey { public_key }), + } => Self::UnknownAccessKey { public_key, block_height, block_hash }, node_runtime::state_viewer::errors::ViewAccessKeyError::InternalError { error_message, - } => Self(QueryError::InternalError { error_message }), + } => Self::InternalError { error_message, block_height, block_hash }, } } -} -impl From for WrappedQueryError { - fn from(error: near_primitives::errors::EpochError) -> Self { - Self(QueryError::InternalError { error_message: error.to_string() }) + pub fn from_epoch_error( + error: near_primitives::errors::EpochError, + block_height: near_primitives::types::BlockHeight, + block_hash: near_primitives::hash::CryptoHash, + ) -> Self { + Self::InternalError { error_message: error.to_string(), block_height, block_hash } } } diff --git a/neard/src/runtime/mod.rs b/neard/src/runtime/mod.rs index be68c90b26f..463acb85d2a 100644 --- a/neard/src/runtime/mod.rs +++ b/neard/src/runtime/mod.rs @@ -59,6 +59,8 @@ use near_primitives::runtime::config::RuntimeConfig; #[cfg(feature = "protocol_feature_rectify_inflation")] use near_epoch_manager::NUM_SECONDS_IN_A_YEAR; +use errors::FromStateViewerErrors; + pub mod errors; const POISONED_LOCK_ERR: &str = "The lock was poisoned."; @@ -1251,7 +1253,7 @@ impl RuntimeAdapter for NightshadeRuntime { QueryRequest::ViewAccount { account_id } => { let account = self .view_account(shard_id, *state_root, account_id) - .map_err(errors::WrappedQueryError::from)?; + .map_err(|err| near_chain::near_chain_primitives::error::QueryError::from_view_account_error(err, block_height, *block_hash))?; Ok(QueryResponse { kind: QueryResponseKind::ViewAccount(account.into()), block_height, @@ -1261,7 +1263,7 @@ impl RuntimeAdapter for NightshadeRuntime { QueryRequest::ViewCode { account_id } => { let contract_code = self .view_contract_code(shard_id, *state_root, account_id) - .map_err(errors::WrappedQueryError::from)?; + .map_err(|err| near_chain::near_chain_primitives::error::QueryError::from_view_contract_code_error(err, block_height, *block_hash))?; Ok(QueryResponse { kind: QueryResponseKind::ViewCode(contract_code.into()), block_height, @@ -1273,9 +1275,13 @@ impl RuntimeAdapter for NightshadeRuntime { let (epoch_height, current_protocol_version) = { let mut epoch_manager = self.epoch_manager.as_ref().write().expect(POISONED_LOCK_ERR); - let epoch_info = epoch_manager - .get_epoch_info(&epoch_id) - .map_err(errors::WrappedQueryError::from)?; + let epoch_info = epoch_manager.get_epoch_info(&epoch_id).map_err(|err| { + near_chain::near_chain_primitives::error::QueryError::from_epoch_error( + err, + block_height, + *block_hash, + ) + })?; (epoch_info.epoch_height, epoch_info.protocol_version) }; @@ -1298,7 +1304,7 @@ impl RuntimeAdapter for NightshadeRuntime { #[cfg(feature = "protocol_feature_evm")] self.evm_chain_id(), ) - .map_err(errors::WrappedQueryError::from)?; + .map_err(|err| near_chain::near_chain_primitives::error::QueryError::from_call_function_error(err, block_height, *block_hash))?; Ok(QueryResponse { kind: QueryResponseKind::CallResult(CallResult { result: call_function_result, @@ -1311,7 +1317,13 @@ impl RuntimeAdapter for NightshadeRuntime { QueryRequest::ViewState { account_id, prefix } => { let view_state_result = self .view_state(shard_id, *state_root, account_id, prefix.as_ref()) - .map_err(errors::WrappedQueryError::from)?; + .map_err(|err| { + near_chain::near_chain_primitives::error::QueryError::from_view_state_error( + err, + block_height, + *block_hash, + ) + })?; Ok(QueryResponse { kind: QueryResponseKind::ViewState(view_state_result), block_height, @@ -1319,9 +1331,14 @@ impl RuntimeAdapter for NightshadeRuntime { }) } QueryRequest::ViewAccessKeyList { account_id } => { - let access_key_list = self - .view_access_keys(shard_id, *state_root, account_id) - .map_err(errors::WrappedQueryError::from)?; + let access_key_list = + self.view_access_keys(shard_id, *state_root, account_id).map_err(|err| { + near_chain::near_chain_primitives::error::QueryError::from_view_access_key_error( + err, + block_height, + *block_hash, + ) + })?; Ok(QueryResponse { kind: QueryResponseKind::AccessKeyList( access_key_list @@ -1339,7 +1356,13 @@ impl RuntimeAdapter for NightshadeRuntime { QueryRequest::ViewAccessKey { account_id, public_key } => { let access_key = self .view_access_key(shard_id, *state_root, account_id, public_key) - .map_err(errors::WrappedQueryError::from)?; + .map_err(|err| { + near_chain::near_chain_primitives::error::QueryError::from_view_access_key_error( + err, + block_height, + *block_hash, + ) + })?; Ok(QueryResponse { kind: QueryResponseKind::AccessKey(access_key.into()), block_height, From 06a3b851682cbbb5d08ee8d384aa9e8ce595ddfc Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Wed, 24 Feb 2021 20:09:30 +0200 Subject: [PATCH 11/22] Address review suggestions. Backward compatible resposne from query --- chain/client-primitives/src/types.rs | 22 +++++---- chain/client/src/view_client.rs | 2 +- chain/jsonrpc-primitives/src/types/query.rs | 2 +- chain/jsonrpc/src/lib.rs | 51 +++++++++++++++++++-- chain/jsonrpc/tests/rpc_query.rs | 44 +++++++++++++----- neard/src/runtime/errors.rs | 4 +- 6 files changed, 94 insertions(+), 31 deletions(-) diff --git a/chain/client-primitives/src/types.rs b/chain/client-primitives/src/types.rs index 58cce322fbe..bbe9b4f755b 100644 --- a/chain/client-primitives/src/types.rs +++ b/chain/client-primitives/src/types.rs @@ -272,39 +272,41 @@ impl Message for Query { #[derive(thiserror::Error, Debug)] pub enum QueryError { - #[error("IO Error: #{error_message}")] - IOError { error_message: String }, + #[error("Internal error: {error_message}")] + InternalError { error_message: String }, #[error("There are no fully synchronized blocks on the node yet")] NoSyncedBlocks, - #[error("The node does not track the shard")] + #[error("The node does not track the shard ID {requested_shard_id}")] UnavailableShard { requested_shard_id: near_primitives::types::ShardId }, - #[error("Account ID #{requested_account_id} is invalid")] + #[error("Account ID {requested_account_id} is invalid")] InvalidAccount { requested_account_id: near_primitives::types::AccountId, block_height: near_primitives::types::BlockHeight, block_hash: near_primitives::hash::CryptoHash, }, - #[error("Account #{requested_account_id} does not exist while viewing")] + #[error( + "Account {requested_account_id} does not exist while viewing at block #{block_height}" + )] UnknownAccount { requested_account_id: near_primitives::types::AccountId, block_height: near_primitives::types::BlockHeight, block_hash: near_primitives::hash::CryptoHash, }, #[error( - "Contract code for contract ID #{contract_account_id} has never been observed on the node" + "Contract code for contract ID {contract_account_id} has never been observed on the node at block #{block_height}" )] NoContractCode { contract_account_id: near_primitives::types::AccountId, block_height: near_primitives::types::BlockHeight, block_hash: near_primitives::hash::CryptoHash, }, - #[error("Access key for public key #{public_key} has never been observed on the node")] + #[error("Access key for public key {public_key} has never been observed on the node at block #{block_height}")] UnknownAccessKey { public_key: near_crypto::PublicKey, block_height: near_primitives::types::BlockHeight, block_hash: near_primitives::hash::CryptoHash, }, - #[error("Function call returned an error: #{vm_error}")] + #[error("Function call returned an error: {vm_error}")] ContractExecutionError { vm_error: String, block_height: near_primitives::types::BlockHeight, @@ -314,7 +316,7 @@ pub enum QueryError { // expected cases, we cannot statically guarantee that no other errors will be returned // in the future. // TODO #3851: Remove this variant once we can exhaustively match all the underlying errors - #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: #{error_message}")] + #[error("It is a bug if you receive this error type, please, report this incident: https://github.com/near/nearcore/issues/new/choose. Details: {error_message}")] Unreachable { error_message: String }, } @@ -322,7 +324,7 @@ impl From for QueryError { fn from(error: near_chain_primitives::Error) -> Self { match error.kind() { near_chain_primitives::ErrorKind::IOErr(error_message) => { - Self::IOError { error_message } + Self::InternalError { error_message } } _ => Self::Unreachable { error_message: error.to_string() }, } diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index d974809d427..2bc28902431 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -252,7 +252,7 @@ impl ViewClientActor { Err(query_error) => Err(match query_error { near_chain::near_chain_primitives::error::QueryError::InternalError { error_message, .. - } => QueryError::Unreachable { error_message }, + } => QueryError::InternalError { error_message }, near_chain::near_chain_primitives::error::QueryError::InvalidAccount { requested_account_id, block_height, block_hash } => QueryError::InvalidAccount { requested_account_id, block_height, block_hash }, diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index 6318eb3aaf7..a5401be4597 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -145,7 +145,7 @@ impl RpcQueryRequest { impl From for RpcQueryError { fn from(error: near_client_primitives::types::QueryError) -> Self { match error { - near_client_primitives::types::QueryError::IOError { error_message } => { + near_client_primitives::types::QueryError::InternalError { error_message } => { Self::InternalError { error_message } } near_client_primitives::types::QueryError::NoSyncedBlocks => Self::NoSyncedBlocks, diff --git a/chain/jsonrpc/src/lib.rs b/chain/jsonrpc/src/lib.rs index ba165b5fa74..05a55182a4b 100644 --- a/chain/jsonrpc/src/lib.rs +++ b/chain/jsonrpc/src/lib.rs @@ -585,10 +585,53 @@ impl JsonRpcHandler { near_jsonrpc_primitives::types::query::RpcQueryError, > { let query = Query::new(request_data.block_reference, request_data.request); - let query_response = self.view_client_addr.send(query).await??; - // view_access_key - // call_function - Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { query_response }) + // This match is used here to give backward compatible error message for specific + // error variants. Should be refactored once structured errors fully shipped + match self + .view_client_addr + .send(query) + .await? + .map_err(near_jsonrpc_primitives::types::query::RpcQueryError::from) + { + Ok(query_response) => { + Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { query_response }) + } + Err(err) => match err { + near_jsonrpc_primitives::types::query::RpcQueryError::ContractExecutionError { + vm_error, + block_height, + block_hash, + } => Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { + query_response: near_primitives::views::QueryResponse { + kind: near_primitives::views::QueryResponseKind::Error( + near_primitives::views::QueryError { error: vm_error, logs: vec![] }, + ), + block_height, + block_hash, + }, + }), + near_jsonrpc_primitives::types::query::RpcQueryError::UnknownAccessKey { + public_key, + block_height, + block_hash, + } => Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { + query_response: near_primitives::views::QueryResponse { + kind: near_primitives::views::QueryResponseKind::Error( + near_primitives::views::QueryError { + error: format!( + "access key {} does not exist while viewing", + public_key.to_string() + ), + logs: vec![], + }, + ), + block_height, + block_hash, + }, + }), + _ => Err(err), + }, + } } async fn tx_status_common( diff --git a/chain/jsonrpc/tests/rpc_query.rs b/chain/jsonrpc/tests/rpc_query.rs index d9acc9a9b26..9f946184b74 100644 --- a/chain/jsonrpc/tests/rpc_query.rs +++ b/chain/jsonrpc/tests/rpc_query.rs @@ -188,7 +188,10 @@ fn test_query_account() { { account } else { - panic!("queried account, but received something else: {:?}", query_response.query_response.kind); + panic!( + "queried account, but received something else: {:?}", + query_response.query_response.kind + ); }; assert_eq!(account_info.amount, 0); assert_eq!(account_info.code_hash.as_ref(), &[0; 32]); @@ -230,11 +233,15 @@ fn test_query_access_keys() { .await .unwrap(); assert_eq!(query_response.query_response.block_height, 0); - let access_keys = if let QueryResponseKind::AccessKeyList(access_keys) = query_response.query_response.kind + let access_keys = if let QueryResponseKind::AccessKeyList(access_keys) = + query_response.query_response.kind { access_keys } else { - panic!("queried access keys, but received something else: {:?}", query_response.query_response.kind); + panic!( + "queried access keys, but received something else: {:?}", + query_response.query_response.kind + ); }; assert_eq!(access_keys.keys.len(), 1); assert_eq!(access_keys.keys[0].access_key, AccessKey::full_access().into()); @@ -281,11 +288,15 @@ fn test_query_access_key() { .await .unwrap(); assert_eq!(query_response.query_response.block_height, 0); - let access_key = if let QueryResponseKind::AccessKey(access_keys) = query_response.query_response.kind { - access_keys - } else { - panic!("queried access keys, but received something else: {:?}", query_response.query_response.kind); - }; + let access_key = + if let QueryResponseKind::AccessKey(access_keys) = query_response.query_response.kind { + access_keys + } else { + panic!( + "queried access keys, but received something else: {:?}", + query_response.query_response.kind + ); + }; assert_eq!(access_key.nonce, 0); assert_eq!(access_key.permission, AccessKeyPermission::FullAccess.into()); }); @@ -306,10 +317,14 @@ fn test_query_state() { .await .unwrap(); assert_eq!(query_response.query_response.block_height, 0); - let state = if let QueryResponseKind::ViewState(state) = query_response.query_response.kind { + let state = if let QueryResponseKind::ViewState(state) = query_response.query_response.kind + { state } else { - panic!("queried state, but received something else: {:?}", query_response.query_response.kind); + panic!( + "queried state, but received something else: {:?}", + query_response.query_response.kind + ); }; assert_eq!(state.values.len(), 0); }); @@ -331,7 +346,9 @@ fn test_query_call_function() { .await .unwrap(); assert_eq!(query_response.query_response.block_height, 0); - let call_result = if let QueryResponseKind::CallResult(call_result) = query_response.query_response.kind { + let call_result = if let QueryResponseKind::CallResult(call_result) = + query_response.query_response.kind + { call_result } else { panic!( @@ -359,7 +376,10 @@ fn test_query_contract_code() { let code = if let QueryResponseKind::ViewCode(code) = query_response.query_response.kind { code } else { - panic!("queried code, but received something else: {:?}", query_response.query_response.kind); + panic!( + "queried code, but received something else: {:?}", + query_response.query_response.kind + ); }; assert_eq!(code.code, Vec::::new()); assert_eq!(code.hash.to_string(), "11111111111111111111111111111111"); diff --git a/neard/src/runtime/errors.rs b/neard/src/runtime/errors.rs index 0d9caab9956..3ebfb4a824d 100644 --- a/neard/src/runtime/errors.rs +++ b/neard/src/runtime/errors.rs @@ -1,8 +1,6 @@ -use easy_ext::ext; - use near_chain::near_chain_primitives::error::QueryError; -#[ext(FromStateViewerErrors)] +#[easy_ext::ext(FromStateViewerErrors)] impl QueryError { pub fn from_call_function_error( error: node_runtime::state_viewer::errors::CallFunctionError, From 14693b4ef8726cac2ea43127db998bc7903b300f Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Wed, 24 Feb 2021 21:16:11 +0200 Subject: [PATCH 12/22] Make backward compatibility in `query` method via near_client::QueryError instead of RpcQueryError --- chain/client/src/lib.rs | 2 +- chain/jsonrpc/src/lib.rs | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/chain/client/src/lib.rs b/chain/client/src/lib.rs index 456866ecbc6..cccca5b7b45 100644 --- a/chain/client/src/lib.rs +++ b/chain/client/src/lib.rs @@ -6,7 +6,7 @@ pub use near_client_primitives::types::{ GetExecutionOutcome, GetExecutionOutcomeResponse, GetExecutionOutcomesForBlock, GetGasPrice, GetNetworkInfo, GetNextLightClientBlock, GetProtocolConfig, GetReceipt, GetStateChanges, GetStateChangesInBlock, GetStateChangesWithCauseInBlock, GetValidatorInfo, GetValidatorOrdered, - Query, Status, StatusResponse, SyncStatus, TxStatus, TxStatusError, + Query, QueryError, Status, StatusResponse, SyncStatus, TxStatus, TxStatusError, }; pub use crate::client::Client; diff --git a/chain/jsonrpc/src/lib.rs b/chain/jsonrpc/src/lib.rs index 05a55182a4b..36845ab9374 100644 --- a/chain/jsonrpc/src/lib.rs +++ b/chain/jsonrpc/src/lib.rs @@ -587,17 +587,12 @@ impl JsonRpcHandler { let query = Query::new(request_data.block_reference, request_data.request); // This match is used here to give backward compatible error message for specific // error variants. Should be refactored once structured errors fully shipped - match self - .view_client_addr - .send(query) - .await? - .map_err(near_jsonrpc_primitives::types::query::RpcQueryError::from) - { + match self.view_client_addr.send(query).await? { Ok(query_response) => { Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { query_response }) } Err(err) => match err { - near_jsonrpc_primitives::types::query::RpcQueryError::ContractExecutionError { + near_client::QueryError::ContractExecutionError { vm_error, block_height, block_hash, @@ -610,7 +605,7 @@ impl JsonRpcHandler { block_hash, }, }), - near_jsonrpc_primitives::types::query::RpcQueryError::UnknownAccessKey { + near_client::QueryError::UnknownAccessKey { public_key, block_height, block_hash, @@ -629,7 +624,7 @@ impl JsonRpcHandler { block_hash, }, }), - _ => Err(err), + _ => Err(err.into()), }, } } From 8db2e3566ac010d38833cb4cba4de66a047fbd6c Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Thu, 25 Feb 2021 10:48:26 +0200 Subject: [PATCH 13/22] Fix some tests --- chain/client/tests/cross_shard_tx.rs | 2 +- neard/tests/stake_nodes.rs | 74 ++++++++++++---------------- 2 files changed, 32 insertions(+), 44 deletions(-) diff --git a/chain/client/tests/cross_shard_tx.rs b/chain/client/tests/cross_shard_tx.rs index c797e3de93d..fc637cfe8ba 100644 --- a/chain/client/tests/cross_shard_tx.rs +++ b/chain/client/tests/cross_shard_tx.rs @@ -59,7 +59,7 @@ fn test_keyvalue_runtime_balances() { QueryRequest::ViewAccount { account_id: flat_validators[i].to_string() }, )) .then(move |res| { - let query_response = res.unwrap().unwrap().unwrap(); + let query_response = res.unwrap().unwrap(); if let ViewAccount(view_account_result) = query_response.kind { assert_eq!(view_account_result.amount, expected); successful_queries2.fetch_add(1, Ordering::Relaxed); diff --git a/neard/tests/stake_nodes.rs b/neard/tests/stake_nodes.rs index 4bb26a8ddc1..a378a8a8fb2 100644 --- a/neard/tests/stake_nodes.rs +++ b/neard/tests/stake_nodes.rs @@ -251,18 +251,16 @@ fn test_validator_kickout() { .clone(), }, )) - .then(move |res| { - match res.unwrap().unwrap().unwrap().kind { - QueryResponseKind::ViewAccount(result) => { - if result.locked == 0 - || result.amount == TESTING_INIT_BALANCE - { - mark.store(true, Ordering::SeqCst); - } - future::ready(()) + .then(move |res| match res.unwrap().unwrap().kind { + QueryResponseKind::ViewAccount(result) => { + if result.locked == 0 + || result.amount == TESTING_INIT_BALANCE + { + mark.store(true, Ordering::SeqCst); } - _ => panic!("wrong return result"), + future::ready(()) } + _ => panic!("wrong return result"), }), ); } @@ -280,23 +278,17 @@ fn test_validator_kickout() { .clone(), }, )) - .then(move |res| { - match res.unwrap().unwrap().unwrap().kind { - QueryResponseKind::ViewAccount(result) => { - assert_eq!( - result.locked, - TESTING_INIT_STAKE - ); - assert_eq!( - result.amount, - TESTING_INIT_BALANCE - - TESTING_INIT_STAKE - ); - mark.store(true, Ordering::SeqCst); - future::ready(()) - } - _ => panic!("wrong return result"), + .then(move |res| match res.unwrap().unwrap().kind { + QueryResponseKind::ViewAccount(result) => { + assert_eq!(result.locked, TESTING_INIT_STAKE); + assert_eq!( + result.amount, + TESTING_INIT_BALANCE - TESTING_INIT_STAKE + ); + mark.store(true, Ordering::SeqCst); + future::ready(()) } + _ => panic!("wrong return result"), }), ); } @@ -421,16 +413,14 @@ fn test_validator_join() { account_id: test_nodes[1].account_id.clone(), }, )) - .then(move |res| { - match res.unwrap().unwrap().unwrap().kind { - QueryResponseKind::ViewAccount(result) => { - if result.locked == 0 { - done1_copy2.store(true, Ordering::SeqCst); - } - future::ready(()) + .then(move |res| match res.unwrap().unwrap().kind { + QueryResponseKind::ViewAccount(result) => { + if result.locked == 0 { + done1_copy2.store(true, Ordering::SeqCst); } - _ => panic!("wrong return result"), + future::ready(()) } + _ => panic!("wrong return result"), }), ); actix::spawn( @@ -442,17 +432,15 @@ fn test_validator_join() { account_id: test_nodes[2].account_id.clone(), }, )) - .then(move |res| { - match res.unwrap().unwrap().unwrap().kind { - QueryResponseKind::ViewAccount(result) => { - if result.locked == TESTING_INIT_STAKE { - done2_copy2.store(true, Ordering::SeqCst); - } - - future::ready(()) + .then(move |res| match res.unwrap().unwrap().kind { + QueryResponseKind::ViewAccount(result) => { + if result.locked == TESTING_INIT_STAKE { + done2_copy2.store(true, Ordering::SeqCst); } - _ => panic!("wrong return result"), + + future::ready(()) } + _ => panic!("wrong return result"), }), ); } From 7c3fb95d8a1856847224fd856f8e9269dc915a98 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Thu, 25 Feb 2021 13:33:28 +0200 Subject: [PATCH 14/22] Address review suggestion --- chain/jsonrpc-primitives/src/types/query.rs | 54 ++++++++++++++++++++- chain/jsonrpc/src/lib.rs | 40 +++++++-------- 2 files changed, 70 insertions(+), 24 deletions(-) diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index a5401be4597..ba1f08a37cc 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -65,7 +65,21 @@ pub enum RpcQueryError { #[derive(Serialize, Deserialize)] pub struct RpcQueryResponse { #[serde(flatten)] - pub query_response: near_primitives::views::QueryResponse, + pub kind: QueryResponseKind, + pub block_height: near_primitives::types::BlockHeight, + pub block_hash: near_primitives::hash::CryptoHash, +} + +#[derive(Serialize, Deserialize)] +#[serde(untagged)] +pub enum QueryResponseKind { + ViewAccount(near_primitives::views::AccountView), + ViewCode(near_primitives::views::ContractCodeView), + ViewState(near_primitives::views::ViewStateResult), + CallResult(near_primitives::views::CallResult), + Error(near_primitives::views::QueryError), + AccessKey(near_primitives::views::AccessKeyView), + AccessKeyList(near_primitives::views::AccessKeyList), } impl RpcQueryRequest { @@ -189,6 +203,44 @@ impl From for RpcQueryError { } } +impl From for RpcQueryResponse { + fn from(query_response: near_primitives::views::QueryResponse) -> Self { + Self { + kind: query_response.kind.into(), + block_hash: query_response.block_hash, + block_height: query_response.block_height, + } + } +} + +impl From for QueryResponseKind { + fn from(query_response_kind: near_primitives::views::QueryResponseKind) -> Self { + match query_response_kind { + near_primitives::views::QueryResponseKind::ViewAccount(account_view) => { + Self::ViewAccount(account_view) + } + near_primitives::views::QueryResponseKind::ViewCode(contract_code_view) => { + Self::ViewCode(contract_code_view) + } + near_primitives::views::QueryResponseKind::ViewState(view_state_result) => { + Self::ViewState(view_state_result) + } + near_primitives::views::QueryResponseKind::CallResult(call_result) => { + Self::CallResult(call_result) + } + near_primitives::views::QueryResponseKind::Error(query_error) => { + Self::Error(query_error) + } + near_primitives::views::QueryResponseKind::AccessKey(access_key_view) => { + Self::AccessKey(access_key_view) + } + near_primitives::views::QueryResponseKind::AccessKeyList(access_key_list) => { + Self::AccessKeyList(access_key_list) + } + } + } +} + impl From for crate::errors::RpcError { fn from(error: RpcQueryError) -> Self { let error_data = Some(Value::String(error.to_string())); diff --git a/chain/jsonrpc/src/lib.rs b/chain/jsonrpc/src/lib.rs index 36845ab9374..7b2f40fec88 100644 --- a/chain/jsonrpc/src/lib.rs +++ b/chain/jsonrpc/src/lib.rs @@ -588,41 +588,35 @@ impl JsonRpcHandler { // This match is used here to give backward compatible error message for specific // error variants. Should be refactored once structured errors fully shipped match self.view_client_addr.send(query).await? { - Ok(query_response) => { - Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { query_response }) - } + Ok(query_response) => Ok(query_response.into()), Err(err) => match err { near_client::QueryError::ContractExecutionError { vm_error, block_height, block_hash, } => Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { - query_response: near_primitives::views::QueryResponse { - kind: near_primitives::views::QueryResponseKind::Error( - near_primitives::views::QueryError { error: vm_error, logs: vec![] }, - ), - block_height, - block_hash, - }, + kind: near_jsonrpc_primitives::types::query::QueryResponseKind::Error( + near_primitives::views::QueryError { error: vm_error, logs: vec![] }, + ), + block_height, + block_hash, }), near_client::QueryError::UnknownAccessKey { public_key, block_height, block_hash, } => Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { - query_response: near_primitives::views::QueryResponse { - kind: near_primitives::views::QueryResponseKind::Error( - near_primitives::views::QueryError { - error: format!( - "access key {} does not exist while viewing", - public_key.to_string() - ), - logs: vec![], - }, - ), - block_height, - block_hash, - }, + kind: near_jsonrpc_primitives::types::query::QueryResponseKind::Error( + near_primitives::views::QueryError { + error: format!( + "access key {} does not exist while viewing", + public_key.to_string() + ), + logs: vec![], + }, + ), + block_height, + block_hash, }), _ => Err(err.into()), }, From feba4c7271ce8e6eec5f9fe032d336c6ed9ae9c9 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Fri, 26 Feb 2021 11:52:55 +0200 Subject: [PATCH 15/22] Fix tests according to changes in jsonrpc query method refactoring --- Cargo.lock | 1 + chain/client/tests/cross_shard_tx.rs | 4 +- chain/jsonrpc-primitives/src/types/query.rs | 2 +- chain/jsonrpc/client/src/lib.rs | 8 ++- chain/jsonrpc/tests/rpc_query.rs | 77 ++++++++------------- core/primitives/src/views.rs | 61 +--------------- neard/tests/rpc_nodes.rs | 3 +- test-utils/testlib/Cargo.toml | 1 + test-utils/testlib/src/user/rpc_user.rs | 49 ++++++++++--- 9 files changed, 82 insertions(+), 124 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ebae686afcc..b5ea5f6212d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5534,6 +5534,7 @@ dependencies = [ "near-evm-runner", "near-jsonrpc", "near-jsonrpc-client", + "near-jsonrpc-primitives", "near-logger-utils", "near-network", "near-primitives", diff --git a/chain/client/tests/cross_shard_tx.rs b/chain/client/tests/cross_shard_tx.rs index fc637cfe8ba..47ae0e345ef 100644 --- a/chain/client/tests/cross_shard_tx.rs +++ b/chain/client/tests/cross_shard_tx.rs @@ -163,7 +163,7 @@ mod tests { } fn test_cross_shard_tx_callback( - res: Result, String>, MailboxError>, + res: Result, MailboxError>, account_id: AccountId, connectors: Arc, Addr)>>>, iteration: Arc, @@ -180,7 +180,7 @@ mod tests { min_ratio: Option, max_ratio: Option, ) { - let res = res.unwrap().and_then(|r| r.ok_or_else(|| "Request routed".to_string())); + let res = res.unwrap(); let query_response = match res { Ok(query_response) => query_response, diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index ba1f08a37cc..a451e135cc4 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -70,7 +70,7 @@ pub struct RpcQueryResponse { pub block_hash: near_primitives::hash::CryptoHash, } -#[derive(Serialize, Deserialize)] +#[derive(Serialize, Deserialize, Debug)] #[serde(untagged)] pub enum QueryResponseKind { ViewAccount(near_primitives::views::AccountView), diff --git a/chain/jsonrpc/client/src/lib.rs b/chain/jsonrpc/client/src/lib.rs index 5603444ce8d..e29727e136c 100644 --- a/chain/jsonrpc/client/src/lib.rs +++ b/chain/jsonrpc/client/src/lib.rs @@ -14,7 +14,7 @@ use near_primitives::hash::CryptoHash; use near_primitives::types::{BlockId, BlockReference, MaybeBlockId, ShardId}; use near_primitives::views::{ BlockView, ChunkView, EpochValidatorInfo, FinalExecutionOutcomeView, GasPriceView, - QueryResponse, StatusResponse, ValidatorStakeView, + StatusResponse, ValidatorStakeView, }; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -194,7 +194,11 @@ 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 { + pub fn query_by_path( + &self, + path: String, + data: String, + ) -> RpcRequest { call_method(&self.client, &self.server_addr, "query", [path, data]) } diff --git a/chain/jsonrpc/tests/rpc_query.rs b/chain/jsonrpc/tests/rpc_query.rs index 9f946184b74..4c62ae02805 100644 --- a/chain/jsonrpc/tests/rpc_query.rs +++ b/chain/jsonrpc/tests/rpc_query.rs @@ -9,12 +9,13 @@ use near_crypto::{KeyType, PublicKey, Signature}; use near_jsonrpc::client::new_client; use near_jsonrpc_client::ChunkId; use near_jsonrpc_primitives::rpc::RpcValidatorsOrderedRequest; +use near_jsonrpc_primitives::types::query::QueryResponseKind; use near_logger_utils::init_test_logger; use near_network::test_utils::WaitOrTimeout; use near_primitives::account::{AccessKey, AccessKeyPermission}; use near_primitives::hash::CryptoHash; use near_primitives::types::{BlockId, BlockReference, ShardId, SyncCheckpoint}; -use near_primitives::views::{QueryRequest, QueryResponseKind}; +use near_primitives::views::QueryRequest; #[macro_use] pub mod test_utils; @@ -181,17 +182,14 @@ fn test_query_account() { .await .unwrap(); for query_response in [query_response_1, query_response_2, query_response_3].iter() { - assert_eq!(query_response.query_response.block_height, 0); - assert_eq!(query_response.query_response.block_hash, block_hash); + assert_eq!(query_response.block_height, 0); + assert_eq!(query_response.block_hash, block_hash); let account_info = if let QueryResponseKind::ViewAccount(ref account) = - query_response.query_response.kind + query_response.kind { account } else { - panic!( - "queried account, but received something else: {:?}", - query_response.query_response.kind - ); + 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]); @@ -232,16 +230,12 @@ fn test_query_access_keys() { }) .await .unwrap(); - assert_eq!(query_response.query_response.block_height, 0); - let access_keys = if let QueryResponseKind::AccessKeyList(access_keys) = - query_response.query_response.kind + 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.query_response.kind - ); + 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()); @@ -287,16 +281,12 @@ fn test_query_access_key() { }) .await .unwrap(); - assert_eq!(query_response.query_response.block_height, 0); - let access_key = - if let QueryResponseKind::AccessKey(access_keys) = query_response.query_response.kind { - access_keys - } else { - panic!( - "queried access keys, but received something else: {:?}", - query_response.query_response.kind - ); - }; + 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()); }); @@ -316,15 +306,11 @@ fn test_query_state() { }) .await .unwrap(); - assert_eq!(query_response.query_response.block_height, 0); - let state = if let QueryResponseKind::ViewState(state) = query_response.query_response.kind - { + assert_eq!(query_response.block_height, 0); + let state = if let QueryResponseKind::ViewState(state) = query_response.kind { state } else { - panic!( - "queried state, but received something else: {:?}", - query_response.query_response.kind - ); + panic!("queried state, but received something else: {:?}", query_response.kind); }; assert_eq!(state.values.len(), 0); }); @@ -345,15 +331,13 @@ fn test_query_call_function() { }) .await .unwrap(); - assert_eq!(query_response.query_response.block_height, 0); - let call_result = if let QueryResponseKind::CallResult(call_result) = - query_response.query_response.kind - { + assert_eq!(query_response.block_height, 0); + let call_result = if let QueryResponseKind::CallResult(call_result) = query_response.kind { call_result } else { panic!( "expected a call function result, but received something else: {:?}", - query_response.query_response.kind + query_response.kind ); }; assert_eq!(call_result.result.len(), 0); @@ -372,14 +356,11 @@ fn test_query_contract_code() { }) .await .unwrap(); - assert_eq!(query_response.query_response.block_height, 0); - let code = if let QueryResponseKind::ViewCode(code) = query_response.query_response.kind { + assert_eq!(query_response.block_height, 0); + let code = if let QueryResponseKind::ViewCode(code) = query_response.kind { code } else { - panic!( - "queried code, but received something else: {:?}", - query_response.query_response.kind - ); + panic!("queried code, but received something else: {:?}", query_response.kind); }; assert_eq!(code.code, Vec::::new()); assert_eq!(code.hash.to_string(), "11111111111111111111111111111111"); @@ -586,7 +567,7 @@ fn test_query_view_account_non_existing_account_must_return_error() { .unwrap(); assert!( - !matches!(query_response.query_response.kind, QueryResponseKind::ViewAccount(_)), + !matches!(query_response.kind, QueryResponseKind::ViewAccount(_)), "queried view account for not exsiting account, but received success instead of error" ); }); @@ -608,7 +589,7 @@ fn test_view_access_key_non_existing_account_id_and_public_key_must_return_error .unwrap(); assert!( - !matches!(query_response.query_response.kind, QueryResponseKind::AccessKey(_)), + !matches!(query_response.kind, QueryResponseKind::AccessKey(_)), "queried access key with not existing account and public key, received success instead of error" ); }); @@ -633,7 +614,7 @@ fn test_call_function_non_existing_account_method_name() { .unwrap(); assert!( - !matches!(query_response.query_response.kind, QueryResponseKind::CallResult(_)), + !matches!(query_response.kind, QueryResponseKind::CallResult(_)), "queried call function with not existing account and method name, received success instead of error" ); }); @@ -654,7 +635,7 @@ fn test_view_access_key_list_non_existing_account() { .unwrap(); assert!( - !matches!(query_response.query_response.kind, QueryResponseKind::AccessKeyList(_)), + !matches!(query_response.kind, QueryResponseKind::AccessKeyList(_)), "queried access key list with not existing account, received success instead of error" ); }); @@ -676,7 +657,7 @@ fn test_view_state_non_existing_account_invalid_prefix() { .unwrap(); assert!( - !matches!(query_response.query_response.kind, QueryResponseKind::ViewState(_)), + !matches!(query_response.kind, QueryResponseKind::ViewState(_)), "queried view account for not exsiting account, but received success instead of error" ); }); diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index ec3a2c72d9b..6cc7aed09ae 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -246,8 +246,7 @@ impl std::iter::FromIterator for AccessKeyList { } } -#[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] -#[serde(untagged)] +#[derive(BorshSerialize, BorshDeserialize, Debug, PartialEq, Eq, Clone)] pub enum QueryResponseKind { ViewAccount(AccountView), ViewCode(ContractCodeView), @@ -287,9 +286,8 @@ pub enum QueryRequest { }, } -#[derive(BorshSerialize, BorshDeserialize, Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] +#[derive(BorshSerialize, BorshDeserialize, Debug, PartialEq, Eq, Clone)] pub struct QueryResponse { - #[serde(flatten)] pub kind: QueryResponseKind, pub block_height: BlockHeight, pub block_hash: CryptoHash, @@ -332,61 +330,6 @@ pub struct StatusResponse { pub validator_account_id: Option, } -impl TryFrom for AccountView { - type Error = String; - - fn try_from(query_response: QueryResponse) -> Result { - match query_response.kind { - QueryResponseKind::ViewAccount(acc) => Ok(acc), - _ => Err("Invalid type of response".into()), - } - } -} - -impl TryFrom for CallResult { - type Error = String; - - fn try_from(query_response: QueryResponse) -> Result { - match query_response.kind { - QueryResponseKind::CallResult(res) => Ok(res), - _ => Err("Invalid type of response".into()), - } - } -} - -impl TryFrom for ViewStateResult { - type Error = String; - - fn try_from(query_response: QueryResponse) -> Result { - match query_response.kind { - QueryResponseKind::ViewState(vs) => Ok(vs), - _ => Err("Invalid type of response".into()), - } - } -} - -impl TryFrom for AccessKeyView { - type Error = String; - - fn try_from(query_response: QueryResponse) -> Result { - match query_response.kind { - QueryResponseKind::AccessKey(access_key) => Ok(access_key), - _ => Err("Invalid type of response".into()), - } - } -} - -impl TryFrom for ContractCodeView { - type Error = String; - - fn try_from(query_response: QueryResponse) -> Result { - match query_response.kind { - QueryResponseKind::ViewCode(contract_code) => Ok(contract_code), - _ => Err("Invalid type of response".into()), - } - } -} - #[derive(Serialize, Deserialize, Debug, Clone)] pub struct ChallengeView { // TODO: decide how to represent challenges in json. diff --git a/neard/tests/rpc_nodes.rs b/neard/tests/rpc_nodes.rs index ce1e258d931..046ac3d7cb6 100644 --- a/neard/tests/rpc_nodes.rs +++ b/neard/tests/rpc_nodes.rs @@ -19,7 +19,6 @@ use near_primitives::transaction::{PartialExecutionStatus, SignedTransaction}; use near_primitives::types::{BlockId, BlockReference, Finality, TransactionOrReceiptId}; use near_primitives::views::{ ExecutionOutcomeView, ExecutionStatusView, FinalExecutionOutcomeViewEnum, FinalExecutionStatus, - QueryResponseKind, }; use neard::config::TESTING_INIT_BALANCE; use std::sync::atomic::AtomicBool; @@ -387,7 +386,7 @@ fn test_rpc_routing() { .query_by_path("account/near.2".to_string(), "".to_string()) .map_err(|err| panic_on_rpc_error!(err)) .map_ok(move |result| match result.kind { - QueryResponseKind::ViewAccount(account_view) => { + near_jsonrpc_primitives::types::query::QueryResponseKind::ViewAccount(account_view) => { assert_eq!( account_view.amount, TESTING_INIT_BALANCE diff --git a/test-utils/testlib/Cargo.toml b/test-utils/testlib/Cargo.toml index 8bffefcdd82..60e41d09631 100644 --- a/test-utils/testlib/Cargo.toml +++ b/test-utils/testlib/Cargo.toml @@ -40,6 +40,7 @@ near-vm-errors = { path = "../../runtime/near-vm-errors" } near-chain = { path = "../../chain/chain" } near-client = { path = "../../chain/client" } near-jsonrpc = { path = "../../chain/jsonrpc" } +near-jsonrpc-primitives = { path = "../../chain/jsonrpc-primitives" } near-network = { path = "../../chain/network" } near-jsonrpc-client = { path = "../../chain/jsonrpc/client" } neard = { path = "../../neard" } diff --git a/test-utils/testlib/src/user/rpc_user.rs b/test-utils/testlib/src/user/rpc_user.rs index 811f042704f..23fe20bbaad 100644 --- a/test-utils/testlib/src/user/rpc_user.rs +++ b/test-utils/testlib/src/user/rpc_user.rs @@ -1,4 +1,3 @@ -use std::convert::TryInto; use std::sync::Arc; use std::thread; use std::time::Duration; @@ -11,6 +10,7 @@ use near_crypto::{PublicKey, Signer}; use near_jsonrpc::client::{new_client, JsonRpcClient}; use near_jsonrpc::ServerError; use near_jsonrpc_client::ChunkId; +use near_jsonrpc_primitives::types::query::RpcQueryResponse; use near_primitives::hash::CryptoHash; use near_primitives::receipt::Receipt; use near_primitives::serialize::{to_base, to_base64}; @@ -20,8 +20,7 @@ use near_primitives::types::{ }; use near_primitives::views::{ AccessKeyView, AccountView, BlockView, CallResult, ChunkView, ContractCodeView, - EpochValidatorInfo, ExecutionOutcomeView, FinalExecutionOutcomeView, QueryResponse, - ViewStateResult, + EpochValidatorInfo, ExecutionOutcomeView, FinalExecutionOutcomeView, ViewStateResult, }; use crate::user::User; @@ -51,7 +50,7 @@ impl RpcUser { self.actix(|client| client.status()).ok() } - pub fn query(&self, path: String, data: &[u8]) -> Result { + pub fn query(&self, path: String, data: &[u8]) -> Result { let data = to_base(data); self.actix(move |client| client.query_by_path(path, data).map_err(|err| err.to_string())) } @@ -63,15 +62,33 @@ impl RpcUser { impl User for RpcUser { fn view_account(&self, account_id: &AccountId) -> Result { - self.query(format!("account/{}", account_id), &[])?.try_into() + let query_response = self.query(format!("account/{}", account_id), &[])?; + match query_response.kind { + near_jsonrpc_primitives::types::query::QueryResponseKind::ViewAccount(account_view) => { + Ok(account_view) + } + _ => Err("Invalid type of response".into()), + } } fn view_state(&self, account_id: &AccountId, prefix: &[u8]) -> Result { - self.query(format!("contract/{}", account_id), prefix)?.try_into() + let query_response = self.query(format!("contract/{}", account_id), prefix)?; + match query_response.kind { + near_jsonrpc_primitives::types::query::QueryResponseKind::ViewState( + view_state_result, + ) => Ok(view_state_result), + _ => Err("Invalid type of response".into()), + } } fn view_contract_code(&self, account_id: &AccountId) -> Result { - self.query(format!("code/{}", account_id), &[])?.try_into() + let query_response = self.query(format!("code/{}", account_id), &[])?; + match query_response.kind { + near_jsonrpc_primitives::types::query::QueryResponseKind::ViewCode( + contract_code_view, + ) => Ok(contract_code_view), + _ => Err("Invalid type of response".into()), + } } fn view_call( @@ -80,8 +97,13 @@ impl User for RpcUser { method_name: &str, args: &[u8], ) -> Result { - self.query(format!("call/{}/{}", account_id, method_name), args) - .and_then(|value| value.try_into()) + let query_response = self.query(format!("call/{}/{}", account_id, method_name), args)?; + match query_response.kind { + near_jsonrpc_primitives::types::query::QueryResponseKind::CallResult(call_result) => { + Ok(call_result) + } + _ => Err("Invalid type of response".into()), + } } fn add_transaction(&self, transaction: SignedTransaction) -> Result<(), ServerError> { @@ -158,7 +180,14 @@ impl User for RpcUser { account_id: &AccountId, public_key: &PublicKey, ) -> Result { - self.query(format!("access_key/{}/{}", account_id, public_key), &[])?.try_into() + let query_response = + self.query(format!("access_key/{}/{}", account_id, public_key), &[])?; + match query_response.kind { + near_jsonrpc_primitives::types::query::QueryResponseKind::AccessKey(access_key) => { + Ok(access_key) + } + _ => Err("Invalid type of response".into()), + } } fn signer(&self) -> Arc { From ad18379a8efa2603fd6a6c627232bd0f40e55a63 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Sun, 28 Feb 2021 16:19:58 +0200 Subject: [PATCH 16/22] Drop QueryResponseKind::Error variant and add backward compatibility on JSON-RPC side --- chain/jsonrpc-primitives/src/types/query.rs | 4 - chain/jsonrpc/src/lib.rs | 81 +++++++++++---------- chain/rosetta-rpc/src/utils.rs | 40 +++++----- core/primitives/src/views.rs | 2 +- 4 files changed, 62 insertions(+), 65 deletions(-) diff --git a/chain/jsonrpc-primitives/src/types/query.rs b/chain/jsonrpc-primitives/src/types/query.rs index a451e135cc4..32623c35625 100644 --- a/chain/jsonrpc-primitives/src/types/query.rs +++ b/chain/jsonrpc-primitives/src/types/query.rs @@ -77,7 +77,6 @@ pub enum QueryResponseKind { ViewCode(near_primitives::views::ContractCodeView), ViewState(near_primitives::views::ViewStateResult), CallResult(near_primitives::views::CallResult), - Error(near_primitives::views::QueryError), AccessKey(near_primitives::views::AccessKeyView), AccessKeyList(near_primitives::views::AccessKeyList), } @@ -228,9 +227,6 @@ impl From for QueryResponseKind { near_primitives::views::QueryResponseKind::CallResult(call_result) => { Self::CallResult(call_result) } - near_primitives::views::QueryResponseKind::Error(query_error) => { - Self::Error(query_error) - } near_primitives::views::QueryResponseKind::AccessKey(access_key_view) => { Self::AccessKey(access_key_view) } diff --git a/chain/jsonrpc/src/lib.rs b/chain/jsonrpc/src/lib.rs index 7b2f40fec88..642c4d3f329 100644 --- a/chain/jsonrpc/src/lib.rs +++ b/chain/jsonrpc/src/lib.rs @@ -11,7 +11,7 @@ use futures::{FutureExt, TryFutureExt}; use prometheus; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; -use serde_json::Value; +use serde_json::{Value, json}; use tokio::time::{sleep, timeout}; use tracing::info; @@ -175,6 +175,43 @@ fn timeout_err() -> RpcError { RpcError::server_error(Some(ServerError::Timeout)) } +/// This function processes response from query method to introduce +/// backward compatible response in case of specific errors +fn process_query_response(query_response: Result) -> Result { + // This match is used here to give backward compatible error message for specific + // error variants. Should be refactored once structured errors fully shipped + match query_response { + Ok(rpc_query_response) => serde_json::to_value(rpc_query_response) + .map_err(|err| RpcError::parse_error(err.to_string())), + Err(err) => match err { + near_jsonrpc_primitives::types::query::RpcQueryError::ContractExecutionError { + vm_error, + block_height, + block_hash, + } => Ok(json!({ + "error": vm_error, + "logs": json!([]), + "block_height": block_height, + "block_hash": block_hash, + })), + near_jsonrpc_primitives::types::query::RpcQueryError::UnknownAccessKey { + public_key, + block_height, + block_hash, + } => Ok(json!({ + "error": format!( + "access key {} does not exist while viewing", + public_key.to_string() + ), + "logs": json!([]), + "block_height": block_height, + "block_hash": block_hash, + })), + _ => Err(err.into()), + } + } +} + struct JsonRpcHandler { client_addr: Addr, view_client_addr: Addr, @@ -271,9 +308,8 @@ impl JsonRpcHandler { "query" => { let rpc_query_request = near_jsonrpc_primitives::types::query::RpcQueryRequest::parse(request.params)?; - let query_response = self.query(rpc_query_request).await?; - serde_json::to_value(query_response) - .map_err(|err| RpcError::parse_error(err.to_string())) + let query_response = self.query(rpc_query_request).await; + process_query_response(query_response) } "status" => self.status().await, "tx" => self.tx_status_common(request.params, false).await, @@ -585,42 +621,7 @@ impl JsonRpcHandler { near_jsonrpc_primitives::types::query::RpcQueryError, > { let query = Query::new(request_data.block_reference, request_data.request); - // This match is used here to give backward compatible error message for specific - // error variants. Should be refactored once structured errors fully shipped - match self.view_client_addr.send(query).await? { - Ok(query_response) => Ok(query_response.into()), - Err(err) => match err { - near_client::QueryError::ContractExecutionError { - vm_error, - block_height, - block_hash, - } => Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { - kind: near_jsonrpc_primitives::types::query::QueryResponseKind::Error( - near_primitives::views::QueryError { error: vm_error, logs: vec![] }, - ), - block_height, - block_hash, - }), - near_client::QueryError::UnknownAccessKey { - public_key, - block_height, - block_hash, - } => Ok(near_jsonrpc_primitives::types::query::RpcQueryResponse { - kind: near_jsonrpc_primitives::types::query::QueryResponseKind::Error( - near_primitives::views::QueryError { - error: format!( - "access key {} does not exist while viewing", - public_key.to_string() - ), - logs: vec![], - }, - ), - block_height, - block_hash, - }), - _ => Err(err.into()), - }, - } + Ok(self.view_client_addr.send(query).await??.into()) } async fn tx_status_common( diff --git a/chain/rosetta-rpc/src/utils.rs b/chain/rosetta-rpc/src/utils.rs index ddc831e876d..c2617dcd6a0 100644 --- a/chain/rosetta-rpc/src/utils.rs +++ b/chain/rosetta-rpc/src/utils.rs @@ -327,16 +327,16 @@ pub(crate) async fn query_account( near_primitives::views::QueryResponseKind::ViewAccount(account_info) => { Ok((account_info_response.block_hash, account_info_response.block_height, account_info)) } - near_primitives::views::QueryResponseKind::Error(near_primitives::views::QueryError { - error, - .. - }) => { - if error.contains("does not exist") { - Err(crate::errors::ErrorKind::NotFound(error)) - } else { - Err(crate::errors::ErrorKind::InternalError(error)) - } - } + // near_primitives::views::QueryResponseKind::Error(near_primitives::views::QueryError { + // error, + // .. + // }) => { + // if error.contains("does not exist") { + // Err(crate::errors::ErrorKind::NotFound(error)) + // } else { + // Err(crate::errors::ErrorKind::InternalError(error)) + // } + // } _ => Err(crate::errors::ErrorKind::InternalInvariantError(format!( "queried ViewAccount, but received {:?}.", account_info_response.kind @@ -410,16 +410,16 @@ pub(crate) async fn query_access_key( access_key_query_response.block_height, access_key, )), - near_primitives::views::QueryResponseKind::Error(near_primitives::views::QueryError { - error, - .. - }) => { - if error.contains("does not exist") { - Err(crate::errors::ErrorKind::NotFound(error)) - } else { - Err(crate::errors::ErrorKind::InternalError(error)) - } - } + // near_primitives::views::QueryResponseKind::Error(near_primitives::views::QueryError { + // error, + // .. + // }) => { + // if error.contains("does not exist") { + // Err(crate::errors::ErrorKind::NotFound(error)) + // } else { + // Err(crate::errors::ErrorKind::InternalError(error)) + // } + // } _ => Err(crate::errors::ErrorKind::InternalInvariantError( "queried ViewAccessKey, but received something else.".to_string(), )), diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index 6cc7aed09ae..113d3bfce46 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -252,7 +252,7 @@ pub enum QueryResponseKind { ViewCode(ContractCodeView), ViewState(ViewStateResult), CallResult(CallResult), - Error(QueryError), + // Error(QueryError), AccessKey(AccessKeyView), AccessKeyList(AccessKeyList), } From 8de65b9e073f3ed3c35844725924aee4b3164126 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Sun, 28 Feb 2021 16:20:47 +0200 Subject: [PATCH 17/22] Remove redundant commented code --- chain/rosetta-rpc/src/utils.rs | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/chain/rosetta-rpc/src/utils.rs b/chain/rosetta-rpc/src/utils.rs index c2617dcd6a0..b2aa498c285 100644 --- a/chain/rosetta-rpc/src/utils.rs +++ b/chain/rosetta-rpc/src/utils.rs @@ -327,16 +327,6 @@ pub(crate) async fn query_account( near_primitives::views::QueryResponseKind::ViewAccount(account_info) => { Ok((account_info_response.block_hash, account_info_response.block_height, account_info)) } - // near_primitives::views::QueryResponseKind::Error(near_primitives::views::QueryError { - // error, - // .. - // }) => { - // if error.contains("does not exist") { - // Err(crate::errors::ErrorKind::NotFound(error)) - // } else { - // Err(crate::errors::ErrorKind::InternalError(error)) - // } - // } _ => Err(crate::errors::ErrorKind::InternalInvariantError(format!( "queried ViewAccount, but received {:?}.", account_info_response.kind @@ -410,16 +400,6 @@ pub(crate) async fn query_access_key( access_key_query_response.block_height, access_key, )), - // near_primitives::views::QueryResponseKind::Error(near_primitives::views::QueryError { - // error, - // .. - // }) => { - // if error.contains("does not exist") { - // Err(crate::errors::ErrorKind::NotFound(error)) - // } else { - // Err(crate::errors::ErrorKind::InternalError(error)) - // } - // } _ => Err(crate::errors::ErrorKind::InternalInvariantError( "queried ViewAccessKey, but received something else.".to_string(), )), From 4ddc300fe95272a9568d0280bcacb426e5f92eda Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Wed, 3 Mar 2021 11:58:18 +0200 Subject: [PATCH 18/22] Add tests for query method. Address issue with wrong refactor logic --- chain/client/src/view_client.rs | 89 +++++++++-------- chain/client/tests/catching_up.rs | 4 +- chain/client/tests/query_client.rs | 2 +- neard/tests/rpc_nodes.rs | 155 +++++++++++++++++++++++------ 4 files changed, 173 insertions(+), 77 deletions(-) diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index 2bc28902431..7c278cb07bf 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -226,52 +226,55 @@ impl ViewClientActor { QueryRequest::ViewCode { account_id, .. } => account_id, }; let shard_id = self.runtime_adapter.account_id_to_shard_id(account_id); - let head = self.chain.head()?; - if self.runtime_adapter.cares_about_shard( - self.validator_account_id.as_ref(), - &head.last_block_hash, + let chunk_extra = self.chain.get_chunk_extra(header.hash(), shard_id)?; + let state_root = chunk_extra.state_root; + match self.runtime_adapter.query( shard_id, - true, + &state_root, + header.height(), + header.raw_timestamp(), + header.prev_hash(), + header.hash(), + header.epoch_id(), + &msg.request, ) { - let chunk_extra = self.chain.get_chunk_extra(header.hash(), shard_id)?; - let state_root = chunk_extra.state_root; - match self - .runtime_adapter - .query( - shard_id, - &state_root, - header.height(), - header.raw_timestamp(), - header.prev_hash(), - header.hash(), - header.epoch_id(), - &msg.request, - ) { - Ok(query_response) => Ok(query_response), - Err(query_error) => Err(match query_error { - near_chain::near_chain_primitives::error::QueryError::InternalError { - error_message, .. - } => QueryError::InternalError { error_message }, - near_chain::near_chain_primitives::error::QueryError::InvalidAccount { - requested_account_id, block_height, block_hash - } => QueryError::InvalidAccount { requested_account_id, block_height, block_hash }, - near_chain::near_chain_primitives::error::QueryError::UnknownAccount { - requested_account_id, block_height, block_hash - } => QueryError::UnknownAccount { requested_account_id, block_height, block_hash }, - near_chain::near_chain_primitives::error::QueryError::NoContractCode { - contract_account_id, block_height, block_hash - } => QueryError::NoContractCode { contract_account_id, block_height, block_hash }, - near_chain::near_chain_primitives::error::QueryError::UnknownAccessKey { - public_key, block_height, block_hash - } => QueryError::UnknownAccessKey { public_key, block_height, block_hash }, - near_chain::near_chain_primitives::error::QueryError::ContractExecutionError { - error_message, block_hash, block_height - } => QueryError::ContractExecutionError { vm_error: error_message, block_height, block_hash }, - }) - } - } else { - Err(QueryError::UnavailableShard { requested_shard_id: shard_id }) + Ok(query_response) => Ok(query_response), + Err(query_error) => Err(match query_error { + near_chain::near_chain_primitives::error::QueryError::InternalError { + error_message, + .. + } => QueryError::InternalError { error_message }, + near_chain::near_chain_primitives::error::QueryError::InvalidAccount { + requested_account_id, + block_height, + block_hash, + } => QueryError::InvalidAccount { requested_account_id, block_height, block_hash }, + near_chain::near_chain_primitives::error::QueryError::UnknownAccount { + requested_account_id, + block_height, + block_hash, + } => QueryError::UnknownAccount { requested_account_id, block_height, block_hash }, + near_chain::near_chain_primitives::error::QueryError::NoContractCode { + contract_account_id, + block_height, + block_hash, + } => QueryError::NoContractCode { contract_account_id, block_height, block_hash }, + near_chain::near_chain_primitives::error::QueryError::UnknownAccessKey { + public_key, + block_height, + block_hash, + } => QueryError::UnknownAccessKey { public_key, block_height, block_hash }, + near_chain::near_chain_primitives::error::QueryError::ContractExecutionError { + error_message, + block_hash, + block_height, + } => QueryError::ContractExecutionError { + vm_error: error_message, + block_height, + block_hash, + }, + }), } } diff --git a/chain/client/tests/catching_up.rs b/chain/client/tests/catching_up.rs index d01a726abe8..bc38b63ca53 100644 --- a/chain/client/tests/catching_up.rs +++ b/chain/client/tests/catching_up.rs @@ -323,7 +323,7 @@ mod tests { )) .then(move |res| { let res_inner = res.unwrap(); - if let Ok(Some(query_response)) = res_inner + if let Ok(query_response) = res_inner { if let ViewAccount( view_account_result, @@ -528,7 +528,7 @@ mod tests { )) .then(move |res| { let res_inner = res.unwrap(); - if let Ok(Some(query_response)) = + if let Ok(query_response) = res_inner { if let ViewAccount( diff --git a/chain/client/tests/query_client.rs b/chain/client/tests/query_client.rs index b255f820e3a..61a1efe1756 100644 --- a/chain/client/tests/query_client.rs +++ b/chain/client/tests/query_client.rs @@ -36,7 +36,7 @@ fn query_client() { QueryRequest::ViewAccount { account_id: "test".to_owned() }, )) .then(|res| { - match res.unwrap().unwrap().unwrap().kind { + match res.unwrap().unwrap().kind { QueryResponseKind::ViewAccount(_) => (), _ => panic!("Invalid response"), } diff --git a/neard/tests/rpc_nodes.rs b/neard/tests/rpc_nodes.rs index 046ac3d7cb6..afc87967fa4 100644 --- a/neard/tests/rpc_nodes.rs +++ b/neard/tests/rpc_nodes.rs @@ -709,40 +709,133 @@ fn test_protocol_config_rpc() { } #[test] -fn test_query_rpc() { +fn test_query_rpc_account_view_must_succeed() { init_integration_logger(); heavy_test(|| { - System::builder() - .stop_on_panic(true) - .run(move || { - let num_nodes = 1; - let dirs = (0..num_nodes) - .map(|i| { - tempfile::Builder::new() - .prefix(&format!("protocol_config{}", i)) - .tempdir() - .unwrap() + run_actix_until_stop(async move { + let num_nodes = 1; + let dirs = (0..num_nodes) + .map(|i| { + tempfile::Builder::new() + .prefix(&format!("protocol_config{}", i)) + .tempdir() + .unwrap() + }) + .collect::>(); + let (_genesis, rpc_addrs, _) = start_nodes(1, &dirs, 1, 0, 10, 0); + + actix::spawn(async move { + let client = new_client(&format!("http://{}", rpc_addrs[0])); + let query_response = client + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { + block_reference: near_primitives::types::BlockReference::Finality( + Finality::Final, + ), + request: near_primitives::views::QueryRequest::ViewAccount { + account_id: "near.0".to_string(), + }, }) - .collect::>(); - let (_genesis, rpc_addrs, _) = start_nodes(1, &dirs, 1, 0, 10, 0); + .await + .unwrap(); + let account = + if let near_jsonrpc_primitives::types::query::QueryResponseKind::ViewAccount( + account, + ) = query_response.kind + { + account + } else { + panic!( + "expected a account view result, but received something else: {:?}", + query_response.kind + ); + }; + assert!(matches!(account, near_primitives::views::AccountView { .. })); + System::current().stop(); + }); + }); + }); +} - actix::spawn(async move { - let client = new_client(&format!("http://{}", rpc_addrs[0])); - let query_response = client - .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { - block_reference: near_primitives::types::BlockReference::Finality( - Finality::Final, - ), - request: near_primitives::views::QueryRequest::ViewAccount { - account_id: "test.near".to_string(), - }, - }) - .await - .unwrap(); - assert_eq!(false, true); - System::current().stop(); - }); - }) - .unwrap(); +#[test] +fn test_query_rpc_account_view_invalid_account_must_return_error() { + init_integration_logger(); + heavy_test(|| { + run_actix_until_stop(async move { + let num_nodes = 1; + let dirs = (0..num_nodes) + .map(|i| { + tempfile::Builder::new() + .prefix(&format!("protocol_config{}", i)) + .tempdir() + .unwrap() + }) + .collect::>(); + let (_genesis, rpc_addrs, _) = start_nodes(1, &dirs, 1, 0, 10, 0); + + actix::spawn(async move { + let client = new_client(&format!("http://{}", rpc_addrs[0])); + let query_response = client + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { + block_reference: near_primitives::types::BlockReference::Finality( + Finality::Final, + ), + request: near_primitives::views::QueryRequest::ViewAccount { + account_id: "1nval$d*@cc0ount".to_string(), + }, + }) + .await; + assert!(query_response.is_err()); + let error_message = match query_response { + Ok(result) => panic!("expected error but received Ok: {:?}", result.kind), + Err(err) => err.data.unwrap(), + }; + assert!(error_message.to_string().contains("is invalid"), "{}", error_message); + System::current().stop(); + }); + }); + }); +} + +#[test] +fn test_query_rpc_account_view_account_doesnt_exist_must_return_error() { + init_integration_logger(); + heavy_test(|| { + run_actix_until_stop(async move { + let num_nodes = 1; + let dirs = (0..num_nodes) + .map(|i| { + tempfile::Builder::new() + .prefix(&format!("protocol_config{}", i)) + .tempdir() + .unwrap() + }) + .collect::>(); + let (_genesis, rpc_addrs, _) = start_nodes(1, &dirs, 1, 0, 10, 0); + + actix::spawn(async move { + let client = new_client(&format!("http://{}", rpc_addrs[0])); + let query_response = client + .query(near_jsonrpc_primitives::types::query::RpcQueryRequest { + block_reference: near_primitives::types::BlockReference::Finality( + Finality::Final, + ), + request: near_primitives::views::QueryRequest::ViewAccount { + account_id: "accountdoesntexist.0".to_string(), + }, + }) + .await; + assert!(query_response.is_err()); + let error_message = match query_response { + Ok(result) => panic!("expected error but received Ok: {:?}", result.kind), + Err(err) => err.data.unwrap(), + }; + assert!( + error_message.to_string().contains("does not exist while viewing"), + "{}", + error_message + ); + System::current().stop(); + }); + }); }); } From c602e37a2bab67b108ff2fa686a80b7261106192 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Thu, 4 Mar 2021 22:31:35 +0200 Subject: [PATCH 19/22] Clean up and make more strict test check --- core/primitives/src/views.rs | 1 - neard/tests/rpc_nodes.rs | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/core/primitives/src/views.rs b/core/primitives/src/views.rs index 113d3bfce46..5d208be61d8 100644 --- a/core/primitives/src/views.rs +++ b/core/primitives/src/views.rs @@ -252,7 +252,6 @@ pub enum QueryResponseKind { ViewCode(ContractCodeView), ViewState(ViewStateResult), CallResult(CallResult), - // Error(QueryError), AccessKey(AccessKeyView), AccessKeyList(AccessKeyList), } diff --git a/neard/tests/rpc_nodes.rs b/neard/tests/rpc_nodes.rs index afc87967fa4..4532375d371 100644 --- a/neard/tests/rpc_nodes.rs +++ b/neard/tests/rpc_nodes.rs @@ -789,7 +789,7 @@ fn test_query_rpc_account_view_invalid_account_must_return_error() { Ok(result) => panic!("expected error but received Ok: {:?}", result.kind), Err(err) => err.data.unwrap(), }; - assert!(error_message.to_string().contains("is invalid"), "{}", error_message); + assert!(error_message.to_string().contains("Account ID 1nval$d*@cc0ount is invalid"), "{}", error_message); System::current().stop(); }); }); @@ -830,7 +830,7 @@ fn test_query_rpc_account_view_account_doesnt_exist_must_return_error() { Err(err) => err.data.unwrap(), }; assert!( - error_message.to_string().contains("does not exist while viewing"), + error_message.to_string().contains("account accountdoesntexist.0 does not exist while viewing"), "{}", error_message ); From 1aa7ca31034dbbda7d128e61525b9d747b52d694 Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Fri, 5 Mar 2021 09:35:08 +0200 Subject: [PATCH 20/22] run cargo fmt --- chain/client/tests/catching_up.rs | 7 ++----- chain/jsonrpc/src/lib.rs | 11 ++++++++--- neard/tests/rpc_nodes.rs | 10 ++++++++-- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/chain/client/tests/catching_up.rs b/chain/client/tests/catching_up.rs index bc38b63ca53..d5ff0627a2d 100644 --- a/chain/client/tests/catching_up.rs +++ b/chain/client/tests/catching_up.rs @@ -323,8 +323,7 @@ mod tests { )) .then(move |res| { let res_inner = res.unwrap(); - if let Ok(query_response) = res_inner - { + if let Ok(query_response) = res_inner { if let ViewAccount( view_account_result, ) = query_response.kind @@ -528,9 +527,7 @@ mod tests { )) .then(move |res| { let res_inner = res.unwrap(); - if let Ok(query_response) = - res_inner - { + if let Ok(query_response) = res_inner { if let ViewAccount( view_account_result, ) = query_response.kind diff --git a/chain/jsonrpc/src/lib.rs b/chain/jsonrpc/src/lib.rs index 642c4d3f329..2ed698bd885 100644 --- a/chain/jsonrpc/src/lib.rs +++ b/chain/jsonrpc/src/lib.rs @@ -11,7 +11,7 @@ use futures::{FutureExt, TryFutureExt}; use prometheus; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; -use serde_json::{Value, json}; +use serde_json::{json, Value}; use tokio::time::{sleep, timeout}; use tracing::info; @@ -177,7 +177,12 @@ fn timeout_err() -> RpcError { /// This function processes response from query method to introduce /// backward compatible response in case of specific errors -fn process_query_response(query_response: Result) -> Result { +fn process_query_response( + query_response: Result< + near_jsonrpc_primitives::types::query::RpcQueryResponse, + near_jsonrpc_primitives::types::query::RpcQueryError, + >, +) -> Result { // This match is used here to give backward compatible error message for specific // error variants. Should be refactored once structured errors fully shipped match query_response { @@ -208,7 +213,7 @@ fn process_query_response(query_response: Result Err(err.into()), - } + }, } } diff --git a/neard/tests/rpc_nodes.rs b/neard/tests/rpc_nodes.rs index 4532375d371..53588dd00e2 100644 --- a/neard/tests/rpc_nodes.rs +++ b/neard/tests/rpc_nodes.rs @@ -789,7 +789,11 @@ fn test_query_rpc_account_view_invalid_account_must_return_error() { Ok(result) => panic!("expected error but received Ok: {:?}", result.kind), Err(err) => err.data.unwrap(), }; - assert!(error_message.to_string().contains("Account ID 1nval$d*@cc0ount is invalid"), "{}", error_message); + assert!( + error_message.to_string().contains("Account ID 1nval$d*@cc0ount is invalid"), + "{}", + error_message + ); System::current().stop(); }); }); @@ -830,7 +834,9 @@ fn test_query_rpc_account_view_account_doesnt_exist_must_return_error() { Err(err) => err.data.unwrap(), }; assert!( - error_message.to_string().contains("account accountdoesntexist.0 does not exist while viewing"), + error_message + .to_string() + .contains("account accountdoesntexist.0 does not exist while viewing"), "{}", error_message ); From efde9e0999959dd1bafa61b8f27cc3b285ebf8bb Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Tue, 9 Mar 2021 16:54:43 +0200 Subject: [PATCH 21/22] Mark rpc routing tests as ignore --- chain/client/src/view_client.rs | 3 ++- neard/tests/rpc_nodes.rs | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/chain/client/src/view_client.rs b/chain/client/src/view_client.rs index 7c278cb07bf..6c30fd06b98 100644 --- a/chain/client/src/view_client.rs +++ b/chain/client/src/view_client.rs @@ -22,7 +22,8 @@ use near_client_primitives::types::{ Error, GetBlock, GetBlockError, GetBlockProof, GetBlockProofResponse, GetBlockWithMerkleTree, GetChunkError, GetExecutionOutcome, GetExecutionOutcomesForBlock, GetGasPrice, GetProtocolConfig, GetProtocolConfigError, GetReceipt, GetReceiptError, - GetStateChangesWithCauseInBlock, GetValidatorInfoError, Query, TxStatus, TxStatusError, + GetStateChangesWithCauseInBlock, GetValidatorInfoError, Query, QueryError, TxStatus, + TxStatusError, }; #[cfg(feature = "adversarial")] use near_network::types::NetworkAdversarialMessage; diff --git a/neard/tests/rpc_nodes.rs b/neard/tests/rpc_nodes.rs index 53588dd00e2..b3817d83397 100644 --- a/neard/tests/rpc_nodes.rs +++ b/neard/tests/rpc_nodes.rs @@ -357,6 +357,7 @@ fn test_tx_status_with_light_client1() { }); } +#[ignore] // TODO: change this test https://github.com/near/nearcore/issues/4062 #[test] fn test_rpc_routing() { init_integration_logger(); @@ -410,6 +411,7 @@ fn test_rpc_routing() { }); } +#[ignore] // TODO: change this test https://github.com/near/nearcore/issues/4062 /// When we call rpc to view an account that does not exist, an error should be routed back. #[test] fn test_rpc_routing_error() { From 4816a805ff85d53606f7177b66340bed7a7ecc7f Mon Sep 17 00:00:00 2001 From: Bohdan Khorolets Date: Tue, 9 Mar 2021 17:38:53 +0200 Subject: [PATCH 22/22] Fix tests --- neard/src/lib.rs | 1 - runtime/runtime/src/state_viewer/errors.rs | 10 +++++----- runtime/runtime/src/state_viewer/mod.rs | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/neard/src/lib.rs b/neard/src/lib.rs index bb8d7833960..e524d2e80cc 100644 --- a/neard/src/lib.rs +++ b/neard/src/lib.rs @@ -29,7 +29,6 @@ use near_store::migrations::{ #[cfg(feature = "protocol_feature_rectify_inflation")] use near_store::migrations::migrate_18_to_rectify_inflation; -pub use runtime::errors as runtime_errors; pub mod config; pub mod genesis_validate; diff --git a/runtime/runtime/src/state_viewer/errors.rs b/runtime/runtime/src/state_viewer/errors.rs index acd0e0f3e94..d0f93a4dd71 100644 --- a/runtime/runtime/src/state_viewer/errors.rs +++ b/runtime/runtime/src/state_viewer/errors.rs @@ -1,6 +1,6 @@ #[derive(thiserror::Error, Debug)] pub enum ViewAccountError { - #[error("Account ID #{requested_account_id} is invalid")] + #[error("Account ID \"{requested_account_id}\" is invalid")] InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Account ID #{requested_account_id} does not exist")] AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, @@ -10,7 +10,7 @@ pub enum ViewAccountError { #[derive(thiserror::Error, Debug)] pub enum ViewContractCodeError { - #[error("Account ID #{requested_account_id} is invalid")] + #[error("Account ID \"{requested_account_id}\" is invalid")] InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Account ID #{requested_account_id} does not exist")] AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, @@ -22,7 +22,7 @@ pub enum ViewContractCodeError { #[derive(thiserror::Error, Debug)] pub enum ViewAccessKeyError { - #[error("Account ID #{requested_account_id} is invalid")] + #[error("Account ID \"{requested_account_id}\" is invalid")] InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Access key for public key #{public_key} does not exist")] AccessKeyDoesNotExist { public_key: near_crypto::PublicKey }, @@ -32,7 +32,7 @@ pub enum ViewAccessKeyError { #[derive(thiserror::Error, Debug)] pub enum ViewStateError { - #[error("Account ID #{requested_account_id} is invalid")] + #[error("Account ID \"{requested_account_id}\" is invalid")] InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Internal error: #{error_message}")] InternalError { error_message: String }, @@ -40,7 +40,7 @@ pub enum ViewStateError { #[derive(thiserror::Error, Debug)] pub enum CallFunctionError { - #[error("Account ID #{requested_account_id} is invalid")] + #[error("Account ID \"{requested_account_id}\" is invalid")] InvalidAccountId { requested_account_id: near_primitives::types::AccountId }, #[error("Account ID #{requested_account_id} does not exist")] AccountDoesNotExist { requested_account_id: near_primitives::types::AccountId }, diff --git a/runtime/runtime/src/state_viewer/mod.rs b/runtime/runtime/src/state_viewer/mod.rs index 1e4e9b2f11e..30607548285 100644 --- a/runtime/runtime/src/state_viewer/mod.rs +++ b/runtime/runtime/src/state_viewer/mod.rs @@ -329,7 +329,7 @@ mod tests { let err = result.unwrap_err(); assert!( - err.to_string().contains(r#"Contract ID "bad!contract" is not valid"#), + err.to_string().contains(r#"Account ID "bad!contract" is invalid"#), format!("Got different error that doesn't match: {}", err) ); }