Efficiency of the supply capping mechanism in ERC20Capped

Support Contracts
1

I am wondering about the efficiency of the implementation of the supply capping mechanism in ERC20Capped.sol.

It seems each time a transfer is made, the _beforeTokenTransfer function will be called to verify that the cap is not exceeded. Whereas it would seem more natural to inline this verification within the functions that may actually increase totalSupply. Eg in the standard case the _mint function.

Incidentally, the error messages "ERC20: mint to the zero address", "ERC20: burn from the zero address" look inverted to me, one burns (mints) to (from) the zero address.

1 Like
2

I assume that you mean this capped contract: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/ERC20Capped.sol.

Whereas it would seem more natural to inline this verification within the functions that may actually increase totalSupply .

And I assume what you mean is that for current code:

_beforeTokenTransfer(address(0), account, amount);

_totalSupply += amount;
_balances[account] += amount;

it should be:

require(totalSupply() + amount <= cap(), "ERC20Capped: cap exceeded");
_totalSupply += amount;
_balances[account] += amount;

I think it is ok for you to write your own capped contract like so, but for the OpenZeppelin team, it seems like not, cause they want to keep a flexible approach, the ERC20.sol is a basic contract and the ERC20Capped.sol is one of the many derivatives of ERC20.sol, in addition to this there are ERC20Burnable.sol, ERC20Pausable.sol and so on, so for this capped contract, you should re-write your own logic only in capped contract to achieve your purpose.

2 Likes
3

Hi @1vd21,

Thanks for raising this.

Using hooks (https://docs.openzeppelin.com/contracts/3.x/extending-contracts#using-hooks) makes for a simpler design compared with overriding _mint though I can see how that would avoid the check on transfers and burns for the from address.

I am not sure what you mean here. The check in _mint is to prevent minting to the zero address and the check in _burn is to prevent burning from the zero address.

1 Like
4

Hello @1vd21

You are right that fact that the cap protection hooks into _beforeTokenTransfer slightly increasses the cost of all transfers because of the if (from == address(0)) check.

An easy solution if to perform this check in the _mint function, has can be seen in this PR: https://github.com/OpenZeppelin/openzeppelin-contracts/pull/2524

We usually prefer not to overload function and use the hook mechanism whenever possible. This is done because child contracts might have to resolve the overloading, which might be disturbing for some users. However, in this case, overloading clearly is the most gas efficient way, and thus feels like the better way.

4 Likes
5

In the future, when you noticed design pattern that could improve gas efficiency, feel free to raise an issue on github, and we will see how we can improve that.

3 Likes