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

Add websocket subprovider #189

Closed
danfinlay opened this issue Oct 12, 2017 · 21 comments
Closed

Add websocket subprovider #189

danfinlay opened this issue Oct 12, 2017 · 21 comments

Comments

@danfinlay
Copy link
Contributor

danfinlay commented Oct 12, 2017

This would allow push updates, and compatibility with Web3 1.0.

Updated March 22, 2018 to help potential new contributors make sense of this issue:

Provider engine is a system for composing middleware objects (which we call subproviders) into a potentially complex system for managing the response to a given request object.

ProviderEngine itself is also an Ethereum Provider, as in web3.currentProvider, so once composed with middleware, it exposes the standard sendAsync() method for Ethereum developers to make requests of the JSON RPC API.

When calling providerEngine.sendAsync(yourOpts, callbackFunction), those options you pass in get passed from one middleware object to the next. You can see how here.

As you can see, each provider is passed the same options object, potentially mutating it, and with a pair of callback functions to either end the response immediately, or to pass the options on to the next provider.

To operate as a subprovider, an object only needs to expose a function handleRequest (options, next, end), as you can see here in the fetch subprovider.

In that function, the subprovider can mutate the options freely, and then either call the next() or end() functions.

next() is only used by true "middleware" subproviders, to pass the options to the next subprovider. That function will not be needed for this feature.

The end() function represents the result that will be returned to the ProviderEngine consumer, and should follow the JavaScript API specification, including its JSON-RPC style error format.

The Fetch Subprovider is how MetaMask currently talks to an Ethereum node. It uses the fetch API, which is pure HTTP, to make requests of whatever RPC it is pointed at.

This issue is to create a similar subprovider, but one that uses Websockets instead of HTTP, and uses the Geth Websocket API instead of the usual HTTP-based JSON RPC API. The request/response format should otherwise be basically identical.

Further Goals

There are other goals that are often associated with this one, which can make it seem more complicated, but are actually separate deliverables.

Websocket Block Tracker

Right now the way MetaMask keeps track of the current block is also via HTTP, via the eth-block-tracker module. We could definitely also improve performance by moving that over to websockets, or making a websocket version of it.

Websocket Based Subscriptions

While #207 adds a subprovider for providing the providerEngine.subscribe() API, it does this with polling under the hood. This API would be much more performant if its functionality were moved into the websocket subprovider.

@kumavis
Copy link
Member

kumavis commented Dec 21, 2017

Relevant notes here: MetaMask/metamask-extension#2350 (comment)

id like to differentiate between:

  • subscription support (can be polyfilled over http transport via block-tracker)
  • websocket rpc transport

@benjamincburns
Copy link
Contributor

@danfinlay & @kumavis I'm working on websockets support for ganache right now, which included updating to the latest provider engine. I'd be all kinds of happy to package up the subscription subprovider I'm writing as a PR.

I have it mostly written already, but I need to do some cleanup/refactoring. Do you have any strong preferences for the way it should be done? At the moment it holds a reference to the FilterSubprovider but I'm likely going to nix that and extend that subprovider instead.

At present it works by exposing an EventEmitter interface, which emits subscription updates out on the data event. This will work fine for a websocket provider, but our server in ganache will need to handle routing the subscriptions accordingly based on their ID, cleaning them up on disconnect, etc.

@danfinlay
Copy link
Contributor Author

I’d defer to @kumavis, but the end subproviders should just pass through requests, allowing server-side filter management, unless a middleware like filter subprovider were before it.

@pablasso
Copy link

Isn't this already tackled by #207?

@danfinlay
Copy link
Contributor Author

@pablasso I don't think so. While #207 adds a websocket-based block-subscription subprovider, it only uses websockets for block subscription.

This issue would be for a full websocket subprovider, fully replacing all of the roles currently performed by the fetch subprovider.

@ryan-rowland
Copy link

I'm working on this.

@ryan-rowland
Copy link

