Object Safe Ethers Middleware

[gakonst]

Is your feature request related to a problem? Please describe.

The middleware stack right now, while flexible, is not object safe. This means that doing conditional instantiation of middlewares can be hard

Describe the solution you'd like

A trait design which is both Object Safe, and is nicely extensible the way it is right now, allowing consumers to only override the methods they specifically want, with everything else being delegated to the fn inner().

Describe alternatives you've considered

@prestwich has a very promising candidate design here (big shoutout as this has been discussed for months): https://github.com/prestwich/ethers-middleware-experimental

[oliver-giersch]

I have a different and somewhat bold proposal, which is likely going to be controversial as it would break existing APIs quite extensively and definitely deserves further discussion. The gist of it is basically this:

Make the JsonRpcClient and PubSubClient traits object-safe, deprecate the Middleware trait entirely and replace its primary use cases with wrapper structs around Provider and extension traits.

Why?

To me, the design of the Middleware trait feels very un-rustlike, like something out of an object-oriented language clumsily adapted for rust. It provides a large subset of the Ethereum JSONRPC API, some additional higher-level abstractions (like send_escalating), all mixed up together in a single trait. For most of these methods there is not really a conceivable use-case for ever overriding them. Some more random pain points, which could probably extended by a lot more I am currently not thinking of:

1. The "base" (innermost) implementation in Provider of sign_transaction just returns error, to me this indicates that this method does not belong at this level of abstraction.
2. The trait implementation often obscures what is going on (ever Ctrl+clicked on any of these methods to see what's going on? Well, bad luck cause all you get to see is the default impl in the trait forwarding to inner)

Alternative

First, I would redesign the two most low-level transport-related traits as follows:

// errors are boxed, to keep the return slot small
pub type RequestPayload = Result<Box<RawValue>, Box<TransportErr>>;

pub trait Transport {
    fn request_id(&self) -> u64;
    // without async fn in traits yet
    fn send_request(&self, request: Box<RawValue>) -> Pin<Box<dyn Future<Output = RequestPayload>>>;
}

This design trades convenience in the base trait due to generics for full object safety. Note, that convenience can be easily added on top with the help of extension traits and blanket impls, e.g. like so (generics in extension traits are ok, as long as the base trait does not use any):

pub struct Request<'a, T> {
    pub id: u64,
    pub method: &'a str,
    pub params: T,
}

impl<'a, T: Serialize> Request<'a, T> {
    pub fn to_json(&self) -> Box<RawValue> {
        todo!()
    }
}

pub trait TransportExt: Transport {
    fn build_request<'a, T>(&self, method: &'a str, params: T) -> Request<'a, T> {
        Request { id: self.request_id(), method, params }
    }
}

impl<T: Transport> TransportExt for T {}

There should also be a blanket impl for pointer/reference types:

impl<T, D> Transport for D
where
    T: Transport,
    D: Deref<Target = T>,
{
    // ...
}

That way, any of the following instantiations for a hypothetical type like EthClient (which could e.g. only expose the "pure" JSONRPC API) would be equally possible, allowing maximum flexibility:

pub struct EthClient<T> {
    // ...
}

type A = EthClient<Ipc>; // e.g. when no sharing is required at all, note that Ipc does not need to be `Clone`
type B<'a> = EthClient<&'a Ipc>; 
type C = EthClient<Arc<Ws>>; // when sharing is required
type D = EthClient<Arc<dyn Transport>>; // object-safe!
type E = EthClient<Rc<LocalHttp>>; // even thread-local impls are possible

An analogue definition of the PubSubClient trait would look similar, but I haven't fully thought it through, yet. Imho, subscribe should return an (U256, mpsc::UnboundedReceiver<RequestPayload>) tuple and everything else would be handled in more concrete and type safe impls on top of that, e.g. wrap it in some stream type.

Here is an example how the JSONRPC API could be implemented using these trait definitions:

