Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

fix(providers): QuorumProvider zero-parameter json Value handling #1613

Merged
merged 2 commits into from
Aug 22, 2022
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
32 changes: 27 additions & 5 deletions ethers-providers/src/transports/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,19 @@ use std::{
};
use thiserror::Error;

/// Helper type that can be used to pass through the `params` value.
/// This is necessary because the wrapper provider is supposed to skip the `params` if it's of
/// size 0, see `crate::transports::common::Request`
#[derive(Debug)]
enum MockParams {
Value(Value),
Zst
}

#[derive(Clone, Debug)]
/// Mock transport used in test environments.
pub struct MockProvider {
requests: Arc<Mutex<VecDeque<(String, Value)>>>,
requests: Arc<Mutex<VecDeque<(String, MockParams)>>>,
responses: Arc<Mutex<VecDeque<Value>>>,
}

Expand All @@ -28,14 +37,19 @@ impl Default for MockProvider {
impl JsonRpcClient for MockProvider {
type Error = MockError;

/// Pushes the `(method, input)` to the back of the `requests` queue,
/// Pushes the `(method, params)` to the back of the `requests` queue,
/// pops the responses from the back of the `responses` queue
async fn request<T: Serialize + Send + Sync, R: DeserializeOwned>(
&self,
method: &str,
input: T,
params: T,
) -> Result<R, MockError> {
self.requests.lock().unwrap().push_back((method.to_owned(), serde_json::to_value(input)?));
let params = if std::mem::size_of::<T>() == 0 {
MockParams::Zst
} else {
MockParams::Value(serde_json::to_value(params)?)
};
self.requests.lock().unwrap().push_back((method.to_owned(), params));
let mut data = self.responses.lock().unwrap();
let element = data.pop_back().ok_or(MockError::EmptyResponses)?;
let res: R = serde_json::from_value(element)?;
Expand All @@ -53,7 +67,15 @@ impl MockProvider {
) -> Result<(), MockError> {
let (m, inp) = self.requests.lock().unwrap().pop_front().ok_or(MockError::EmptyRequests)?;
assert_eq!(m, method);
assert_eq!(serde_json::to_value(data).expect("could not serialize data"), inp);
assert!(!matches!(inp, MockParams::Value(serde_json::Value::Null)));
if std::mem::size_of::<T>() == 0 {
mattiekat marked this conversation as resolved.
Show resolved Hide resolved
assert!(matches!(inp, MockParams::Zst));
} else if let MockParams::Value(inp) = inp {
assert_eq!(serde_json::to_value(data).expect("could not serialize data"), inp);
} else {
unreachable!("Zero sized types must be denoted with MockParams::Zst")
}

Ok(())
}

Expand Down
45 changes: 35 additions & 10 deletions ethers-providers/src/transports/quorum.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use std::{
fmt,
fmt::Debug,
future::Future,
pin::Pin,
Expand Down Expand Up @@ -166,7 +165,7 @@ impl<T: JsonRpcClientWrapper> QuorumProvider<T> {
/// This is the minimum of all provider's block numbers
async fn get_minimum_block_number(&self) -> Result<U64, ProviderError> {
let mut numbers = join_all(self.providers.iter().map(|provider| async move {
let block = provider.inner.request("eth_blockNumber", serde_json::json!(())).await?;
let block = provider.inner.request("eth_blockNumber", QuorumParams::Zst).await?;
serde_json::from_value::<U64>(block).map_err(ProviderError::from)
}))
.await
Expand All @@ -181,7 +180,13 @@ impl<T: JsonRpcClientWrapper> QuorumProvider<T> {
}

/// Normalizes the request payload depending on the call
async fn normalize_request(&self, method: &str, params: &mut Value) {
async fn normalize_request(&self, method: &str, q_params: &mut QuorumParams) {
let params = if let QuorumParams::Value(v) = q_params {
v
} else {
// at this time no normalization is required for calls with zero parameters.
return
};
Comment on lines +186 to +189
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is correct, since no params

match method {
"eth_call" |
"eth_createAccessList" |
Expand Down Expand Up @@ -364,8 +369,8 @@ impl From<QuorumError> for ProviderError {

#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
pub trait JsonRpcClientWrapper: Send + Sync + fmt::Debug {
async fn request(&self, method: &str, params: Value) -> Result<Value, ProviderError>;
pub trait JsonRpcClientWrapper: Send + Sync + Debug {
async fn request(&self, method: &str, params: QuorumParams) -> Result<Value, ProviderError>;
}
type NotificationStream =
Box<dyn futures_core::Stream<Item = Box<RawValue>> + Send + Unpin + 'static>;
Expand All @@ -381,22 +386,28 @@ pub trait PubsubClientWrapper: JsonRpcClientWrapper {
#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
impl<C: JsonRpcClient> JsonRpcClientWrapper for C {
async fn request(&self, method: &str, params: Value) -> Result<Value, ProviderError> {
Ok(JsonRpcClient::request(self, method, params).await.map_err(C::Error::into)?)
async fn request(&self, method: &str, params: QuorumParams) -> Result<Value, ProviderError> {
let fut = if let QuorumParams::Value(params) = params {
JsonRpcClient::request(self, method, params)
} else {
JsonRpcClient::request(self, method, ())
gakonst marked this conversation as resolved.
Show resolved Hide resolved
};

Ok(fut.await.map_err(C::Error::into)?)
}
}
#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
impl JsonRpcClientWrapper for Box<dyn JsonRpcClientWrapper> {
async fn request(&self, method: &str, params: Value) -> Result<Value, ProviderError> {
async fn request(&self, method: &str, params: QuorumParams) -> Result<Value, ProviderError> {
self.as_ref().request(method, params).await
}
}

#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
impl JsonRpcClientWrapper for Box<dyn PubsubClientWrapper> {
async fn request(&self, method: &str, params: Value) -> Result<Value, ProviderError> {
async fn request(&self, method: &str, params: QuorumParams) -> Result<Value, ProviderError> {
self.as_ref().request(method, params).await
}
}
Expand Down Expand Up @@ -437,7 +448,12 @@ where
method: &str,
params: T,
) -> Result<R, Self::Error> {
let mut params = serde_json::to_value(params)?;
let mut params = if std::mem::size_of::<T>() == 0 {
// we don't want `()` to become `"null"`.
QuorumParams::Zst
} else {
QuorumParams::Value(serde_json::to_value(params)?)
};
self.normalize_request(method, &mut params).await;

let requests = self
Expand Down Expand Up @@ -556,6 +572,15 @@ where
}
}

/// Helper type that can be used to pass through the `params` value.
/// This is necessary because the wrapper provider is supposed to skip the `params` if it's of
/// size 0, see `crate::transports::common::Request`
#[derive(Clone)]
pub enum QuorumParams {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't need to expose probably

Suggested change
pub enum QuorumParams {
enum QuorumParams {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sadly we do because the wrapper is also pub. Oh, BTW, I have a bone to pick about this because if you use ethers::prelude::* it currently means the JsonRpcClientWrapper creates a conflict on the request function forcing you to call it via JsonRpcClient::request(...) or just not use ethers::preulde::*,

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see,

that's actually a good point.

imo quorum should not be reexported via prelude::*

Value(Value),
Zst
}

#[cfg(test)]
#[cfg(not(target_arch = "wasm32"))]
mod tests {
Expand Down