FIx: starknet_call errs if contract nonexistent (#1555)

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## Next release
 
+- fix: starknet_call errs if contract nonexistent
 - fix: txn hash calculation and refactor
 - chore: remove all da/settlement related code
 - fix: re-execute txs instead of simulating for txn receipts

crates/client/rpc/Cargo.toml

@@ -44,6 +44,8 @@ mp-felt = { workspace = true }
 mp-hashers = { workspace = true }
 mp-simulations = { workspace = true }
 mp-transactions = { workspace = true, features = ["client"] }
+serde = { workspace = true }
+serde_json = { workspace = true }
 starknet-core = { workspace = true }
 starknet-ff = { workspace = true }
 starknet_api = { workspace = true }

crates/client/rpc/src/errors.rs

@@ -1,67 +1,126 @@
+use blockifier::transaction::errors::TransactionExecutionError;
 use jsonrpsee::types::error::{CallError, ErrorObject};
-use pallet_starknet_runtime_api::StarknetTransactionExecutionError;
+use mp_simulations::{InternalSubstrateError, SimulationError};
+use serde::Serialize;
+use thiserror::Error;
 
 // Comes from the RPC Spec:
 // https://github.com/starkware-libs/starknet-specs/blob/0e859ff905795f789f1dfd6f7340cdaf5015acc8/api/starknet_write_api.json#L227
-#[derive(thiserror::Error, Clone, Copy, Debug)]
+#[derive(Error, Debug)]
 pub enum StarknetRpcApiError {
     #[error("Failed to write transaction")]
-    FailedToReceiveTxn = 1,
+    FailedToReceiveTxn,
     #[error("Contract not found")]
-    ContractNotFound = 20,
+    ContractNotFound,
     #[error("Block not found")]
-    BlockNotFound = 24,
+    BlockNotFound,
     #[error("Invalid transaction index in a block")]
-    InvalidTxnIndex = 27,
+    InvalidTxnIndex,
     #[error("Class hash not found")]
-    ClassHashNotFound = 28,
+    ClassHashNotFound,
     #[error("Transaction hash not found")]
-    TxnHashNotFound = 29,
+    TxnHashNotFound,
     #[error("Requested page size is too big")]
-    PageSizeTooBig = 31,
+    PageSizeTooBig,
     #[error("There are no blocks")]
-    NoBlocks = 32,
+    NoBlocks,
     #[error("The supplied continuation token is invalid or unknown")]
-    InvalidContinuationToken = 33,
+    InvalidContinuationToken,
     #[error("Too many keys provided in a filter")]
-    TooManyKeysInFilter = 34,
+    TooManyKeysInFilter,
     #[error("Failed to fetch pending transactions")]
-    FailedToFetchPendingTransactions = 38,
-    #[error("Contract error")]
-    ContractError = 40,
+    FailedToFetchPendingTransactions,
+    #[error("Contract Error")]
+    ContractError(#[from] ContractError),
     #[error("Invalid contract class")]
-    InvalidContractClass = 50,
+    InvalidContractClass,
     #[error("Class already declared")]
-    ClassAlreadyDeclared = 51,
+    ClassAlreadyDeclared,
     #[error("Account validation failed")]
-    ValidationFailure = 55,
+    ValidationFailure,
     #[error("The transaction version is not supported")]
-    UnsupportedTxVersion = 61,
+    UnsupportedTxVersion,
     #[error("Internal server error")]
-    InternalServerError = 500,
+    InternalServerError,
     #[error("Unimplemented method")]
-    UnimplementedMethod = 501,
+    UnimplementedMethod,
     #[error("Too many storage keys requested")]
-    ProofLimitExceeded = 10000,
+    ProofLimitExceeded,
 }
 
-impl From<StarknetTransactionExecutionError> for StarknetRpcApiError {
-    fn from(err: StarknetTransactionExecutionError) -> Self {
-        match err {
-            StarknetTransactionExecutionError::ContractNotFound => StarknetRpcApiError::ContractNotFound,
-            StarknetTransactionExecutionError::ClassAlreadyDeclared => StarknetRpcApiError::ClassAlreadyDeclared,
-            StarknetTransactionExecutionError::ClassHashNotFound => StarknetRpcApiError::ClassHashNotFound,
-            StarknetTransactionExecutionError::InvalidContractClass => StarknetRpcApiError::InvalidContractClass,
-            StarknetTransactionExecutionError::ContractError => StarknetRpcApiError::ContractError,
-        }
-    }
+#[derive(Debug, Error, Serialize)]
+#[error("revert error: {revert_error}")]
+pub struct ContractError {
+    revert_error: String,
 }
 
 impl From<StarknetRpcApiError> for jsonrpsee::core::Error {
     fn from(err: StarknetRpcApiError) -> Self {
-        // TODO:
-        // when able to ge rich errors out of runtime,
-        // match and populate the `data` field
-        jsonrpsee::core::Error::Call(CallError::Custom(ErrorObject::owned(err as i32, err.to_string(), None::<()>)))
+        let code = match err {
+            StarknetRpcApiError::FailedToReceiveTxn => 1,
+            StarknetRpcApiError::ContractNotFound => 20,
+            StarknetRpcApiError::BlockNotFound => 24,
+            StarknetRpcApiError::InvalidTxnIndex => 27,
+            StarknetRpcApiError::ClassHashNotFound => 28,
+            StarknetRpcApiError::TxnHashNotFound => 29,
+            StarknetRpcApiError::PageSizeTooBig => 31,
+            StarknetRpcApiError::NoBlocks => 32,
+            StarknetRpcApiError::InvalidContinuationToken => 33,
+            StarknetRpcApiError::TooManyKeysInFilter => 34,
+            StarknetRpcApiError::FailedToFetchPendingTransactions => 38,
+            StarknetRpcApiError::ContractError(_) => 40,
+            StarknetRpcApiError::InvalidContractClass => 50,
+            StarknetRpcApiError::ClassAlreadyDeclared => 51,
+            StarknetRpcApiError::ValidationFailure => 55,
+            StarknetRpcApiError::UnsupportedTxVersion => 61,
+            StarknetRpcApiError::InternalServerError => 500,
+            StarknetRpcApiError::UnimplementedMethod => 501,
+            StarknetRpcApiError::ProofLimitExceeded => 10000,
+        };
+
+        let data = match &err {
+            StarknetRpcApiError::ContractError(ref error) => Some(error),
+            _ => None,
+        };
+
+        jsonrpsee::core::Error::Call(CallError::Custom(ErrorObject::owned(code, err.to_string(), data)))
+    }
+}
+
+impl From<String> for ContractError {
+    fn from(value: String) -> Self {
+        ContractError { revert_error: value }
+    }
+}
+
+impl From<TransactionExecutionError> for ContractError {
+    fn from(e: TransactionExecutionError) -> Self {
+        ContractError { revert_error: e.to_string() }
+    }
+}
+
+impl From<TransactionExecutionError> for StarknetRpcApiError {
+    fn from(e: TransactionExecutionError) -> Self {
+        StarknetRpcApiError::ContractError(e.into())
+    }
+}
+
+impl From<SimulationError> for StarknetRpcApiError {
+    fn from(value: SimulationError) -> Self {
+        match value {
+            SimulationError::ContractNotFound => StarknetRpcApiError::ContractNotFound,
+            SimulationError::TransactionExecutionFailed(e) => StarknetRpcApiError::ContractError(e.into()),
+            SimulationError::MissingL1GasUsage | SimulationError::StateDiff => StarknetRpcApiError::InternalServerError,
+        }
+    }
+}
+
+impl From<InternalSubstrateError> for StarknetRpcApiError {
+    fn from(value: InternalSubstrateError) -> Self {
+        match value {
+            InternalSubstrateError::FailedToCreateATransactionalStorageExecution => {
+                StarknetRpcApiError::InternalServerError
+            }
+        }
     }
 }

crates/client/rpc/src/lib.rs

@@ -52,7 +52,6 @@ use sp_blockchain::HeaderBackend;
 use sp_core::H256;
 use sp_runtime::traits::{Block as BlockT, Header as HeaderT};
 use sp_runtime::transaction_validity::InvalidTransaction;
-use sp_runtime::DispatchError;
 use starknet_api::block::BlockHash;
 use starknet_api::core::Nonce;
 use starknet_api::hash::StarkFelt;
@@ -618,16 +617,13 @@ where
         })?;
 
         let calldata = Calldata(Arc::new(request.calldata.iter().map(|x| Felt252Wrapper::from(*x).into()).collect()));
-
         let result = self.do_call(
             substrate_block_hash,
             Felt252Wrapper(request.contract_address).into(),
             Felt252Wrapper(request.entry_point_selector).into(),
             calldata,
         )?;
 
-        let result = self.convert_error(substrate_block_hash, result)?;
-
         Ok(result.iter().map(|x| format!("{:#x}", x.0)).collect())
     }
 