impl<T: Transport> Client<T> {
    // async fn outside of trait, much nicer to use currently
    pub async fn get_balance(&self, address: &Address) -> Result<U256, Box<ClientErr>> {
        // uses the extension trait, note that only a single allocation is required and a reference
        // to the address is sufficient
        let request = self.transport.build_request("eth_getBalance", [address]).to_json();
        let result = self.transport.send_request(request).await.map_err(|err| {
            let context = "failed `eth_getBalance` call, failed RPC call".into();
            ClientErr::transport(err, Some(context))
        })?;

        let balance: U256 = serde_json::from_str(result.get()).map_err(|err| {
            let context = "failed `eth_getBalance` call, unable to deserialize `U256`".into();
            ClientErr::json(err, Some(context))
        })?;

        Ok(balance)
    }
}

impl<T: DuplexTransport> EthClient<T> {
    // all subscription-related functionality goes here
}

Note, that the error handling is just examplary here, but in my mind every failure should allow the caller to diagnose what precisely went wrong in which call.

Replacing the Middleware API

As far as I can see, the primary benefit of the current API is that it allows transparently composing different wrappers, such as a SignerMiddleware with a NonceManager. I don't have concrete designs for replacing this functionality, yet. If I get some positive signals on this proposal I could probably start coming up with some proof-of-concept stuff, but I think all of this can be more easily achieved using either simple wrapper structs or some pretty focused extra traits for specific functionality (with only one or two methods each). E.g., a SignerEthClient could wrap an EthClient and a (generic) Signer, expose all adapted methods directly and everything else by returning a reference to the inner client (client.inner().get_block(..).await?), all very explicitly. Signing-related functions should probably just take a reference to a Signer as an argument.

Further Use Cases

So far, I only came up with one other use case and that is implementing custom (non-standard) geth APIs, which could be easily integrated by writing an extension trait and implementing it for EthClient or, alternatively, providing yet another wrapper struct.

[mattsse]

thanks for this write-up,
this has been bugging us for a while now, and I fully agree that the current delegate approach is a bit horrible.

First priority is definitely making the Transport traits object safe. since we only deal with RPC requests making this the main type the transport deals with sending requests and receiving responses.

What's probably missing here is a, which I think should work, even if T : Serialize

impl TransportExt for dyn T {}

This is great because it removes the requirement to directly pass a raw value or worse a value to the transport.

That should cover Transports (Http, Ws, Ipc) and yeah a pubsub transport would return a Stream type instead.

This would of course require changes to the middleware which would now have to handle deserializing a response, which is fine, we could solve this similar to reqwest with a json() ext function or something, so no big deal since the response type of an rpc method is known.

A big benefit the current Middleware trait has is its default impls, which basically delegates everything to the provider which makes implementing custom Middlewares, like SignerMiddleware very easy.

imo we'd need a trait akin to the Middleware otherwise you wouldn't be able to simply write custom providers that for example modify a transaction, signing, or preparing flashbots bundles
https://github.com/onbjerg/ethers-flashbots/blob/master/src/middleware.rs#L222 or something that only allows transactions that match a certain policy etc..

but there's a pattern for stacking a bunch of Middlewares in a different way like

pub struct Provider<T:Transport> {
   transport :T, // or even `Arc<dyn Transport>`?
   stack: Box<[Arc<dyn Middleware>]>, // assuming the stack will be determined when the provider is created
}

impl Provider {
 
   async fn send_transaction(&self, tx: impl Into<Transaction) -> Result<...> {
       let next = Next::new(&self.transport, &self.stack);
       next.send_transaction(tx.into().await
   }
}


#[derive(Clone)]
pub struct Next<'a> {
    transport: &'a Transport // maybe even Arc<dyn Transport> 
    middlewares: &'a [Arc<dyn Middleware>],
}

impl<'a> Next<'a> {

 pub async fn send_transaction(
        mut self,
        tx: Transaction,
    ) -> Result<...> {
        if let Some((current, rem)) = self.middlewares.split_first() {
            self.middlewares = rem;
            current.send_transaction(req, self).await
        } else {
            let request = self.transport.build_request("eth_sendTransaction", [tx]).to_json();
            self.transport.send_request(req).await
        }
    }

}

Basically peeling off the middleware layers of the stack.
This would mean the user would only interact with the Provider struct which can even be non generic, and the Middleware trait would include the next param which is wrapper for the stack of middleware.
I think this would even allow for default impls for every function, which would be the else branch (empty stack).

also tagging @prestwich here

[oliver-giersch]

First priority is definitely making the Transport traits object safe. since we only deal with RPC requests making this the main type the transport deals with sending requests and receiving responses.

