From df500e41526460112b7b49ddf78107421d5e6e9b Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 8 Sep 2023 12:24:55 +0300 Subject: [PATCH 01/22] fix(drive): -32603 error code on broadcast --- packages/rs-drive-abci/src/abci/handlers.rs | 24 +++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index b5d3993bf4..6084b92d2e 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -37,6 +37,7 @@ use crate::abci::server::AbciApplication; use crate::error::execution::ExecutionError; +use ciborium::cbor; use crate::error::Error; use crate::rpc::core::CoreRPCLike; @@ -56,6 +57,7 @@ use super::AbciError; use dpp::platform_value::string_encoding::{encode, Encoding}; +use crate::error::serialization::SerializationError; use crate::execution::types::block_execution_context::v0::{ BlockExecutionContextV0Getters, BlockExecutionContextV0MutableGetters, BlockExecutionContextV0Setters, @@ -598,11 +600,16 @@ where .serialize_to_bytes_with_platform_version(platform_version) .map_err(|e| ResponseException::from(Error::Protocol(e)))?; - let error_data = json!({ - "data": { + let error_data = cbor!({ + "data" => { "serializedError": consensus_error_bytes } - }); + }) + .map_err(|err| { + Error::Serialization(SerializationError::CorruptedSerialization(format!( + "can't create cbor: {err}" + ))) + })?; let mut error_data_buffer: Vec = Vec::new(); ciborium::ser::into_writer(&error_data, &mut error_data_buffer) @@ -633,9 +640,14 @@ where }) } Err(error) => { - let error_data = json!({ - "message": "Internal error", - }); + let error_data = cbor!({ + "message" => "Internal error", + }) + .map_err(|err| { + Error::Serialization(SerializationError::CorruptedSerialization(format!( + "can't create cbor: {err}" + ))) + })?; let mut error_data_buffer: Vec = Vec::new(); ciborium::ser::into_writer(&error_data, &mut error_data_buffer) From 45485452d405ec529d7031fb400fb1fc29461799 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 8 Sep 2023 12:28:20 +0300 Subject: [PATCH 02/22] fix: wrong cbor macros syntax --- packages/rs-drive-abci/src/abci/handlers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index 6084b92d2e..d10915c4cd 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -602,7 +602,7 @@ where let error_data = cbor!({ "data" => { - "serializedError": consensus_error_bytes + "serializedError" => consensus_error_bytes } }) .map_err(|err| { From 0d72be2a4920f6afb787a219be9ff36bc38bc80f Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 8 Sep 2023 12:37:03 +0300 Subject: [PATCH 03/22] fix: wrong error data encoded --- packages/rs-drive-abci/src/abci/handlers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index d10915c4cd..cc3a291ad4 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -617,7 +617,7 @@ where ( consensus_error.code(), - encode(&consensus_error_bytes, Encoding::Base64), + encode(&error_data_buffer, Encoding::Base64), ) } else { // If there are no execution errors the code will be 0 From db84706674b75442550e614bebaac691e62f73dd Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 8 Sep 2023 13:05:22 +0300 Subject: [PATCH 04/22] fix: json error from tenderdash code passed as an actual code in metadata --- .../handlers/platform/broadcastStateTransitionHandlerFactory.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js b/packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js index 0650168119..15e369f9ce 100644 --- a/packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js +++ b/packages/dapi/lib/grpcServer/handlers/platform/broadcastStateTransitionHandlerFactory.js @@ -41,7 +41,7 @@ function broadcastStateTransitionHandlerFactory(rpcClient, createGrpcErrorFromDr if (jsonRpcError) { if (jsonRpcError.data === 'tx already exists in cache') { - throw new AlreadyExistsGrpcError('State transition already in chain', jsonRpcError); + throw new AlreadyExistsGrpcError('State transition already in chain'); } const error = new Error(); From bb525d7ab75478d4ce4f6f4e9909e4ef5fbe5c11 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 8 Sep 2023 16:01:49 +0100 Subject: [PATCH 05/22] feat(rs-platform-value): to_cbor_buffer on Value --- packages/rs-platform-value/src/converter/ciborium.rs | 8 ++++++++ packages/rs-platform-value/src/error.rs | 3 +++ 2 files changed, 11 insertions(+) diff --git a/packages/rs-platform-value/src/converter/ciborium.rs b/packages/rs-platform-value/src/converter/ciborium.rs index b16136dbe8..8703fed88b 100644 --- a/packages/rs-platform-value/src/converter/ciborium.rs +++ b/packages/rs-platform-value/src/converter/ciborium.rs @@ -23,6 +23,14 @@ impl Value { .map(|(key, value)| Ok((key, value.try_into()?))) .collect() } + + pub fn to_cbor_buffer(&self) -> Result, Error> { + let mut buffer: Vec = Vec::new(); + ciborium::ser::into_writer(self, &mut buffer) + .map_err(|e| Error::CborSerializationError(e.to_string()))?; + + Ok(buffer) + } } impl TryFrom for Value { diff --git a/packages/rs-platform-value/src/error.rs b/packages/rs-platform-value/src/error.rs index 953944a7b8..129456bbc2 100644 --- a/packages/rs-platform-value/src/error.rs +++ b/packages/rs-platform-value/src/error.rs @@ -39,6 +39,9 @@ pub enum Error { #[error("serde deserialization error: {0}")] SerdeDeserializationError(String), + + #[error("cbor serialization error: {0}")] + CborSerializationError(String), } impl serde::ser::Error for Error { From 3bc2f74ffcf3d386775713e18a2fd049dab6c894 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 8 Sep 2023 16:02:17 +0100 Subject: [PATCH 06/22] refactor(drive-abci): response errors --- packages/rs-drive-abci/src/abci/handlers.rs | 70 +++++++++------------ packages/rs-drive-abci/src/error/mod.rs | 8 +++ 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index cc3a291ad4..566609325a 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -37,12 +37,10 @@ use crate::abci::server::AbciApplication; use crate::error::execution::ExecutionError; -use ciborium::cbor; -use crate::error::Error; +use crate::error::{Error, ErrorCodes}; use crate::rpc::core::CoreRPCLike; use dpp::errors::consensus::codes::ErrorWithCode; -use serde_json::{json, Value}; use tenderdash_abci::proto::abci::response_verify_vote_extension::VerifyStatus; use tenderdash_abci::proto::abci::tx_record::TxAction; use tenderdash_abci::proto::abci::{self as proto, ExtendVoteExtension, ResponseException}; @@ -72,9 +70,9 @@ use crate::platform_types::platform_state::PlatformState; use crate::platform_types::withdrawal::withdrawal_txs; use dpp::dashcore::hashes::Hash; use dpp::fee::SignedCredits; +use dpp::platform_value::platform_value; use dpp::serialization::PlatformSerializableWithPlatformVersion; use dpp::version::{PlatformVersion, PlatformVersionCurrentVersion}; -use serde_json::Map; impl<'a, C> tenderdash_abci::Application for AbciApplication<'a, C> where @@ -600,20 +598,13 @@ where .serialize_to_bytes_with_platform_version(platform_version) .map_err(|e| ResponseException::from(Error::Protocol(e)))?; - let error_data = cbor!({ - "data" => { - "serializedError" => consensus_error_bytes + let error_data_buffer = platform_value!({ + "data": { + "serializedError": consensus_error_bytes } }) - .map_err(|err| { - Error::Serialization(SerializationError::CorruptedSerialization(format!( - "can't create cbor: {err}" - ))) - })?; - - let mut error_data_buffer: Vec = Vec::new(); - ciborium::ser::into_writer(&error_data, &mut error_data_buffer) - .map_err(|e| e.to_string())?; + .to_cbor_buffer() + .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; ( consensus_error.code(), @@ -640,23 +631,16 @@ where }) } Err(error) => { - let error_data = cbor!({ - "message" => "Internal error", + let error_data_buffer = platform_value!({ + "message": format!("Internal error {}", error.to_string()), }) - .map_err(|err| { - Error::Serialization(SerializationError::CorruptedSerialization(format!( - "can't create cbor: {err}" - ))) - })?; - - let mut error_data_buffer: Vec = Vec::new(); - ciborium::ser::into_writer(&error_data, &mut error_data_buffer) - .map_err(|e| e.to_string())?; + .to_cbor_buffer() + .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; tracing::error!(method = "check_tx", ?error, "check_tx failed"); Ok(ResponseCheckTx { - code: 13, // Internal error gRPC code + code: ErrorCodes::InternalError as u32, // Internal error gRPC code data: vec![], info: encode(&error_data_buffer, Encoding::Base64), gas_wanted: 0 as SignedCredits, @@ -674,12 +658,16 @@ where let RequestQuery { data, path, .. } = &request; let Some(platform_version) = PlatformVersion::get_maybe_current() else { + let error_data_buffer = platform_value!({ + "message": "Platform not initialized", + }) + .to_cbor_buffer() + .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; + let response = ResponseQuery { - //todo: right now just put GRPC error codes, - // later we will use own error codes - code: 1, + code: ErrorCodes::InternalError as u32, log: "".to_string(), - info: "Platform not initialized".to_string(), + info: encode(&error_data_buffer, Encoding::Base64), index: 0, key: vec![], value: vec![], @@ -707,16 +695,20 @@ where "Unknown Drive error".to_string() }; - let mut error_data = Map::new(); - error_data.insert("message".to_string(), Value::String(error_message)); + let error_data_buffer = platform_value!({ + "message": error_message, + }) + .to_cbor_buffer() + .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; - let mut error_data_buffer: Vec = Vec::new(); - ciborium::ser::into_writer(&error_data, &mut error_data_buffer) - .map_err(|e| e.to_string())?; - // TODO(rs-drive-abci): restore different error codes? + // TODO(error-codes): restore different error codes? // For now return error code 2, because it is recognized by DAPI as UNKNOWN error // and error code 1 corresponds to CANCELED grpc request which is not suitable - (2, vec![], encode(&error_data_buffer, Encoding::Base64)) + ( + ErrorCodes::Unknown as u32, + vec![], + encode(&error_data_buffer, Encoding::Base64), + ) }; let response = ResponseQuery { diff --git a/packages/rs-drive-abci/src/error/mod.rs b/packages/rs-drive-abci/src/error/mod.rs index c189cc22c2..f005f3c9a0 100644 --- a/packages/rs-drive-abci/src/error/mod.rs +++ b/packages/rs-drive-abci/src/error/mod.rs @@ -64,3 +64,11 @@ impl From for ResponseException { } } } + +/// A set of error codes to respond to clients with +pub enum ErrorCodes { + /// Unknown drive error + Unknown = 2, + /// Internal drive error + InternalError = 13, +} From c0a34cbf9d2a7f9e772df2f14a88a7e57e29b5fa Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Wed, 13 Sep 2023 17:13:40 +0100 Subject: [PATCH 07/22] feat: internal error --- .../dapi/lib/externalApis/drive/ErrorCodes.js | 5 ++ .../createGrpcErrorFromDriveResponse.js | 74 +++++++++++-------- packages/rs-drive-abci/src/abci/error.rs | 9 +++ packages/rs-drive-abci/src/abci/handlers.rs | 11 ++- 4 files changed, 67 insertions(+), 32 deletions(-) create mode 100644 packages/dapi/lib/externalApis/drive/ErrorCodes.js diff --git a/packages/dapi/lib/externalApis/drive/ErrorCodes.js b/packages/dapi/lib/externalApis/drive/ErrorCodes.js new file mode 100644 index 0000000000..2df8a00081 --- /dev/null +++ b/packages/dapi/lib/externalApis/drive/ErrorCodes.js @@ -0,0 +1,5 @@ +module.exports = { + INTERNAL: 1, + INVALID_ARGUMENT: 2, + NOT_FOUND: 3, +}; diff --git a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js index 1d190d63bf..0e68a1b0d9 100644 --- a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js +++ b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js @@ -19,6 +19,7 @@ const { default: loadWasmDpp, deserializeConsensusError } = require('@dashevo/wa const GrpcErrorCodes = require('@dashevo/grpc-common/lib/server/error/GrpcErrorCodes'); const AlreadyExistsGrpcError = require('@dashevo/grpc-common/lib/server/error/AlreadyExistsGrpcError'); +const DriveErrorCodes = require('../../externalApis/drive/ErrorCodes'); /** * @param {Object} data @@ -35,13 +36,14 @@ function createRawMetadata(data) { } const COMMON_ERROR_CLASSES = { - [GrpcErrorCodes.INVALID_ARGUMENT]: InvalidArgumentGrpcError, - [GrpcErrorCodes.DEADLINE_EXCEEDED]: DeadlineExceededGrpcError, - [GrpcErrorCodes.NOT_FOUND]: NotFoundGrpcError, - [GrpcErrorCodes.ALREADY_EXISTS]: AlreadyExistsGrpcError, - [GrpcErrorCodes.RESOURCE_EXHAUSTED]: ResourceExhaustedGrpcError, - [GrpcErrorCodes.FAILED_PRECONDITION]: FailedPreconditionGrpcError, - [GrpcErrorCodes.UNAVAILABLE]: UnavailableGrpcError, + // [GrpcErrorCodes.INTERNAL]: InternalGrpcError, + // [GrpcErrorCodes.INVALID_ARGUMENT]: InvalidArgumentGrpcError, + // [GrpcErrorCodes.DEADLINE_EXCEEDED]: DeadlineExceededGrpcError, + // [GrpcErrorCodes.NOT_FOUND]: NotFoundGrpcError, + // [GrpcErrorCodes.ALREADY_EXISTS]: AlreadyExistsGrpcError, + // [GrpcErrorCodes.RESOURCE_EXHAUSTED]: ResourceExhaustedGrpcError, + // [GrpcErrorCodes.FAILED_PRECONDITION]: FailedPreconditionGrpcError, + // [GrpcErrorCodes.UNAVAILABLE]: UnavailableGrpcError, }; /** @@ -64,22 +66,8 @@ async function createGrpcErrorFromDriveResponse(code, info) { const data = decodedInfo.data || {}; // gRPC error codes - if (code <= 16) { - const CommonErrorClass = COMMON_ERROR_CLASSES[code.toString()]; - if (CommonErrorClass) { - return new CommonErrorClass( - message, - createRawMetadata(data), - ); - } - - // TODO(rs-drive-abci): revisit. - // Rust does not provide stack trace in case of an error. - // It is possible however to use Backtrace crate to report stack. - // Decide whether it worth using Backtrace in rs-drive-abci queries - // and remove if not needed - // Restore stack for internal error - if (code === GrpcErrorCodes.INTERNAL) { + if (code <= 3) { + if (code === DriveErrorCodes.INTERNAL) { const error = new Error(message); // in case of verbose internal error @@ -91,16 +79,42 @@ async function createGrpcErrorFromDriveResponse(code, info) { return new InternalGrpcError(error, createRawMetadata(data)); } - - return new GrpcError( - code, - message, - createRawMetadata(data), - ); + // const CommonErrorClass = COMMON_ERROR_CLASSES[code.toString()]; + // if (CommonErrorClass) { + // return new CommonErrorClass( + // message, + // createRawMetadata(data), + // ); + // } + // + // // TODO(rs-drive-abci): revisit. + // // Rust does not provide stack trace in case of an error. + // // It is possible however to use Backtrace crate to report stack. + // // Decide whether it worth using Backtrace in rs-drive-abci queries + // // and remove if not needed + // // Restore stack for internal error + // if (code === GrpcErrorCodes.INTERNAL) { + // const error = new Error(message); + // + // // in case of verbose internal error + // if (data.stack) { + // error.stack = data.stack; + // + // delete data.stack; + // } + // + // return new InternalGrpcError(error, createRawMetadata(data)); + // } + // + // return new GrpcError( + // code, + // message, + // createRawMetadata(data), + // ); } // Undefined Drive and DAPI errors - if (code >= 17 && code < 1000) { + if (code >= 3 && code < 1000) { return new GrpcError( GrpcErrorCodes.UNKNOWN, message, diff --git a/packages/rs-drive-abci/src/abci/error.rs b/packages/rs-drive-abci/src/abci/error.rs index ce2016900c..78d08c82ff 100644 --- a/packages/rs-drive-abci/src/abci/error.rs +++ b/packages/rs-drive-abci/src/abci/error.rs @@ -72,3 +72,12 @@ impl From for String { value.to_string() } } + +pub enum AbciErrorCodes { + /// Unexpected internal drive error + Internal = 1, + /// Invalid query argument + InvalidArgument = 2, + /// Entity not found error + NotFound = 3, +} diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index 566609325a..5882293cb8 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -55,6 +55,7 @@ use super::AbciError; use dpp::platform_value::string_encoding::{encode, Encoding}; +use crate::abci::error::AbciErrorCodes; use crate::error::serialization::SerializationError; use crate::execution::types::block_execution_context::v0::{ BlockExecutionContextV0Getters, BlockExecutionContextV0MutableGetters, @@ -633,6 +634,9 @@ where Err(error) => { let error_data_buffer = platform_value!({ "message": format!("Internal error {}", error.to_string()), + // TODO: consider capturing stack with one of the libs + // and send it to the client + //"stack": "..." }) .to_cbor_buffer() .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; @@ -640,7 +644,7 @@ where tracing::error!(method = "check_tx", ?error, "check_tx failed"); Ok(ResponseCheckTx { - code: ErrorCodes::InternalError as u32, // Internal error gRPC code + code: AbciErrorCodes::Internal as u32, // Internal error gRPC code data: vec![], info: encode(&error_data_buffer, Encoding::Base64), gas_wanted: 0 as SignedCredits, @@ -660,12 +664,15 @@ where let Some(platform_version) = PlatformVersion::get_maybe_current() else { let error_data_buffer = platform_value!({ "message": "Platform not initialized", + // TODO: consider capturing stack with one of the libs + // and send it to the client + //"stack": "..." }) .to_cbor_buffer() .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; let response = ResponseQuery { - code: ErrorCodes::InternalError as u32, + code: AbciErrorCodes::Internal as u32, log: "".to_string(), info: encode(&error_data_buffer, Encoding::Base64), index: 0, From 1c8202c78f58fd61fa4435cc37121088445a6c08 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Wed, 13 Sep 2023 18:41:32 +0100 Subject: [PATCH 08/22] feat: not found error --- .../createGrpcErrorFromDriveResponse.js | 10 ++- .../SDK/Client/Platform/Fetcher/Fetcher.ts | 12 +--- packages/rs-drive-abci/src/abci/error.rs | 2 + packages/rs-drive-abci/src/abci/handlers.rs | 26 +++---- packages/rs-drive-abci/src/error/mod.rs | 8 --- packages/rs-drive-abci/src/error/query.rs | 8 +++ packages/rs-drive-abci/src/query/v0/mod.rs | 70 ++++++++++++++----- 7 files changed, 87 insertions(+), 49 deletions(-) diff --git a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js index 0e68a1b0d9..7fb29d4a39 100644 --- a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js +++ b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js @@ -39,7 +39,7 @@ const COMMON_ERROR_CLASSES = { // [GrpcErrorCodes.INTERNAL]: InternalGrpcError, // [GrpcErrorCodes.INVALID_ARGUMENT]: InvalidArgumentGrpcError, // [GrpcErrorCodes.DEADLINE_EXCEEDED]: DeadlineExceededGrpcError, - // [GrpcErrorCodes.NOT_FOUND]: NotFoundGrpcError, + [DriveErrorCodes.NOT_FOUND]: NotFoundGrpcError, // [GrpcErrorCodes.ALREADY_EXISTS]: AlreadyExistsGrpcError, // [GrpcErrorCodes.RESOURCE_EXHAUSTED]: ResourceExhaustedGrpcError, // [GrpcErrorCodes.FAILED_PRECONDITION]: FailedPreconditionGrpcError, @@ -67,6 +67,14 @@ async function createGrpcErrorFromDriveResponse(code, info) { // gRPC error codes if (code <= 3) { + const CommonErrorClass = COMMON_ERROR_CLASSES[code.toString()]; + if (CommonErrorClass) { + return new CommonErrorClass( + message, + createRawMetadata(data), + ); + } + if (code === DriveErrorCodes.INTERNAL) { const error = new Error(message); diff --git a/packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts b/packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts index dccb83e80b..144543247b 100644 --- a/packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts +++ b/packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts @@ -96,13 +96,7 @@ class Fetcher { public async fetchIdentity(id: Identifier): Promise { // Define query const query = async (): Promise => { - const result = await this.dapiClient.platform - .getIdentity(id); - - // TODO(rs-drive-abci): Remove this when rs-drive-abci returns error instead of empty bytes - if (result.getIdentity().length === 0) { - throw new NotFoundError(`Identity with id "${id}" not found`); - } + const result = this.dapiClient.platform.getIdentity(id); return result; }; @@ -123,10 +117,6 @@ class Fetcher { const result = await this.dapiClient.platform .getDataContract(id); - // TODO(rs-drive-abci): Remove this when rs-drive-abci returns error instead of empty bytes - if (result.getDataContract().length === 0) { - throw new NotFoundError(`DataContract with id "${id}" not found`); - } return result; }; diff --git a/packages/rs-drive-abci/src/abci/error.rs b/packages/rs-drive-abci/src/abci/error.rs index 78d08c82ff..198984920b 100644 --- a/packages/rs-drive-abci/src/abci/error.rs +++ b/packages/rs-drive-abci/src/abci/error.rs @@ -80,4 +80,6 @@ pub enum AbciErrorCodes { InvalidArgument = 2, /// Entity not found error NotFound = 3, + /// Unknown drive error + Unknown = 4, } diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index 5882293cb8..1f023f4f67 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -38,7 +38,7 @@ use crate::abci::server::AbciApplication; use crate::error::execution::ExecutionError; -use crate::error::{Error, ErrorCodes}; +use crate::error::Error; use crate::rpc::core::CoreRPCLike; use dpp::errors::consensus::codes::ErrorWithCode; use tenderdash_abci::proto::abci::response_verify_vote_extension::VerifyStatus; @@ -56,6 +56,7 @@ use super::AbciError; use dpp::platform_value::string_encoding::{encode, Encoding}; use crate::abci::error::AbciErrorCodes; +use crate::error::query::QueryError; use crate::error::serialization::SerializationError; use crate::execution::types::block_execution_context::v0::{ BlockExecutionContextV0Getters, BlockExecutionContextV0MutableGetters, @@ -696,10 +697,18 @@ where } else { let error = result.errors.first(); - let error_message = if let Some(error) = error { - error.to_string() + let (code, error_message) = if let Some(error) = error { + match error { + QueryError::NotFound(message) => { + (AbciErrorCodes::NotFound as u32, message.to_owned()) + } + _ => (2, error.to_string()), + } } else { - "Unknown Drive error".to_string() + ( + AbciErrorCodes::Unknown as u32, + "Unknown Drive error".to_string(), + ) }; let error_data_buffer = platform_value!({ @@ -708,14 +717,7 @@ where .to_cbor_buffer() .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; - // TODO(error-codes): restore different error codes? - // For now return error code 2, because it is recognized by DAPI as UNKNOWN error - // and error code 1 corresponds to CANCELED grpc request which is not suitable - ( - ErrorCodes::Unknown as u32, - vec![], - encode(&error_data_buffer, Encoding::Base64), - ) + (code, vec![], encode(&error_data_buffer, Encoding::Base64)) }; let response = ResponseQuery { diff --git a/packages/rs-drive-abci/src/error/mod.rs b/packages/rs-drive-abci/src/error/mod.rs index f005f3c9a0..c189cc22c2 100644 --- a/packages/rs-drive-abci/src/error/mod.rs +++ b/packages/rs-drive-abci/src/error/mod.rs @@ -64,11 +64,3 @@ impl From for ResponseException { } } } - -/// A set of error codes to respond to clients with -pub enum ErrorCodes { - /// Unknown drive error - Unknown = 2, - /// Internal drive error - InternalError = 13, -} diff --git a/packages/rs-drive-abci/src/error/query.rs b/packages/rs-drive-abci/src/error/query.rs index d8fe2df8fe..eb839fe344 100644 --- a/packages/rs-drive-abci/src/error/query.rs +++ b/packages/rs-drive-abci/src/error/query.rs @@ -32,6 +32,14 @@ pub enum QueryError { /// Decoding error Error #[error("protobuf decoding error: {0}")] ProtobufDecode(#[from] DecodeError), + + /// Invalid argument Error + #[error("invalid argument error: {0}")] + InvalidArgument(String), + + /// Not found Error + #[error("not found error: {0}")] + NotFound(String), } impl From for ResponseException { diff --git a/packages/rs-drive-abci/src/query/v0/mod.rs b/packages/rs-drive-abci/src/query/v0/mod.rs index bbfe9f94a0..edff840f79 100644 --- a/packages/rs-drive-abci/src/query/v0/mod.rs +++ b/packages/rs-drive-abci/src/query/v0/mod.rs @@ -122,6 +122,13 @@ impl Platform { None, &platform_version.drive )); + + if proof.is_empty() { + return Ok(QueryValidationResult::new_with_error(QueryError::NotFound( + format!("identity {} not found", identity_id), + ))); + } + GetIdentityResponse { result: Some(get_identity_response::Result::Proof(Proof { grovedb_proof: proof, @@ -135,16 +142,19 @@ impl Platform { } .encode_to_vec() } else { - let identity = check_validation_result_with_data!(self + let maybe_identity = check_validation_result_with_data!(self .drive .fetch_full_identity(identity_id.into_buffer(), None, platform_version) - .map_err(QueryError::Drive) + .map_err(QueryError::Drive)); + + let identity = check_validation_result_with_data!(maybe_identity + .ok_or_else(|| { + QueryError::NotFound(format!("identity {} not found", identity_id)) + }) .and_then(|identity| identity - .map(|identity| identity - .serialize_consume_to_bytes() - .map_err(QueryError::Protocol)) - .transpose())) - .unwrap_or_default(); + .serialize_consume_to_bytes() + .map_err(QueryError::Protocol))); + GetIdentityResponse { result: Some(get_identity_response::Result::Identity(identity)), metadata: Some(metadata), @@ -396,6 +406,13 @@ impl Platform { None, platform_version )); + + if proof.is_empty() { + return Ok(QueryValidationResult::new_with_error(QueryError::NotFound( + format!("data contract {} not found", contract_id), + ))); + } + GetDataContractResponse { result: Some(get_data_contract_response::Result::Proof(Proof { grovedb_proof: proof, @@ -409,7 +426,7 @@ impl Platform { } .encode_to_vec() } else { - let contract = check_validation_result_with_data!(self + let maybe_data_contract = check_validation_result_with_data!(self .drive .fetch_contract( contract_id.into_buffer(), @@ -418,16 +435,20 @@ impl Platform { None, platform_version ) - .unwrap()) - .map(|contract| { - contract + .unwrap()); + + let data_contract = check_validation_result_with_data!(maybe_data_contract + .ok_or_else(|| { + QueryError::NotFound(format!("data contract {} not found", contract_id)) + }) + .and_then(|data_contract| data_contract .contract .serialize_to_bytes_with_platform_version(platform_version) - }) - .transpose()?; + .map_err(QueryError::Protocol))); + GetDataContractResponse { result: Some(get_data_contract_response::Result::DataContract( - contract.unwrap_or_default(), + data_contract, )), metadata: Some(metadata), } @@ -756,6 +777,13 @@ impl Platform { None, platform_version )); + + if proof.is_empty() { + return Ok(QueryValidationResult::new_with_error(QueryError::NotFound( + format!("identity {} not found", hex::encode(public_key_hash)), + ))); + } + GetIdentityByPublicKeyHashesResponse { metadata: Some(metadata), result: Some(get_identity_by_public_key_hashes_response::Result::Proof( @@ -779,13 +807,21 @@ impl Platform { platform_version )); let serialized_identity = check_validation_result_with_data!(maybe_identity - .map(|identity| identity.serialize_consume_to_bytes()) - .transpose()); + .ok_or_else(|| { + QueryError::NotFound(format!( + "identity {} not found", + hex::encode(public_key_hash) + )) + }) + .and_then(|identity| identity + .serialize_consume_to_bytes() + .map_err(QueryError::Protocol))); + GetIdentityByPublicKeyHashesResponse { metadata: Some(metadata), result: Some( get_identity_by_public_key_hashes_response::Result::Identity( - serialized_identity.unwrap_or_default(), + serialized_identity, ), ), } From ca5e06f6af3b89e17fc7add2a220fcec6bec2394 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Thu, 14 Sep 2023 15:34:20 +0100 Subject: [PATCH 09/22] feat: invalid argument error --- .../createGrpcErrorFromDriveResponse.js | 2 +- packages/rs-drive-abci/src/abci/handlers.rs | 8 +- packages/rs-drive-abci/src/query/v0/mod.rs | 121 ++++++++++++++---- 3 files changed, 101 insertions(+), 30 deletions(-) diff --git a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js index 7fb29d4a39..e412030447 100644 --- a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js +++ b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js @@ -37,7 +37,7 @@ function createRawMetadata(data) { const COMMON_ERROR_CLASSES = { // [GrpcErrorCodes.INTERNAL]: InternalGrpcError, - // [GrpcErrorCodes.INVALID_ARGUMENT]: InvalidArgumentGrpcError, + [DriveErrorCodes.INVALID_ARGUMENT]: InvalidArgumentGrpcError, // [GrpcErrorCodes.DEADLINE_EXCEEDED]: DeadlineExceededGrpcError, [DriveErrorCodes.NOT_FOUND]: NotFoundGrpcError, // [GrpcErrorCodes.ALREADY_EXISTS]: AlreadyExistsGrpcError, diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index 1f023f4f67..66c3b8dc97 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -702,7 +702,13 @@ where QueryError::NotFound(message) => { (AbciErrorCodes::NotFound as u32, message.to_owned()) } - _ => (2, error.to_string()), + QueryError::InvalidArgument(message) => { + (AbciErrorCodes::InvalidArgument as u32, message.to_owned()) + } + QueryError::Query(error) => { + (AbciErrorCodes::InvalidArgument as u32, error.to_string()) + } + _ => (AbciErrorCodes::Unknown as u32, error.to_string()), } } else { ( diff --git a/packages/rs-drive-abci/src/query/v0/mod.rs b/packages/rs-drive-abci/src/query/v0/mod.rs index edff840f79..88aba59a55 100644 --- a/packages/rs-drive-abci/src/query/v0/mod.rs +++ b/packages/rs-drive-abci/src/query/v0/mod.rs @@ -115,7 +115,11 @@ impl Platform { "/identity" => { let GetIdentityRequest { id, prove } = check_validation_result_with_data!(GetIdentityRequest::decode(query_data)); - let identity_id: Identifier = check_validation_result_with_data!(id.try_into()); + let identity_id: Identifier = check_validation_result_with_data!(id + .try_into() + .map_err(|_| QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string() + ))); let response_data = if prove { let proof = check_validation_result_with_data!(self.drive.prove_full_identity( identity_id.into_buffer(), @@ -169,9 +173,15 @@ impl Platform { let identity_ids = check_validation_result_with_data!(ids .into_iter() .map(|identity_id_vec| { - Bytes32::from_vec(identity_id_vec).map(|bytes| bytes.0) + Bytes32::from_vec(identity_id_vec) + .map(|bytes| bytes.0) + .map_err(|_| { + QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string(), + ) + }) }) - .collect::, dpp::platform_value::Error>>()); + .collect::, QueryError>>()); let response_data = if prove { let proof = check_validation_result_with_data!(self.drive.prove_full_identities( @@ -229,7 +239,11 @@ impl Platform { "/identity/balance" => { let GetIdentityRequest { id, prove } = check_validation_result_with_data!(GetIdentityRequest::decode(query_data)); - let identity_id: Identifier = check_validation_result_with_data!(id.try_into()); + let identity_id: Identifier = check_validation_result_with_data!(id + .try_into() + .map_err(|_| QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string() + ))); let response_data = if prove { let proof = check_validation_result_with_data!(self.drive.prove_identity_balance( @@ -266,7 +280,11 @@ impl Platform { "/identity/balanceAndRevision" => { let GetIdentityRequest { id, prove } = check_validation_result_with_data!(GetIdentityRequest::decode(query_data)); - let identity_id: Identifier = check_validation_result_with_data!(id.try_into()); + let identity_id: Identifier = check_validation_result_with_data!(id + .try_into() + .map_err(|_| QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string() + ))); let response_data = if prove { let proof = check_validation_result_with_data!(self .drive @@ -318,8 +336,11 @@ impl Platform { offset, prove, } = check_validation_result_with_data!(GetIdentityKeysRequest::decode(query_data)); - let identity_id: Identifier = - check_validation_result_with_data!(identity_id.try_into()); + let identity_id: Identifier = check_validation_result_with_data!(identity_id + .try_into() + .map_err(|_| QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string() + ))); if let Some(limit) = limit { if limit > u16::MAX as u32 { return Ok(QueryValidationResult::new_with_error(QueryError::Query( @@ -399,7 +420,11 @@ impl Platform { "/dataContract" => { let GetDataContractRequest { id, prove } = check_validation_result_with_data!(GetDataContractRequest::decode(query_data)); - let contract_id: Identifier = check_validation_result_with_data!(id.try_into()); + let contract_id: Identifier = check_validation_result_with_data!(id + .try_into() + .map_err(|_| QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string() + ))); let response_data = if prove { let proof = check_validation_result_with_data!(self.drive.prove_contract( contract_id.into_buffer(), @@ -462,9 +487,15 @@ impl Platform { let contract_ids = check_validation_result_with_data!(ids .into_iter() .map(|contract_id_vec| { - Bytes32::from_vec(contract_id_vec).map(|bytes| bytes.0) + Bytes32::from_vec(contract_id_vec) + .map(|bytes| bytes.0) + .map_err(|_| { + QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string(), + ) + }) }) - .collect::, dpp::platform_value::Error>>()); + .collect::, QueryError>>()); let response_data = if prove { let proof = check_validation_result_with_data!(self.drive.prove_contracts( contract_ids.as_slice(), @@ -539,7 +570,11 @@ impl Platform { } = check_validation_result_with_data!(GetDataContractHistoryRequest::decode( query_data )); - let contract_id: Identifier = check_validation_result_with_data!(id.try_into()); + let contract_id: Identifier = check_validation_result_with_data!(id + .try_into() + .map_err(|_| QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string() + ))); // TODO: make a cast safe let limit = limit @@ -635,8 +670,11 @@ impl Platform { prove, start, } = check_validation_result_with_data!(GetDocumentsRequest::decode(query_data)); - let contract_id: Identifier = - check_validation_result_with_data!(data_contract_id.try_into()); + let contract_id: Identifier = check_validation_result_with_data!(data_contract_id + .try_into() + .map_err(|_| QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string() + ))); let (_, contract) = check_validation_result_with_data!(self .drive .get_contract_with_fetch_info_and_fee( @@ -765,10 +803,12 @@ impl Platform { } = check_validation_result_with_data!( GetIdentityByPublicKeyHashesRequest::decode(query_data) ); - let public_key_hash = check_validation_result_with_data!(Bytes20::from_vec( - public_key_hash - ) - .map(|bytes| bytes.0)); + let public_key_hash = + check_validation_result_with_data!(Bytes20::from_vec(public_key_hash) + .map(|bytes| bytes.0) + .map_err(|_| QueryError::InvalidArgument( + "public key hash must be 20 bytes long".to_string() + ))); let response_data = if prove { let proof = check_validation_result_with_data!(self .drive @@ -839,9 +879,15 @@ impl Platform { let public_key_hashes = check_validation_result_with_data!(public_key_hashes .into_iter() .map(|pub_key_hash_vec| { - Bytes20::from_vec(pub_key_hash_vec).map(|bytes| bytes.0) + Bytes20::from_vec(pub_key_hash_vec) + .map(|bytes| bytes.0) + .map_err(|_| { + QueryError::InvalidArgument( + "public key hash must be 20 bytes long".to_string(), + ) + }) }) - .collect::, dpp::platform_value::Error>>()); + .collect::, QueryError>>()); let response_data = if prove { let proof = check_validation_result_with_data!(self .drive @@ -902,15 +948,26 @@ impl Platform { let contract_ids = check_validation_result_with_data!(contracts .into_iter() .map(|contract_request| { - Bytes32::from_vec(contract_request.contract_id).map(|bytes| bytes.0) + Bytes32::from_vec(contract_request.contract_id) + .map(|bytes| bytes.0) + .map_err(|_| { + QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string(), + ) + }) }) - .collect::, dpp::platform_value::Error>>()); + .collect::, QueryError>>()); let identity_requests = check_validation_result_with_data!(identities .into_iter() .map(|identity_request| { Ok(IdentityDriveQuery { identity_id: Bytes32::from_vec(identity_request.identity_id) - .map(|bytes| bytes.0)?, + .map(|bytes| bytes.0) + .map_err(|_| { + QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string(), + ) + })?, prove_request_type: IdentityProveRequestType::try_from( identity_request.request_type as u8, )?, @@ -921,9 +978,17 @@ impl Platform { .into_iter() .map(|document_proof_request| { let contract_id: Identifier = - document_proof_request.contract_id.try_into()?; + document_proof_request.contract_id.try_into().map_err(|_| { + QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string(), + ) + })?; let document_id: Identifier = - document_proof_request.document_id.try_into()?; + document_proof_request.document_id.try_into().map_err(|_| { + QueryError::InvalidArgument( + "id must be a valid identifier (32 bytes long)".to_string(), + ) + })?; Ok(SingleDocumentDriveQuery { contract_id: contract_id.into_buffer(), @@ -934,7 +999,7 @@ impl Platform { block_time_ms: None, //None because we want latest }) }) - .collect::, dpp::platform_value::Error>>()); + .collect::, QueryError>>()); let proof = check_validation_result_with_data!(self.drive.prove_multiple( &identity_requests, &contract_ids, @@ -956,9 +1021,9 @@ impl Platform { .encode_to_vec(); Ok(QueryValidationResult::new_with_data(response_data)) } - other => Ok(QueryValidationResult::new_with_error(QueryError::Query( - QuerySyntaxError::Unsupported(format!("query path '{}' is not supported", other)), - ))), + other => Ok(QueryValidationResult::new_with_error( + QueryError::InvalidArgument(format!("query path '{}' is not supported", other)), + )), } } } From 5256a210615f7505f97985ac06f037078bd437a7 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Thu, 14 Sep 2023 17:12:43 +0100 Subject: [PATCH 10/22] test(dapi): fix --- .../createGrpcErrorFromDriveResponse.js | 10 ---------- .../externalApis/drive/DriveClient.spec.js | 5 +++-- .../createGrpcErrorFromDriveResponse.spec.js | 20 ++++++++++++------- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js index e412030447..c75d5bd5dc 100644 --- a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js +++ b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js @@ -5,11 +5,8 @@ const { error: { InternalGrpcError, InvalidArgumentGrpcError, - DeadlineExceededGrpcError, - ResourceExhaustedGrpcError, NotFoundGrpcError, FailedPreconditionGrpcError, - UnavailableGrpcError, GrpcError, }, }, @@ -18,7 +15,6 @@ const { const { default: loadWasmDpp, deserializeConsensusError } = require('@dashevo/wasm-dpp'); const GrpcErrorCodes = require('@dashevo/grpc-common/lib/server/error/GrpcErrorCodes'); -const AlreadyExistsGrpcError = require('@dashevo/grpc-common/lib/server/error/AlreadyExistsGrpcError'); const DriveErrorCodes = require('../../externalApis/drive/ErrorCodes'); /** @@ -36,14 +32,8 @@ function createRawMetadata(data) { } const COMMON_ERROR_CLASSES = { - // [GrpcErrorCodes.INTERNAL]: InternalGrpcError, [DriveErrorCodes.INVALID_ARGUMENT]: InvalidArgumentGrpcError, - // [GrpcErrorCodes.DEADLINE_EXCEEDED]: DeadlineExceededGrpcError, [DriveErrorCodes.NOT_FOUND]: NotFoundGrpcError, - // [GrpcErrorCodes.ALREADY_EXISTS]: AlreadyExistsGrpcError, - // [GrpcErrorCodes.RESOURCE_EXHAUSTED]: ResourceExhaustedGrpcError, - // [GrpcErrorCodes.FAILED_PRECONDITION]: FailedPreconditionGrpcError, - // [GrpcErrorCodes.UNAVAILABLE]: UnavailableGrpcError, }; /** diff --git a/packages/dapi/test/unit/externalApis/drive/DriveClient.spec.js b/packages/dapi/test/unit/externalApis/drive/DriveClient.spec.js index 418c9c2b06..d76db87035 100644 --- a/packages/dapi/test/unit/externalApis/drive/DriveClient.spec.js +++ b/packages/dapi/test/unit/externalApis/drive/DriveClient.spec.js @@ -7,7 +7,7 @@ const dirtyChai = require('dirty-chai'); const getIdentityFixture = require('@dashevo/wasm-dpp/lib/test/fixtures/getIdentityFixture'); const InvalidArgumentGrpcError = require('@dashevo/grpc-common/lib/server/error/InvalidArgumentGrpcError'); -const GrpcErrorCodes = require('@dashevo/grpc-common/lib/server/error/GrpcErrorCodes'); + const { v0: { GetIdentitiesByPublicKeyHashesRequest, @@ -26,6 +26,7 @@ const { } = require('@dashevo/dapi-grpc'); const DriveClient = require('../../../../lib/externalApis/drive/DriveClient'); +const DriveErrorCodes = require('../../../../lib/externalApis/drive/ErrorCodes'); const RPCError = require('../../../../lib/rpcServer/RPCError'); @@ -68,7 +69,7 @@ describe('DriveClient', () => { .resolves({ result: { response: { - code: GrpcErrorCodes.INVALID_ARGUMENT, + code: DriveErrorCodes.INVALID_ARGUMENT, info: cbor.encode({ data: { name: 'someData', diff --git a/packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js b/packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js index 88420d1f43..70b8bd0822 100644 --- a/packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js +++ b/packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js @@ -5,13 +5,14 @@ const InternalGrpcError = require('@dashevo/grpc-common/lib/server/error/Interna const InvalidArgumentGrpcError = require('@dashevo/grpc-common/lib/server/error/InvalidArgumentGrpcError'); const FailedPreconditionGrpcError = require('@dashevo/grpc-common/lib/server/error/FailedPreconditionGrpcError'); const generateRandomIdentifierAsync = require('@dashevo/wasm-dpp/lib/test/utils/generateRandomIdentifierAsync'); - const { default: loadWasmDpp, ProtocolVersionParsingError, IdentityNotFoundError, BalanceIsNotEnoughError, DataContractAlreadyPresentError, } = require('@dashevo/wasm-dpp'); +const DriveErrorCodes = require('../../../../lib/externalApis/drive/ErrorCodes'); + const createGrpcErrorFromDriveResponse = require( '../../../../lib/grpcServer/handlers/createGrpcErrorFromDriveResponse', ); @@ -37,18 +38,23 @@ describe('createGrpcErrorFromDriveResponse', () => { encodedInfo = cbor.encode(info).toString('base64'); }); - Object.entries(GrpcErrorCodes) + const COMMON_ERROR_CLASSES = { + [DriveErrorCodes.INVALID_ARGUMENT]: GrpcErrorCodes.INVALID_ARGUMENT, + [DriveErrorCodes.NOT_FOUND]: GrpcErrorCodes.NOT_FOUND, + }; + + Object.entries(DriveErrorCodes) // We have special tests below for these error codes .filter(([, code]) => ( - ![GrpcErrorCodes.VERSION_MISMATCH, GrpcErrorCodes.INTERNAL].includes(code) + ![DriveErrorCodes.INTERNAL].includes(code) )) .forEach(([codeClass, code]) => { - it(`should throw ${codeClass} if response code is ${code}`, async () => { + it(`should throw ${codeClass} if drive error code is ${code}`, async () => { const error = await createGrpcErrorFromDriveResponse(code, encodedInfo); expect(error).to.be.an.instanceOf(GrpcError); expect(error.getMessage()).to.equal(message); - expect(error.getCode()).to.equal(code); + expect(error.getCode()).to.equal(COMMON_ERROR_CLASSES[code]); expect(error.getRawMetadata()).to.deep.equal({ 'drive-error-data-bin': cbor.encode(info.data), }); @@ -155,7 +161,7 @@ describe('createGrpcErrorFromDriveResponse', () => { expect(error.getError().message).to.deep.equal('Drive’s error code is empty'); }); - it('should return InternalGrpcError if code = 13', async () => { + it('should return InternalGrpcError if DriveCode is internal error', async () => { const errorInfo = { message, data: { @@ -165,7 +171,7 @@ describe('createGrpcErrorFromDriveResponse', () => { }; const error = await createGrpcErrorFromDriveResponse( - GrpcErrorCodes.INTERNAL, + DriveErrorCodes.INTERNAL, cbor.encode(errorInfo).toString('base64'), ); From e541dba965ac309e8a7f94f700c5e7dbf8fcebe6 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Thu, 14 Sep 2023 17:16:04 +0100 Subject: [PATCH 11/22] refactor(dapi): remove commented code --- .../createGrpcErrorFromDriveResponse.js | 32 ------------------- 1 file changed, 32 deletions(-) diff --git a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js index c75d5bd5dc..011a9ffe51 100644 --- a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js +++ b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js @@ -77,38 +77,6 @@ async function createGrpcErrorFromDriveResponse(code, info) { return new InternalGrpcError(error, createRawMetadata(data)); } - // const CommonErrorClass = COMMON_ERROR_CLASSES[code.toString()]; - // if (CommonErrorClass) { - // return new CommonErrorClass( - // message, - // createRawMetadata(data), - // ); - // } - // - // // TODO(rs-drive-abci): revisit. - // // Rust does not provide stack trace in case of an error. - // // It is possible however to use Backtrace crate to report stack. - // // Decide whether it worth using Backtrace in rs-drive-abci queries - // // and remove if not needed - // // Restore stack for internal error - // if (code === GrpcErrorCodes.INTERNAL) { - // const error = new Error(message); - // - // // in case of verbose internal error - // if (data.stack) { - // error.stack = data.stack; - // - // delete data.stack; - // } - // - // return new InternalGrpcError(error, createRawMetadata(data)); - // } - // - // return new GrpcError( - // code, - // message, - // createRawMetadata(data), - // ); } // Undefined Drive and DAPI errors From 3180b4f9505d1b5d9347f7b4deabb63ea81a88fb Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 15 Sep 2023 14:08:58 +0100 Subject: [PATCH 12/22] fix comments --- packages/dapi/lib/externalApis/drive/ErrorCodes.js | 1 + .../handlers/createGrpcErrorFromDriveResponse.js | 2 +- .../src/SDK/Client/Platform/Fetcher/Fetcher.ts | 10 ++++------ 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/dapi/lib/externalApis/drive/ErrorCodes.js b/packages/dapi/lib/externalApis/drive/ErrorCodes.js index 2df8a00081..a0a7d5f1a1 100644 --- a/packages/dapi/lib/externalApis/drive/ErrorCodes.js +++ b/packages/dapi/lib/externalApis/drive/ErrorCodes.js @@ -2,4 +2,5 @@ module.exports = { INTERNAL: 1, INVALID_ARGUMENT: 2, NOT_FOUND: 3, + UNKNOWN: 4, }; diff --git a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js index 011a9ffe51..8175b2bac9 100644 --- a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js +++ b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js @@ -55,7 +55,7 @@ async function createGrpcErrorFromDriveResponse(code, info) { const message = decodedInfo.message; const data = decodedInfo.data || {}; - // gRPC error codes + // Drive error codes if (code <= 3) { const CommonErrorClass = COMMON_ERROR_CLASSES[code.toString()]; if (CommonErrorClass) { diff --git a/packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts b/packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts index 144543247b..1ec345812c 100644 --- a/packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts +++ b/packages/js-dash-sdk/src/SDK/Client/Platform/Fetcher/Fetcher.ts @@ -96,8 +96,8 @@ class Fetcher { public async fetchIdentity(id: Identifier): Promise { // Define query const query = async (): Promise => { - const result = this.dapiClient.platform.getIdentity(id); - return result; + const { platform } = this.dapiClient; + return platform.getIdentity(id); }; // Define retry attempts. @@ -114,10 +114,8 @@ class Fetcher { public async fetchDataContract(id: Identifier): Promise { // Define query const query = async (): Promise => { - const result = await this.dapiClient.platform - .getDataContract(id); - - return result; + const { platform } = this.dapiClient; + return platform.getDataContract(id); }; // Define retry attempts. From 3bc1b6c59ade418f186958ed75f77fc66c7adbf3 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 15 Sep 2023 16:45:56 +0100 Subject: [PATCH 13/22] feat(drive): return all error codes --- .../dapi/lib/externalApis/drive/ErrorCodes.js | 6 - .../createGrpcErrorFromDriveResponse.js | 34 ++++- .../externalApis/drive/DriveClient.spec.js | 5 +- .../createGrpcErrorFromDriveResponse.spec.js | 20 +-- packages/rs-drive-abci/src/abci/error.rs | 11 -- packages/rs-drive-abci/src/abci/handlers.rs | 43 +++--- packages/rs-drive-abci/src/error/handlers.rs | 127 ++++++++++++++++++ packages/rs-drive-abci/src/error/mod.rs | 2 + 8 files changed, 180 insertions(+), 68 deletions(-) delete mode 100644 packages/dapi/lib/externalApis/drive/ErrorCodes.js create mode 100644 packages/rs-drive-abci/src/error/handlers.rs diff --git a/packages/dapi/lib/externalApis/drive/ErrorCodes.js b/packages/dapi/lib/externalApis/drive/ErrorCodes.js deleted file mode 100644 index a0a7d5f1a1..0000000000 --- a/packages/dapi/lib/externalApis/drive/ErrorCodes.js +++ /dev/null @@ -1,6 +0,0 @@ -module.exports = { - INTERNAL: 1, - INVALID_ARGUMENT: 2, - NOT_FOUND: 3, - UNKNOWN: 4, -}; diff --git a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js index 8175b2bac9..1d190d63bf 100644 --- a/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js +++ b/packages/dapi/lib/grpcServer/handlers/createGrpcErrorFromDriveResponse.js @@ -5,8 +5,11 @@ const { error: { InternalGrpcError, InvalidArgumentGrpcError, + DeadlineExceededGrpcError, + ResourceExhaustedGrpcError, NotFoundGrpcError, FailedPreconditionGrpcError, + UnavailableGrpcError, GrpcError, }, }, @@ -15,7 +18,7 @@ const { const { default: loadWasmDpp, deserializeConsensusError } = require('@dashevo/wasm-dpp'); const GrpcErrorCodes = require('@dashevo/grpc-common/lib/server/error/GrpcErrorCodes'); -const DriveErrorCodes = require('../../externalApis/drive/ErrorCodes'); +const AlreadyExistsGrpcError = require('@dashevo/grpc-common/lib/server/error/AlreadyExistsGrpcError'); /** * @param {Object} data @@ -32,8 +35,13 @@ function createRawMetadata(data) { } const COMMON_ERROR_CLASSES = { - [DriveErrorCodes.INVALID_ARGUMENT]: InvalidArgumentGrpcError, - [DriveErrorCodes.NOT_FOUND]: NotFoundGrpcError, + [GrpcErrorCodes.INVALID_ARGUMENT]: InvalidArgumentGrpcError, + [GrpcErrorCodes.DEADLINE_EXCEEDED]: DeadlineExceededGrpcError, + [GrpcErrorCodes.NOT_FOUND]: NotFoundGrpcError, + [GrpcErrorCodes.ALREADY_EXISTS]: AlreadyExistsGrpcError, + [GrpcErrorCodes.RESOURCE_EXHAUSTED]: ResourceExhaustedGrpcError, + [GrpcErrorCodes.FAILED_PRECONDITION]: FailedPreconditionGrpcError, + [GrpcErrorCodes.UNAVAILABLE]: UnavailableGrpcError, }; /** @@ -55,8 +63,8 @@ async function createGrpcErrorFromDriveResponse(code, info) { const message = decodedInfo.message; const data = decodedInfo.data || {}; - // Drive error codes - if (code <= 3) { + // gRPC error codes + if (code <= 16) { const CommonErrorClass = COMMON_ERROR_CLASSES[code.toString()]; if (CommonErrorClass) { return new CommonErrorClass( @@ -65,7 +73,13 @@ async function createGrpcErrorFromDriveResponse(code, info) { ); } - if (code === DriveErrorCodes.INTERNAL) { + // TODO(rs-drive-abci): revisit. + // Rust does not provide stack trace in case of an error. + // It is possible however to use Backtrace crate to report stack. + // Decide whether it worth using Backtrace in rs-drive-abci queries + // and remove if not needed + // Restore stack for internal error + if (code === GrpcErrorCodes.INTERNAL) { const error = new Error(message); // in case of verbose internal error @@ -77,10 +91,16 @@ async function createGrpcErrorFromDriveResponse(code, info) { return new InternalGrpcError(error, createRawMetadata(data)); } + + return new GrpcError( + code, + message, + createRawMetadata(data), + ); } // Undefined Drive and DAPI errors - if (code >= 3 && code < 1000) { + if (code >= 17 && code < 1000) { return new GrpcError( GrpcErrorCodes.UNKNOWN, message, diff --git a/packages/dapi/test/unit/externalApis/drive/DriveClient.spec.js b/packages/dapi/test/unit/externalApis/drive/DriveClient.spec.js index d76db87035..418c9c2b06 100644 --- a/packages/dapi/test/unit/externalApis/drive/DriveClient.spec.js +++ b/packages/dapi/test/unit/externalApis/drive/DriveClient.spec.js @@ -7,7 +7,7 @@ const dirtyChai = require('dirty-chai'); const getIdentityFixture = require('@dashevo/wasm-dpp/lib/test/fixtures/getIdentityFixture'); const InvalidArgumentGrpcError = require('@dashevo/grpc-common/lib/server/error/InvalidArgumentGrpcError'); - +const GrpcErrorCodes = require('@dashevo/grpc-common/lib/server/error/GrpcErrorCodes'); const { v0: { GetIdentitiesByPublicKeyHashesRequest, @@ -26,7 +26,6 @@ const { } = require('@dashevo/dapi-grpc'); const DriveClient = require('../../../../lib/externalApis/drive/DriveClient'); -const DriveErrorCodes = require('../../../../lib/externalApis/drive/ErrorCodes'); const RPCError = require('../../../../lib/rpcServer/RPCError'); @@ -69,7 +68,7 @@ describe('DriveClient', () => { .resolves({ result: { response: { - code: DriveErrorCodes.INVALID_ARGUMENT, + code: GrpcErrorCodes.INVALID_ARGUMENT, info: cbor.encode({ data: { name: 'someData', diff --git a/packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js b/packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js index 70b8bd0822..88420d1f43 100644 --- a/packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js +++ b/packages/dapi/test/unit/grpcServer/handlers/createGrpcErrorFromDriveResponse.spec.js @@ -5,14 +5,13 @@ const InternalGrpcError = require('@dashevo/grpc-common/lib/server/error/Interna const InvalidArgumentGrpcError = require('@dashevo/grpc-common/lib/server/error/InvalidArgumentGrpcError'); const FailedPreconditionGrpcError = require('@dashevo/grpc-common/lib/server/error/FailedPreconditionGrpcError'); const generateRandomIdentifierAsync = require('@dashevo/wasm-dpp/lib/test/utils/generateRandomIdentifierAsync'); + const { default: loadWasmDpp, ProtocolVersionParsingError, IdentityNotFoundError, BalanceIsNotEnoughError, DataContractAlreadyPresentError, } = require('@dashevo/wasm-dpp'); -const DriveErrorCodes = require('../../../../lib/externalApis/drive/ErrorCodes'); - const createGrpcErrorFromDriveResponse = require( '../../../../lib/grpcServer/handlers/createGrpcErrorFromDriveResponse', ); @@ -38,23 +37,18 @@ describe('createGrpcErrorFromDriveResponse', () => { encodedInfo = cbor.encode(info).toString('base64'); }); - const COMMON_ERROR_CLASSES = { - [DriveErrorCodes.INVALID_ARGUMENT]: GrpcErrorCodes.INVALID_ARGUMENT, - [DriveErrorCodes.NOT_FOUND]: GrpcErrorCodes.NOT_FOUND, - }; - - Object.entries(DriveErrorCodes) + Object.entries(GrpcErrorCodes) // We have special tests below for these error codes .filter(([, code]) => ( - ![DriveErrorCodes.INTERNAL].includes(code) + ![GrpcErrorCodes.VERSION_MISMATCH, GrpcErrorCodes.INTERNAL].includes(code) )) .forEach(([codeClass, code]) => { - it(`should throw ${codeClass} if drive error code is ${code}`, async () => { + it(`should throw ${codeClass} if response code is ${code}`, async () => { const error = await createGrpcErrorFromDriveResponse(code, encodedInfo); expect(error).to.be.an.instanceOf(GrpcError); expect(error.getMessage()).to.equal(message); - expect(error.getCode()).to.equal(COMMON_ERROR_CLASSES[code]); + expect(error.getCode()).to.equal(code); expect(error.getRawMetadata()).to.deep.equal({ 'drive-error-data-bin': cbor.encode(info.data), }); @@ -161,7 +155,7 @@ describe('createGrpcErrorFromDriveResponse', () => { expect(error.getError().message).to.deep.equal('Drive’s error code is empty'); }); - it('should return InternalGrpcError if DriveCode is internal error', async () => { + it('should return InternalGrpcError if code = 13', async () => { const errorInfo = { message, data: { @@ -171,7 +165,7 @@ describe('createGrpcErrorFromDriveResponse', () => { }; const error = await createGrpcErrorFromDriveResponse( - DriveErrorCodes.INTERNAL, + GrpcErrorCodes.INTERNAL, cbor.encode(errorInfo).toString('base64'), ); diff --git a/packages/rs-drive-abci/src/abci/error.rs b/packages/rs-drive-abci/src/abci/error.rs index 198984920b..ce2016900c 100644 --- a/packages/rs-drive-abci/src/abci/error.rs +++ b/packages/rs-drive-abci/src/abci/error.rs @@ -72,14 +72,3 @@ impl From for String { value.to_string() } } - -pub enum AbciErrorCodes { - /// Unexpected internal drive error - Internal = 1, - /// Invalid query argument - InvalidArgument = 2, - /// Entity not found error - NotFound = 3, - /// Unknown drive error - Unknown = 4, -} diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index 64672963e9..8da459b813 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -55,9 +55,7 @@ use super::AbciError; use dpp::platform_value::string_encoding::{encode, Encoding}; -use crate::abci::error::AbciErrorCodes; -use crate::error::query::QueryError; -use crate::error::serialization::SerializationError; +use crate::error::handlers::{HandlerError, HandlerErrorCode}; use crate::execution::types::block_execution_context::v0::{ BlockExecutionContextV0Getters, BlockExecutionContextV0MutableGetters, BlockExecutionContextV0Setters, @@ -630,7 +628,7 @@ where tracing::error!(?error, "check_tx failed"); Ok(ResponseCheckTx { - code: AbciErrorCodes::Internal as u32, // Internal error gRPC code + code: HandlerErrorCode::Internal as u32, data: vec![], info: encode(&error_data_buffer, Encoding::Base64), gas_wanted: 0 as SignedCredits, @@ -658,7 +656,7 @@ where .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; let response = ResponseQuery { - code: AbciErrorCodes::Internal as u32, + code: HandlerErrorCode::Internal as u32, log: "".to_string(), info: encode(&error_data_buffer, Encoding::Base64), index: 0, @@ -680,35 +678,24 @@ where let (code, data, info) = if result.is_valid() { (0, result.data.unwrap_or_default(), "success".to_string()) } else { - let error = result.errors.first(); - - let (code, error_message) = if let Some(error) = error { - match error { - QueryError::NotFound(message) => { - (AbciErrorCodes::NotFound as u32, message.to_owned()) - } - QueryError::InvalidArgument(message) => { - (AbciErrorCodes::InvalidArgument as u32, message.to_owned()) - } - QueryError::Query(error) => { - (AbciErrorCodes::InvalidArgument as u32, error.to_string()) - } - _ => (AbciErrorCodes::Unknown as u32, error.to_string()), - } - } else { - ( - AbciErrorCodes::Unknown as u32, - "Unknown Drive error".to_string(), - ) - }; + let error = result + .errors + .first() + .expect("validation result should have at least one error"); + + let handler_error = HandlerError::from(error); let error_data_buffer = platform_value!({ - "message": error_message, + "message": handler_error.to_string(), }) .to_cbor_buffer() .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; - (code, vec![], encode(&error_data_buffer, Encoding::Base64)) + ( + handler_error.code() as u32, + vec![], + encode(&error_data_buffer, Encoding::Base64), + ) }; let response = ResponseQuery { diff --git a/packages/rs-drive-abci/src/error/handlers.rs b/packages/rs-drive-abci/src/error/handlers.rs new file mode 100644 index 0000000000..7cbacb0317 --- /dev/null +++ b/packages/rs-drive-abci/src/error/handlers.rs @@ -0,0 +1,127 @@ +use crate::error::query::QueryError; + +/// ABCI handlers errors +#[derive(Debug, thiserror::Error)] +pub enum HandlerError { + /// ABCI Handler error (Cancelled) + #[error("ABCI Handler error (Cancelled): {0}")] + Cancelled(String), + /// ABCI Handler error (Unknown) + #[error("ABCI Handler error (Unknown): {0}")] + Unknown(String), + /// ABCI Handler error (InvalidArgument) + #[error("ABCI Handler error (InvalidArgument): {0}")] + InvalidArgument(String), + /// ABCI Handler error (DeadlineExceeded) + #[error("ABCI Handler error (DeadlineExceeded): {0}")] + DeadlineExceeded(String), + /// ABCI Handler error (NotFound) + #[error("ABCI Handler error (NotFound): {0}")] + NotFound(String), + /// ABCI Handler error (AlreadyExists) + #[error("ABCI Handler error (AlreadyExists): {0}")] + AlreadyExists(String), + /// ABCI Handler error (PermissionDenied) + #[error("ABCI Handler error (PermissionDenied): {0}")] + PermissionDenied(String), + /// ABCI Handler error (ResourceExhausted) + #[error("ABCI Handler error (ResourceExhausted): {0}")] + ResourceExhausted(String), + /// ABCI Handler error (FailedPrecondition) + #[error("ABCI Handler error (FailedPrecondition): {0}")] + FailedPrecondition(String), + /// ABCI Handler error (Aborted) + #[error("ABCI Handler error (Aborted): {0}")] + Aborted(String), + /// ABCI Handler error (OutOfRange) + #[error("ABCI Handler error (OutOfRange): {0}")] + OutOfRange(String), + /// ABCI Handler error (Unimplemented) + #[error("ABCI Handler error (Unimplemented): {0}")] + Unimplemented(String), + /// ABCI Handler error (Internal) + #[error("ABCI Handler error (Internal): {0}")] + Internal(String), + /// ABCI Handler error (Unavailable) + #[error("ABCI Handler error (Unavailable): {0}")] + Unavailable(String), + /// ABCI Handler error (DataLoss) + #[error("ABCI Handler error (DataLoss): {0}")] + DataLoss(String), + /// ABCI Handler error (Unauthenticated) + #[error("ABCI Handler error (Unauthenticated): {0}")] + Unauthenticated(String), +} + +/// Error codes for ABCI handlers +pub enum HandlerErrorCode { + /// ABCI Handler error (Cancelled) + Cancelled = 1, + /// ABCI Handler error (Unknown) + Unknown = 2, + /// ABCI Handler error (InvalidArgument) + InvalidArgument = 3, + /// ABCI Handler error (DeadlineExceeded) + DeadlineExceeded = 4, + /// ABCI Handler error (NotFound) + NotFound = 5, + /// ABCI Handler error (AlreadyExists) + AlreadyExists = 6, + /// ABCI Handler error (PermissionDenied) + PermissionDenied = 7, + /// ABCI Handler error (ResourceExhausted) + ResourceExhausted = 8, + /// ABCI Handler error (FailedPrecondition) + FailedPrecondition = 9, + /// ABCI Handler error (Aborted) + Aborted = 10, + /// ABCI Handler error (OutOfRange) + OutOfRange = 11, + /// ABCI Handler error (Unimplemented) + Unimplemented = 12, + /// ABCI Handler error (Internal) + Internal = 13, + /// ABCI Handler error (Unavailable) + Unavailable = 14, + /// ABCI Handler error (DataLoss) + DataLoss = 15, + /// ABCI Handler error (Unauthenticated) + Unauthenticated = 16, +} + +impl HandlerError { + /// Returns ABCI handler error code + pub fn code(&self) -> HandlerErrorCode { + match self { + HandlerError::Cancelled(_) => HandlerErrorCode::Cancelled, + HandlerError::Unknown(_) => HandlerErrorCode::Unknown, + HandlerError::InvalidArgument(_) => HandlerErrorCode::InvalidArgument, + HandlerError::DeadlineExceeded(_) => HandlerErrorCode::DeadlineExceeded, + HandlerError::NotFound(_) => HandlerErrorCode::NotFound, + HandlerError::AlreadyExists(_) => HandlerErrorCode::AlreadyExists, + HandlerError::PermissionDenied(_) => HandlerErrorCode::PermissionDenied, + HandlerError::ResourceExhausted(_) => HandlerErrorCode::ResourceExhausted, + HandlerError::FailedPrecondition(_) => HandlerErrorCode::FailedPrecondition, + HandlerError::Aborted(_) => HandlerErrorCode::Aborted, + HandlerError::OutOfRange(_) => HandlerErrorCode::OutOfRange, + HandlerError::Unimplemented(_) => HandlerErrorCode::Unimplemented, + HandlerError::Internal(_) => HandlerErrorCode::Internal, + HandlerError::Unavailable(_) => HandlerErrorCode::Unavailable, + HandlerError::DataLoss(_) => HandlerErrorCode::DataLoss, + HandlerError::Unauthenticated(_) => HandlerErrorCode::Unauthenticated, + } + } +} + +impl From<&QueryError> for HandlerError { + fn from(value: &QueryError) -> Self { + match value { + QueryError::NotFound(message) => HandlerError::NotFound(message.to_owned()), + QueryError::InvalidArgument(message) => { + HandlerError::InvalidArgument(message.to_owned()) + } + QueryError::Query(error) => HandlerError::InvalidArgument(error.to_string()), + _ => HandlerError::Unknown(value.to_string()), + } + } +} diff --git a/packages/rs-drive-abci/src/error/mod.rs b/packages/rs-drive-abci/src/error/mod.rs index c189cc22c2..cb3ad1894e 100644 --- a/packages/rs-drive-abci/src/error/mod.rs +++ b/packages/rs-drive-abci/src/error/mod.rs @@ -12,6 +12,8 @@ use tracing::error; /// Execution errors module pub mod execution; +/// ABCI handlers erors module +pub mod handlers; /// Query errors module pub mod query; /// Serialization errors module From 8b7eb876b6b23c268a6e9ff27526b29e28e9e9d4 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 15 Sep 2023 16:48:50 +0100 Subject: [PATCH 14/22] refactor(drive): rename Query error to DocumentQuery --- packages/rs-drive-abci/src/error/handlers.rs | 2 +- packages/rs-drive-abci/src/error/query.rs | 5 +- packages/rs-drive-abci/src/query/v0/mod.rs | 75 +++++++++++--------- 3 files changed, 46 insertions(+), 36 deletions(-) diff --git a/packages/rs-drive-abci/src/error/handlers.rs b/packages/rs-drive-abci/src/error/handlers.rs index 7cbacb0317..3632de5cb5 100644 --- a/packages/rs-drive-abci/src/error/handlers.rs +++ b/packages/rs-drive-abci/src/error/handlers.rs @@ -120,7 +120,7 @@ impl From<&QueryError> for HandlerError { QueryError::InvalidArgument(message) => { HandlerError::InvalidArgument(message.to_owned()) } - QueryError::Query(error) => HandlerError::InvalidArgument(error.to_string()), + QueryError::DocumentQuery(error) => HandlerError::InvalidArgument(error.to_string()), _ => HandlerError::Unknown(value.to_string()), } } diff --git a/packages/rs-drive-abci/src/error/query.rs b/packages/rs-drive-abci/src/error/query.rs index eb839fe344..7f110f7658 100644 --- a/packages/rs-drive-abci/src/error/query.rs +++ b/packages/rs-drive-abci/src/error/query.rs @@ -13,9 +13,10 @@ pub enum QueryError { /// Proof Error #[error("proof error: {0}")] Proof(#[from] ProofError), + /// Syntax Error - #[error("query syntax error: {0}")] - Query(#[from] SyntaxError), + #[error("document query syntax error: {0}")] + DocumentQuery(#[from] SyntaxError), /// Protocol Error #[error("protocol error: {0}")] diff --git a/packages/rs-drive-abci/src/query/v0/mod.rs b/packages/rs-drive-abci/src/query/v0/mod.rs index 53f2965d8e..f2b001cc7b 100644 --- a/packages/rs-drive-abci/src/query/v0/mod.rs +++ b/packages/rs-drive-abci/src/query/v0/mod.rs @@ -71,20 +71,20 @@ fn convert_key_request_type( let purpose_map = search_key.purpose_map.into_iter().map(|(purpose, security_level_map)| { let security_level_map = security_level_map.security_level_map.into_iter().map(|(security_level, key_kind_request_type)| { if security_level > u8::MAX as u32 { - return Err(QueryError::Query(QuerySyntaxError::InvalidKeyParameter("security level out of bounds".to_string()))); + return Err(QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter("security level out of bounds".to_string()))); } - let security_level = SecurityLevel::try_from(security_level as u8).map_err(|_| QueryError::Query(QuerySyntaxError::InvalidKeyParameter(format!("security level {} not recognized", security_level))))?; + let security_level = SecurityLevel::try_from(security_level as u8).map_err(|_| QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter(format!("security level {} not recognized", security_level))))?; - let key_kind_request_type = from_i32_to_key_kind_request_type(key_kind_request_type).ok_or(QueryError::Query(QuerySyntaxError::InvalidKeyParameter(format!("unknown key kind request type {}", key_kind_request_type))))?; + let key_kind_request_type = from_i32_to_key_kind_request_type(key_kind_request_type).ok_or(QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter(format!("unknown key kind request type {}", key_kind_request_type))))?; Ok(( security_level as u8, key_kind_request_type, )) }).collect::, QueryError>>()?; if purpose > u8::MAX as u32 { - return Err(QueryError::Query(QuerySyntaxError::InvalidKeyParameter("purpose out of bounds".to_string()))); + return Err(QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter("purpose out of bounds".to_string()))); } - let purpose = Purpose::try_from(purpose as u8).map_err(|_| QueryError::Query(QuerySyntaxError::InvalidKeyParameter(format!("purpose {} not recognized", purpose))))?; + let purpose = Purpose::try_from(purpose as u8).map_err(|_| QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter(format!("purpose {} not recognized", purpose))))?; Ok((purpose as u8, security_level_map)) }).collect::>, QueryError>>()?; @@ -343,40 +343,44 @@ impl Platform { ))); if let Some(limit) = limit { if limit > u16::MAX as u32 { - return Ok(QueryValidationResult::new_with_error(QueryError::Query( - QuerySyntaxError::InvalidParameter("limit out of bounds".to_string()), - ))); + return Ok(QueryValidationResult::new_with_error( + QueryError::DocumentQuery(QuerySyntaxError::InvalidParameter( + "limit out of bounds".to_string(), + )), + )); } if limit as u16 > self.config.drive.max_query_limit { - return Ok(QueryValidationResult::new_with_error(QueryError::Query( - QuerySyntaxError::InvalidLimit(format!( + return Ok(QueryValidationResult::new_with_error( + QueryError::DocumentQuery(QuerySyntaxError::InvalidLimit(format!( "limit greater than max limit {}", self.config.drive.max_query_limit - )), - ))); + ))), + )); } } if let Some(offset) = offset { if offset > u16::MAX as u32 { - return Ok(QueryValidationResult::new_with_error(QueryError::Query( - QuerySyntaxError::InvalidParameter("limit out of bounds".to_string()), - ))); + return Ok(QueryValidationResult::new_with_error( + QueryError::DocumentQuery(QuerySyntaxError::InvalidParameter( + "limit out of bounds".to_string(), + )), + )); } } let Some(request_type) = request_type else { - return Ok(QueryValidationResult::new_with_error(QueryError::Query( - QuerySyntaxError::InvalidParameter( + return Ok(QueryValidationResult::new_with_error( + QueryError::DocumentQuery(QuerySyntaxError::InvalidParameter( "key request must be defined".to_string(), - ), - ))); + )), + )); }; let Some(request) = request_type.request else { - return Ok(QueryValidationResult::new_with_error(QueryError::Query( - QuerySyntaxError::InvalidParameter( + return Ok(QueryValidationResult::new_with_error( + QueryError::DocumentQuery(QuerySyntaxError::InvalidParameter( "key request must be defined".to_string(), - ), - ))); + )), + )); }; let key_request_type = check_validation_result_with_data!(convert_key_request_type(request)); @@ -685,7 +689,7 @@ impl Platform { platform_version, )); let contract = check_validation_result_with_data!(contract.ok_or( - QueryError::Query(QuerySyntaxError::DataContractNotFound( + QueryError::DocumentQuery(QuerySyntaxError::DataContractNotFound( "contract not found when querying from value with contract info", )) )); @@ -701,7 +705,7 @@ impl Platform { r#where.as_slice() ) .map_err(|_| { - QueryError::Query(QuerySyntaxError::DeserializationError( + QueryError::DocumentQuery(QuerySyntaxError::DeserializationError( "unable to decode 'where' query from cbor".to_string(), )) })) @@ -715,7 +719,7 @@ impl Platform { order_by.as_slice() ) .map_err(|_| { - QueryError::Query(QuerySyntaxError::DeserializationError( + QueryError::DocumentQuery(QuerySyntaxError::DeserializationError( "unable to decode 'order_by' query from cbor".to_string(), )) })) @@ -729,7 +733,7 @@ impl Platform { false, Some(check_validation_result_with_data!(after .try_into() - .map_err(|_| QueryError::Query( + .map_err(|_| QueryError::DocumentQuery( QuerySyntaxError::InvalidStartsWithClause( "start after should be a 32 byte identifier", ) @@ -738,9 +742,11 @@ impl Platform { Start::StartAt(at) => ( true, Some(check_validation_result_with_data!(at.try_into().map_err( - |_| QueryError::Query(QuerySyntaxError::InvalidStartsWithClause( - "start at should be a 32 byte identifier", - )) + |_| QueryError::DocumentQuery( + QuerySyntaxError::InvalidStartsWithClause( + "start at should be a 32 byte identifier", + ) + ) ))), ), } @@ -749,9 +755,12 @@ impl Platform { }; if limit > u16::MAX as u32 { - return Ok(QueryValidationResult::new_with_error(QueryError::Query( - QuerySyntaxError::InvalidLimit(format!("limit {} out of bounds", limit)), - ))); + return Ok(QueryValidationResult::new_with_error( + QueryError::DocumentQuery(QuerySyntaxError::InvalidLimit(format!( + "limit {} out of bounds", + limit + ))), + )); } let drive_query = From 1bdf49469bb3f9c94ca55888888cbe16a4a3d286 Mon Sep 17 00:00:00 2001 From: Ivan Shumkov Date: Fri, 22 Sep 2023 13:47:35 +0300 Subject: [PATCH 15/22] refactor: remove info encoding duplication --- packages/rs-drive-abci/src/abci/handlers.rs | 41 ++++------------ packages/rs-drive-abci/src/error/handlers.rs | 49 +++++++++++++++++++- 2 files changed, 57 insertions(+), 33 deletions(-) diff --git a/packages/rs-drive-abci/src/abci/handlers.rs b/packages/rs-drive-abci/src/abci/handlers.rs index 531b705020..0e7fad59db 100644 --- a/packages/rs-drive-abci/src/abci/handlers.rs +++ b/packages/rs-drive-abci/src/abci/handlers.rs @@ -616,21 +616,14 @@ where }) } Err(error) => { - let error_data_buffer = platform_value!({ - "message": format!("Internal error {}", error.to_string()), - // TODO: consider capturing stack with one of the libs - // and send it to the client - //"stack": "..." - }) - .to_cbor_buffer() - .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; + let handler_error = HandlerError::Internal(error.to_string()); tracing::error!(?error, "check_tx failed"); Ok(ResponseCheckTx { - code: HandlerErrorCode::Internal as u32, + code: handler_error.code(), data: vec![], - info: encode(&error_data_buffer, Encoding::Base64), + info: handler_error.response_info()?, gas_wanted: 0 as SignedCredits, codespace: "".to_string(), sender: "".to_string(), @@ -645,20 +638,15 @@ where let RequestQuery { data, path, .. } = &request; + // TODO: It must be ResponseException let Some(platform_version) = PlatformVersion::get_maybe_current() else { - let error_data_buffer = platform_value!({ - "message": "Platform not initialized", - // TODO: consider capturing stack with one of the libs - // and send it to the client - //"stack": "..." - }) - .to_cbor_buffer() - .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; + let handler_error = + HandlerError::Unavailable("platform is not initialized".to_string()); let response = ResponseQuery { - code: HandlerErrorCode::Internal as u32, + code: handler_error.code(), log: "".to_string(), - info: encode(&error_data_buffer, Encoding::Base64), + info: handler_error.response_info()?, index: 0, key: vec![], value: vec![], @@ -666,6 +654,7 @@ where height: self.platform.state.read().unwrap().height() as i64, codespace: "".to_string(), }; + tracing::error!(?response, "platform version not initialized"); return Ok(response); @@ -685,17 +674,7 @@ where let handler_error = HandlerError::from(error); - let error_data_buffer = platform_value!({ - "message": handler_error.to_string(), - }) - .to_cbor_buffer() - .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; - - ( - handler_error.code() as u32, - vec![], - encode(&error_data_buffer, Encoding::Base64), - ) + (handler_error.code(), vec![], handler_error.response_info()?) }; let response = ResponseQuery { diff --git a/packages/rs-drive-abci/src/error/handlers.rs b/packages/rs-drive-abci/src/error/handlers.rs index 3632de5cb5..e9ea330061 100644 --- a/packages/rs-drive-abci/src/error/handlers.rs +++ b/packages/rs-drive-abci/src/error/handlers.rs @@ -1,4 +1,8 @@ use crate::error::query::QueryError; +use crate::error::Error; +use dpp::platform_value::platform_value; +use dpp::platform_value::string_encoding::{encode, Encoding}; +use tenderdash_abci::proto::abci::ResponseException; /// ABCI handlers errors #[derive(Debug, thiserror::Error)] @@ -54,6 +58,7 @@ pub enum HandlerError { } /// Error codes for ABCI handlers +#[repr(u32)] pub enum HandlerErrorCode { /// ABCI Handler error (Cancelled) Cancelled = 1, @@ -91,8 +96,8 @@ pub enum HandlerErrorCode { impl HandlerError { /// Returns ABCI handler error code - pub fn code(&self) -> HandlerErrorCode { - match self { + pub fn code(&self) -> u32 { + let code = match self { HandlerError::Cancelled(_) => HandlerErrorCode::Cancelled, HandlerError::Unknown(_) => HandlerErrorCode::Unknown, HandlerError::InvalidArgument(_) => HandlerErrorCode::InvalidArgument, @@ -109,8 +114,48 @@ impl HandlerError { HandlerError::Unavailable(_) => HandlerErrorCode::Unavailable, HandlerError::DataLoss(_) => HandlerErrorCode::DataLoss, HandlerError::Unauthenticated(_) => HandlerErrorCode::Unauthenticated, + }; + + code as u32 + } + + /// Returns error message + pub fn message(&self) -> &str { + match self { + HandlerError::Cancelled(message) => message, + HandlerError::Unknown(message) => message, + HandlerError::InvalidArgument(message) => message, + HandlerError::DeadlineExceeded(message) => message, + HandlerError::NotFound(message) => message, + HandlerError::AlreadyExists(message) => message, + HandlerError::PermissionDenied(message) => message, + HandlerError::ResourceExhausted(message) => message, + HandlerError::FailedPrecondition(message) => message, + HandlerError::Aborted(message) => message, + HandlerError::OutOfRange(message) => message, + HandlerError::Unimplemented(message) => message, + HandlerError::Internal(message) => message, + HandlerError::Unavailable(message) => message, + HandlerError::DataLoss(message) => message, + HandlerError::Unauthenticated(message) => message, } } + + // Returns base64-encoded message for info field of ABCI handler responses + pub fn response_info(&self) -> Result { + let error_data_buffer = platform_value!({ + "message": self.message().to_string(), + // TODO: consider capturing stack with one of the libs + // and send it to the client + //"stack": "..." + }) + .to_cbor_buffer() + .map_err(|e| ResponseException::from(Error::Protocol(e.into())))?; + + let error_data_base64 = encode(&error_data_buffer, Encoding::Base64); + + Ok(error_data_base64) + } } impl From<&QueryError> for HandlerError { From ddb0aa942b0a4385b2575188beaaf88ae6188a47 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 22 Sep 2023 12:39:12 +0100 Subject: [PATCH 16/22] fix(drive): get rid of redundant handler error prefix --- packages/rs-drive-abci/src/error/handlers.rs | 32 ++++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/rs-drive-abci/src/error/handlers.rs b/packages/rs-drive-abci/src/error/handlers.rs index 3632de5cb5..3482d0c7e2 100644 --- a/packages/rs-drive-abci/src/error/handlers.rs +++ b/packages/rs-drive-abci/src/error/handlers.rs @@ -4,52 +4,52 @@ use crate::error::query::QueryError; #[derive(Debug, thiserror::Error)] pub enum HandlerError { /// ABCI Handler error (Cancelled) - #[error("ABCI Handler error (Cancelled): {0}")] + #[error("{0}")] Cancelled(String), /// ABCI Handler error (Unknown) - #[error("ABCI Handler error (Unknown): {0}")] + #[error("{0}")] Unknown(String), /// ABCI Handler error (InvalidArgument) - #[error("ABCI Handler error (InvalidArgument): {0}")] + #[error("{0}")] InvalidArgument(String), /// ABCI Handler error (DeadlineExceeded) - #[error("ABCI Handler error (DeadlineExceeded): {0}")] + #[error("{0}")] DeadlineExceeded(String), /// ABCI Handler error (NotFound) - #[error("ABCI Handler error (NotFound): {0}")] + #[error("{0}")] NotFound(String), /// ABCI Handler error (AlreadyExists) - #[error("ABCI Handler error (AlreadyExists): {0}")] + #[error("{0}")] AlreadyExists(String), /// ABCI Handler error (PermissionDenied) - #[error("ABCI Handler error (PermissionDenied): {0}")] + #[error("{0}")] PermissionDenied(String), /// ABCI Handler error (ResourceExhausted) - #[error("ABCI Handler error (ResourceExhausted): {0}")] + #[error("{0}")] ResourceExhausted(String), /// ABCI Handler error (FailedPrecondition) - #[error("ABCI Handler error (FailedPrecondition): {0}")] + #[error("{0}")] FailedPrecondition(String), /// ABCI Handler error (Aborted) - #[error("ABCI Handler error (Aborted): {0}")] + #[error("{0}")] Aborted(String), /// ABCI Handler error (OutOfRange) - #[error("ABCI Handler error (OutOfRange): {0}")] + #[error("{0}")] OutOfRange(String), /// ABCI Handler error (Unimplemented) - #[error("ABCI Handler error (Unimplemented): {0}")] + #[error("{0}")] Unimplemented(String), /// ABCI Handler error (Internal) - #[error("ABCI Handler error (Internal): {0}")] + #[error("{0}")] Internal(String), /// ABCI Handler error (Unavailable) - #[error("ABCI Handler error (Unavailable): {0}")] + #[error("{0}")] Unavailable(String), /// ABCI Handler error (DataLoss) - #[error("ABCI Handler error (DataLoss): {0}")] + #[error("{0}")] DataLoss(String), /// ABCI Handler error (Unauthenticated) - #[error("ABCI Handler error (Unauthenticated): {0}")] + #[error("{0}")] Unauthenticated(String), } From b4e5685a209aa89bf88af29d9c9c363d7be0bf2e Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 22 Sep 2023 12:43:24 +0100 Subject: [PATCH 17/22] Revert "refactor(drive): rename Query error to DocumentQuery" This reverts commit 8b7eb876b6b23c268a6e9ff27526b29e28e9e9d4. --- packages/rs-drive-abci/src/error/handlers.rs | 2 +- packages/rs-drive-abci/src/error/query.rs | 5 +- packages/rs-drive-abci/src/query/v0/mod.rs | 75 +++++++++----------- 3 files changed, 36 insertions(+), 46 deletions(-) diff --git a/packages/rs-drive-abci/src/error/handlers.rs b/packages/rs-drive-abci/src/error/handlers.rs index 0ab69918b7..88d4781774 100644 --- a/packages/rs-drive-abci/src/error/handlers.rs +++ b/packages/rs-drive-abci/src/error/handlers.rs @@ -165,7 +165,7 @@ impl From<&QueryError> for HandlerError { QueryError::InvalidArgument(message) => { HandlerError::InvalidArgument(message.to_owned()) } - QueryError::DocumentQuery(error) => HandlerError::InvalidArgument(error.to_string()), + QueryError::Query(error) => HandlerError::InvalidArgument(error.to_string()), _ => HandlerError::Unknown(value.to_string()), } } diff --git a/packages/rs-drive-abci/src/error/query.rs b/packages/rs-drive-abci/src/error/query.rs index 7f110f7658..eb839fe344 100644 --- a/packages/rs-drive-abci/src/error/query.rs +++ b/packages/rs-drive-abci/src/error/query.rs @@ -13,10 +13,9 @@ pub enum QueryError { /// Proof Error #[error("proof error: {0}")] Proof(#[from] ProofError), - /// Syntax Error - #[error("document query syntax error: {0}")] - DocumentQuery(#[from] SyntaxError), + #[error("query syntax error: {0}")] + Query(#[from] SyntaxError), /// Protocol Error #[error("protocol error: {0}")] diff --git a/packages/rs-drive-abci/src/query/v0/mod.rs b/packages/rs-drive-abci/src/query/v0/mod.rs index f2b001cc7b..53f2965d8e 100644 --- a/packages/rs-drive-abci/src/query/v0/mod.rs +++ b/packages/rs-drive-abci/src/query/v0/mod.rs @@ -71,20 +71,20 @@ fn convert_key_request_type( let purpose_map = search_key.purpose_map.into_iter().map(|(purpose, security_level_map)| { let security_level_map = security_level_map.security_level_map.into_iter().map(|(security_level, key_kind_request_type)| { if security_level > u8::MAX as u32 { - return Err(QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter("security level out of bounds".to_string()))); + return Err(QueryError::Query(QuerySyntaxError::InvalidKeyParameter("security level out of bounds".to_string()))); } - let security_level = SecurityLevel::try_from(security_level as u8).map_err(|_| QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter(format!("security level {} not recognized", security_level))))?; + let security_level = SecurityLevel::try_from(security_level as u8).map_err(|_| QueryError::Query(QuerySyntaxError::InvalidKeyParameter(format!("security level {} not recognized", security_level))))?; - let key_kind_request_type = from_i32_to_key_kind_request_type(key_kind_request_type).ok_or(QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter(format!("unknown key kind request type {}", key_kind_request_type))))?; + let key_kind_request_type = from_i32_to_key_kind_request_type(key_kind_request_type).ok_or(QueryError::Query(QuerySyntaxError::InvalidKeyParameter(format!("unknown key kind request type {}", key_kind_request_type))))?; Ok(( security_level as u8, key_kind_request_type, )) }).collect::, QueryError>>()?; if purpose > u8::MAX as u32 { - return Err(QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter("purpose out of bounds".to_string()))); + return Err(QueryError::Query(QuerySyntaxError::InvalidKeyParameter("purpose out of bounds".to_string()))); } - let purpose = Purpose::try_from(purpose as u8).map_err(|_| QueryError::DocumentQuery(QuerySyntaxError::InvalidKeyParameter(format!("purpose {} not recognized", purpose))))?; + let purpose = Purpose::try_from(purpose as u8).map_err(|_| QueryError::Query(QuerySyntaxError::InvalidKeyParameter(format!("purpose {} not recognized", purpose))))?; Ok((purpose as u8, security_level_map)) }).collect::>, QueryError>>()?; @@ -343,44 +343,40 @@ impl Platform { ))); if let Some(limit) = limit { if limit > u16::MAX as u32 { - return Ok(QueryValidationResult::new_with_error( - QueryError::DocumentQuery(QuerySyntaxError::InvalidParameter( - "limit out of bounds".to_string(), - )), - )); + return Ok(QueryValidationResult::new_with_error(QueryError::Query( + QuerySyntaxError::InvalidParameter("limit out of bounds".to_string()), + ))); } if limit as u16 > self.config.drive.max_query_limit { - return Ok(QueryValidationResult::new_with_error( - QueryError::DocumentQuery(QuerySyntaxError::InvalidLimit(format!( + return Ok(QueryValidationResult::new_with_error(QueryError::Query( + QuerySyntaxError::InvalidLimit(format!( "limit greater than max limit {}", self.config.drive.max_query_limit - ))), - )); + )), + ))); } } if let Some(offset) = offset { if offset > u16::MAX as u32 { - return Ok(QueryValidationResult::new_with_error( - QueryError::DocumentQuery(QuerySyntaxError::InvalidParameter( - "limit out of bounds".to_string(), - )), - )); + return Ok(QueryValidationResult::new_with_error(QueryError::Query( + QuerySyntaxError::InvalidParameter("limit out of bounds".to_string()), + ))); } } let Some(request_type) = request_type else { - return Ok(QueryValidationResult::new_with_error( - QueryError::DocumentQuery(QuerySyntaxError::InvalidParameter( + return Ok(QueryValidationResult::new_with_error(QueryError::Query( + QuerySyntaxError::InvalidParameter( "key request must be defined".to_string(), - )), - )); + ), + ))); }; let Some(request) = request_type.request else { - return Ok(QueryValidationResult::new_with_error( - QueryError::DocumentQuery(QuerySyntaxError::InvalidParameter( + return Ok(QueryValidationResult::new_with_error(QueryError::Query( + QuerySyntaxError::InvalidParameter( "key request must be defined".to_string(), - )), - )); + ), + ))); }; let key_request_type = check_validation_result_with_data!(convert_key_request_type(request)); @@ -689,7 +685,7 @@ impl Platform { platform_version, )); let contract = check_validation_result_with_data!(contract.ok_or( - QueryError::DocumentQuery(QuerySyntaxError::DataContractNotFound( + QueryError::Query(QuerySyntaxError::DataContractNotFound( "contract not found when querying from value with contract info", )) )); @@ -705,7 +701,7 @@ impl Platform { r#where.as_slice() ) .map_err(|_| { - QueryError::DocumentQuery(QuerySyntaxError::DeserializationError( + QueryError::Query(QuerySyntaxError::DeserializationError( "unable to decode 'where' query from cbor".to_string(), )) })) @@ -719,7 +715,7 @@ impl Platform { order_by.as_slice() ) .map_err(|_| { - QueryError::DocumentQuery(QuerySyntaxError::DeserializationError( + QueryError::Query(QuerySyntaxError::DeserializationError( "unable to decode 'order_by' query from cbor".to_string(), )) })) @@ -733,7 +729,7 @@ impl Platform { false, Some(check_validation_result_with_data!(after .try_into() - .map_err(|_| QueryError::DocumentQuery( + .map_err(|_| QueryError::Query( QuerySyntaxError::InvalidStartsWithClause( "start after should be a 32 byte identifier", ) @@ -742,11 +738,9 @@ impl Platform { Start::StartAt(at) => ( true, Some(check_validation_result_with_data!(at.try_into().map_err( - |_| QueryError::DocumentQuery( - QuerySyntaxError::InvalidStartsWithClause( - "start at should be a 32 byte identifier", - ) - ) + |_| QueryError::Query(QuerySyntaxError::InvalidStartsWithClause( + "start at should be a 32 byte identifier", + )) ))), ), } @@ -755,12 +749,9 @@ impl Platform { }; if limit > u16::MAX as u32 { - return Ok(QueryValidationResult::new_with_error( - QueryError::DocumentQuery(QuerySyntaxError::InvalidLimit(format!( - "limit {} out of bounds", - limit - ))), - )); + return Ok(QueryValidationResult::new_with_error(QueryError::Query( + QuerySyntaxError::InvalidLimit(format!("limit {} out of bounds", limit)), + ))); } let drive_query = From 75fac6d9b19239d61e8b19cb9d80b883ff3e008c Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 22 Sep 2023 12:45:14 +0100 Subject: [PATCH 18/22] fix(drive): documentation --- packages/rs-drive-abci/src/error/handlers.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rs-drive-abci/src/error/handlers.rs b/packages/rs-drive-abci/src/error/handlers.rs index 88d4781774..5539ea8310 100644 --- a/packages/rs-drive-abci/src/error/handlers.rs +++ b/packages/rs-drive-abci/src/error/handlers.rs @@ -141,7 +141,7 @@ impl HandlerError { } } - // Returns base64-encoded message for info field of ABCI handler responses + /// Returns base64-encoded message for info field of ABCI handler responses pub fn response_info(&self) -> Result { let error_data_buffer = platform_value!({ "message": self.message().to_string(), From 72bf115e4a3999b89afd0c222fd7905a385528e8 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 22 Sep 2023 12:46:31 +0100 Subject: [PATCH 19/22] fix(drive): get rid of unnecessary proof length check --- packages/rs-drive-abci/src/query/v0/mod.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/packages/rs-drive-abci/src/query/v0/mod.rs b/packages/rs-drive-abci/src/query/v0/mod.rs index 53f2965d8e..f0c7a7927b 100644 --- a/packages/rs-drive-abci/src/query/v0/mod.rs +++ b/packages/rs-drive-abci/src/query/v0/mod.rs @@ -127,12 +127,6 @@ impl Platform { &platform_version.drive )); - if proof.is_empty() { - return Ok(QueryValidationResult::new_with_error(QueryError::NotFound( - format!("identity {} not found", identity_id), - ))); - } - GetIdentityResponse { result: Some(get_identity_response::Result::Proof(Proof { grovedb_proof: proof, @@ -432,12 +426,6 @@ impl Platform { platform_version )); - if proof.is_empty() { - return Ok(QueryValidationResult::new_with_error(QueryError::NotFound( - format!("data contract {} not found", contract_id), - ))); - } - GetDataContractResponse { result: Some(get_data_contract_response::Result::Proof(Proof { grovedb_proof: proof, @@ -822,12 +810,6 @@ impl Platform { platform_version )); - if proof.is_empty() { - return Ok(QueryValidationResult::new_with_error(QueryError::NotFound( - format!("identity {} not found", hex::encode(public_key_hash)), - ))); - } - GetIdentityByPublicKeyHashesResponse { metadata: Some(metadata), result: Some(get_identity_by_public_key_hashes_response::Result::Proof( From 6c5a2ce74129db80fb8551d8723434f1d172ae78 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Fri, 29 Sep 2023 09:25:52 +0100 Subject: [PATCH 20/22] build: cargo.lock update --- Cargo.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f4d89d8bae..c6ceaef5c5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1169,7 +1169,7 @@ dependencies = [ [[package]] name = "dpp" -version = "0.1.0" +version = "0.25.0-dev.30" dependencies = [ "anyhow", "async-trait", @@ -1224,7 +1224,7 @@ dependencies = [ [[package]] name = "drive" -version = "0.24.0-dev.1" +version = "0.25.0-dev.30" dependencies = [ "anyhow", "base64 0.21.4", @@ -1264,7 +1264,7 @@ dependencies = [ [[package]] name = "drive-abci" -version = "0.1.0" +version = "0.25.0-dev.30" dependencies = [ "anyhow", "atty", From 7bd8b4625e2b823ed06522d31744a59ad1a806b1 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Tue, 3 Oct 2023 08:57:50 +0100 Subject: [PATCH 21/22] wip: test rs-platform-value buffer --- packages/rs-platform-value/src/converter/ciborium.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/rs-platform-value/src/converter/ciborium.rs b/packages/rs-platform-value/src/converter/ciborium.rs index 8703fed88b..7c7562d4e7 100644 --- a/packages/rs-platform-value/src/converter/ciborium.rs +++ b/packages/rs-platform-value/src/converter/ciborium.rs @@ -25,11 +25,11 @@ impl Value { } pub fn to_cbor_buffer(&self) -> Result, Error> { - let mut buffer: Vec = Vec::new(); - ciborium::ser::into_writer(self, &mut buffer) + let mut test_buffer: Vec = Vec::new(); + ciborium::ser::into_writer(self, &mut test_buffer) .map_err(|e| Error::CborSerializationError(e.to_string()))?; - Ok(buffer) + Ok(test_buffer) } } From 0d99331c80238103ad85767926c020300bfe95e1 Mon Sep 17 00:00:00 2001 From: "markin.io" Date: Tue, 3 Oct 2023 09:21:19 +0100 Subject: [PATCH 22/22] Revert "wip: test rs-platform-value buffer" This reverts commit 7bd8b4625e2b823ed06522d31744a59ad1a806b1. --- packages/rs-platform-value/src/converter/ciborium.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/rs-platform-value/src/converter/ciborium.rs b/packages/rs-platform-value/src/converter/ciborium.rs index 7c7562d4e7..8703fed88b 100644 --- a/packages/rs-platform-value/src/converter/ciborium.rs +++ b/packages/rs-platform-value/src/converter/ciborium.rs @@ -25,11 +25,11 @@ impl Value { } pub fn to_cbor_buffer(&self) -> Result, Error> { - let mut test_buffer: Vec = Vec::new(); - ciborium::ser::into_writer(self, &mut test_buffer) + let mut buffer: Vec = Vec::new(); + ciborium::ser::into_writer(self, &mut buffer) .map_err(|e| Error::CborSerializationError(e.to_string()))?; - Ok(test_buffer) + Ok(buffer) } }