optional chainId (extracted from #4932) #5134

hackmod · 2018-08-24T10:19:05Z

from #4932

Description

This PR separates all other stuff except optional chainid support.

Done

fixed custom Rpc form to support optional chainid.
- both old-ui and new-ui
fixed custom Rpc css style
show optional ticker symbol + nickname
hide/show all optional parameters (chainId, symbol and nickname)
- only new-ui supported

Screen shot

chrisfranko · 2018-08-24T12:24:21Z

:D 🕺

ghost

Much better, thanks 👍

hackmod · 2018-08-27T14:42:46Z

rebased and fixed defaultValue bug.

hackmod · 2018-08-28T03:15:31Z

@buhrmi any suggestions are welcome 😃
//
rebased and conflict resolved

buhrmi · 2018-08-28T03:46:06Z

@hackmod well first of all the failing test needs to be fixed :p

hackmod · 2018-08-28T04:23:14Z

@buhrmi // test-e2e-fireforx fail is random. I think its circleci fault.
(and now, simply git commit --amend and git push -f again solve this issue)

buhrmi · 2018-08-28T04:53:29Z

I've just tested it. I was able to deploy and interact with a contract on the classic network within remix. Seems to work fine and good to go :)

chrisfranko · 2018-08-28T13:29:15Z

Launched a contract using reminx and the public https://node.expanse.tech node with chain_id 2 perfectly fine.

ghost

When merge?

bdresser · 2018-09-12T18:13:20Z

Hi @hackmod @eosclassicteam, thanks again for your patience with this. We appreciate your work and want to make sure it lands with the appropriate implementation that makes it useable and minimally disruptive.

After chatting with the team, here's what we think makes sense:

UI

The default view in the settings menu should remain the same, all additional options should be hidden behind the Show Advanced Options link
Header for this section should update to New Network
Clicking Show Advanced Options link should reveal three optional fields

Functionality

Symbol should update the currency symbol from ETH to whatever the user provides
Nickname should determine how the network is named in the Networks dropdown (top-right)
No field for block explorer yet, we'd like to think about how to communicate the template format
We would not like to change our default pricing APIs from Infura (for ETH) and Balanc3 (for tokens). If the user has specified a custom network, it's acceptable to use cryptocompare, but the PR should not change our default behavior here.

Future improvements

We'd eventually like to move in a direction where dapps can suggest a new network to a use (so users don't have to fiddle with these settings). If you're interested, we'd appreciate your feedback on this proposal: API for dapp to request network change #5101
Currently the network drop-down menu remembers the last three networks a user joined and drops older ones. We plan to update this functionality to remember all networks a user has joined and allow them to remove whichever they'd like (similar to how imported accounts work now) Editable Networks #5246

Does this spec make sense? Sorry we haven't communicated these needs clearly until now, and thanks for bearing with us as we work through this together. I'll do my best to keep this top of mind for the team until we have something merged. And thank you for all your work! (cc @danfinlay)

hackmod · 2018-09-12T23:34:43Z

@bdresser // thank you for your support :)
I'm already prepared with extra fixes like as ticker symbol/price api, will follow your suggestion asap!!

hackmod · 2018-09-13T11:11:52Z

rebased, and follow additional suggestion/recent changes

optional symbol / optional nickname added
use cryptocompare api conditionally
~~[ ] fix remain bugs~~
deposit with ShapeShift feature is not applied

screen shots

whymarrh · 2018-10-25T18:34:57Z

@hackmod I've added a few commits to this PR that we think help the implementation be a bit more generic. I think the commits speak for themselves but let me know if you they don't make sense. The gist of the changes are 1) fromCurrency in the CurrencyController is better named nativeCurrency (47c4508) and 2) the changes in the CurrencyDisplay container can be avoided if we pass in the chain's native currency when needed (99acde4).

As-is, this looks good changes-wise. @estebanmino had pointed out some issues with the login above that I think still persist with these changes (maybe with the RPC URL you shared in the OP). Do you have a suggested set of RPC information we can use to do a proper QA pass? I'm seeing a lot of connection issues when using information you've shared so far.

estebanmino · 2018-10-25T19:48:10Z

