Skip to content

Commit

Permalink
[CLI] Add clever error support to Sui CLI (#17920)
Browse files Browse the repository at this point in the history
## Description 

Enables clever error rendering in the Sui CLI.

It also abstracts out some of the rendering logic for constants into
their own file so it can be reused across the graphql and Sui CLI
renderer.

This also updates the Move disassembler to use this new (nicer!)
renderer to that we support rendered values for constants in the Move
disassembler instead of just strings in constants.

## Test plan 

Added new tests for this to the Sui CLI, and updated/added additional
tests for the disassembler.

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [X] CLI: Add better rendering for Move aborts, and added support for
rendering clever errors.
- [ ] Rust SDK:
  • Loading branch information
tzakian authored Jun 19, 2024
1 parent 540c268 commit aa1ad98
Show file tree
Hide file tree
Showing 18 changed files with 657 additions and 80 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

36 changes: 6 additions & 30 deletions crates/sui-package-resolver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ use move_binary_format::file_format::{
AbilitySet, DatatypeTyParameter, EnumDefinitionIndex, FunctionDefinitionIndex,
Signature as MoveSignature, SignatureIndex, Visibility,
};
use move_command_line_common::error_bitset::ErrorBitset;
use move_command_line_common::display::RenderResult;
use move_command_line_common::{display::try_render_constant, error_bitset::ErrorBitset};
use move_core_types::annotated_value::MoveEnumLayout;
use move_core_types::language_storage::ModuleId;
use move_core_types::u256::U256;
use std::collections::BTreeSet;
use std::num::NonZeroUsize;
use std::sync::{Arc, Mutex};
Expand Down Expand Up @@ -594,40 +594,16 @@ impl<S: PackageStore> Resolver<S> {
.and_then(|x| String::from_utf8(x).ok())?;
let bytes = error_value_constant.data.clone();

let rendered = match &error_value_constant.type_ {
SignatureToken::Vector(inner_ty) if inner_ty.as_ref() == &SignatureToken::U8 => {
bcs::from_bytes::<Vec<u8>>(&bytes)
.ok()
.and_then(|x| String::from_utf8(x).ok())
}
SignatureToken::U8 => bcs::from_bytes::<u8>(&bytes).ok().map(|x| x.to_string()),
SignatureToken::U16 => bcs::from_bytes::<u16>(&bytes).ok().map(|x| x.to_string()),
SignatureToken::U32 => bcs::from_bytes::<u32>(&bytes).ok().map(|x| x.to_string()),
SignatureToken::U64 => bcs::from_bytes::<u64>(&bytes).ok().map(|x| x.to_string()),
SignatureToken::U128 => bcs::from_bytes::<u128>(&bytes).ok().map(|x| x.to_string()),
SignatureToken::U256 => bcs::from_bytes::<U256>(&bytes).ok().map(|x| x.to_string()),
SignatureToken::Address => bcs::from_bytes::<AccountAddress>(&bytes)
.ok()
.map(|x| x.to_canonical_string(true)),
SignatureToken::Bool => bcs::from_bytes::<bool>(&bytes).ok().map(|x| x.to_string()),

SignatureToken::Signer
| SignatureToken::Vector(_)
| SignatureToken::Datatype(_)
| SignatureToken::DatatypeInstantiation(_)
| SignatureToken::Reference(_)
| SignatureToken::MutableReference(_)
| SignatureToken::TypeParameter(_) => None,
};
let rendered = try_render_constant(error_value_constant);

let error_info = match rendered {
None => ErrorConstants::Raw {
RenderResult::NotRendered => ErrorConstants::Raw {
identifier: error_identifier,
bytes,
},
Some(error_constant) => ErrorConstants::Rendered {
RenderResult::AsString(s) | RenderResult::AsValue(s) => ErrorConstants::Rendered {
identifier: error_identifier,
constant: error_constant,
constant: s,
},
};

Expand Down
193 changes: 193 additions & 0 deletions crates/sui/src/clever_error_rendering.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
// Copyright (c) Mysten Labs, Inc.
// SPDX-License-Identifier: Apache-2.0

//! This module provides a function to render a Move abort status string into a more human-readable
//! error message using the clever error rendering logic.
//!
//! The logic in this file is largely a stop-gap to provide Clever Error rendering in the CLI while
//! it still uses the JSON-RPC API. The new GraphQL API already renderd Clever Errors on the server
//! side in a much more robust and efficient way.
//!
//! Once the CLI is updated to use the GraphQL API, this file can be removed, and the GraphQL-based
//! rendering logic for Clever Errors should be used instead.
use fastcrypto::encoding::{Base64, Encoding};
use move_binary_format::{
binary_config::BinaryConfig, file_format::SignatureToken, CompiledModule,
};
use move_command_line_common::{
display::{try_render_constant, RenderResult},
error_bitset::ErrorBitset,
};
use move_core_types::account_address::AccountAddress;
use sui_json_rpc_types::{SuiObjectDataOptions, SuiRawData};
use sui_sdk::apis::ReadApi;
use sui_types::{base_types::ObjectID, Identifier};

/// Take a Move abort status string and render it into a more human-readable error message using
/// by parsing the string (as best we can) and seeing if the abort code is a Clever Error abort
/// code. If it is, we attempt to render the error in a more huma-readable manner using the Read
/// API and decoding the Clever Error encoding in the abort code.
///
/// This function is used to render Clever Errors for on-chain errors only within the Sui CLI. This
/// function is _not_ used at all for off-chain errors or Move unit tests. You should only use this
/// function within this crate.
pub(crate) async fn render_clever_error_opt(
error_string: &str,
read_api: &ReadApi,
) -> Option<String> {
let (address, module_name, function_name, instruction, abort_code, command_index) =
parse_abort_status_string(error_string).ok()?;

let error = 'error: {
let Some(error_bitset) = ErrorBitset::from_u64(abort_code) else {
break 'error format!(
"function '{}::{}::{}' at instruction {} with code {}",
address.to_canonical_display(true),
module_name,
function_name,
instruction,
abort_code
);
};

let line_number = error_bitset.line_number()?;

if error_bitset.constant_index().is_none() && error_bitset.identifier_index().is_none() {
break 'error format!(
"function '{}::{}::{}' at line {}",
address.to_canonical_display(true),
module_name,
function_name,
line_number
);
}

let SuiRawData::Package(package) = read_api
.get_object_with_options(
ObjectID::from_address(address),
SuiObjectDataOptions::bcs_lossless(),
)
.await
.ok()?
.into_object()
.ok()?
.bcs?
else {
return None;
};

let module = package.module_map.get(module_name.as_str())?;
let module =
CompiledModule::deserialize_with_config(module, &BinaryConfig::standard()).ok()?;

let error_identifier_constant = module
.constant_pool()
.get(error_bitset.identifier_index()? as usize)?;
let error_value_constant = module
.constant_pool()
.get(error_bitset.constant_index()? as usize)?;

if !matches!(&error_identifier_constant.type_, SignatureToken::Vector(x) if x.as_ref() == &SignatureToken::U8)
{
return None;
};

let error_identifier = bcs::from_bytes::<Vec<u8>>(&error_identifier_constant.data)
.ok()
.and_then(|x| String::from_utf8(x).ok())?;

let const_str = match try_render_constant(error_value_constant) {
RenderResult::NotRendered => {
format!("'{}'", Base64::encode(&error_value_constant.data))
}
RenderResult::AsString(s) => format!("'{s}'"),
RenderResult::AsValue(v_str) => v_str,
};

format!(
"function '{}::{}::{}' at line {}. Aborted with '{}' -- {}",
address.to_canonical_display(true),
module_name,
function_name,
line_number,
error_identifier,
const_str
)
};

// Convert the command index into an ordinal.
let command = command_index + 1;
let suffix = match command % 10 {
1 => "st",
2 => "nd",
3 => "rd",
_ => "th",
};

Some(format!("{command}{suffix} command aborted within {error}"))
}

/// Parsing the error with a regex is not great, but it's the best we can do with the current
/// JSON-RPC API since we only get error messages as strings. This function attempts to parse a
/// Move abort status string into its different parts, and then parses it back into the structured
/// format that we can then use to render a Clever Error.
///
/// If we are able to parse the string, we return a tuple with the address, module name, function
/// name, instruction, abort code, and command index. If we are unable to parse the string, we
/// return `Err`.
///
/// You should delete this function with glee once the CLI is updated to use the GraphQL API.
fn parse_abort_status_string(
s: &str,
) -> Result<(AccountAddress, Identifier, Identifier, u16, u64, u16), anyhow::Error> {
use regex::Regex;
let re = Regex::new(r#"MoveAbort.*address:\s*(.*?),.* name:.*Identifier\((.*?)\).*instruction:\s+(\d+),.*function_name:.*Some\((.*?)\).*},\s*(\d+).*in command\s*(\d+)"#).unwrap();
let Some(captures) = re.captures(s) else {
anyhow::bail!(
"Cannot parse abort status string: {} as a move abort string",
s
);
};

// Remove any escape characters from the string if present.
let clean_string = |s: &str| s.replace(['\\', '\"'], "");

let address = AccountAddress::from_hex(&captures[1])?;
let module_name = Identifier::new(clean_string(&captures[2]))?;
let instruction = captures[3].parse::<u16>()?;
let function_name = Identifier::new(clean_string(&captures[4]))?;
let abort_code = captures[5].parse::<u64>()?;
let command_index = captures[6].parse::<u16>()?;
Ok((
address,
module_name,
function_name,
instruction,
abort_code,
command_index,
))
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_parse_abort_status_string() {
let corpus = vec![
r#"Failure { error: "MoveAbort(MoveLocation { module: ModuleId { address: 60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20, name: Identifier(\"clever_errors\") }, function: 0, instruction: 1, function_name: Some(\"aborter\") }, 0) in command 0" }"#,
r#"Failure { error: "MoveAbort(MoveLocation { module: ModuleId { address: 60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20, name: Identifier(\"clever_errors\") }, function: 1, instruction: 1, function_name: Some(\"aborter_line_no\") }, 9223372105574252543) in command 0" }"#,
r#"Failure { error: "MoveAbort(MoveLocation { module: ModuleId { address: 60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20, name: Identifier(\"clever_errors\") }, function: 2, instruction: 1, function_name: Some(\"clever_aborter\") }, 9223372118459154433) in command 0" }"#,
r#"Failure { error: "MoveAbort(MoveLocation { module: ModuleId { address: 60197a0c146e31dd12689e890208767fe2fefb2f726710b4a9fa0b857f7a2c20, name: Identifier(\"clever_errors\") }, function: 3, instruction: 1, function_name: Some(\"clever_aborter_not_a_string\") }, 9223372135639154691) in command 0" }"#,
r#"MoveAbort(MoveLocation { module: ModuleId { address: 24bf9e624820625ac1e38076901421d2630b2b225b638aaf0b85264b857a608b, name: Identifier(\"tester\") }, function: 0, instruction: 1, function_name: Some(\"test\") }, 9223372071214514177) in command 0"#,
r#"MoveAbort(MoveLocation { module: ModuleId { address: 24bf9e624820625ac1e38076901421d2630b2b225b638aaf0b85264b857a608b, name: Identifier("tester") }, function: 0, instruction: 1, function_name: Some("test") }, 9223372071214514177) in command 0"#,
];
let parsed: Vec<_> = corpus.into_iter().map(|c| {
let (address, module_name, function_name, instruction, abort_code, command_index) =
parse_abort_status_string(c).unwrap();
format!("original abort message: {}\n address: {}\n module_name: {}\n function_name: {}\n instruction: {}\n abort_code: {}\n command_index: {}", c, address, module_name, function_name, instruction, abort_code, command_index)
}).collect();
insta::assert_snapshot!(parsed.join("\n------\n"));
}
}
75 changes: 66 additions & 9 deletions crates/sui/src/client_commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// SPDX-License-Identifier: Apache-2.0

use crate::{
clever_error_rendering::render_clever_error_opt,
client_ptb::ptb::PTB,
displays::Pretty,
key_identity::{get_identity_address, KeyIdentity},
Expand Down Expand Up @@ -42,7 +43,8 @@ use sui_json_rpc_types::{
Coin, DryRunTransactionBlockResponse, DynamicFieldPage, SuiCoinMetadata, SuiData,
SuiExecutionStatus, SuiObjectData, SuiObjectDataOptions, SuiObjectResponse,
SuiObjectResponseQuery, SuiParsedData, SuiProtocolConfigValue, SuiRawData,
SuiTransactionBlockEffectsAPI, SuiTransactionBlockResponse, SuiTransactionBlockResponseOptions,
SuiTransactionBlockEffects, SuiTransactionBlockEffectsAPI, SuiTransactionBlockResponse,
SuiTransactionBlockResponseOptions,
};
use sui_keys::keystore::AccountKeystore;
use sui_move_build::{
Expand Down Expand Up @@ -663,7 +665,7 @@ impl SuiClientCommands {
self,
context: &mut WalletContext,
) -> Result<SuiClientCommandResult, anyhow::Error> {
let ret = Ok(match self {
let ret = match self {
SuiClientCommands::ProfileTransaction {
tx_digest,
profile_output,
Expand Down Expand Up @@ -1583,8 +1585,9 @@ impl SuiClientCommands {
ptb.execute(context).await?;
SuiClientCommandResult::NoOutput
}
});
ret
};
let client = context.get_client().await?;
Ok(ret.prerender_clever_errors(client.read_api()).await)
}

pub fn switch_env(config: &mut SuiClientConfig, env: &str) -> Result<(), anyhow::Error> {
Expand Down Expand Up @@ -2204,6 +2207,42 @@ impl SuiClientCommandResult {
_ => None,
}
}

pub async fn prerender_clever_errors(mut self, read_api: &ReadApi) -> Self {
match &mut self {
SuiClientCommandResult::DryRun(DryRunTransactionBlockResponse { effects, .. })
| SuiClientCommandResult::TransactionBlock(SuiTransactionBlockResponse {
effects: Some(effects),
..
}) => prerender_clever_errors(effects, read_api).await,

SuiClientCommandResult::TransactionBlock(SuiTransactionBlockResponse {
effects: None,
..
}) => (),
SuiClientCommandResult::ActiveAddress(_)
| SuiClientCommandResult::ActiveEnv(_)
| SuiClientCommandResult::Addresses(_)
| SuiClientCommandResult::Balance(_, _)
| SuiClientCommandResult::ChainIdentifier(_)
| SuiClientCommandResult::DynamicFieldQuery(_)
| SuiClientCommandResult::Envs(_, _)
| SuiClientCommandResult::Gas(_)
| SuiClientCommandResult::NewAddress(_)
| SuiClientCommandResult::NewEnv(_)
| SuiClientCommandResult::NoOutput
| SuiClientCommandResult::Object(_)
| SuiClientCommandResult::Objects(_)
| SuiClientCommandResult::RawObject(_)
| SuiClientCommandResult::SerializedSignedTransaction(_)
| SuiClientCommandResult::SerializedUnsignedTransaction(_)
| SuiClientCommandResult::Switch(_)
| SuiClientCommandResult::SyncClientState
| SuiClientCommandResult::VerifyBytecodeMeter { .. }
| SuiClientCommandResult::VerifySource => (),
}
self
}
}

#[derive(Serialize)]
Expand Down Expand Up @@ -2568,7 +2607,10 @@ pub async fn execute_dry_run(
.dry_run_transaction_block(dry_run_tx_data)
.await
.map_err(|e| anyhow!("Dry run failed: {e}"))?;
Ok(SuiClientCommandResult::DryRun(response))
let resp = SuiClientCommandResult::DryRun(response)
.prerender_clever_errors(client.read_api())
.await;
Ok(resp)
}

/// Call a dry run with the transaction data to estimate the gas budget.
Expand Down Expand Up @@ -2718,17 +2760,32 @@ pub(crate) async fn dry_run_or_execute_or_serialize(
))
} else {
let transaction = Transaction::new(sender_signed_data);
let response = context.execute_transaction_may_fail(transaction).await?;
let mut response = context.execute_transaction_may_fail(transaction).await?;
if let Some(effects) = response.effects.as_mut() {
prerender_clever_errors(effects, client.read_api()).await;
}
let effects = response.effects.as_ref().ok_or_else(|| {
anyhow!("Effects from SuiTransactionBlockResult should not be empty")
})?;
if matches!(effects.status(), SuiExecutionStatus::Failure { .. }) {
if let SuiExecutionStatus::Failure { error } = effects.status() {
return Err(anyhow!(
"Error executing transaction: {:#?}",
effects.status()
"Error executing transaction '{}': {error}",
response.digest
));
}
Ok(SuiClientCommandResult::TransactionBlock(response))
}
}
}

pub(crate) async fn prerender_clever_errors(
effects: &mut SuiTransactionBlockEffects,
read_api: &ReadApi,
) {
let SuiTransactionBlockEffects::V1(effects) = effects;
if let SuiExecutionStatus::Failure { error } = &mut effects.status {
if let Some(rendered) = render_clever_error_opt(error, read_api).await {
*error = rendered;
}
}
}
Loading

0 comments on commit aa1ad98

Please sign in to comment.