-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Discussion: EIP-1193 #2319
Comments
Summary of Devcon 5 EIP-1193 meetingParticipantsThis is an incomplete list of people/projects that participated. Please feel free to add yourself or others: @alcuadrado, @nivida, @davidmurdoch, @joshstevens19, @adrianmcli, @iurimatias, @ricmoo, @spalladino, @javier-tarazaga, @ylv-io, and members from Alchemy, Chainsafe, Metamask, and possibly more projects. Format of the discussionInitially, every member raised what they thought were the most important concerns to move the EIP forward. After that, we discussed most of them and proposed possible solutions. List of concerns
Proposed solutions
6 and 7. Most of the time was spent discussing these items. We considered different approaches like not doing anything at all, imposing a certain class name, and many other techniques. What we concluded is that providers would have a whitelist of the capabilities they offer. The most basic provider (i.e. HTTP-based provider) will have such a whitelist empty. More featureful providers will declare all of their extra features. For example, a websocket/polling-based provider that supports subscriptions, should declare that in the whitelist. We analyzed the risk of this whitelist getting huge with time, but still decided to go with this approach. It's not clear that it will actually get huge, nor that this would even care. We chose this approach over others, as it's more explicit and simpler. Next steps
(Disclaimer: I will probably do these things next week, as I have to get back to Argentina and will be super jet-lagged). |
As a historical addition: We discussed always both and as @frozeman told me is this issue the source of this EIP.
Yes, same here I also think we should create a separate EIP which defines the injection of providers. This EIP should have a section about security concerns as well and should provide some possible solutions for doing it in a secure way. (probably MetaMask has some good inputs here for creating a RFC @danfinlay) Required Sections:
(we probably should have further discussion about the injection of providers in the related RFC PR)
Discussed 2y ago here.
Agreed.
Sounds good!
export interface EIP1193 {
// .... other methods
getCapabilities(): Capabilities;
}
export interface Capabilities {
[capability_key: string]: any;
} |
Thanks @alcuadrado for sharing the summary of Devcon's meeting 🙏 I agree that However we technically don't need the I will update the #2261 to add the note for no-ops but other than that I don't see the need to have Regarding all other points, I have no other objections and I'm glad we have reached consensus specially on point number 2. We should merge #2090 which includes related changes. |
Interesting – the EIP itself lists https://ethereum-magicians.org/t/eip-1193-ethereum-provider-javascript-api/640 as the official forum. In case there is an agreement to use this issue, can someone update the EIP as well as perhaps add links to the other two mentioned forums into a References section? |
Great to see this going back to GitHub issues. I find them better readable. Where can I see the latest agreed version of this eip? |
@pedrouid EIP-1102 states that Starting within a release or two, the MetaMask inpage provider will support both. We believe that it would be too disruptive for dapp developers to remove |
I created #2419 to update the EIP
Completely agree with this. Thanks for updating #2261 !
I'll explore different alternatives this week and post the results here or in a PR.
I agree with this. I wouldn't add |
Since @alcuadrado seems to be very actively pursuing this EIP (perhaps even to @frozeman @ryanio @marcgarreau @MaiaVictor what do you think? |
@axic Sounds like a good idea. @alcuadrado took the lead for this EIP in the related meeting we had during Devcon. |
I am open to adding @alcuadrado to lead closing discussions to bring us to final, if I can get a +1 from another author in agreement. Thank you for taking the initiative. |
Regarding
|
For me I always interpreted this way. The same way as other wbe3 provider allow you to logout. This is useful for dapps that for example support both a builtin provider and web based one. As for a "close" as a way to block rpc request. I don't see the point. If an app do not want to send rpc request, it can simply not call them anymore. Regarding "open", assuming this would be a call to be made for dapps that want to make rpc request, this would be a breaking change as now dapp would not be able to call |
@wighawag I think that's a compelling use case, but IMO The changes to the spec I've described are not breaking, because they remain optional.
Similarly, if a dapp doesn't want to use a particular provider, couldn't it also just ignore it? |
In that case, I would also add that the Also given so many dapps are using |
That is why I call it if we want
that would make it redundant. |
But the EIP-1102 already caused a lot of breaking changes, couldn't we make use of that developer time already spent on the |
We can keep enable (for backward compatibility) but we should move towards using rpc methods, keeping EIP-1193 as simple as possible As for adding a |
Can we move this to Last Call/Final or close it? Is there any reason to leave it in Draft state at this point? Is work being done on it? |
This EIP, as currently worded, is incompatible with current window.ethereum.send(payload: JsonRpcRequest, callback: (error: JsonRpcErrorResponse | null, response: JsonRpcSuccessResponse | null) => void): void cannot also implement window.ethereum.send(method: string, params: unknown[]): Promise<unknown> A client needs to choose which one of those it expects, and a provider needs to choose which one it implements. Things will only work if the client and the provider both align. Recommended remediation: Add a method for asking the provider what version of the protocol it is running. This could be as simple as a string, or a string array, where the string(s) represent supported protocols. version: (): Promise<('eip-1193' | 'eip-1234')[]> If a given provider supports several different EIPs, then it can return each of them in the array. The client then merely needs to check if that function exists, and if it does do a |
@MicahZoltu, why can't they implement both signatures? Providers can use method overloading to provide both ways. An example in TypeScript:
Or are you just worried about callers not being able to differentiate between the legacy way the new 1193 way? I don't think this EIP should be concerned too much with this level of backwards-compatibility; providers, and clients that consume these providers, should be free to decide if they want to continue to support the legacy signature and for how long (and they should log a deprecation notice before complete removal). |
@davidmurdoch Sorry for the delay in getting back to you. A provider can provide multiple overloads and switch on parameters (which is a terrible pattern IMO, but it does work in this case). However, callers cannot do the same thing. All they know is that there is a There is currently no graceful migration from old style to new style. This could easily be avoided with the addition of a protocol handshake or by just using different function names. I don't see a particularly compelling argument in making the migration more painful than necessary here. |
Method overloading is a very common feature and I personally have no issue using it in JS. I don't mind overloading to support the old style in Ganache for another year or so (likely logging a deprecation notice in the meantime, and then an obsolete notice after that). I get that it's a bit inconvenient for everyone who writes a provider to have to keep the old API around if they want to be useful, but doing so provides a sense of stability around Ethereum's client-side APIs. As a user, especially a new user, It's beyond annoying when a project changes their long established public interface in a backwards-incompatible manner, rendering years of community documentation and examples outdated. Because this EIP describes an API that is exposed to clients by the browser itself, we should consider treating it with the same level of back-compat longevity that browsers do -- supporting the old signature for the foreseeable future. All that said, I know Ganache has it easy here, as implementors choose us and don't have to worry about the browser's/wallet's provider's API. Still, I think provider implementors should consider maintaining the old style for while. As far as changing the method name, have any suggestions? For the version signaling idea (handshake or |
With the ability to handshake, as a dapp developer I can ask the provider, "I would like to communicate with 1193 style send, do you support that?" and if it doesn't respond or responds with no I can then either gracefully degrade to legacy style sends, or I can present the user with a useful error message. On the other hand, if there is no way to probe for versioning all I can do is try to do a modern send and then if it fails tell the user that something went wrong (and the error message is likely useless to the user, since I'm calling an API with the wrong parameters). @davidmurdoch What is your argument against a version/handshake? |
Thanks @enolan for pointing these errata out, I have started a PR to fix: #3138 For an EIP-1193 provider that implements websocket support, check out the MetaMask
|
@enolan you can also try with |
@ryanio MetaMask doesn't actually support websockets, at least according to MetaMask/metamask-extension#1645. Using it in the browser console I do get events - they seem to be polling and providing the streaming interface. For some reason I wasn't getting the events in my Rust code, so I assumed it was just dropping the Thanks for fixing the examples! |
This PR fixes two errata in the EIP-1193 examples section, special thanks to @enolan for noting in #2319 (comment) and #2319 (comment).
hey, some dapps have stopped working with our provider, probably because they moved to |
MM has this example
but an error is expected here |
Hi @flexsurfer, that's because MetaMask implements EIP-1102. You have to call |
hey @rekmarks thanks, some dapps request 'eth_accounts' before 'eth_requestAccounts', in that case we throw an error in our provider, but MM doesn't, and in that case, dapp works in MM but not with our provider, so it seems MM doesn't throw an error and returns empty accounts? but in that case how dapp can distinguish empty accounts and unauthorized request? according to EIP-1193 provider should throw 4100 | Unauthorized error |
There is no specification governing the behavior of Your reading of 1193 is correct, and it's certainly fine (probably better) to reject with an error if no accounts are available. However, my sense is that most dapps interpret an empty array as an authorization problem. I can't think of other reasons why a wallet wouldn't have any accounts available. |
Would a smart contract wallet that's trying to either deploy for the first
time or having issues upgrading (e.g. fail mid'upgrade) present a situation
like this?
…On Mon, Feb 8, 2021, 8:41 AM Erik Marks ***@***.***> wrote:
There is no specification governing the behavior of eth_accounts in this
situation. Historically, it's returned an empty array, and MetaMask
continues to do so in order to prevent breaking the existing, widespread
use of that method.
Your reading of 1193 is correct, and it's certainly fine (probably better)
to reject with an error if no accounts are available. However, my sense is
that most dapps interpret an empty array as an authorization problem. I
can't think of other reasons why a wallet wouldn't have any accounts
available.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#2319 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AH2D4LHV52MWJW7KSLVHQLDS6AH2LANCNFSM4JDKF4BA>
.
|
* Moves 2696 to final. * Removes review end date from 2696. * Markes RequestArguments as readonly. This came up in the discussion at ethereum#2319 (comment) (which this is just a break-out of).
This PR fixes two errata in the EIP-1193 examples section, special thanks to @enolan for noting in ethereum#2319 (comment) and ethereum#2319 (comment).
Closing this for housekeeping purposes since this EIP is final, but feel free to continue discussing here as needed. |
I noticed in this EIP there wasn't a mention about privacy considerations in this document. Given this one is final I doubt we want to be modifying this one any more, but I'm curious where others who worked on this originally stand on the general principle of when we should be exposing that the ethereum provider versus when it should not be injected since this can present a fingerprinting privacy risk for users that can give websites the ability to fingerprint users. |
@kdenhartog check out https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1102.md although it seems the status is stagnant, @rekmarks maybe we should get this one over the finish line to final unless there was some reason or if another EIP took precedence. What is MM's current position on it? |
From a quick read of that EIP it looks like it shares common goals, although it looks like it's codifying behavior that has been in production for quite some time. I'll jump into the discussion on ethereum-magicians since that looks like the best way to keep that discussion going. |
Here's some other security considerations we think would be useful for this EIP. @MicahZoltu for Finalized EIPs like this one are we allowed to make updates here or should we open a new EIP to move this forward? |
If you want to make a normative change, you will need a new EIP/standard. If you want to, for example, add a security consideration and the author is OK with it then that would be a non-normative change and is allowed. The key is, anything (no matter how silly/dumb) that is compliant with this EIP today must still be compliant after the change. This precludes pretty much any change to the specification itself other than typo fixes and the like. As an example, there are many problems with ERC-20, yet we cannot change it and instead we have to develop new standards that either extend or replace it. |
👍🏻 awesome makes sense to me. I think we'll want to aim for something a bit more normative here so I'll draft up a starter EIP and take that route instead |
@🥰 |
As agreed at Devcon 5, I'm creating this issue to move the EIP-1193 discussion here. It's been fragmented in multiple places, making it hard for most people to follow it.
Link to the current version of the EIP: https://eips.ethereum.org/EIPS/eip-1193
Previous discussion channels
The text was updated successfully, but these errors were encountered: