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

add POA.network support #2815

Closed
wants to merge 3 commits into from
Closed

add POA.network support #2815

wants to merge 3 commits into from

Conversation

rstormsf
Copy link

@rstormsf rstormsf commented Dec 27, 2017

We would really like to provide an easy way for our users to use POA network in metamask so they can switch to POA network.
Poa.network is the compliant EIP155 network.
MEW already supports it.
https://myetherwallet.com
MyEtherWallet/etherwallet#1427

Please let us know if there is anything else that we need to add in this PR.

@kumavis @danfinlay

@igorbarinov
Copy link

Hi guys,

Dear @kumavis when we discussed the subject on Monero party in Las Vegas you told me that it's better to submit a PR then fork the repo under our name and publish it. Please let me know is it possible to add our network or not. Thank you in advance!

@rstormsf
Copy link
Author

Any updates @kumavis ?

@rstormsf rstormsf mentioned this pull request Dec 30, 2017
@danfinlay
Copy link
Contributor

We're on vacation this week, and many of us next week as well, so please be patient, don't expect this to be merged within days.

This would set a new precedent for us, in that we don't currently point to third party providers that we don't work closely with (Infura).

In the short term, instructing users how to use the "Custom RPC" menu option would easily allow this functionality without us needing to set new policy standards.

We can talk about this more when we're back from our break.

@rstormsf
Copy link
Author

rstormsf commented Dec 30, 2017

@danfinlay Thank you so much for your response. We would be waiting for your decision.
I'd like just to point out that currently, Metamask supports unstable/broken fork of ETH kovan chain:
https://authorities.kovan.network
image

https://core-netstat.poa.network/
Spec.json: https://github.com/poanetwork/poa-chain-spec/tree/core
POA Network team is committed to providing stable PoA network.

As well as stable sokol PoA testnet: https://sokol-netstat.poa.network/ with 100% working faucet:
https://faucet-sokol.herokuapp.com/
https://github.com/poanetwork/poa-chain-spec/tree/sokol

@danfinlay
Copy link
Contributor

Thanks for bringing that to our attention, it deserves its own issue and possible deprecation.

If PoA is becoming a popular testnet, it should be feasible to convince Infura to host an instance of it, which would allow us to support it much more easily than establishing a new infrastructure of support and response with whoever is hosting this one (you?).

Let's bring Infura into the conversation when we're back.

@rstormsf
Copy link
Author

@danfinlay Perfect! We would love to have POA Network on infura

@kumavis
Copy link
Member

kumavis commented Jan 2, 2018

Poa.network is the compliant EIP155 network.

@rstormsf @igorbarinov what are the values for networkId and chainId?
(eth mainnet, ropsten, kovan, rinkeby all have networkId == chainId but for ETC networkId != chainId)
we use net_version for the EIP155 chainId value

@kumavis
Copy link
Member

kumavis commented Jan 2, 2018

If PoA is becoming a popular testnet,

@danfinlay This is not a testnet but a federated consensus network
https://poa.network/

POA Network is the first Ethereum-based public network with Proof of Authority consensus, reached by independent validators.

@rstormsf
Copy link
Author

rstormsf commented Jan 2, 2018

@kumavis NetworkId: 99, ChainId: 99
https://poa.trustwalletapp.com/ - proof of network id
Here is the request/response from CORE chainId node:

curl --data '{"method":"net_version","params":[],"id":1,"jsonrpc":"2.0"}' -H "Content-Type: application/json" -X POST localhost:8777

{"jsonrpc":"2.0","result":"99","id":1}

We are EIP155 compliant.

@kumavis
Copy link
Member

kumavis commented Jan 2, 2018

@rstormsf do you have a hosted rpc endpoint?

@rstormsf
Copy link
Author

rstormsf commented Jan 2, 2018

yes, it's in the PR as well

https://core.poa.network

@kumavis
I also provided PR for MEW which was already accepted:

https://myetherwallet.com
MyEtherWallet/etherwallet#1427

@igorbarinov
Copy link

