Skip to content

Commit

Permalink
2/n improve sui-json-rpc error codes and handling (MystenLabs#11833)
Browse files Browse the repository at this point in the history
  • Loading branch information
wlmyng authored May 12, 2023
1 parent 03f3a04 commit b7625b6
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 132 deletions.
209 changes: 105 additions & 104 deletions crates/sui-core/src/authority.rs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion crates/sui-core/src/authority/authority_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1464,7 +1464,7 @@ impl AuthorityStore {
pub fn multi_get_transaction_blocks(
&self,
tx_digests: &[TransactionDigest],
) -> Result<Vec<Option<VerifiedTransaction>>, SuiError> {
) -> SuiResult<Vec<Option<VerifiedTransaction>>> {
Ok(self
.perpetual_tables
.transactions
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-core/src/transaction_input_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,15 +113,15 @@ pub(crate) async fn check_dev_inspect_input(
config: &ProtocolConfig,
kind: &TransactionKind,
gas_object: Object,
) -> Result<(ObjectRef, InputObjects), anyhow::Error> {
) -> SuiResult<(ObjectRef, InputObjects)> {
let gas_object_ref = gas_object.compute_object_reference();
kind.validity_check(config)?;
match kind {
TransactionKind::ProgrammableTransaction(_) => (),
TransactionKind::ChangeEpoch(_)
| TransactionKind::Genesis(_)
| TransactionKind::ConsensusCommitPrologue(_) => {
anyhow::bail!("Transaction kind {} is not supported in dev-inspect", kind)
return Err(UserInputError::Unsupported(format!("Transaction kind {} is not supported in dev-inspect", kind)).into())
}
}
let mut input_objects = kind.input_objects()?;
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-core/src/unit_tests/authority_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4287,7 +4287,7 @@ pub async fn call_dev_inspect(
function: &str,
type_arguments: Vec<TypeTag>,
test_args: Vec<TestCallArg>,
) -> Result<DevInspectResults, anyhow::Error> {
) -> SuiResult<DevInspectResults> {
let mut builder = ProgrammableTransactionBuilder::new();
let mut arguments = Vec::with_capacity(test_args.len());
for a in test_args {
Expand Down
6 changes: 3 additions & 3 deletions crates/sui-json-rpc-types/src/sui_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ use sui_types::base_types::{
};
use sui_types::digests::{ObjectDigest, TransactionEventsDigest};
use sui_types::effects::{TransactionEffects, TransactionEffectsAPI, TransactionEvents};
use sui_types::error::{ExecutionError, SuiError};
use sui_types::error::{ExecutionError, SuiError, SuiResult};
use sui_types::execution_status::ExecutionStatus;
use sui_types::gas::GasCostSummary;
use sui_types::messages_checkpoint::CheckpointSequenceNumber;
Expand Down Expand Up @@ -706,7 +706,7 @@ impl SuiTransactionBlockEvents {
tx_digest: TransactionDigest,
timestamp_ms: Option<u64>,
resolver: &impl GetModule,
) -> Result<Self, SuiError> {
) -> SuiResult<Self> {
Ok(Self {
data: events
.data
Expand Down Expand Up @@ -761,7 +761,7 @@ impl DevInspectResults {
events: TransactionEvents,
return_values: Result<Vec<ExecutionResult>, ExecutionError>,
resolver: &impl GetModule,
) -> Result<Self, anyhow::Error> {
) -> SuiResult<Self> {
let tx_digest = *effects.transaction_digest();
let mut error = None;
let mut results = None;
Expand Down
11 changes: 7 additions & 4 deletions crates/sui-json-rpc/src/indexer_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use crate::api::{
cap_page_limit, validate_limit, IndexerApiServer, JsonRpcMetrics, ReadApiServer,
QUERY_MAX_RESULT_LIMIT,
};
use crate::error::Error;
use crate::with_tracing;
use crate::SuiRpcModule;

Expand Down Expand Up @@ -157,9 +158,10 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
let opts = query.options.unwrap_or_default();

// Retrieve 1 extra item for next cursor
let mut digests =
self.state
.get_transactions(query.filter, cursor, Some(limit + 1), descending)?;
let mut digests = self
.state
.get_transactions(query.filter, cursor, Some(limit + 1), descending)
.map_err(Error::from)?;

// extract next cursor
let has_next_page = digests.len() > limit;
Expand Down Expand Up @@ -206,7 +208,8 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
// Retrieve 1 extra item for next cursor
let mut data = self
.state
.query_events(query, cursor.clone(), limit + 1, descending)?;
.query_events(query, cursor.clone(), limit + 1, descending)
.map_err(Error::from)?;
let has_next_page = data.len() > limit;
data.truncate(limit);
let next_cursor = data.last().map_or(cursor, |e| Some(e.id.clone()));
Expand Down
32 changes: 21 additions & 11 deletions crates/sui-json-rpc/src/read_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,11 @@ impl ReadApiServer for ReadApi {
#[instrument(skip(self))]
async fn get_total_transaction_blocks(&self) -> RpcResult<BigInt<u64>> {
with_tracing!("get_total_transaction_blocks", async move {
Ok(self.state.get_total_transaction_blocks()?.into())
Ok(self
.state
.get_total_transaction_blocks()
.map_err(Error::from)?
.into()) // converts into BigInt<u64>
})
}

Expand All @@ -612,7 +616,8 @@ impl ReadApiServer for ReadApi {
)
})
.await
.map_err(|e| anyhow!(e))??;
.map_err(Error::from)?
.map_err(Error::from)?;
let input_objects = transaction
.data()
.inner()
Expand All @@ -636,7 +641,8 @@ impl ReadApiServer for ReadApi {
)
})
.await
.map_err(|e| anyhow!(e))??,
.map_err(Error::from)?
.map_err(Error::from)?,
);
}

Expand Down Expand Up @@ -767,7 +773,7 @@ impl ReadApiServer for ReadApi {
let state = self.state.clone();
spawn_monitored_task!(async move{
let store = state.load_epoch_store_one_call_per_task();
let effect = state.get_executed_effects(transaction_digest)?;
let effect = state.get_executed_effects(transaction_digest).map_err(Error::from)?;
let events = if let Some(event_digest) = effect.events_digest() {
state
.get_transaction_events(event_digest)
Expand Down Expand Up @@ -837,7 +843,8 @@ impl ReadApiServer for ReadApi {
state.get_checkpoints(cursor.map(|s| *s), limit as u64 + 1, descending_order)
})
.await
.map_err(|e| anyhow!(e))??;
.map_err(Error::from)?
.map_err(Error::from)?;

let has_next_page = data.len() > limit;
data.truncate(limit);
Expand Down Expand Up @@ -981,12 +988,15 @@ fn get_display_object_by_type(
object_type: &StructTag,
// TODO: add query version support
) -> RpcResult<Option<DisplayVersionUpdatedEvent>> {
let mut events = fullnode_api.state.query_events(
EventFilter::MoveEventType(DisplayVersionUpdatedEvent::type_(object_type)),
None,
1,
true,
)?;
let mut events = fullnode_api
.state
.query_events(
EventFilter::MoveEventType(DisplayVersionUpdatedEvent::type_(object_type)),
None,
1,
true,
)
.map_err(Error::from)?;

// If there's any recent version of Display, give it to the client.
// TODO: add support for version query.
Expand Down
3 changes: 2 additions & 1 deletion crates/sui-json-rpc/src/transaction_execution_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ impl WriteApiServer for TransactionExecutionApi {
.state
.dev_inspect_transaction_block(sender_address, tx_kind, gas_price.map(|i| *i))
.instrument(error_span!("dev_inspect_transaction_block"))
.await?)
.await
.map_err(Error::from)?)
}

async fn dry_run_transaction_block(
Expand Down
9 changes: 5 additions & 4 deletions crates/sui-storage/src/indexes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicU64, Ordering};
use std::sync::Arc;

use anyhow::anyhow;
use itertools::Itertools;
use move_core_types::identifier::Identifier;
use move_core_types::language_storage::{ModuleId, StructTag, TypeTag};
Expand Down Expand Up @@ -621,12 +620,12 @@ impl IndexStore {
cursor: Option<TransactionDigest>,
limit: Option<usize>,
reverse: bool,
) -> Result<Vec<TransactionDigest>, anyhow::Error> {
) -> SuiResult<Vec<TransactionDigest>> {
// Lookup TransactionDigest sequence number,
let cursor = if let Some(cursor) = cursor {
Some(
self.get_transaction_seq(&cursor)?
.ok_or_else(|| anyhow!("Transaction [{cursor:?}] not found."))?,
.ok_or(SuiError::TransactionNotFound { digest: cursor })?,
)
} else {
None
Expand All @@ -653,7 +652,9 @@ impl IndexStore {
}
// NOTE: filter via checkpoint sequence number is implemented in
// `get_transactions` of authority.rs.
Some(_) => Err(anyhow!("Unsupported filter: {:?}", filter)),
Some(_) => Err(SuiError::UserInputError {
error: UserInputError::Unsupported(format!("{:?}", filter)),
}),
None => {
let iter = self.tables.transaction_order.iter();

Expand Down
22 changes: 21 additions & 1 deletion crates/sui-types/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
use crate::{
base_types::*,
committee::{Committee, EpochId, StakeUnit},
digests::CheckpointContentsDigest,
messages_checkpoint::CheckpointSequenceNumber,
object::Owner,
};

Expand Down Expand Up @@ -206,11 +208,29 @@ pub enum UserInputError {
#[error("Transaction is denied: {}", error)]
TransactionDenied { error: String },

#[error("Feature is not yet supported: {0}")]
#[error("Feature is not supported: {0}")]
Unsupported(String),

#[error("Query transactions with move function input error: {0}")]
MoveFunctionInputError(String),

#[error("Verified checkpoint not found for sequence number: {0}")]
VerifiedCheckpointNotFound(CheckpointSequenceNumber),

#[error("Verified checkpoint not found for digest: {0}")]
VerifiedCheckpointDigestNotFound(String),

#[error("Latest checkpoint sequence number not found")]
LatestCheckpointSequenceNumberNotFound,

#[error("Checkpoint contents not found for digest: {0}")]
CheckpointContentsNotFound(CheckpointContentsDigest),

#[error("Genesis transaction not found")]
GenesisTransactionNotFound,

#[error("Transaction {0} not found")]
TransactionCursorNotFound(u64),
}

#[derive(
Expand Down

0 comments on commit b7625b6

Please sign in to comment.