Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Jan 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions gsdk/tests/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}"));
breathx marked this conversation as resolved.
Show resolved Hide resolved

Ok(())
}

#[tokio::test]
async fn test_calculate_upload_gas() -> Result<()> {
let node = dev_node();
Expand Down
4 changes: 2 additions & 2 deletions pallets/gear/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
77 changes: 53 additions & 24 deletions pallets/gear/src/runtime_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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());
Expand All @@ -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()))
})?;

Expand All @@ -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;
};
Expand All @@ -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;

Expand All @@ -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
Expand All @@ -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());
Expand Down Expand Up @@ -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);
Expand All @@ -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()
);
}

_ => (),
Expand Down
22 changes: 8 additions & 14 deletions pallets/gear/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}'"));
});
}

Expand Down Expand Up @@ -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(
Expand Down