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

EIP-1193: Add close method #2261

Closed
wants to merge 2 commits into from
Closed

Conversation

pedrouid
Copy link
Contributor

@pedrouid pedrouid commented Sep 5, 2019

Add the close method to the Ethereum provider API to allow Dapps to close the connection.

The motivation behind this change is to allow Dapps to switch between providers and closing the connection with first one before opening the connection with the second one.

Example: User enables connection with injected provider but then wants to disconnect to connect to another provider like WalletConnect, Portis or Fortmatic

@pedrouid
Copy link
Contributor Author

pedrouid commented Sep 5, 2019

Already have a deployed implementation for this with WalletConnect web3-provider package
https://www.npmjs.com/package/@walletconnect/web3-provider#provider-methods

@eip-automerger
Copy link

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):

@alcuadrado
Copy link
Member

alcuadrado commented Dec 2, 2019

I think the CI failing here is unrelated to the actual change being made. Rerunning it should fix it, but I can't do that. Can you, Pedro?

Anyway, I think this PR should be merged. As I mentioned in this EIP'ss issue, this was discussed during Devcon V and we agreed on implementing this change.

@ryanio
Copy link
Contributor

ryanio commented Dec 3, 2019

I believe the original vision from @frozeman was to keep this provider as a pure simple clean wrapper over the Ethereum JSON-RPC API. It was not intended to have any methods, other than send(), and any methods should be proposed as extensions to the JSON-RPC API.

This is why ethereum.enable() was updated to ethereum.send('eth_requestAccounts') early on.

Any messages from the JSON-RPC API are to be emitted over key notification as a single channel for inbound notifications, messages, subscriptions, requests, etc.

Emitting: ethereum.emit('notification')
Listening: ethereum.on('notification', (message) => {})

@frozeman urged that should be the maximum extent for this API, along with suggesting that it would be of great accessibility to have this wrapper available on window.ethereum so js dapps could be written once and last forever.

I only meant to add notifications connect, close, networkChanged (should now be deprecated for chainChanged) for convenience, from the perspective of having developed dapps and those were some of the most important events I would want in terms of updating the UI for the sake of having a reactive user experience, and it was not difficult at all to implement in Mist at the time so I figured it would not be an unreasonable ask to other provider teams; MetaMask and Token.im among others were on board.


cc #2319

@ryanio
Copy link
Contributor

ryanio commented Dec 3, 2019

In the case of this PR, I would suggest proposing it as new methods to the JSON-RPC API such as net_close and net_open.

I think other useful ones based on prior discussion could include net_changeChain and net_getCapabilities.

@pedrouid
Copy link
Contributor Author

Replaced by #2586

@pedrouid pedrouid closed this Apr 27, 2020
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.

5 participants