go-ethereum icon indicating copy to clipboard operation
go-ethereum copied to clipboard

rpc-client: Enable passing single argument to `Call` & improve `Subscribe`

Open ngotchac opened this issue 2 years ago • 4 comments

This PR does 2 things (which could be separated in 2 PRs if needed, currently in 2 commits):

  1. Add a CallContractSingle method to the rpc.Client, which enables calling an RPC server with a single argument instead of an array
  2. Add SubscribeWith method to the rpc.Client, in order to be able to create subscription without the _subscribe suffix

For (1), this is useful because according to specs a method on a JSON-RPC server can be called with params being an array or an object. I didn't add this feature to the server part of the library, since it might be a breaking change.

For (2), this is useful for servers not respecting the _subscribe suffix for subscriptions.

ngotchac avatar Jan 03 '23 15:01 ngotchac

Historically^1, the go-ethereum rpc package has not prioritized name rpc parameters and only positional. I think I prefer the NamedParams approach from that PR over the additional method introduced in this.

As for the Subscribe improvement, do you have examples of servers not supporting the _subscribe suffix? I think the use case is probably pretty niche and I'm not sure it is worthwhile to introduce a new public method to support the functionality. _subscribe is the standard for Ethereum-related rpcs.

lightclient avatar Mar 11 '23 16:03 lightclient

@lightclient Totally understandable. I'm not a huge fan of CallContextSingle introduced in this PR either, but it's much simpler that the PR you linked. And I come from the same place as the author, that is that the jsonrpc library from this repo is the best Go library there is!

Re. the _subscribe, for example Bloxroute let you subscribe to new transactions with a subscribe call (no prefix): https://docs.bloxroute.com/streams/newtxs-and-pendingtxs#requests-gateway-api

ngotchac avatar Mar 11 '23 18:03 ngotchac

Sorry I didn't respond to this earlier. I'm not fundamentally opposed to adding support for named parameters, but the way it's done here is not good. In order to use named parameters, users of the rpc package would need to use this new special method CallContextSingle in a special way, passing the args as a map. That's a lot of friction for such a basic feature.

It'd be much better to support named parameters using a wrapper type like rpc.NamedParams. In order to make a call with named parameters, the user would wrap their args map with this type. The client would recognize this type when creating the JSON-RPC message and encode the args as named.

Regarding the subscription change, it's a niche thing to support. I can't figure out from the BloxRoute documentation whether their subscription API is generally compatible with our client. I also don't have an account to test it with. Did you test this change with BloxRoute?

fjl avatar Aug 22 '23 13:08 fjl

Thanks for the answer @fjl !

So what you're suggesting for NamedParams is what was implemented in https://github.com/ethereum/go-ethereum/pull/22656 right? Why was this eventually closed?

Re. the subscription thing, yeah it's a bit of a niche, but it's also unconventional for this JSON RPC library to impose that every subscription must end with _subscribe. And yes, with these changes, the connection to BloxRoute WS works flawlessly.

ngotchac avatar Aug 23 '23 07:08 ngotchac