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

feature: add goerli support #6459

Merged
merged 4 commits into from
Apr 17, 2019
Merged

feature: add goerli support #6459

merged 4 commits into from
Apr 17, 2019

Conversation

bitpshr
Copy link
Contributor

@bitpshr bitpshr commented Apr 15, 2019

This pull request adds goerli network support.

Resolves #6249

Screen Shot 2019-04-15 at 11 10 07 AM

Screen Shot 2019-04-15 at 11 10 11 AM

Screen Shot 2019-04-15 at 11 18 22 AM

@metamaskbot
Copy link
Collaborator

Builds ready [b940757]: chrome, firefox, edge, opera

Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

LGTM, one small nitpick: can we drop the changes to the package-lock.json file as there are no changes to the package.json file in this PR?

@bitpshr bitpshr requested a review from whymarrh April 15, 2019 17:03
@bitpshr
Copy link
Contributor Author

bitpshr commented Apr 15, 2019

Thanks @whymarrh, good catch. Updated.

@metamaskbot
Copy link
Collaborator

Builds ready [b711734]: chrome, firefox, edge, opera

whymarrh
whymarrh previously approved these changes Apr 15, 2019
@whymarrh whymarrh requested a review from tmashuang April 16, 2019 12:49
@whymarrh
Copy link
Contributor

@tmashuang would you be able to give this a QA pass as well? It seems good to me.

Copy link
Contributor

@tmashuang tmashuang left a comment

Choose a reason for hiding this comment

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

Will need to add the network list to the the buyEth deposit modal

const isTestNetwork = ['3', '4', '42'].find(n => n === network)

and provide a url for the test network

function getBuyEthUrl ({ network, amount, address, service }) {

I recommend https://bridge.goerli.com/ from
https://github.com/goerli/testnet/blob/master/README.md#meta-data-görli but requires eth from other test networks, could use the other ones recommended.

Edit: Couldn't get the bridge to work.
https://goerli-faucet.slock.it/ provides 0.05/tx
https://faucet.goerli.mudit.blog/ will need a social media link with an address as the message

@tmashuang
Copy link
Contributor

This PR, #6444 also touched the network-dropdown.js will need to resolve those conflicts.

@metamaskbot
Copy link
Collaborator

Builds ready [180f1b8]: chrome, firefox, edge, opera

Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

Seems legit (cc @bitpshr)

@whymarrh whymarrh merged commit 0db0a18 into develop Apr 17, 2019
@whymarrh whymarrh deleted the feature/goerli branch April 17, 2019 17:35
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.

Add Goerli test net
4 participants