Skip to content

Commit

Permalink
feat(json-rpc): use Cow instead of &'static str for method names (#…
Browse files Browse the repository at this point in the history
…319)

* feat(json-rpc): use `Cow` instead of `&'static str` for method names

* chore: return str
  • Loading branch information
DaniPopes authored Mar 15, 2024
1 parent 889bc61 commit 6fada4c
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 28 deletions.
2 changes: 0 additions & 2 deletions crates/json-rpc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ Requests are sent via transports (see [alloy-transports]). This results in 1 of

### Limitations

- This library models the method name as a `&'static str`, and is therefore
unsuitable for use with RPC servers with dynamic method names.
- This library does not support borrowing response data from the deserializer.
This is intended to simplify client implementations, but makes the library
poorly suited for use in a high-performance server.
23 changes: 12 additions & 11 deletions crates/json-rpc/src/request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ use crate::{common::Id, RpcParam};
use alloy_primitives::{keccak256, B256};
use serde::{de::DeserializeOwned, ser::SerializeMap, Deserialize, Serialize};
use serde_json::value::RawValue;
use std::borrow::Cow;

/// `RequestMeta` contains the [`Id`] and method name of a request.
#[derive(Debug, Clone)]
pub struct RequestMeta {
/// The method name.
pub method: &'static str,
pub method: Cow<'static, str>,
/// The request ID.
pub id: Id,
/// Whether the request is a subscription, other than `eth_subscribe`.
Expand All @@ -16,13 +17,13 @@ pub struct RequestMeta {

impl RequestMeta {
/// Create a new `RequestMeta`.
pub const fn new(method: &'static str, id: Id) -> Self {
pub const fn new(method: Cow<'static, str>, id: Id) -> Self {
Self { method, id, is_subscription: false }
}

/// Returns `true` if the request is a subscription.
pub const fn is_subscription(&self) -> bool {
self.is_subscription || matches!(self.method.as_bytes(), b"eth_subscribe")
pub fn is_subscription(&self) -> bool {
self.is_subscription || self.method == "eth_subscribe"
}

/// Indicates that the request is a non-standard subscription (i.e. not
Expand Down Expand Up @@ -51,12 +52,12 @@ pub struct Request<Params> {

impl<Params> Request<Params> {
/// Create a new `Request`.
pub const fn new(method: &'static str, id: Id, params: Params) -> Self {
Self { meta: RequestMeta::new(method, id), params }
pub fn new(method: impl Into<Cow<'static, str>>, id: Id, params: Params) -> Self {
Self { meta: RequestMeta::new(method.into(), id), params }
}

/// Returns `true` if the request is a subscription.
pub const fn is_subscription(&self) -> bool {
pub fn is_subscription(&self) -> bool {
self.meta.is_subscription()
}

Expand Down Expand Up @@ -146,7 +147,7 @@ where
let sized_params = std::mem::size_of::<Params>() != 0;

let mut map = serializer.serialize_map(Some(3 + sized_params as usize))?;
map.serialize_entry("method", self.meta.method)?;
map.serialize_entry("method", &self.meta.method[..])?;

// Params may be omitted if it is 0-sized
if sized_params {
Expand Down Expand Up @@ -194,8 +195,8 @@ impl SerializedRequest {
}

/// Returns the request method.
pub const fn method(&self) -> &'static str {
self.meta.method
pub fn method(&self) -> &str {
&self.meta.method
}

/// Mark the request as a non-standard subscription (i.e. not
Expand All @@ -205,7 +206,7 @@ impl SerializedRequest {
}

/// Returns `true` if the request is a subscription.
pub const fn is_subscription(&self) -> bool {
pub fn is_subscription(&self) -> bool {
self.meta.is_subscription()
}

Expand Down
9 changes: 5 additions & 4 deletions crates/provider/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use alloy_transport::{BoxTransport, Transport, TransportErrorKind, TransportResu
use alloy_transport_http::Http;
use serde_json::value::RawValue;
use std::{
borrow::Cow,
fmt,
marker::PhantomData,
sync::{Arc, OnceLock},
Expand Down Expand Up @@ -897,7 +898,7 @@ pub trait Provider<N: Network, T: Transport + Clone = BoxTransport>: Send + Sync
/* ---------------------------------------- raw calls --------------------------------------- */

/// Sends a raw JSON-RPC request.
async fn raw_request<P, R>(&self, method: &'static str, params: P) -> TransportResult<R>
async fn raw_request<P, R>(&self, method: Cow<'static, str>, params: P) -> TransportResult<R>
where
P: RpcParam,
R: RpcReturn,
Expand All @@ -909,7 +910,7 @@ pub trait Provider<N: Network, T: Transport + Clone = BoxTransport>: Send + Sync
/// Sends a raw JSON-RPC request with type-erased parameters and return.
async fn raw_request_dyn(
&self,
method: &'static str,
method: Cow<'static, str>,
params: &RawValue,
) -> TransportResult<Box<RawValue>> {
self.client().request(method, params).await
Expand Down Expand Up @@ -1086,7 +1087,7 @@ mod tests {
init_tracing();
let (provider, _anvil) = spawn_anvil();

let num: U64 = provider.raw_request("eth_blockNumber", ()).await.unwrap();
let num: U64 = provider.raw_request("eth_blockNumber".into(), ()).await.unwrap();
assert_eq!(0, num.to::<u64>())
}

Expand Down Expand Up @@ -1128,7 +1129,7 @@ mod tests {
let block = provider.get_block_by_number(tag, true).await.unwrap().unwrap();
let hash = block.header.hash.unwrap();
let block: Block = provider
.raw_request::<(B256, bool), Block>("eth_getBlockByHash", (hash, true))
.raw_request::<(B256, bool), Block>("eth_getBlockByHash".into(), (hash, true))
.await
.unwrap();
assert_eq!(block.header.hash.unwrap(), hash);
Expand Down
2 changes: 1 addition & 1 deletion crates/pubsub/src/managers/in_flight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ impl InFlight {
}

/// Check if the request is a subscription.
pub(crate) const fn is_subscription(&self) -> bool {
pub(crate) fn is_subscription(&self) -> bool {
self.request.is_subscription()
}

Expand Down
3 changes: 1 addition & 2 deletions crates/rpc-client/src/batch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,14 @@ impl<'a, Conn> BatchRequest<'a, Conn>
where
Conn: Transport + Clone,
{
#[must_use = "Waiters do nothing unless polled. A Waiter will never resolve unless the batch is sent!"]
/// Add a call to the batch.
///
/// ### Errors
///
/// If the request cannot be serialized, this will return an error.
pub fn add_call<Params: RpcParam, Resp: RpcReturn>(
&mut self,
method: &'static str,
method: impl Into<Cow<'static, str>>,
params: &Params,
) -> TransportResult<Waiter<Resp>> {
let request = self.transport.make_request(method, Cow::Borrowed(params));
Expand Down
7 changes: 4 additions & 3 deletions crates/rpc-client/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use alloy_json_rpc::{Id, Request, RpcParam, RpcReturn};
use alloy_transport::{BoxTransport, Transport, TransportConnect, TransportError};
use alloy_transport_http::Http;
use std::{
borrow::Cow,
ops::Deref,
sync::{
atomic::{AtomicU64, Ordering},
Expand Down Expand Up @@ -91,7 +92,7 @@ impl<T: Transport> RpcClient<T> {
/// See [`PollerBuilder`] for examples and more details.
pub fn prepare_static_poller<Params, Resp>(
&self,
method: &'static str,
method: impl Into<Cow<'static, str>>,
params: Params,
) -> PollerBuilder<T, Params, Resp>
where
Expand Down Expand Up @@ -198,7 +199,7 @@ impl<T> RpcClientInner<T> {
#[inline]
pub fn make_request<Params: RpcParam>(
&self,
method: &'static str,
method: impl Into<Cow<'static, str>>,
params: Params,
) -> Request<Params> {
Request::new(method, self.next_id(), params)
Expand Down Expand Up @@ -248,7 +249,7 @@ impl<T: Transport + Clone> RpcClientInner<T> {
#[doc(alias = "prepare")]
pub fn request<Params: RpcParam, Resp: RpcReturn>(
&self,
method: &'static str,
method: impl Into<Cow<'static, str>>,
params: Params,
) -> RpcCall<T, Params, Resp> {
let request = self.make_request(method, params);
Expand Down
15 changes: 10 additions & 5 deletions crates/rpc-client/src/poller.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use futures::{Stream, StreamExt};
use serde::Serialize;
use serde_json::value::RawValue;
use std::{
borrow::Cow,
marker::PhantomData,
ops::{Deref, DerefMut},
time::Duration,
Expand Down Expand Up @@ -61,7 +62,7 @@ pub struct PollerBuilder<Conn, Params, Resp> {
client: WeakClient<Conn>,

/// Request Method
method: &'static str,
method: Cow<'static, str>,
params: Params,

// config options
Expand All @@ -79,12 +80,16 @@ where
Resp: RpcReturn + Clone,
{
/// Create a new poller task.
pub fn new(client: WeakClient<Conn>, method: &'static str, params: Params) -> Self {
pub fn new(
client: WeakClient<Conn>,
method: impl Into<Cow<'static, str>>,
params: Params,
) -> Self {
let poll_interval =
client.upgrade().map_or_else(|| Duration::from_secs(7), |c| c.default_poll_interval());
Self {
client,
method,
method: method.into(),
params,
channel_size: 16,
poll_interval,
Expand Down Expand Up @@ -144,7 +149,7 @@ where
/// Starts the poller in a new Tokio task, returning a channel to receive the responses on.
pub fn spawn(self) -> PollChannel<Resp> {
let (tx, rx) = broadcast::channel(self.channel_size);
let span = debug_span!("poller", method = self.method);
let span = debug_span!("poller", method = %self.method);
let fut = async move {
let mut params = ParamsOnce::Typed(self.params);
let mut retries = MAX_RETRIES;
Expand All @@ -165,7 +170,7 @@ where

loop {
trace!("polling");
match client.request(self.method, params).await {
match client.request(self.method.clone(), params).await {
Ok(resp) => {
if tx.send(resp).is_err() {
debug!("channel closed");
Expand Down

0 comments on commit 6fada4c

Please sign in to comment.