Called function suddenly needs to be payable

Question

I have a public uint variable denoting 'which round' it is and a function that advances rounds and does processing alongside the round advancement:

       uint public round;

       function completeRound() public inPaused() inRound() {
            if (round == 6) {
              // win
          } else {
            reduceByHalf();
            round.add(1);
          }
          
       }

If i run this in remix, it runs 4 times and then consistently fails on the 5th, indicating that a function suddenly needs to be payable:

transact to Playingwithsmartcontracts.completeRound errored: VM error: revert. revert The transaction has been reverted to the initial state. Note: The called function should be payable if you send value and the value you send should be less than your current balance. Debug the transaction to get more information.

If I comment out round.add(1) right under where reduceByHalf is called, the code works all day long. I can click it indefinitely with no errors in Remix.

Strangely, this started as an Enum to track the rounds and that had the same exact problem. While advancing the enum, i could do it 5 times before the above failure and commenting it out made everything work.

reduceByHalf code doesnt seem to be the offender, but it is shown below in case it has a bearing on the problem:


    struct Foo {
        address owner;
        uint mintedRound;
        uint winningRound;
    }
    struct FooOwner {
        uint[] foos;
        uint totalWinningFoos;
    }

    uint[][5] roundFoos;
    uint[][5] roundWinners;

    mapping(uint => Foo) public winningFoos;
    mapping(address => FooOwner) public fooOwners;

    uint totalWinningFoos;

    function shuffleFoos (uint256[] memory _array) internal view returns(uint[] memory){
        uint[] memory clone = cloneArray(_array, _array.length);
    
        for (uint256 i = 0; i < clone.length; i++) {
            uint256 n = i + uint256(keccak256(abi.encodePacked(msg.sender, block.timestamp))) % (clone.length - i);
            uint256 temp = clone[n];
            clone[n] = clone[i];
            clone[i] = temp;
        }
        
        return clone;
    }
    
    function cloneArray(uint256[] memory _array, uint256 _length) internal pure returns (uint256[] memory) {
        uint256[] memory array = new uint256[](_length);
        for (uint256 i = 0; i < _length; i++) {
            array[i] = _array[i];
        }
        return array;
    }

    function reduceByHalf() internal  {
        uint[] memory clone = shuffleFoos(roundFoos[round]);
      
        uint halfLength = 0;

        halfLength = roundFoos[round].length.div(2);
        for (uint w = 0; w < halfLength; w++) {
           
           uint fooId = clone[w];
           
           roundWinners[round].push(fooId); 
           winningFoos[round].winningRound = round;
           
           address fooOwner = winningFoos[fooId].owner;
           
           fooOwners[fooOwner].totalWinningFoos = fooOwners[fooOwner].totalWinningFoos.add(1);    
        }
        
        totalWinningFoos = totalWinningFoos.add(halfLength);
    }

As far as I know, I am not sending value, and not sure why it only thinks im sending value on transaction execution 5.

Would anyone be able to help me understand what Remix/Solidity is mad about?

I totally must not be understanding somehthing but it looks like it's something about the number 5... I can advance the round to 6, but as soon as I set the uint value to 5 is when I start seeing these problems.... so wierd....

Petr Hejda Petr Hejda · Accepted Answer · 2021-05-02T09:32:02

The transaction has been reverted to the initial state.

This is the important part of the error message in your case.

Note: The called function should be payable if you send value

This is just a note, possibly because this combination often happens. But since your function and transaction doesn't send any value, it doesn't apply to your case.

round.add(1);

This (failing) snippet suggests, that there's supposed to be a library used for uint, but it's not defined. I'm gonna go with the SameMath library, because of the .add() function name and the use on uint. But in theory, it could be any library, SafeMath is just the most probable option in this context.

Mind that round.add(1); (using SafeMath) returns the value of round incremented by 1, but it doesn't store the (incremented) value anywhere. This looks like a typo and the real usage should be round = round.add(1);

Your code doesn't show any usage of the SafeMath library, but also doesn't show the Solidity version, so I'm going to divide my answer into 3 parts.

You're using Solidity 0.8+.

SameMath is not needed, because integer overflow is handled on a lower level, and you can safely replace
```
// even with correctly imported SafeMath, it doesn't update the stored value
round.add(1);
```
to
```
// updates the stored value
round++;
```
You're using Solidity 0.7 or older, and SafeMath for uint256 (not uint)

Change the definition
```
uint public round;
```
to
```
uint256 public round;
```
This way, SafeMath will be used for round and it will allow to use the function .add().

Mind that you might want to also store the incremented value, see the bold paragraph with example above.
You're using Solidity 0.7 or older, and not using SafeMath at all.

You need to import the SafeMath library and then make changes described in the point 2.

Called function suddenly needs to be payable

1 Answers