search

EthereumCommonwealth / Roadmap

GNU Lesser General Public License v2.1
57 stars 17 forks source link

ClassicEtherWallet updates and MEW patches #26

Closed Dexaran closed 5 years ago

Dexaran commented 6 years ago
Dexaran commented 6 years ago

Detailed description of requested changes:

1. Merge 3.10.2 MEW updates into ClassicEtherWallet

I think that it is needed to update CEW to support all recent MyEtherWallet upgrades. The most important things are:

MyEtherWallet main repo: https://github.com/kvhnuke/etherwallet

ClassicEtherWallet repo: https://github.com/ethereumproject/etherwallet

2. Make ETC node default on CEW

I want ClassicEtherWallet to open ETC node by default instead of ETH node like MEW is doing. It is mostly a tool for Classic chain.

3. Color patch for MEW and CEW

It should be a patch to CEW. Later we will propose the corresponding changes on MyEtherWallet. I want each network to have a specified color. It is very important to reduce user mistakes with wrong chain selection!

How MEW looks right now: https://www.myetherwallet.com/

How CEW looks right now: https://ethereumproject.github.io/etherwallet/

4. Replace "MyEtherWallet" mentions by "ClassicEtherWallet" info on ClassicEtherWallet

ClassicEtherWallet is a fork of MEW. It contains a lot of info that is related to MyEtherWallet. It needs to be replaced:

5. Add DexNS support to CEW / make a patch to MEW

First of all it should be a patch to ClassicEtherWallet. DexNS is a smart-contract service to allow users interact with smart-contracts without requirement of ABI.

This will require a lot of updates and it is a low priority goal now. Smaller graphical updates should be merged first.

6. Add ClassicMask support on CEW / make a patch to MEW (not yet finished)

ClassicMask is still in development. We need to merge MetaMask support from MEW and it will be enough for now.

7. Add modular ENS support on MEW

Nothing to do here. It depends on MEW to merge the PR that is already sent.

randytorres commented 6 years ago

@Dexaran I forked a fresh version of MEW and made some changes to the home page replacing some of the MEW references with CEW. I also made ETC node on default which would complete Task 2. https://github.com/randytorres/etherwallet/commit/aa1a5a6ae040fc02240c9f7781aefd5261e71e29

I should be able to complete tasks 1-4 by this monday.

Dexaran commented 6 years ago

I will also ask to place the ETC node at the top of the node list in CEW. (ETH nodes are at the top at the moment)

We will also need to change the name lookup with modular ENS support patch. We need to create a customisable lookup to add DexNS support in future. I will provide more updates about this soon.

randytorres commented 6 years ago

@Dexaran I removed a lot of references to MEW and added the dynamic colors. As far as removing mentions of MEW, should I remove ALL mentions including and links to their github, help channels/rooms, donation addresses, etc?

https://github.com/randytorres/etherwallet/commit/dde63548c4f9063ce733237a9442179762876cdb

Dexaran commented 6 years ago

@randytorres Let's leave links to github, help channels/rooms, as is for now. Let's replace the donation address with Ethereum Commonwealth address: 0x52823e725a34d42e14a1b66fb67299c30c4d8edf

I'd like to mention some issues with your current implementation.

  1. Yellow color theme is a native color theme of Musicoin. Let's change the color theme of all testnets to gray #C4C4C4 and replace Musicoin with colors with yellow.

  2. Dark buttons like "Generate transaction" should also be affected by the color theme. They should be darker but change their colors. A user often pays attention to the buttons that they click.

image

  1. MEW logo should be replaced.

mew_logo

randytorres commented 6 years ago

Aright I made the changes you requested. I tried creating a CEW favicon based off the logo but It doesn't look very good, do you have a favicon.ico of the logo by any chance?

randytorres commented 6 years ago

Hey @Dexaran I just finished adding ENS support and included it in the PR. https://github.com/Dexaran/etherwallet/pull/3/commits/862eec31085d1be3158d24bb2380d4e71439488c

Dexaran commented 6 years ago

