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: Decouple window implementation from interface #2090

Closed
wants to merge 4 commits into from

Conversation

pedrouid
Copy link
Contributor

No description provided.

Copy link
Contributor

@danfinlay danfinlay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it beneficial to standardize on a "transport" abstraction? Many groups could build their own provider object, which is already just an incredibly thin wrapper around what you're calling Transport. Would there be any benefit to making that incredibly thin wrapper portable across contexts? And if it were beneficial, is it necessary to formalize that portability in an EIP?

From where I'm standing: Each client probably builds its own provider today, and builds it around its own transport, and each of those transports could easily be ported to this format, but the files being changed are so small in this case, that I'm not sure it offers any significant gains in underlying functionality. Actual methods would still need to be added on the "other side" of the transport, etc.

The MetaMask equivalent:
The metamask-inpage-provider is initialized with a Stream that represents its transport, and that stream embeds its own message protocol. This abstraction has allowed us to port this provider across a couple contexts, including other extensions.

So to me, this looks like a suggestion to "remove an abstraction that is working in favor of this other one that would also work.". It seems like a "if it's not broke, don't fix it" situation.

I'm sure there's a reason this came to your mind though, so I'm curious what value you'd hope to gain from this abstraction.

EIPS/eip-1193.md Outdated Show resolved Hide resolved
EIPS/eip-1193.md Outdated Show resolved Hide resolved
@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):

EIPS/eip-1193.md Outdated Show resolved Hide resolved
@alcuadrado
Copy link
Member

I agree with these changes, and with not specifying the transport in this EIP.

@frozeman
Copy link
Contributor

If we don’t agree to a common object name how would this standardize anything?

If everybody now uses their own name. Wouldn’t this be messy. Or is the plan to standardize the name separate. Or make it different for different environments?

@frozeman
Copy link
Contributor

@danfinlay standardization if this minimal interface is super necessary for fall developers. Otherwise we all build for 5 different platforms differently.

@pedrouid @nividia I think we should remove the custom events from this standard for now. And define that in a separate standard. As I think it doesn’t really belong here.

@alcuadrado
Copy link
Member

Or is the plan to standardize the name separate. Or make it different for different environments?

Yes, exactly. Each environment type (e.g. browsers) can propose different standards.

@pedrouid
Copy link
Contributor Author

If we don’t agree to a common object name how would this standardize anything?

Standardizing the common object name is an implementation of injected providers, IMO this EIP should only concern with the interface of ethereum providers and I would include the events that are listed so far.

I think we sould draft a new EIP for how injected providers are available in window, that EIP would then define the common object name and how it would transport the messages with the webpage.

I think we should remove the custom events from this standard for now. And define that in a separate standard. As I think it doesn’t really belong here.

I think that the events that are listed so far are pretty standard between all Wallet solutions and we should include it as part of the Ethereum Provider interface.

@axic axic added the ERC label Jun 2, 2019
@nivida
Copy link
Member

nivida commented Jun 3, 2019

@frozeman

@pedrouid @nividia I think we should remove the custom events from this standard for now. And define that in a separate standard. As I think it doesn’t really belong here.

Yes, I think the same way as you.

@pedrouid
Copy link
Contributor Author

Where do we stand on this PR?
cc @MaiaVictor @frozeman @marcgarreau @ryanio @axic @nivida @danfinlay @alcuadrado

@alcuadrado
Copy link
Member

I'd just remove the mention to web3 compatibility. A few changes are coming to web3 versioning, and having this section pointing to a specific beta version is weird.

@alcuadrado
Copy link
Member

alcuadrado commented Jun 19, 2019

Actually, I'll do it in a separate PR, so we can merge this one as is.

Update: Created #2127

@pedrouid pedrouid changed the title Decouple window implementation from ethereum provider interface EIP-1193: Decouple window implementation from interface Jun 19, 2019
@alcuadrado
Copy link
Member

Can any of the original authors do a review of this change? @frozeman maybe? Thanks!

@ryanio
Copy link
Contributor

ryanio commented Jul 3, 2019

I believe that one of the best reasons for this EIP was standardizing across the common name window.ethereum so js dapps could be written once and usable across multiple platforms and environments.

If we want to create another EIP specifying the recommended locations of the provider across platforms, I'm all for that, but that should come first and then modify this EIP to point to that. Same with events - happy to decouple but let's create the EIP first so we can point to it from this EIP before removing it.

@pedrouid
Copy link
Contributor Author

pedrouid commented Jul 3, 2019

TBH there aren't really many ways to expose the provider. You either have it in window from CDN or extension. Or you bundle it from NPM on your website.

Regardless I don't see how that changes the API spec. The location of the API doesn't change its interface

@alcuadrado
Copy link
Member

I believe that one of the best reasons for this EIP was standardizing across the common name window.ethereum so js dapps could be written once and usable across multiple platforms and environments.

Thanks for your answer @ryanio! I know this was part of the original intent of this proposal, but during the discussions that took place around it, we arrived into a rough consensus that this EIP would just define an interface and that a new EIP will spec how it is exposed.

I understand the motivation for wanting to link between them, but waiting until the other EIP is completed to finalize this one, seems counterproductive to me.

@pedrouid pedrouid mentioned this pull request Oct 23, 2019
@pedrouid
Copy link
Contributor Author

Replaced by #2577

@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.

8 participants