Check allowance before safeTransferFrom?

Smart Contracts
1

Using safeTransferFrom, is there any benefit of checking allowance before performing the safeTransferFrom?
Just with the safeTransferFrom it would revert is there is not enough allowance, right?
So can I remove the allowance check and save gas? Or is there a good reason to keep it?

Example with allowance:

require(token.allowance(msg.sender, address(this)) >= amount, "Insufficient allowance");
token.safeTransferFrom(msg.sender, address(this), amount);
2

It is needed to give betters details in case of missing allowance.

3

If missing allowance, just revert, as stated in the require.

AFAIK safeTransferFrom also would revert if no allowance, since the transferfrom wouldn't succeed. Since there is no specific operation other than revert, is there a point on performing the check of allowance? Or could I just ignore the first line as it doesn't add any value/security?

4

It will revert even without that require but that line can help logging. You will get Insufficient allowance error message if allowance is not enough. Removing that line you won't see that message.

5

Well. At least OZ implementation of ERC20 will output this error:
"ERC20: insufficient allowance" (https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20.sol#L343)
Right?

It will depend on the ERC-20 implementation, but most will output something like that, right? Or am I missing something?

May I safely remove it?
Or do you think of a scenario where this could be useful?

6

I am sorry but maybe I misunderstood. Where do you have this?

7

If your compiler version is >= 0.8.0 the compiled code already includes the under/overflow checks but will revert with a code (you can find a list here)

The require() is there to provide you with a clear error message in cases where a revert() will happen anyway

8

In a Contract to send funds from a user wallet to buy something using ERC-20.

9

I don't know if I understand you, but the require here has nothing to do with under/overflow. But rather with checking if there is enough allowance before a safeTransferFrom, and know if it is useful at all.

10

It does. Not having enough allowance would cause a revertinstead, assuming no other checks. This returns error 0x000...000011, which is kind of cryptic. When using a require, there is a clear error message

1 Like
11

As linked above, that's not what OpenZeppelin implementation does:

At least OZ implementation of ERC20 will output this error:
"ERC20: insufficient allowance"

12

correct. In the OZ implementations the common pattern is to find a require and then an unchecked block to optimize for gas. You don't seem to be implementing a token, by your first example. It seems that you are interacting with a token. two different things

13

I'm interacting with a token.
I'm just asking if there is any advantage on using that line. And I just get that in some tokens, I may get less information if it reverts. But no security risks.
Right?

14

that's right. if you are calling a token there is no risk in not checking. it just means that you might get an error message or an error code depending on the token implementation instead of a consistent error message

1 Like
15

Then I don't think itr is necessary. You can just use the safeTransferFrom

16

Can we call safetransferfrom before calling approval?

17

Up to the contract logic, but generally, it is expected to call approve at first, then transferFrom can be used.

18

The only reason I see to check allowance is to be able to throw a custom error

If you don't need it, you can safely remove the check

1 Like