Skip to content

Commit

Permalink
[rpc] Simplify error response from execute_transaction_block of trans…
Browse files Browse the repository at this point in the history
…action_execution_api (#12989)

## Description 

Overall simplify execution-side errors on `execute_transaction_block`
RPC method by informing the caller how to resolve in the error message
and reducing the data we put into the `data` field.

1. (modify) QuorumDriverError::InvalidUserSignature,
::ObjectsDoubleUsed, NonRecoverableTransactionError are now considered
client errors
2. (new) ::TimeoutBeforeFinality, as a transient error -32000
4. (modify) For NonRecoverableTransactionError, populate 'data' field
with vector of error strings. Previously, we dump Vec<(SuiError,
StakeUnit, Vec<ConciseAuthorityPublicKeyBytes>)> which just becomes a
huge object. Transient errors are filtered out, unless they are the only
error in the data field, in which case we return a generic error string
and ask the caller to just try again.
6. (new) For ObjectsDoubleUsed error response, populate the `data` field
with a map of transaction -> vec(object_ref)
7. unit testing to verify conversion

## Test Plan 

How did you test the new or updated feature?

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [x] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
All transaction execution errors from execute_transaction_block of
client-fault will now return a -32002 error code. If you encounter this
error code, there is most likely something at fault in your transaction
inputs.
Previously, when executing a transaction fails on the rpc, you would
receive a, "Transaction has non recoverable errors from at least 1/3 of
validators" after failing to execute a transaction. You will now receive
an improved error message, "Transaction execution failed due to issues
with transaction inputs, please review the errors and try again:
{errors}". {errors} is a string list of actionable errors. Upon
remediation, you should be able to successfully retry your transaction.

---------

Co-authored-by: Lu Zhang <8418040+longbowlu@users.noreply.github.com>
  • Loading branch information
2 people authored and damirka committed Aug 23, 2023
1 parent 7625c8e commit 3a4ccc7
Show file tree
Hide file tree
Showing 3 changed files with 334 additions and 27 deletions.
345 changes: 319 additions & 26 deletions crates/sui-json-rpc/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,18 @@
use fastcrypto::error::FastCryptoError;
use hyper::header::InvalidHeaderValue;
use jsonrpsee::core::Error as RpcError;
use jsonrpsee::types::error::CallError;
use jsonrpsee::types::error::{CallError, INTERNAL_ERROR_CODE};
use jsonrpsee::types::ErrorObject;
use std::collections::BTreeMap;
use sui_types::error::{SuiError, SuiObjectResponseError, UserInputError};
use sui_types::quorum_driver_types::{QuorumDriverError, NON_RECOVERABLE_ERROR_MSG};
use sui_types::quorum_driver_types::QuorumDriverError;
use thiserror::Error;
use tokio::task::JoinError;

use crate::authority_state::StateReadError;

pub const TRANSIENT_ERROR_CODE: i32 = -32001;
pub const TRANSACTION_EXECUTION_CLIENT_ERROR_CODE: i32 = -32002;

pub type RpcInterimResult<T = ()> = Result<T, Error>;

Expand Down Expand Up @@ -62,12 +64,6 @@ pub enum Error {
StateReadError(#[from] StateReadError),
}

impl From<Error> for RpcError {
fn from(e: Error) -> Self {
e.to_rpc_error()
}
}

impl From<SuiError> for Error {
fn from(e: SuiError) -> Self {
match e {
Expand All @@ -78,11 +74,10 @@ impl From<SuiError> for Error {
}
}

impl Error {
/// `InvalidParams`/`INVALID_PARAMS_CODE` for client errors.
pub fn to_rpc_error(self) -> RpcError {
match self {
Error::UserInputError(_) => RpcError::Call(CallError::InvalidParams(self.into())),
impl From<Error> for RpcError {
fn from(e: Error) -> RpcError {
match e {
Error::UserInputError(_) => RpcError::Call(CallError::InvalidParams(e.into())),
Error::SuiObjectResponseError(err) => match err {
SuiObjectResponseError::NotExists { .. }
| SuiObjectResponseError::DynamicFieldNotFound { .. }
Expand All @@ -101,31 +96,111 @@ impl Error {
}
_ => RpcError::Call(CallError::Failed(sui_error.into())),
},
Error::StateReadError(err) => match err {
StateReadError::Client(_) => RpcError::Call(CallError::InvalidParams(err.into())),
_ => {
let error_object = ErrorObject::owned(
jsonrpsee::types::error::INTERNAL_ERROR_CODE,
err.to_string(),
None::<()>,
);
RpcError::Call(CallError::Custom(error_object))
}
},
Error::QuorumDriverError(err) => match err {
QuorumDriverError::InvalidUserSignature(err) => {
let inner_error_str = match err {
// TODO(wlmyng): update SuiError display trait to render UserInputError with display
SuiError::UserInputError { error } => error.to_string(),
_ => err.to_string(),
};

let error_message = format!("Invalid user signature: {inner_error_str}");

let error_object = ErrorObject::owned(
TRANSACTION_EXECUTION_CLIENT_ERROR_CODE,
error_message,
None::<()>,
);
RpcError::Call(CallError::Custom(error_object))
}
QuorumDriverError::TimeoutBeforeFinality
| QuorumDriverError::FailedWithTransientErrorAfterMaximumAttempts { .. } => {
let error_object =
ErrorObject::owned(TRANSIENT_ERROR_CODE, err.to_string(), None::<()>);
RpcError::Call(CallError::Custom(error_object))
}
QuorumDriverError::ObjectsDoubleUsed {
conflicting_txes,
retried_tx,
retried_tx_success,
} => {
let error_message = format!(
"Failed to sign transaction by a quorum of validators because of locked objects. Retried a conflicting transaction {:?}, success: {:?}",
retried_tx,
retried_tx_success
);

let new_map = conflicting_txes
.into_iter()
.map(|(digest, (pairs, _))| {
(
digest,
pairs.into_iter().map(|(_, obj_ref)| obj_ref).collect(),
)
})
.collect::<BTreeMap<_, Vec<_>>>();

let error_object = ErrorObject::owned(
TRANSACTION_EXECUTION_CLIENT_ERROR_CODE,
error_message,
Some(new_map),
);
RpcError::Call(CallError::Custom(error_object))
}
QuorumDriverError::NonRecoverableTransactionError { errors } => {
// Note: we probably want a more precise error than `INVALID_PARAMS_CODE`
// but to keep the error code consistent we still use `INVALID_PARAMS_CODE`
let new_errors: Vec<String> = errors
.into_iter()
.filter(|(err, _, _)| !err.is_retryable().0) // consider retryable errors as transient errors
.map(|(err, _, _)| {
match err {
// TODO(wlmyng): update SuiError display trait to render UserInputError with display
SuiError::UserInputError { error } => error.to_string(),
_ => err.to_string(),
}
})
.collect();

assert!(
!new_errors.is_empty(),
"NonRecoverableTransactionError should have at least one non-retryable error"
);

let error_list = new_errors.join(", ");
let error_msg = format!("Transaction execution failed due to issues with transaction inputs, please review the errors and try again: {}.", error_list);

let error_object = ErrorObject::owned(
jsonrpsee::types::error::INVALID_PARAMS_CODE,
NON_RECOVERABLE_ERROR_MSG,
Some(errors),
TRANSACTION_EXECUTION_CLIENT_ERROR_CODE,
error_msg,
None::<()>,
);
RpcError::Call(CallError::Custom(error_object))
}
_ => RpcError::Call(CallError::Failed(err.into())),
},
Error::StateReadError(err) => match err {
StateReadError::Client(_) => RpcError::Call(CallError::InvalidParams(err.into())),
_ => {
QuorumDriverError::QuorumDriverInternalError(_) => {
let error_object = ErrorObject::owned(
jsonrpsee::types::error::INTERNAL_ERROR_CODE,
err.to_string(),
INTERNAL_ERROR_CODE,
"Internal error occurred while executing transaction.",
None::<()>,
);
RpcError::Call(CallError::Custom(error_object))
}
QuorumDriverError::SystemOverload { .. } => {
let error_object =
ErrorObject::owned(TRANSIENT_ERROR_CODE, err.to_string(), None::<()>);
RpcError::Call(CallError::Custom(error_object))
}
},
_ => RpcError::Call(CallError::Failed(self.into())),
_ => RpcError::Call(CallError::Failed(e.into())),
}
}
}
Expand Down Expand Up @@ -168,3 +243,221 @@ pub enum SuiRpcInputError {
#[error(transparent)]
UserInputError(#[from] UserInputError),
}

impl From<SuiRpcInputError> for RpcError {
fn from(e: SuiRpcInputError) -> Self {
RpcError::Call(CallError::InvalidParams(e.into()))
}
}

#[cfg(test)]
mod tests {
use super::*;
use expect_test::expect;
use jsonrpsee::types::ErrorObjectOwned;
use sui_types::base_types::AuthorityName;
use sui_types::base_types::ObjectID;
use sui_types::base_types::ObjectRef;
use sui_types::base_types::SequenceNumber;
use sui_types::committee::StakeUnit;
use sui_types::crypto::AuthorityPublicKey;
use sui_types::crypto::AuthorityPublicKeyBytes;
use sui_types::digests::ObjectDigest;
use sui_types::digests::TransactionDigest;

fn test_object_ref() -> ObjectRef {
(
ObjectID::ZERO,
SequenceNumber::from_u64(0),
ObjectDigest::new([0; 32]),
)
}

mod match_quorum_driver_error_tests {
use super::*;

#[test]
fn test_invalid_user_signature() {
let quorum_driver_error =
QuorumDriverError::InvalidUserSignature(SuiError::InvalidSignature {
error: "Test inner invalid signature".to_string(),
});

let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into();

let error_object: ErrorObjectOwned = rpc_error.into();
let expected_code = expect!["-32002"];
expected_code.assert_eq(&error_object.code().to_string());
let expected_message = expect![
"Invalid user signature: Signature is not valid: Test inner invalid signature"
];
expected_message.assert_eq(error_object.message());
}

#[test]
fn test_timeout_before_finality() {
let quorum_driver_error = QuorumDriverError::TimeoutBeforeFinality;

let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into();

let error_object: ErrorObjectOwned = rpc_error.into();
let expected_code = expect!["-32001"];
expected_code.assert_eq(&error_object.code().to_string());
let expected_message = expect!["Transaction timed out before reaching finality"];
expected_message.assert_eq(error_object.message());
}

#[test]
fn test_failed_with_transient_error_after_maximum_attempts() {
let quorum_driver_error =
QuorumDriverError::FailedWithTransientErrorAfterMaximumAttempts {
total_attempts: 10,
};

let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into();

let error_object: ErrorObjectOwned = rpc_error.into();
let expected_code = expect!["-32001"];
expected_code.assert_eq(&error_object.code().to_string());
let expected_message = expect![
"Transaction failed to reach finality with transient error after 10 attempts."
];
expected_message.assert_eq(error_object.message());
}

#[test]
fn test_objects_double_used() {
use sui_types::crypto::VerifyingKey;
let mut conflicting_txes: BTreeMap<
TransactionDigest,
(Vec<(AuthorityName, ObjectRef)>, StakeUnit),
> = BTreeMap::new();
let tx_digest = TransactionDigest::default();
let object_ref = test_object_ref();
let stake_unit: StakeUnit = 10;
let authority_name = AuthorityPublicKeyBytes([0; AuthorityPublicKey::LENGTH]);
conflicting_txes.insert(tx_digest, (vec![(authority_name, object_ref)], stake_unit));

let quorum_driver_error = QuorumDriverError::ObjectsDoubleUsed {
conflicting_txes,
retried_tx: Some(TransactionDigest::default()),
retried_tx_success: Some(true),
};

let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into();

let error_object: ErrorObjectOwned = rpc_error.into();
let expected_code = expect!["-32002"];
expected_code.assert_eq(&error_object.code().to_string());
let expected_message = expect!["Failed to sign transaction by a quorum of validators because of locked objects. Retried a conflicting transaction Some(TransactionDigest(11111111111111111111111111111111)), success: Some(true)"];
expected_message.assert_eq(error_object.message());
let expected_data = expect![[
r#"{"11111111111111111111111111111111":[["0x0000000000000000000000000000000000000000000000000000000000000000",0,"11111111111111111111111111111111"]]}"#
]];
let actual_data = error_object.data().unwrap().to_string();
expected_data.assert_eq(&actual_data);
}

#[test]
fn test_non_recoverable_transaction_error() {
let quorum_driver_error = QuorumDriverError::NonRecoverableTransactionError {
errors: vec![
(
SuiError::UserInputError {
error: UserInputError::GasBalanceTooLow {
gas_balance: 10,
needed_gas_amount: 100,
},
},
0,
vec![],
),
(
SuiError::UserInputError {
error: UserInputError::ObjectVersionUnavailableForConsumption {
provided_obj_ref: test_object_ref(),
current_version: 10.into(),
},
},
0,
vec![],
),
],
};

let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into();

let error_object: ErrorObjectOwned = rpc_error.into();
let expected_code = expect!["-32002"];
expected_code.assert_eq(&error_object.code().to_string());
let expected_message =
expect!["Transaction execution failed due to issues with transaction inputs, please review the errors and try again: Balance of gas object 10 is lower than the needed amount: 100., Object (0x0000000000000000000000000000000000000000000000000000000000000000, SequenceNumber(0), o#11111111111111111111111111111111) is not available for consumption, its current version: SequenceNumber(10).."];
expected_message.assert_eq(error_object.message());
}

#[test]
#[should_panic(
expected = "NonRecoverableTransactionError should have at least one non-retryable error"
)]
fn test_non_recoverable_transaction_error_with_transient_errors() {
let quorum_driver_error = QuorumDriverError::NonRecoverableTransactionError {
errors: vec![
(
SuiError::UserInputError {
error: UserInputError::ObjectNotFound {
object_id: test_object_ref().0,
version: None,
},
},
0,
vec![],
),
(
SuiError::RpcError("Hello".to_string(), "Testing".to_string()),
0,
vec![],
),
],
};

let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into();

let error_object: ErrorObjectOwned = rpc_error.into();
let expected_code = expect!["-32001"];
expected_code.assert_eq(&error_object.code().to_string());
let expected_message =
expect!["Transaction execution failed due to transient errors, please try again."];
expected_message.assert_eq(error_object.message());
}

#[test]
fn test_quorum_driver_internal_error() {
let quorum_driver_error =
QuorumDriverError::QuorumDriverInternalError(SuiError::UnexpectedMessage);

let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into();

let error_object: ErrorObjectOwned = rpc_error.into();
let expected_code = expect!["-32603"];
expected_code.assert_eq(&error_object.code().to_string());
let expected_message = expect!["Internal error occurred while executing transaction."];
expected_message.assert_eq(error_object.message());
}

#[test]
fn test_system_overload() {
let quorum_driver_error = QuorumDriverError::SystemOverload {
overloaded_stake: 10,
errors: vec![(SuiError::UnexpectedMessage, 0, vec![])],
};

let rpc_error: RpcError = Error::QuorumDriverError(quorum_driver_error).into();

let error_object: ErrorObjectOwned = rpc_error.into();
let expected_code = expect!["-32001"];
expected_code.assert_eq(&error_object.code().to_string());
let expected_message = expect!["Transaction is not processed because 10 of validators by stake are overloaded with certificates pending execution."];
expected_message.assert_eq(error_object.message());
}
}
}
Loading

0 comments on commit 3a4ccc7

Please sign in to comment.