Change anyhow for thiserror #1
Conversation
| pub fn to_bytes(&self) -> Vec<u8> { | ||
| let mut bytes = vec![]; | ||
| bytes.extend(&self.x.len().to_be_bytes()); | ||
| bytes.extend(&self.x); | ||
| bytes.extend(&self.y.len().to_be_bytes()); | ||
| bytes.extend(&self.y); | ||
|
|
||
| bytes | ||
| } |
There was a problem hiding this comment.
why are you deleting all these to_bytes? Were they used for something before?
| pub async fn new(config: EigenConfig, secrets: EigenSecrets) -> anyhow::Result<Self> { | ||
| let private_key = SecretKey::from_str(secrets.private_key.0.expose_secret().as_str()) | ||
| .map_err(|e| anyhow::anyhow!("Failed to parse private key: {}", e))?; | ||
| pub async fn new(config: EigenConfig, secrets: EigenSecrets) -> Result<Self, EigenClientError> { |
There was a problem hiding this comment.
Is using a single huge crate-level error enum like EigenClientError the typical way to go in rust? Reading https://www.reddit.com/r/rust/comments/1bb7dco/error_handling_goodbest_practices/ it seems like positions are mitigated on this issue.
It feels a bit weird to me. Wouldn't it be better to have a specific error enum per method? For eg, one guy says:
As a rule of thumb, yes, errors should be specific to the operation. This ensures that client code doesn't have to be littered with unreachable!() statements for failure cases that are known to be impossible for that operation.
There was a problem hiding this comment.
Having a single error type that encapsulates the possible errors from a library makes it easier to use (specially if you use several modules from the same library together).
We did a small refactor of the error types since your last review. So feel free to take a look again.
| let blob_info = self.get_commitment(blob_id).await?; | ||
| let rlp_encoded_bytes = hex::decode(blob_info)?; | ||
| let blob_info: BlobInfo = rlp::decode(&rlp_encoded_bytes)?; | ||
| let inclusion_data = blob_info.blob_verification_proof.inclusion_proof; | ||
| Ok(inclusion_data) | ||
| } | ||
|
|
||
| fn blob_size_limit(&self) -> Option<usize> { | ||
| pub fn blob_size_limit(&self) -> Option<usize> { |
There was a problem hiding this comment.
Is it necessary to expose this? BLOCK_SIZE_LIMIT is a const right now
src/sdk.rs
Outdated
| @@ -108,10 +113,10 @@ impl RawEigenClient { | |||
| ), | |||
| ) | |||
| .await | |||
| .map_err(|_| anyhow::anyhow!("Failed to verify certificate")) | |||
| .map_err(EigenClientError::from) | |||
There was a problem hiding this comment.
I think you can use ? here to automatically call EigenClientError::from
This PR removes anyhow in favor of thiserror