What's probably missing here is a, which I think should work, even if T : Serialize

impl TransportExt for dyn T {}

This is great because it removes the requirement to directly pass a raw value or worse a value to the transport.

Sorry, I am not entirely sure what you meant by this, but I assume you intended something like

impl TransportExt for dyn Transport + '_ {}

I just checked and you are correct, this would indeed be required for instantiations like EthClient<Arc<dyn Transport>> to work.
I would not worry too much about Transport taking a RawValue, since the traits API is not really meant for public consumption and even internally, it would just always be called with a value generated by TransportExt::build_request, so it is not easy at all to use incorrectly (it's also comparatively difficult to construct a Box<RawValue>).

There is some debate to be had, though, whether it would make sense to use an even less specific type like &[u8] rather than RawValue and whether to require a boxed value or a reference. With &[u8] you could even send hand written requests if you absolutely have to. Boxing is generally required for Ipc and Ws, since these send the request through a channel to some server task/thread, but not for Http. On the other hand, serializing a struct will always return some heap-allocated memory, anyways, so it seemed like a sensible default choice to me.

A big benefit the current Middleware trait has is its default impls, which basically delegates everything to the provider which makes implementing custom Middlewares, like SignerMiddleware very easy.

imo we'd need a trait akin to the Middleware otherwise you wouldn't be able to simply write custom providers that for example modify a transaction, signing, or preparing flashbots bundles https://github.com/onbjerg/ethers-flashbots/blob/master/src/middleware.rs#L222 or something that only allows transactions that match a certain policy etc..

To me, an all-encompassing trait like Middleware that is meant to be stacked several layers deep is not the right kind of abstraction for this, but I need to think some more about how to word this convincingly and find some examples and alternatives, so I will address this later.

[gakonst]

Thanks for the input / brainstorm @oliver-giersch. I am +1 w Seitz re easy extensibility of Middleware trait. Curious to hear what other abstraction you think can satisfy our needs..

[oliver-giersch]

Ok, so I managed to get an overview over the full extent of the Middleware trait. Following is a list of all its methods, less their arguments, generics and return types.

pub trait Middleware {
    //////////// 1) Middleware trait related methods (3) ///////////////////////////////////////////
    fn inner(&self);
    fn provider(&self);
    fn default_sender(&self);
    async fn is_signer(&self) -> ..;

    //////////// 2) Higher-level abstractions (13) /////////////////////////////////////////////////
    async fn fill_transaction(&self, ..) -> ..;
    async fn send_transaction(&self, ..) -> ..;
    async fn send_escalating(&self, ..) -> ..;
    async fn resolve_name(&self, ..) -> ..;
    async fn lookup_address(&self, ..) -> ..;
    async fn resolve_avatar(&self, ..) -> ..;
    async fn resolve_nft(&self, ..) -> ..;
    async fn resolve_field(&self, ..) -> ..;
    async fn sign_transaction(&self, ..) -> ..;
    async fn watch(&self, ..) -> ..;
    async fn watch_pending_transactions(&self) -> ..;
    async fn watch_blocks(&self) -> ..;

    //////////// 3) Low-Level Geth API (46) ////////////////////////////////////////////////////////
    async fn client_version(&self) -> ..;
    async fn get_block_number(&self) -> ..;
    async fn get_block(&self, ..) -> ..;
    async fn get_block_with_txs(&self, ..) -> ..;
    async fn get_uncle_count(&self, ..) -> ..;
    async fn get_uncle(&self, ..) -> ..;
    async fn get_transaction_count(&self, ..) -> ..;
    async fn estimate_gas(&self, .) -> ..;
    async fn call(&self, ..) -> ..;
    async fn syncing(&self) -> ..;
    async fn get_chainid(&self) -> ..;
    async fn get_net_version(&self) -> ..;
    async fn get_balance(&self, ..) -> ..;
    async fn get_transaction(&self, ..) -> ..;
    async fn get_transaction_receipt(&self, ..) -> ..;
    async fn get_block_receipts(&self, ..) -> ..;
    async fn get_gas_price(&self) -> ..;
    async fn get_accounts(&self) -> ..;
    async fn send_raw_transaction(&self, ..) -> ..;
    async fn sign(&self, ..) -> ..;
    async fn get_logs(&self, ..) -> ..;
    async fn new_filter(&self, ..) -> ..;
    async fn uninstall_filter(&self, ..) -> ..;
    async fn get_code(&self, ..) -> ..;
    async fn get_storage_at(&self, ..) -> ..;
    async fn get_proof(&self, ..) -> ..;
    async fn txpool_content(&self) -> ..;
    async fn txpool_inspect(&self) -> ..;
    async fn txpool_status(&self) -> ..;
    async fn trace_call(&self, ..) -> ..;
    async fn trace_call_many(&self, ..) -> ..;
    async fn trace_raw_transaction(&self, ..) -> ..;
    async fn trace_replay_transaction(&self, ..) -> ..;
    async fn trace_replay_block_transactions(&self, ..) -> ..;
    async fn trace_block(&self, ..) -> ..;
    async fn trace_filter(&self, ..) -> ..;
    async fn trace_get(&self, ..) -> ..;
    async fn trace_transaction(&self, ..) -> ..;
    async fn parity_block_receipts(&self, ..) -> ..;
    async fn subscribe(&self, ..) -> ..;
    async fn unsubscribe(&self, ..) -> ..;
    async fn subscribe_blocks(&self) -> ..;
    async fn subscribe_pending_txs(&self) -> ..;
    async fn subscribe_logs(&self, ..) -> ..;
    async fn fee_history(&self, ..) -> ..;
    async fn create_access_list(&self, ..) -> ..;

So there is roughly 3 categories of methods. There are low-level functions, which more or less directly map to some JSONRPC API exposed by geth/parity in a type-safe manner (category 3).
Then there is another set of functions, which generally are abstractions built on on-top of one or more low level API functions (category 2), not all of which are really related to one another. Finally, there are some functions which are only relevant because of the specific design of the trait itself, which I will not consider further here.

I also went and took a look at all the custom middlewares in ether-middlewares crate to see, which of the above ~50 methods are most commonly overridden. As I expected, the most commonly overridden methods are from the 2nd category, such as fill_transaction, send_transaction, sign_transaction or estimate_eip1559_fees. Overrides of methods from the 3rd category are not very common and (at least to me) appear to be semantically dubious in practically all cases.
For instance, the SignerMiddlerware overrides the low-level sign method, which otherwise maps to the eth_sign JSONRPC call, to use its local signer instead, which I would consider unexpected behaviour. It also overrides a number of methods low-level methods that take a transaction to set its from address before delegating them, which to me seems like something that should be the responsibility of some other function like fill_transaction.

The flashbots example mentioned by @mattsse I would also call semantically dubious, i.e., a misuse of the Middleware trait. It overrides the send_raw_transaction method, but sending a flashbots bundle I would claim is semantically a different action from sending a regular Ethereum transaction. These two actions are only remotely similar, in that they both return a pending transaction hash. Therefore, this should use a different construct with its own, separate API, imo.

Most "legitimate" uses of this trait seem to revolve around providing a nice wrapper for sending transactions without having to fiddle with nonces, gas limits, gas prices and so on, with customizable behavior such as using different gas oracles and this could imo be realized with a much simpler design. I am thinking of something that roughly handles like the following:

let etherscan = Etherscan::new(..);

let txn_sender = SenderBuilder::new(provider)
    .signer(LocalWallet::try_new(..)?) // no signer: sign transactions by RPC call
    .gas_oracle(etherscan) // generic, takes anything that impls GasOracle
    .nonce_manager(LocalNonceManager::new())
    .build();

let mut txn = TransactionRequest::new(...).value(100).to(..);
txn_sender.fill_transaction(&mut txn).await?; // sets from address, gas limits, gas prices, ..
let _ = txn_sender.send_transaction(txn).await?.await?;

The whole gas escalator stuff seems sufficiently specialized to warrant yet another separate wrapper struct around such a hypothetical TransactionSender, similar to the flashbots bundle sender.

Ofc, I will freely admit that such a design is more limiting than and not as customizable as the current design, I am, however, doubtful, whether this customizability does in fact justify the resulting complexity, given my perceived lack for any real use-cases beyond the ones I have addressed above (please correct me if I am wrong on that).

For now, my incremental proposal would be, to implement all of the category 3 methods as inherent methods of the Provider type (I know this name comes from the ethers-js package, but it would be cool if it could be renamed to something more specific at some point). The Middleware trait could be moved as-is to the ethers-middleware crate and be implemented for Provider there, delegating all low-level methods to the respective inherent methods of this type. This would provide a somewhat cleaner separation of low-level and high-level APIs without causing to much breakage, at least for those people who mostly want access to the former.

Whether to split up the Middleware trait or not could then be decided at a later point. The way I currently see it, a split into some sort of TransactionSender (+ builder), a GasEscalator and a bunch of freestanding utility functions (e.g. resolve_nft) might be sensible approach.

[mattsse]

thanks for this write up,
I thought about this a bit.

I think it makes sense how clustered it, what we'll probably end up with is

• Higher level abstractions
• Eth API, supported by all clients
• Client specific functions like debug_traceTransaction on geth for example

let txn_sender = SenderBuilder::new(provider)
    .signer(LocalWallet::try_new(..)?) // no signer: sign transactions by RPC call
    .gas_oracle(etherscan) // generic, takes anything that impls GasOracle
    .nonce_manager(LocalNonceManager::new())
    .build();

would this be equivalent to the final Provider type?
it looks like this then will be a type that looks like

struct Provider {
  tx,
  gas,
  nonce,
}

?

this could be a bit tricky because if we have multiple handlers for the endpoints, I can image you'll end up using the gas and nonce types within tx type as well?

I think I understand and we bootstrap this process with:

struct Provider {
   pub async fn get_balance() {}
   //... basically _all_ functions we have
}

trait Middleware {
 // all higher level abstraction functions +  eth api
}

trait Geth: Middleware {
 // geth specific endpoints
}

impl MiddleWare for Provider {
// delegating all functions
}

I guess this way we could also get rid of lots of generic M: Middleware and use Provider instead?

[gakonst]

This is an amazing analysis - thank you @oliver-giersch

The trait Geth: Middleware approach also exist in @prestwich's abstraction FYI. I don't think I yet understand what are the benefits of the approach above over the one in the OP, so would appreciate a comparison with that as well!

[prestwich]

The trait Geth: Middleware approach also exist in @prestwich's abstraction FYI. I don't think I yet understand what are the benefits of the approach above over the one in the OP, so would appreciate a comparison with that as well!

After implementing the experimental middleware, I think we get relatively little from this amount of abstraction. there's essentially no user-driven need for distinguishing between Geth and Parity/whatever at compile time, and working it into the type system adds a lot of complexity and mental overhead

[mattsse]

yeh, sounds like we can replace the Middleware trait with the Provider type then

[oliver-giersch]

let txn_sender = SenderBuilder::new(provider)
    .signer(LocalWallet::try_new(..)?) // no signer: sign transactions by RPC call
    .gas_oracle(etherscan) // generic, takes anything that impls GasOracle
    .nonce_manager(LocalNonceManager::new())
    .build();

would this be equivalent to the final Provider type? it looks like this then will be a type that looks like

struct Provider {
  tx,
  gas,
  nonce,
}

?

this could be a bit tricky because if we have multiple handlers for the endpoints, I can image you'll end up using the gas and nonce types within tx type as well?

No, the resulting TransactionSender type would be wrapper around a Provider with a bunch of additional generic fields for customizing the was transaction are prepared before being sent as "raw" transactions, so it's internal makeup would look more like this:

pub struct TransactionSender<T, S, G, N> {
    provider: Provider<T>,
    signer: S
    gas_oracle: G,
    nonce_manager: N,
    // + potential other fields I haven't considered yet
}

impl<T: Transport, S: Signer, G: GasOracle, N: NonceManager> {
    pub async fn fill_transaction(&self, txn: &mut TransactionRequest) -> Result<(), Error> {
        if txn.gas_price.is_none() {
            // use gas oracle to get an appropriate value (simplified, needs provisions for Eip1159 txns ofc
        }

        if txn.gas.is_none() {
            // use `provider` to call `eth_estimateGas`
        }
        // ...and so on
    }
}

Something along those lines, anyways...

It would probably be possible to emulate the different API namespaces by introducing thin wrapper types like this:

pub struct Provider<T> {
    transport: T,
}

impl<T: Transport> Provider<T> {
    pub fn debug(&self) -> DebugNamespace<'_, T> { ... }
}

pub struct DebugNamespace<'a, T> {
    transport: &'a T
}

impl<T: Transport> DebugNamespace<'_, T> {
    pub async fn trace_transaction(&self, ..)  {}
    pub async fn trace_replay_transaction(&self, ..) {}
    // ..
}