Let's start crosschain ENS lookup:

  1. Add the lookup dropdown button. It should contain ENS (.eth) and ECNS (.etc) now. We will add UBQ and EXP as soon as ENS will be deployed on this chains.

  2. I think that we need to initialize a second instance of web3 to perform a lookup on the corresponding chain. If the user has unlocked account under ETC chain but he wishes to send 1 ETC to the dexaran.eth then we should find the address that is associated with dexaran.eth in ENS on ETH chain (using Infura API endpoint). Take the address and send a transaction to this address on ETC chain (using Epool API).

  3. If the user has typed dexaran.eth but the Lookup source is set to ECNS then we should automatically switch the Lookup source to the ENS and update the Lookup dropdown button. .eth resolutions must always result in ENS lookup and .etc resolutions must result in ECNS lookup. If the user has typed a name that doesn't contain resolution or resolution is not assigned to any ENS then this should result in DexNS lookup on ETC chain. DexNS will be added later.

_lookup

Dexaran commented 6 years ago

Replace unlocking wallet "MetaMask / Mist" menu item by "MetaMask or ClassicMask / Mist" since ClassicMask is supported as well.

Dexaran commented 6 years ago

Support adding custom tokens by DexNS token names

1.

We should add the third menu item to add custom tokens on CLassicEtherWallet as follows:

token_by_name

2.

If the user has clicked the new "Add custom token by token name (DexNS)" menu item then we should connect to ETC network (initialize an instance of web3 with ETC API endpoint provider) and dropdown the DexNS token adding menu:

dexns__token_search_dropdown_unfinished

DexNS token adding menu should contain only one input item "Token symbol" (string text) and one button "SEARCH".

3.

CEW should perform a getName("inputted name") function call to DexNS Storage contract (this address: 0x429611c633806a03447391026a538a022e1e2731 and this ABI) when the user has typed the token name and clicked the "SEARCH" button. We should parse the result of the call to extract associated address and stringified metadata of the desired contract.

Example of the call with the "GNT" input will be represented in the following format:

dexns_call_result

4.

If the extraction of the metadata was successful then we should parse it again. Metadata is returned in the following format:

-<network id flag> -A <abi> -flag1 flag1_params -flag2 flag2_params -i <information> ...

Example of the metadata:

-ETC -A [{"constant":false,"inputs":[],"name":"foo","outputs":[],"payable":false,"type":"function"}] where -ETC means that the desired contract is deployed in the ETC network, and -A is the flag that predicts that the next symbols pattern will be a separator (whitespace) and the ABI of the desired contract.

We should take the chain id parameter (-ETC / -ETH / -UBQ or -EXP) and throw away anything else.

5.

If everything is fine then we should dropdown the menu to suggest user to save the desired token.

We should display the "SUCCESS" result to the user at the bottom of the token symbol input box.

Save token menu should contain two input fields.

dexns__token_search_dropdown_light

6.

We should call the desired token address with by the default ERC20 ABI to extract token decimals after the "SAVE" button was pressed. Then we should add the desired token to user's tokens at the specified chain.

IMPORTANT: We should add token to the specified chain that was extracted from DexNS name metadata. It doesn't matter if the user is currently on the matching chain or on any of the alternative chains.

Dexaran commented 6 years ago

Multi-currency balance display.

Let CLassicEtherWallet display user balances on different chains: ETH / ETC / EXP / UBQ.

We should perform four calls to access user's balances on different chains when unlocking account/ displaying balances.

We should display the corresponding balance as "active". For example if the user is currently on ETC network then we should display ETC balance with big digits and black color. ETH / UBQ / EXP balances should be displayed with small digits and gray color on ETC network.

Examples

cew_multiple_currencies

cew_multiple_currencies2

yograterol commented 6 years ago

token-cew

https://github.com/EthereumCommonwealth/etherwallet/pull/6

Dexaran commented 6 years ago

Fix the problem of an accidentally transaction sending to the contract in the wrong chain.

GNT contract: https://etherscan.io/token/Golem?a=0xa74476443119a942de498590fe1f2454d7d4ac0d It is deployed on ETH chain

GNT contract address on ETC chain (no contract): http://gastracker.io/addr/0xa74476443119a942de498590fe1f2454d7d4ac0d

