search

OasisDEX / eth2dai

Eth2Dai is a fully on-chain marketplace for Ether and Dai
https://eth2dai.com
GNU Affero General Public License v3.0
48 stars 10 forks source link

After performing a successful "Average price fill or kill" order type, somehow UI may revert back to "Limit" order type #198

Open knocte opened 5 years ago

knocte commented 5 years ago

I'm a regular user of eth2dai.com and had never any problems with it, until today. I was selling WETH for DAI, and the first transaction went well:

https://etherscan.io/tx/0x0ab8f8be20b698360f0d81384ffc0bfc2687977c8f02fffbcfc7ce13134b2089

(Using the "Average price fill or kill order type" with a slippage limit of 5%.)

However, I did a second trade to sell the rest of my WETH, and the DAI never came back! The transaction was:

https://etherscan.io/tx/0xf36b80d352a5cc8576a474d6a18e8a7bbf7eae78ae8a6bcda3317f86458398b4

At first I was scared that there was some bug regarding the "atomicity" of the smart contract, but in the end what had happened is that I had sent a limit order instead of an "average or kill" one (once I canceled the order, I got my WETH back). So then I'm sure there's a bug somewhere in the UI that changed the order type automatically back to the default.

krzkaczor commented 5 years ago

@knocte my quick assessment shows that the second trade was not done by using "average price fill or kill" order type — it used offer method. It looks like id of a new order is: 10391

knocte commented 5 years ago

I see, so I can cancel that order? let me double check.

krzkaczor commented 5 years ago

@knocte yes, you should be able to see it in "my trades" in eth2dai.com interface. Open or closed depending if it was already filled.

krzkaczor commented 5 years ago

Btw. once you confirm that it's there can u describe in details what happened between first and second trade? Because this may be a UI issue if order type changes silently when the user doesn't expect it.

knocte commented 5 years ago

yes, you should be able to see it in "my trades"

True, once I cancelled, I got my WETH and could re-trade them.

can u describe in details what happened between first and second trade?

Well, I don't remember exactly, but I'm sure I never chose "Limit order type", because I'm never interested in that. So for sure there's a UX bug somewhere that made the UI default to Limit order type after I had done the first trade. Maybe it has to do with the fact that the first 2 or 3 transactions I sent resulted in failed transactions (maybe I was trying to sell too many WETH with too small slippage limit), you can see this in etherscan.

I'm going then to edit the details of the issue to make it more like a UX problem, and remove the "WETH vanishing" bits. Thanks for the quick answer.

krzkaczor commented 5 years ago

@knocte okay, thanks. We will triage it and make part of the next sprint.

Let's keep this ticket open until it's fixed.

peculiarity commented 5 years ago

@knocte Can you recall if you had refreshed the page somehow ( accidentally or on purpose ) ?

knocte commented 5 years ago

Can you recall if you had refreshed the page somehow ( accidentally or on purpose ) ?

Not sure. Maybe I did?

If you used a throw-away 30min timeout-cookie I guess you could store these settings on the server side to prevent from reloading problems?

knocte commented 5 years ago

If you used a throw-away 30min timeout-cookie

Another option is show the Order types as two unfilled (no default) radio buttons. If not one is chosen, put a visual alert that it's a required element to fill in the form when about to send the order.

peculiarity commented 5 years ago

Can you recall if you had refreshed the page somehow ( accidentally or on purpose ) ?

Not sure. Maybe I did?

If you used a throw-away 30min timeout-cookie I guess you could store these settings on the server side to prevent from reloading problems?

We don't have a backend. Another point to be considered is that right now you think like this but there might a case where you specified AFOK now but in a week you decide to use the exchange again and don't pay attention much to the order type and want to place a Limit Order. There might an issue with that as well.

At the end it's good to reset the state after refresh/revisit. That way the state is deterministic/predictable.

knocte commented 5 years ago

At the end it's good to reset the state after refresh/revisit. That way the state is deterministic/predictable.

Yes, but resetting to an invalid default should be better, like I suggested with the radio buttons in my last comment.