So all methods would be called liked this:

let balance = provider.eth().get_balance(&addr).await?;
let trace = provider.debug().trace_transaction(txn).await?;

There is something to it, but it might also obscure things unnecessarily and hamper the discoverability of these methods in the docs, etc.

So with everyone's sentiment being generally favourable, I'd go ahead and prepare a PR that

1. slims down the ethers-providers package to mainly two object-safe transport traits and a Provider type that implements the entirety of the JSONRPC API
2. moves as much as possible including the Middleware trait (as-is) into the ethers-middleware crate

[mattsse]

No, the resulting TransactionSender type would be wrapper around a Provider with a bunch of additional generic fields for customizing the was transaction are prepared before being sent as "raw" transactions, so it's internal makeup

I think this will break a lot of the types that rely on Middleware.
abigen!'s generated contract types look like

   struct MyContract<M:Middleware>(ethers::contract::Contract<M>)

unsure currently how this would look like with the TransactionSender?

I also think the namespace getters aren't very convenient, considering most of the time eth namespace will be used

[prestwich]

I also think the namespace getters aren't very convenient, considering most of the time eth namespace will be used

I am definitely opposed to the namespace getters

meta-point:
ethers.js and ethers-rs are essentially reactions to the poor developer experience of web3.js and web3.rs. I think we shouldn't prioritize type system accuracy over dev ergonomics. namespaces are an example of that, I think, as are highly generic structs

