almindor / etherwall

Ethereum QT5 Wallet
GNU General Public License v3.0
142 stars 59 forks source link

ERC20 support #56

Closed almindor closed 6 years ago

almindor commented 6 years ago

Add support for balances and sending of ERC20 tokens

UGatgithub commented 6 years ago

I have seen there is a Token (ERC20) column label in version 2.0.4 under Contracts already. I found in the change log ERC20 support is Work In Progress (WIP) so it is currently in an interim state. If I understand correctly Etherwall will be able to handle e.g. REP tokens (Augur), correct?

almindor commented 6 years ago

So right now etherwall "recognizes" token contracts. If you add an existing contract and it's ERC20 compatible (right now I also expect it to have "symbol" method) it should show it's symbol under the column.

That's it in this version, but you can use methods like getBalanceOf or transfer manually by invoking them and it'll work. ERC20 support is literally a bunch of shortcuts on top of normal contract method calls. E.g. getting of balances for each account via balanceOf etc.

almindor commented 6 years ago

If you're interested in the details have a look at: https://theethereum.wiki/w/index.php/ERC20_Token_Standard

I'm mainly aiming to support balanceOf and transfer in a convenient way so that people will be able to check their account balances for a specific token and send tokens out easier than by invoking the transfer method manually.

UGatgithub commented 6 years ago

Thanks for the information, I will definitely have a look at it. Not sure yet how to keep Augur token balances under control and where to keep REP. Don't see Kraken as a long term solution for that ...

almindor commented 6 years ago

So you can try it out with a small amount, just add the token contract to etherwall first, see if it catches the symbol and all that.

After that send a small amount to an address you control in Etherwall and you can check your balance by invoking balanceOf (that's a free operation).

NOTE: tokens are always stored to an address/account. Even if a wallet doesn't show them if you control the account they're sent to you control the tokens. With kraken and others they are stored on an account controlled by the operator of said exchange.

Rob-2017 commented 6 years ago

hello, if someone send me ERC20 tokens , you say "If you add an existing contract" it will work. how to do that? where can i get info about it. is the contract added automatically if someone send me ERC20? if i do nothing etherwall will show this token somehow under ERC20? in any case the ERC20 tokens are save and related to the etheraddress no matter if etherwall show them or not right?

almindor commented 6 years ago

Hey so contracts are almost similar to normal addresses. Their "code" resides on the network and they are referenced by their address.

Token contracts then store your tokens using their logic and your "caller address" as the "key". So as long as you have the caller address (or the receiver in your case) you're fine.

To add a contract to Etherwall go to the contracts tab and click Add Existing. You then have to know the address of the contract. If the contract is certified open source by Etherscan the JSON interface should fill in automatically, otherwise you have to get that too from whoever made it.

If the contract was successfuly added and detected as being ERC20 compatible you should see your Token's symbol in the ERC20 column in the contracts list.

To check your balance then you select the newly added contract and invoke it. Select the balanceOf function, paste in your address that should have the tokens and execute the transaction (no need to change the defaults in the transactions tab, but you might want to change the from address to the same as the input). You should see the result in the results tab after you send it. NOTE how the balanceOf function is "free to execute" as it's just a read-only state accessor.

Also note, the result will be in smallest denomination, so it might seem huge. Most tokens use 18 decimals similar to ETH so you need to divide by that to know how many "full" tokens you have.

Rob-2017 commented 6 years ago

ok, i will experiment a bit with a small amount. thnx alot for explanation.

UGatgithub commented 6 years ago

Just tried to add the Augur Reputation Contract. After adding a contract name and pasting the address 0xe94327d07fc17907b4db788e5adf2ed424addff6 into the Contract Details Address field when I click OK the system replied with Error: Contract address invalid checksum (Orange Save ! icon). The addresses for Augur I copied from here: New Augur contract: https://etherscan.io/token/REP Old Augur contract: https://etherscan.io/address/0x48c80F1f4D53D5951e5D5438B54Cba84f29F32a5 Afterwards I copied the old Augur contract address over it (Change to the green Save icon) - and again the actual one copied over the old and the Save stayed green this time, so I was finally able to save the new address.

The Token (ERC20) column in Contracts stays empty so even so my blockchain is up to date now.

I also added Augur to Watches and find Augur and Block numbers in Events now where the Event column is empty by now.

UGatgithub commented 6 years ago

I meanwhile sent a small amount of REP to one of my accounts and get it displayed in Ethereum Wallet / Mist as well as on etherscan.io but can't get information in Etherwall.

UGatgithub commented 6 years ago

Deleted and added the Augur contract to see if something changes, but no - still the ERC20 column is empty and the first column in Events also. When I invoke the contract and use the balanceOfI do not get info about my Tokens. From time to time I get a feedback Received event from Contract Augur ()

Almindor, you wrote above:

If the contract is certified open source by Etherscan the JSON interface should fill in automatically, otherwise you have to get that too from whoever made it.

What exactly do you mean by "fill in automatically"? The symbol?

almindor commented 6 years ago

Hey, thanks for taking the time to test a WIP "feature" :)

