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.

Sign up for GitHub

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

Jump to bottom

Rename and clarify disconnection errors #2634

Merged
merged 2 commits into from
May 11, 2020
Merged

Rename and clarify disconnection errors #2634

merged 2 commits into from
May 11, 2020

Conversation

rekmarks
Copy link
Contributor

@rekmarks rekmarks commented May 10, 2020 •
edited
Loading

Link to rendered file with updates.

The errors related to connectivity were somewhat underspecified, and oddly named. Their names and descriptions have been updated, and a non-normative comment has been added.

@adrianmcli
Copy link

Just curious, is this to pave the way for a multi-chain provider? I'm really opening up to that idea.

@rekmarks
Copy link
Contributor Author

@adrianmcli, these particular changes are more about clarifying the distinctions between 4900 and 4901.

The provider doesn’t have to be “multi-chain” (scare quotes because we haven’t really defined the concept) in order for 4901 to be useful. Imagine a provider exposes a custom RPC method for switching the chain from the dapp. (Not saying that’s a good idea, but just as an example.) If the provider doesn’t recognize a requested chain ID, it can return 4901.

That being said, as a side-effect, if wallet developers start experimenting with multi-chain providers, there’s now already a standardized way to handle a particular chain not being serviced. :)

@rekmarks rekmarks marked this pull request as ready for review May 11, 2020 18:30
ryanio
ryanio approved these changes May 11, 2020
View reviewed changes
Copy link
Contributor

@ryanio ryanio left a comment

Choose a reason for hiding this comment

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

Looks great, thx!

@eip-automerger eip-automerger merged commit 77adcb4 into ethereum:master May 11, 2020
pizzarob pushed a commit to pizzarob/EIPs that referenced this pull request Jun 12, 2020
@rekmarks @pizzarob
Automatically merged updates to draft EIP(s) 1193 (ethereum#2634)
6d94829 
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
@rekmarks @tkstanczak
Automatically merged updates to draft EIP(s) 1193 (ethereum#2634)
8c0dafc 
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
@rekmarks @Arachnid
Automatically merged updates to draft EIP(s) 1193 (ethereum#2634)
76dfdc6 
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
Reviewers

@ryanio ryanio ryanio approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@rekmarks @adrianmcli @ryanio @eip-automerger