fixup! fixup! fixup! [CLI] Add clever error support to Sui CLI

crates/sui/src/clever_error_rendering.rs

@@ -1,6 +1,16 @@
 // 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,
@@ -14,7 +24,18 @@ use sui_json_rpc_types::{SuiObjectDataOptions, SuiRawData};
 use sui_sdk::apis::ReadApi;
 use sui_types::{base_types::ObjectID, Identifier};
 
-pub async fn render_clever_error_opt(error_string: &str, read_api: &ReadApi) -> Option<String> {
+/// 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()?;
 
@@ -107,6 +128,16 @@ pub async fn render_clever_error_opt(error_string: &str, read_api: &ReadApi) ->
     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> {

crates/sui/tests/cli_tests.rs

@@ -3729,6 +3729,12 @@ async fn test_clever_errors() -> Result<(), anyhow::Error> {
         .find(|refe| matches!(refe.owner, Owner::Immutable))
         .unwrap();
 
+    let elide_transaction_digest = |s: String| -> String {
+        let mut x = s.splitn(3, '\'').collect::<Vec<_>>();
+        x[1] = "<ELIDED_TRANSACTION_DIGEST>";
+        x.join("'")
+    };
+
     // Normal abort
     let non_clever_abort = SuiClientCommands::Call {
         package: package.reference.object_id,
@@ -3785,11 +3791,14 @@ async fn test_clever_errors() -> Result<(), anyhow::Error> {
     .await
     .unwrap_err();
 
-    insta::assert_snapshot!(
-        format!(
-            "Non-clever-abort\n---\n{}\n---\nLine-only-abort\n---\n{}\n---\nClever-error-utf8\n---\n{}\n---\nClever-error-non-utf8\n---\n{}\n---\n",
-             non_clever_abort, line_only_abort, clever_error_utf8, clever_error_non_utf8
-        )
+    let error_string = format!(
+        "Non-clever-abort\n---\n{}\n---\nLine-only-abort\n---\n{}\n---\nClever-error-utf8\n---\n{}\n---\nClever-error-non-utf8\n---\n{}\n---\n",
+        elide_transaction_digest(non_clever_abort.to_string()), 
+        elide_transaction_digest(line_only_abort.to_string()),
+        elide_transaction_digest(clever_error_utf8.to_string()),
+        elide_transaction_digest(clever_error_non_utf8.to_string())
     );
+
+    insta::assert_snapshot!(error_string);
     Ok(())
 }

crates/sui/tests/data/clever_errors/sources/clever_errors.move

@@ -1,3 +1,6 @@
+// Copyright (c) Mysten Labs, Inc.
+// SPDX-License-Identifier: Apache-2.0
+
 // Do _not_ edit this file (yes, even whitespace!). Editing this file will
 // cause the tests that use this module to fail.
 module clever_errors::clever_errors {

crates/sui/tests/snapshots/cli_tests__body_fn.snap

@@ -1,21 +1,21 @@
 ---
 source: crates/sui/tests/cli_tests.rs
-expression: "format!(\"Non-clever-abort\\n---\\n{}\\n---\\nLine-only-abort\\n---\\n{}\\n---\\nClever-error-utf8\\n---\\n{}\\n---\\nClever-error-non-utf8\\n---\\n{}\\n---\\n\",\n    non_clever_abort, line_only_abort, clever_error_utf8,\n    clever_error_non_utf8)"
+expression: error_string
 ---
 Non-clever-abort
 ---
-Error executing transaction 'BQaYuRVbz7BjpjmDXUmFYyxADDK4WySrg1AfTqTao1jo': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::aborter' at instruction 1 with code 0
+Error executing transaction '<ELIDED_TRANSACTION_DIGEST>': 1st command aborted within function '0x04becddebc3ae9e7e48c3f31144860e0f99a7414e533716a16db14a831ac4be6::clever_errors::aborter' at instruction 1 with code 0
 ---
 Line-only-abort
 ---
-Error executing transaction '3w3diTUzG3ryjpMA3WPrfx9eLNArYqRn1dMDWyTggKGA': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::aborter_line_no' at line 15
+Error executing transaction '<ELIDED_TRANSACTION_DIGEST>': 1st command aborted within function '0x04becddebc3ae9e7e48c3f31144860e0f99a7414e533716a16db14a831ac4be6::clever_errors::aborter_line_no' at line 18
 ---
 Clever-error-utf8
 ---
-Error executing transaction '7YmFWxA3WKzKF7Bp3sJisD83EZTGXFosD4TKmq8ughLY': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::clever_aborter' at line 19. Aborted with 'ENotFound' -- 'Element not found in vector 💥 🚀 🌠'
+Error executing transaction '<ELIDED_TRANSACTION_DIGEST>': 1st command aborted within function '0x04becddebc3ae9e7e48c3f31144860e0f99a7414e533716a16db14a831ac4be6::clever_errors::clever_aborter' at line 22. Aborted with 'ENotFound' -- 'Element not found in vector 💥 🚀 🌠'
 ---
 Clever-error-non-utf8
 ---
-Error executing transaction 'FK7UrheD4TWakAnK5j5tC3LySXd9obtixumbE9ZS5UYp': 1st command aborted within function '0xb691b7e8d182a5d5c8dd834b234a2bd55d9c42210cba8e852cd134586a74390e::clever_errors::clever_aborter_not_a_string' at line 23. Aborted with 'ENotAString' -- 'BAEAAAAAAAAAAgAAAAAAAAADAAAAAAAAAAQAAAAAAAAA'
+Error executing transaction '<ELIDED_TRANSACTION_DIGEST>': 1st command aborted within function '0x04becddebc3ae9e7e48c3f31144860e0f99a7414e533716a16db14a831ac4be6::clever_errors::clever_aborter_not_a_string' at line 26. Aborted with 'ENotAString' -- 'BAEAAAAAAAAAAgAAAAAAAAADAAAAAAAAAAQAAAAAAAAA'
 ---