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?

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.

  • 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
  • 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. – 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

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.