search

artemii235 / etomic-swap

Etomic Swap Smart Contract allowing ETH and ERC20 atomic swaps on AtomicDex platform.
https://atomicdex.io/
37 stars 19 forks source link

ERC20 test token tx send fails #9

Closed himu007 closed 6 years ago

himu007 commented 6 years ago

The swap between ETH and other coins are working both ways using testnet and mainnet. When we tried swapping ERC20 test tokens, we see swap completed message in console, then the tx from bob in etherscan says pending and it fails after that.

I used {\"coin\":\"\EBR\",\"name\":\"Beereum\",\"etomic\":\"0x354ad7bd20ec1c8919a80d7ff40c854202301e78\",\"rpcport\":80}, in my coins file.

Please check the last transactions got failed from this address. They are related to those swaps. https://ropsten.etherscan.io/address/0x012f7e74120d3d002e8f215ccc6f6f65618f00cb

artemii235 commented 6 years ago

Hello @himu007! Thank you very much for your report. There is additional requirement to perform Etomic swap of ERC20 - Bob and Alice smart contracts should be allowed to transfer ERC20 token from your address before swap starts - it can be done by calling approve ERC20 method. It should be done manually - I will add this functionality to marketmaker a bit later. Also I have noticed that EBR token has 8 decimals. Please use ERC20 with 18 decimals as marketmaker doesn't support custom decimals value yet. Please let me know if you need additional help with this issue.

jl777 commented 6 years ago

can we automatically call the approve? i think we also need to automatically detect misconfigured contracts, though having 8 decimals seems a sane config

artemii235 commented 6 years ago

@jl777 Yes, approve call might be done automatically by market maker. I was also thinking that approve might be called from BarterDex GUI because approve call also consumes gas and end user might want to approve larger amount than single order to have a reserve for further trades. Marketmaker API can check that token approval is sufficient for current order and respond with error if it's not.

artemii235 commented 6 years ago

@himu007 ERC20 integration was done and merged to jl777/dev branch, I'm closing this issue.