is pull over push pattern recommended for erc20 withdrawal too?

Question

I had a chance to read the pull over push pattern for transferring/withdrawing ethers, but i'm not sure if i need to apply this for erc20 transfers.

i.e.

  function buy(uint256 _someId, uint256 _price) external {      
        require(weth.allowance(msgSender(), address(this)) >= _price, 'insuffient weth');
        // more codes ...
        weth.safeTransferFrom(msgSender(), test[_someId].beneficiary, _price);
        _mint(msgSender(), tokenId);
  }

This is my current implementation where weth is transfered to beneficiary EOA right away, but if I apply pull and push pattern here, it would look like

 function buy(uint256 _someId, uint256 _price) external {      
        require(weth.allowance(msgSender(), address(this)) >= _price, 'insuffient weth');
        // more codes ...
        balance[msgSender()] += _price;
        weth.safeTransferFrom(msgSender(), **address(this)**, _price);
        _mint(msgSender(), tokenId);
  }

 function withdraw()... {
    uint amount = balance[msgSender()];

    require(amount != 0);
    require(weth.balanceOf(address(this)) >= amount);

    balance[msgSender()] = 0;

    weth.transfer(msgSender(), amount);
 }

contract receives weth first and records the balance of the erc20 deposit.

Then user would withdraw it later by contract sending directly to beneficiary EOA.

To me it seems not right to apply erc20 transfers with pull over push? What's the suggestion?

Answer 1

The pull-over-push method is used for ether transfers because by transferring ETH to a recipient you're also passing the execution to it. By transferring a token, the execution goes instead to the token contract.

So, it all depends what the token actually will do. It's clear that if a token makes a hook call to the recipient (like ERC777), we need to use the pull-over-push method for that. However these kind of tokens are rare, so a simple push is ok.

So the suggestion is to check what tokens your smart contract is going to interact with for any possible misbehavior. Fortunately, all common ERC20 tokens don't need the pull-over-push.