[gakonst]

yeah similarly against namespace getters for the same reason

[oliver-giersch]

Re using namespaces, I agree with the overall sentiment, I just wanted to bring up the possibility. I've begun working on a PR, but I expect that to take a couple of weeks (this is the branch, if anyone wants to take a look).

I've tweaked the proposed API to this for now:

pub type RequestFuture<'a> = Pin<Box<dyn Future<Output = ResponsePayload> + 'a>>;
pub type ResponsePayload = Result<Box<RawValue>, Box<TransportError>>;

pub trait Transport {
    /// Returns a unique request ID.
    fn request_id(&self) -> u64;

    /// Sends a JSON-RPC request to the underlying API provider and returns its
    /// response.
    ///
    /// The caller has to ensure that `id` is identical to the id encoded in
    /// `request` and that the latter represents a valid JSONRPC 2.0 request
    /// whose contents match the specification defined by the Ethereum
    /// [JSON-RPC API](https://eth.wiki/json-rpc/API).
    fn send_raw_request(&self, id: u64, request: String) -> RequestFuture<'_>;
}

/// A trait providing additional convenience methods for the [`Transport`] trait.
pub trait TransportExt: Transport {
    /// Serializes and sends an RPC request for `method` and using `params`.
    ///
    /// In order to match the JSON-RPC specification, `params` must serialize
    /// either to `null` (e.g., with `()`), an array or a map.
    fn send_request<T: Serialize>(&self, method: &str, params: T) -> RequestFuture<'_> {
        let id = self.request_id();
        let request = Request { id, method, params }.to_json();
        self.send_raw_request(id, request)
    }
}