@@ -1709,20 +1705,6 @@ where
 
         Ok(MaybePendingTransactionReceipt::PendingReceipt(receipt))
     }
-
-    fn convert_error<T>(
-        &self,
-        best_block_hash: <B as BlockT>::Hash,
-        call_result: Result<T, DispatchError>,
-    ) -> Result<T, StarknetRpcApiError> {
-        match call_result {
-            Ok(val) => Ok(val),
-            Err(e) => {
-                let starknet_error = self.convert_dispatch_error(best_block_hash, e)?;
-                Err(starknet_error.into())
-            }
-        }
-    }
 }
 
 async fn submit_extrinsic<P, B>(

crates/client/rpc/src/runtime_api.rs

@@ -11,15 +11,12 @@ pub use mc_rpc_core::{
 use mp_felt::Felt252Wrapper;
 use mp_hashers::HasherT;
 use mp_simulations::SimulationFlags;
-use pallet_starknet_runtime_api::{
-    ConvertTransactionRuntimeApi, StarknetRuntimeApi, StarknetTransactionExecutionError,
-};
+use pallet_starknet_runtime_api::{ConvertTransactionRuntimeApi, StarknetRuntimeApi};
 use sc_client_api::backend::Backend;
 use sc_transaction_pool::ChainApi;
-use sp_api::ProvideRuntimeApi;
+use sp_api::{ApiError, ProvideRuntimeApi};
 use sp_blockchain::HeaderBackend;
 use sp_runtime::traits::Block as BlockT;
-use sp_runtime::DispatchError;
 use starknet_api::core::{ContractAddress, EntryPointSelector};
 use starknet_api::transaction::{Calldata, Event, TransactionHash};
 
@@ -43,29 +40,24 @@ where
         contract_address: ContractAddress,
         entry_point_selector: EntryPointSelector,
         calldata: Calldata,
-    ) -> RpcApiResult<Result<Vec<Felt252Wrapper>, sp_runtime::DispatchError>> {
-        self.client.runtime_api().call(best_block_hash, contract_address, entry_point_selector, calldata).map_err(|e| {
-            error!("Request parameters error: {e}");
-            StarknetRpcApiError::InternalServerError
-        })
+    ) -> RpcApiResult<Vec<Felt252Wrapper>> {
+        Ok(self.client.runtime_api().call(best_block_hash, contract_address, entry_point_selector, calldata).map_err(
+            |e| {
+                error!("Request parameters error: {e}");
+                StarknetRpcApiError::InternalServerError
+            },
+        )??)
     }
 
     pub fn do_estimate_message_fee(
         &self,
         block_hash: B::Hash,
         message: L1HandlerTransaction,
     ) -> RpcApiResult<(u128, u128, u128)> {
-        self.client
-            .runtime_api()
-            .estimate_message_fee(block_hash, message)
-            .map_err(|e| {
-                error!("Runtime Api error: {e}");
-                StarknetRpcApiError::InternalServerError
-            })?
-            .map_err(|e| {
-                error!("Function execution failed: {:#?}", e);
-                StarknetRpcApiError::ContractError
-            })
+        Ok(self.client.runtime_api().estimate_message_fee(block_hash, message).map_err(|e| {
+            error!("Runtime Api error: {e}");
+            StarknetRpcApiError::InternalServerError
+        })???)
     }
 
     pub fn do_get_tx_execution_outcome(
@@ -96,17 +88,6 @@ where
         })
     }
 