This is the same address on ETC chain and it contains 5000 ETC ($60,000 at the moment)

The problem is that Golem (or any other) contract can reject Ether (ETH on Ethereum chain) but there is no contract on ETC chain. If they send ETH to the contract address then the transaction will fail. If they will send ETC/ UBQ/ EXP to the same address (there is no contract on this address on ETC/ UBQ/ EXP chains at all. It's just an address without bytecode) then the transaction will submit and ETC would be lost.

We need:

  1. If the user is trying to send a transaction with >21 000 GAS then he is trying to call something and we need to check if the receiver is a contract on the selected chain or not

  2. Check if the receiver is a contract or not

  3. If the receiver is not a contract then warn the user: "WARNING! It looks like you are trying to call the contract on $chain_name chain. The destination address is not a $chain_name contract. Are you sure want to do it?"

  4. If the user has clicked YES then it's his own problems (may be he is sending hex-encoded message as transaction data) if NO then we have saved his funds

yaronvel commented 6 years ago

Maybe before issuing the warning check that there is a contract on same address in ETH?

Dexaran commented 6 years ago

The desired contract may be deployed on any of the alternative chains (not only ETH). Checking all of the existent chains doesn't seem to be a good idea. Furthermore, if the user has mistyped the address of the ETC contract this will save him as well.

yaronvel commented 6 years ago

Btw, doesnt the wallet calculate gas limit automatically for users?

Dexaran commented 6 years ago

I would say that MEW is calculating gas limit automatically but the user has an opportunity to change it at will.

yograterol commented 6 years ago

DexNS support to add custom tokens

https://github.com/EthereumCommonwealth/etherwallet/pull/10

dexns

Dexaran commented 6 years ago

I suppose that we need to make some changes to avoid user confusion:

  1. Add "Loading..." text or loading animation that will appear to show the user that searching is currently in progress. This should disappear when the response will be retreived.

  2. Start searching (send async requests) 2 seconds after user has typed any character. Token symbols are not limited to 3-characters. For example AE (AEternity token) and SNGLS are not 3 characters length.

  3. When changing the network CEW shows that token is not a valid ERC20 token if this token is deployed on alternative chain. I suppose that this behavior will cause user confusion. I suppose that CEW should remember what network the token uses and hide all the tokens that belong to previous chain when you are switching to new chain.

yograterol commented 6 years ago

@Dexaran I agree, I'll do it

Dexaran commented 6 years ago

It would be nice to allow adjusting gas price in 0.1 GWEI instead of 1 GWEI to ClassicMask and CEW. MEW recently did it: https://github.com/kvhnuke/etherwallet/releases/tag/v3.10.5

Not a critical issue, but nice to have.

Dexaran commented 6 years ago

cew_rineth

CEW displays RinkebyETH and ETH as the same amounts of currencies while they are different separate currencies.

Dexaran commented 6 years ago

Update:

yograterol commented 6 years ago

https://github.com/EthereumCommonwealth/Roadmap/issues/26#issuecomment-338910299 It's not a problem anymore

Dexaran commented 6 years ago

CEW bugs that needs to be fixed:

  1. "Add custom token by token Symbol" starts to search token when you type 3 characters. If you type 4-characters of token symbol then CEW throws error because of the first three symbols are not found even if 4-characters symbol string represents a valid token name.

  2. S-ETH symbol is successfully found but could not be added to token list. I suppose that it's because of - character.

  3. Adding tokens with already-existing symbol results in same tokens added multiple times.

  4. GasPrice slider is too hard to manually adjust. We should replace a static "Gas Price: <amount>" a text with an input to let users to adjust the GasPrice by typing digits.

gasprice

yograterol commented 6 years ago

https://github.com/EthereumCommonwealth/Roadmap/issues/26#issuecomment-345657999 Fixed

Dexaran commented 6 years ago

Merged https://github.com/EthereumCommonwealth/etherwallet/pull/14

Dexaran commented 6 years ago

BUG: CEW automatically evaluates gaslimit to -1 when trying to deploy contract. If I set the gaslimit to 4700000 then contract is successfully deployed.

Dexaran commented 6 years ago

Point 9 resolved with the last update.