Fix uo

[Georgi87]

A fix for the front running issue in the UO discovered by @niran

(For some reason I cannot add you as reviewer @niran. I guess because you are not part of the Gnosis organization?)

[cag]

totalAmount >= totalOutcomeAmounts[_outcome]
spreadMultiplier >= 2 >= 1

maxAmount = sM * (tA - tOA[i]) - tOA[i] == sM * tA - (sM + 1) * tOA[i] >= 0 when
tA >= ((sM + 1) / sM)  *  tOA[i]

Edit: originally, I had a minus instead of a plus sign, and that caused my math to say that this expression would work no matter what, but I've since corrected that.

[cag]

Oops =___= I made an error in my analysis. Lemme see if there is still an edge case.

[cag]

Okay, so (totalAmount - totalOutcomeAmounts[_outcome]).mul(spreadMultiplier) could be less than totalOutcomeAmounts[_outcome] if somebody was already voting for an outcome which is far enough in the lead, like if somebody voted for the challenge outcome immediately after the outcome was set. In that case, the oracle would reject the transaction instead of doing a 0 transaction, which is what would happen if a person over-committed to a vote. However, these seem to be analogous situations to me.

I recommend adding a test section under the challenge:

        // Sender 1 tries to increase their bid but can't
        await etherToken.deposit({value: 50, from: accounts[sender1]})
        await etherToken.approve(ultimateOracle.address, 50, { from: accounts[sender1] })
        await ultimateOracle.voteForOutcome(2, 50, { from: accounts[sender1] })
        assert.equal(await ultimateOracle.outcomeAmounts(accounts[sender1], 2), 100)

And clamping the subtraction so the maxAmount gets set to 0 when the subtraction would underflow.

I made a mistake

[niran]

Looks good to me! That additional test is a good idea.

[Georgi87]

Thank you both for checking, I will add the test.

[Georgi87]

Added a test for this @cag

The transaction throws, because the .sub method throws.

[codecov-io]

Codecov Report

Merging #28 into master will increase coverage by 0.22%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #28      +/-   ##
==========================================
+ Coverage   84.78%   85.01%   +0.22%     
==========================================
  Files          25       25              
  Lines         631      634       +3     
  Branches      105      106       +1     
==========================================
+ Hits          535      539       +4     
+ Misses         96       95       -1

Impacted Files	Coverage Δ
contracts/Oracles/UltimateOracle.sol	100% <100%> (+1.96%)	⬆️
contracts/Markets/StandardMarket.sol	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 85a5077...e979337. Read the comment docs.