Looking closer at this, it seems there's more to do than just replace FetchProvider with a WebsocketProvider. The provider stack seems to be built around a polling mechanism (using eth_getBlockByNumber). To correctly provide websocket support, the information we're receiving from the server should be expected via subscription rather than polling. This means writing a new subscriptions subprovider at least. I believe filters and other subproviders may also be built around this assumption of polling.

@danfinlay
Copy link
Contributor Author

@ryan-rowland You are correct, sorry for not including this in the original post (will update it):

Since provider-engine is reliant on block-polling, there has been an effort to re-write provider-engine without these ethereum-opinionated architectures.

Enter: json-rpc-engine. Basically the same as provider-engine, but without the ethereum-opinionated portions like block-polling.

It would be more correct to write the websocket subprovider for that, and then move MetaMask over to it from provider-engine.

I'm hoping @kumavis can come in and shed additional light on this, since he's the one who's been re-writing provider-engine as json-rpc-engine.

@kumavis
Copy link
Member

kumavis commented Apr 10, 2018

@ryan-rowland you added WebsocketSubprovider (thanks!) in #227 but did not actually setup forwarding subscription responses (server-sent json rpc 'notifications') on the provider-engine. I did the final steps, and I'm releasing in 14.0.0

@kumavis
Copy link
Member

kumavis commented Apr 10, 2018

Fixed in 14.0.0

@kumavis kumavis closed this as completed Apr 10, 2018
@ryan-rowland
Copy link

Thanks for following up @kumavis ! Glad to see this issue moving forward.

@ryan-rowland
Copy link

@lazaridiscom This adds the logic to talk to the websocket gateways, so it's a step forward. The issue I ran into at this point was getting disconnected from the gateway because the provider was still using polling logic rather than subscription. It seems like @kumavis may have updated the logic to use subscriptions, in which case I'd say this is a big step toward web3 1.0 support.

I'll let @kumavis speak to it in more detail as he's been following up on it.

@matthewlilley
Copy link

@lazaridiscom @ryan-rowland

matthewlilley/metamask-extension@6827fdf

This is the simplest solution I came up with for providing Web3 1.0+ subscription support to MetaMask, and it's inpage provider. It works. Needs review. @danfinlay @kumavis

@ryan-rowland
Copy link

@matthewlilley I left comments on your commit. Open a PR next time please. 😄

@lazaridiscom Sorry I won't be a position to test any time this week.

@matthewlilley
Copy link

@ryan-rowland Thank you Ryan.

Pull request MetaMask/metamask-extension@65d907f

@ryan-rowland
Copy link

@matthewlilley You tricked me again! That's a commit, not a PR.

@matthewlilley
Copy link

gislik added a commit to gislik/provider-engine that referenced this issue Aug 28, 2018
kumavis added a commit that referenced this issue Oct 4, 2018
Fix randomly failing filter and subscription tests #189
@jtakalai
Copy link

jtakalai commented Feb 4, 2019

So... does Metamask now support connecting to custom WS-RPC? If I select Custom RPC and enter New RPC URL that starts with ws://, I get error "URIs require the appropriate HTTP/HTTPS prefix."

@d10r
Copy link

d10r commented Nov 20, 2019

@jtakalai no, seems to not yet seem the case.
I guess the issue you need to track is MetaMask/metamask-extension#1645

@danfinlay
Copy link
Contributor Author

Worth noting that MetaMask has converted to a different module we wrote, json-rpc-engine, which can accomplish the same goals in combination with eth-json-rpc-middleware.

It is less coupled to the ethereum RPC, and allows a more modular composition of features.

@danfinlay
Copy link
Contributor Author

Yes, a subprovider was added to provider-engine, but it was never added to MetaMask for a few reasons. Moving off provider-engine was more important to allowing performance than websockets (allowed better block-tracker pausing and cache busting by decoupling them from the main engine).

That work could potentially be ported to json-rpc-engine, but I'm not sure what else might be needed to get that to work. At the very least, this file would need to be moved from eth-json-rpc-infura to a websocket equivalent, but I think there would also be additional work to allow our current filter-middleware (which polyfills subscription behavior with polling) to be deactivated when connected to a websocket source.

There might be other implications, I would ask @kumavis to chime in.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants