Contract call fails and spends enormous amount of gas when attempting to trade against existing order

[burdakovd]

Scenario:

1. I have 18.55 SAI (or so the UI said)
2. I trade against existing big order to buy ETH for these SAI. I use "BUY MAX" button to populate amount of SAI I want to spend, and it populates it to 18.55
3. The confirmation window says that it is going to use $69 in gas costs (but I didn't notice it initially)
4. I confirm the transaction (thanks to Metamask being aware of too high gas limit I reduce the gas limit from 6M to 600k)
5. Transaction fails without clear error message, and $5 in fees are waster
6. Not knowing what is happening, I attempt trade multiple times, and $5 in fees are wasted multiple times

Transactions:
https://etherscan.io/tx/0x2d8cf26141a73ecc6e3461d32928ada858cb7ccc2312c4c722405fe8df28b38d
https://etherscan.io/tx/0xad98a83ea08c05921f4eee8c723b12356164735417b97d776f6dbf9aa3e49697
https://etherscan.io/tx/0x7310903aba9bfe6e14f4d5603a7204fc5c7b051071b6f69e7c434b7914fee35b

Here is what I think is happening:

• JS somehow fails in rounding (because in JS all numbers are 53 bit float)
• JS attempts to spend tiny bit more SAI than I have
• When constructing transaction it somehow knows that it will fail (perhaps by executing it locally), and for some reason instead of showing error message, bumps gas price and gas limit to very high values.
• When contract throws (due to incorrect amount of SAI spent), it burns all the gas (because it is using assert() instead of require() to check balances) - see https://ethereum.stackexchange.com/questions/15166/difference-between-require-and-assert-and-the-difference-between-revert-and-thro
• Luckily Metamask complains, otherwise I would've ended up with $69 wasted in gas costs.

When I manually change amount of SAI to spend to 18.54 (leaving 0.01 as buffer), transaction seems to succeed (still pending, but at least estimated gas cost and limit are reasonable): https://etherscan.io/tx/0xeb4f8fbef2322368a7eca3e8785193c4a2d5197598b66af93a1c102f7fc80ca3

Oasis UI (note gas cost):

Metamask complaining:

[nmushegian]

ABORT opcode didn't exist before byzantium. Leaving this issue open until we double-check that latest version won't drain your gas like this.

See this for context/history: ethereum/EIPs#140

[burdakovd]

Just to clarify, I see three bugs here:

• Javascript "BUY MAX" button attempts to spend more coins than I have which is not visible in UI
• When contract call is going to fail, JS code somehow detects it and instead of telling user about it, suddenly sets gas price and limit to big numbers
• Then finally the contract code is using ASSERT instead of ABORT (which you mentioned).

Do you agree these three bugs are in the scope for this issue?

[nmushegian]

This repository contains the on-chain contract code, I'm not too familiar with the rest and the issue should probably be split up. See: https://github.com/oasisdex