refactor(rpc-call): Never return <wasm:stripped> from rpc calls (#3655

)

gsdk/tests/rpc.rs

@@ -28,6 +28,36 @@ use utils::{alice_account_id, dev_node, node_uri};
 
 mod utils;
 
+#[tokio::test]
+async fn pallet_errors_formatting() -> Result<()> {
+    let node = dev_node();
+    let api = Api::new(Some(&node_uri(&node))).await?;
+
+    let err = api
+        .calculate_upload_gas(
+            [0u8; 32].into(),
+            /* invalid code */ vec![],
+            vec![],
+            0,
+            true,
+            None,
+        )
+        .await
+        .expect_err("Must return error");
+
+    let expected_err = Error::Subxt(SubxtError::Rpc(RpcError::ClientError(Box::new(
+        CallError::Custom(ErrorObject::owned(
+            8000,
+            "Runtime error",
+            Some("Extrinsic `gear.upload_program` failed: 'ProgramConstructionFailed'"),
+        )),
+    ))));
+
+    assert_eq!(format!("{err}"), format!("{expected_err}"));
+
+    Ok(())
+}
+
 #[tokio::test]
 async fn test_calculate_upload_gas() -> Result<()> {
     let node = dev_node();

pallets/gear/rpc/src/lib.rs

@@ -39,11 +39,11 @@ use sp_runtime::traits::Block as BlockT;
 use std::sync::Arc;
 
 /// Converts a runtime trap into a [`CallError`].
-fn runtime_error_into_rpc_error(err: impl std::fmt::Debug) -> JsonRpseeError {
+fn runtime_error_into_rpc_error(err: impl std::fmt::Display) -> JsonRpseeError {
     CallError::Custom(ErrorObject::owned(
         8000,
         "Runtime error",
-        Some(format!("{err:?}")),
+        Some(format!("{err}")),
     ))
     .into()
 }

pallets/gear/src/runtime_api.rs

@@ -21,12 +21,15 @@ use crate::queue::{ActorResult, QueueStep};
 use common::ActiveProgram;
 use core::convert::TryFrom;
 use core_processor::common::PrechargedDispatch;
+use frame_support::traits::PalletInfo;
 use gear_core::{code::TryNewCodeConfig, pages::WasmPage, program::MemoryInfix};
 use gear_wasm_instrument::syscalls::SyscallName;
+use sp_runtime::{DispatchErrorWithPostInfo, ModuleError};
 
 // Multiplier 6 was experimentally found as median value for performance,
 // security and abilities for calculations on-chain.
 pub(crate) const RUNTIME_API_BLOCK_LIMITS_COUNT: u64 = 6;
+pub(crate) const ALLOWANCE_LIMIT_ERR: &str = "Calculation gas limit exceeded. Use your own RPC node with `--rpc-calculations-multiplier` parameter raised";
 
 pub(crate) struct CodeWithMemoryData {
     pub instrumented_code: InstrumentedCode,
@@ -69,16 +72,45 @@ where
 
         QueueOf::<T>::clear();
 
+        let map_extrinsic_err = |extrinsic_name: &'static str,
+                                 e: DispatchErrorWithPostInfo<PostDispatchInfo>|
+         -> Vec<u8> {
+            let error_module_idx = if let DispatchError::Module(ModuleError { index, .. }) = e.error
+            {
+                Some(index as usize)
+            } else {
+                None
+            };
+
+            let error_message: &'static str = e.into();
+
+            let gear_module_idx =
+                <<T as frame_system::Config>::PalletInfo as PalletInfo>::index::<Pallet<T>>()
+                    .expect("No index found for the gear pallet in the runtime!");
+
+            let mut res = format!("Extrinsic `gear.{extrinsic_name}` failed: '{error_message}'");
+
+            if let Some(module_idx) = error_module_idx.filter(|i| *i != gear_module_idx) {
+                res = format!("{res} (pallet index of the error: {module_idx}");
+            }
+
+            res.into_bytes()
+        };
+
+        let internal_err = |message: &'static str| -> Vec<u8> {
+            format!("Internal error: entered unreachable code '{message}'").into_bytes()
+        };
+
         match kind {
             HandleKind::Init(code) => {
                 let salt = b"calculate_gas_salt".to_vec();
+
                 Self::upload_program(who.into(), code, salt, payload, initial_gas, value, false)
-                    .map_err(|e| {
-                        format!("Internal error: upload_program failed with '{e:?}'").into_bytes()
-                    })?;
+                    .map_err(|e| map_extrinsic_err("upload_program", e))?;
             }
             HandleKind::InitByHash(code_id) => {
                 let salt = b"calculate_gas_salt".to_vec();
+
                 Self::create_program(
                     who.into(),
                     code_id,
@@ -88,21 +120,15 @@ where
                     value,
                     false,
                 )
-                .map_err(|e| {
-                    format!("Internal error: create_program failed with '{e:?}'").into_bytes()
-                })?;
+                .map_err(|e| map_extrinsic_err("create_program", e))?;
             }
             HandleKind::Handle(destination) => {
                 Self::send_message(who.into(), destination, payload, initial_gas, value, false)
-                    .map_err(|e| {
-                        format!("Internal error: send_message failed with '{e:?}'").into_bytes()
-                    })?;
+                    .map_err(|e| map_extrinsic_err("send_message", e))?;
             }
             HandleKind::Reply(reply_to_id, _status_code) => {
                 Self::send_reply(who.into(), reply_to_id, payload, initial_gas, value, false)
-                    .map_err(|e| {
-                        format!("Internal error: send_reply failed with '{e:?}'").into_bytes()
-                    })?;
+                    .map_err(|e| map_extrinsic_err("send_reply", e))?;
             }
             HandleKind::Signal(_signal_from, _status_code) => {
                 return Err(b"Gas calculation for `handle_signal` is not supported".to_vec());
@@ -111,10 +137,10 @@ where
 
         let (main_message_id, main_program_id) = QueueOf::<T>::iter()
             .next()
-            .ok_or_else(|| b"Internal error: failed to get last message".to_vec())
+            .ok_or_else(|| internal_err("Failed to get last message from the queue"))
             .and_then(|queued| {
                 queued
-                    .map_err(|_| b"Internal error: failed to retrieve queued dispatch".to_vec())
+                    .map_err(|_| internal_err("Failed to extract queued dispatch"))
                     .map(|dispatch| (dispatch.id(), dispatch.destination()))
             })?;
 
@@ -135,13 +161,11 @@ where
 
         loop {
             if QueueProcessingOf::<T>::denied() {
-                return Err(
-                    b"Calculation gas limit exceeded. Consider using custom built node.".to_vec(),
-                );
+                return Err(ALLOWANCE_LIMIT_ERR.as_bytes().to_vec());
             }
 
             let Some(queued_dispatch) =
-                QueueOf::<T>::dequeue().map_err(|_| b"MQ storage corrupted".to_vec())?
+                QueueOf::<T>::dequeue().map_err(|_| internal_err("Message queue corrupted"))?
             else {
                 break;
             };
@@ -164,8 +188,9 @@ where
                 .reply_details()
                 .map(|rd| rd.to_reply_code().is_success())
                 .unwrap_or(false);
+
             let gas_limit = GasHandlerOf::<T>::get_limit(dispatch_id)
-                .map_err(|_| b"Internal error: unable to get gas limit".to_vec())?;
+                .map_err(|_| internal_err("Failed to get gas limit"))?;
 
             let skip_if_allowed = success_reply && gas_limit == 0;
 
@@ -177,7 +202,10 @@ where
                 balance: balance.unique_saturated_into(),
                 get_actor_data,
             };
-            let journal = step.execute().unwrap_or_else(|e| unreachable!("{e:?}"));
+
+            let journal = step
+                .execute()
+                .map_err(|_| internal_err("Queue execution error"))?;
 
             let get_main_limit = || {
                 // For case when node is not consumed and has any (even zero) balance
@@ -201,7 +229,7 @@ where
 
             let get_origin_msg_of = |msg_id| {
                 GasHandlerOf::<T>::get_origin_key(msg_id)
-                    .map_err(|_| b"Internal error: unable to get origin key".to_vec())
+                    .map_err(|_| internal_err("Failed to get origin key"))
             };
             let from_main_chain =
                 |msg_id| get_origin_msg_of(msg_id).map(|v| v == main_message_id.into());
@@ -239,8 +267,7 @@ where
                                 .gas_limit()
                                 .or_else(|| GasHandlerOf::<T>::get_limit(dispatch.id()).ok())
                                 .ok_or_else(|| {
-                                    b"Internal error: unable to get gas limit after execution"
-                                        .to_vec()
+                                    internal_err("Failed to get gas limit after execution")
                                 })?;
 
                             reserved = reserved.saturating_add(gas_limit);
@@ -260,7 +287,9 @@ where
                     } if (message_id == main_message_id || !allow_other_panics)
                         && !(skip_if_allowed && allow_skip_zero_replies) =>
                     {
-                        return Err(format!("Program terminated with a trap: {trap}").into_bytes());
+                        return Err(
+                            format!("Program terminated with a trap: '{trap}'").into_bytes()
+                        );
                     }
 
                     _ => (),

pallets/gear/src/tests.rs

@@ -44,7 +44,7 @@ use crate::{
         USER_3,
     },
     pallet,
-    runtime_api::RUNTIME_API_BLOCK_LIMITS_COUNT,
+    runtime_api::{ALLOWANCE_LIMIT_ERR, RUNTIME_API_BLOCK_LIMITS_COUNT},
     AccountIdOf, BlockGasLimitOf, Config, CostsPerBlockOf, CurrencyOf, DbWeightOf, DispatchStashOf,
     Error, Event, GasAllowanceOf, GasBalanceOf, GasHandlerOf, GasInfo, GearBank, MailboxOf,
     ProgramStorageOf, QueueOf, Schedule, TaskPoolOf, WaitlistOf,
@@ -8341,22 +8341,19 @@ fn call_forbidden_function() {
 
         run_to_block(2, None);
 
-        let res = Gear::calculate_gas_info(
+        let err = Gear::calculate_gas_info(
             USER_1.into_origin(),
             HandleKind::Handle(prog_id),
             EMPTY_PAYLOAD.to_vec(),
             0,
             true,
             true,
-        );
+        )
+        .expect_err("Must return error");
 
-        assert_eq!(
-            res,
-            Err(format!(
-                "Program terminated with a trap: {}",
-                TrapExplanation::ForbiddenFunction,
-            ))
-        );
+        let trap = TrapExplanation::ForbiddenFunction;
+
+        assert_eq!(err, format!("Program terminated with a trap: '{trap}'"));
     });
 }
 
@@ -13111,10 +13108,7 @@ fn calculate_gas_fails_when_calculation_limit_exceeded() {
         );
 
         assert!(gas_info_result.is_err());
-        assert_eq!(
-            gas_info_result.unwrap_err(),
-            "Calculation gas limit exceeded. Consider using custom built node."
-        );
+        assert_eq!(gas_info_result.unwrap_err(), ALLOWANCE_LIMIT_ERR);
 
         // ok result when we use custom multiplier
         let gas_info_result = Gear::calculate_gas_info_impl(