In OpenZeppelin's implementation of ERC20's transferFrom, why is _transfer() called before withdrawer allowance is checked

Question

This is taken from OpenZeppelin's ERC20 transferFrom implementation:

    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");
        _approve(sender, _msgSender(), currentAllowance - amount);

        return true;
    }

Is there a reason why _transfer is called prior to checking the withdrawer (msg.sender) allowance?

Would transferFrom still work correctly if the allowance require check was done first instead?

Petr Hejda · Accepted Answer · 2021-03-05T00:22:38.547

I found discussion about the same topic on the OpenZeppelin GitHub Issues and the author of the code confirms the order is irrelevant in the context of ERC-20.

In the case of ERC20, the order is irrelevant because _transfer and _approve are independent and never call any other contract: they are executed atomically.

Plus he states it's important to keep the order stabilized for the case when someone wants to override the functions.

However, this is very relevant for users that intent to override _transfer or _approve with their own implementations: we should be clear about the order in which these things happen.

Source: https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2030#issuecomment-568487500

So assuming you're not using custom logic in overriden _transfer() and _approve() internal functions, the transferFrom() would still work correctly if you switched the order.

Thanks for confirming this. Since it's irrelevant, it's odd they didn't choose the ordering with the most sense semantically. ERC1155's implementation follows this pattern, however, which is nice: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155/ERC1155.sol — Leeren, Mar 05 '21 at 00:43

score 0 · Answer 2 · answered Dec 22 '21 at 07:19

0

I was actually looking for the same thing, then I found this discussion [https://forum.openzeppelin.com/t/in-erc20-tranferfrom-why-transfer-before-checking-allowance/5312/3] It gives more details and shed the light on the reasoning behind it.

answered Dec 22 '21 at 07:19

Kareem

819
9
11

In OpenZeppelin's implementation of ERC20's transferFrom, why is _transfer() called before withdrawer allowance is checked

2 Answers2