Together with a small private helper function, this makes writing the Provider code very straightforward:

impl<T: Transport> Provider<T> {
    /// Returns the value from a storage position at a given address.
    pub async fn get_storage_at(
        &self,
        address: &Address,
        pos: &U256,
        block: Option<&BlockNumber>,
    ) -> Result<U256, Box<ProviderError>> {
        self.send_request("eth_getStorageAt", (address, pos, block)).await
    }

    // ...
 
    async fn send_request<P, R>(&self, method: &str, params: P) -> Result<R, Box<ProviderError>>
    where
        P: Serialize,
        R: for<'de> Deserialize<'de>,
    {
        // send the request & await its (raw) response
        let raw = self.transport.send_request(method, params).await.map_err(|err| {
            transport_err(err)
                .with_ctx(format!("failed RPC call to `{method}` (rpc request failed)"))
        })?;

        // decode the response to the expected result type
        let decoded = serde_json::from_str(raw.get()).map_err(|err| {
            ProviderError::json(err).with_ctx(format!(
                "failed RPC call to `{method}` (response deserialization failed)"
            ))
        })?;

        Ok(decoded)
    }
}

I am still experimenting with the best way to realize PubSubTransports but I already have a promising approach.
Not quite sure what to do with the Quorum and ReadWrite transports, yet, though.

