# Smart contract : wrong code or race condition?

We have developed a simple Smart Contract which offers this following public transaction method:

``````struct Bid {
uint256 userCode;
uint256 amount;
}

Bid public winningBid;
Bid[] public bids;

function bidAmount(uint256 _userCode, uint256 _amount) public {

assert(_userCode> 0);
assert(_amount> 0);
assert(_amount > winningBid.amount + winningBid.amount * (5/100));

winningBid.userCode= _userCode;
winningBid.amount= _amount;

var bidData=Bid(_userCode, _amount);
bids.push(bidData);
}
``````

The `bidAmount` method checks if the bid is valid verifying if the amount is bigger than the amount of the current winning bid plus a mandatory amount step `(winningBid.amount * (5/100))` ; if the check is verified, the winning bid becomes the current winning bid and a new bid is pushed in the bids list.

How is it possible that we've been able to store in ethereum a bidding list like this?

``````22/01/2018 11:51 13.500,00 MrX
22/01/2018 11:51 13.440,00 MrY
22/01/2018 11:49 12.800,00 MrZ
``````

MrX bid violates the Smart Contract check:

``````assert(_amount > winningBid.amount + winningBid.amount * (5/100));
``````

It looks like a "race condition" where `MrX` and `MrY` biddings were both done just checking the validity on `MrZ bid`. What we expected is to have one of the two biddings fails for assert violation.

To the best of my knowledge, contract executions are serialised within a block and are not performed in parallel.
Is it simply bad coding or a race condition?

## 1 Answer

Ethereum only uses integer arithmetic and will truncate mathematical operations.

The expression, will calculate 5/100 first which is 0.

``````winningBid.amount * (5/100) = winningBid.amount * 0 = 0
``````

The solution is to force the order of the operations to first multiply

``````require(_amount > winningBid.amount + (winningBid.amount * 5) / 100);
``````

Also it is suggested to use `require` instead of `assert` to validate input, `assert` is for totally unexpected circumstances. After byzantinum `require` will cause a `revert()` returning the unused gas to the sender, and `assert` will consume all the remaining gas.

• Uhm, that's the problem. But your suggested solution does not seem correct too. It would invalidate a correct amount due to the truncation. Having for example 210 as last bid, a licit bid of 220,9 would be rejected. – systempuntoout Jan 23 '18 at 16:03
• Reading from the documentation `Division on integer literals used to truncate in earlier versions, but it will now convert into a rational number, i.e. 5 / 2 is not equal to 2, but to 2.5.` Do you think I am using an old version? – systempuntoout Jan 23 '18 at 16:13
• @systempuntoout If you are using integers you will not be able to send 220.9. Until now to increase the precision the common solution is to use a fixed point arithmetic. I think currently the support for rationals is incomplete. For example 5 / 2 internally is 2.5 but you cannot assign it to a variable, there's no `rational` type yet. – Ismael Jan 23 '18 at 17:34
• Yes, we are already multiplying the import * 1000 . – systempuntoout Jan 24 '18 at 14:46
• @systempuntoout If you use fixed point arithmetic * 1000 then the proposed solution should work. If `A = 210000` and `B = 220900`, then `A + A * 5 / 100 == 211050`, and `require(B > A + A * 5 / 100)` will be satisfied. – Ismael Jan 24 '18 at 20:55