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

Feature: Subscription support #5458

Merged
merged 6 commits into from
Oct 9, 2018
Merged

Feature: Subscription support #5458

merged 6 commits into from
Oct 9, 2018

Conversation

kumavis
Copy link
Member

@kumavis kumavis commented Oct 8, 2018

This enables partial support for the experimental "web3 1.0 Ethereum JSON RPC subscriptions", covering the two most commonly used subscription types newHeads and logs.

supported subscription types:

changes in deps

Lionshare of the changes appears in the following deps:

testing and QA

While subscription support is tested in the eth-json-rpc-filters module, MetaMask is lacking dapp-level integration tests. It can be QA'd like this:

  • navigate to a dapp like metamask.io and open the console
  • log ALL subscription hits to console
web3.currentProvider.on('data', console.log)

subscribe to newHeads

web3.currentProvider.send({ id:1, method: 'eth_subscribe', params:['newHeads']}, console.log)

subscribe to a log filter (eg DAI txs)

web3.currentProvider.send({ id:1, method: 'eth_subscribe', params:['logs', { address: '0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359'}]}, console.log)

subscribe to all logs (fire hose warning)

web3.currentProvider.send({ id:1, method: 'eth_subscribe', params:['logs', {}]}, console.log)

Relevant Issues and PRs

This PR replaces MetaMask/providers#3
Fixes #5425
Fixes #3642

@kumavis
Copy link
Member Author

kumavis commented Oct 8, 2018

Addressing the "missing provider" test failures...

@metamaskbot
Copy link
Collaborator

Builds ready [a0700de]: mascara, chrome, firefox, edge, opera

@metamaskbot
Copy link
Collaborator

Builds ready [a0700de]: mascara, chrome, firefox, edge, opera

@metamaskbot
Copy link
Collaborator

Builds ready [9bce0ae]: mascara, chrome, firefox, edge, opera

@metamaskbot
Copy link
Collaborator

Builds ready [9bce0ae]: mascara, chrome, firefox, edge, opera

@metamaskbot
Copy link
Collaborator

Builds ready [45feb43]: mascara, chrome, firefox, edge, opera

@metamaskbot
Copy link
Collaborator

Builds ready [45feb43]: mascara, chrome, firefox, edge, opera

@whymarrh
Copy link
Contributor

whymarrh commented Oct 9, 2018

Can we add "fixes #5425" to this PR?

@brunobar79 brunobar79 merged commit 17b9f4c into develop Oct 9, 2018
@danfinlay danfinlay deleted the provider-subs branch October 9, 2018 21:39
@HowardBraham
Copy link
Contributor

@kumavis and @danfinlay, does this change the way one has to write web3 code in a Dapp?

I am using web3.js@v1.0.0-beta.36, and when I do this on Rinkeby:
let contract = new web3.eth.Contract( ...
contract.methods.transfer( ...

After this PR, the UI will show the error:
ALERT: Transaction Error. Exception thrown in contract code.

And deeper, the RPC error when calling eth_estimateGas is:
Error: [ethjs-rpc] rpc error with payload ... TypeError: blockRef.slice is not a function

My PR #5283 depends on this functionality, and I thought possibly it's just that the web3 syntax has changed.

@tmashuang
Copy link
Contributor

@HowardBraham this has been fixed in MetaMask/eth-json-rpc-middleware#6

@HowardBraham
Copy link
Contributor

Ahah, thank you @tmashuang!

@elie222
Copy link

elie222 commented Oct 10, 2018

So with this merged in, is Metamask now compatible with web3 1.0?

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

Successfully merging this pull request may close these issues.

None yet

7 participants