So it seems I made a change in between that broke the symbol/ERC20 detection, but I want to explain all the stumbling points you had:

  1. Any destination address is now enforced to be checksum-ed properly (ERC55). The reason is to prevent bad sends due to mistyping/mispasting. To see a sad example of that just look at issue #27 I decided to block non-checksumed addresses after that. If you have an all-lowercase address to use, just put it in etherscan and copy the capitalized version in.

  2. You don't need to own tokens to actually test balance checking. You can use someone else's address, just find one in etherscan that has given token and paste it in the balanceOf input of the added contract. This will work regardless of the token column detection.

  3. The contract interface (the JSON) is taken automatically from Etherscan by Etherwall if the given contract is "certified opensource". If you type in the proper address the interface should fill-in. Example such contract is Golem: https://etherscan.io/address/0xa74476443119A942dE498590Fe1f2454d7D4aC0d If you type in just 0xa74476443119A942dE498590Fe1f2454d7D4aC0d you should get the interface filled in.

  4. Events will show any event reported by contracts for any address by default. You can use topics to filter them but Etherwall currently only support the "binary" token input so you'd have to have them pre-built. This means that you get to see every event from anyone that executes something on the contract. You should get those in the events list and be able to check the details (outputs etc.)

I'll fix the symbol detection by next release. Note that some ERC20 tokens DON'T supply it at all and I plan on adding an editbox to provide the symbol name in those cases. This is all a WIP right now.

UGatgithub commented 6 years ago

Hi, regarding

  1. I had a look at #27 and actually the address was wrong in start and end their, not only an issue of capital and small letters, but anyhow good to know that this is important to better take care of capital and small letters when sending Ether.
  2. then I should be able to check e.g. the KrakenREP address 0xA2a8f158aed54CE9A73d41EEEc23Bf3a51b5654D but I do get a Value of 0 in Results for that as for my own contract. FYI: When I close and reopen the Call contract dialog box there appears an Invalid Input: Incorrect amount of parameters passed to function "balanceOf" got 0 expected 1 which disappears if I remove and renter the last character of the address. Also tried it for Golem, same result. The Value field should contain the balanceOf, correct?
  3. Ok now I understood you mean it fills in the Interface field, yes that it also does for the Augur contract
  4. Understood and checked and found the following: The first field Name is empty in the Event details dialog box as well as in the table where also Type and Value are empty.
almindor commented 6 years ago

Hmm I just tested on Mac and it works with the REP contract and the KrakenREP address for me.

I've added contract 0xE94327D07Fc17907b4DB788E5aDf2ed424adDff6 (REP) and it even detected the symbol. Then I invoked balanceOf with input 0xA2a8f158aed54CE9A73d41EEEc23Bf3a51b5654D and got a result of 597373479201111913505319 (that's in the lowest denomination)

NOTE: this is on Mac OS X version. I'll try Linux and Windows later. I think I saw the symbol detection fail on linux, maybe there's something wrong that only happens on non-mac.

