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

1193: Make spec availability-agnostic #2588

Merged
merged 3 commits into from
Apr 8, 2020

Conversation

rekmarks
Copy link
Contributor

@rekmarks rekmarks commented Apr 7, 2020

File: https://github.com/rekmarks/EIPs/blob/1193-availability/EIPS/eip-1193.md

For the purpose of this PR, let's define the problem of how to expose the Provider API as "availability" or "the availability problem."

Per this comment by @alcuadrado, one of the concerns from the 1193 meeting at Devcon 5 was:

  1. EIP-1193 began as a spec on how to expose a provider in a dapp browser, but mutated into specifying its interface. It still depends on EIPs related to browser-specific stuff.
  2. The way a provider is exposed (i.e. window.ethereum) only supports a single provider. This is in conflict with users installing multiple wallet extensions, and may be problematic with a multi-chain future.

Meanwhile, the proposed solution was:

  1. EIP-1193 will only focus on the provider's interface. How to expose in browser environments will be part of another document, possibly an RFC. Any dependency on browser-specific EIPs will be removed.
  2. As this EIP won't dictate how to expose the provider, this item is outside of its scope.

The goal of stripping all (normative) availability-related language was never fully realized, and recently merged PRs (#2577, #2586) in fact introduced more such language.

This PR makes the spec completely availability-agnostic, while including a non-normative section noting that window.ethereum is, by convention, the de facto standard for exposing the Provider API. Said section notes some limitations of this convention, for the benefit of Provider implementers, dapp developers, and future authors of future EIPs.

Code examples remain unchanged, and use the window.ethereum convention (although note that it is merely a convention).

@rekmarks
Copy link
Contributor Author

rekmarks commented Apr 7, 2020

cc: @adrianmcli @ryanio

@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):

@rekmarks rekmarks mentioned this pull request Apr 7, 2020
@adrianmcli
Copy link

The diff looks good to me! Nice work @rekmarks

@frozeman
Copy link
Contributor

frozeman commented Apr 8, 2020

I like it to be generic. Though we should talk about how a multi EVM chain future can be possible with this provider. E.g. ethereum.chainX.request(), or ethereum.request('chainIDX', 'method', [params])

@rekmarks
Copy link
Contributor Author

rekmarks commented Apr 8, 2020

@frozeman I was thinking about how to change the request method signature to enable the targeting of a specific chain, or even another protocol entirely.

That'd be a different PR, of course, but the two things that come to mind are:

  1. Just add a third config param, like so:
request(method: string, params: Object | Array, config: Object): Promise<unknown>
  1. Change/overload the signature such that it can be passed a single object param:
interface RequestArgs {
  method: string;
  params: Array | Object;
  chainId?: string;
  ...otherStuff: Array<any>;
}

request(args: RequestArgs): Promise<unknown>

@eip-automerger eip-automerger merged commit 6c35d9c into ethereum:master Apr 8, 2020
pizzarob pushed a commit to pizzarob/EIPs that referenced this pull request Jun 12, 2020
Hi, I'm a bot! This change was automatically merged because:

 - It only modifies existing Draft or Last Call EIP(s)
 - The PR was approved or written by at least one author of each modified EIP
 - The build is passing
tkstanczak pushed a commit to tkstanczak/EIPs that referenced this pull request Nov 7, 2020
Hi, I'm a bot! This change was automatically merged because:

 - It only modifies existing Draft or Last Call EIP(s)
 - The PR was approved or written by at least one author of each modified EIP
 - The build is passing
Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 6, 2021
Hi, I'm a bot! This change was automatically merged because:

 - It only modifies existing Draft or Last Call EIP(s)
 - The PR was approved or written by at least one author of each modified EIP
 - The build is passing
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.

6 participants