That RPC endpoint is a chain of two load balancers

  • different hostings
  • different DNS providers
                         +--------+
                         | node a |
             +---------->| node b |
             |           | node c |
             |           +--------+
+------------+---+
|loadbalancer #1 |
|core.poa.network|
+------------+---+
             |
             |            +---------------+       +------+
             |            |loadbalancer #2|       |node d|
             +----------->|poa.jp         +------>|node e|
                          +---------------+       +------+

@rstormsf
Copy link
Author

rstormsf commented Jan 3, 2018

@kumavis have you got everything that you needed?

@rstormsf
Copy link
Author

rstormsf commented Jan 5, 2018

@kumavis @danfinlay any updates?

@rstormsf
Copy link
Author

rstormsf commented Jan 8, 2018

@kumavis @danfinlay
Any updates? Please let us know if you ever plan to accept our PR

@danfinlay
Copy link
Contributor

Sorry, right now the main network is under enough stress that we're focusing our efforts on it working more smoothly. We're still open to adding this, but adding a test network is a much lower priority at the moment. We'll get to it when we get a little time free, maybe early next week.

@danfinlay
Copy link
Contributor

Re:

Should we fork metamask and release it with POA in it?

As I said before, if you're in a big hurry:

In the short term, instructing users how to use the "Custom RPC" menu option would easily allow this functionality without us needing to set new policy standards.

You're welcome to fork & rebrand if you think it's that big of a deal.

@h4r5h4
Copy link

h4r5h4 commented Jan 9, 2018

@danfinlay I would like to point out that this is not testnet and it is mainnet for https://poa.network/.
There are a lot of people(including me) who would love to see this being integrated into metamask.

@micwebnet
Copy link

+1 Much prefer using the well designed MetaMask UX over the complexity of MEW, so I'd love to be able to switch to POA in MetaMask natively. POA Core is indeed a mainnet, not a testnet.

Best, MM

@maratP
Copy link

maratP commented Jan 22, 2018

+1

@rstormsf
Copy link
Author

any update @danfinlay

@danfinlay
Copy link
Contributor

danfinlay commented Jan 25, 2018

We've been pressed on a lot of critical issues lately, sorry this hasn't made the top of our priority list.

A few thoughts on this topic that occurred to me recently, that could help explain why I personally haven't been comfortable jumping into this with both feet:

By calling it "not a testnet", you sound like you're absolutely selling this alternative network coin, making it a highly political move for us to potentially add it. Proof of Authority chains are relatively easy to set up, (thanks, Quorum!). This definitely won't be the last one, and so we have to decide how we want to treat various networks. Do we add them all to our provider menu, and make users scroll through them regularly?

That sounds like bad user experience, and also, I don't want to be responsible for these kinds of political moves. I don't want to be pressured by people like you to add your brand name into what should really be an un-opinionated, general purpose codebase.

I would dramatically prefer that we define and provide a new API for a site to declare the network it prefers to connect to, potentially even passing your own provider URL to metamask, and allowing it to operate solely as signer for those custom networks.

This has the side benefit of defining a "sign in moment" which fits very nicely with the privacy concerns that recently became recently popularized, represented by #714.

This also paves the way nicely for a multi-chain world, post Plasma and Cosmos.

This would allow dapps using alternative networks (like PoA, Rootstock, ETC, Dfinity, Rchain, whatever else comes along) without requiring the tedious direction of the user to "go log in to our network in metamask....", while simultaneously avoiding the MetaMask team having to make highly political decisions.

I made this case in the past to some ETC people in a similar PR, but they got hostile, and were not interested in deeper changes, and seemed only interested in getting the price bump from the headline of MetaMask inclusion. I hope your interest runs deeper.

I began exploring what this alternative type of API would look like here, would be happy to hear your comments on that proposal:
#2532

@rstormsf
Copy link
Author

@danfinlay
Thank you for your detailed response. I understand now your challenges.
I looked over the new API proposal, so I think that would be great so that I, as Front end developer, can enforce a user to be on POA network when he/she goes to my Dapp.

Just a quick note:

