Error when trying to add a payable function in erc20

Question

Most of the ERC20 examples on the internet have this function:

// ------------------------------------------------------------------------
// Don't accept ETH
// ------------------------------------------------------------------------
function () public payable {
    revert();
}

However, I would like to deploy an ERC20 token on Ropsten where a user can send ETH and receive my token in return So what I have tried is deleting this function and replacing it with:

uint public sellPrice =0.5;


function setPrices(uint newSellPrice) public onlyOwner {
    sellPrice = newSellPrice;
}


function buy(uint256 payload) public payable {
    uint amount = safeDiv(msg.value, sellPrice);               
   //balances[msg.sender] += amount; //I commented this out cuz im not sure if I need it

    transferFrom(owner, msg.sender, amount);


}

Sadly this doesn't work .. im getting this error : Warning! Error encountered during contract execution [Reverted]

Note: im using metamask to send the Ropsten Ether .. Im not putting any Hex Data (cuz I thought since this is the only paypal function maybe I don't need it? ) plus, I don't know the hex data for this function.. so maybe another question, how to get a hex data for unnamed function?

UPDATE

I have tried the suggest solutions below, and here is the updated contract

epragma solidity ^0.4.20;



 contract SafeMath {
function safeAdd(uint a, uint b) public pure returns (uint c) {
    c = a + b;
    require(c >= a);
}
function safeSub(uint a, uint b) public pure returns (uint c) {
    require(b <= a);
    c = a - b;
}
function safeMul(uint a, uint b) public pure returns (uint c) {
    c = a * b;
    require(a == 0 || c / a == b);
}
function safeDiv(uint a, uint b) public pure returns (uint c) {
    require(b > 0);
    c = a / b;
}

}

  contract ERC20Interface {
function totalSupply() public view returns (uint);
function balanceOf(address tokenOwner) public view returns (uint balance);
function allowance(address tokenOwner, address spender) public view returns (uint remaining);
function transfer(address to, uint tokens) public returns (bool success);
function approve(address spender, uint tokens) public returns (bool success);
function transferFrom(address from, address to, uint tokens) public returns (bool success);

event Transfer(address indexed from, address indexed to, uint tokens);
event Approval(address indexed tokenOwner, address indexed spender, uint tokens);

}

 contract ApproveAndCallFallBack {
function receiveApproval(address from, uint256 tokens, address token, bytes data) public;

}

 contract Owned {
address public owner;
address public newOwner;

event OwnershipTransferred(address indexed _from, address indexed _to);

function Owned() public {
    owner = msg.sender;
}

modifier onlyOwner {
    require(msg.sender == owner);
    _;
}

function transferOwnership(address _newOwner) public onlyOwner {
    newOwner = _newOwner;
}
function acceptOwnership() public {
    require(msg.sender == newOwner);
    OwnershipTransferred(owner, newOwner);
    owner = newOwner;
    newOwner = address(0);
}

}

 contract FToken is ERC20Interface, Owned, SafeMath {
string public symbol;
string public  name;
uint8 public decimals;
uint public _totalSupply;


mapping(address => uint) balances;
mapping(address => mapping(address => uint)) allowed;


function FToken() public {
    symbol = "fff";
    name = "FAdi Token";
    decimals = 18;
    _totalSupply = 1000000000;

    balances[owner] = _totalSupply;
    Transfer(address(0), owner, _totalSupply);
}


function totalSupply() public view returns (uint) {
    return _totalSupply - balances[address(0)];
}



function balanceOf(address tokenOwner) public view returns (uint balance) {
    return balances[tokenOwner];
}



function transfer(address to, uint tokens) public returns (bool success) {
    balances[msg.sender] = safeSub(balances[msg.sender], tokens);
    balances[to] = safeAdd(balances[to], tokens);
    Transfer(msg.sender, to, tokens);
    return true;
}



function approve(address spender, uint tokens) public returns (bool success) {
    allowed[msg.sender][spender] = tokens;
    Approval(msg.sender, spender, tokens);
    return true;
}


function transferFrom(address from, address to, uint tokens) public returns (bool success) {
    balances[from] = safeSub(balances[from], tokens);
    allowed[from][msg.sender] = safeSub(allowed[from][msg.sender], tokens);
    balances[to] = safeAdd(balances[to], tokens);
    Transfer(from, to, tokens);
    return true;
}

function allowance(address tokenOwner, address spender) public constant returns (uint remaining) {
    return allowed[tokenOwner][spender];
}



function approveAndCall(address spender, uint tokens, bytes data) public returns (bool success) {
    allowed[msg.sender][spender] = tokens;
    Approval(msg.sender, spender, tokens);
    ApproveAndCallFallBack(spender).receiveApproval(msg.sender, tokens, this, data);
    return true;
}


uint public sellPrice =5;


function setPrices(uint newSellPrice) public onlyOwner {
    sellPrice = newSellPrice;
}


function buy() public payable returns (bool success) {

    uint amount = safeMul(msg.value, sellPrice);              // calculates the amount

    //balances[msg.sender] += amount;
    if (approve(owner, amount))

    transferFrom( owner,msg.sender, amount);


}


function transferAnyERC20Token(address tokenAddress, uint tokens) public onlyOwner returns (bool success) {
    return ERC20Interface(tokenAddress).transfer(owner, tokens);
}

}

Sadly it didn't work .. Same error (Error encountered during contract execution [Reverted])

Answer 1

When using transferFrom on an ERC20 token, you must first approve the amount to send on the owner's account. This transaction must be completed prior to this transaction being called, and is a known pain point with ERC20 tokens.

There are suggestions like EIP777 to remove this additional step, but none are canonical as of yet.

Answer 2

Two things.

First, this is a misguided approach. You should separate the definition and accounting of a token from the offer/sale of the token. This will leave you with a well-solved token such as openzeppelin, with no changes so you can be confident that much probably works as expected. You will also have a Sale contract.

After the ERC20 is created, the owner/minter/deployer normally possesses 100% of the supply. They use a regular .transfer method to send some or all of the token to the Sale contract. It's the Sale contract's job to exchange tokens on hand for ETH received.

In case it helps, consider a lemonade stand. It's the lemonade stand's job to exchange delicious lemonade (which it must have in the first place) for a certain amount of dollars. The dollars and the lemonade (ETH and tokens) have no inbuilt properties related to the sale process. Attempting to do so would only adulterate the purity of their design.

Second thing. This line gets you into trouble.

uint public sellPrice =0.5;

Since sell price is an unsigned integer, 0.5 = 0. This leads to "divide by zero" in the buy() function. Consider multiplying by 2 (.mul(uint(2)) instead of dividing by 0.5.

Hope it helps.

Update

Shane also has point with your use of transferFrom. You're not using it correctly. However, since the sale process/contract will already, presumably, hold an inventory of tokens to sell, a regular transfer(receiver, amount) should suffice.

Answer 3

The solution was as Rob Hitchens mentioned ... use a simple Transfer method. However, make sure you use the event Transfer, not transfer function as he wrote it (with small t).

the Transfer event takes 3 arguments, while the transfer function takes 2