Skip to content

Commit

Permalink
3/n improve sui-json-rpc error codes and handling (#11928)
Browse files Browse the repository at this point in the history 
## Description 

Replace anyhow errors with Error enum on sui-json-rpc where possible

## 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)

- [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
[API behavioral changes] - rpc methods that result in errors of variant
`UserInputError`, `SuiRpcInputError`, `SuiError::TransactionNotFound` or
`SuiError::TransactionsNotFound` now return error code `-32602` instead
of `-32000`.
  • Loading branch information
@wlmyng
wlmyng authored May 16, 2023
1 parent 1876d0b commit c3d9cc8
Show file tree
Hide file tree
Showing 9 changed files with 283 additions and 186 deletions.
5 changes: 5 additions & 0 deletions .changeset/early-dancers-walk.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@mysten/sui.js": patch
---

Update ts-sdk e2e test to reflect new rpc error language
17 changes: 8 additions & 9 deletions crates/sui-json-rpc/src/coin_api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

use std::sync::Arc;

use anyhow::anyhow;

use async_trait::async_trait;
use cached::proc_macro::cached;
use cached::SizedCache;
Expand All @@ -28,7 +26,7 @@ use sui_types::object::Object;
use sui_types::parse_sui_struct_tag;

use crate::api::{cap_page_limit, CoinReadApiServer, JsonRpcMetrics};
use crate::error::Error;
use crate::error::{Error, SuiRpcInputError};
use crate::{with_tracing, SuiRpcModule};

pub struct CoinReadApi {
Expand All @@ -47,7 +45,7 @@ impl CoinReadApi {
cursor: (String, ObjectID),
limit: Option<usize>,
one_coin_type_only: bool,
) -> anyhow::Result<CoinPage> {
) -> Result<CoinPage, Error> {
let limit = cap_page_limit(limit);
self.metrics.get_coins_limit.report(limit as u64);
let state = self.state.clone();
Expand Down Expand Up @@ -198,15 +196,16 @@ impl CoinReadApiServer for CoinReadApi {
Some(obj) => {
let coin_type = obj.coin_type_maybe();
if coin_type.is_none() {
Err(anyhow!(
"Invalid Cursor {:?}, Object is not a coin",
object_id
))
Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(
format!("Invalid Cursor {:?}, Object is not a coin", object_id),
)))
} else {
Ok((coin_type.unwrap().to_string(), object_id))
}
}
None => Err(anyhow!("Invalid Cursor {:?}, Object not found", object_id)),
None => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(
format!("Invalid Cursor {:?}, Object not found", object_id),
))),
}
}
None => {
Expand Down
30 changes: 30 additions & 0 deletions crates/sui-json-rpc/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ pub enum Error {

#[error(transparent)]
SuiObjectResponseError(#[from] SuiObjectResponseError),

#[error(transparent)]
SuiRpcInputError(#[from] SuiRpcInputError),
}

impl From<Error> for RpcError {
Expand All @@ -62,6 +65,9 @@ impl Error {
Error::UserInputError(user_input_error) => {
RpcError::Call(CallError::InvalidParams(user_input_error.into()))
}
Error::SuiRpcInputError(sui_json_rpc_input_error) => {
RpcError::Call(CallError::InvalidParams(sui_json_rpc_input_error.into()))
}
Error::SuiError(sui_error) => match sui_error {
SuiError::TransactionNotFound { .. } | SuiError::TransactionsNotFound { .. } => {
RpcError::Call(CallError::InvalidParams(sui_error.into()))
Expand All @@ -83,3 +89,27 @@ impl Error {
}
}
}

#[derive(Debug, Error)]
pub enum SuiRpcInputError {
#[error("Input contains duplicates")]
ContainsDuplicates,

#[error("Input exceeds limit of {0}")]
SizeLimitExceeded(String),

#[error("{0}")]
GenericNotFound(String),

#[error("{0}")]
GenericInvalid(String),

#[error("request_type` must set to `None` or `WaitForLocalExecution` if effects is required in the response")]
InvalidExecuteTransactionRequestType,

#[error("Unsupported protocol version requested. Min supported: {0}, max supported: {1}")]
ProtocolVersionUnsupported(u64, u64),

#[error("Unable to serialize: {0}")]
CannotSerialize(#[from] bcs::Error),
}
11 changes: 6 additions & 5 deletions crates/sui-json-rpc/src/governance_api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use std::cmp::max;
use std::collections::BTreeMap;
use std::sync::Arc;

use anyhow::anyhow;
use async_trait::async_trait;
use cached::proc_macro::cached;
use cached::SizedCache;
Expand All @@ -32,7 +31,7 @@ use sui_types::sui_system_state::SuiSystemStateTrait;
use sui_types::sui_system_state::{get_validator_from_table, SuiSystemState};

use crate::api::{GovernanceReadApiServer, JsonRpcMetrics};
use crate::error::Error;
use crate::error::{Error, SuiRpcInputError};
use crate::{with_tracing, ObjectProvider, SuiRpcModule};

#[derive(Clone)]
Expand Down Expand Up @@ -157,9 +156,11 @@ impl GovernanceReadApi {
let mut delegated_stakes = vec![];
for (pool_id, stakes) in pools {
// Rate table and rate can be null when the pool is not active
let rate_table = rates
.get(&pool_id)
.ok_or_else(|| anyhow!("Cannot find rates for staking pool {pool_id}"))?;
let rate_table = rates.get(&pool_id).ok_or_else(|| {
SuiRpcInputError::GenericNotFound(
"Cannot find rates for staking pool {pool_id}".to_string(),
)
})?;
let current_rate = rate_table.rates.first().map(|(_, rate)| rate);

let mut delegations = vec![];
Expand Down
108 changes: 70 additions & 38 deletions crates/sui-json-rpc/src/indexer_api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

use std::sync::Arc;

use anyhow::{anyhow, Context};
use async_trait::async_trait;
use futures::Stream;
use jsonrpsee::core::error::SubscriptionClosed;
Expand Down Expand Up @@ -36,7 +35,7 @@ use crate::api::{
cap_page_limit, validate_limit, IndexerApiServer, JsonRpcMetrics, ReadApiServer,
QUERY_MAX_RESULT_LIMIT,
};
use crate::error::Error;
use crate::error::{Error, SuiRpcInputError};
use crate::with_tracing;
use crate::SuiRpcModule;

Expand Down Expand Up @@ -105,7 +104,7 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
let mut objects = self
.state
.get_owner_objects(address, cursor, limit + 1, filter)
.map_err(|e| anyhow!("{e}"))?;
.map_err(Error::from)?;

// objects here are of size (limit + 1), where the last one is the cursor for the next page
let has_next_page = objects.len() > limit;
Expand Down Expand Up @@ -264,7 +263,7 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
let mut data = self
.state
.get_dynamic_fields(parent_object_id, cursor, limit + 1)
.map_err(|e| anyhow!("{e}"))?;
.map_err(Error::from)?;
let has_next_page = data.len() > limit;
data.truncate(limit);
let next_cursor = data.last().cloned().map_or(cursor, |c| Some(c.0));
Expand Down Expand Up @@ -299,7 +298,7 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
let id = self
.state
.get_dynamic_field_object_id(parent_object_id, name_type, &name_bcs_value)
.map_err(|e| anyhow!("{e}"))?;
.map_err(Error::from)?;
// TODO(chris): add options to `get_dynamic_field_object` API as well
if let Some(id) = id {
self.read_api
Expand All @@ -326,7 +325,12 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
name: STD_UTF8_STRUCT_NAME.to_owned(),
type_params: vec![],
}));
let name_bcs_value = bcs::to_bytes(&name).context("Unable to serialize name")?;
let name_bcs_value = bcs::to_bytes(&name).map_err(|e| {
Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(format!(
"Unable to serialize name: {:?} with error: {:?}",
name, e
)))
})?;
// record of the input `name`
let record_object_id_option = self
.state
Expand All @@ -336,10 +340,10 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
&name_bcs_value,
)
.map_err(|e| {
anyhow!(
Error::UnexpectedError(format!(
"Read name service dynamic field table failed with error: {:?}",
e
)
))
})?;
if let Some(record_object_id) = record_object_id_option {
let record_object_read =
Expand All @@ -348,33 +352,38 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
"Failed to get object read of name: {:?} with error: {:?}",
record_object_id, e
);
anyhow!("{e}")
Error::from(e)
})?;
let record_parsed_move_object =
SuiParsedMoveObject::try_from_object_read(record_object_read)?;
// NOTE: "value" is the field name to get the address info
let address_info_move_value = record_parsed_move_object
.read_dynamic_field_value(NAME_SERVICE_VALUE)
.ok_or_else(|| anyhow!("Cannot find value field in record Move struct"))?;
.ok_or_else(|| {
Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(
"Cannot find value field in record Move struct".to_string(),
))
})?;
let address_info_move_struct = match address_info_move_value {
SuiMoveValue::Struct(a) => Ok(a),
_ => Err(anyhow!("value field is not found.")),
_ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(
"value field is not found.".to_string(),
))),
}?;
// NOTE: "marker" is the field name to get the address
let address_str_move_value = address_info_move_struct
.read_dynamic_field_value(NAME_SERVICE_MARKER)
.ok_or_else(|| {
anyhow!(
Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(format!(
"Cannot find marker field in address info Move struct: {:?}",
address_info_move_struct
)
)))
})?;
let addr = match address_str_move_value {
SuiMoveValue::Address(addr) => Ok(addr),
_ => Err(anyhow!(
"No SuiAddress found in: {:?}",
address_str_move_value
)),
_ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(
format!("No SuiAddress found in: {:?}", address_str_move_value),
))),
}?;
return Ok(Some(addr));
}
Expand All @@ -394,7 +403,12 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
.get_name_service_dynamic_field_table_object_id(/* reverse_lookup */ true)
.await?;
let name_type_tag = TypeTag::Address;
let name_bcs_value = bcs::to_bytes(&address).context("Unable to serialize address")?;
let name_bcs_value = bcs::to_bytes(&address).map_err(|e| {
Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(format!(
"Unable to serialize address: {:?} with error: {:?}",
address, e
)))
})?;
let addr_object_id = self
.state
.get_dynamic_field_object_id(
Expand All @@ -403,30 +417,41 @@ impl<R: ReadApiServer> IndexerApiServer for IndexerApi<R> {
&name_bcs_value,
)
.map_err(|e| {
anyhow!(
Error::UnexpectedError(format!(
"Read name service reverse dynamic field table failed with error: {:?}",
e
)
))
})?
.ok_or_else(|| anyhow!("Record not found for address: {:?}", address))?;
.ok_or_else(|| {
Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(format!(
"Record not found for address: {:?}",
address
)))
})?;
let addr_object_read = self.state.get_object_read(&addr_object_id).map_err(|e| {
warn!(
"Failed to get object read of address {:?} with error: {:?}",
addr_object_id, e
);
anyhow!("{e}")
Error::from(e)
})?;
let addr_parsed_move_object =
SuiParsedMoveObject::try_from_object_read(addr_object_read)?;
let address_info_move_value = addr_parsed_move_object
.read_dynamic_field_value(NAME_SERVICE_VALUE)
.ok_or_else(|| anyhow!("Cannot find value field in record Move struct"))?;
.ok_or_else(|| {
Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(
"Cannot find value field in record Move struct".to_string(),
))
})?;
let primary_name = match address_info_move_value {
SuiMoveValue::String(s) => Ok(s),
_ => Err(anyhow!(
"No string field for primary name is found in {:?}",
address_info_move_value
)),
_ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(
format!(
"No string field for primary name is found in {:?}",
address_info_move_value
),
))),
}?;
Ok(Page {
data: vec![primary_name],
Expand All @@ -451,14 +476,14 @@ impl<R: ReadApiServer> IndexerApi<R> {
async fn get_name_service_dynamic_field_table_object_id(
&self,
reverse_lookup: bool,
) -> RpcResult<ObjectID> {
) -> Result<ObjectID, Error> {
if let Some(resolver_id) = self.ns_resolver_id {
let resolver_object_read = self.state.get_object_read(&resolver_id).map_err(|e| {
warn!(
"Failed to get object read of resolver {:?} with error: {:?}",
resolver_id, e
);
anyhow!("{e}")
Error::from(e)
})?;

let resolved_parsed_move_object =
Expand All @@ -472,30 +497,37 @@ impl<R: ReadApiServer> IndexerApi<R> {
};
let records_value = resolved_parsed_move_object
.read_dynamic_field_value(dynamic_field_table_key)
.ok_or_else(|| anyhow!("Cannot find records field in resolved object"))?;
.ok_or_else(|| {
Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(
"Cannot find records field in resolved object".to_string(),
))
})?;
let records_move_struct = match records_value {
SuiMoveValue::Struct(s) => Ok(s),
_ => Err(anyhow!(
"{} field is not a Move struct",
dynamic_field_table_key
)),
_ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(
format!("{} field is not a Move struct", dynamic_field_table_key),
))),
}?;

let dynamic_field_table_object_id_struct = records_move_struct
.read_dynamic_field_value(NAME_SERVICE_ID)
.ok_or_else(|| {
anyhow!(
Error::SuiRpcInputError(SuiRpcInputError::GenericNotFound(format!(
"Cannot find id field in {} Move struct",
dynamic_field_table_key
)
)))
})?;
let dynamic_field_table_object_id = match dynamic_field_table_object_id_struct {
SuiMoveValue::UID { id } => Ok(id),
_ => Err(anyhow!("id field is not a UID")),
_ => Err(Error::SuiRpcInputError(SuiRpcInputError::GenericInvalid(
"id field is not a UID".to_string(),
))),
}?;
Ok(dynamic_field_table_object_id)
} else {
Err(anyhow!("Name service resolver is not set"))?
Err(Error::UnexpectedError(
"Name service resolver is not set".to_string(),
))?
}
}
}
Loading