almindor commented 6 years ago

Also thanks for finding the repeat call bug, I can confirm that happens on mac too, it goes to invalid state and you have to change the input currently. I'll make it wipe the old data so you have to re-input.

UGatgithub commented 6 years ago

If there is something else I can test for you, let me know. Everything that I give feedback on is regarding Windows 10 Pro.

UGatgithub commented 6 years ago

Do you know OmiseGo? Just found I own a few tokens I received them about 2 days ago from its smart contract but don't know why. lol

almindor commented 6 years ago

Found out there's an odd bug in Linux and Windows with data encoding when sending function calls to the node. It seems it all works ok in Mac OS X. It's really odd because the code is 100% same between the platforms. Going to investigate further.

I heard about OmiseGo, but don't really know what it's about.

almindor commented 6 years ago

Turns out the windows/linux vs mac was a Qt version problem due to this QT5 bug which was fixed in Qt5.9. I'm still using older Qt5 in Mac due to some deploy compatibility reasons so the older mac Qt5 was producing the "bugged" keccak instead of SHA3 hashes.

Ethereum uses Keccak as well. There's a lot of confusion due to this in JS libraries too. I'll have a fix by next release as I already have an internal keccak implementation for the address checksums. I just need to use it for everything I used Qt5's hashing before too.

UGatgithub commented 6 years ago

Thanks for the feedback, another learning for me as I did not know about SHA-3 (Keccak) functions before. Really odd that Windows and/or Linux but not Mac are effected by this QT 5 bug. Following your explanation I will wait for the next release and test again.

almindor commented 6 years ago

So it's not about windows vs mac version of Qt5 but about which major version of Qt5 is used.

In windows I use 5.9 (latest) which "fixed" their SHA3 hash functions. This "fix" actually broke my functionality because up until 5.9 they actually used Keccak as the hash (Keccak is the precursor to the SHA3 standard).

So if I compile Etherwall 2.0.x with Qt5.8 or older everything works ok, if I use Qt5.9+ it will break on the data parsing due to using the hash functionality.

I've already pushed a fix to master that just changes the function to use my internal implementation so it won't matter which Qt5 version is used anymore. I'm just a bit disappointed in how silently Qt5 "fixed" this with no major notice.

Rob-2017 commented 6 years ago

hi, im also on windows (win7x64) so i have all the same issues as gatgithub sayed. I see contract code in the interface (i used it with EOS token) but if i invoke and set "ballance of" i get this "...got 0 expected 1" error. u think there is a chance for etherwall to become a wallet with full monitor and sell feature for erc20 in 2017? i would realy like to use this pure wallet for the tokens i will get in near future.

almindor commented 6 years ago

ERC20 is going to happen before 2018.

If you can please test this pre release of 2.0.7 on windows. It includes geth 1.7.0 so I wanna make sure everything still works.

You'll need to remove and re-add token contracts.

Thanks

Rob-2017 commented 6 years ago

hi, with 2.0.7. and new geth, etherwall shows "cant connect to node" (i have no blockchain on my comp atm so i can only use thinclient if it matters). if i point to old geth (1.6.7) it can connect with the same behavior when it comes to contracts. rgds

Rob-2017 commented 6 years ago

edit: sry i missed that it shows same error (incorrect amount of parameters passed to funktion "ballanceOf" got 0 expected 1) but the ERC20 column is no more empty it shows 454f53 followed by like 60 "0". also the behavior of etherwallet changes. after i tried 2.0.4 with new geth 1.7.0 with same results(no connection) now i was switching back to 2.0.7 and geth 1.0.7. after restart i get infinite instances of etherwallet starting and the exact error it shows is: unable to establish ipc connection to geth. fix path to geth and try again. WS: QLocal::Socket: connectToServer: invalid name . switching back to geth 1.6.7. make the wallet connect again.

almindor commented 6 years ago

@Rob-2017 do I understand it correctly that with Etherwall 2.0.7 using the bundled geth 1.7.0 you get cannot connect to node? And if you point the same Etherwall to use the 1.6.7 geth that part works but you get funky result from the contract symbol if you add the contract?

