0

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?

Improve this question

1 Answer 1

Reset to default
2

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.

Improve this answer
5
  • 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. Commented Jan 23, 2018 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? Commented Jan 23, 2018 at 16:13
  • 1
    @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. Commented Jan 23, 2018 at 17:34
  • Yes, we are already multiplying the import * 1000 . Commented Jan 24, 2018 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. Commented Jan 24, 2018 at 20:55

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.