Proof of Authority chains are relatively easy to set up, (thanks, Quorum!).

First of all, thanks to Parity, not Quorum.
Second, good luck setting up PoA network with custom governance model being managed by smart contracts, develop secure DevOps infrastructure, explore and learn all security vulnerabilities, provide scaling solution for current mainnet by connecting them via
https://github.com/poanetwork/parity-bridge

@danfinlay
Copy link
Contributor

First of all, thanks to Parity, not Quorum.
Second, good luck setting up PoA network with custom governance model being managed by smart contracts, develop secure DevOps infrastructure, explore and learn all security vulnerabilities, provide scaling solution for current mainnet by connecting them via
https://github.com/poanetwork/parity-bridge

Okay, apologies, I don't know the nuances of your project. I'm sure it's really great. But understand there are a lot of really great alternative chains, and that's why I'm treading lightly.

@alex-miller-0
Copy link
Contributor

alex-miller-0 commented Jan 25, 2018

Do we add them all to our provider menu, and make users scroll through them regularly?

Why not allow users to add networks via RPC address and configure the name of that network on their extension? While on the topic, why not allow users to delete networks? I'd love to kill Kovan and some localhosts I misspelled 9 months ago.

we define and provide a new API for a site to declare the network it prefers to connect to, potentially even passing your own provider URL to metamask, and allowing it to operate solely as signer for those custom networks

I think that's a great idea, but having a personalized speed-dial list that users can manage would be good too.

@danfinlay
Copy link
Contributor

Why not allow users to add networks via RPC address and configure the name of that network on their extension? While on the topic, why not allow users to delete networks? I'd love to kill Kovan and some localhosts I misspelled 9 months ago.

That'd be a pretty cheap & easy shorter-term win, opened as new issue.

@rstormsf
Copy link
Author

rstormsf commented Jan 27, 2018

@danfinlay I'm very excited about your new API where I can programmatically change user's network, if I understand your idea correctly, that would solve the issue.
However, I do see some security risks there when people can create EIP155 non compliant fork, tell them to generate TXs and execute replay attack.
Maybe if metamask can utilize Chrome Notifications API to tell them that You are connected to non-compliant EIP155 network, please be aware! However, I'm not sure if Geth or parity nodes have RPC method that would return such information. I will dig in and let you know if it's possible make such call.

@alex-miller-0 I totally support your idea when users can create their own names with custom network URL for now, like MEW does it. + possibility to remove networks.

@wighawag
Copy link

wighawag commented Feb 1, 2018

@danfinlay I really like the idea of dapp telling which network to use.

With the idea of bridges between blockchain network getting traction, it might be useful if such api/ui would allow dapps to easily switch between 2 network as the user is interacting.

Let say you want to interact with a dapp that operate on another network but where you need lock some ether first (so it get mirrored on the other chain via a bridge)

It would be great for the dapp to automatically redirect the user to the second network once it detect the fund got mirrored in that network.

From what I gathered from https://www.youtube.com/watch?v=aWvzQMorof0 Giveth @GriffGreen is planning to do something along these line via https://github.com/paritytech/parity-bridge

In other words, the web3 api should be adapted to a multi network scenario. I am sure there is security implication and more UI complexity as users would need to be protected from interacting on non expected chains but it would be great to start thinking about it.

@hashguide
Copy link

Why not allow users to add networks via RPC address and configure the name of that network on their extension? While on the topic, why not allow users to delete networks? I'd love to kill Kovan and some localhosts I misspelled 9 months ago.

I like this idea, but also adding a way for a frontend developer to add a button/element to click on that will automatically submit the RPC address to MM RPC Menu, asking for confirmation from the user of course.

Allowing this extra feature would prevent the extra steps for the user and the network owners can fill in the rpc name and address how they like. This also insures that no side chain will get jealous of the other for being listed in MM and all users will be able to easily choose which networks exist in MM.

@danfinlay
Copy link
Contributor

I agree. I think I explained why we don't want to add a new network to the menu right now, but instead want to help make it easier for new networks to be used.

Closing in favor of #3102, #3604.

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.

10 participants