6 comments on commit c3d9cc8

@vercel
Copy link

@vercel vercel bot commented on c3d9cc8 May 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

offline-signer-helper – ./dapps/offline-signer-helper

offline-signer-helper.vercel.app
offline-signer-helper-git-main-mysten-labs.vercel.app
offline-signer-helper-mysten-labs.vercel.app

@vercel
Copy link

@vercel vercel bot commented on c3d9cc8 May 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

explorer – ./apps/explorer

explorer-git-main-mysten-labs.vercel.app
explorer-mysten-labs.vercel.app
explorer-topaz.vercel.app
suiexplorer.com
explorer.sui.io
www.explorer.sui.io
www.suiexplorer.com

@vercel
Copy link

@vercel vercel bot commented on c3d9cc8 May 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

explorer-storybook – ./apps/explorer

explorer-storybook.vercel.app
explorer-storybook-git-main-mysten-labs.vercel.app
explorer-storybook-mysten-labs.vercel.app

@vercel
Copy link

@vercel vercel bot commented on c3d9cc8 May 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

wallet-adapter – ./sdk/wallet-adapter/example

wallet-adapter-mysten-labs.vercel.app
sui-wallet-adapter.vercel.app
wallet-adapter-git-main-mysten-labs.vercel.app

@vercel
Copy link

