Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Web3 1.0 "subscription" support with a polling subprovider #3642

Closed
danfinlay opened this issue Mar 21, 2018 · 36 comments
Closed

Add Web3 1.0 "subscription" support with a polling subprovider #3642

danfinlay opened this issue Mar 21, 2018 · 36 comments
Assignees
Labels
area-background Issues relating to the extension background process. area-injection Relating to how the JS interface is injected into a website. area-provider Relating to the provider module. T08-featureRequest

Comments

@danfinlay
Copy link
Contributor

danfinlay commented Mar 21, 2018

The MVP version of #3639, a refined version of the older/messy #2350.

To allow dapp developers to use web3 1.0 in the least possible time, we could add support for the "subscription" API (provided by websockets normally), even over our current polling-based HTTP connection.

This will involve adding a subscription subprovider (very similar to our current filter subprovider to MetaMask's current provider, which is composed from json-rpc-engine as viewable here. A subscription subprovider already exists in a similar but incompatible form in provider-engine here, and that one can be used as a model.

Major differences between the provider-engine and json-rpc-engine versions:

  • provider-engine subproviders are objects with a handleRequest method. json-rpc-engine subproviders are simple functions.
  • provider-engine handlers take three params: (payload, next, end). json-rpc-engine breaks params into two params: (req, res, next, end).
  • provider-engine included a reference to a block-tracker, json-rpc-engine is unaware of the ethereum protocol, and so subproviders need these data sources passed in as function parameters.

Additionally, these subscriptions should not persist with each individual page, and should clean up themselves. One approach to this we've explored in the past is to initialize this "subscription" not in the main network provider, but in the inpage provider which pipes each page's API requests into the core MetaMask process. This allows the memory management of the "fake" (client-side) subscriptions to benefit from the browser's own content-script memory management.

When this works, users should be able to import web3 1.0 and use the subscription API with no issue, like this:

// Importing the latest version from NPM
const Web3 = require('web3')

// initializing a local instance
const web3 = new Web3(window.web3.currentProvider)
web3.eth.subscribe('logs', {
  address: '0x86fa049857e0209aa7d9e616f7eb3b3b78ecfdb0',  // EOS token is popular atm.
  topics: [], // A token transfer log would be good for now.
}, function (err, result) {
  if (err) throw err
  console.log('Success!', result)
})

I will try to continue refining and updating this spec here on the parent comment to minimize required reading.

Some other optional approaches to solving this issue are reviewed in this comment.

A brief summary of the architecture involved in this feature is in this comment #3639.

Bounty information

Because we have moved our develop branch over to json-rpc-engine, which lacks subscription support, this issue is now a deploy-blocker. We never officially supported the subscription method before, but it was merged in, and since trying to publish our latest version, we've become aware that several major dapps and at least one framework (Drizzle) build on our subscription support. This dramatically increases the urgency of the completion of this feature.

To be paid out, the bounty hunter should:

  • provide a subscription middleware
  • include tests similar to the previous subprovider.
    that at least matches the coverage of the previous implementation.
  • should perform at least as well as the previous implementation.

This is a contest-type bounty, and so multiple devs can take up the cause at once, but we will only pay out an acceptable implementation, so please do communicate as you progress, and ask about any major tradeoffs you're considering making.

We will also be pursuing this feature, but since we have been so slow to complete it, we invite you to race us, and get credit for this valuable feature.

@danfinlay danfinlay added P1-asap area-provider Relating to the provider module. area-injection Relating to how the JS interface is injected into a website. area-background Issues relating to the extension background process. T08-featureRequest labels Mar 21, 2018
@danfinlay danfinlay added this to the April 10 Sprint milestone Mar 21, 2018
@danfinlay danfinlay changed the title Add Web3 1.0 "subscription" support with a minimal polyfill Add Web3 1.0 "subscription" support with an MVP subprovider Mar 21, 2018
@danfinlay danfinlay mentioned this issue Mar 21, 2018
1 task
@danfinlay
Copy link
Contributor Author

To help clarify what would be involved in adding websocket support (optional to this feature, since this can also be achieved with the existing HTTP subprovider), I've enhanced the issue description here:

MetaMask/web3-provider-engine#189

@gitcoinbot
Copy link

This issue now has a funding of 1.0 ETH (527.42 USD @ $527.42/ETH) attached to it.

  • If you would like to work on this issue you can claim it here.
  • If you've completed this issue and want to claim the bounty you can do so here
  • Questions? Get help on the Gitcoin Slack
  • $3549.77 more Funded OSS Work Available at: https://gitcoin.co/explorer

@danfinlay danfinlay changed the title Add Web3 1.0 "subscription" support with an MVP subprovider Add Web3 1.0 "subscription" support with a polling subprovider Mar 23, 2018
@terencechow
Copy link

@danfinlay I'd like to see if I can help on this but I may need some guidance...

I'm a bit confused by the inpage-provider using the json-rpc-engine and the suggestion to use provider-engine/subprovider/subscriptions.js. Those appear to be incompatible with each other? How are those supposed to work together ?

@terencechow
Copy link

I've had a chance to look at the code today and I think I am going to take a step back and let someone else give this a go. This seems to be a lot more complicated than I initially anticipated and i don't know the ethereum ecosystem well enough right now. I'll monitor this and help if I can but for now I don't think Ill be able to help.

@danfinlay danfinlay modified the milestone: April 10 Sprint Mar 26, 2018
@danfinlay
Copy link
Contributor Author

We need a second bounty for doing this for json-rpc-engine.

@danfinlay danfinlay removed this from the April 10 Sprint milestone Mar 26, 2018
@danfinlay
Copy link
Contributor Author

@lazaridiscom I think you linked to the wrong issue.

@matthewlilley
Copy link

@danfinlay I've got subscriptions working out of the box with currentProvider and Web 1.0, based on previous work by benjamincburns. I'll try to get it wrapped up tomorrow if possible.

@matthewlilley
Copy link

@lazaridiscom Sorry, I haven't had a chance to pick this back up again, I'll stop work on this for now since it's Easter and kids are off School, so I'm not going to have much free time for the next week. Benjamincburns has done most of the work for this already. Once the subscription subprovider is added to ZeroProvider, you can subscribe over the RPC stream, and get back a subscription id from the RPC handle method. I can see the successful subscription through the generated background page, but the Web3 subscribe callback seems to get lost somewhere.

@vs77bb
Copy link

vs77bb commented Apr 7, 2018

Howdy @danfinlay: Want me to pub this one out to the community or is it closer to being completed in house? Curious on your feedback here 🤔

@lazaridiscom Good to see you 🙂

@matthewlilley
Copy link

matthewlilley commented Apr 9, 2018

@lazaridiscom @danfinlay I'm back Today so I'll finish this off. Last week I setup a minimal example using the same wiring as metamask extension. I've just tested that setup in metamask extension Today and it's working. I'll submit a PR asap.

@matthewlilley
Copy link

I have a bug with web3.clearSubscriptions, but it seems to be related to web3 itself since it wasn't working with the regular websocket provider either. Other than that, this looks good. I'll make the PRs to the effected repos Today.

@matthewlilley
Copy link

@matthewlilley
Copy link

@lazaridiscom Just build the extension, "npm run start", and add the dist/chrome folder as a Chrome extension.

Then you can use Web 1.0 subscription code like @danfinlay posted in the OP, with the Metamask inpage provider.

@danfinlay
Copy link
Contributor Author

Hi @matthewlilley, my understanding is that @kumavis is currently working on a robust version of this feature, and prefers that solution to this one.

I'm very sorry you've been dragged along here. It was probably unfair to post a bounty on an issue that was so large and we were going to be so picky about the implementation of.

I would be willing to pay out the bounty, because it looks like you have a working version, but I think we're hoping to go full websockets soon, so we might not need to push this to production.

@vs77bb Go ahead and pay out, we shouldn't have made Matthew wait so long.

@matthewlilley
Copy link

matthewlilley commented May 10, 2018

@danfinlay Thanks for getting back to me. I'd assumed you were working on a more thorough solution.

It's okay, I understand. This was merely supposed to provide MVP support, since the transition to a Websocket provider was underway, and I wanted a way to use subscriptions with MetaMask out of the box when building dApps.

Appreciate it!

@gitcoinbot
Copy link

gitcoinbot commented May 14, 2018

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

  1. @matthewlilley

has committed to working on this project to be completed 1 month from now.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work for 1.0 ETH (711.13 USD @ $711.13/ETH) has been submitted by:

  1. @matthewlilley

If you are the bounty funder, please take a look at the submitted work:


@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


The funding of 1.0 ETH (712.71 USD @ $712.71/ETH) attached to this issue has been approved & issued to @matthewlilley.

@leonprou
Copy link

I'm very interested in this feature, where can I see the status? Is there a PR?
Thanks 😄

@danfinlay
Copy link
Contributor Author

Current progress on this feature is resuming on this issue:
#4279

@upperwal
Copy link

I hope #4279 is merged soon but till that time if someone is desperate like me they can use upperwal/web3-event-compatibility

Someone suggested in the comments to use two versions of web3 so I tried that and it worked 💪. So thanks to that guy (sorry don't remember who that was)

If you want to see how it works check out upperwal/react-box. It is a truffle box for react which supports web3 v1.x.x

Refer this for how to use it (supports existing API but only callback) and this to initialise it.

@wjmelements
Copy link
Contributor

More work is still required after #4279. In 4.10 we still get the The current provider doesn't support subscriptions: MetamaskInpageProvider error message when using web3.eth.subscribe.

@pors
Copy link

pors commented Oct 5, 2018

Is there an ETA for subscription support in MetaMask? I don't intend to push, I just need to make decisions for a project I'm working on. With an ETA I know how to move forward. Thanks!

@danfinlay
Copy link
Contributor Author

We inadvertently introduced support for subscriptions in April, but have since introduced a refactor of our provider logic which no longer includes this feature. We are actively working on it now, and I am about to post a bounty to push its urgency up.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 5.0 ETH (1116.53 USD @ $223.31/ETH) attached to it.

@danfinlay
Copy link
Contributor Author

Bounty information (Added to parent post)

Because we have moved our develop branch over to json-rpc-engine, which lacks subscription support, this issue is now a deploy-blocker. We never officially supported the subscription method before, but it was merged in, and since trying to publish our latest version, we've become aware that several major dapps and at least one framework (Drizzle) build on our subscription support. This dramatically increases the urgency of the completion of this feature.

To be paid out, the bounty hunter should:

  • provide a subscription middleware
  • include tests similar to the previous subprovider.
    that at least matches the coverage of the previous implementation.
  • should perform at least as well as the previous implementation.

This is a contest-type bounty, and so multiple devs can take up the cause at once, but we will only pay out an acceptable implementation, so please do communicate as you progress, and ask about any major tradeoffs you're considering making.

We will also be pursuing this feature, but since we have been so slow to complete it, we invite you to race us, and get credit for this valuable feature.

@leonprou
Copy link

leonprou commented Oct 7, 2018

@danfinlay I'm interested in this one, but I feel that I need some help understanding the repos.

Before start, I'me trying to understand the middlewares flow.

Let's take the wallet middleware as example. It defines a dictionary of Ethereum JSON RPC methods.

  • Can I assume that when client uses web3.eth.accounts eventually those methods are called?
  • I see that wallet middleware do just part of the job. Other part of the workflow is given in functions like lookupAccounts, lookupDefaultAccount etc. Can you show me where those methods defined?

@danfinlay
Copy link
Contributor Author

Sorry for any bounty hunters who had interest in this, we have a PR we expect to close this issue:
#5458

@gitcoinbot
Copy link

gitcoinbot commented Oct 23, 2018

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 4 months ago.
Please review their action plans below:

1) liamzebedee has been approved to start work.

I encountered this issue a couple weeks back at work. Currently am connecting to two providers, one for tx's and one for realtime events. Confident that I can approach this issue and fix it, and I'm sure it's a big trouble for many others too.

Learn more on the Gitcoin Issue Details page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-background Issues relating to the extension background process. area-injection Relating to how the JS interface is injected into a website. area-provider Relating to the provider module. T08-featureRequest
Projects
None yet
Development

No branches or pull requests

12 participants