[gakonst]

@oliver-giersch this is great! Could you also please take a look at @prestwich work in #1215 to make sure you're not doing duplicate work?

[prestwich]

I like this approach. However, one thing I'd suggest is that String -> Box<RawValue> is probably not right for this layer of API. The transport is explicitly for ethereum JSON-RPC so the type system should really enforce that. Something like:

/// The payload of a JSON-RPC request
#[derive(Serialize, Debug, Clone, PartialEq)]
pub struct RawRequest {
    method: &'static str,
    #[serde(skip_serializing_if = "Value::is_null")]
    params: Value,
}

/// A JSON-RPC error response
#[derive(Deserialize, Debug, Clone, PartialEq)]
pub struct JsonRpcError {
    /// The error code
    pub code: i64,
    /// The error message
    pub message: String,
    /// Any error data
    pub data: Option<Value>,
}

/// The raw response payload in a JSON-RPC response
#[derive(Deserialize, Debug, Clone, PartialEq)]
#[serde(untagged)]
pub enum RawResponse {
    /// Success
    Success {
        /// The serialized result
        result: Value,
    },
    /// Error
    Error {
        /// The error body
        error: JsonRpcError,
    },
}


pub type RequestFuture<'a> = Pin<Box<dyn Future<Output = RawResponse> + 'a>>;

pub trait Transport {
    fn send_raw_request(&self, id: u64, request: RawRequest) -> RequestFuture<'_>;
}

[oliver-giersch]

I like this approach. However, one thing I'd suggest is that String -> Box<RawValue> is probably not right for this layer of API. The transport is explicitly for ethereum JSON-RPC so the type system should really enforce that. Something like:

I disagree with using Value at this (lowest) level of the API, imo the added overhead is not worth the marginal increase in type safety. Since a Value is basically a HashMap<String, Value>, there would be at least 7 additional allocations for a regular request instead of just one. That's a bunch of unnecessary overhead for zero gain.

Note that, even though the Transport trait is public, its API is very much only for internal consumption and its design is primarily driven by the constraint of being object-safe (which is basically synonymous with more loosely typed). And with my proposed API, the Transport methods aren't even directly used internally for the most part. The "real" API is in the TransportExt trait, which can use generics without losing object-safety in the base trait (a rather common pattern in rust), and is hence much more type-safe than a Value:

pub trait TransportExt: Transport {
    fn send_request<T: Serialize>(&self, method: &str, params: T) -> RequestFuture<'_>;
}

(even this is not perfectly type-safe, btw, because the JSON-RPC spec only allows null, array or map params iirc, but I doubt geth even checks this explicitly)

BTW, Being a relentless optimizer, I had initially intended to use either Box<[u8]> or Cow<'a, str> for the request parameter, but didn't want to go completely overboard with this stuff ;)

[recmo]

ethers.js and ethers-rs are essentially reactions to the poor developer experience of web3.js and web3.rs. I think we shouldn't prioritize type system accuracy over dev ergonomics. namespaces are an example of that, I think, as are highly generic structs

Ethereum's JSON-RPC also has poor developer experience. This carries over into the devex of the Middleware trait. There is no reason the architecture around Middleware should reflect JSON-RPC.

After trying to improve the robustness and devex of gas oracles I'm starting to think the Middleware trait should be split up in a number of functionally disjunct parts, roughly: ro chain state w/ events, gas oracle, gas estimate/eth call/access list, sign/nonce, gas escalation, tx babysitting/escalation.

[zsluedem]

I just come across this issue when I was fighting again the error type in ethers-rs.

I just want to bring up another perspective on writing the middleware in a composable way instead of a big trait.

I would take tower-rs as a very good example for making a mini composable trait api. The middleware is serving a very simple function which take I in and output O like

trait Middleware<In>{
   type Output;
   fn call(&mut self, in:In) -> Self::Output;
}

The above codes is highly inspired from tower-rs service codes.

Then you could just implement all kinds of Middleware to compose the way you want. Even the user could define the middleware themselves.