Payable function error: The called function should be payable if you send value and the value you send should be less than your current balance

Question

I want to create simple auction contract (see code below). But newBid() function is not working.
To run this contract, you must first create an item newItem(name, price, time) and then run newBid(itemId) - check the code.

There is 2 files`

Auction.sol

pragma solidity ^0.7.0;
import "./Item.sol";

contract Auction {

    uint256 lastId = 0;
    mapping (uint256 => Item) idToItem;

    address admin;

constructor () {
    admin = msg.sender;
}

function newItem(string memory _name, uint256 _price, uint256 _time) public {
    lastId += 1;
    idToItem[lastId] = new Item(_name, _price, msg.sender, _time);
}

function newBid(uint256 itemId) public payable {
    Item item = idToItem[itemId];
    
    item.newBid(msg.sender, msg.value);
}

function timeLeft(uint256 itemId) public view returns(uint256){
    return idToItem[itemId].timeLeft();
}

function getMaxBid(uint256 itemId) public view returns(uint256) {
    return idToItem[itemId].getMaxBid();
}

}

Item.sol

pragma solidity ^0.7.0;

contract Item {
    // Global fee is 14% of item price  
    uint256 fee;

    // Main Parameters of item
    string name;
    uint256 price;
    address payable public owner;
    uint256 time;
    bool sold; 

    uint256 maxBid;

    struct Bid {
        address payable bider;
        uint256 amount;
    }

    Bid[] Bids;

    constructor(string memory _itemName, uint256 _price, address payable _owner, uint256 _time) {
        name = _itemName;
        price = _price;
        owner = _owner;
        time = block.timestamp + _time;
        sold = false;
    }

// Returns item ramaining time (in seconds)
function timeLeft() public view returns(uint256) {
    return time - block.timestamp;
    
}

// Registers new bid for this item.
function newBid (address payable _bider, uint256 _amount) public payable {
    if (block.timestamp + time > time) {
        sellItem();
        require(block.timestamp < time, "Sorry, time is up");
    }else {
        require(_amount > maxBid, "The bid must be higher than the current maximum bid");
        Bids.push(Bid( _bider, _amount ));
        
        fee = (_amount / 100) * 14;
    }
}

// Sells item and refunds money
function sellItem () private {
    address _maxAmountAddress;
    uint256 _maxAmount = 0;
    
    for (uint i = 0; i <= Bids.length; i++) {
        if (Bids[i].amount > _maxAmount) {
            _maxAmountAddress = Bids[i].bider;
            _maxAmount = Bids[i].amount;
        }
    }
    
    for (uint j = 0; j <= Bids.length; j++) {
        if (_maxAmountAddress != Bids[j].bider) {
            Bids[j].bider.transfer(Bids[j].amount);
        }
    }
    
    
    owner.transfer(_maxAmount - fee);
    sold = true;
} 

// Returns max bid for the item
function getMaxBid () public view returns(uint256) {
    return maxBid;
}
}

Answer 1

"The called function should be payable" is a wild guess of a hint and usually wrong. The contract has reverted for any number of reasons. You will have to use debugging techniques to zero in on the actual cause which could be array index out of bounds, divide by zero, one of the requires isn't happy, etc.

I made it as far as Item.sellItem(). As a general observation, there is a little too much going on in this one transaction. You should try to simplify and eliminate the for loops.

It looks like you are using Remix. The debugger might help you zoom in on the actual problem. You might see the problem if you step over the code until it blows up.

Hope it helps.

Answer 2

The error "The called function should be payable" when compile a contract from REMIX is a general error that indicates an excess of GAS cost in the transaction (normally infinity due to error).

This could happen when the function calls a variable or array without a value yet assigned.

In your case, maybe idToItem was declared but this idToItem[itemId] has not a value yet.

I wait helps you.