@vercel vercel bot commented on c3d9cc8 May 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

sui-wallet-kit – ./sdk/wallet-adapter/site

sui-wallet-kit-git-main-mysten-labs.vercel.app
sui-wallet-kit.vercel.app
sui-wallet-kit-mysten-labs.vercel.app

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 Validators 500/s Owned Transactions Benchmark Results

Benchmark Report:
+-------------+------+------+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps  | cps  | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=========================================================================================================================================+
| 60          | 1000 | 1000 | 0      | 15            | 25            | 38            | 803,153,952,000       | 48,189,237,120,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 40  | 49  |

4 Validators 500/s Shared Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 994 | 994 | 0      | 14            | 264           | 445           | 960,387,830,400       | 57,623,269,824,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 44  | 56  |

20 Validators 50/s Owned Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 200 | 200 | 0      | 27            | 76            | 100           | 160,751,616,000       | 9,645,096,960,000          |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 50  | 65  |

20 Validators 50/s Shared Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 197 | 197 | 0      | 45            | 567           | 841           | 190,234,663,200       | 11,414,079,792,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 52  | 75  |

Narwhal Benchmark Results

 SUMMARY:
-----------------------------------------
 + CONFIG:
 Faults: 0 node(s)
 Committee size: 4 node(s)
 Worker(s) per node: 1 worker(s)
 Collocate primary and workers: True
 Input rate: 50,000 tx/s
 Transaction size: 512 B
 Execution time: 0 s

 Header number of batches threshold: 32 digests
 Header maximum number of batches: 1,000 digests
 Max header delay: 2,000 ms
 GC depth: 50 round(s)
 Sync retry delay: 10,000 ms
 Sync retry nodes: 3 node(s)
 batch size: 500,000 B
 Max batch delay: 200 ms
 Max concurrent requests: 500,000 

 + RESULTS:
 Batch creation avg latency: 202 ms
 Header creation avg latency: -1 ms
 	Batch to header avg latency: -1 ms
 Header to certificate avg latency: 2 ms
 	Request vote outbound avg latency: 0 ms
 Certificate commit avg latency: 848 ms

 Consensus TPS: 0 tx/s
 Consensus BPS: 0 B/s
 Consensus latency: 0 ms

 End-to-end TPS: 0 tx/s
 End-to-end BPS: 0 B/s
 End-to-end latency: 0 ms
-----------------------------------------

Please sign in to comment.