-
Notifications
You must be signed in to change notification settings - Fork 26
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
fix: improve convertion errors and handle invalid ABI #387
base: main
Are you sure you want to change the base?
Changes from all commits
663cb2b
7de9da5
8da4aa1
9a5ff62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,19 @@ | ||
use reqwest::{Response, StatusCode}; | ||
use serde::de::DeserializeOwned; | ||
use serde::Deserialize; | ||
use serde_json::json; | ||
use starknet_types_core::felt::Felt; | ||
|
||
use crate::pathfinder::proofs::{PathfinderClassProof, PathfinderProof}; | ||
|
||
#[derive(Debug, thiserror::Error)] | ||
pub enum ClientError { | ||
#[error("Encountered a request error: {0}")] | ||
ReqwestError(#[from] reqwest::Error), | ||
#[error("Encountered a custom error: {0}")] | ||
Msg(String), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
fn jsonrpc_request(method: &str, params: serde_json::Value) -> serde_json::Value { | ||
json!({ | ||
"jsonrpc": "2.0", | ||
|
@@ -19,7 +28,7 @@ async fn post_jsonrpc_request<T: DeserializeOwned>( | |
rpc_provider: &str, | ||
method: &str, | ||
params: serde_json::Value, | ||
) -> Result<T, reqwest::Error> { | ||
) -> Result<T, ClientError> { | ||
let request = jsonrpc_request(method, params); | ||
let response = client.post(format!("{}/rpc/pathfinder/v0.1", rpc_provider)).json(&request).send().await?; | ||
|
||
|
@@ -28,12 +37,21 @@ async fn post_jsonrpc_request<T: DeserializeOwned>( | |
result: T, | ||
} | ||
|
||
let response_text = response.text().await?; | ||
let response: TransactionReceiptResponse<T> = | ||
serde_json::from_str(&response_text).unwrap_or_else(|_| panic!("Error: {}", response_text)); | ||
let response: TransactionReceiptResponse<T> = handle_error(response).await?; | ||
|
||
Ok(response.result) | ||
} | ||
|
||
async fn handle_error<T: DeserializeOwned>(response: Response) -> Result<T, ClientError> { | ||
match response.status() { | ||
StatusCode::OK => Ok(response.json().await?), | ||
s => { | ||
let error = response.text().await?; | ||
Err(ClientError::Msg(format!("Received response: {s:?} Error: {error}"))) | ||
} | ||
} | ||
} | ||
|
||
pub struct PathfinderRpcClient { | ||
/// A raw client to access endpoints not covered by starknet-rs. | ||
http_client: reqwest::Client, | ||
|
@@ -56,7 +74,7 @@ impl PathfinderRpcClient { | |
block_number: u64, | ||
contract_address: Felt, | ||
keys: &[Felt], | ||
) -> Result<PathfinderProof, reqwest::Error> { | ||
) -> Result<PathfinderProof, ClientError> { | ||
post_jsonrpc_request( | ||
&self.http_client, | ||
&self.rpc_base_url, | ||
|
@@ -70,7 +88,7 @@ impl PathfinderRpcClient { | |
&self, | ||
block_number: u64, | ||
class_hash: &Felt, | ||
) -> Result<PathfinderClassProof, reqwest::Error> { | ||
) -> Result<PathfinderClassProof, ClientError> { | ||
log::debug!("querying pathfinder_getClassProof for {:x}", class_hash); | ||
post_jsonrpc_request( | ||
&self.http_client, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,12 +40,13 @@ impl GenericSierraContractClass { | |
} | ||
|
||
fn build_cairo_lang_class(&self) -> Result<CairoLangSierraContractClass, ContractClassError> { | ||
if let Ok(serialized_class) = self.get_serialized_contract_class() { | ||
let contract_class = serde_json::from_slice(serialized_class)?; | ||
return Ok(contract_class); | ||
match self.get_serialized_contract_class() { | ||
Ok(serialized_class) => { | ||
let contract_class = serde_json::from_slice(serialized_class)?; | ||
Ok(contract_class) | ||
} | ||
Err(e) => Err(e), | ||
Comment on lines
+43
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can improve this by using |
||
} | ||
|
||
Err(ContractClassError::NoPossibleConversion) | ||
} | ||
|
||
pub fn get_serialized_contract_class(&self) -> Result<&Vec<u8>, ContractClassError> { | ||
|
@@ -140,7 +141,8 @@ impl TryFrom<&FlattenedSierraClass> for FlattenedSierraClassWithAbi { | |
type Error = serde_json::error::Error; | ||
|
||
fn try_from(sierra_class: &FlattenedSierraClass) -> Result<Self, Self::Error> { | ||
let abi: Option<cairo_lang_starknet_classes::abi::Contract> = serde_json::from_str(&sierra_class.abi)?; | ||
let abi: Option<cairo_lang_starknet_classes::abi::Contract> = | ||
serde_json::from_str(&sierra_class.abi).unwrap_or_default(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @whichqua what default would represent here? |
||
|
||
Ok(Self { | ||
sierra_program: sierra_class.sierra_program.clone(), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I've seen of Rust errors, shouldn't this just be
Reqwest
? The enum is already named "Error".