protofire / omen-exchange

Omen exchange
https://omen.eth.link/
GNU Affero General Public License v3.0
66 stars 77 forks source link

App crashes after buying outcome tokens #2152

Closed pimato closed 3 years ago

pimato commented 3 years ago

Describe the bug It looks like the app is crashing after user does several buys on an outcome (smaller buys leading with bigger buys). According to @KadenZipfel and @Mi-Lan it seems to be related with @hexyls multicall PR as it calls functions which should not be called after a trade has happened.

To Reproduce Steps to reproduce the behavior:

  1. Create a market with two outcomes.
  2. Buy outcomes with small amounts
  3. Buy outcomes with bigger amounts
  4. BUG: App crashes

Expected behavior App should not crash after a user has bought outcome tokens.

Screenshots

Bildschirmfoto 2021-08-05 um 12 23 25
kadenzipfel commented 3 years ago

We can see why this happens in the below snippet from computeBalanceAfterTrade

const newPoolBalances = holdings.map((h, i) => {
    return h.add(amountCollateralSpent).sub(i === outcomeIndex ? amountShares : bigNumberify(0))
  })
  if (newPoolBalances.some(balance => balance.lte(0))) {
    throw new Error(`Trade is invalid: trade results in liquidity pool owning a negative number of tokens`)
  }

For each holding, we add the amount to spend in collateral (buy amount input) and subtract the amount of shares the user should receive their purchase amount. We then check that each of these returned balances are > 0 and error if not. The problem lies in the fact that after the transaction, these values don't necessarily all update at the same time, and thus we can end up with an invalid return value, e.g. < 0.

This is not a problem because we're computing the amount of shares that will be received from a trade that is not actually being made since it is only after the transaction. Within a second or two the issue resolves itself once all the necessary data becomes available.

To resolve this, I'm simply changing this error to be a warning since if we actually were to have an issue related to this function, the error would simply revert at the cpk level.