From 8218c96205ac2f24cda53cd7760cb3da46aed488 Mon Sep 17 00:00:00 2001 From: Mattie Conover Date: Mon, 22 Aug 2022 19:32:47 -0400 Subject: [PATCH] fix(providers): QuorumProvider zero-parameter json Value handling (#1613) * Correction to QuorumProvider json Value handling with zero-parameter cases * Update to properly pass through when no parameters are specified --- ethers-providers/src/transports/mock.rs | 32 +++++++++++++--- ethers-providers/src/transports/quorum.rs | 45 ++++++++++++++++++----- 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/ethers-providers/src/transports/mock.rs b/ethers-providers/src/transports/mock.rs index 8d387552..2c129b61 100644 --- a/ethers-providers/src/transports/mock.rs +++ b/ethers-providers/src/transports/mock.rs @@ -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>>, + requests: Arc>>, responses: Arc>>, } @@ -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( &self, method: &str, - input: T, + params: T, ) -> Result { - self.requests.lock().unwrap().push_back((method.to_owned(), serde_json::to_value(input)?)); + let params = if std::mem::size_of::() == 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)?; @@ -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::() == 0 { + 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(()) } diff --git a/ethers-providers/src/transports/quorum.rs b/ethers-providers/src/transports/quorum.rs index 976d29c8..918d3058 100644 --- a/ethers-providers/src/transports/quorum.rs +++ b/ethers-providers/src/transports/quorum.rs @@ -1,5 +1,4 @@ use std::{ - fmt, fmt::Debug, future::Future, pin::Pin, @@ -166,7 +165,7 @@ impl QuorumProvider { /// This is the minimum of all provider's block numbers async fn get_minimum_block_number(&self) -> Result { 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::(block).map_err(ProviderError::from) })) .await @@ -181,7 +180,13 @@ impl QuorumProvider { } /// 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 + }; match method { "eth_call" | "eth_createAccessList" | @@ -364,8 +369,8 @@ impl From 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; +pub trait JsonRpcClientWrapper: Send + Sync + Debug { + async fn request(&self, method: &str, params: QuorumParams) -> Result; } type NotificationStream = Box> + Send + Unpin + 'static>; @@ -381,14 +386,20 @@ pub trait PubsubClientWrapper: JsonRpcClientWrapper { #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] #[cfg_attr(not(target_arch = "wasm32"), async_trait)] impl JsonRpcClientWrapper for C { - async fn request(&self, method: &str, params: Value) -> Result { - Ok(JsonRpcClient::request(self, method, params).await.map_err(C::Error::into)?) + async fn request(&self, method: &str, params: QuorumParams) -> Result { + let fut = if let QuorumParams::Value(params) = params { + JsonRpcClient::request(self, method, params) + } else { + JsonRpcClient::request(self, method, ()) + }; + + 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 { - async fn request(&self, method: &str, params: Value) -> Result { + async fn request(&self, method: &str, params: QuorumParams) -> Result { self.as_ref().request(method, params).await } } @@ -396,7 +407,7 @@ impl JsonRpcClientWrapper for Box { #[cfg_attr(target_arch = "wasm32", async_trait(?Send))] #[cfg_attr(not(target_arch = "wasm32"), async_trait)] impl JsonRpcClientWrapper for Box { - async fn request(&self, method: &str, params: Value) -> Result { + async fn request(&self, method: &str, params: QuorumParams) -> Result { self.as_ref().request(method, params).await } } @@ -437,7 +448,12 @@ where method: &str, params: T, ) -> Result { - let mut params = serde_json::to_value(params)?; + let mut params = if std::mem::size_of::() == 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 @@ -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 { + Value(Value), + Zst +} + #[cfg(test)] #[cfg(not(target_arch = "wasm32"))] mod tests {