-    pub fn convert_dispatch_error(
-        &self,
-        best_block_hash: B::Hash,
-        error: DispatchError,
-    ) -> RpcApiResult<StarknetTransactionExecutionError> {
-        self.client.runtime_api().convert_error(best_block_hash, error).map_err(|e| {
-            error!("Failed to convert dispatch error: {:?}", e);
-            StarknetRpcApiError::InternalServerError
-        })
-    }
-
     pub fn convert_tx_to_extrinsic(
         &self,
         best_block_hash: <B as BlockT>::Hash,
@@ -124,17 +105,12 @@ where
         transactions: Vec<AccountTransaction>,
         simulation_flags: SimulationFlags,
     ) -> RpcApiResult<Vec<(u128, u128)>> {
-        self.client
-            .runtime_api()
-            .estimate_fee(block_hash, transactions, simulation_flags)
-            .map_err(|e| {
+        Ok(self.client.runtime_api().estimate_fee(block_hash, transactions, simulation_flags).map_err(
+            |e: ApiError| {
                 error!("Request parameters error: {e}");
                 StarknetRpcApiError::InternalServerError
-            })?
-            .map_err(|e| {
-                error!("Failed to call function: {:#?}", e);
-                StarknetRpcApiError::ContractError
-            })
+            },
+        )???)
     }
 
     pub fn get_best_block_hash(&self) -> B::Hash {
@@ -206,8 +182,8 @@ where
             })?
             .map_err(|e| {
                 error!("Failed to call function: {:#?}", e);
-                StarknetRpcApiError::ContractError
-            })?
+                StarknetRpcApiError::from(e)
+            })??
             .swap_remove(0)
             .1
             .map_err(|e| {
@@ -232,7 +208,7 @@ where
             })?
             .map_err(|e| {
                 error!("Failed to call function: {:#?}", e);
-                StarknetRpcApiError::ContractError
+                StarknetRpcApiError::from(e)
             })?
             .map_err(|e| {
                 error!("Failed to simulate L1 Message: {:?}", e);