I'm just double-checking I got it right. It works for me as bundled in windows 8, so this is good to know before making a new release.

Can you make a screenshot of the contract line with the 2.0.7/1.6.7 combo if you re-add the contract? Which token is this?

Rob-2017 commented 6 years ago

hi, ok i think its because geth 1.0.7 is not installed. i only set the path to 1.0.7 in etherwallet ? 1.6.7 came with installation. i dont see 1.0.7 in headline of etherwallet, just "Etherdyne Ethereum Wallet 2.0.7 []". with 1.6.7 i see ""Etherdyne Ethereum Wallet 2.0.7 [Geth/v1.6.7- stable- ab564c5/windows-amd64/go1.8.3] The multiple instance thing dont happend anymore after i rebooted windows. maybe it was just because of mixing all the geth and etherwallet versions confused the os. also im not sure where "cant connect to node " appeared, i dont see it no more, if i remember right it was no error-window it was just in the bottom line. now if i point to geth 1.0.7 i see only a error-window "unable to establish ipc connection to geth. fix path to geth and try again." followed by "WS:Disconnected from websocket". i added a screen of 2.0.7/1.6.7 combo after re-adding the contract, invoke it and choose ballanceOf. as contract-name i just pasted what etherscan shows = amount and type of token. btw after moving walletwindow the contract-name is not shown correct anymore in the name-column as u see in the screenshot. on the invoke-button it shows the correct name. img 2 0 7-1 6 7

@Rob-2017 do I understand it correctly that with Etherwall 2.0.7 using the bundled geth 1.7.0 you get cannot connect to node? And if you point the same Etherwall to use the 1.6.7 geth that part works but you get funky result from the contract symbol if you add the contract?

forgot to mentoin... yes that is correct.

almindor commented 6 years ago

Can you send me the contract address from the screenshot please? I can't make the complete string. There's def. something wrong with the symbol detection there. The input error is OK as you didn't fill in the address to check balance of in there.

When it comes to the geth issue, can you please try pointing to 1.7.0 again, start and do a clipboard copy of the geth log? Go to logs/geth and press copy to clipboard then paste here in a comment. You can use the code tags for it (three backticks, the key above tab used as quotes around the whole paste)

NOTE: let it error out with cannot connect before going to the logs first. Maybe even wait a few seconds after that, I want to know what geth is doing all the time.

UGatgithub commented 6 years ago

I just installed above linked 1.0.7 and use it with Geth 1.6.7 by now. After removing and adding again the following contracts I get the symbol for Golem (GNT) and Augur (REP) but not for OmiseGo which is according to my understanding also a ERC20 token (JSON interface loads when adding the address). I once also was able to get the balanceOf for my Account with Augur. Events from Golem and Augur appear now with Event column getting filled, after adding OmiseGo to the Watchlist now also Events from that appear, but the Symbol and therefore maybe therefore also balanceOf for that Token does not work,

Currently my blockchain is syncing, when done I will test further and give feedback here.

UGatgithub commented 6 years ago

Interface parcer error when trying to add Cryptonex Contract (ERC20) (just found one Token added for this to one of my addresses) Contract address: 0x94d6b4fB35fB08Cb34Aa716ab40049Ec88002079

UGatgithub commented 6 years ago

Multiple screen freezes when deleting / adding contracts during running sync. - always used the right top X and was able to close Etherwall by that.

almindor commented 6 years ago

So the sync delays on operations are normal. Generally speaking if it's syncing you shouldn't be able to do anything that needs a request, I'll check that. I know that geth sometimes "intermittently" stops syncing and then resumes which makes these enabled for the time being.

I'll check Cryptonex in a sec. seems their JSON ABI contains something odd. You got it filled in automatically right?

UGatgithub commented 6 years ago

No, Cryptonex did not load the Interface, directly the error. OmiseGo loaded but without symbol.

Just had a Etherwall.exe Runtime Error while being away from the PC during sync.

UGatgithub commented 6 years ago

