transferFrom in ERC20 and ERC721

General
1

Hi there, in ERC721, the function transferFrom does check if from is the same with sender, however, in ERC20, the function transferFrom does not check that, but only require allowance is sufficient, even for the sender himself, i.e., to use transferFrom for an ERC20 token, the allowance of sender to sender must be bigger than amount. Is this redundant? Would it make sense to check if sender is owner in ERC20 as well just like in ERC721?

2

Found this function in a contract in the repo of Pancakeswap here, the transferFrom function is implemented similar to what is described above.

function transferFrom(address src, address dst, uint wad)
public
returns (bool)
{
    require(balanceOf[src] >= wad);

    if (src != msg.sender && allowance[src][msg.sender] != uint(-1)) {
        require(allowance[src][msg.sender] >= wad);
        allowance[src][msg.sender] -= wad;
    }

    balanceOf[src] -= wad;
    balanceOf[dst] += wad;

    Transfer(src, dst, wad);

    return true;
}
3

Do you mean _isApprovedOrOwner()?

Cause ERC721 token is Non-Fungible Token, so before transfer, should check whether from is the owner of the id token, and ERC20 token is Fungible Token, so before transfer, should check the token balance of from.

1 Like
4

What I meant was for an ERC20 token, the transferFrom function should not check allowance if address sender is the msg.sender. It would be redundant to increase the allowance of sender to sender in order to use transferFrom function for an ERC20 token.

5

If when call transferFrom, the sender is the msg.sender, that means msg.sender approves msg.sender to transfer token, so why not msg.sender just call transfer to transfer token, so I think msg.sender approves msg.sender to call transferFrom does not make sense.

6

I think it’s just a generic function for all callers in some DApps, irrespective of whether the caller is the owner. If it happens the owner is the caller, and the approved value is not set yet, this could revert with an error. Not mentioning delegate calls. I am just assuming.

7

Yeah, users call approve, and then dAPPs can call transferFrom

8

I hope I’m making myself clear on this, but if I fail, then I fail.

This snippet of code is saying if the sender is the owner in transferFrom function, let’s skip the step of checking allowances, hence there is no need to do approve sender to sender prior. This is a subtle change, and I think it makes sense.

    if (src != msg.sender && allowance[src][msg.sender] != uint(-1)) {
        require(allowance[src][msg.sender] >= wad);
        allowance[src][msg.sender] -= wad;
    }
1 Like
9

I think in your shared code, the function transfer() will reuse the code of the transferFrom:

function transfer(address dst, uint wad) public returns (bool) {
        return transferFrom(msg.sender, dst, wad);
    }

function transferFrom(address src, address dst, uint wad)
    public
    returns (bool)
    {
        require(balanceOf[src] >= wad);

        if (src != msg.sender && allowance[src][msg.sender] != uint(-1)) {
            require(allowance[src][msg.sender] >= wad);
            allowance[src][msg.sender] -= wad;
        }

        balanceOf[src] -= wad;
        balanceOf[dst] += wad;

        Transfer(src, dst, wad);

        return true;
    }

so in the function transferFrom(), it needs to discharge the condition: msg.sender is equal to src, and as for the same functions transfer() and transferFrom() in the OpenZeppelin, they reuse the function _transfer(), so ideally, msg.sender will not be equal to src in the function transferFrom(src, dst, amount), what do you think?

1 Like
10

Yes, OZ reuses _transfer, and this piece of code reuses transferFrom. In the OZ version below, if sender is msg.sender, then _allowances[sender][sender] will be checked, which will usually be 0. And that was my concern. And your point is, since sender and msg.sender are the same, transfer instead of transferFrom would be used and the issue I just raised would be avoided. Am I on the same page with you now?

function transferFrom(address sender, address recipient, uint256 amount) public virtual override returns (bool) {
    _transfer(sender, recipient, amount);

    uint256 currentAllowance = _allowances[sender][_msgSender()];
    require(currentAllowance >= amount, "ERC20: transfer amount exceeds allowance");
    unchecked {
        _approve(sender, _msgSender(), currentAllowance - amount);
    }

    return true;
}
1 Like
11

Yes, we are now on the same page.

1 Like