SelfKeyFoundation / Identity-Wallet

Code for the SelfKey Identity Wallet
https://selfkeyfoundation.github.io/Identity-Wallet/
MIT License
57 stars 26 forks source link

Swap token feature #2142

Open andregoncalves opened 4 years ago

CortecKYC commented 4 years ago

@rhdelima Have you finished testing this ticket?

andregoncalves commented 4 years ago

FYI the feature flag is: FEATURE_SWAP_TOKENS=1

rhdelima commented 4 years ago

@andregoncalves Failing this one. Critical error is encountered when clicking Calculate Fees. See recording below. http://recordit.co/92GzdOnV5g

image

build: dev/2020.04.07.12

andregoncalves commented 4 years ago

It seems totle changed one of their API responses. Looking into it.

andregoncalves commented 4 years ago

Totle stopped returning the swap fee on the api response. Not sure if this is because Kyber is not live yet, in any case I made a PR to handle this case.

rhdelima commented 4 years ago

@andregoncalves please see initial issues below:

  1. The default Change To value is KEY however after transaction, the exchange was made to another KEY token and NOT SelfKey. Transaction: https://etherscan.io/tx/0xf1c430066738c9f1709a084a0ae813d20793150bdaca9540b99d9e9a52d867cc image

  2. Missing sent to address image

  3. Shows $0.00 USD value for KEY token (not SK) and NaN on Amount image

andregoncalves commented 4 years ago

Investigating it.

But first impression is that it's an issue with totle because we're not live yet on kyber, and totle swap routing goes to the other "KEY"

Because we can only test on mainnet, it's probably a good idea to hold testing until we're live.

CortecKYC commented 4 years ago

@andregoncalves You mean, hold testing until we're live on Kyber, right?

andregoncalves commented 4 years ago

yes

oanamangiurea commented 4 years ago

Also other issues: https://gyazo.com/97fcdb575654846cbdc5c6e7d91b2eee vs https://projects.invisionapp.com/d/main#/console/15222689/393424599/preview

  1. Button should be called "Exchange"
  2. Amount input correct placeholder is "Amount to swap*"
  3. Missing the correlated token value under amount input (in the example above, KEY value for that given amount)
  4. Reduce the big space at the end of the modal
andregoncalves commented 4 years ago

Regarding the other "KEY" token: https://mattermost.kyc-chain.com/kyc-chain/pl/85q3me3by7dfxqbug5pp97esar

oanamangiurea commented 4 years ago

Trading screen is updatedhttps://projects.invisionapp.com/d/main#/console/15222689/393424599/preview and https://projects.invisionapp.com/d/main#/console/15222689/413340456/preview - in the last one, what feature we were missing, is say I calculated fees for exchanging 1000$, now if I change my mind, and type 1500$ it should go back to 1st screen, with only calculate fees button enabled

andregoncalves commented 4 years ago

2170 handles the "other" key token.

rhdelima commented 4 years ago

@andregoncalves Please see some issues below:

  1. CALCULATE FEES not working for SWAPPED tokens. In the recording below, KNC was a swapped token but when I tried to swap it with other token, CALCULATE FEES button is not working https://recordit.co/HOnbjWLEaD

  2. Unable to swap token with enough balance. See recording: http://recordit.co/EE4abTjYlI

  3. Is it possible to add newly swapped token to My Tokens?

  4. Not enough ETH shows Your Address to receive KEY. Replace KEY with ETH image

andregoncalves commented 4 years ago

Work in progress on fixes to 1,2 and 4. Adding new feature for automatically add the swapped token.

CortecKYC commented 4 years ago

@andregoncalves What's the status of this ticket? What's left to do?

andregoncalves commented 4 years ago

I'm working on this fix: https://github.com/SelfKeyFoundation/Identity-Wallet/issues/2174#issuecomment-624494844 An an issue reported by ralph on mattermost QA channel.

rhdelima commented 4 years ago

@andregoncalves I can't do a successful swap due to high transaction fee. Please see issues below:

  1. Transaction fee is too high. Please confirm if this is correct
  2. When using trezor or ledger, we should show modal informing user to confirm swap transaction on the device.
  3. Even if eth not enough modal is shown, Swap completed modal is still displayed http://recordit.co/KJf5eGcRco