Meanwhile switched to Geth 1.7.0 - Upgrading chain index took a while. At 81 % I got one WARN: Skipping deep transaction reorg depth=104, sync continues ... Same WARN later with depth=2048 again

Rob-2017 commented 6 years ago

Hi, 0x86Fa049857E0209aa7D9e616F7eb3b3B78ECfdb0 is the address. Not much in my geth 1.7.0 error log only this (no matter if i start wallet as admin or not): ``Fatal: Error starting protocol stack: Access is denied.

Rob-2017 commented 6 years ago

@UGatgithub so u see geth 1.7.0 in headline of wallet and its working for you? need just to set the path and no further action needed?

UGatgithub commented 6 years ago

Yes I use Geth 1.7.0 with Etherwall now but did not change the path in Etherwall. I have geth installed to a separate folder so I just closed Etherwall, renamed the Geth folder to Geth163 and dowloaded Geth 1.7.0 from here which I than again installed to the previous used path: C://Program Files/Geth.

I just found is that my he blockchain folder is 189GB ... too big ... can I just delete and recync fast and what would the size then be?

Rob-2017 commented 6 years ago

hmm back to 2.0.7+ 1.7.0 i just added contract adress to watch and see that the name and the address is reversed in watches ?? is that ok? @UGatgithub hmmm by installed u mean copied the new geth in folder of old geth and used the old names? 189 gb for blockchain ? afaik it should be max 50 or 60 gb or less. check disk

almindor commented 6 years ago

Fully resynced using --fast should be ~40gb or so. 1.7.0 geth should make it even smaller.

NOTE: if you have old chaindata from 1.6.7 and upgrade geth to 1.7.0 it does an "upgrade" which I think can stall it long enough for Etherwall to fail with the can't connect error. Shouldn't happen if you only used thin client tho.

Not sure about the access denied error.

almindor commented 6 years ago

@UGatgithub for the contract 0x94d6b4fB35fB08Cb34Aa716ab40049Ec88002079 how did you obtain the JSON ABI interface? You need it if you want to add it.

almindor commented 6 years ago

@Rob-2017 not sure what you mean by reversed here. Can you be more specific? The watches table has a watch name column and contract should show the contract name (whatever you named the contract when you added it).

UGatgithub commented 6 years ago

OK then that's the reason for the interface parcer error lol

almindor commented 6 years ago

Yes heh, so you need to obtain that. I only auto-fill the ones where Etherscan.io has the sources. I should maybe add a more explicit error with a FAQ link there.

Confirmed that OmiseGo is not detected as ERC20, looking into why now.

UGatgithub commented 6 years ago

Can I just rename the chaindata folder (keep as backup until newly synced) and will it then restart sync when I start Etherwall?

almindor commented 6 years ago

You can but make sure you have enough space for each.

Rob-2017 commented 6 years ago

its just confusing that in watches the address is in name column and in contracs not.

UGatgithub commented 6 years ago

Regarding Cryptonex I did not find the JSON ABI Interface on their Website yet: https://cryptonex.org/ maybe it can be found here? https://github.com/Cryptonex?tab=repositories Their wallet looks more like a Bitcoin wallet, but I received a Token on Ethereum - strange.

Rob-2017 commented 6 years ago

ok, cant get geth 1.7.0 working. now i used same source as ugatgithub with installation in same folder. i also tried if it matters to uninstall older geth first and switch thinclient on/off but still "Fatal: Error starting protocol stack: Access is denied." it looks like i should try with win 10. need to get to my other comp tomorow atm im on win7x64. if u have news about the name/token issue of the EOS contract pls let me know ty.

almindor commented 6 years ago

@Rob-2017 So you named your watch with the address? That's what it seems.

@UGatgithub no idea, but whoever created the token has to have the interface.

almindor commented 6 years ago

When it comes to OmiseGo the reason why Etherwall rejects it as ERC20 is because they changed the transfer function return value.

In the ERC20 standard, the return value is supposed to be a boolean, but they removed it and it doesn't return anything with OmiseGo. I'm considering adding an exception for this.