about @whymarrh comments, I tried it out and I don't have found any issue as the one I mentioned before, for me seems that is working both with local ganache-cli and the information provided, when/where are you having those issues?. I agree that it would be very useful to have more RPC information.

whymarrh · 2018-10-26T07:05:21Z

ui/app/components/transaction-activity-log/transaction-activity-log.component.js

    const { eventKey, value, timestamp } = activity
    const ethValue = index === 0
      ? `${getValueFromWeiHex({
        value,
-        toCurrency: ETH,
+        nativeCurrency,


Whoops, this should be fromCurrency: nativeCurrency

whymarrh · 2018-10-26T07:06:50Z

ui/app/components/transaction-activity-log/transaction-activity-log.component.js

-      })} ${ETH}`
-      : getEthConversionFromWeiHex({ value, toCurrency: ETH, conversionRate })
+      })} ${nativeCurrency}`
+      : getEthConversionFromWeiHex({ value, nativeCurrency, toCurrency: nativeCurrency, conversionRate })


Whoops, this should be fromCurrency: nativeCurrency

whymarrh · 2018-10-26T08:12:55Z

when/where are you having those issues?

They're intermittent so I believe they're related to the network connectivity of the added RPC URL. I'm cool with the state of this PR as that's up to what the user adds as their desired network.

whymarrh · 2018-10-26T08:27:23Z

@hackmod thanks again for all the work you put into this!

pyskell · 2018-11-21T14:04:24Z

when/where are you having those issues?

They're intermittent so I believe they're related to the network connectivity of the added RPC URL. I'm cool with the state of this PR as that's up to what the user adds as their desired network.

@whymarrh I run the ethereumclassic.network RPC, what issues are you seeing? Intermittent connectivity problems or any specific errors?

filips123 · 2018-12-26T14:55:34Z

How to rename custom network after creation?

whymarrh · 2019-01-03T19:30:09Z

@pyskell I can't remember (sorry!) but I do use that for testing and I'll be sure to mention anything that I run into

@filips123 remove and re-add is the only option right now

hackmod requested review from alextsg, danjm and whymarrh as code owners August 24, 2018 10:19

hackmod mentioned this pull request Aug 24, 2018

Optional Chain_id w/o Native ETC Support #5130

Closed

hackmod changed the title ~~optional chainId (separated from #4932)~~ optional chainId (extracted from #4932) Aug 24, 2018

hackmod force-pushed the optional-chain_id branch from dbc1d05 to 1072aa9 Compare August 24, 2018 10:50

hackmod mentioned this pull request Aug 24, 2018

Initial multi-chain support (aka. optional chainid fix) #4932

Closed

8 tasks

ghost previously approved these changes Aug 24, 2018

View reviewed changes

hackmod dismissed ghost ’s stale review via 9a82a5d August 27, 2018 14:42

hackmod force-pushed the optional-chain_id branch from 1072aa9 to 9a82a5d Compare August 27, 2018 14:42

hackmod force-pushed the optional-chain_id branch from 9a82a5d to 4102a95 Compare August 27, 2018 15:06

buhrmi mentioned this pull request Aug 28, 2018

Feature: Override chain id in network config #3453

Closed

hackmod force-pushed the optional-chain_id branch from 4102a95 to e722092 Compare August 28, 2018 03:21

hackmod force-pushed the optional-chain_id branch from e722092 to ea31a91 Compare August 28, 2018 04:24

bdresser mentioned this pull request Sep 4, 2018

API for dapp to request network change #5101

Closed

ghost previously approved these changes Sep 4, 2018

View reviewed changes

hackmod dismissed ghost ’s stale review via e57f530 September 13, 2018 10:38

hackmod force-pushed the optional-chain_id branch from ea31a91 to e57f530 Compare September 13, 2018 10:38

hackmod force-pushed the optional-chain_id branch 2 times, most recently from 16b1a65 to 660e21e Compare September 13, 2018 12:34

hackmod added 7 commits October 23, 2018 13:59

fixed selectors and testcase

5dee37b

use fromCurrency

23e4772

check currency conversion available for a customRpc

7445924

fixed conflict mistake and follow up for recent changes

b548e5b

fixed testcases

45c8171

remove unused vars

15d5389

fixed testcase after rebase

aa4d0df

hackmod force-pushed the optional-chain_id branch from 7cb8d17 to aa4d0df Compare October 23, 2018 05:02

whymarrh added 3 commits October 25, 2018 12:52

Rename fromCurrency to nativeCurrency in CurrencyController

47c4508

Rename preference for switching primary currency

5abfd8d

Show native currency symbol for custom chains

b551822

bdresser mentioned this pull request Oct 25, 2018

CustomRPC settings: URL box should be on the top #5605

Closed

Remove changes from CurrencyDisplay component

99acde4

whymarrh force-pushed the optional-chain_id branch from cd73ac4 to 99acde4 Compare October 25, 2018 18:30

whymarrh suggested changes Oct 26, 2018

View reviewed changes

whymarrh added 2 commits October 26, 2018 09:09

Fix parameter names for conversion utils

4aae804

Fix fromCurrency prop passed to CurrencyInput

2b7d4ab

whymarrh approved these changes Oct 26, 2018

View reviewed changes

alextsg approved these changes Oct 26, 2018

View reviewed changes

whymarrh merged commit 54a8ade into MetaMask:develop Oct 26, 2018

whymarrh mentioned this pull request Oct 26, 2018

Multi-Chain Support #1820

Closed

2 tasks

wbt mentioned this pull request Oct 29, 2018

Add custom name to private networks/custom RPCs #5630

Closed

hackmod deleted the optional-chain_id branch March 8, 2019 22:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

optional chainId (extracted from #4932) #5134

optional chainId (extracted from #4932) #5134

hackmod commented Aug 24, 2018 •

edited

Loading

chrisfranko commented Aug 24, 2018

ghost left a comment

hackmod commented Aug 27, 2018

hackmod commented Aug 28, 2018 •

edited

Loading

buhrmi commented Aug 28, 2018

hackmod commented Aug 28, 2018 •

edited

Loading

buhrmi commented Aug 28, 2018

chrisfranko commented Aug 28, 2018

ghost left a comment

bdresser commented Sep 12, 2018

hackmod commented Sep 12, 2018

hackmod commented Sep 13, 2018 •

edited

Loading

whymarrh commented Oct 25, 2018

estebanmino commented Oct 25, 2018

whymarrh Oct 26, 2018

whymarrh Oct 26, 2018

whymarrh commented Oct 26, 2018

whymarrh commented Oct 26, 2018

pyskell commented Nov 21, 2018

filips123 commented Dec 26, 2018

whymarrh commented Jan 3, 2019

optional chainId (extracted from #4932) #5134

optional chainId (extracted from #4932) #5134

Conversation

hackmod commented Aug 24, 2018 • edited Loading

Description

Done

See also

Screen shot

chrisfranko commented Aug 24, 2018

ghost left a comment

Choose a reason for hiding this comment

hackmod commented Aug 27, 2018

hackmod commented Aug 28, 2018 • edited Loading

buhrmi commented Aug 28, 2018

hackmod commented Aug 28, 2018 • edited Loading

buhrmi commented Aug 28, 2018

chrisfranko commented Aug 28, 2018

ghost left a comment

Choose a reason for hiding this comment

bdresser commented Sep 12, 2018

UI

Functionality

Future improvements

hackmod commented Sep 12, 2018

hackmod commented Sep 13, 2018 • edited Loading

screen shots

whymarrh commented Oct 25, 2018

estebanmino commented Oct 25, 2018

whymarrh Oct 26, 2018

Choose a reason for hiding this comment

whymarrh Oct 26, 2018

Choose a reason for hiding this comment

whymarrh commented Oct 26, 2018

whymarrh commented Oct 26, 2018

pyskell commented Nov 21, 2018

filips123 commented Dec 26, 2018

whymarrh commented Jan 3, 2019

hackmod commented Aug 24, 2018 •

edited

Loading

hackmod commented Aug 28, 2018 •

edited

Loading

hackmod commented Aug 28, 2018 •

edited

Loading

hackmod commented Sep 13, 2018 •

edited

Loading